feat(auth): handle oidc provider signout #132

peppescg · 2025-12-03T09:37:51Z

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.

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

Copilot · 2025-12-03T09:45:52Z

src/lib/auth/__tests__/auth-client.test.ts

+ "clearOidcTokenAction",
+ "window.location.replace",
+ "authClient.signOut",


Test validates incorrect behavior

This test verifies the execution order expects window.location.replace to happen before authClient.signOut (lines 94-99). However, window.location.replace() causes an immediate browser redirect, making any code after it unreachable.

This test passes because mocks don't actually redirect, but in real execution, authClient.signOut() would never be called. The test is validating broken behavior instead of catching the bug.

Expected order should be:

expect(callOrder).toEqual([ "getOidcSignOutUrl", "authClient.signOut", // Sign out BEFORE redirect "clearOidcTokenAction", "window.location.replace", // Redirect LAST ]);

Suggested change

"clearOidcTokenAction",

"window.location.replace",

"authClient.signOut",

"authClient.signOut",

"clearOidcTokenAction",

"window.location.replace",

Copilot · 2025-12-03T09:45:52Z

src/lib/auth/utils.ts

 userId: account.userId,
 };

+ console.log("[account] Token data to save:", JSON.stringify(account));


Debug logging should be removed or placed behind feature flag

This debug log statement logs the entire account object, which may contain sensitive data. Line 182 also enables debug: true globally. Debug logging should either:

Be removed before production

Be conditional based on NODE_ENV !== 'production'

Avoid logging sensitive data like tokens

Suggested fix:

if (process.env.NODE_ENV !== 'production') { console.log("[account] Account saved with token"); }

Copilot · 2025-12-03T09:45:52Z

src/lib/auth/utils.ts

 return (
 typeof obj.accessToken === "string" &&
- typeof obj.expiresAt === "number" &&
+ typeof obj.accessTokenExpiresAt === "number" &&
 typeof obj.userId === "string" &&
 (obj.refreshToken === undefined || typeof obj.refreshToken === "string") &&
 (obj.refreshTokenExpiresAt === undefined ||
 typeof obj.refreshTokenExpiresAt === "number")
 );


Type guard doesn't validate all required properties from OidcTokenData

The OidcTokenData interface (via Account extension) includes many properties like accessToken, refreshToken, userId, etc. However, this type guard only validates:

accessToken (string)

accessTokenExpiresAt (number)

userId (string)

Optional: refreshToken, refreshTokenExpiresAt

According to the type definition, accessTokenExpiresAt is actually optional (accessTokenExpiresAt?: number), but this guard requires it to be present. This inconsistency could cause valid token data to be rejected.

Suggested fix:

return ( typeof obj.accessToken === "string" && (obj.accessTokenExpiresAt === undefined || typeof obj.accessTokenExpiresAt === "number") && typeof obj.userId === "string" && (obj.refreshToken === undefined || typeof obj.refreshToken === "string") && (obj.refreshTokenExpiresAt === undefined || typeof obj.refreshTokenExpiresAt === "number") );

Copilot · 2025-12-03T09:45:53Z

src/lib/auth/auth.ts

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


Incomplete cache invalidation logic

The function caches both cachedTokenEndpoint and cachedEndSessionEndpoint (lines 72-73), but only checks if cachedTokenEndpoint exists (line 53). If cachedEndSessionEndpoint fails to fetch but cachedTokenEndpoint succeeds, subsequent calls will return stale/null endSessionEndpoint.

Suggested fix:

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

Or cache both values together as a single object.

Copilot · 2025-12-03T09:45:53Z

src/lib/auth/types.ts

+export interface OidcTokenData
+ extends Omit<
+ Account,
+ | "accessTokenExpiresAt"
+ | "accountId"
+ | "providerId"
+ | "refreshTokenExpiresAt"
+ | "createdAt"
+ | "updatedAt"
+ | "id"
+ > {
+ accessTokenExpiresAt?: number;
 refreshTokenExpiresAt?: number;
- userId: string;
+ providerId?: string;
+ accountId?: string;
+ updatedAt?: Date;
+ createdAt?: Date;
+ id?: string;
 }


Type definition is overly complex and potentially inconsistent

The OidcTokenData interface extends Account while:

Omitting several fields (accessTokenExpiresAt, refreshTokenExpiresAt, etc.)

Then re-declaring them as optional with different types (e.g., accessTokenExpiresAt?: number instead of Date)

This pattern is error-prone because:

If Account type changes in better-auth, this could break

The type conversion from Date to number happens at runtime in saveAccountToken, not at the type level

Optional fields that should be required (like userId, accessToken) are being spread from Account without validation

Suggested approach:
Define 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 on better-auth's internal types.

Copilot · 2025-12-03T09:45:54Z

src/lib/auth/auth-client.ts

+ // 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();


Critical Bug: Sign-out order is incorrect

The sign-out flow has a critical logic error:

Line 23: Gets logout URL ✓

Line 26: Clears OIDC token cookie ✓

Line 29: window.location.replace() redirects immediately ❌

Line 31: authClient.signOut() never executes because the browser has already redirected

Expected order:

// 1. Get logout URL FIRST (while session exists) const redirectUrl = await getOidcSignOutUrl(); // 2. Sign out from Better Auth (invalidates session) await authClient.signOut(); // 3. Clear OIDC token cookie await clearOidcTokenAction(); // 4. Finally redirect to OIDC provider logout window.location.replace(redirectUrl);

The comment on line 25 even states "Clear OIDC token cookie AFTER BA session is gone", but the Better Auth sign-out never happens.

Suggested change

// 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();

// 2. Sign out from Better Auth (invalidates session)

await authClient.signOut();

// 3. Clear OIDC token cookie AFTER BA session is gone

await clearOidcTokenAction();

// 4. Redirect to OIDC provider logout

window.location.replace(redirectUrl);

Copilot · 2025-12-03T09:45:54Z

src/lib/auth/utils.ts

 const tokenData: OidcTokenData = {
+ ...account,
 accessToken: account.accessToken,
 refreshToken: account.refreshToken || undefined,
- expiresAt,
+ accessTokenExpiresAt,
 refreshTokenExpiresAt,
 userId: account.userId,
 };


Redundant property assignments after spread operator

Lines 148-153 use the spread operator ...account which already includes accessToken, refreshToken, and userId, then immediately reassign these same properties. This is redundant and could cause confusion.

Suggested fix:

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

This ensures the timestamp conversions and undefined coercion for refreshToken are applied correctly without redundant assignments.

Copilot · 2025-12-03T09:45:54Z

src/lib/auth/utils.ts

 import * as jose from "jose";
-import { TOKEN_ONE_HOUR_MS } from "./constants";
+import { cookies } from "next/headers";
+import { saveTokenCookie } from "./auth";


Potential circular dependency with saveTokenCookie import

The file imports saveTokenCookie from ./auth at line 9, and ./auth imports functions from ./utils. This creates a circular dependency pattern:

utils.ts imports from auth.ts (line 9)

auth.ts imports from utils.ts (line 23)

While the previous code had a comment about avoiding this with dynamic imports, the new code removes the dynamic import strategy. This could cause module initialization issues.

Recommendation: Consider moving saveTokenCookie to a separate file (e.g., cookie.ts) or re-introducing the dynamic import to break the cycle.

Suggested change

import { saveTokenCookie } from "./auth";

// import { saveTokenCookie } from "./auth"; // Removed to break circular dependency

Copilot · 2025-12-03T09:45:54Z

src/lib/auth/auth.ts

 }

 export const auth: Auth<BetterAuthOptions> = betterAuth({
+ debug: true,


Debug mode enabled in production

debug: true is enabled globally, which can expose sensitive authentication flow details in production logs. This should be conditional:

debug: process.env.NODE_ENV !== "production",

Suggested change

debug: true,

debug: !IS_PRODUCTION,

Copilot · 2025-12-03T09:45:55Z

src/lib/auth/constants.ts

+const OIDC_ISSUER_URL = process.env.OIDC_ISSUER_URL || "";
 export const OIDC_CLIENT_ID = process.env.OIDC_CLIENT_ID || "";
 export const OIDC_CLIENT_SECRET = process.env.OIDC_CLIENT_SECRET || "";
 export const BASE_URL = process.env.BETTER_AUTH_URL || "http://localhost:3000";
 export const IS_PRODUCTION = process.env.NODE_ENV === "production";
+export const OIDC_DISCOVERY_URL = `${OIDC_ISSUER_URL}/.well-known/openid-configuration`;


Incorrect visibility change for OIDC_ISSUER_URL

The OIDC_ISSUER_URL export was changed from export const to const (private). However, line 16 constructs OIDC_DISCOVERY_URL from it, which IS exported. This means:

The URL is still exposed indirectly via OIDC_DISCOVERY_URL

Other parts of the codebase that may have imported OIDC_ISSUER_URL directly will break

If the intention was to hide the issuer URL for security, this doesn't achieve that. If it was to consolidate usage, ensure no other files import OIDC_ISSUER_URL directly.

peppescg added 4 commits December 3, 2025 10:30

feat: save idToken, refactor oidc account types

15303f3

chore: update biome schema

57df80f

feat: handle oidc signout

241ab13

fix: add cursor pointer to signin btn

2369d34

peppescg self-assigned this Dec 3, 2025

Copilot AI review requested due to automatic review settings December 3, 2025 09:37

peppescg linked an issue Dec 3, 2025 that may be closed by this pull request

[Task] signout from oidc provider #57

Open

peppescg added the DO NOT MERGE label Dec 3, 2025

Copilot started reviewing on behalf of peppescg December 3, 2025 09:38 View session

peppescg changed the title ~~feat(auth): handle oidc provider signout~~ feat(auth): handle oidc provider signout Dec 3, 2025

Copilot finished reviewing on behalf of peppescg December 3, 2025 09:43

Copilot AI reviewed Dec 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(auth): handle oidc provider signout #132

feat(auth): handle oidc provider signout #132

Uh oh!

peppescg commented Dec 3, 2025

Copilot AI left a comment

Copilot AI Dec 3, 2025

Copilot AI Dec 3, 2025

Copilot AI Dec 3, 2025

Copilot AI Dec 3, 2025

Copilot AI Dec 3, 2025

Copilot AI Dec 3, 2025

Copilot AI Dec 3, 2025

Copilot AI Dec 3, 2025

Copilot AI Dec 3, 2025

Copilot AI Dec 3, 2025

Labels

2 participants

	import { saveTokenCookie } from "./auth";
	// import { saveTokenCookie } from "./auth"; // Removed to break circular dependency

feat(auth): handle oidc provider signout #132

Are you sure you want to change the base?

feat(auth): handle oidc provider signout #132

Uh oh!

Conversation

peppescg commented Dec 3, 2025

OIDC RP-Initiated Logout

Summary

What Changed

Result

Tests

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

Labels

2 participants