Skip to content

fix: revoke OAuth 2.0 old refresh tokens when refreshing#27581

Closed
supalarry wants to merge 66 commits intomainfrom
lauris/cal-7035-fix-invalidate-old-refresh-tokens
Closed

fix: revoke OAuth 2.0 old refresh tokens when refreshing#27581
supalarry wants to merge 66 commits intomainfrom
lauris/cal-7035-fix-invalidate-old-refresh-tokens

Conversation

@supalarry
Copy link
Contributor

@supalarry supalarry commented Feb 3, 2026

Fixes #26538


Summary by cubic

Adds OAuth 2.0 refresh token rotation with server-side tracking so old refresh tokens are rejected after use. Introduces a unified POST /token endpoint (form-encoded, snake_case) and keeps legacy endpoints. Addresses CAL-7035.

  • Bug Fixes

    • Store a per-token secret and persist refresh token records; on reuse return invalid_grant (refresh_token_revoked).
    • Accept legacy refresh tokens (no secret) once, then issue new-format tokens; reject tokens missing both userId and teamId.
    • Rotate by user or team on refresh and avoid deleting unrelated tokens (fix undefined vs null filters).
    • Unify exchange/refresh into POST /token; accept application/x-www-form-urlencoded; flat snake_case response; keep legacy /exchange and /refresh.
    • Standardize OAuth errors (error + error_description) across API v2 and web; add e2e tests for legacy acceptance, rotation, and reuse rejection.
  • Migration

    • Add OAuthRefreshToken table with indexes and FKs; run Prisma migrate.
    • No backfill needed; records are created on token issue and refresh.

Written for commit 48be831. Summary will update on new commits.


Updates since last revision

  • Fixed a bug in OAuthRefreshTokenRepository.rotateToken() where the condition userId != null failed to distinguish between undefined and null. When userId is explicitly null (client credentials token), the previous condition would skip adding userId to the where clause, causing deleteMany to match ALL tokens for the client—including tokens belonging to actual users. Changed to userId !== undefined to properly handle all cases.
  • Replaced generic throw new Error(...) with typed throw new ErrorWithCode(ErrorCode.BadRequest, "invalid_grant", ...) in the else branch of refreshAccessToken() so clients get a proper OAuth error response instead of a 500 (identified by Cubic AI review).

What does this PR do?

Implements OAuth 2.0 refresh token rotation to prevent token reuse attacks. When a refresh token is used, it is invalidated and a new one is issued. Attempting to reuse an old refresh token returns invalid_grant with reason refresh_token_revoked.

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. N/A - internal OAuth implementation detail.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  1. Create an OAuth client and obtain tokens via authorization code flow
  2. Use the refresh token to get new tokens - should succeed
  3. Try to reuse the same refresh token again - should fail with invalid_grant / refresh_token_revoked
  4. E2E tests cover both API v2 (oauth2.controller.e2e-spec.ts) and web (oauth-refresh-tokens.e2e.ts)

Human Review Checklist

  • Verify the undefined vs null fix in rotateToken() correctly isolates client credentials tokens (where userId is null) from user tokens
  • Confirm legacy token migration path (tokens without secret field) works correctly for one-time use
  • Note: logger import in OAuthService.ts may be unused after error handling refactor (lint warning, non-blocking)

Link to Devin run: https://app.devin.ai/sessions/78d60d454fa2432cb97575bc00747c05
Requested by: unknown ()

supalarry and others added 30 commits January 30, 2026 11:26
- Fix getClient endpoint to use proper REST API error format instead of OAuth token error format (confidence 9/10)
- Add missing space after comma in error format string in token.input.pipe.ts (confidence 9/10)
- Support both camelCase and snake_case inputs in authorize endpoint for backward compatibility (confidence 10/10)
- Restore legacy /exchange and /refresh endpoints alongside new /token endpoint for backward compatibility (confidence 10/10)
- Add OAuth2TokensResponseDto for legacy endpoint wrapped responses
- Add OAuth2LegacyExchangeInput and OAuth2LegacyRefreshInput for legacy endpoints

Co-Authored-By: unknown <>
- Log errors when status code >= 500 in handleClientError (confidence 9/10)
- Add Cache-Control: no-store and Pragma: no-cache headers to legacy /exchange and /refresh endpoints (confidence 9/10)

Co-Authored-By: unknown <>
- Fix getClient to use handleClientError instead of handleTokenError (confidence 10)
- Restore legacy /exchange and /refresh endpoints for backward compatibility (confidence 9)
- Fix RFC 6749 error format: use human-readable messages in error_description (confidence 9)
- Fix errorDescription in OAuthService to use OAUTH_ERROR_REASONS mapping (confidence 9)

Co-Authored-By: unknown <>
- Fix security issue: Replace 'CALENDSO_ENCRYPTION_KEY is not set' with generic 'Internal server configuration error' message (confidence 10/10)
- Fix backward compatibility: Create OAuth2LegacyTokensDto with camelCase properties for legacy /exchange and /refresh endpoints (confidence 9/10)
- Skipped: RFC 6749 error field issue (confidence 8/10, below threshold)

Co-Authored-By: unknown <>
- Restore OAuth2LegacyExchangeInput and OAuth2LegacyRefreshInput classes
- Restore legacy /exchange and /refresh endpoints in OAuth2Controller
- Restore OAuth2LegacyTokensDto and OAuth2TokensResponseDto classes
- Restore OAUTH_ERROR_DESCRIPTIONS mapping in oauth2-error.service.ts
- Restore OAUTH_ERROR_REASONS lookup in OAuthService.ts mapErrorToOAuthError
- Fix encryption_key_missing error to not expose internal env var name

Addresses Cubic AI feedback with confidence >= 9/10:
- Comment 32 (9/10): Legacy endpoints and input classes
- Comment 34 (9/10): Error description mapping in OAuthService
- Comment 35 (10/10): OAUTH_ERROR_DESCRIPTIONS in error service

Skipped (confidence < 9/10):
- Comment 33 (8/10): getClient handleTokenError vs handleClientError

Co-Authored-By: unknown <>
…ent-is-in-review' into lauris/cal-7035-fix-invalidate-old-refresh-tokens
…ent-is-in-review' into lauris/cal-7035-fix-invalidate-old-refresh-tokens
Base automatically changed from lauris/cal-7029-feat-enable-test-access-while-oauth-client-is-in-review to main February 19, 2026 11:11
@CarinaWolli CarinaWolli dismissed stale reviews from eunjae-lee and themself February 19, 2026 11:11

The base branch was changed.

@github-actions github-actions bot added the Medium priority Created by Linear-GitHub Sync label Feb 19, 2026
@supalarry supalarry enabled auto-merge (squash) February 19, 2026 11:36
@github-actions
Copy link
Contributor

github-actions bot commented Feb 19, 2026

E2E results are ready!

@supalarry supalarry force-pushed the lauris/cal-7035-fix-invalidate-old-refresh-tokens branch from 08ccf0d to 5e56c63 Compare February 19, 2026 13:47
@keithwillcode
Copy link
Contributor

Moving back to draft while we discuss

@keithwillcode keithwillcode marked this pull request as draft February 19, 2026 15:07
auto-merge was automatically disabled February 19, 2026 15:07

Pull request was converted to draft

@supalarry supalarry marked this pull request as ready for review February 19, 2026 15:37
@supalarry
Copy link
Contributor Author

Moving back to draft while we discuss

marking it as ready for review for Pedro

@supalarry supalarry closed this Mar 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

consumer core area: core, team members only Medium priority Created by Linear-GitHub Sync ❗️ migrations contains migration files ready-for-e2e size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: invalidate old refresh tokens

4 participants