Skip to content

feat(auth): support refresh tokens#137

Open
casey-brooks wants to merge 2 commits intomainfrom
noa/issue-135-refresh
Open

feat(auth): support refresh tokens#137
casey-brooks wants to merge 2 commits intomainfrom
noa/issue-135-refresh

Conversation

@casey-brooks
Copy link
Copy Markdown
Contributor

Summary

  • issue refresh tokens for regular clients when offline_access + refresh_token grant are enabled
  • add refresh grant validation/audit logging plus test routes for proxy token exchange + scope updates
  • make proxy-mockauth-upstream e2e assert refresh tokens and upstream refresh logs

Testing

  • pnpm lint
  • pnpm typecheck
  • pnpm test
  • pnpm test:e2e

Relates to #135

@casey-brooks casey-brooks requested a review from a team as a code owner March 29, 2026 13:16
@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 29, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
mockauth Ready Ready Preview, Comment Mar 29, 2026 1:32pm

Request Review

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • add refresh token issuance/validation for regular clients and refresh grant handling
  • add test routes for proxy token exchange + scope updates
  • update proxy-mockauth-upstream e2e to assert refresh tokens and upstream refresh flow

Testing

  • pnpm lint (no errors)
  • pnpm typecheck (no errors)
  • pnpm test (171 passed, 0 failed, 0 skipped)
  • pnpm test:e2e (36 passed, 0 failed, 0 skipped)

@casey-brooks
Copy link
Copy Markdown
Contributor Author

cc @noa - refresh-token proxy upstream updates ready.

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

The refresh token implementation is well-structured — JWT-based refresh tokens with proper verification, scope down-scoping, audit logging, and the E2E now deterministically asserts the full refresh path including upstream audit logs.

Requesting changes for one issue:

  • [major] Duplicate updateClientScopes call in the happy-path test — trivial to fix, but needs to be removed before merge.
  • [minor] Double DB lookups in the token route due to the new oauthClientMode branch. Not blocking.

The rest looks solid: boundary parsing in parseRefreshTokenPayload and verifyRefreshToken is clean, the normalizeRefreshTokenScopes correctly uses invalid_grant per RFC 6749, and the test-only routes follow existing conventions with proper gating and Zod validation.

Comment thread tests/e2e/proxy-mockauth-upstream.spec.ts Outdated
Comment thread src/app/r/[apiResourceId]/oidc/token/route.ts
@casey-brooks
Copy link
Copy Markdown
Contributor Author

Summary

  • remove duplicate updateClientScopes call in proxy-mockauth-upstream e2e

Testing

  • pnpm lint (no errors)
  • pnpm typecheck (no errors)
  • pnpm test (171 passed, 0 failed, 0 skipped)
  • pnpm test:e2e (36 passed, 0 failed, 0 skipped)

@casey-brooks
Copy link
Copy Markdown
Contributor Author

cc @noa - fixed duplicate scope update, tests re-run.

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Duplicate updateClientScopes call removed. Both threads resolved. LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants