diff --git a/.changeset/login-config-race-sso-timeout.md b/.changeset/login-config-race-sso-timeout.md new file mode 100644 index 000000000..038e22666 --- /dev/null +++ b/.changeset/login-config-race-sso-timeout.md @@ -0,0 +1,21 @@ +--- +'@object-ui/auth': patch +--- + +Login-page auth-config hardening (#2625, #2626): + +- `createAuthClient.getConfig` now single-flights + caches the `/auth/config` + fetch (the login page's three consumers used to fire three requests) and + retries failures with backoff (500ms/1.5s/3.5s, 8s per-attempt abort) before + rejecting. A cold-starting environment kernel no longer strands the page + without its SSO buttons; a final failure clears the cache so later callers + retry. +- `LoginForm` holds a spinner instead of painting the password-form defaults + while config resolves — an SSO-only deployment must never flash a password + wall at JIT users who have no password. A failed config still falls back to + the password form (break-glass beats lock-out). +- `signInWithProvider` gains a 20s watchdog: a sign-in request that hangs now + rejects with a clear timeout error so the provider button recovers instead + of spinning forever. +- Removed LoginForm's duplicate "or" divider — SocialSignInButtons already + renders its own, and the stacked pair read as a rendering glitch. diff --git a/packages/auth/src/LoginForm.tsx b/packages/auth/src/LoginForm.tsx index bcd0fbb03..795192a7a 100644 --- a/packages/auth/src/LoginForm.tsx +++ b/packages/auth/src/LoginForm.tsx @@ -188,6 +188,13 @@ export function LoginForm({ // state because SSO routing is a raw fetch, not the shared `signIn` (whose // `isLoading` drives the password submit button). const [ssoSubmitting, setSsoSubmitting] = useState(false); + // Config not resolved yet: render a spinner INSTEAD of the password-form + // defaults. Painting the defaults first showed an SSO-only deployment a + // password wall (no SSO button, no enforced collapse) whenever the config + // fetch was slow/failed on first load — and platform-SSO JIT users have no + // password at all (#2625). A FAILED fetch (after the client's retries) + // still falls back to the password form: break-glass beats lock-out. + const [configPending, setConfigPending] = useState(true); useEffect(() => { let cancelled = false; @@ -206,6 +213,9 @@ export function LoginForm({ .catch(() => { // SSO is an enhancement, not required — leave the buttons/form hidden // or shown at their safe defaults. + }) + .finally(() => { + if (!cancelled) setConfigPending(false); }); return () => { cancelled = true; @@ -349,6 +359,14 @@ export function LoginForm({ />
+ {configPending ? ( + /* Auth config still resolving — hold the layout instead of painting + the password-form defaults that may be wrong for this server. */ +
+ +
+ ) : ( + <> setHasSocialProviders(hasProviders)} /> {passwordFormVisible ? ( @@ -357,8 +375,9 @@ export function LoginForm({ Server-side the number is guarded by a per-number cooldown + hourly cap — the resend button mirrors it with a countdown. */
- {hasSocialProviders && } - + {/* No divider here: SocialSignInButtons already renders its own + "or continue with email" divider under the provider buttons — + stacking a second "or" line read as a rendering glitch (#2625). */} {error && }
@@ -428,8 +447,9 @@ export function LoginForm({ ) : (
- {hasSocialProviders && } - + {/* No divider here: SocialSignInButtons already renders its own + "or continue with email" divider under the provider buttons — + stacking a second "or" line read as a rendering glitch (#2625). */} {error && }
@@ -539,6 +559,8 @@ export function LoginForm({
)} + + )}
{registerUrl && !ssoEnforced && ( diff --git a/packages/auth/src/__tests__/LoginForm.test.tsx b/packages/auth/src/__tests__/LoginForm.test.tsx index 47f9a8394..d141f7f32 100644 --- a/packages/auth/src/__tests__/LoginForm.test.tsx +++ b/packages/auth/src/__tests__/LoginForm.test.tsx @@ -287,3 +287,29 @@ describe('LoginForm — SSO button pending state (objectui#2458 item 1)', () => } }); }); + +describe('LoginForm — config-loading gate (#2625)', () => { + it('holds a loading state instead of painting the password form while config resolves', async () => { + let resolveConfig!: (c: AuthPublicConfig) => void; + const pending = new Promise((resolve) => { resolveConfig = resolve; }); + renderLogin(createMockClient({}, { getConfig: vi.fn().mockReturnValue(pending) })); + + // While pending: spinner, NO password form (an SSO-only server must never + // flash a password wall at users who have no password). + expect(screen.getByTestId('login-config-loading')).toBeTruthy(); + expect(screen.queryByLabelText('Email')).toBeNull(); + + resolveConfig({ features: { ssoEnforced: true } }); + await waitFor(() => expect(screen.queryByTestId('login-config-loading')).toBeNull()); + // Enforced mode honoured on FIRST paint after resolve: password form + // hidden, break-glass link offered. + expect(screen.queryByLabelText('Email')).toBeNull(); + expect(screen.getByRole('button', { name: 'Use a password instead' })).toBeTruthy(); + }); + + it('falls back to the password form when config ultimately fails (break-glass beats lock-out)', async () => { + renderLogin(createMockClient({}, { getConfig: vi.fn().mockRejectedValue(new Error('config down')) })); + await screen.findByLabelText('Email'); + expect(screen.queryByTestId('login-config-loading')).toBeNull(); + }); +}); diff --git a/packages/auth/src/__tests__/createAuthClient.test.ts b/packages/auth/src/__tests__/createAuthClient.test.ts index 3641d3b88..5c1843fb7 100644 --- a/packages/auth/src/__tests__/createAuthClient.test.ts +++ b/packages/auth/src/__tests__/createAuthClient.test.ts @@ -456,3 +456,91 @@ describe('createAuthClient — signInWithProvider redirect contract (objectui#24 ).rejects.toThrow('Provider not found'); }); }); + +describe('createAuthClient — getConfig single-flight / cache / retry (#2625)', () => { + it('single-flights concurrent callers and caches the success', async () => { + const { mockFn, calls } = createMockFetch({ + '/config': { body: { data: { features: { sso: true } } } }, + }); + const client = createAuthClient({ baseURL: 'http://localhost/api/auth', fetchFn: mockFn }); + const [a, b] = await Promise.all([client.getConfig(), client.getConfig()]); + const c = await client.getConfig(); + expect(a.features?.sso).toBe(true); + expect(b).toBe(a); + expect(c).toBe(a); + // Three consumers, ONE request — the login page used to fire three. + expect(calls.filter((x) => x.url.includes('/config'))).toHaveLength(1); + }); + + it('retries a failing config fetch with backoff before resolving', async () => { + vi.useFakeTimers(); + try { + let attempts = 0; + const mockFn = vi.fn(async () => { + attempts += 1; + if (attempts < 3) { + return new Response(JSON.stringify({ message: 'cold start' }), { status: 503 }); + } + return new Response(JSON.stringify({ data: { features: { ssoEnforced: true } } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }); + const client = createAuthClient({ baseURL: 'http://localhost/api/auth', fetchFn: mockFn }); + const pending = client.getConfig(); + // Backoff schedule is 500ms → 1500ms; advance past both. + await vi.advanceTimersByTimeAsync(500 + 1500 + 50); + const config = await pending; + expect(config.features?.ssoEnforced).toBe(true); + expect(attempts).toBe(3); + } finally { + vi.useRealTimers(); + } + }); + + it('a final failure rejects but does not poison the cache', async () => { + vi.useFakeTimers(); + try { + let attempts = 0; + const mockFn = vi.fn(async () => { + attempts += 1; + if (attempts <= 4) { + return new Response('down', { status: 503 }); + } + return new Response(JSON.stringify({ data: { features: {} } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }); + const client = createAuthClient({ baseURL: 'http://localhost/api/auth', fetchFn: mockFn }); + const first = client.getConfig(); + const firstAssertion = expect(first).rejects.toThrow(/Failed to load auth config/); + await vi.advanceTimersByTimeAsync(500 + 1500 + 3500 + 50); + await firstAssertion; + // Next caller starts a FRESH cycle (attempt 5 succeeds immediately). + const second = await client.getConfig(); + expect(second.features).toBeDefined(); + expect(attempts).toBe(5); + } finally { + vi.useRealTimers(); + } + }); +}); + +describe('createAuthClient — signInWithProvider watchdog (#2626)', () => { + it('rejects with a timeout error when the sign-in request never returns', async () => { + vi.useFakeTimers(); + try { + // A fetch that hangs forever — the cold-start failure mode where the + // provider button used to spin until a page refresh. + const mockFn = vi.fn(() => new Promise(() => { /* never settles */ })); + const client = createAuthClient({ baseURL: 'http://localhost/api/auth', fetchFn: mockFn }); + const pending = client.signInWithProvider('objectstack-cloud', { type: 'oidc' }); + const assertion = expect(pending).rejects.toThrow(/timed out/); + await vi.advanceTimersByTimeAsync(20_000 + 50); + await assertion; + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/packages/auth/src/createAuthClient.ts b/packages/auth/src/createAuthClient.ts index e3dda7d30..63185166f 100644 --- a/packages/auth/src/createAuthClient.ts +++ b/packages/auth/src/createAuthClient.ts @@ -229,6 +229,64 @@ export function createAuthClient(config: AuthClientConfig): AuthClient { return payload ?? {}; } + /** + * Fetch `GET {authUrl}/config` once, with a per-attempt timeout so a + * request that HANGS (a freshly provisioned environment whose kernel is + * still cold-starting) converts into a retryable failure instead of + * stalling the login page forever. + */ + async function fetchConfigAttempt(timeoutMs: number): Promise { + const controller = typeof AbortController !== 'undefined' ? new AbortController() : undefined; + const timer = controller ? setTimeout(() => controller.abort(), timeoutMs) : undefined; + try { + const url = `${origin}${basePath}/config`; + const response = await bearerFetch(url, { + method: 'GET', + headers: { 'Content-Type': 'application/json' }, + ...(controller ? { signal: controller.signal } : {}), + }); + if (!response.ok) { + throw new Error(`Failed to load auth config (status ${response.status})`); + } + const body = (await response.json()) as + | { success?: boolean; data?: AuthPublicConfig; error?: { message?: string } } + | AuthPublicConfig; + // Server wraps the payload as `{ success, data }`; tolerate both shapes. + if (body && typeof body === 'object' && 'data' in body && body.data) { + return body.data as AuthPublicConfig; + } + return body as AuthPublicConfig; + } finally { + if (timer !== undefined) clearTimeout(timer); + } + } + + // Auth config is static per server boot: single-flight + cache the success + // so the login page's multiple consumers (LoginForm, SocialSignInButtons, + // RegisterForm) share ONE request instead of three. Failures RETRY with a + // short backoff before rejecting — a first-load failure used to render the + // page without its SSO buttons, which strands SSO-only users on a password + // wall they have no password for (#2625). A final failure clears the cache + // so the next caller starts a fresh cycle. + const CONFIG_RETRY_DELAYS_MS = [500, 1500, 3500]; + const CONFIG_ATTEMPT_TIMEOUT_MS = 8_000; + let configPromise: Promise | null = null; + + async function loadConfigWithRetry(): Promise { + let lastError: unknown; + for (let attempt = 0; attempt <= CONFIG_RETRY_DELAYS_MS.length; attempt++) { + try { + return await fetchConfigAttempt(CONFIG_ATTEMPT_TIMEOUT_MS); + } catch (err) { + lastError = err; + if (attempt < CONFIG_RETRY_DELAYS_MS.length) { + await new Promise((resolve) => setTimeout(resolve, CONFIG_RETRY_DELAYS_MS[attempt])); + } + } + } + throw lastError instanceof Error ? lastError : new Error(String(lastError)); + } + return { async signIn(credentials: SignInCredentials) { const { data, error } = await betterAuth.signIn.email({ @@ -549,36 +607,44 @@ export function createAuthClient(config: AuthClientConfig): AuthClient { }, async getConfig(): Promise { - const url = `${origin}${basePath}/config`; - const response = await bearerFetch(url, { - method: 'GET', - headers: { 'Content-Type': 'application/json' }, - }); - if (!response.ok) { - throw new Error(`Failed to load auth config (status ${response.status})`); - } - const body = (await response.json()) as - | { success?: boolean; data?: AuthPublicConfig; error?: { message?: string } } - | AuthPublicConfig; - // Server wraps the payload as `{ success, data }`; tolerate both shapes. - if (body && typeof body === 'object' && 'data' in body && body.data) { - return body.data as AuthPublicConfig; + if (!configPromise) { + configPromise = loadConfigWithRetry().catch((err) => { + configPromise = null; + throw err; + }); } - return body as AuthPublicConfig; + return configPromise; }, async signInWithProvider(providerId: string, options: SignInWithProviderOptions = {}) { const { type = 'social', callbackURL, errorCallbackURL } = options; + // Watchdog (#2626): a sign-in request that HANGS (e.g. the environment + // kernel is cold-starting on first open) used to leave the provider + // button spinning forever — no error, no way to retry. Convert a + // no-response into a rejection so the button contract (#2458 pending + + // inline error) can recover it. The abandoned request may still land + // later; the user retrying just starts a fresh redirect, which is safe. + const SIGN_IN_TIMEOUT_MS = 20_000; + const withTimeout = (p: Promise): Promise => new Promise((resolve, reject) => { + const timer = setTimeout( + () => reject(new Error('Sign-in timed out — the server did not respond. Please try again.')), + SIGN_IN_TIMEOUT_MS, + ); + p.then( + (v) => { clearTimeout(timer); resolve(v); }, + (e) => { clearTimeout(timer); reject(e); }, + ); + }); // We pass `disableDefaultFetchPlugins: true` to better-auth above, // which also disables better-auth's `redirectPlugin` (the one that // navigates the browser to `data.url` when the server responds with // `{ url, redirect: true }`). The server still returns that payload // for OAuth/OIDC providers — we just have to honour it ourselves. - const signInSocial = async () => await betterAuth.signIn.social({ + const signInSocial = async () => await withTimeout(betterAuth.signIn.social({ provider: providerId as Parameters[0]['provider'], callbackURL, errorCallbackURL, - }) as { data: { url?: string; redirect?: boolean } | null; error: { message?: string; status: number } | null }; + })) as { data: { url?: string; redirect?: boolean } | null; error: { message?: string; status: number } | null }; let result: Awaited>; if (type === 'oidc') { // better-auth ≥ 1.7 registers generic OAuth/OIDC providers through @@ -592,7 +658,9 @@ export function createAuthClient(config: AuthClientConfig): AuthClient { const oauth2 = (betterAuth as unknown as { signIn: { oauth2?: (args: Record) => Promise<{ data: { url?: string; redirect?: boolean } | null; error: { message?: string; status: number } | null }> }; }).signIn.oauth2; - const legacy = oauth2 ? await oauth2({ providerId, callbackURL, errorCallbackURL }) : null; + const legacy = oauth2 + ? await withTimeout(oauth2({ providerId, callbackURL, errorCallbackURL })).catch(() => null) + : null; if (legacy && !legacy.error) { result = legacy; }