diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 3208ee20d1..96a1fcf474 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -20,6 +20,37 @@ ## Purpose +### SSO editor feedback and exact group names (24 October 2026) + +The provider editor scrolls its fields independently of its header and recovery +footer. Save refusals, metadata-preview errors and clipboard outcomes stay in +that active dialog, with live alert/status semantics; connection tests use the +existing inline result. They do not rely on the intentionally inert background +notification portal. Copy confirms success only after clipboard completion and +otherwise offers manual copying. The full callback URL wraps at narrow widths. +Save/Cancel stay reachable, and a pending save neither repeats nor dismisses. +Only success-confirmation-only toast stacks lower below the modal backdrop while +a blocking dialog is open. Any error, warning or info toast keeps the whole stack +at its ordinary foreground layer, including when added alongside a retained +success: existing delete, assignment and role dialogs still depend on those +notifications while remaining open. Toasts are not unmounted or made interactive +through the modal. Shared modal focus trapping, inertness and restoration remain +unchanged. The SSO header and footer are non-scrolling flex siblings of the +scrollable fields; the header needs no sticky positioning. + +Allowed Groups are comma- or newline-separated exact IdP group names, preserving +spaces through details → edit → no-op/mapping-only save for OIDC and SAML. This +replaces the former whitespace split for that field only. Scopes, domains and +emails keep their existing parsing; explicit empty restriction lists still clear +them. Payload shape, secret omission and server-owned authorisation are unchanged. +`SSOProvidersPanel.test.tsx`, `ssoProvidersModel.test.ts`, `Toast.test.tsx`, the +consuming `SSOProviderNotifications.test.tsx` and the settings architecture suite +cover this boundary, including an actual refused DELETE while its confirmation +stays open and a successful save/reopen with the previous confirmation retained. +Browser evidence uses the production +panel, state hook, API client, Dialog, Toast and CSS with synthetic HTTP at desktop +and 320px widths; it is not native IdP, encrypted persistence or restart proof. + ### Locked update pill names the command-access option by its shared label (21 October 2026) For a host whose agent was installed without command access, the locked pill @@ -7783,8 +7814,9 @@ OIDC, SAML, and multi-provider SSO are Community-tier capabilities. The group-to-role mapping form must retain IdP group names with embedded spaces through provider detail, edit, payload, and reload. Its entries are comma- or newline-delimited `group=roleId` pairs; whitespace within a group -name is not an entry separator. Keep the existing whitespace parsing for -OIDC scopes and the other allowed lists separate from mapping parsing. +name is not an entry separator. Allowed Groups use the same comma/newline +boundary so no-op saves preserve those exact names too. Keep the existing whitespace parsing for OIDC scopes, +allowed domains and allowed emails separate from group-name parsing. `ssoProvidersModel.test.ts` and `SSOProvidersPanel.test.tsx` pin this round trip, and browser verification must inspect the saved mapping in the desktop and narrow edit dialog. diff --git a/frontend-modern/browser-verification/5bdf00026ba6b130.json b/frontend-modern/browser-verification/5bdf00026ba6b130.json new file mode 100644 index 0000000000..cad7c1f160 --- /dev/null +++ b/frontend-modern/browser-verification/5bdf00026ba6b130.json @@ -0,0 +1,39 @@ +{ + "version": 1, + "base_sha": "518fda75b3581951868935e861c5c90c1b5c16cb", + "verified_at": "2026-10-24T02:25:40.484534Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Settings/SSOProvidersPanel.tsx", + "frontend-modern/src/components/Toast/Toast.tsx" + ], + "content_sha256": { + "frontend-modern/src/components/Settings/SSOProvidersPanel.tsx": "6fa2023f853a5a6080158459e0fb30593aa3a3853c3be43dfe35986b49ef946e", + "frontend-modern/src/components/Toast/Toast.tsx": "7c6236ffa6215a6bb4d52dc7932e24d3dee19fb361a02103b118597e6a3d84f1" + }, + "routes": [ + "/settings/security/sso (production panel consumed in a safe synthetic HTTP fixture at /candidate/)" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 320, + "height": 812 + } + ], + "states": [ + "Production SSO panel/state/API/Dialog/Toast/CSS; baseline is the retained candidate composed with current main. No native IdP, encryption, restart or installed recovery proof.", + "Actual modal-only DELETE refusal remains visible in the foreground while Delete provider stays open, even alongside the retained successful-save notification. Error, warning and info stacks keep their original foreground behaviour; only success-only stacks lower.", + "Successful mapping save and immediate reopen preserve the exact Server Access group, scopes, domain and omitted secret; the retained success does not obscure editor actions. Existing modal inertness stays intact.", + "Non-scrolling flex header is visible in initial, connection-failure, metadata-failure and scrolled-reopen narrow pixels. Earlier archived missing-title pixels were inspected; the current baseline also paints its heading, so no universal WebKit cause or fix is inferred.", + "All twelve retained captures were opened with the native image viewer, including modal-only failure controls and desktop/320px candidate states." + ], + "interactions": [ + "Real production HTTP adapter GET/detail/PUT/DELETE/test/metadata-preview producers, with fixed synthetic responses; no automatic DELETE retry and no inline delete error substituted for the existing toast.", + "Keyboard save and consuming reopen, exact access restrictions and visible unobscured recovery controls. Removing the non-success feedback restores normal background isolation after close.", + "Playwright 1.56.1; desktop Chromium and narrow WebKit. Fourteen cases pass. No page errors or external requests; only deliberate HTTP400/409 and their existing error logs. Contexts, browsers and preview server closed." + ] +} diff --git a/frontend-modern/browser-verification/91ae95b4d6779efe.json b/frontend-modern/browser-verification/91ae95b4d6779efe.json new file mode 100644 index 0000000000..787aa719e6 --- /dev/null +++ b/frontend-modern/browser-verification/91ae95b4d6779efe.json @@ -0,0 +1,45 @@ +{ + "version": 1, + "base_sha": "a079862edc1284b90a89ebae1a0fb6086e0e1df2", + "verified_at": "2026-10-24T01:35:17.926Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Settings/SSOProvidersPanel.tsx", + "frontend-modern/src/components/Settings/ssoProvidersModel.ts", + "frontend-modern/src/components/Settings/useSSOProvidersState.ts", + "frontend-modern/src/components/Toast/Toast.tsx", + "frontend-modern/src/utils/ssoProviderPresentation.ts" + ], + "content_sha256": { + "frontend-modern/src/components/Settings/SSOProvidersPanel.tsx": "2eb2ecc8c72fb7462bcd4eab6213e4af730d2c29febdfa839855f8a0f2c79c8f", + "frontend-modern/src/components/Settings/ssoProvidersModel.ts": "3c995445a21034ed7ca3242bccc5ef14662cf0d6aeddc71f8555218aea6ac23e", + "frontend-modern/src/components/Settings/useSSOProvidersState.ts": "465e68954b4aa66b3461d480b409c47592864ebc7412b41fc40453e45f6415ba", + "frontend-modern/src/components/Toast/Toast.tsx": "c1c564f3f6af3acebf8671d723f23a8b1242dbf7e056af2dff70faaa366a7954", + "frontend-modern/src/utils/ssoProviderPresentation.ts": "cf5d992ef94776f00e31722155388ab622d6d0401273e24d4b195981ecb7face" + }, + "routes": [ + "/settings/security/sso (production panel with synthetic HTTP, not full application)" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 320, + "height": 812 + } + ], + "states": [ + "Production SSO panel, state hook, API client, Dialog, Toast and CSS with synthetic safe HTTP and controlled clipboard success/refusal. Baseline is the exact supplied parent. No native IdP, encrypted persistence or restart acceptance.", + "Refused write remains inside the active editor, preserving draft, omitted client secret and exact spaced group restrictions. An explicit keyboard correction saves and reopens with the same restrictions and corrected role.", + "Header/callback layout, clipboard refusal/manual fallback, connection-test failure and SAML metadata-preview failure are inspected at desktop and narrow widths. Feedback and footer are readable. Some WebKit capture frames retain top-heading clipping observations in the lane dossier, not a claim of perfect general layout.", + "Retained success notification lowers below blocking dialogs; correction/reopen does not cover the footer. Shared modal isolation is unchanged.", + "All fourteen retained captures, including two exact-parent controls, were opened with the native image viewer." + ], + "interactions": [ + "Real HTTP-adapter GET/detail/PUT/test/preview flows and callback-copy control; exact payload, scopes, groups/domains, secret omission and explicit correction/reopen checks.", + "One pending save does not repeat or dismiss on Escape; read-only controls, focus trapping, Escape close and focus return remain.", + "Playwright 1.56.1, Chromium desktop and WebKit 320px; no page errors or external requests. Only deliberately refused HTTP400 and their existing error logs in the final run. Browser contexts and server closed." + ] +} diff --git a/frontend-modern/src/components/Settings/SSOProvidersPanel.tsx b/frontend-modern/src/components/Settings/SSOProvidersPanel.tsx index 0853a41a39..2016479c2e 100644 --- a/frontend-modern/src/components/Settings/SSOProvidersPanel.tsx +++ b/frontend-modern/src/components/Settings/SSOProvidersPanel.tsx @@ -42,6 +42,7 @@ export const SSOProvidersPanel: Component = (props) => { providers, loading, saving, + editorFeedback, showModal, setShowModal, editingProvider, @@ -272,18 +273,20 @@ export const SSOProvidersPanel: Component = (props) => { setShowModal(false)} + onClose={() => { + if (!saving()) setShowModal(false); + }} panelClass="max-w-2xl" ariaLabel={`${editingProvider() ? 'Edit' : 'Add'} ${getSSOProviderTypeLabel(form.type)} provider`} > -
-
+
+

