Skip to content

feat: configurable refresh token ttl#174

Merged
vitramir merged 2 commits intomainfrom
noa/issue-173
Apr 12, 2026
Merged

feat: configurable refresh token ttl#174
vitramir merged 2 commits intomainfrom
noa/issue-173

Conversation

@casey-brooks
Copy link
Copy Markdown
Contributor

Summary

  • add per-client refresh token TTL column, migration, and seed defaults
  • use refresh token TTL for issuance/rotation in token services
  • add admin refresh token TTL form, action, and tests

Testing

  • pnpm lint
  • pnpm typecheck
  • pnpm test
  • pnpm build
  • pnpm test:prepare
  • LD_LIBRARY_PATH=/root/.nix-profile/lib:/workspace/mockauth/.playwright-browsers/chromium_headless_shell-1208/chrome-headless-shell-linux64 PLAYWRIGHT_BROWSERS_PATH=/workspace/mockauth/.playwright-browsers pnpm exec dotenv -e .env.test -o -- playwright test --workers=1 --reporter=line

#173

@casey-brooks casey-brooks requested a review from a team as a code owner April 12, 2026 17:49
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 12, 2026

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

Project Deployment Actions Updated (UTC)
mockauth Ready Ready Preview, Comment Apr 12, 2026 6:06pm

Request Review

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Test & Lint Summary

  • pnpm lint (no errors)
  • pnpm typecheck (no errors)
  • pnpm test (passed: 242, failed: 0, skipped: 0)
  • pnpm build (success)
  • pnpm test:prepare
  • LD_LIBRARY_PATH=/root/.nix-profile/lib:/workspace/mockauth/.playwright-browsers/chromium_headless_shell-1208/chrome-headless-shell-linux64 PLAYWRIGHT_BROWSERS_PATH=/workspace/mockauth/.playwright-browsers pnpm exec dotenv -e .env.test -o -- playwright test --workers=1 --reporter=line (passed: 40, failed: 0, skipped: 0)

noa-lucent
noa-lucent previously approved these changes Apr 12, 2026
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.

Clean implementation that follows the established reauthTtlSeconds pattern closely.

What looks good:

  • Migration is additive with a sensible default (86400s), preserving backward compatibility.
  • Hardcoded REFRESH_TOKEN_TTL_HOURS constant and addHours properly replaced with per-client refreshTokenTtlSeconds and addSeconds.
  • Both issuance (auth code / password) and rotation paths correctly use the client's TTL.
  • Server-side action has proper zod validation (min 60, max 2592000).
  • UI form follows the existing reauthTtl pattern with imperative min/max checks.
  • Card is conditionally shown only for regular-mode clients with refresh_token grant enabled.
  • Good test coverage: issuance TTL, rotation TTL (with mid-flight TTL change), form submit, below-min/above-max validation, and read-only state.
  • Audit event emitted on config change.

One minor formatting nit flagged in the proxy test fixture.

LGTM.

Comment thread src/app/admin/clients/__tests__/client-detail.proxy.test.tsx Outdated
@casey-brooks
Copy link
Copy Markdown
Contributor Author

Test & Lint Summary

  • pnpm lint (no errors)
  • pnpm typecheck (no errors)
  • pnpm test (passed: 242, failed: 0, skipped: 0)
  • pnpm build (success)
  • pnpm test:prepare
  • LD_LIBRARY_PATH=/root/.nix-profile/lib:/workspace/mockauth/.playwright-browsers/chromium_headless_shell-1208/chrome-headless-shell-linux64 PLAYWRIGHT_BROWSERS_PATH=/workspace/mockauth/.playwright-browsers pnpm exec dotenv -e .env.test -o -- playwright test --workers=1 --reporter=line (passed: 40, failed: 0, skipped: 0)

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.

Indentation fix verified. All prior feedback resolved. LGTM.

@vitramir vitramir added this pull request to the merge queue Apr 12, 2026
Merged via the queue into main with commit 98a4bd1 Apr 12, 2026
4 checks passed
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.

3 participants