Skip to content

Commit

Permalink
Clean up pkce skip check for proxy providers
Browse files Browse the repository at this point in the history
  • Loading branch information
allenzhou101 committed Feb 27, 2025
1 parent 89c4ec3 commit da8bb7e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
10 changes: 6 additions & 4 deletions src/server/auth/handlers/token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
10 changes: 10 additions & 0 deletions src/server/auth/proxyProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
protected readonly _endpoints: ProxyEndpoints;
protected readonly _verifyAccessToken: (token: string) => Promise<AuthInfo>;
protected readonly _getClient: (clientId: string) => Promise<OAuthClientInformationFull | undefined>;

/**
* 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,
Expand Down

0 comments on commit da8bb7e

Please sign in to comment.