feat(auth): handle oidc provider signout

[peppescg]

Kapture.2025-12-03.at.10.33.10.mp4

OIDC RP-Initiated Logout

Summary

Implements complete sign-out from the OIDC provider (Okta) in addition to local session cleanup. Previously, users were only signed out locally, leaving their SSO session active.

What Changed

• OIDC Discovery: Now fetches and caches the end_session_endpoint from the OIDC provider
• ID Token Storage: The id_token is now stored in the encrypted OIDC cookie for use during logout
• Sign-out Flow:
  1. Builds the OIDC logout URL with id_token_hint and post_logout_redirect_uri
  2. Clears local OIDC token cookie
  3. Redirects to OIDC provider's logout endpoint
  4. OIDC provider terminates SSO session and redirects back to /signin

Result

Users are now fully signed out from Okta SSO. On next sign-in, they will be prompted for credentials again.

Tests

Added unit tests to verify the sign-out flow executes in the correct order and handles errors gracefully.

Large PR Justification

• Increase test coverage
• Split logic in the utils

[Copilot]

Pull request overview

This PR implements OIDC RP-Initiated Logout to properly sign users out from both the local application session and the OIDC provider (Okta) SSO session. Previously, only local sign-out was performed, leaving the SSO session active.

Key Changes:

• Added OIDC discovery caching for the end_session_endpoint and ID token storage in the encrypted cookie for logout
• Implemented server action getOidcSignOutUrl() to construct the OIDC provider logout URL with proper parameters
• Modified client-side signOut() function to redirect users to the OIDC provider's logout endpoint before completing local cleanup

Reviewed changes

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

Show a summary per file

