Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/remove-expired-token-retry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': patch
---

Skip `expired_token` retry flow when Session Minter is enabled. When `sessionMinter` is on, the token is sent in the POST body, so the retry-with-expired-token fallback is unnecessary. The retry flow is preserved for non-Session Minter mode.
11 changes: 7 additions & 4 deletions packages/clerk-js/src/core/resources/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -490,16 +490,19 @@ export class Session extends BaseResource implements SessionResource {
organizationId: organizationId ?? null,
...(sessionMinterEnabled && this.lastActiveToken ? { token: this.lastActiveToken.getRawString() } : {}),
};
const lastActiveToken = this.lastActiveToken?.getRawString();

const tokenResolver = Token.create(path, params, skipCache ? { debug: 'skip_cache' } : undefined).catch(e => {
if (sessionMinterEnabled) {
// Session Minter sends the token in the body, no expired_token retry needed
return Token.create(path, params, skipCache ? { debug: 'skip_cache' } : undefined);
}

const lastActiveToken = this.lastActiveToken?.getRawString();
return Token.create(path, params, skipCache ? { debug: 'skip_cache' } : undefined).catch(e => {
if (MissingExpiredTokenError.is(e) && lastActiveToken) {
return Token.create(path, { ...params }, { expired_token: lastActiveToken });
}
Comment on lines +494 to 503
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Remove expired_token retry path entirely to prevent JWT query-string leakage

Line 500-503 still retries with expired_token in the URL query. This keeps the exact leakage risk (logs/proxies/referrers) this PR is intended to eliminate. Also, gating body token by sessionMinterEnabled (Line 491) conflicts with the objective that token is now sent in POST body for /tokens.

Proposed fix
@@
 import {
   ClerkOfflineError,
   ClerkRuntimeError,
   ClerkWebAuthnError,
   is4xxError,
   is429Error,
-  MissingExpiredTokenError,
 } from '@clerk/shared/error';
@@
   `#createTokenResolver`(
     template: string | undefined,
     organizationId: string | undefined | null,
     skipCache: boolean,
   ): Promise<TokenResource> {
@@
-    const sessionMinterEnabled = Session.clerk?.__internal_environment?.authConfig?.sessionMinter;
     const params: Record<string, string | null> = template
       ? {}
       : {
           organizationId: organizationId ?? null,
-          ...(sessionMinterEnabled && this.lastActiveToken ? { token: this.lastActiveToken.getRawString() } : {}),
+          ...(this.lastActiveToken ? { token: this.lastActiveToken.getRawString() } : {}),
         };
-
-    if (sessionMinterEnabled) {
-      // Session Minter sends the token in the body, no expired_token retry needed
-      return Token.create(path, params, skipCache ? { debug: 'skip_cache' } : undefined);
-    }
-
-    const lastActiveToken = this.lastActiveToken?.getRawString();
-    return Token.create(path, params, skipCache ? { debug: 'skip_cache' } : undefined).catch(e => {
-      if (MissingExpiredTokenError.is(e) && lastActiveToken) {
-        return Token.create(path, { ...params }, { expired_token: lastActiveToken });
-      }
-      throw e;
-    });
+    return Token.create(path, params, skipCache ? { debug: 'skip_cache' } : undefined);
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/clerk-js/src/core/resources/Session.ts` around lines 494 - 503, The
code still retries with expired_token in the query string (using Token.create in
the catch and MissingExpiredTokenError with lastActiveToken), which reintroduces
JWT-in-URL leakage; remove that retry branch entirely and stop passing
expired_token via query params. Update the logic around sessionMinterEnabled and
the Token.create calls so that /tokens requests always send the token in the
POST body (not in query params) and eliminate the catch block that checks
MissingExpiredTokenError/lastActiveToken and re-invokes Token.create with {
expired_token: ... }.

throw e;
});

return tokenResolver;
}

#dispatchTokenEvents(token: TokenResource, shouldDispatch: boolean): void {
Expand Down
16 changes: 4 additions & 12 deletions scripts/validate-staging-instances.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,7 @@ describe('collapseAttributeMismatches', () => {
{ path: 'user_settings.attributes.phone_number.verifications', prod: ['phone_code'], staging: [] },
];
const result = collapseAttributeMismatches(mismatches);
expect(result).toEqual([
{ path: 'user_settings.attributes.phone_number.enabled', prod: true, staging: false },
]);
expect(result).toEqual([{ path: 'user_settings.attributes.phone_number.enabled', prod: true, staging: false }]);
});

it('keeps child diffs when .enabled does NOT differ', () => {
Expand Down Expand Up @@ -286,9 +284,7 @@ describe('collapseSocialMismatches', () => {
{ path: 'user_settings.social.google.strategy', prod: 'oauth_google', staging: undefined },
];
const result = collapseSocialMismatches(mismatches);
expect(result).toEqual([
{ path: 'user_settings.social.google', prod: { enabled: true }, staging: undefined },
]);
expect(result).toEqual([{ path: 'user_settings.social.google', prod: { enabled: true }, staging: undefined }]);
});

it('collapses child diffs for extra social provider on staging', () => {
Expand All @@ -297,9 +293,7 @@ describe('collapseSocialMismatches', () => {
{ path: 'user_settings.social.github.enabled', prod: undefined, staging: true },
];
const result = collapseSocialMismatches(mismatches);
expect(result).toEqual([
{ path: 'user_settings.social.github', prod: undefined, staging: { enabled: true } },
]);
expect(result).toEqual([{ path: 'user_settings.social.github', prod: undefined, staging: { enabled: true } }]);
});

it('keeps child diffs when both prod and staging have the provider', () => {
Expand All @@ -313,9 +307,7 @@ describe('collapseSocialMismatches', () => {
});

it('does not affect non-social mismatches', () => {
const mismatches = [
{ path: 'auth_config.session_token_ttl', prod: 3600, staging: 7200 },
];
const mismatches = [{ path: 'auth_config.session_token_ttl', prod: 3600, staging: 7200 }];
const result = collapseSocialMismatches(mismatches);
expect(result).toEqual(mismatches);
});
Expand Down
Loading