feat: handle replayed OIDC callbacks and improve auth error handling

.changeset/oauth-callback-back-button.md

@@ -0,0 +1,5 @@
+---
+'@truefoundry/trueforge': patch
+---
+
+Treat a replayed OIDC callback (browser Back after a successful login) as already signed-in instead of `/?error=login_failed`, and ignore that stale query when a session is still valid.

packages/frontend/src/App.tsx

@@ -9,7 +9,7 @@ import { useEffect, useMemo, useState } from 'react';
 import { AuthErrorScreen } from './AuthErrorScreen';
 import { createAuthAwareFetch } from './authFetch';
 import { probeSession, type SessionState } from './authSession';
-import { parseAuthErrorReason } from './authStatusSearch';
+import { parseAuthErrorReason, shouldShowAuthErrorScreen, stripAuthErrorSearch } from './authStatusSearch';
 import { GetStartedScreen } from './GetStartedScreen';
 import { LogoutButton } from './LogoutButton';
 
@@ -29,10 +29,9 @@ export function App() {
 
   // Gate boot on a non-redirecting `/me` probe: unauthenticated users see the
   // welcome screen instead of being bounced to login by the auth-aware fetch.
+  // Probe even when the URL has `?error=` — a replayed callback can leave that
+  // query on a session that is still valid.
   useEffect(() => {
-    if (authError != null) {
-      return;
-    }
     const state = { cancelled: false };
     void probeSession().then(result => {
       if (!state.cancelled) {
@@ -42,7 +41,25 @@ export function App() {
     return () => {
       state.cancelled = true;
     };
-  }, [authError]);
+  }, []);
+
+  useEffect(() => {
+    if (session !== 'authenticated') {
+      return;
+    }
+    if (parseAuthErrorReason(window.location.search) == null) {
+      return;
+    }
+    window.history.replaceState(
+      window.history.state,
+      '',
+      stripAuthErrorSearch({
+        pathname: window.location.pathname,
+        search: window.location.search,
+        hash: window.location.hash,
+      }),
+    );
+  }, [session]);
 
   useEffect(() => {
     if (session !== 'authenticated') {
@@ -98,10 +115,11 @@ export function App() {
 
   const overrides: SlotOverrides = useMemo(() => ({ ShellActionsActionSlot: LogoutButton }), []);
 
-  if (authError != null) {
+  const authErrorReason = shouldShowAuthErrorScreen({ authError, session });
+  if (authErrorReason != null) {
     return (
       <ThemeProvider>
-        <AuthErrorScreen reason={authError} />
+        <AuthErrorScreen reason={authErrorReason} />
       </ThemeProvider>
     );
   }

packages/frontend/src/authStatusSearch.ts

@@ -1,3 +1,5 @@
+import type { SessionState } from './authSession';
+
 /** Reads `/?error=<reason>` from OIDC login failures. Returns null when not an error landing. */
 export function parseAuthErrorReason(search: string): string | null {
   const reason = new URLSearchParams(search).get('error')?.trim();
@@ -6,3 +8,33 @@ export function parseAuthErrorReason(search: string): string | null {
   }
   return reason;
 }
+
+/** Returns the error reason when the auth error screen should show; otherwise null. */
+export function shouldShowAuthErrorScreen({
+  authError,
+  session,
+}: {
+  authError: string | null;
+  session: SessionState | 'checking';
+}): string | null {
+  if (authError == null || session !== 'unauthenticated') {
+    return null;
+  }
+  return authError;
+}
+
+/** Path + search + hash with the OIDC `error` query removed. */
+export function stripAuthErrorSearch({
+  pathname,
+  search,
+  hash,
+}: {
+  pathname: string;
+  search: string;
+  hash: string;
+}): string {
+  const params = new URLSearchParams(search);
+  params.delete('error');
+  const nextSearch = params.toString();
+  return `${pathname}${nextSearch === '' ? '' : `?${nextSearch}`}${hash}`;
+}

packages/frontend/tests/authStatusSearch.test.ts

@@ -0,0 +1,30 @@
+import assert from 'node:assert/strict';
+import { describe, it } from 'node:test';
+import { parseAuthErrorReason, shouldShowAuthErrorScreen, stripAuthErrorSearch } from '../src/authStatusSearch';
+
+describe('authStatusSearch', () => {
+  it('parseAuthErrorReason reads a non-empty error query', () => {
+    assert.equal(parseAuthErrorReason('?error=login_failed'), 'login_failed');
+    assert.equal(parseAuthErrorReason('?error=%20'), null);
+    assert.equal(parseAuthErrorReason(''), null);
+  });
+
+  it('shouldShowAuthErrorScreen only for unauthenticated error landings', () => {
+    assert.equal(shouldShowAuthErrorScreen({ authError: 'login_failed', session: 'unauthenticated' }), 'login_failed');
+    assert.equal(shouldShowAuthErrorScreen({ authError: 'login_failed', session: 'checking' }), null);
+    assert.equal(shouldShowAuthErrorScreen({ authError: 'login_failed', session: 'authenticated' }), null);
+    assert.equal(shouldShowAuthErrorScreen({ authError: null, session: 'unauthenticated' }), null);
+  });
+
+  it('stripAuthErrorSearch drops error and keeps other query and hash', () => {
+    assert.equal(stripAuthErrorSearch({ pathname: '/', search: '?error=login_failed', hash: '' }), '/');
+    assert.equal(
+      stripAuthErrorSearch({
+        pathname: '/sessions/abc',
+        search: '?error=login_failed&tab=1',
+        hash: '#composer',
+      }),
+      '/sessions/abc?tab=1#composer',
+    );
+  });
+});

packages/trueforge/src/apis/auth.ts

@@ -4,7 +4,7 @@ import type { Configuration } from 'openid-client';
 import type { Logger } from 'winston';
 import { clearAuthCookie, ID_TOKEN_COOKIE, OAUTH_STATE_COOKIE, readOAuthStateCookie } from '../auth/cookies';
 import { resolveUserContext } from '../auth/identity';
-import { authMiddleware } from '../auth/middleware';
+import { authMiddleware, resolveAuthUser } from '../auth/middleware';
 import { buildLoginAuthorization, exchangeAuthorizationCode, getOidcVerify } from '../auth/oidc';
 import { safeReturnTo } from '../auth/safeReturnTo';
 import { authLoginRoute, authLogoutRoute, meRoute, oAuthCallbackRoute } from '../routes/authRoutes';
@@ -51,6 +51,10 @@ export function createAuthRouter(params: { oidcClient: Configuration | undefined
     clearAuthCookie({ context: c, name: OAUTH_STATE_COOKIE });
 
     if (pending?.state !== query.state || query.error || !query.code) {
+      // If already authenticated, redirect home instead of showing an error.
+      if (await resolveAuthUser(c)) {
+        return c.redirect('/', 302);
+      }
       // Only reflect a non-empty IdP `error_description` when the IdP returned an error.
       // No fallback to the `error` code — blank/missing descriptions use the default.
       // Our own validation failures (state mismatch / missing code) stay generic too.
@@ -70,6 +74,9 @@ export function createAuthRouter(params: { oidcClient: Configuration | undefined
       return c.redirect(safeReturnTo(pending.return_to), 302);
     } catch (error) {
       params.logger.error('Failed to exchange authorization code', extractErrorLogFields(error));
+      if (await resolveAuthUser(c)) {
+        return c.redirect(safeReturnTo(pending.return_to), 302);
+      }
       const reason = error instanceof Error ? error.message : 'login_failed';
       return c.redirect(oauthErrorRedirect(reason), 302);
     }

packages/trueforge/tests/unit/apis/auth.test.ts

@@ -303,6 +303,54 @@ describe('auth router (auth enabled)', () => {
     expect(res.headers.get('location')).toBe('/?error=login_failed');
   });
 
+  it('GET /callback replays home when already authenticated and the state cookie is spent', async () => {
+    const token = await createIdToken();
+    const res = await createAuthRouter({ oidcClient, logger }).request('/callback?code=abc&state=spent', {
+      redirect: 'manual',
+      headers: { Cookie: `${ID_TOKEN_COOKIE}=${token}` },
+    });
+    expect(res.status).toBe(302);
+    expect(res.headers.get('location')).toBe('/');
+  });
+
+  it('GET /callback replays home when already authenticated even if the IdP returned an error', async () => {
+    const token = await createIdToken();
+    const res = await createAuthRouter({ oidcClient, logger }).request(
+      '/callback?state=any&error=access_denied&error_description=user%20cancelled',
+      {
+        redirect: 'manual',
+        headers: { Cookie: `${ID_TOKEN_COOKIE}=${token}` },
+      },
+    );
+    expect(res.status).toBe(302);
+    expect(res.headers.get('location')).toBe('/');
+  });
+
+  it('GET /callback keeps the existing session when code exchange fails', async () => {
+    const token = await createIdToken();
+    const router = createAuthRouter({ oidcClient, logger });
+    const loginRes = await router.request('/login?return_to=/sessions/abc123', { redirect: 'manual' });
+    const stateCookieRaw = cookieValue(setCookies(loginRes), STATE_COOKIE) ?? '';
+    const authorizationUrl = new URL(loginRes.headers.get('location') ?? '');
+    const state = authorizationUrl.searchParams.get('state') ?? '';
+
+    const fetchStub = globalThis.fetch;
+    const failingFetch: typeof fetch = async (input, init) => {
+      if (String(input) === `${ISSUER}/token` && init?.method === 'POST') {
+        return new Response('invalid_grant', { status: 400 });
+      }
+      return fetchStub(input, init);
+    };
+    globalThis.fetch = failingFetch;
+
+    const res = await router.request(`/callback?code=abc&state=${state}&iss=${encodeURIComponent(ISSUER)}`, {
+      redirect: 'manual',
+      headers: { Cookie: `${STATE_COOKIE}=${stateCookieRaw}; ${ID_TOKEN_COOKIE}=${token}` },
+    });
+    expect(res.status).toBe(302);
+    expect(res.headers.get('location')).toBe('/sessions/abc123');
+  });
+
   it('POST /logout clears id_token even when no cookie is present', async () => {
     const res = await createAuthRouter({ oidcClient, logger }).request('/logout', {
       method: 'POST',