fix: refresh JWT tokens after user updates#1082
fix: refresh JWT tokens after user updates#1082deanpektas27 wants to merge 4 commits intostack-auth:devfrom
Conversation
Older cmux preview screenshots (latest comment is below)Preview Screenshots⏳ Preview screenshots are being captured... Workspace and dev browser links will appear here once the preview environment is ready. Generated by cmux preview system |
|
@deanpektas27 is attempting to deploy a commit to the Stack Auth Team on Vercel. A member of the Team first needs to authorize it. |
|
|
📝 WalkthroughWalkthroughAdds an awaited call to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryFixes JWT token staleness after user updates by adding a
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User as User Code
participant SDK as Client SDK
participant Cache as Local Cache
participant API as Backend API
participant Session as InternalSession
User->>SDK: setSelectedTeam(team)
SDK->>SDK: update({ selectedTeamId })
SDK->>SDK: _updateClientUser(update, session)
SDK->>API: updateClientUser(update)
API-->>SDK: response
SDK->>Cache: _refreshUser(session)
Cache-->>SDK: cache updated
SDK->>Session: fetchNewTokens()
Session->>API: refresh access token
API-->>Session: new JWT with updated selected_team_id
Session-->>SDK: new tokens
SDK-->>User: complete
|
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Always add new E2E tests when changing the API or SDK interface
For blocking alerts and errors, never use toast; use alerts instead as they are less easily missed by the user
NEVER try-catch-all, NEVER void a promise, and NEVER .catch(console.error); use loading indicators and async callbacks instead, or use runAsynchronously/runAsynchronouslyWithAlert for error handling
Use ES6 maps instead of records wherever you can
Files:
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts
**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,css}: Keep hover/click transitions snappy and fast; avoid fade-in delays on hover. Apply transitions after action completion instead, like smooth fade-out when hover ends
Use hover-exit transitions instead of hover-enter transitions; for example, use 'transition-colors hover:transition-none' instead of fade-in on hover
Files:
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts
{.env*,**/*.{ts,tsx,js}}
📄 CodeRabbit inference engine (AGENTS.md)
Prefix environment variables with STACK_ (or NEXT_PUBLIC_STACK_ if public) so changes are picked up by Turborepo and improves readability
Files:
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
packages/template/src/lib/stack-app/apps/implementations/client-app-impl.ts
Show resolved
Hide resolved
|
Can you add testing? Following our existing testing structure. |
Verifies that JWT tokens are properly refreshed with updated claims (like selected_team_id) after calling setSelectedTeam() or user.update(). Tests: - setSelectedTeam() triggers token refresh with correct team ID - user.update() triggers token refresh with correct team ID - Old and new tokens are different (new token issued, not mutated) - Multiple team switches all produce fresh tokens
Preview Screenshots⏳ Preview screenshots are being captured... Workspace and dev browser links will appear here once the preview environment is ready. Generated by cmux preview system |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/e2e/tests/js/user-update-token-refresh.test.ts (1)
1-172: Consider adding a test for the nullselected_team_idcase during onboarding.The PR description mentions that "the JWT is not refreshed; it can contain the old team ID or remain null (noted when a user signs up and is onboarding)." While your tests comprehensively cover team switching scenarios, you might want to add a specific test that:
- Signs up a new user
- Verifies the initial token has
selected_team_id: null(or undefined)- Creates and selects the first team
- Verifies the token is refreshed with the correct
selected_team_idThis would explicitly document the onboarding flow mentioned in the PR description.
💡 Optional test for null selected_team_id during onboarding
You could add a test like this:
it("should refresh JWT token from null selected_team_id during initial onboarding", async ({ expect }) => { const { clientApp } = await createApp({ config: { clientTeamCreationEnabled: true, }, }); // Sign up a user await signUp(clientApp, "onboarding@test.com"); const user = await clientApp.getUser({ or: "throw" }); // Get initial token and verify selected_team_id is null/undefined const initialAccessToken = await user.getAccessToken(); expect(initialAccessToken).toBeDefined(); const initialPayload = jose.decodeJwt(initialAccessToken!); expect(initialPayload.selected_team_id).toBeNullish(); // or toBeUndefined() // Create first team (onboarding scenario) const firstTeam = await user.createTeam({ displayName: "First Team" }); await user.setSelectedTeam(firstTeam); // Verify token is refreshed with the correct team const newAccessToken = await user.getAccessToken(); const newPayload = jose.decodeJwt(newAccessToken!); expect(newPayload.selected_team_id).toBe(firstTeam.id); expect(newAccessToken).not.toBe(initialAccessToken); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/e2e/tests/js/user-update-token-refresh.test.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Always add new E2E tests when changing the API or SDK interface
For blocking alerts and errors, never use toast; use alerts instead as they are less easily missed by the user
NEVER try-catch-all, NEVER void a promise, and NEVER .catch(console.error); use loading indicators and async callbacks instead, or use runAsynchronously/runAsynchronouslyWithAlert for error handling
Use ES6 maps instead of records wherever you can
Files:
apps/e2e/tests/js/user-update-token-refresh.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer .toMatchInlineSnapshot over other selectors in tests when possible; check snapshot-serializer.ts for formatting details
Files:
apps/e2e/tests/js/user-update-token-refresh.test.ts
**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,css}: Keep hover/click transitions snappy and fast; avoid fade-in delays on hover. Apply transitions after action completion instead, like smooth fade-out when hover ends
Use hover-exit transitions instead of hover-enter transitions; for example, use 'transition-colors hover:transition-none' instead of fade-in on hover
Files:
apps/e2e/tests/js/user-update-token-refresh.test.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
NEVER use Next.js dynamic functions if you can avoid them; prefer using client components and hooks like usePathname instead of await params to keep pages static
Files:
apps/e2e/tests/js/user-update-token-refresh.test.ts
{.env*,**/*.{ts,tsx,js}}
📄 CodeRabbit inference engine (AGENTS.md)
Prefix environment variables with STACK_ (or NEXT_PUBLIC_STACK_ if public) so changes are picked up by Turborepo and improves readability
Files:
apps/e2e/tests/js/user-update-token-refresh.test.ts
apps/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always add new E2E tests when changing API or SDK interface; err on the side of creating too many tests due to the critical nature of the industry
Files:
apps/e2e/tests/js/user-update-token-refresh.test.ts
🧠 Learnings (2)
📚 Learning: 2025-12-04T18:03:49.984Z
Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T18:03:49.984Z
Learning: Applies to **/*.{ts,tsx} : Always add new E2E tests when changing the API or SDK interface
Applied to files:
apps/e2e/tests/js/user-update-token-refresh.test.ts
📚 Learning: 2025-12-04T18:03:49.984Z
Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T18:03:49.984Z
Learning: Applies to apps/e2e/**/*.{ts,tsx} : Always add new E2E tests when changing API or SDK interface; err on the side of creating too many tests due to the critical nature of the industry
Applied to files:
apps/e2e/tests/js/user-update-token-refresh.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
🔇 Additional comments (5)
apps/e2e/tests/js/user-update-token-refresh.test.ts (5)
1-15: LGTM! Clean imports and helper setup.The imports are appropriate for JWT token testing, and the
signUphelper function follows DRY principles by centralizing the sign-up logic used across all tests.
17-57: Excellent test coverage for setSelectedTeam.This test comprehensively verifies that:
- Tokens are reissued (not reused)
- The
selected_team_idclaim is updated correctly- The new token differs from the initial token
The assertions are thorough and directly address the PR objective of ensuring JWTs are refreshed after user updates.
59-91: Good coverage of the user.update() API path.This test ensures that token refresh works not just for
setSelectedTeambut also for the more generaluser.update()method when changingselectedTeamId. This dual coverage is valuable for catching regressions in either code path.
93-129: Valuable test for verifying token reissuance.This test explicitly verifies that old tokens aren't reused after
setSelectedTeam, which directly addresses the problem statement in the PR description. The comparison of old vs. new token claims (lines 127-128) provides additional confidence that the token refresh is working correctly.
131-172: Comprehensive edge case coverage for multiple team switches.This test ensures that token refresh works correctly across multiple sequential team changes, including switching back to a previously selected team. The assertions that all three tokens are unique (lines 169-171) are particularly valuable for catching potential caching issues where the SDK might incorrectly reuse an old token.
|
📝 Documentation updates detected! New suggestion: Clarify that JWT tokens refresh on user data changes |
|
@deanpektas27 Dean, thanks for the PR. This was actually included in #1069 and is merged at this point. We appreciate you contributing nonetheless! Thanks, |
4 participants
When calling user.setSelectedTeam, the JWT does not refresh until the user re-authenticates.
Problem
After calling user.setSelectedTeam, the sdk updates cache but the JWT contains either the old team ID, or remains null (in my case its when the user signs up and is onboarding).
Adding
await session.fetchNewTokens()ensures JWT's are refreshed after user updates.Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.