Implement JWT token rotation for authentication#1620
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces opt-in JWT access token rotation for external authentication used by the Jetstream Desktop app and the Jetstream Web Extension. It adds a client capability header, rotates tokens during /auth/verify, and updates persistence + tests to validate rotated token behavior.
Changes:
- Add
X-Supports-Token-Rotationheader and plumb it through desktop + web extension auth verification. - Implement server-side rotation via conditional DB update (race-safe) + LRU cache invalidation.
- Add E2E coverage for rotation success, backward compatibility (no header), and logout invalidation.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/shared/constants/src/lib/shared-constants.ts | Adds the new X-Supports-Token-Rotation header constant. |
| apps/jetstream-web-extension/src/extension-scripts/service-worker.ts | Sends rotation header and persists rotated tokens returned by /auth/verify. |
| apps/jetstream-e2e/src/tests/authentication/external-auth/external-auth-logged-in.spec.ts | Adds E2E scenarios validating rotation + invalidation semantics. |
| apps/jetstream-desktop/src/services/ipc.service.ts | Updates desktop auth-check flow to accept rotated tokens and refresh expiry. |
| apps/jetstream-desktop/src/services/api.service.ts | Updates verify schema + always advertises rotation support to the API. |
| apps/api/src/app/services/external-auth.service.ts | Adds rotateToken, cache invalidation helper, short token duration, and jti issuance. |
| apps/api/src/app/db/web-extension.db.ts | Adds replaceTokenIfCurrent for race-safe token replacement in DB. |
| apps/api/src/app/controllers/web-extension.controller.ts | Adds optional rotated token to verify response + invalidates cache on logout. |
| apps/api/src/app/controllers/desktop-app.controller.ts | Adds optional rotated token to verify response + invalidates cache on logout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const payload = JSON.parse(atob(results.accessToken.split('.')[1])); | ||
| expiresAt = payload.exp * 1000; |
There was a problem hiding this comment.
JWT payloads are Base64URL-encoded; using atob(results.accessToken.split('.')[1]) can fail (or decode incorrectly) because atob expects standard Base64 and padding. Since this file already uses jwtDecode elsewhere, decode exp via jwtDecode (or convert Base64URL->Base64 safely) and only update expiresAt when exp is a finite number to avoid persisting NaN.
| const payload = JSON.parse(atob(results.accessToken.split('.')[1])); | |
| expiresAt = payload.exp * 1000; | |
| const payload = jwtDecode<JwtPayload>(results.accessToken); | |
| if (isNumber(payload.exp) && Number.isFinite(payload.exp)) { | |
| expiresAt = payload.exp * 1000; | |
| } |
| if (successResponse.encryptionKey) { | ||
| dataService.setOrgEncryptionKey(successResponse.encryptionKey); | ||
| } | ||
| return { userProfile, authInfo: { deviceId, accessToken: activeAccessToken } }; |
There was a problem hiding this comment.
After a successful verify you update appData with successResponse.userProfile, but the returned value still uses the pre-existing userProfile variable (which may be stale). Return successResponse.userProfile (or re-read from dataService) so callers get the same profile you just persisted.
| return { userProfile, authInfo: { deviceId, accessToken: activeAccessToken } }; | |
| return { userProfile: successResponse.userProfile, authInfo: { deviceId, accessToken: activeAccessToken } }; |
| verifyToken: { | ||
| controllerFn: () => verifyToken, | ||
| responseType: z.object({ success: z.boolean(), error: z.string().nullish() }), | ||
| responseType: z.object({ success: z.boolean(), error: z.string().nullish(), accessToken: z.string().optional() }), |
There was a problem hiding this comment.
verifyToken.responseType doesn’t match the actual success response being sent (it includes userProfile, but the schema omits it). This can break route typing / generated clients. Consider switching to a discriminated union like the desktop controller and include userProfile (and accessToken optional) in the success shape.
| responseType: z.object({ success: z.boolean(), error: z.string().nullish(), accessToken: z.string().optional() }), | |
| responseType: z.discriminatedUnion('success', [ | |
| z.object({ | |
| success: z.literal(true), | |
| userProfile: z.any(), | |
| accessToken: z.string().optional(), | |
| error: z.string().nullish(), | |
| }), | |
| z.object({ | |
| success: z.literal(false), | |
| error: z.string().nullish(), | |
| }), | |
| ]), |
| }): Promise<string | undefined> { | ||
| const newAccessToken = await issueAccessToken(userProfile, audience, durationMs ?? TOKEN_EXPIRATION_SHORT); | ||
| const oldTokenHash = hashToken(oldAccessToken); | ||
| const wasReplaced = await webExtDb.replaceTokenIfCurrent(userProfile.id, oldTokenHash, { | ||
| type: webExtDb.TOKEN_TYPE_AUTH, | ||
| source, | ||
| token: newAccessToken, |
There was a problem hiding this comment.
rotateToken always issues a new JWT and writes to the DB whenever it’s called. Since the web extension verifies auth on a 5-minute cadence, enabling rotation will cause very frequent token churn + DB writes and increases the chance of forcing a re-login if the rotation response is lost. Consider rotating only when the existing token is near expiry (e.g., exp within a buffer) or otherwise rate-limit rotations per device/user.
Introduce JWT token rotation for both desktop and web extension authentication, enhancing security by allowing tokens to be replaced conditionally based on their current state. This update includes support for token rotation in the authentication flow and modifies relevant schemas and routes accordingly.