fix(auth): standard OIDC RP behavior — auto-redirect and drop prompt=login

When OIDC is the primary auth method, the login page now auto-redirects to the OIDC provider's authorization endpoint instead of showing an intermediate "Sign in with SSO" button. The `prompt=login` parameter that forced re-authentication is removed — the provider manages its own session state. If the user already has a session (e.g. from the SaaS platform sharing the same Logto instance), the provider issues a code without showing a login form, enabling seamless SSO. Admin recovery via ?local is unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-28 21:09:38 +02:00
parent 99d9d193ac
commit 21b38d2a30
2 changed files with 103 additions and 98 deletions
--- a/ui/src/auth/LoginPage.test.tsx
+++ b/ui/src/auth/LoginPage.test.tsx
@@ -1,5 +1,5 @@
 import { describe, it, expect, beforeEach, vi } from 'vitest';
-import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import { render, screen, waitFor } from '@testing-library/react';
 import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
 import { MemoryRouter } from 'react-router';
 import type { ReactNode } from 'react';
@@ -46,18 +46,40 @@ function mockCaps(body: any) {
 describe('LoginPage', () => {
  beforeEach(() => vi.clearAllMocks());

-  it('SSO primary, no ?local: renders SSO button only and admin-recovery link, no local form', async () => {
+  it('SSO primary, no ?local: auto-redirects to OIDC provider and shows admin-recovery link', async () => {
    mockCaps({
      oidc: { enabled: true, providerName: 'Logto', primary: true },
      localAccounts: { enabled: true, adminRecoveryOnly: true },
    });

-    render(<LoginPage />, { wrapper: wrapper(['/login']) });
+    const originalLocation = window.location;
+    const hrefSetter = vi.fn();
+    Object.defineProperty(window, 'location', {
+      configurable: true,
+      value: { ...originalLocation, get href() { return ''; }, set href(v: string) { hrefSetter(v); } },
+    });

-    expect(await screen.findByRole('button', { name: /sign in with logto/i })).toBeInTheDocument();
-    expect(screen.queryByLabelText(/username/i)).toBeNull();
-    expect(screen.queryByLabelText(/password/i)).toBeNull();
-    expect(screen.getByRole('link', { name: /admin recovery/i })).toBeInTheDocument();
+    try {
+      render(<LoginPage />, { wrapper: wrapper(['/login']) });
+
+      // Auto-redirect fires without user interaction
+      await waitFor(() => expect(hrefSetter).toHaveBeenCalled());
+      const url: string = hrefSetter.mock.calls[0][0];
+      expect(url).toMatch(/^https:\/\/auth\.logto\.example\/oidc\/auth\?/);
+      expect(url).toMatch(/response_type=code/);
+      expect(url).toMatch(/client_id=spa-client/);
+
+      // No prompt parameter — let the OIDC provider manage its session
+      expect(url).not.toMatch(/prompt=/);
+
+      // Admin recovery escape hatch is still visible
+      expect(screen.getByRole('link', { name: /admin recovery/i })).toBeInTheDocument();
+      // No local form fields rendered
+      expect(screen.queryByLabelText(/username/i)).toBeNull();
+      expect(screen.queryByLabelText(/password/i)).toBeNull();
+    } finally {
+      Object.defineProperty(window, 'location', { configurable: true, value: originalLocation });
+    }
  });

  it('SSO primary, ?local present: renders local form with amber recovery banner and back-to-SSO link', async () => {
@@ -99,37 +121,7 @@ describe('LoginPage', () => {
    expect(screen.getByText(/sign-in options couldn't load/i)).toBeInTheDocument();
  });

-  it('SSO button click: navigates to authorize URL WITHOUT prompt=none', async () => {
-    mockCaps({
-      oidc: { enabled: true, providerName: 'Logto', primary: true },
-      localAccounts: { enabled: true, adminRecoveryOnly: true },
-    });
-
-    const originalLocation = window.location;
-    const hrefSetter = vi.fn();
-    Object.defineProperty(window, 'location', {
-      configurable: true,
-      value: { ...originalLocation, get href() { return ''; }, set href(v: string) { hrefSetter(v); } },
-    });
-
-    try {
-      render(<LoginPage />, { wrapper: wrapper(['/login']) });
-      const btn = await screen.findByRole('button', { name: /sign in with logto/i });
-      fireEvent.click(btn);
-
-      await waitFor(() => expect(hrefSetter).toHaveBeenCalled());
-      const url: string = hrefSetter.mock.calls[0][0];
-      expect(url).toMatch(/^https:\/\/auth\.logto\.example\/oidc\/auth\?/);
-      expect(url).not.toMatch(/prompt=none/);
-      expect(url).toMatch(/response_type=code/);
-      expect(url).toMatch(/client_id=spa-client/);
-      expect(url).toMatch(/scope=/);
-    } finally {
-      Object.defineProperty(window, 'location', { configurable: true, value: originalLocation });
-    }
-  });
-
-  it('SSO button click: when /auth/oidc/config fails, button unlocks and error is set', async () => {
+  it('SSO auto-redirect failure: shows manual SSO button as fallback', async () => {
    const setStateMock = vi.fn();
    const useAuthStoreMock = vi.mocked(useAuthStore) as unknown as { setState: typeof setStateMock };
    useAuthStoreMock.setState = setStateMock;
@@ -144,13 +136,9 @@ describe('LoginPage', () => {
    });

    render(<LoginPage />, { wrapper: wrapper(['/login']) });
-    const btn = await screen.findByRole('button', { name: /sign in with logto/i });
-    fireEvent.click(btn);

    await waitFor(() => expect(setStateMock).toHaveBeenCalled());
    const errorPayload = setStateMock.mock.calls[0][0];
    expect(errorPayload.error).toMatch(/OIDC configuration unavailable/i);
-    // Button should not stay locked in "Redirecting…"
-    await waitFor(() => expect(btn).not.toHaveTextContent(/redirecting/i));
  });
 });
--- a/ui/src/auth/LoginPage.tsx
+++ b/ui/src/auth/LoginPage.tsx
@@ -1,10 +1,10 @@
-import { type FormEvent, useMemo, useState } from 'react';
+import { type FormEvent, useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { Link, Navigate, useSearchParams } from 'react-router';
 import { useAuthStore } from './auth-store';
 import { api } from '../api/client';
 import { config } from '../config';
 import { useAuthCapabilities } from '../api/queries/auth';
-import { Card, Input, Button, Alert, FormField } from '@cameleer/design-system';
+import { Card, Input, Button, Alert, FormField, Spinner } from '@cameleer/design-system';
 import brandLogo from '@cameleer/design-system/assets/cameleer-logo.svg';
 import styles from './LoginPage.module.css';

@@ -46,6 +46,7 @@ export function LoginPage() {
  const [username, setUsername] = useState('');
  const [password, setPassword] = useState('');
  const [oidcLoading, setOidcLoading] = useState(false);
+  const autoRedirected = useRef(false);

  // Mirrors cameleer-saas: when logout sets this flag, render a "Signed out"
  // confirmation instead of the regular form. The flag is one-shot — read +
@@ -58,6 +59,61 @@ export function LoginPage() {

  const { data: caps, isError: capsFailed, isLoading: capsLoading } = useAuthCapabilities();

+  // Derive render decisions before hooks that depend on them and before
+  // any conditional returns (React rules-of-hooks).
+  const oidcPrimary = caps?.oidc?.primary === true;
+  const adminRecoveryOnly = caps?.localAccounts?.adminRecoveryOnly === true;
+  const providerName = caps?.oidc?.providerName || 'Single Sign-On';
+  const showSsoPrimary = oidcPrimary && adminRecoveryOnly && !forceLocal;
+  const showLocalForm = !oidcPrimary || forceLocal || !adminRecoveryOnly || capsFailed;
+  const showAdminRecoveryBanner = oidcPrimary && adminRecoveryOnly && forceLocal;
+
+  // Standard OIDC RP redirect — shared between auto-redirect and manual retry.
+  // No `prompt` parameter: the OIDC provider decides whether to show a login
+  // form based on its own session state.  If the user already has an active
+  // session (e.g. from the SaaS platform sharing the same provider) the
+  // provider issues a code immediately without showing a login form.
+  const triggerOidcRedirect = useCallback(async () => {
+    setOidcLoading(true);
+    try {
+      const { data } = await api.GET('/auth/oidc/config');
+      if (!data?.authorizationEndpoint || !data?.clientId) {
+        useAuthStore.setState({ error: 'OIDC configuration unavailable. Try the local form via /login?local.' });
+        return;
+      }
+      if (data.endSessionEndpoint) {
+        localStorage.setItem('cameleer-oidc-end-session', data.endSessionEndpoint);
+      }
+      if (data.clientId) {
+        localStorage.setItem('cameleer-oidc-client-id', data.clientId);
+      }
+      const redirectUri = `${window.location.origin}${config.basePath}oidc/callback`;
+      const scopes = ['openid', 'email', 'profile', ...PLATFORM_SCOPES, ...(data.additionalScopes || [])];
+      const params = new URLSearchParams({
+        response_type: 'code',
+        client_id: data.clientId,
+        redirect_uri: redirectUri,
+        scope: scopes.join(' '),
+      });
+      if (data.resource) params.set('resource', data.resource);
+      window.location.href = `${data.authorizationEndpoint}?${params}`;
+    } catch {
+      useAuthStore.setState({ error: 'OIDC configuration unavailable. Try the local form via /login?local.' });
+    } finally {
+      setOidcLoading(false);
+    }
+  }, []);
+
+  // When the OIDC provider is the primary auth method, redirect to its
+  // authorization endpoint immediately.  The provider handles session
+  // management — the user only sees a login form if no session exists.
+  useEffect(() => {
+    if (showSsoPrimary && !signedOut && !autoRedirected.current) {
+      autoRedirected.current = true;
+      triggerOidcRedirect();
+    }
+  }, [showSsoPrimary, signedOut, triggerOidcRedirect]);
+
  if (isAuthenticated) return <Navigate to="/" replace />;
  if (capsLoading) return null;

@@ -84,56 +140,11 @@ export function LoginPage() {
    );
  }

-  const oidcPrimary = caps?.oidc?.primary === true;
-  const adminRecoveryOnly = caps?.localAccounts?.adminRecoveryOnly === true;
-  const providerName = caps?.oidc?.providerName || 'Single Sign-On';
-
-  // Render decisions
-  const showSsoPrimary = oidcPrimary && adminRecoveryOnly && !forceLocal;
-  const showLocalForm = !oidcPrimary || forceLocal || !adminRecoveryOnly || capsFailed;
-  const showAdminRecoveryBanner = oidcPrimary && adminRecoveryOnly && forceLocal;
-
  const handleSubmit = (e: FormEvent) => {
    e.preventDefault();
    login(username, password);
  };

-  const handleOidcLogin = async () => {
-    setOidcLoading(true);
-    try {
-      const { data } = await api.GET('/auth/oidc/config');
-      if (!data?.authorizationEndpoint || !data?.clientId) {
-        useAuthStore.setState({ error: 'OIDC configuration unavailable. Try the local form via /login?local.' });
-        return;
-      }
-      if (data.endSessionEndpoint) {
-        localStorage.setItem('cameleer-oidc-end-session', data.endSessionEndpoint);
-      }
-      if (data.clientId) {
-        localStorage.setItem('cameleer-oidc-client-id', data.clientId);
-      }
-      const redirectUri = `${window.location.origin}${config.basePath}oidc/callback`;
-      const scopes = ['openid', 'email', 'profile', ...PLATFORM_SCOPES, ...(data.additionalScopes || [])];
-      const params = new URLSearchParams({
-        response_type: 'code',
-        client_id: data.clientId,
-        redirect_uri: redirectUri,
-        scope: scopes.join(' '),
-        // Defence-in-depth: even if RP-Initiated Logout did not fully clear
-        // the IdP session (proxy/cookie edge cases), prompt=login forces the
-        // IdP to re-prompt for credentials instead of silent re-auth.
-        // OIDC Core 1.0 §3.1.2.1.
-        prompt: 'login',
-      });
-      if (data.resource) params.set('resource', data.resource);
-      window.location.href = `${data.authorizationEndpoint}?${params}`;
-    } catch {
-      useAuthStore.setState({ error: 'OIDC configuration unavailable. Try the local form via /login?local.' });
-    } finally {
-      setOidcLoading(false);
-    }
-  };
-
  return (
    <div className={styles.page}>
      <Card className={styles.card}>
@@ -167,15 +178,21 @@ export function LoginPage() {

          {showSsoPrimary && (
            <div className={styles.socialSection}>
-              <Button
-                variant="primary"
-                className={styles.ssoButton}
-                onClick={handleOidcLogin}
-                disabled={oidcLoading}
-                type="button"
-              >
-                {oidcLoading ? 'Redirecting\u2026' : `Sign in with ${providerName}`}
-              </Button>
+              {!error ? (
+                <div style={{ display: 'flex', justifyContent: 'center', padding: '16px 0' }}>
+                  <Spinner />
+                </div>
+              ) : (
+                <Button
+                  variant="primary"
+                  className={styles.ssoButton}
+                  onClick={triggerOidcRedirect}
+                  disabled={oidcLoading}
+                  type="button"
+                >
+                  {oidcLoading ? 'Redirecting\u2026' : `Sign in with ${providerName}`}
+                </Button>
+              )}
              <Link to="/login?local" className={styles.adminRecoveryLink}>
                Admin recovery →
              </Link>