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.
This commit is contained in:
Anso
2026-06-01 09:00:35 -04:00
committed by GitHub
parent bf61ecae3e
commit 4248ac0e72
2 changed files with 146 additions and 9 deletions
+27 -9
View File
@@ -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
@@ -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<typeof vi.fn>;
const mockedToast = toast as unknown as { error: ReturnType<typeof vi.fn> };
function res(ok: boolean, body: unknown): { ok: boolean; json: () => Promise<unknown> } {
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(<SSOSection />);
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(<SSOSection />);
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(<SSOSection />);
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(<SSOSection />);
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(<SSOSection />);
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');
});
});
});