Harden OAuth PKCE session consistency

Author: baggiiiie

packages/core/sdk/src/oauth-helpers.test.ts

@@ -273,6 +273,10 @@ describe("buildAuthorizationUrl", () => {
         ...withoutPkce,
         authorizationUrl:
           "https://example.com/authorize?code_challenge=stale&code_challenge_method=S256",
+        extraParams: {
+          code_challenge: "also-stale",
+          code_challenge_method: "plain",
+        },
       }),
     );
     expect(url.searchParams.has("code_challenge_method")).toBe(false);

packages/core/sdk/src/oauth-helpers.ts

@@ -252,9 +252,6 @@ export const buildAuthorizationUrl = (input: BuildAuthorizationUrlInput): string
   if (input.codeChallenge) {
     url.searchParams.set("code_challenge_method", "S256");
     url.searchParams.set("code_challenge", input.codeChallenge);
-  } else {
-    url.searchParams.delete("code_challenge_method");
-    url.searchParams.delete("code_challenge");
   }
   if (input.resource) {
     url.searchParams.set("resource", input.resource);
@@ -264,6 +261,12 @@ export const buildAuthorizationUrl = (input: BuildAuthorizationUrlInput): string
       url.searchParams.set(k, v);
     }
   }
+  // When this flow does not use PKCE, configured endpoint or provider-extra
+  // parameters must not reintroduce a challenge without a persisted verifier.
+  if (!input.codeChallenge) {
+    url.searchParams.delete("code_challenge_method");
+    url.searchParams.delete("code_challenge");
+  }
   return url.toString();
 };
 

packages/core/sdk/src/oauth-service.ts

@@ -2236,10 +2236,10 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
         }
       }
 
-      const usePkce = shouldUsePkce(client.authorizationUrl, client.clientSecret);
+      const requiresPkce = shouldUsePkce(client.authorizationUrl, client.clientSecret);
       // Every authorization-code flow except LinkedIn's confidential web flow
       // requires the verifier minted by `start`. Missing one is a corrupt row.
-      if (usePkce && session.pkceVerifier == null) {
+      if (requiresPkce && session.pkceVerifier == null) {
         return yield* new OAuthCompleteError({
           message: `OAuth session ${input.state} is missing its PKCE code verifier; restart the flow.`,
           restartRequired: true,
@@ -2261,7 +2261,9 @@ export const makeOAuthService = (deps: OAuthServiceDeps): OAuthService => {
         clientId: client.clientId,
         clientSecret: client.clientSecret,
         redirectUrl: session.redirectUrl,
-        codeVerifier: usePkce ? (session.pkceVerifier ?? undefined) : undefined,
+        // The persisted verifier records the request that actually started.
+        // Keep using it if client settings change while that request is open.
+        codeVerifier: session.pkceVerifier ?? undefined,
         code: input.code,
         clientAuth: client.tokenEndpointAuthMethod,
         requestFormat: client.tokenRequestFormat,