chore: use refresh tokens in local oidc provider

[kantord]

No description provided.

[kantord]

@claude can you please do a deep review of this?

[claude]

Claude finished @kantord's task —— View job

---

Deep Review of PR #76

Thanks for asking me to review this PR! I've analyzed the changes to add refresh token support to the local OIDC provider. Here's my comprehensive review:

---

🎯 Summary

This PR properly configures the development OIDC provider to support refresh tokens, ensuring the local dev environment matches production behavior. The changes are well-implemented and align with the project's authentication architecture.

---

✅ What's Good

1. Correct Refresh Token Implementation

The changes properly configure the OIDC provider to issue refresh tokens:

• ✅ Added offline_access scope declaration (line 73)
• ✅ Added refresh_token grant type (line 38)
• ✅ Configured issueRefreshToken: async () => true to always issue refresh tokens in dev (line 84)
• ✅ Auto-grants offline_access in the consent flow (lines 126-128)

2. Good Dev Experience

The very short access token TTL (15 seconds) forces frequent token refreshes during development, which is excellent for testing:

• ✅ Catches refresh token bugs early
• ✅ Verifies the full refresh flow works correctly
• ✅ Well-documented with clear comments explaining the rationale

3. Client-Side Changes Are Correct

The addition of offline_access to the scopes in both the sign-in page and test is appropriate:

• ✅ Matches the server-side auth configuration (see src/lib/auth/auth.ts:180)
• ✅ Test updated to verify the new scope
• ✅ Consistent across the codebase

4. Code Quality

• ✅ Clean, readable code with good comments
• ✅ No TypeScript any types (follows project guidelines)
• ✅ Proper error handling preserved
• ✅ Follows existing code patterns

---

🔍 Areas for Consideration

1. Comment Accuracy (Minor)

File: dev-auth/oidc-provider.mjs:68

The comment says:

// Disable built-in dev interactions so our custom interaction logic below
// (auto-login + auto-consent that grants offline_access) takes effect

Issue: The comment implies that disabling devInteractions is required for the custom interaction logic to work. However, the custom interaction logic (lines 90-150) uses the oidc.use() middleware, which should work regardless of the devInteractions setting.

Suggestion: Clarify the comment to explain the actual reason for disabling it. For example:

// Disable built-in dev interactions in favor of our custom auto-login/auto-consent logic
// that always grants offline_access for testing refresh tokens

2. Scope Validation Logic (Enhancement Opportunity)

File: dev-auth/oidc-provider.mjs:117-131

The current scope granting logic is defensive but verbose:

const requestedScopes = interaction.params.scope
  ?.split(" ")
  .filter(Boolean) || ["openid", "email", "profile"];
const allowedScopes = ["openid", "email", "profile", "offline_access"];
const grantedScopes = requestedScopes.filter((s) =>
  allowedScopes.includes(s),
);
// Ensure offline_access is granted in dev so refresh tokens are issued
if (!grantedScopes.includes("offline_access")) {
  grantedScopes.push("offline_access");
}
const scopeString = grantedScopes.length
  ? grantedScopes.join(" ")
  : "openid email profile";
grant.addOIDCScope(scopeString);

Observation: The fallback "openid email profile" (line 131) can never be reached because:

1. If grantedScopes has elements from the filter, it's non-empty
2. If the filter returns empty, we push offline_access (line 127)

Suggestion: Simplify the logic since we always ensure offline_access is present in dev:

const requestedScopes = interaction.params.scope
  ?.split(" ")
  .filter(Boolean) || ["openid", "email", "profile"];
const allowedScopes = ["openid", "email", "profile", "offline_access"];
const grantedScopes = requestedScopes.filter((s) =>
  allowedScopes.includes(s),
);
// Ensure offline_access is always granted in dev for testing refresh tokens
if (!grantedScopes.includes("offline_access")) {
  grantedScopes.push("offline_access");
}
grant.addOIDCScope(grantedScopes.join(" "));

This is purely a code clarity enhancement and doesn't affect functionality.

3. Refresh Token Configuration Validation

