From 8d7b3875455d00152bcdca5ab45cffd15404c572 Mon Sep 17 00:00:00 2001 From: Allen Zhou <46854522+allenzhou101@users.noreply.github.com> Date: Thu, 27 Feb 2025 12:41:14 -0800 Subject: [PATCH] Clean up pkce skip check for proxy providers --- src/server/auth/handlers/token.ts | 10 ++++++---- src/server/auth/proxyProvider.ts | 10 ++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/server/auth/handlers/token.ts b/src/server/auth/handlers/token.ts index 3a29bb55..acf2c0ab 100644 --- a/src/server/auth/handlers/token.ts +++ b/src/server/auth/handlers/token.ts @@ -91,16 +91,18 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand const { code, code_verifier } = parseResult.data; - // Skip local PKCE verification for proxy providers since the code_challenge is stored on the upstream server. - // The code_verifier will be passed to the upstream server during token exchange. - if (!(provider instanceof ProxyOAuthServerProvider)) { + const skipLocalPkceValidation = provider instanceof ProxyOAuthServerProvider ? provider.skipLocalPkceValidation : false; + + // Perform local PKCE validation unless explicitly delegated (e.g. by proxy provider) + if (!skipLocalPkceValidation) { const codeChallenge = await provider.challengeForAuthorizationCode(client, code); if (!(await verifyChallenge(code_verifier, codeChallenge))) { throw new InvalidGrantError("code_verifier does not match the challenge"); } } - const tokens = await provider.exchangeAuthorizationCode(client, code, code_verifier); + // Passes the code_verifier to the provider if PKCE validation didn't occur locally + const tokens = await provider.exchangeAuthorizationCode(client, code, skipLocalPkceValidation ? code_verifier : undefined); res.status(200).json(tokens); break; } diff --git a/src/server/auth/proxyProvider.ts b/src/server/auth/proxyProvider.ts index d9dd1b37..3fbff54b 100644 --- a/src/server/auth/proxyProvider.ts +++ b/src/server/auth/proxyProvider.ts @@ -43,6 +43,16 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider { protected readonly _endpoints: ProxyEndpoints; protected readonly _verifyAccessToken: (token: string) => Promise; protected readonly _getClient: (clientId: string) => Promise; + + /** + * Always true for proxy providers since PKCE validation happens at the upstream server. + * Can consider adding to the base OAuthServerProvider interface if it becomes useful elsewhere. + * This ensures that: + * 1. We skip local PKCE validation (which would fail since we don't store challenges) + * 2. The code_verifier is still passed through to the upstream server + * 3. The upstream server performs the actual PKCE validation + */ + readonly skipLocalPkceValidation = true; revokeToken?: ( client: OAuthClientInformationFull,