File	Description
src/lib/auth/types.ts	Extended OidcTokenData to include idToken and updated OidcDiscovery interface with logout endpoint
src/lib/auth/constants.ts	Made OIDC_ISSUER_URL private and added OIDC_DISCOVERY_URL constant; renamed cookie constant
src/lib/auth/utils.ts	Added getOidcIdToken() function, updated saveAccountToken() to store ID token, fixed type guard for accessTokenExpiresAt
src/lib/auth/auth.ts	Enhanced getOidcDiscovery() to cache logout endpoint, updated refreshAccessToken() signature to handle ID token, enabled debug mode
src/lib/auth/actions.ts	New file with server actions for clearOidcTokenAction() and getOidcSignOutUrl()
src/lib/auth/auth-client.ts	Refactored signOut() to implement OIDC logout flow with redirect to provider
src/app/api/auth/refresh-token/route.ts	Added session validation before token refresh and user ID verification
src/lib/auth/__tests__/*.test.ts	Updated test data to match new OidcTokenData structure with additional fields
src/lib/auth/__tests__/auth-client.test.ts	New test suite for client-side sign-out flow
src/app/catalog/page.tsx	Removed redundant auth check (already protected by getAuthenticatedClient())
src/app/signin/signin-button.tsx	Added cursor-pointer class to button
biome.json	Updated schema version to 2.3.8

[peppescg]

@claude could you review the PR?

[claude]

Claude finished @peppescg's task —— View job

---

Code Review Complete ✅

I've completed a thorough review of this PR implementing OIDC RP-Initiated Logout. Overall, this is a solid implementation with good architectural decisions. Below are my findings:

---

🟢 Strengths

1. Good architectural separation: The code is well-organized with separate modules for cookie.ts, crypto.ts, and actions.ts, which helps avoid circular dependencies.

2. Proper security practices:

   • Tokens are encrypted using JWE with AES-256-GCM
   • HTTP-only cookies prevent XSS attacks
   • Type guards validate decrypted data structure
   • Session validation before token operations

3. Error handling: Comprehensive try-catch blocks with fallback behavior to /signin on errors.

4. TypeScript strict mode compliance: No any types found ✅

5. Good test coverage: The test suite validates the sign-out flow and error scenarios.

---

🟡 Critical Issue: Sign-Out Flow Logic

Location: src/lib/auth/auth-client.ts lines 20-42

There's a critical logic error in the sign-out sequence. The current code:

// 1. Get logout URL FIRST (while session still exists)
const redirectUrl = await getOidcSignOutUrl();

// 2. Clear OIDC token cookie AFTER BA session is gone
await clearOidcTokenAction();

// 3. Redirect to OIDC provider logout
window.location.replace(redirectUrl);
// 4. Sign out from Better Auth (invalidates session)
await authClient.signOut();

The Problem: Line 29 calls window.location.replace(redirectUrl) which causes an immediate page navigation. This means line 31 (await authClient.signOut()) should never execute in real browser behavior.

Why the test passes: The test mocks window.location.replace so it doesn't actually navigate. In the test environment, the function continues executing. But in a real browser, the page navigates away immediately.

Author's Response Analysis: You stated "that's not true the await authClient.signOut(); will always be run" - but this contradicts how window.location.replace() works in browsers. According to MDN:

"The replace() method replaces the current resource with the one at the provided URL. The difference from the assign() method is that after using replace() the current page will not be saved in session History."

While location.replace() doesn't synchronously halt execution like location.href might in some browsers, relying on code after navigation is unreliable because:

1. The browser begins navigating immediately
2. Pending async operations may be cancelled
3. The execution context may be torn down before promises resolve

Recommended Fix:

export const signOut = async () => {
  try {
    // 1. Get logout URL FIRST (while session still exists)
    const redirectUrl = await getOidcSignOutUrl();

    // 2. Sign out from Better Auth (invalidates session)
    await authClient.signOut();

    // 3. Clear OIDC token cookie AFTER BA session is gone
    await clearOidcTokenAction();

    // 4. Finally redirect to OIDC provider logout
    window.location.replace(redirectUrl);
  } catch (error) {
    console.error("[Auth] Sign out error:", error);
    toast.error("Sign out failed", {
      description:
        error instanceof Error ? error.message : "An unexpected error occurred",
    });

    // Fallback redirect on error
    window.location.replace("/signin");
  }
};

This ensures all cleanup happens before navigation, making the behavior predictable and testable.

---

🟡 Medium Priority Issues

1. Debug Mode in Production

Location: src/lib/auth/auth.ts line 169

debug: !IS_PRODUCTION,

Good! This correctly disables debug mode in production. However, there's still a debug log statement in:

Location: src/lib/auth/utils.ts line 80

console.log("[Save Token] Token cookie saved successfully");

Recommendation: Remove this or make it conditional:

if (!IS_PRODUCTION) {
  console.log("[Save Token] Token cookie saved successfully");
}

2. Incomplete Cache Validation

Location: src/lib/auth/auth.ts lines 40-45

if (cachedTokenEndpoint) {
  return {
    tokenEndpoint: cachedTokenEndpoint,
    endSessionEndpoint: cachedEndSessionEndpoint,
  };
}

Issue: The function caches both cachedTokenEndpoint and cachedEndSessionEndpoint (lines 59-60), but only checks if cachedTokenEndpoint exists. If the discovery fetch partially fails (gets token endpoint but not logout endpoint), subsequent calls will return stale/null endSessionEndpoint.

Recommended Fix:

if (cachedTokenEndpoint && cachedEndSessionEndpoint) {
  return {
    tokenEndpoint: cachedTokenEndpoint,
    endSessionEndpoint: cachedEndSessionEndpoint,
  };
}

3. Type Definition Complexity

Location: src/lib/auth/types.ts lines 10-28

The OidcTokenData interface extends Account while omitting several fields, then re-declaring some as optional with different types. This is fragile because:

1. If Account type changes in better-auth, this could break
2. The type conversion from Date to number happens at runtime, not at the type level
3. Creates an implicit dependency on better-auth internals

Recommendation: Consider defining OidcTokenData explicitly without extending Account:

export interface OidcTokenData {
  userId: string;
  accessToken: string;
  accessTokenExpiresAt: number;
  refreshToken?: string;
  refreshTokenExpiresAt?: number;
  idToken?: string;
  // Include only fields you actually store
}

This makes the contract clear and avoids implicit dependencies.

---

🟢 Minor/Nitpick Issues

1. Redundant Property Assignments

Location: src/lib/auth/utils.ts lines 69-76

const tokenData: OidcTokenData = {
  ...account,
  accessToken: account.accessToken,  // Already spread above
  refreshToken: account.refreshToken || undefined,  // Already spread above
  accessTokenExpiresAt,
  refreshTokenExpiresAt,
  userId: account.userId,  // Already spread above
};

The spread operator already includes these properties. Only assign if you're transforming them:

const tokenData: OidcTokenData = {
  ...account,
  accessTokenExpiresAt,
  refreshTokenExpiresAt,
  refreshToken: account.refreshToken || undefined,
};

2. OIDC_ISSUER_URL Visibility

Location: src/lib/auth/constants.ts line 11

The constant was changed from export const to const, making it private. This is good for encapsulation, but ensure no other files were importing it directly. The URL is still exposed via OIDC_DISCOVERY_URL (line 16), so this is more of an organizational change than a security improvement.

---

🟢 Positive Changes Worth Highlighting

1. Good refactoring: Splitting crypto and cookie logic into separate modules prevents circular dependencies while maintaining clean imports.

2. Proper ID token handling: Storing the ID token in the cookie for logout is the correct approach for RP-Initiated Logout.

3. Fallback handling: The sign-out flow gracefully falls back to /signin when OIDC logout is unavailable.

4. Session validation: The refresh token route properly validates the Better Auth session before attempting token refresh (lines 19-27).

---

📝 Summary

Must Fix Before Merge:

• ❗ Sign-out flow order (lines 23-31 in auth-client.ts) - window.location.replace() should be last

Should Fix:

• Debug console.log in production code
• Cache validation logic for discovery endpoints

Nice to Have:

• Simplify OidcTokenData type definition
• Remove redundant property assignments

Question for Author:
Have you tested the sign-out flow in a real browser (not just unit tests)? The authClient.signOut() call after window.location.replace() is unreliable in production environments.

---

Job Run: View logs
Branch: issues/57

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]

- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details on the pull request process.

---

This review will be automatically dismissed once you add the justification section.