From 4248ac0e7235cb7926ebda07d8d57d888f0e73d3 Mon Sep 17 00:00:00 2001 From: Anso Date: Mon, 1 Jun 2026 09:00:35 -0400 Subject: [PATCH] fix(sso): surface config load, test, and removal errors in the SSO settings UI (#1265) The SSO provider settings previously swallowed three failure paths: the provider-config load on mount, the connection test, and provider removal. A transient backend error, a tier-gate rejection, or a failed removal left the admin with no feedback at all. Each path now surfaces the backend message (or a clear fallback) through the standard error toast, matching the existing save handler. The connection-test handler also checks the response status before treating the body as a test result, so an API or authorization failure is no longer reported as a failed identity-provider connection. Adds component tests covering the non-ok and network-failure branches for each handler, including the unparseable-error-body fallback. --- frontend/src/components/SSOSection.tsx | 36 ++++-- .../components/__tests__/SSOSection.test.tsx | 119 ++++++++++++++++++ 2 files changed, 146 insertions(+), 9 deletions(-) create mode 100644 frontend/src/components/__tests__/SSOSection.test.tsx diff --git a/frontend/src/components/SSOSection.tsx b/frontend/src/components/SSOSection.tsx index 6569b67b..81aba157 100644 --- a/frontend/src/components/SSOSection.tsx +++ b/frontend/src/components/SSOSection.tsx @@ -105,15 +105,23 @@ function ProviderCard({ providerId, type, label, initialConfig, onSave }: { setTestResult(null); try { const res = await apiFetch(`/sso/config/${providerId}/test`, { method: 'POST' }); - const data = await res.json(); + const data = await res.json().catch(() => null); + if (!res.ok) { + const message = data?.error || data?.message || 'Connection test failed'; + setTestResult({ success: false, error: message }); + toast.error(message); + return; + } setTestResult(data); - if (data.success) { + if (data?.success) { toast.success('Connection successful'); } else { - toast.error(data.error || 'Connection failed'); + toast.error(data?.error || 'Connection failed'); } - } catch { - setTestResult({ success: false, error: 'Connection test failed' }); + } catch (error: unknown) { + const message = (error as Error)?.message || 'Connection test failed'; + setTestResult({ success: false, error: message }); + toast.error(message); } finally { setTesting(false); } @@ -127,9 +135,12 @@ function ProviderCard({ providerId, type, label, initialConfig, onSave }: { setConfig({ enabled: false }); setExpanded(false); onSave(); + } else { + const data = await res.json().catch(() => null); + toast.error(data?.error || data?.message || 'Failed to remove provider'); } - } catch { - toast.error('Failed to remove provider'); + } catch (error: unknown) { + toast.error((error as Error)?.message || 'Failed to remove provider'); } }; @@ -408,8 +419,15 @@ export function SSOSection() { const fetchConfigs = async () => { try { const res = await apiFetch('/sso/config'); - if (res.ok) setConfigs(await res.json()); - } catch { /* ignore fetch errors */ } + if (res.ok) { + setConfigs(await res.json()); + } else { + const data = await res.json().catch(() => null); + toast.error(data?.error || data?.message || 'Failed to load SSO configuration'); + } + } catch (error: unknown) { + toast.error((error as Error)?.message || 'Failed to load SSO configuration'); + } }; // eslint-disable-next-line react-hooks/set-state-in-effect diff --git a/frontend/src/components/__tests__/SSOSection.test.tsx b/frontend/src/components/__tests__/SSOSection.test.tsx new file mode 100644 index 00000000..ecd2440b --- /dev/null +++ b/frontend/src/components/__tests__/SSOSection.test.tsx @@ -0,0 +1,119 @@ +/** + * Coverage for SSOSection error surfacing. + * + * Locks the three handlers that previously swallowed failures silently: + * the config load on mount, the connection test, and provider removal now + * each surface the backend message (or a fallback) through toast.error + * instead of leaving the admin with no feedback. + */ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +vi.mock('@/lib/api', () => ({ + apiFetch: vi.fn(), +})); + +vi.mock('@/components/ui/toast-store', () => ({ + toast: { + error: vi.fn(), + success: vi.fn(), + warning: vi.fn(), + info: vi.fn(), + loading: vi.fn(), + dismiss: vi.fn(), + }, +})); + +// Render the gated cards directly; tier/capability gating is exercised in the +// backend suite and is not what this test is about. +vi.mock('../CapabilityGate', () => ({ + CapabilityGate: ({ children }: { children: React.ReactNode }) => <>{children}, +})); +vi.mock('../PaidGate', () => ({ + PaidGate: ({ children }: { children: React.ReactNode }) => <>{children}, +})); +vi.mock('../AdmiralGate', () => ({ + AdmiralGate: ({ children }: { children: React.ReactNode }) => <>{children}, +})); +vi.mock('../settings/MastheadStatsContext', () => ({ + useMastheadStats: () => undefined, +})); + +import { apiFetch } from '@/lib/api'; +import { toast } from '@/components/ui/toast-store'; +import { SSOSection } from '../SSOSection'; + +const mockedFetch = apiFetch as unknown as ReturnType; +const mockedToast = toast as unknown as { error: ReturnType }; + +function res(ok: boolean, body: unknown): { ok: boolean; json: () => Promise } { + return { ok, json: () => Promise.resolve(body) }; +} + +beforeEach(() => { + mockedFetch.mockReset(); + mockedToast.error.mockReset(); +}); + +describe('SSOSection error surfacing', () => { + it('toasts the backend message when the config load returns a non-ok response', async () => { + mockedFetch.mockResolvedValue(res(false, { error: 'config store offline' })); + render(); + await waitFor(() => { + expect(mockedToast.error).toHaveBeenCalledWith('config store offline'); + }); + }); + + it('toasts a fallback when the config load throws (network failure)', async () => { + mockedFetch.mockRejectedValue(new Error('Failed to fetch')); + render(); + await waitFor(() => { + expect(mockedToast.error).toHaveBeenCalledWith('Failed to fetch'); + }); + }); + + it('toasts the literal fallback when a non-ok config load has an unparseable body', async () => { + mockedFetch.mockResolvedValue({ ok: false, json: () => Promise.reject(new Error('no body')) }); + render(); + await waitFor(() => { + expect(mockedToast.error).toHaveBeenCalledWith('Failed to load SSO configuration'); + }); + }); + + it('toasts the backend message when a connection test returns a non-ok response', async () => { + const user = userEvent.setup(); + mockedFetch.mockImplementation((path: string) => { + if (path === '/sso/config') return Promise.resolve(res(true, [])); + if (path.endsWith('/test')) return Promise.resolve(res(false, { error: 'provider tier locked' })); + return Promise.resolve(res(true, {})); + }); + render(); + + await user.click(await screen.findByText('Custom OIDC')); + await user.click(screen.getByRole('button', { name: /Test Connection/i })); + + await waitFor(() => { + expect(mockedToast.error).toHaveBeenCalledWith('provider tier locked'); + }); + }); + + it('toasts the backend message when removing a provider returns a non-ok response', async () => { + const user = userEvent.setup(); + mockedFetch.mockImplementation((path: string, opts?: { method?: string }) => { + if (path === '/sso/config') { + return Promise.resolve(res(true, [{ provider: 'oidc_custom', enabled: true, displayName: 'Custom OIDC' }])); + } + if (opts?.method === 'DELETE') return Promise.resolve(res(false, { error: 'delete rejected' })); + return Promise.resolve(res(true, {})); + }); + render(); + + await user.click(await screen.findByText('Custom OIDC')); + await user.click(screen.getByRole('button', { name: /Remove/i })); + + await waitFor(() => { + expect(mockedToast.error).toHaveBeenCalledWith('delete rejected'); + }); + }); +});