-
Notifications
You must be signed in to change notification settings - Fork 30
fix: scope JWT cookies by project ID to prevent audience mismatch warnings #1546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… warnings Co-Authored-By: Alfonso <[email protected]>
🦋 Changeset detectedLatest commit: d220cf1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Original prompt from Alfonso |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| } | ||
|
|
||
| /** | ||
| * Cookie storage that scopes cookies by API key prefix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you believe that "ck_staging" is the project name, or something. But it's not. That indicates Client Key in Staging environment
With the code change you have made, two keys from the same environment will always be stored in the same place
What you need to do is not to extract the API key prefix. You need to extract the project ID from the key, and use that as the storage key. Check the codebase as thee's some parsing function to decode the project ID from it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! I've updated the implementation to properly extract the project ID from the API key using the existing validateAPIKey function from @crossmint/common-sdk-base. The cookie names are now scoped by project ID (e.g., crossmint-jwt-{projectId}) instead of just the API key prefix.
Co-Authored-By: Alfonso <[email protected]>
…token usage Co-Authored-By: Alfonso <[email protected]>
- For JWTs: Only read from scoped cookies to prevent using wrong project's JWT - For refresh tokens: Fall back to legacy cookies to allow migration - If legacy refresh token is for wrong project, server rejects and SDK calls logout() - Remove API key change detection logic from CrossmintAuthBaseProvider - This prevents the audience mismatch warnings while maintaining backward compatibility Co-Authored-By: Alfonso <[email protected]>
Co-Authored-By: Alfonso <[email protected]>
This prevents cross-project token usage when a legacy refresh token is used with a different project. The server accepts the refresh token but issues a JWT for the original project, not the current one. This fix detects the mismatch and calls logout instead of storing the mismatched JWT. Co-Authored-By: Alfonso <[email protected]>
Co-Authored-By: Alfonso <[email protected]>
.changeset/wicked-lemons-tell.md
Outdated
| "@crossmint/client-sdk-auth": patch | ||
| --- | ||
|
|
||
| Fix JWT cookie scoping to prevent cross-project token usage. JWTs are now scoped by project ID and only refresh tokens fall back to legacy cookies for migration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@devinai this is not entirely accurate. I woudl say something on the lines of "Moved JWT cookie scoping to allow for multiple project cookies in the same domain, and prevent issues of cookie mismatch during development"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the changeset description as suggested. Thanks!
Co-Authored-By: Alfonso <[email protected]>
| console.debug( | ||
| `JWT audience mismatch: JWT is for project ${jwtAudience}, but current project is ${currentProjectId}. Logging out.` | ||
| ); | ||
| await this.logout(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@devinai explain to me why we need this logout here, and what is it clearing (a) the current auth token, or (b) the one mapping to the api key for the current project?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why we need this logout:
This scenario happens when a legacy refresh token (from before cookie scoping) is used with a different project. The server accepts the refresh token but issues a JWT for the original project, not the current one. We can't use this JWT because it would cause the audience mismatch warnings you were seeing.
What logout() clears:
The answer is (b) the cookies mapping to the API key for the current project.
The storageProvider is a ScopedCookieStorage initialized with the current API key (Project 2). When logout() is called, it removes:
crossmint-jwt-{project2Id}(scoped cookie - doesn't exist yet)crossmint-refresh-token-{project2Id}(scoped cookie - doesn't exist yet)crossmint-jwt(legacy cookie)crossmint-refresh-token(legacy cookie - this is the one that caused the issue)
Important: Project 1's scoped cookies (crossmint-jwt-{project1Id}, crossmint-refresh-token-{project1Id}) are NOT affected by this logout. They remain intact.
The purpose is to clean up the legacy refresh token that caused the mismatch, so we don't keep trying to use it. The user will need to re-authenticate for Project 2, but their Project 1 session remains valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right - I've removed this block. The refresh doesn't actually fail (server accepts the token and issues a JWT), but the warnings don't break functionality. As users re-authenticate with the updated SDK, they'll get scoped cookies and the warnings will naturally stop. The forced logout was more disruptive than the warnings themselves.
Co-Authored-By: Alfonso <[email protected]>
Co-Authored-By: Alfonso <[email protected]>
| return decoded.exp; | ||
| } | ||
|
|
||
| export function getJWTAudience(token: string): string | undefined { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the unused getJWTAudience function.
| * If a legacy refresh token is used with the wrong project, the server will reject it | ||
| * and the SDK will call logout(), cleaning up the legacy cookie. | ||
| */ | ||
| export class ScopedCookieStorage implements StorageProvider { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would change cookiestorage to use this implementation instead of having 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion! I kept both implementations for now to minimize the scope of this change, but consolidating them would be a good follow-up. The main difference is that ScopedCookieStorage adds project ID scoping and legacy cookie fallback for refresh tokens. Would you like me to consolidate them in this PR or as a separate follow-up?
|
We're still calling our prefix var |
Co-Authored-By: Alfonso <[email protected]>
|
Good question! The |
Description
Fixes the "Unexpected JWT encountered while verifying" warning logs that occur when developers switch between different Crossmint projects (especially during local development). The root cause was that JWT cookies were stored with fixed names (
crossmint-jwt,crossmint-refresh-token) not scoped to the project, causing JWTs from one project to be sent with requests for another project.Changes:
crossmint-jwt-{projectId}). The project ID is extracted from the API key using the existingvalidateAPIKeyfunction from@crossmint/common-sdk-base. For backward compatibility, the SDK reads from legacy (unscoped) cookies as fallback but writes to new scoped cookies.Key files to review:
packages/client/auth/src/utils/storage.ts- NewScopedCookieStorageclass andgetProjectIdFromApiKeyutilitypackages/client/react-base/src/providers/CrossmintAuthBaseProvider.tsx- API key change detection effectHuman review checklist
validateAPIKeycorrectly extracts project ID for all API key formats used in productionTest plan
pnpm lint- passedpnpm test:vitest- all tests passPackage updates
@crossmint/client-sdk-auth: patch@crossmint/client-sdk-react-base: patchChangeset added via
pnpm change:add.Link to Devin run: https://crossmint.devinenterprise.com/sessions/83bec0c3906643d591cd8d648b6549db
Requested by: Alfonso (@alfonso-paella)