{getSSOProviderModalTitle(Boolean(editingProvider()), form.type)}

- +
{/* Common fields */}
@@ -342,10 +345,12 @@ export const SSOProvidersPanel: Component = (props) => { onCopyValue={(value) => copyToClipboard(value, 'Callback URL')} label="Copy callback / redirect URL" variant="accent" - size="chip" - class="max-w-full text-xs" + size="sm" + class="min-w-0 max-w-full justify-between gap-2 py-2 text-left text-xs [&>svg]:shrink-0" > - {editingProvider()?.oidcCallbackUrl} + + {editingProvider()?.oidcCallbackUrl} +
@@ -609,7 +614,10 @@ export const SSOProvidersPanel: Component = (props) => { {/* Test result display */} -
+
{testResult()?.success ? ( @@ -712,7 +720,7 @@ export const SSOProvidersPanel: Component = (props) => { type="button" variant="ghost" size="xs" - class="font-semibold hover:underline" + class="h-auto max-w-full whitespace-normal text-left font-semibold hover:underline" onClick={() => setAdvancedOpen(!advancedOpen())} > {advancedOpen() ? 'Hide' : 'Show'} access restrictions & role mapping @@ -748,7 +756,7 @@ export const SSOProvidersPanel: Component = (props) => { onInput={(e) => setForm('allowedGroups', e.currentTarget.value)} placeholder="admin, sso-users" textareaBaseClass={controlClass('min-h-[60px]')} - help="Comma-separated. Empty allows all." + help="Comma- or newline-separated. Group names may contain spaces. Empty allows all." /> = (props) => {
+
- {/* Actions */} -
+ {/* Keep feedback and recovery actions inside the active modal, clear of toasts. */} +
+ + {(feedback) => ( +

+ {feedback.message} +

+ )} + +
- -
+
+
diff --git a/frontend-modern/src/components/Settings/__tests__/SSOProviderNotifications.test.tsx b/frontend-modern/src/components/Settings/__tests__/SSOProviderNotifications.test.tsx new file mode 100644 index 0000000000..e3f8a7a5fe --- /dev/null +++ b/frontend-modern/src/components/Settings/__tests__/SSOProviderNotifications.test.tsx @@ -0,0 +1,78 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, fireEvent, render, screen, waitFor, within } from '@solidjs/testing-library'; +import { SSOProvidersPanel } from '../SSOProvidersPanel'; +import { ToastContainer } from '@/components/Toast/Toast'; + +const provider = { + id: 'corp-oidc', + name: 'Corporate OIDC', + type: 'oidc', + enabled: true, + priority: 0, + oidc: { issuerUrl: 'https://idp.example.test', clientId: 'pulse' }, +}; + +describe('SSO provider consuming notification layers', () => { + afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + }); + + it('reveals a modal-only delete failure without losing the retained-success reopen behaviour', async () => { + const fetchMock = vi.fn((url: string, options?: RequestInit) => { + const method = options?.method ?? 'GET'; + let body: unknown; + let status = 200; + if (url === '/api/security/status') body = { publicUrl: 'https://pulse.example.test' }; + else if (method === 'DELETE') { + body = { error: 'Synthetic provider deletion refused' }; + status = 409; + } else if (url === '/api/security/sso/providers' && method === 'GET') + body = { providers: [provider] }; + else body = provider; + return Promise.resolve( + new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }), + ); + }); + vi.stubGlobal('fetch', fetchMock); + document.cookie = 'pulse_csrf=synthetic-csrf'; + render(() => ( + <> + + + + )); + fireEvent.click(await screen.findByRole('button', { name: 'Edit provider' })); + let editor = await screen.findByRole('dialog', { name: 'Edit OIDC provider' }); + fireEvent.submit(within(editor).getByRole('button', { name: 'Save Changes' }).closest('form')!); + await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + const notifications = screen.getByRole('region', { name: 'Notifications' }); + expect(notifications).toHaveTextContent('SSO provider has been updated.'); + + fireEvent.click(await screen.findByRole('button', { name: 'Edit provider' })); + editor = await screen.findByRole('dialog', { name: 'Edit OIDC provider' }); + expect(notifications).toHaveClass('z-50'); + expect(notifications.closest('[inert]')).not.toBeNull(); + fireEvent.click(within(editor).getByRole('button', { name: 'Cancel' })); + + fireEvent.click(screen.getByRole('button', { name: 'Delete provider' })); + const confirmation = screen.getByRole('dialog', { name: 'Delete provider' }); + fireEvent.click(within(confirmation).getByRole('button', { name: 'Delete Provider' })); + await waitFor(() => + expect(notifications).toHaveTextContent('Unable to remove the SSO provider.'), + ); + expect(confirmation).toBeInTheDocument(); + expect(notifications).toHaveClass('z-9999'); + expect(notifications).not.toHaveClass('z-50'); + expect(notifications.closest('[inert]')).not.toBeNull(); + expect(fetchMock.mock.calls.filter(([, options]) => options?.method === 'DELETE')).toHaveLength( + 1, + ); + fireEvent.click(within(confirmation).getByRole('button', { name: 'Cancel' })); + expect(notifications.closest('[inert]')).toBeNull(); + expect(notifications).toHaveTextContent('Unable to remove the SSO provider.'); + }); +}); diff --git a/frontend-modern/src/components/Settings/__tests__/SSOProvidersPanel.test.tsx b/frontend-modern/src/components/Settings/__tests__/SSOProvidersPanel.test.tsx index 66e6a026f5..26eb91a9ee 100644 --- a/frontend-modern/src/components/Settings/__tests__/SSOProvidersPanel.test.tsx +++ b/frontend-modern/src/components/Settings/__tests__/SSOProvidersPanel.test.tsx @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { cleanup, fireEvent, render, screen, waitFor } from '@solidjs/testing-library'; +import { cleanup, fireEvent, render, screen, waitFor, within } from '@solidjs/testing-library'; import { SSOProvidersPanel } from '../SSOProvidersPanel'; const fetchMock = vi.fn(); @@ -156,6 +156,7 @@ describe('SSOProvidersPanel OIDC group role mappings', () => { it('preserves a multi-word group from provider details through the edit form and PUT', async () => { setupFetch([corpProvider], { ...corpProviderDetails, + allowedGroups: ['Server Access'], groupRoleMappings: { 'Server Access': 'admin' }, }); render(() => ); @@ -183,6 +184,7 @@ describe('SSOProvidersPanel OIDC group role mappings', () => { expect(saveCall).toBeTruthy(); const body = JSON.parse((saveCall![1] as RequestInit).body as string); expect(body.groupRoleMappings).toEqual({ 'Server Access': 'admin', Everyone: 'viewer' }); + expect(body.allowedGroups).toEqual(['Server Access']); expect(body.oidc.scopes).toEqual(['openid', 'profile', 'email', 'groups']); }); }); @@ -265,3 +267,164 @@ describe('SSOProvidersPanel IdP endpoint URLs', () => { ).toBeTruthy(); }); }); + +describe('SSOProvidersPanel modal feedback and recovery', () => { + beforeEach(() => { + vi.resetAllMocks(); + vi.stubGlobal('fetch', fetchMock); + document.cookie = 'pulse_csrf=test-csrf-token'; + }); + + afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + }); + + const editProvider = async () => { + render(() => ); + fireEvent.click(await screen.findByRole('button', { name: 'Edit provider' })); + return within(await screen.findByRole('dialog', { name: 'Edit OIDC provider' })); + }; + + it('keeps a refused save and the unchanged draft inside the active dialog until an explicit correction', async () => { + setupFetch([corpProvider]); + const read = fetchMock.getMockImplementation()!; + let refuse = true; + fetchMock.mockImplementation((url: string, options?: RequestInit) => { + if (options?.method === 'PUT') { + return Promise.resolve( + refuse ? new Response('Invalid role mapping', { status: 400 }) : jsonResponse({}), + ); + } + return read(url, options); + }); + const dialog = await editProvider(); + const name = dialog.getByLabelText('Provider Name') as HTMLInputElement; + const secret = dialog.getByLabelText('Client Secret') as HTMLInputElement; + fireEvent.input(name, { target: { value: 'Edited provider' } }); + const save = dialog.getByRole('button', { name: 'Save Changes' }); + fireEvent.submit(save.closest('form')!); + + expect(await dialog.findByRole('alert')).toHaveTextContent('Invalid role mapping'); + expect(name.value).toBe('Edited provider'); + expect(secret.value).toBe(''); + expect(notificationErrorMock).not.toHaveBeenCalled(); + expect(notificationSuccessMock).not.toHaveBeenCalled(); + expect(fetchMock.mock.calls.filter(([, options]) => options?.method === 'PUT')).toHaveLength(1); + + refuse = false; + fireEvent.input(name, { target: { value: 'Corrected provider' } }); + fireEvent.submit(save.closest('form')!); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + const writes = fetchMock.mock.calls.filter(([, options]) => options?.method === 'PUT'); + expect(writes).toHaveLength(2); + const payload = JSON.parse(writes[1][1].body); + expect(payload.name).toBe('Corrected provider'); + expect(payload.oidc.clientSecret).toBeUndefined(); + expect(payload.allowedGroups).toEqual(['admins']); + expect(payload.groupRoleMappings).toEqual({ admins: 'admin' }); + expect(notificationSuccessMock).toHaveBeenCalledOnce(); + }); + + it('ignores repeated submit and keeps the modal open while a save is pending', async () => { + setupFetch([corpProvider]); + const read = fetchMock.getMockImplementation()!; + let finish!: (response: Response) => void; + fetchMock.mockImplementation((url: string, options?: RequestInit) => + options?.method === 'PUT' + ? new Promise((resolve) => { + finish = resolve; + }) + : read(url, options), + ); + const dialog = await editProvider(); + const form = dialog.getByRole('button', { name: 'Save Changes' }).closest('form')!; + fireEvent.submit(form); + await waitFor(() => + expect(fetchMock.mock.calls.filter(([, options]) => options?.method === 'PUT')).toHaveLength( + 1, + ), + ); + fireEvent.submit(form); + fireEvent.keyDown(document, { key: 'Escape' }); + expect(screen.getByRole('dialog')).toBeTruthy(); + expect(fetchMock.mock.calls.filter(([, options]) => options?.method === 'PUT')).toHaveLength(1); + expect(dialog.getByRole('button', { name: 'Cancel' })).toBeDisabled(); + finish(jsonResponse({})); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + }); + + it('clears failed-save feedback when a different editor session opens', async () => { + setupFetch([corpProvider]); + const read = fetchMock.getMockImplementation()!; + fetchMock.mockImplementation((url: string, options?: RequestInit) => + options?.method === 'PUT' + ? Promise.resolve(new Response('Refused', { status: 400 })) + : read(url, options), + ); + const dialog = await editProvider(); + fireEvent.submit(dialog.getByRole('button', { name: 'Save Changes' }).closest('form')!); + expect(await dialog.findByRole('alert')).toHaveTextContent('Refused'); + fireEvent.click(dialog.getByRole('button', { name: 'Cancel' })); + fireEvent.click(screen.getByRole('button', { name: 'Add OIDC' })); + expect(within(screen.getByRole('dialog')).queryByRole('alert')).toBeNull(); + }); + + it.each([false, true])( + 'reports clipboard completion inside the modal, rejected=%s', + async (rejected) => { + const callback = 'https://pulse.example.com/api/oidc/corp-oidc/callback'; + setupFetch([{ ...corpProvider, oidcCallbackUrl: callback }]); + const writeText = vi + .fn() + .mockImplementation(() => + rejected ? Promise.reject(new Error('Denied')) : Promise.resolve(), + ); + Object.defineProperty(navigator, 'clipboard', { value: { writeText }, configurable: true }); + const dialog = await editProvider(); + fireEvent.click(dialog.getByRole('button', { name: 'Copy callback / redirect URL' })); + expect(await dialog.findByRole(rejected ? 'alert' : 'status')).toHaveTextContent( + rejected ? 'Unable to copy callback url' : 'Callback URL has been copied', + ); + expect(writeText).toHaveBeenCalledWith(callback); + expect(notificationErrorMock).not.toHaveBeenCalled(); + expect(notificationSuccessMock).not.toHaveBeenCalled(); + }, + ); + + it('keeps SAML preview failures inside the modal without an inert toast', async () => { + setupFetch([]); + const read = fetchMock.getMockImplementation()!; + fetchMock.mockImplementation((url: string, options?: RequestInit) => + url.endsWith('/metadata/preview') + ? Promise.resolve(new Response('Metadata unavailable', { status: 502 })) + : read(url, options), + ); + render(() => ); + fireEvent.click(await screen.findByRole('button', { name: 'Add SAML' })); + const dialog = within(screen.getByRole('dialog')); + fireEvent.input(dialog.getByLabelText('IdP Metadata URL'), { + target: { value: 'https://idp.example.com/metadata' }, + }); + fireEvent.click(dialog.getByRole('button', { name: 'Preview' })); + expect(await dialog.findByRole('alert')).toHaveTextContent('Metadata unavailable'); + expect(notificationErrorMock).not.toHaveBeenCalled(); + }); + + it('announces connection-test failure in the existing result without a background toast', async () => { + setupFetch([corpProvider]); + const read = fetchMock.getMockImplementation()!; + fetchMock.mockImplementation((url: string, options?: RequestInit) => + url.endsWith('/test') + ? Promise.resolve(jsonResponse({ success: false, message: 'Synthetic IdP unavailable' })) + : read(url, options), + ); + const dialog = await editProvider(); + fireEvent.click(dialog.getByRole('button', { name: 'Test' })); + expect(await dialog.findByRole('alert')).toHaveTextContent('Synthetic IdP unavailable'); + expect(notificationErrorMock).not.toHaveBeenCalled(); + fireEvent.click(dialog.getByRole('button', { name: 'Cancel' })); + fireEvent.click(screen.getByRole('button', { name: 'Edit provider' })); + expect((await screen.findByRole('dialog')).querySelector('[role="alert"]')).toBeNull(); + }); +}); diff --git a/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts b/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts index 987793791b..1e01d3f89b 100644 --- a/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts @@ -1161,6 +1161,17 @@ describe('settings architecture guardrails', () => { expect(ssoProvidersPanelSource).not.toContain('Add SAML (Pro)'); }); + it('keeps the SSO editor header outside its independently scrolling fields', () => { + const editor = ssoProvidersPanelSource.slice( + ssoProvidersPanelSource.indexOf('{/* Add/Edit Modal */}'), + ssoProvidersPanelSource.indexOf('{/* Delete confirmation modal */}'), + ); + expect(editor).toContain('min-h-0 flex-col'); + expect(editor).toContain('shrink-0 border-b border-border bg-surface'); + expect(editor).toContain('min-h-0 flex-1 space-y-4 overflow-y-auto'); + expect(editor).not.toContain('sticky'); + }); + it('keeps SAML SSO available without a self-hosted Pro upsell boundary', () => { expect(ssoProvidersPanelSource).toContain('@/components/shared/Button'); expect(ssoProvidersPanelSource).toContain('ActionIconButton'); @@ -1184,6 +1195,9 @@ describe('settings architecture guardrails', () => { it('separates SSO mapping entries without splitting spaces inside IdP group names', () => { expect(ssoProvidersModelSource).toContain('input.split(/[,\\r\\n]+/)'); expect(ssoProvidersModelSource).not.toContain('splitList(input).forEach'); + expect(ssoProvidersModelSource).toContain('allowedGroups: splitGroupList(form.allowedGroups)'); + expect(ssoProvidersModelSource).toContain('scopes: splitList(form.oidcScopes)'); + expect(ssoProvidersPanelSource).toContain('Comma- or newline-separated.'); expect(ssoProvidersPanelSource).toContain('Group names may contain spaces.'); }); diff --git a/frontend-modern/src/components/Settings/__tests__/ssoProvidersModel.test.ts b/frontend-modern/src/components/Settings/__tests__/ssoProvidersModel.test.ts index 41ac3ec230..bb4632d094 100644 --- a/frontend-modern/src/components/Settings/__tests__/ssoProvidersModel.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/ssoProvidersModel.test.ts @@ -6,6 +6,50 @@ import { } from '../ssoProvidersModel'; describe('ssoProvidersModel', () => { + it.each(['oidc', 'saml'] as const)( + 'preserves exact %s group restrictions on no-op and mapping-only saves', + (type) => { + const form = mapProviderDetailsToForm({ + id: 'corp', + name: 'Corporate SSO', + enabled: true, + type, + allowedGroups: ['Server Access', 'Read Only'], + allowedDomains: ['example.com', 'corp.example.com'], + allowedEmails: ['admin@example.com'], + groupRoleMappings: { 'Server Access': 'admin' }, + oidc: { scopes: ['openid', 'profile', 'email', 'groups'] }, + }); + for (const draft of [form, { ...form, groupRoleMappings: 'Server Access=viewer' }]) { + const payload = buildProviderPayload(draft); + expect(payload.allowedGroups).toEqual(['Server Access', 'Read Only']); + expect(payload.allowedDomains).toEqual(['example.com', 'corp.example.com']); + expect(payload.allowedEmails).toEqual(['admin@example.com']); + } + expect( + buildProviderPayload({ ...form, allowedGroups: ' Server Access,\n Read Only\r\n ' }) + .allowedGroups, + ).toEqual(['Server Access', 'Read Only']); + expect(buildProviderPayload({ ...form, allowedGroups: ' ' }).allowedGroups).toEqual([]); + // Scope/domain/email conveniences remain whitespace-separated, not group-name parsing. + const oidc = buildProviderPayload({ + ...form, + type: 'oidc', + oidcScopes: 'openid profile email groups', + allowedDomains: 'example.com corp.example.com', + allowedEmails: 'admin@example.com ops@example.com', + }); + expect((oidc.oidc as Record).scopes).toEqual([ + 'openid', + 'profile', + 'email', + 'groups', + ]); + expect(oidc.allowedDomains).toEqual(['example.com', 'corp.example.com']); + expect(oidc.allowedEmails).toEqual(['admin@example.com', 'ops@example.com']); + }, + ); + it('serializes OIDC group claim settings for restrictions and role mappings', () => { const form = { ...createEmptyProviderForm(), @@ -47,7 +91,7 @@ describe('ssoProvidersModel', () => { const payload = buildProviderPayload({ ...form, groupRoleMappings: ' Server Access = admin,\n Everyone = viewer ', - allowedGroups: 'admins operators', + allowedGroups: 'admins, operators', }); expect(payload.groupRoleMappings).toEqual({ 'Server Access': 'admin', Everyone: 'viewer' }); expect(payload.allowedGroups).toEqual(['admins', 'operators']); diff --git a/frontend-modern/src/components/Settings/ssoProvidersModel.ts b/frontend-modern/src/components/Settings/ssoProvidersModel.ts index 82102d5318..07ec8ccc4b 100644 --- a/frontend-modern/src/components/Settings/ssoProvidersModel.ts +++ b/frontend-modern/src/components/Settings/ssoProvidersModel.ts @@ -166,6 +166,13 @@ export const splitList = (input: string) => .map((value) => value.trim()) .filter(Boolean); +// Group restrictions use exact IdP names, just like role-mapping keys. +const splitGroupList = (input: string) => + input + .split(/[,\r\n]+/) + .map((value) => value.trim()) + .filter(Boolean); + export const mappingsToString = (mappings?: Record) => mappings ? Object.entries(mappings) @@ -175,7 +182,7 @@ export const mappingsToString = (mappings?: Record) => export const stringToMappings = (input: string) => { const result: Record = {}; - // A group name can contain spaces. Unlike scopes and allowed lists, mappings + // A group name can contain spaces. Like allowed groups, mappings // are separated only by commas or newlines, as shown by mappingsToString. input.split(/[,\r\n]+/).forEach((pair) => { const [key, value] = pair.split('=').map((segment) => segment.trim()); @@ -224,7 +231,7 @@ export const buildProviderPayload = (form: ProviderForm): Record { const [providers, setProviders] = createSignal([]); const [loading, setLoading] = createSignal(true); const [saving, setSaving] = createSignal(false); + const [editorFeedback, setEditorFeedback] = createSignal<{ + tone: 'success' | 'error'; + message: string; + } | null>(null); const [showModal, setShowModal] = createSignal(false); const [editingProvider, setEditingProvider] = createSignal(null); const [form, setForm] = createStore(createEmptyProviderForm()); @@ -90,6 +93,8 @@ export const useSSOProvidersState = (props: SSOProvidersPanelProps) => { if (!canManage()) { return; } + setEditorFeedback(null); + setTestResult(null); setEditingProvider(null); setForm(createEmptyProviderForm()); setForm('type', type); @@ -101,6 +106,8 @@ export const useSSOProvidersState = (props: SSOProvidersPanelProps) => { if (!canManage()) { return; } + setEditorFeedback(null); + setTestResult(null); setEditingProvider(provider); try { const { apiFetch } = await import('@/utils/apiClient'); @@ -119,9 +126,10 @@ export const useSSOProvidersState = (props: SSOProvidersPanelProps) => { const handleSave = async (event?: Event) => { event?.preventDefault(); - if (!canManage()) { + if (!canManage() || saving()) { return; } + setEditorFeedback(null); setSaving(true); try { @@ -149,7 +157,7 @@ export const useSSOProvidersState = (props: SSOProvidersPanelProps) => { props.onConfigUpdated?.(); } catch (error) { logger.error('[SSOProvidersPanel] Failed to save provider:', error); - notificationStore.error(getSSOProviderSaveErrorMessage(error)); + setEditorFeedback({ tone: 'error', message: getSSOProviderSaveErrorMessage(error) }); } finally { setSaving(false); } @@ -221,9 +229,23 @@ export const useSSOProvidersState = (props: SSOProvidersPanelProps) => { } }; - const copyToClipboard = (text: string, label: string) => { - void navigator.clipboard.writeText(text); - notificationStore.success(getSSOCopySuccessMessage(label)); + const copyToClipboard = async (text: string, label: string) => { + try { + await navigator.clipboard.writeText(text); + const message = getSSOCopySuccessMessage(label); + if (showModal()) { + setEditorFeedback({ tone: 'success', message }); + } else { + notificationStore.success(message); + } + } catch { + const message = getSSOCopyErrorMessage(label); + if (showModal()) { + setEditorFeedback({ tone: 'error', message }); + } else { + notificationStore.error(message); + } + } }; const testResultPresentation = createMemo(() => @@ -231,6 +253,7 @@ export const useSSOProvidersState = (props: SSOProvidersPanelProps) => { ); const testConnection = async () => { + setEditorFeedback(null); setTesting(true); setTestResult(null); @@ -245,11 +268,7 @@ export const useSSOProvidersState = (props: SSOProvidersPanelProps) => { const result = (await response.json()) as SSOProviderTestResult; setTestResult(result); - if (result.success) { - notificationStore.success(getSSOConnectionTestSuccessMessage()); - } else { - notificationStore.error(getSSOConnectionTestFailureMessage(result.message)); - } + // The result belongs to the active dialog, not its inert background toast. } catch (error) { logger.error('[SSOProvidersPanel] Test connection error:', error); setTestResult({ @@ -257,7 +276,6 @@ export const useSSOProvidersState = (props: SSOProvidersPanelProps) => { message: getSSOConnectionTestErrorMessage(), error: String(error), }); - notificationStore.error(getSSOConnectionTestErrorMessage()); } finally { setTesting(false); } @@ -266,8 +284,9 @@ export const useSSOProvidersState = (props: SSOProvidersPanelProps) => { const canTest = () => canTestProviderForm(form); const fetchMetadataPreview = async () => { + setEditorFeedback(null); if (!form.samlIdpMetadataUrl.trim()) { - notificationStore.error(getSSOMetadataUrlRequiredMessage()); + setEditorFeedback({ tone: 'error', message: getSSOMetadataUrlRequiredMessage() }); return; } @@ -291,7 +310,7 @@ export const useSSOProvidersState = (props: SSOProvidersPanelProps) => { setShowMetadataPreview(true); } catch (error) { logger.error('[SSOProvidersPanel] Metadata preview error:', error); - notificationStore.error(getSSOMetadataFetchErrorMessage(error)); + setEditorFeedback({ tone: 'error', message: getSSOMetadataFetchErrorMessage(error) }); } finally { setLoadingPreview(false); } @@ -301,6 +320,7 @@ export const useSSOProvidersState = (props: SSOProvidersPanelProps) => { providers, loading, saving, + editorFeedback, showModal, setShowModal, editingProvider, diff --git a/frontend-modern/src/components/Toast/Toast.tsx b/frontend-modern/src/components/Toast/Toast.tsx index b531d3a6f7..9dbc8f8ced 100644 --- a/frontend-modern/src/components/Toast/Toast.tsx +++ b/frontend-modern/src/components/Toast/Toast.tsx @@ -6,6 +6,7 @@ import CircleAlertIcon from 'lucide-solid/icons/circle-alert'; import InfoIcon from 'lucide-solid/icons/info'; import XIcon from 'lucide-solid/icons/x'; import { ActionIconButton } from '@/components/shared/Button'; +import { dialogStackHasBlockingDialog } from '@/components/shared/useDialogState'; import { POLLING_INTERVALS } from '@/constants'; import { getSemanticTonePresentation } from '@/utils/semanticTonePresentation'; @@ -192,6 +193,11 @@ declare global { export const ToastContainer: Component = () => { const [toasts, setToasts] = createSignal([]); + // Only confirmation-only stacks may sit behind a dialog. Existing editors + // still rely on error, warning and info toasts while they remain open. + const lowerSuccessConfirmations = () => + dialogStackHasBlockingDialog() && toasts().every((toast) => toast.type === 'success'); + const removeToast = (id: string) => { setToasts((prev) => prev.filter((t) => t.id !== id)); }; @@ -217,7 +223,9 @@ export const ToastContainer: Component = () => {
{(toast) => }
diff --git a/frontend-modern/src/components/Toast/__tests__/Toast.test.tsx b/frontend-modern/src/components/Toast/__tests__/Toast.test.tsx index c6e46be7e8..a70844555a 100644 --- a/frontend-modern/src/components/Toast/__tests__/Toast.test.tsx +++ b/frontend-modern/src/components/Toast/__tests__/Toast.test.tsx @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; -import { batch } from 'solid-js'; +import { batch, createSignal } from 'solid-js'; +import { Dialog } from '@/components/shared/Dialog'; import { cleanup, fireEvent, render, screen } from '@solidjs/testing-library'; import { Toast, ToastContainer } from '@/components/Toast/Toast'; import toastSource from '@/components/Toast/Toast.tsx?raw'; @@ -37,6 +38,65 @@ describe('Toast', () => { expect(screen.queryByRole(role === 'alert' ? 'status' : 'alert')).not.toBeInTheDocument(); }); + it('keeps a retained success below blocking dialogs, restoring it after close', () => { + const [open, setOpen] = createSignal(false); + render(() => ( + <> + + setOpen(false)} ariaLabel="Editor"> + + + + )); + window.showToast('success', 'Previous operation completed'); + const notifications = screen.getByRole('region', { name: 'Notifications' }); + expect(notifications).toHaveClass('z-9999'); + setOpen(true); + expect(screen.getByRole('dialog')).toBeTruthy(); + expect(notifications).toHaveClass('z-50'); + expect(notifications).not.toHaveClass('z-9999'); + expect(notifications.closest('[inert]')).not.toBeNull(); + fireEvent.click(screen.getByRole('button', { name: 'Close editor' })); + expect(notifications).toHaveClass('z-9999'); + expect(notifications.closest('[inert]')).toBeNull(); + expect(screen.getByText('Previous operation completed')).toBeInTheDocument(); + }); + + it.each(['error', 'warning', 'info'] as const)( + 'keeps %s feedback above an open dialog even beside a retained success', + (type) => { + const [open, setOpen] = createSignal(false); + render(() => ( + <> + + setOpen(false)} ariaLabel="Editor"> + + + + + )); + window.showToast('success', 'Previous operation completed'); + const notifications = screen.getByRole('region', { name: 'Notifications' }); + setOpen(true); + expect(notifications).toHaveClass('z-50'); + + fireEvent.click(screen.getByRole('button', { name: 'Try operation' })); + + expect(notifications).toHaveClass('z-9999'); + expect(notifications).not.toHaveClass('z-50'); + expect(screen.getByText('Operation needs attention')).toBeInTheDocument(); + expect(screen.getByRole('dialog')).toBeInTheDocument(); + // Layering must not weaken the existing modal background isolation. + expect(notifications.closest('[inert]')).not.toBeNull(); + fireEvent.click(screen.getByRole('button', { name: 'Close editor' })); + expect(notifications.closest('[inert]')).toBeNull(); + expect(notifications).toHaveClass('z-9999'); + expect(screen.getByText('Previous operation completed')).toBeInTheDocument(); + }, + ); + it('keeps all toasts created within a batch', () => { render(() => ); diff --git a/frontend-modern/src/utils/__tests__/frontendResourceTypeBoundaries.test.ts b/frontend-modern/src/utils/__tests__/frontendResourceTypeBoundaries.test.ts index 06ab84a524..80562c8e78 100644 --- a/frontend-modern/src/utils/__tests__/frontendResourceTypeBoundaries.test.ts +++ b/frontend-modern/src/utils/__tests__/frontendResourceTypeBoundaries.test.ts @@ -3180,7 +3180,12 @@ describe('frontend resource type boundaries', () => { expect(ssoProvidersStateSource).toContain('getSSOProviderDetailsLoadErrorMessage'); expect(ssoProvidersStateSource).toContain('getSSOProviderSaveSuccessMessage'); expect(ssoProvidersStateSource).toContain('getSSOProviderDeleteSuccessMessage'); - expect(ssoProvidersStateSource).toContain('getSSOConnectionTestSuccessMessage'); + // Test results belong to the active editor, not its inert toast portal. + expect(ssoProvidersStateSource).toContain('setTestResult(result)'); + expect(ssoProvidersStateSource).toContain('getSSOProviderSaveErrorMessage(error)'); + expect(ssoProvidersStateSource).not.toContain( + 'notificationStore.success(getSSOConnectionTestSuccessMessage()', + ); expect(ssoProvidersModelSource).toContain('export const createEmptyProviderForm ='); expect(ssoProvidersModelSource).toContain('export const mapProviderDetailsToForm ='); expect(ssoProvidersModelSource).toContain('export const buildProviderPayload ='); diff --git a/frontend-modern/src/utils/__tests__/ssoProviderPresentation.test.ts b/frontend-modern/src/utils/__tests__/ssoProviderPresentation.test.ts index a13b1bb0f5..aed54dc799 100644 --- a/frontend-modern/src/utils/__tests__/ssoProviderPresentation.test.ts +++ b/frontend-modern/src/utils/__tests__/ssoProviderPresentation.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import { getSSOCopySuccessMessage, + getSSOCopyErrorMessage, getSSOProviderAddButtonLabel, getSSOCertificatePresentation, getSSOConnectionTestErrorMessage, @@ -84,6 +85,7 @@ describe('ssoProviderPresentation', () => { expect(getSSOProviderToggleSuccessMessage(true)).toBe('SSO provider has been enabled.'); expect(getSSOProviderToggleSuccessMessage(false)).toBe('SSO provider has been disabled.'); expect(getSSOProviderToggleErrorMessage()).toBe('Unable to update the SSO provider.'); + expect(getSSOCopyErrorMessage('Callback URL')).toContain('Select and copy the URL manually.'); expect(getSSOCopySuccessMessage('XML')).toBe('XML has been copied to the clipboard.'); expect(getSSOConnectionTestSuccessMessage()).toBe('Connection test completed successfully.'); expect(getSSOConnectionTestFailureMessage('Bad certificate')).toBe( diff --git a/frontend-modern/src/utils/ssoProviderPresentation.ts b/frontend-modern/src/utils/ssoProviderPresentation.ts index 787736cdde..07a78563db 100644 --- a/frontend-modern/src/utils/ssoProviderPresentation.ts +++ b/frontend-modern/src/utils/ssoProviderPresentation.ts @@ -83,6 +83,10 @@ export function getSSOCopySuccessMessage(label: string): string { return `${label} has been copied to the clipboard.`; } +export function getSSOCopyErrorMessage(label: string): string { + return `Unable to copy ${label.toLowerCase()}. Select and copy the URL manually.`; +} + export function getSSOConnectionTestSuccessMessage(): string { return 'Connection test completed successfully.'; }