From 1d94e1c1cdd26fcfb4478ab3bc5e024c678b9fd6 Mon Sep 17 00:00:00 2001 From: Eli Bosley Date: Wed, 12 Aug 2026 12:30:00 -0400 Subject: [PATCH 1/5] chore(ai-review): record design forecast --- ...i-oidc-code-binding-pkce-6db58e97e7b8.json | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .limetech/ai-review-markers/codex-api-oidc-code-binding-pkce-6db58e97e7b8.json diff --git a/.limetech/ai-review-markers/codex-api-oidc-code-binding-pkce-6db58e97e7b8.json b/.limetech/ai-review-markers/codex-api-oidc-code-binding-pkce-6db58e97e7b8.json new file mode 100644 index 0000000000..00ff71348c --- /dev/null +++ b/.limetech/ai-review-markers/codex-api-oidc-code-binding-pkce-6db58e97e7b8.json @@ -0,0 +1,30 @@ +{ + "disposition": "PATCH", + "expected_structural_delta": "Add one transient verifier field across the existing state and token-exchange path, one internal provider capability flag, and focused tests; add no durable store, redirect allowlist, new runtime owner, or parallel OIDC flow.", + "minimum_design": "Extend the existing API OIDC state envelope and openid-client flow with a per-request S256 code verifier, and enable it for the built-in Unraid.net provider.", + "outcome": "Prevent authorization-code interception by binding the API OIDC client flow to the same browser transaction through PKCE.", + "reuse_decisions": [ + { + "decision": "extend", + "name": "Existing OIDC state cache", + "rationale": "Reuse the existing state cache and secure state envelope to carry the transient PKCE verifier." + }, + { + "decision": "extend", + "name": "openid-client authorization flow", + "rationale": "Reuse the existing openid-client authorization URL and token exchange APIs for S256 PKCE." + }, + { + "decision": "extend", + "name": "OIDC provider configuration", + "rationale": "Add an internal provider capability flag so only the Unraid.net provider opts into PKCE." + }, + { + "decision": "not applicable", + "name": "New persistent store or redirect allowlist", + "rationale": "PKCE is transient per authorization and the product contract permits arbitrary user-controlled callback URLs." + } + ], + "schema": "limetech.ai-review-marker.v2", + "stage": "forecast" +} From 8d909bd2d0a28cdd0e671350e8a28d9da21c95e8 Mon Sep 17 00:00:00 2001 From: Eli Bosley Date: Wed, 12 Aug 2026 12:30:20 -0400 Subject: [PATCH 2/5] fix(api): add PKCE to Unraid OIDC client --- .../auth/oidc-token-exchange.service.test.ts | 31 ++++++++++++ .../sso/auth/oidc-token-exchange.service.ts | 5 +- .../resolvers/sso/core/oidc-config.service.ts | 2 + .../resolvers/sso/core/oidc.service.test.ts | 41 +++++++++++++++- .../graph/resolvers/sso/core/oidc.service.ts | 48 +++++++++++++++---- .../sso/models/oidc-provider.model.ts | 6 +++ .../session/oidc-state-extractor.util.spec.ts | 18 +++++++ .../sso/session/oidc-state-extractor.util.ts | 2 + .../sso/session/oidc-state.service.spec.ts | 18 +++++++ .../sso/session/oidc-state.service.ts | 14 +++++- 10 files changed, 171 insertions(+), 14 deletions(-) diff --git a/api/src/unraid-api/graph/resolvers/sso/auth/oidc-token-exchange.service.test.ts b/api/src/unraid-api/graph/resolvers/sso/auth/oidc-token-exchange.service.test.ts index 44694e5159..c5c23bb50c 100644 --- a/api/src/unraid-api/graph/resolvers/sso/auth/oidc-token-exchange.service.test.ts +++ b/api/src/unraid-api/graph/resolvers/sso/auth/oidc-token-exchange.service.test.ts @@ -193,6 +193,37 @@ describe('OidcTokenExchangeService', () => { expect(client.authorizationCodeGrant).toHaveBeenCalled(); }); + it('should pass the stored PKCE verifier and exact redirect URI to the grant', async () => { + const code = 'test-code'; + const state = 'test-state'; + const redirectUri = 'https://self-hosted.example/callback'; + const codeVerifier = 'test-code-verifier'; + const mockTokens = { + access_token: 'test-access-token', + id_token: 'test-id-token', + }; + + vi.mocked(client.authorizationCodeGrant).mockResolvedValue(mockTokens as any); + + await service.exchangeCodeForTokens( + mockConfig, + mockProvider, + code, + state, + redirectUri, + undefined, + codeVerifier + ); + + const grantCall = vi.mocked(client.authorizationCodeGrant).mock.calls[0]; + const cleanUrl = grantCall[1] as URL; + const checks = grantCall[2] as client.AuthorizationCodeGrantChecks; + + expect(cleanUrl.origin + cleanUrl.pathname).toBe(redirectUri); + expect(checks.expectedState).toBe(state); + expect(checks.pkceCodeVerifier).toBe(codeVerifier); + }); + it('should handle non-string fullCallbackUrl types gracefully', async () => { const code = 'test-code'; const state = 'test-state'; diff --git a/api/src/unraid-api/graph/resolvers/sso/auth/oidc-token-exchange.service.ts b/api/src/unraid-api/graph/resolvers/sso/auth/oidc-token-exchange.service.ts index ffeae6854f..b9b4fc698e 100644 --- a/api/src/unraid-api/graph/resolvers/sso/auth/oidc-token-exchange.service.ts +++ b/api/src/unraid-api/graph/resolvers/sso/auth/oidc-token-exchange.service.ts @@ -19,7 +19,8 @@ export class OidcTokenExchangeService { code: string, state: string, redirectUri: string, - fullCallbackUrl?: string + fullCallbackUrl?: string, + codeVerifier?: string ): Promise { this.logger.debug(`Provider ${provider.id} config loaded`); this.logger.debug(`Redirect URI: ${redirectUri}`); @@ -116,12 +117,14 @@ export class OidcTokenExchangeService { const requestChecks: ExtendedGrantChecks = { expectedState: state, + ...(codeVerifier ? { pkceCodeVerifier: codeVerifier } : {}), }; // Log what we're about to send this.logger.debug(`Executing authorizationCodeGrant with:`); this.logger.debug(`- Clean URL: ${cleanUrl.href}`); this.logger.debug(`- Expected state: ${state}`); + this.logger.debug(`- PKCE enabled: ${codeVerifier ? 'Yes' : 'No'}`); this.logger.debug(`- Grant type: authorization_code`); const tokens = await client.authorizationCodeGrant(config, cleanUrl, requestChecks); diff --git a/api/src/unraid-api/graph/resolvers/sso/core/oidc-config.service.ts b/api/src/unraid-api/graph/resolvers/sso/core/oidc-config.service.ts index ee5f7fb228..deee4335af 100644 --- a/api/src/unraid-api/graph/resolvers/sso/core/oidc-config.service.ts +++ b/api/src/unraid-api/graph/resolvers/sso/core/oidc-config.service.ts @@ -122,6 +122,7 @@ export class OidcConfigPersistence extends ConfigFilePersister { clientId: 'CONNECT_SERVER_SSO', issuer: 'https://account.unraid.net', scopes: ['openid', 'profile', 'email'], + usePkce: true, authorizationRules: [], buttonText: 'Login With Unraid.net', buttonIcon: @@ -286,6 +287,7 @@ export class OidcConfigPersistence extends ConfigFilePersister { name: provider.name, clientId: provider.clientId, clientSecret: provider.clientSecret, + usePkce: provider.usePkce, issuer: provider.issuer, authorizationEndpoint: provider.authorizationEndpoint, tokenEndpoint: provider.tokenEndpoint, diff --git a/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.test.ts b/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.test.ts index 14ca9f1b78..1a8261606c 100644 --- a/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.test.ts +++ b/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.test.ts @@ -18,6 +18,8 @@ import { OidcStateService } from '@app/unraid-api/graph/resolvers/sso/session/oi // Mock openid-client vi.mock('openid-client', () => ({ + randomPKCECodeVerifier: vi.fn(() => 'test-code-verifier'), + calculatePKCECodeChallenge: vi.fn(async (verifier: string) => `challenge-${verifier}`), buildAuthorizationUrl: vi.fn((config, params) => { const url = new URL(config.serverMetadata().authorization_endpoint); Object.entries(params).forEach(([key, value]) => { @@ -193,6 +195,34 @@ describe('OidcService Integration', () => { expect(url).toContain('https://discovery.example.com/authorize'); }); + it('should add S256 PKCE parameters for opted-in providers', async () => { + const provider: OidcProvider = { + id: 'pkce-provider', + name: 'PKCE Provider', + clientId: 'test-client-id', + authorizationEndpoint: 'https://custom.example.com/auth', + scopes: ['openid'], + usePkce: true, + authorizationRules: [], + }; + + oidcConfig.getProvider.mockResolvedValue(provider); + + const url = await service.getAuthorizationUrl({ + providerId: 'pkce-provider', + state: 'client-state-123', + requestOrigin: 'https://example.com', + requestOriginInfo: { + protocol: 'https', + host: 'example.com', + }, + }); + + const urlObj = new URL(url); + expect(urlObj.searchParams.get('code_challenge')).toBe('challenge-test-code-verifier'); + expect(urlObj.searchParams.get('code_challenge_method')).toBe('S256'); + }); + it('should throw when provider not found', async () => { oidcConfig.getProvider.mockResolvedValue(null); @@ -254,6 +284,7 @@ describe('OidcService Integration', () => { originalState: 'original-state', clientState: 'original-state', redirectUri: 'https://example.com/callback', + codeVerifier: 'test-code-verifier', } ); @@ -269,7 +300,15 @@ describe('OidcService Integration', () => { const token = await service.handleCallback(params); expect(token).toBe('padded-token-123'); - expect(tokenExchangeService.exchangeCodeForTokens).toHaveBeenCalled(); + expect(tokenExchangeService.exchangeCodeForTokens).toHaveBeenCalledWith( + mockConfig, + provider, + 'auth-code-123', + 'original-state', + 'https://example.com/callback', + params.fullCallbackUrl, + 'test-code-verifier' + ); expect(claimsService.parseIdToken).toHaveBeenCalledWith('id.token.here'); expect(claimsService.validateClaims).toHaveBeenCalledWith(mockClaims); expect(authorizationService.checkAuthorization).toHaveBeenCalledWith(provider, mockClaims); diff --git a/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.ts b/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.ts index ca1432cf6b..5afc242764 100644 --- a/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.ts +++ b/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.ts @@ -65,8 +65,18 @@ export class OidcService { this.logger.debug(`Using redirect URI for authorization: ${redirectUri}`); this.logger.debug(`Request origin was: ${requestOrigin}`); - // Generate secure state with cryptographic signature, including redirect URI - const secureState = await this.stateService.generateSecureState(providerId, state, redirectUri); + let codeVerifier: string | undefined; + let codeChallenge: string | undefined; + if (provider.usePkce === true) { + codeVerifier = client.randomPKCECodeVerifier(); + codeChallenge = await client.calculatePKCECodeChallenge(codeVerifier); + } + + // Generate secure state with cryptographic signature, including the + // exact redirect URI and the transient PKCE verifier. + const secureState = codeVerifier + ? await this.stateService.generateSecureState(providerId, state, redirectUri, codeVerifier) + : await this.stateService.generateSecureState(providerId, state, redirectUri); // Build authorization URL if (provider.authorizationEndpoint) { @@ -79,6 +89,10 @@ export class OidcService { authUrl.searchParams.set('scope', provider.scopes.join(' ')); authUrl.searchParams.set('state', secureState); authUrl.searchParams.set('response_type', 'code'); + if (codeChallenge) { + authUrl.searchParams.set('code_challenge', codeChallenge); + authUrl.searchParams.set('code_challenge_method', 'S256'); + } this.logger.debug(`Built authorization URL for provider ${provider.id}`); this.logger.debug( @@ -96,6 +110,10 @@ export class OidcService { state: secureState, response_type: 'code', }; + if (codeChallenge) { + parameters.code_challenge = codeChallenge; + parameters.code_challenge_method = 'S256'; + } // For HTTP endpoints, we need to call allowInsecureRequests on the config if (provider.issuer) { @@ -196,14 +214,24 @@ export class OidcService { this.logger.debug(`Client state extracted: ${originalState}`); // Use the token exchange service - const tokens = await this.tokenExchangeService.exchangeCodeForTokens( - config, - provider, - code, - originalState, - redirectUri, - fullCallbackUrl - ); + const tokens = stateInfo.codeVerifier + ? await this.tokenExchangeService.exchangeCodeForTokens( + config, + provider, + code, + originalState, + redirectUri, + fullCallbackUrl, + stateInfo.codeVerifier + ) + : await this.tokenExchangeService.exchangeCodeForTokens( + config, + provider, + code, + originalState, + redirectUri, + fullCallbackUrl + ); // Parse ID token to get user info const claims = this.claimsService.parseIdToken(tokens.id_token); diff --git a/api/src/unraid-api/graph/resolvers/sso/models/oidc-provider.model.ts b/api/src/unraid-api/graph/resolvers/sso/models/oidc-provider.model.ts index 10b0e9e39b..5f56493b14 100644 --- a/api/src/unraid-api/graph/resolvers/sso/models/oidc-provider.model.ts +++ b/api/src/unraid-api/graph/resolvers/sso/models/oidc-provider.model.ts @@ -4,6 +4,7 @@ import { PrefixedID } from '@unraid/shared/prefixed-id-scalar.js'; import { Type } from 'class-transformer'; import { IsArray, + IsBoolean, IsEnum, IsNotEmpty, IsOptional, @@ -77,6 +78,11 @@ export class OidcProvider { @IsOptional() clientSecret?: string; + // Internal provider capability; the built-in Unraid.net client opts into PKCE. + @IsBoolean() + @IsOptional() + usePkce?: boolean; + @Field(() => String, { description: 'OIDC issuer URL (e.g., https://accounts.google.com). Required for auto-discovery via /.well-known/openid-configuration', diff --git a/api/src/unraid-api/graph/resolvers/sso/session/oidc-state-extractor.util.spec.ts b/api/src/unraid-api/graph/resolvers/sso/session/oidc-state-extractor.util.spec.ts index 9769a88bd5..bcea233cf2 100644 --- a/api/src/unraid-api/graph/resolvers/sso/session/oidc-state-extractor.util.spec.ts +++ b/api/src/unraid-api/graph/resolvers/sso/session/oidc-state-extractor.util.spec.ts @@ -57,6 +57,24 @@ describe('OidcStateExtractor', () => { expect(result.redirectUri).toBe(redirectUri); }); + it('should extract the PKCE verifier from validated state', async () => { + const providerId = 'test-provider'; + const clientState = 'client-state-123'; + const redirectUri = 'https://example.com/callback'; + const codeVerifier = 'pkce-code-verifier'; + + const state = await stateService.generateSecureState( + providerId, + clientState, + redirectUri, + codeVerifier + ); + + const result = await OidcStateExtractor.extractAndValidateState(state, stateService); + + expect(result.codeVerifier).toBe(codeVerifier); + }); + it('should extract and validate a valid state without redirectUri', async () => { const providerId = 'test-provider'; const clientState = 'client-state-123'; diff --git a/api/src/unraid-api/graph/resolvers/sso/session/oidc-state-extractor.util.ts b/api/src/unraid-api/graph/resolvers/sso/session/oidc-state-extractor.util.ts index 9b0ae5b61e..cf39986ef2 100644 --- a/api/src/unraid-api/graph/resolvers/sso/session/oidc-state-extractor.util.ts +++ b/api/src/unraid-api/graph/resolvers/sso/session/oidc-state-extractor.util.ts @@ -7,6 +7,7 @@ export interface StateExtractionResult { originalState: string; clientState?: string; redirectUri?: string; + codeVerifier?: string; } /** @@ -55,6 +56,7 @@ export class OidcStateExtractor { originalState: state, clientState: stateValidation.clientState, redirectUri: stateValidation.redirectUri, + codeVerifier: stateValidation.codeVerifier, }; } } diff --git a/api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.spec.ts b/api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.spec.ts index 6c65a3e67e..65ee1850ff 100644 --- a/api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.spec.ts +++ b/api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.spec.ts @@ -81,6 +81,24 @@ describe('OidcStateService', () => { expect(validation.clientState).toBe(clientState); expect(validation.redirectUri).toBe(redirectUri); }); + + it('should store and return the PKCE verifier with the state', async () => { + const providerId = 'test-provider'; + const clientState = 'client-state-123'; + const redirectUri = 'https://example.com/callback'; + const codeVerifier = 'pkce-code-verifier'; + + const state = await service.generateSecureState( + providerId, + clientState, + redirectUri, + codeVerifier + ); + const validation = await service.validateSecureState(state, providerId); + + expect(validation.isValid).toBe(true); + expect(validation.codeVerifier).toBe(codeVerifier); + }); }); describe('validateSecureState', () => { diff --git a/api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.ts b/api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.ts index f0fb001ac4..cc9ca332ad 100644 --- a/api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.ts +++ b/api/src/unraid-api/graph/resolvers/sso/session/oidc-state.service.ts @@ -8,6 +8,7 @@ interface StateData { timestamp: number; providerId: string; redirectUri?: string; + codeVerifier?: string; } @Injectable() @@ -33,7 +34,8 @@ export class OidcStateService { async generateSecureState( providerId: string, clientState: string, - redirectUri?: string + redirectUri?: string, + codeVerifier?: string ): Promise { const nonce = crypto.randomBytes(16).toString('hex'); const timestamp = Date.now(); @@ -45,6 +47,7 @@ export class OidcStateService { timestamp, providerId, redirectUri, + codeVerifier, }; // Store in cache with TTL (in milliseconds for cache-manager v7) @@ -78,7 +81,13 @@ export class OidcStateService { async validateSecureState( state: string, expectedProviderId: string - ): Promise<{ isValid: boolean; clientState?: string; redirectUri?: string; error?: string }> { + ): Promise<{ + isValid: boolean; + clientState?: string; + redirectUri?: string; + codeVerifier?: string; + error?: string; + }> { try { // Extract provider ID and signed state const parts = state.split(':'); @@ -180,6 +189,7 @@ export class OidcStateService { isValid: true, clientState: cachedState.clientState, redirectUri: cachedState.redirectUri, + codeVerifier: cachedState.codeVerifier, }; } catch (error) { this.logger.error( From df1b8f1f56eb889cc7d66eb13c40d02c3eabc149 Mon Sep 17 00:00:00 2001 From: Eli Bosley Date: Wed, 12 Aug 2026 13:14:21 -0400 Subject: [PATCH 3/5] test(api): cover discovered OIDC PKCE --- .../graph/resolvers/sso/core/oidc.service.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.test.ts b/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.test.ts index 1a8261606c..1b6ec445e4 100644 --- a/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.test.ts +++ b/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.test.ts @@ -166,6 +166,7 @@ describe('OidcService Integration', () => { clientId: 'test-client-id', issuer: 'https://discovery.example.com', scopes: ['openid'], + usePkce: true, authorizationRules: [], }; @@ -192,7 +193,11 @@ describe('OidcService Integration', () => { const url = await service.getAuthorizationUrl(params); expect(clientConfigService.getOrCreateConfig).toHaveBeenCalledWith(provider); - expect(url).toContain('https://discovery.example.com/authorize'); + const urlObj = new URL(url); + expect(urlObj.origin).toBe('https://discovery.example.com'); + expect(urlObj.pathname).toBe('/authorize'); + expect(urlObj.searchParams.get('code_challenge')).toBe('challenge-test-code-verifier'); + expect(urlObj.searchParams.get('code_challenge_method')).toBe('S256'); }); it('should add S256 PKCE parameters for opted-in providers', async () => { From a147b7c0d78049e01643435afa77677cd623f46e Mon Sep 17 00:00:00 2001 From: Eli Bosley Date: Wed, 12 Aug 2026 13:30:29 -0400 Subject: [PATCH 4/5] test(api): use valid PKCE fixtures --- .../auth/oidc-token-exchange.service.test.ts | 2 +- .../resolvers/sso/core/oidc.service.test.ts | 17 +++++++++++------ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/api/src/unraid-api/graph/resolvers/sso/auth/oidc-token-exchange.service.test.ts b/api/src/unraid-api/graph/resolvers/sso/auth/oidc-token-exchange.service.test.ts index c5c23bb50c..6084ea509b 100644 --- a/api/src/unraid-api/graph/resolvers/sso/auth/oidc-token-exchange.service.test.ts +++ b/api/src/unraid-api/graph/resolvers/sso/auth/oidc-token-exchange.service.test.ts @@ -197,7 +197,7 @@ describe('OidcTokenExchangeService', () => { const code = 'test-code'; const state = 'test-state'; const redirectUri = 'https://self-hosted.example/callback'; - const codeVerifier = 'test-code-verifier'; + const codeVerifier = 'test-code-verifier'.padEnd(43, '-'); const mockTokens = { access_token: 'test-access-token', id_token: 'test-id-token', diff --git a/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.test.ts b/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.test.ts index 1b6ec445e4..1e8517605c 100644 --- a/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.test.ts +++ b/api/src/unraid-api/graph/resolvers/sso/core/oidc.service.test.ts @@ -16,10 +16,15 @@ import { OidcProvider } from '@app/unraid-api/graph/resolvers/sso/models/oidc-pr import { OidcSessionService } from '@app/unraid-api/graph/resolvers/sso/session/oidc-session.service.js'; import { OidcStateService } from '@app/unraid-api/graph/resolvers/sso/session/oidc-state.service.js'; +const pkceFixtures = vi.hoisted(() => ({ + verifier: 'test-code-verifier-------------------------', + challenge: 'He8Y2ddQw0a5vkMogWuyUbJoC8aUAhgQGdMOelEFFic', +})); + // Mock openid-client vi.mock('openid-client', () => ({ - randomPKCECodeVerifier: vi.fn(() => 'test-code-verifier'), - calculatePKCECodeChallenge: vi.fn(async (verifier: string) => `challenge-${verifier}`), + randomPKCECodeVerifier: vi.fn(() => pkceFixtures.verifier), + calculatePKCECodeChallenge: vi.fn(async () => pkceFixtures.challenge), buildAuthorizationUrl: vi.fn((config, params) => { const url = new URL(config.serverMetadata().authorization_endpoint); Object.entries(params).forEach(([key, value]) => { @@ -196,7 +201,7 @@ describe('OidcService Integration', () => { const urlObj = new URL(url); expect(urlObj.origin).toBe('https://discovery.example.com'); expect(urlObj.pathname).toBe('/authorize'); - expect(urlObj.searchParams.get('code_challenge')).toBe('challenge-test-code-verifier'); + expect(urlObj.searchParams.get('code_challenge')).toBe(pkceFixtures.challenge); expect(urlObj.searchParams.get('code_challenge_method')).toBe('S256'); }); @@ -224,7 +229,7 @@ describe('OidcService Integration', () => { }); const urlObj = new URL(url); - expect(urlObj.searchParams.get('code_challenge')).toBe('challenge-test-code-verifier'); + expect(urlObj.searchParams.get('code_challenge')).toBe(pkceFixtures.challenge); expect(urlObj.searchParams.get('code_challenge_method')).toBe('S256'); }); @@ -289,7 +294,7 @@ describe('OidcService Integration', () => { originalState: 'original-state', clientState: 'original-state', redirectUri: 'https://example.com/callback', - codeVerifier: 'test-code-verifier', + codeVerifier: pkceFixtures.verifier, } ); @@ -312,7 +317,7 @@ describe('OidcService Integration', () => { 'original-state', 'https://example.com/callback', params.fullCallbackUrl, - 'test-code-verifier' + pkceFixtures.verifier ); expect(claimsService.parseIdToken).toHaveBeenCalledWith('id.token.here'); expect(claimsService.validateClaims).toHaveBeenCalledWith(mockClaims); From 454bad96f7505621f82d3fff15a82d8103e75d96 Mon Sep 17 00:00:00 2001 From: Eli Bosley Date: Wed, 12 Aug 2026 14:48:21 -0400 Subject: [PATCH 5/5] chore(ai-review): record final review receipt --- ...i-oidc-code-binding-pkce-6db58e97e7b8.json | 30 +++---------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/.limetech/ai-review-markers/codex-api-oidc-code-binding-pkce-6db58e97e7b8.json b/.limetech/ai-review-markers/codex-api-oidc-code-binding-pkce-6db58e97e7b8.json index 00ff71348c..40d1008493 100644 --- a/.limetech/ai-review-markers/codex-api-oidc-code-binding-pkce-6db58e97e7b8.json +++ b/.limetech/ai-review-markers/codex-api-oidc-code-binding-pkce-6db58e97e7b8.json @@ -1,30 +1,8 @@ { "disposition": "PATCH", - "expected_structural_delta": "Add one transient verifier field across the existing state and token-exchange path, one internal provider capability flag, and focused tests; add no durable store, redirect allowlist, new runtime owner, or parallel OIDC flow.", - "minimum_design": "Extend the existing API OIDC state envelope and openid-client flow with a per-request S256 code verifier, and enable it for the built-in Unraid.net provider.", - "outcome": "Prevent authorization-code interception by binding the API OIDC client flow to the same browser transaction through PKCE.", - "reuse_decisions": [ - { - "decision": "extend", - "name": "Existing OIDC state cache", - "rationale": "Reuse the existing state cache and secure state envelope to carry the transient PKCE verifier." - }, - { - "decision": "extend", - "name": "openid-client authorization flow", - "rationale": "Reuse the existing openid-client authorization URL and token exchange APIs for S256 PKCE." - }, - { - "decision": "extend", - "name": "OIDC provider configuration", - "rationale": "Add an internal provider capability flag so only the Unraid.net provider opts into PKCE." - }, - { - "decision": "not applicable", - "name": "New persistent store or redirect allowlist", - "rationale": "PKCE is transient per authorization and the product contract permits arbitrary user-controlled callback URLs." - } - ], + "forecast_commit": "1d94e1c1cdd26fcfb4478ab3bc5e024c678b9fd6", + "reviewed_sha": "a147b7c0d78049e01643435afa77677cd623f46e", "schema": "limetech.ai-review-marker.v2", - "stage": "forecast" + "stage": "final", + "unresolved_proportionality_findings": [] }