-
-
Notifications
You must be signed in to change notification settings - Fork 0
release: promote OAuth consent error handling and automated test teardown to main #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
775cc83
0df3a5d
9443ff2
b4c6179
5a0b5bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,6 +5,10 @@ import { FetchWrapperComponent } from '@gitroom/helpers/utils/custom.fetch'; | |||||||||||||||||||||||||||||||||||||
| import { deleteDialog } from '@gitroom/react/helpers/delete.dialog'; | ||||||||||||||||||||||||||||||||||||||
| import { useReturnUrl } from '@gitroom/frontend/app/(app)/auth/return.url.component'; | ||||||||||||||||||||||||||||||||||||||
| import { useVariables } from '@gitroom/react/helpers/variable.context'; | ||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||
| shouldHandleGlobalLogout, | ||||||||||||||||||||||||||||||||||||||
| shouldPreserveOAuthConsentUnauthorized, | ||||||||||||||||||||||||||||||||||||||
| } from './oauth-consent-unauthorized'; | ||||||||||||||||||||||||||||||||||||||
| export default function LayoutContext(params: { children: ReactNode }) { | ||||||||||||||||||||||||||||||||||||||
| if (params?.children) { | ||||||||||||||||||||||||||||||||||||||
| // eslint-disable-next-line react/no-children-prop | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -42,6 +46,15 @@ function LayoutContextInner(params: { children: ReactNode }) { | |||||||||||||||||||||||||||||||||||||
| response?.headers?.get('Impersonate'); | ||||||||||||||||||||||||||||||||||||||
| const logout = | ||||||||||||||||||||||||||||||||||||||
| response?.headers?.get('logout') || response?.headers?.get('Logout'); | ||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||
| shouldPreserveOAuthConsentUnauthorized( | ||||||||||||||||||||||||||||||||||||||
| url, | ||||||||||||||||||||||||||||||||||||||
| options.method, | ||||||||||||||||||||||||||||||||||||||
| response.status | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| if (headerAuth) { | ||||||||||||||||||||||||||||||||||||||
| setCookie('auth', headerAuth, 365); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -51,7 +64,15 @@ function LayoutContextInner(params: { children: ReactNode }) { | |||||||||||||||||||||||||||||||||||||
| if (impersonate) { | ||||||||||||||||||||||||||||||||||||||
| setCookie('impersonate', impersonate, 365); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| if (logout && !isSecured) { | ||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||
| shouldHandleGlobalLogout( | ||||||||||||||||||||||||||||||||||||||
| url, | ||||||||||||||||||||||||||||||||||||||
| options.method, | ||||||||||||||||||||||||||||||||||||||
| response.status, | ||||||||||||||||||||||||||||||||||||||
| Boolean(logout) | ||||||||||||||||||||||||||||||||||||||
| ) && | ||||||||||||||||||||||||||||||||||||||
| !isSecured | ||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+67
to
+75
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Accessing
Suggested change
|
||||||||||||||||||||||||||||||||||||||
| setCookie('auth', '', -10); | ||||||||||||||||||||||||||||||||||||||
| setCookie('showorg', '', -10); | ||||||||||||||||||||||||||||||||||||||
| setCookie('impersonate', '', -10); | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -80,7 +101,15 @@ function LayoutContextInner(params: { children: ReactNode }) { | |||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if (response.status === 401 || response?.headers?.get('logout')) { | ||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||
| response.status === 401 || | ||||||||||||||||||||||||||||||||||||||
| shouldHandleGlobalLogout( | ||||||||||||||||||||||||||||||||||||||
| url, | ||||||||||||||||||||||||||||||||||||||
| options.method, | ||||||||||||||||||||||||||||||||||||||
| response.status, | ||||||||||||||||||||||||||||||||||||||
| Boolean(logout) | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+104
to
+112
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Accessing
Suggested change
|
||||||||||||||||||||||||||||||||||||||
| if (!isSecured) { | ||||||||||||||||||||||||||||||||||||||
| setCookie('auth', '', -10); | ||||||||||||||||||||||||||||||||||||||
| setCookie('showorg', '', -10); | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -93,8 +122,7 @@ function LayoutContextInner(params: { children: ReactNode }) { | |||||||||||||||||||||||||||||||||||||
| await deleteDialog( | ||||||||||||||||||||||||||||||||||||||
| 'You are currently on trial, in order to use the feature you must finish the trial', | ||||||||||||||||||||||||||||||||||||||
| 'Finish the trial, charge me now', | ||||||||||||||||||||||||||||||||||||||
| 'Trial', | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| 'Trial' | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||
| window.open('/billing?finishTrial=true', '_blank'); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| export function shouldPreserveOAuthConsentUnauthorized( | ||
| url: string, | ||
| method: string | undefined, | ||
| status: number | ||
| ) { | ||
| if (status !== 401 || method?.toUpperCase() !== 'POST') return false; | ||
| try { | ||
| return ( | ||
| new URL(url, 'https://oauth.invalid').pathname === '/oauth/authorize' | ||
| ); | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| export function shouldHandleGlobalLogout( | ||
| url: string, | ||
| method: string | undefined, | ||
| status: number, | ||
| hasLogoutHeader: boolean | ||
| ) { | ||
| return ( | ||
| hasLogoutHeader && | ||
| !shouldPreserveOAuthConsentUnauthorized(url, method, status) | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| <# | ||
| .SYNOPSIS | ||
| Automated integration test runner with disposable Docker test containers and guaranteed teardown. | ||
| .DESCRIPTION | ||
| 1. Spawns isolated PostgreSQL and Redis test containers on random high ports | ||
| 2. Runs Prisma migrations and executes Jest bootstrap integration tests | ||
| 3. Guarantees cleanup (docker rm -f) of all test containers in a finally block | ||
| .EXAMPLE | ||
| .\scripts\test-integration.ps1 | ||
| #> | ||
|
|
||
| [CmdletBinding()] | ||
| param ( | ||
| [Parameter()] | ||
| [int]$PgPort = 15491, | ||
|
|
||
| [Parameter()] | ||
| [int]$RedisPort = 16391, | ||
|
|
||
| [Parameter()] | ||
| [switch]$KeepContainers | ||
| ) | ||
|
|
||
| $ErrorActionPreference = "Stop" | ||
|
|
||
| $Timestamp = Get-Date -Format "yyyyMMddHHmmss" | ||
| $PgContainer = "crove-test-pg-$Timestamp" | ||
| $RedisContainer = "crove-test-redis-$Timestamp" | ||
|
|
||
| Write-Host "==========================================================" -ForegroundColor Cyan | ||
| Write-Host " CROVE POST - INTEGRATION TEST RUNNER (AUTO-TEARDOWN) " -ForegroundColor Yellow | ||
| Write-Host "==========================================================" -ForegroundColor Cyan | ||
|
|
||
| try { | ||
| Write-Host "`n[1/4] Starting disposable test containers..." -ForegroundColor Green | ||
| docker run -d --name $PgContainer -p "127.0.0.1:${PgPort}:5432" -e POSTGRES_PASSWORD=postiz-password -e POSTGRES_USER=postiz-user -e POSTGRES_DB=postiz-db-local postgres:17-alpine | Out-Null | ||
| docker run -d --name $RedisContainer -p "127.0.0.1:${RedisPort}:6379" redis:7.2 | Out-Null | ||
|
Comment on lines
+36
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In PowerShell, external commands (like |
||
|
|
||
| Write-Host "Waiting for database readiness on port ${PgPort}..." -ForegroundColor DarkGray | ||
| $attempts = 0 | ||
| $ready = $false | ||
| while ($attempts -lt 30 -and -not $ready) { | ||
| Start-Sleep -Seconds 1 | ||
| $res = docker exec $PgContainer pg_isready -U postiz-user -d postiz-db-local 2>&1 | ||
| if ($LASTEXITCODE -eq 0) { | ||
| $ready = $true | ||
| } | ||
| $attempts++ | ||
| } | ||
|
|
||
| if (-not $ready) { | ||
| throw "PostgreSQL test container failed to become healthy within 30 seconds." | ||
| } | ||
| Write-Host "-> Test containers ready: $PgContainer (port $PgPort), $RedisContainer (port $RedisPort)" -ForegroundColor Green | ||
|
|
||
| # Set temporary environment variables for integration tests | ||
| $env:DATABASE_URL = "postgresql://postiz-user:postiz-password@127.0.0.1:${PgPort}/postiz-db-local" | ||
| $env:DATABASE_DIRECT_URL = "postgresql://postiz-user:postiz-password@127.0.0.1:${PgPort}/postiz-db-local" | ||
| $env:REDIS_URL = "redis://127.0.0.1:${RedisPort}" | ||
| $env:JWT_SECRET = "test-jwt-secret-key-32-chars-minimum-length-ok" | ||
|
|
||
| Write-Host "`n[2/4] Pushing Prisma schema to test database..." -ForegroundColor Green | ||
| pnpm dlx prisma@6.5.0 db push --accept-data-loss --schema ./libraries/nestjs-libraries/src/database/prisma/schema.prisma --skip-generate | ||
| if ($LASTEXITCODE -ne 0) { | ||
| throw "Prisma db push to test database failed." | ||
| } | ||
|
|
||
| Write-Host "`n[3/4] Running Jest integration test suite..." -ForegroundColor Green | ||
| pnpm exec jest --config tests/bootstrap.jest.cjs --runInBand --no-cache | ||
| if ($LASTEXITCODE -ne 0) { | ||
| throw "Integration tests failed." | ||
| } | ||
|
|
||
| Write-Host "`n[4/4] All integration tests PASSED successfully!" -ForegroundColor Green | ||
|
|
||
| } catch { | ||
| Write-Error "Test execution failed: $_" | ||
| } finally { | ||
| if (-not $KeepContainers) { | ||
| Write-Host "`n[Teardown] Cleaning up disposable test containers..." -ForegroundColor DarkYellow | ||
| docker rm -f $PgContainer 2>$null | Out-Null | ||
| docker rm -f $RedisContainer 2>$null | Out-Null | ||
| Write-Host "-> Successfully removed test containers: $PgContainer, $RedisContainer" -ForegroundColor DarkGray | ||
| } else { | ||
| Write-Host "`n[Notice] Preserving test containers (-KeepContainers specified)." -ForegroundColor Yellow | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
optionsparameter inafterRequestcan beundefinedornullif the fetch wrapper is called without options (e.g., for simple GET requests). Accessingoptions.methoddirectly will throw aTypeError: Cannot read properties of undefined (reading 'method')and crash the application layout. Use optional chaining (options?.method) to safely access the method.