File: dev-auth/oidc-provider.mjs:38

The configuration includes grant_types: ["authorization_code", "refresh_token"], which is correct. However, it would be good to verify that Better Auth is actually using the refresh tokens.

Question: Have you tested that:

1. The frontend receives a refresh token after sign-in?
2. Better Auth's refreshAccessToken() function (in src/lib/auth/auth.ts:76) is being called when the access token expires?
3. The refresh flow works end-to-end in the local dev environment?

If not tested yet, I'd recommend manual testing or adding integration tests for this flow.

4. Documentation

File: CLAUDE.md or README.md

Consider adding a brief note in the documentation about refresh token behavior in development vs. production:

• Dev: 15-second access tokens force frequent refreshes
• Prod: Longer-lived tokens (configuration dependent)

This would help future contributors understand why the dev environment behaves differently.

---

🔒 Security Review

✅ No Security Issues Found

1. Refresh tokens properly scoped: The offline_access scope is correctly used to gate refresh token issuance
2. Dev-only behavior: The forced refresh token issuance (issueRefreshToken: async () => true) is clearly marked as dev-only and won't affect production
3. Consistent with production auth: The Better Auth configuration in src/lib/auth/auth.ts already includes offline_access in scopes (line 180), so this PR brings dev environment in line with production expectations
4. Encrypted storage: The refresh tokens are stored encrypted via Better Auth's saveTokenCookie() mechanism (see src/lib/auth/auth.ts:29)
5. HTTP-only cookies: Tokens are stored in HTTP-only cookies (line 34), preventing XSS attacks

---

📋 Testing

✅ Test Coverage Updated

The test in src/app/signin/__tests__/signin.test.tsx was properly updated to include the new offline_access scope in the expected call parameters (line 53).

💡 Suggestion: Integration Test

Consider adding an integration test that verifies:

1. Access token expires (or mock the expiration)
2. Refresh token is used to obtain a new access token
3. User remains authenticated throughout the process

This would give confidence that the full refresh flow works end-to-end.

---

🎨 Code Style

All changes follow the project's code style guidelines:

• ✅ No any types (TypeScript strict mode)
• ✅ Clear, descriptive comments
• ✅ Consistent formatting
• ✅ Follows existing patterns in the codebase

---

🚀 Recommendation

Approve with minor suggestions. The core implementation is solid and the changes are safe to merge. The suggestions above are all optional enhancements that would improve code clarity and documentation, but they don't block the PR.

Priority for follow-up:

1. Must verify: Manual testing of the refresh token flow in dev environment
2. Should do: Clarify the devInteractions comment (1-minute fix)
3. Nice to have: Simplify scope validation logic and add documentation

---

Summary of Changes

• ✅ OIDC provider configured for refresh tokens
• ✅ 15-second access tokens for dev testing
• ✅ Auto-grants offline_access in consent flow
• ✅ Client-side scopes updated
• ✅ Tests updated

Great work on improving the dev environment's authentication fidelity! 🎉

---

Job Run: [Link to run](https://github.com/stacklok/toolhive-cloud-ui/actions/runs/${{ github.run_id }})
Branch: configure-refresh-token

[Copilot]

Pull Request Overview

This PR adds refresh token support to the local OIDC provider for development, enabling testing of token refresh flows. The changes add the offline_access scope to OAuth2 authentication requests, configure the dev OIDC provider to issue refresh tokens with very short-lived access tokens (15 seconds), and update the corresponding tests.

• Adds offline_access scope to OAuth2 signin flow in both implementation and tests
• Configures dev OIDC provider to support refresh tokens with shortened access token TTL
• Updates auto-consent logic to always grant offline_access in development

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/app/signin/page.tsx	Adds offline_access scope to the OAuth2 signin call to request refresh tokens
src/app/signin/tests/signin.test.tsx	Updates test assertions to include the new offline_access scope in expected call parameters
dev-auth/oidc-provider.mjs	Configures OIDC provider to support refresh tokens: declares offline_access scope, shortens access token TTL to 15s, disables built-in dev interactions, and updates auto-consent logic to always grant offline_access