diff --git a/CHANGELOG.md b/CHANGELOG.md index 0da3cf3..fbbbaea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -144,6 +144,26 @@ stored binding — still passes through unchecked, but that window is bounded by the 10-minute pre-login TTL. +- **OIDC provider Advanced fields are now editable in the GUI (Audit 2026-05-11 A-7).** + The MED-4 row had been DEFERRED to v3 with the rationale "backend + already accepts these fields." The verifier hit the GUI and found + that the read-only display claimed the values were editable, but the + edit form had no inputs — the save handler passed `provider.scopes` + / `provider.groups_claim_path` / `provider.groups_claim_format` / + `provider.iat_window_seconds` / `provider.jwks_cache_ttl_seconds` + unchanged from the loaded object. Operators who wanted to bump the + IAT window or change the groups-claim path had to drop to curl / + MCP and trust the GUI's display matched what they'd set elsewhere. + Lying UX. The OIDCProviderDetailPage edit form now has a collapsible + Advanced section with five inputs (scopes as a space-separated text + field; groups-claim path; groups-claim format select with the + backend's `string-array` / `json-path` enum; IAT window number input + bounded 1–600; JWKS cache TTL number input with floor 60). Client-side + validation mirrors the backend `Validate` rules so common operator + mistakes (IAT > 600, JWKS TTL < 60, empty scopes, empty groups-claim-path) + reject inline instead of round-tripping a 400. The read-only `
` + also gained the previously-invisible `jwks_cache_ttl_seconds` row. + - **Pre-login cookie Path widened from `/auth/oidc/` to `/` (Audit MED-14 follow-on).** Required to satisfy the `__Host-` prefix's `Path=/` rule. The cookie lifetime is unchanged (10 minutes) and only the callback handler diff --git a/web/src/pages/auth/OIDCProviderDetailPage.test.tsx b/web/src/pages/auth/OIDCProviderDetailPage.test.tsx index 902a7ec..da029c8 100644 --- a/web/src/pages/auth/OIDCProviderDetailPage.test.tsx +++ b/web/src/pages/auth/OIDCProviderDetailPage.test.tsx @@ -320,4 +320,178 @@ describe('OIDCProviderDetailPage', () => { expect(screen.getByTestId('oidc-provider-edit-allowed-email-domains-error')).toBeTruthy(); }); }); + + // ============================================================================= + // Audit 2026-05-11 A-7 — Advanced fields are editable (MED-4 closure). + // ============================================================================= + + async function openEditFormWithEditPerms() { + vi.mocked(client.listOIDCProviders).mockResolvedValue({ providers: [sampleProvider] }); + vi.mocked(client.updateOIDCProvider).mockResolvedValue(sampleProvider); + vi.mocked(client.authMe).mockResolvedValue({ + actor_id: 'u-admin', + actor_type: 'User', + tenant_id: 't-default', + admin: true, + roles: ['r-admin'], + effective_permissions: [ + { permission: 'auth.oidc.list', scope_type: 'global' }, + { permission: 'auth.oidc.edit', scope_type: 'global' }, + ], + }); + renderRoute(); + await waitFor(() => screen.getByTestId('oidc-provider-edit-button')); + fireEvent.click(screen.getByTestId('oidc-provider-edit-button')); + await waitFor(() => screen.getByTestId('oidc-provider-edit-advanced')); + } + + it('A-7 Advanced details section is collapsed by default and visible in edit mode', async () => { + await openEditFormWithEditPerms(); + const details = screen.getByTestId('oidc-provider-edit-advanced') as HTMLDetailsElement; + expect(details).toBeTruthy(); + //
with no `open` attribute = collapsed. + expect(details.open).toBe(false); + }); + + it('A-7 Advanced fields pre-populate from the live provider', async () => { + vi.mocked(client.listOIDCProviders).mockResolvedValue({ + providers: [{ + ...sampleProvider, + scopes: ['openid', 'profile', 'email', 'groups'], + groups_claim_path: 'realm_access.roles', + groups_claim_format: 'json-path', + iat_window_seconds: 120, + jwks_cache_ttl_seconds: 600, + }], + }); + vi.mocked(client.authMe).mockResolvedValue({ + actor_id: 'u-admin', + actor_type: 'User', + tenant_id: 't-default', + admin: true, + roles: ['r-admin'], + effective_permissions: [ + { permission: 'auth.oidc.list', scope_type: 'global' }, + { permission: 'auth.oidc.edit', scope_type: 'global' }, + ], + }); + renderRoute(); + await waitFor(() => screen.getByTestId('oidc-provider-edit-button')); + fireEvent.click(screen.getByTestId('oidc-provider-edit-button')); + await waitFor(() => screen.getByTestId('oidc-provider-edit-advanced')); + + expect((screen.getByTestId('oidc-provider-edit-scopes') as HTMLInputElement).value) + .toBe('openid profile email groups'); + expect((screen.getByTestId('oidc-provider-edit-groups-claim-path') as HTMLInputElement).value) + .toBe('realm_access.roles'); + expect((screen.getByTestId('oidc-provider-edit-groups-claim-format') as HTMLSelectElement).value) + .toBe('json-path'); + expect((screen.getByTestId('oidc-provider-edit-iat-window-seconds') as HTMLInputElement).valueAsNumber) + .toBe(120); + expect((screen.getByTestId('oidc-provider-edit-jwks-cache-ttl-seconds') as HTMLInputElement).valueAsNumber) + .toBe(600); + }); + + it('A-7 all five Advanced fields round-trip into the PUT body', async () => { + await openEditFormWithEditPerms(); + + fireEvent.change(screen.getByTestId('oidc-provider-edit-scopes'), { + target: { value: ' openid profile email groups ' }, + }); + fireEvent.change(screen.getByTestId('oidc-provider-edit-groups-claim-path'), { + target: { value: 'realm_access.roles' }, + }); + fireEvent.change(screen.getByTestId('oidc-provider-edit-groups-claim-format'), { + target: { value: 'json-path' }, + }); + fireEvent.change(screen.getByTestId('oidc-provider-edit-iat-window-seconds'), { + target: { value: '120' }, + }); + fireEvent.change(screen.getByTestId('oidc-provider-edit-jwks-cache-ttl-seconds'), { + target: { value: '600' }, + }); + fireEvent.click(screen.getByTestId('oidc-provider-save-button')); + + await waitFor(() => expect(client.updateOIDCProvider).toHaveBeenCalledTimes(1)); + const [, body] = vi.mocked(client.updateOIDCProvider).mock.calls[0]; + // Whitespace normalization: collapsed runs, no empty strings. + expect(body.scopes).toEqual(['openid', 'profile', 'email', 'groups']); + expect(body.groups_claim_path).toBe('realm_access.roles'); + expect(body.groups_claim_format).toBe('json-path'); + expect(body.iat_window_seconds).toBe(120); + expect(body.jwks_cache_ttl_seconds).toBe(600); + }); + + it('A-7 IAT window above 600 rejects with inline error and does NOT POST', async () => { + await openEditFormWithEditPerms(); + fireEvent.change(screen.getByTestId('oidc-provider-edit-iat-window-seconds'), { + target: { value: '601' }, + }); + fireEvent.click(screen.getByTestId('oidc-provider-save-button')); + await waitFor(() => screen.getByTestId('oidc-provider-detail-error')); + expect(screen.getByTestId('oidc-provider-detail-error').textContent).toContain('IAT window'); + expect(client.updateOIDCProvider).not.toHaveBeenCalled(); + }); + + it('A-7 IAT window <= 0 rejects with inline error', async () => { + await openEditFormWithEditPerms(); + fireEvent.change(screen.getByTestId('oidc-provider-edit-iat-window-seconds'), { + target: { value: '0' }, + }); + fireEvent.click(screen.getByTestId('oidc-provider-save-button')); + await waitFor(() => screen.getByTestId('oidc-provider-detail-error')); + expect(client.updateOIDCProvider).not.toHaveBeenCalled(); + }); + + it('A-7 JWKS cache TTL below 60 rejects with inline error', async () => { + await openEditFormWithEditPerms(); + fireEvent.change(screen.getByTestId('oidc-provider-edit-jwks-cache-ttl-seconds'), { + target: { value: '30' }, + }); + fireEvent.click(screen.getByTestId('oidc-provider-save-button')); + await waitFor(() => screen.getByTestId('oidc-provider-detail-error')); + expect(screen.getByTestId('oidc-provider-detail-error').textContent).toContain('JWKS'); + expect(client.updateOIDCProvider).not.toHaveBeenCalled(); + }); + + it('A-7 empty scopes input rejects (operator can\'t accidentally wipe the array)', async () => { + await openEditFormWithEditPerms(); + fireEvent.change(screen.getByTestId('oidc-provider-edit-scopes'), { + target: { value: ' ' }, + }); + fireEvent.click(screen.getByTestId('oidc-provider-save-button')); + await waitFor(() => screen.getByTestId('oidc-provider-detail-error')); + expect(screen.getByTestId('oidc-provider-detail-error').textContent).toContain('Scopes'); + expect(client.updateOIDCProvider).not.toHaveBeenCalled(); + }); + + it('A-7 empty groups-claim-path rejects', async () => { + await openEditFormWithEditPerms(); + fireEvent.change(screen.getByTestId('oidc-provider-edit-groups-claim-path'), { + target: { value: ' ' }, + }); + fireEvent.click(screen.getByTestId('oidc-provider-save-button')); + await waitFor(() => screen.getByTestId('oidc-provider-detail-error')); + expect(screen.getByTestId('oidc-provider-detail-error').textContent).toContain('Groups claim path'); + expect(client.updateOIDCProvider).not.toHaveBeenCalled(); + }); + + it('A-7 unchanged Advanced fields still round-trip as the existing values (no lying field)', async () => { + await openEditFormWithEditPerms(); + // Operator only changes Display name; advanced section is untouched. + fireEvent.change(screen.getByTestId('oidc-provider-edit-name'), { + target: { value: 'Okta Rename' }, + }); + fireEvent.click(screen.getByTestId('oidc-provider-save-button')); + await waitFor(() => expect(client.updateOIDCProvider).toHaveBeenCalledTimes(1)); + const [, body] = vi.mocked(client.updateOIDCProvider).mock.calls[0]; + // Pre-A-7 these would have been the provider's pass-through; now + // they come from state pre-populated by startEdit. Either way the + // wire value should be the live provider's existing config. + expect(body.scopes).toEqual(sampleProvider.scopes); + expect(body.groups_claim_path).toBe(sampleProvider.groups_claim_path); + expect(body.groups_claim_format).toBe(sampleProvider.groups_claim_format); + expect(body.iat_window_seconds).toBe(sampleProvider.iat_window_seconds); + expect(body.jwks_cache_ttl_seconds).toBe(sampleProvider.jwks_cache_ttl_seconds); + }); }); diff --git a/web/src/pages/auth/OIDCProviderDetailPage.tsx b/web/src/pages/auth/OIDCProviderDetailPage.tsx index ed32fe9..2bef966 100644 --- a/web/src/pages/auth/OIDCProviderDetailPage.tsx +++ b/web/src/pages/auth/OIDCProviderDetailPage.tsx @@ -55,6 +55,26 @@ export default function OIDCProviderDetailPage() { const [editAllowedEmailDomains, setEditAllowedEmailDomains] = useState([]); const [emailDomainInput, setEmailDomainInput] = useState(''); const [emailDomainErr, setEmailDomainErr] = useState(null); + // Audit 2026-05-11 A-7 — Advanced edit fields. Pre-fix, the saveEdit + // handler passed these through unchanged from the provider object, + // so the read-only `
` claimed the value was editable but the + // PUT body never carried operator input. The 5 fields the backend + // validator accepts (internal/auth/oidc/domain/types.go::Validate): + // - scopes (string array; min 1 entry; default openid profile email) + // - groups_claim_path (string; default "groups") + // - groups_claim_format (enum: string-array | json-path) + // - iat_window_seconds (int, 1–600; default 300) + // - jwks_cache_ttl_seconds (int, ≥60; default 3600) + // Scopes are rendered as a space-separated text input (single-line) + // because that's the operator's mental model — every OIDC IdP docs + // page shows scopes as space-separated. The submit handler splits on + // whitespace + filters empty strings; an empty input renders an + // inline error rather than wiping the array. + const [editScopesInput, setEditScopesInput] = useState(''); + const [editGroupsClaimPath, setEditGroupsClaimPath] = useState(''); + const [editGroupsClaimFormat, setEditGroupsClaimFormat] = useState('string-array'); + const [editIATWindow, setEditIATWindow] = useState(300); + const [editJWKSCacheTTL, setEditJWKSCacheTTL] = useState(3600); const [submitting, setSubmitting] = useState(false); const [error, setError] = useState(null); const [success, setSuccess] = useState(null); @@ -106,6 +126,14 @@ export default function OIDCProviderDetailPage() { setEditAllowedEmailDomains([...(provider.allowed_email_domains || [])]); setEmailDomainInput(''); setEmailDomainErr(null); + // Audit 2026-05-11 A-7 — pre-populate the Advanced fields from + // the live provider so the operator sees the current values when + // they expand the section. + setEditScopesInput((provider.scopes ?? []).join(' ')); + setEditGroupsClaimPath(provider.groups_claim_path || 'groups'); + setEditGroupsClaimFormat(provider.groups_claim_format || 'string-array'); + setEditIATWindow(provider.iat_window_seconds || 300); + setEditJWKSCacheTTL(provider.jwks_cache_ttl_seconds || 3600); setError(null); setSuccess(null); setEditing(true); @@ -155,20 +183,62 @@ export default function OIDCProviderDetailPage() { setError(null); setSuccess(null); try { + // Audit 2026-05-11 A-7 — client-side validation mirrors the + // backend's internal/auth/oidc/domain/types.go::Validate rules. + // Server is still the source of truth (we surface its 400 if + // anything slips past); the client validator is for fast + // feedback so operators don't round-trip just to learn that + // "iat_window_seconds=601" is rejected. + const trimmedPath = editGroupsClaimPath.trim(); + if (trimmedPath === '') { + setError('Groups claim path cannot be empty (default: "groups").'); + setSubmitting(false); + return; + } + if (editGroupsClaimFormat !== 'string-array' && editGroupsClaimFormat !== 'json-path') { + setError('Groups claim format must be "string-array" or "json-path".'); + setSubmitting(false); + return; + } + const scopes = editScopesInput + .trim() + .split(/\s+/) + .filter(s => s.length > 0); + if (scopes.length === 0) { + setError('Scopes cannot be empty. At minimum include "openid".'); + setSubmitting(false); + return; + } + if (!Number.isInteger(editIATWindow) || editIATWindow <= 0 || editIATWindow > 600) { + setError('IAT window must be a positive integer ≤ 600 seconds.'); + setSubmitting(false); + return; + } + if (!Number.isInteger(editJWKSCacheTTL) || editJWKSCacheTTL < 60) { + setError('JWKS cache TTL must be an integer ≥ 60 seconds.'); + setSubmitting(false); + return; + } + const req: Parameters[1] = { name: editName, issuer_url: editIssuerURL, client_id: editClientID, redirect_uri: editRedirectURI, - groups_claim_path: provider.groups_claim_path, - groups_claim_format: provider.groups_claim_format, + // Audit 2026-05-11 A-7 — formerly pass-through from + // provider.*, now wired to the operator-edited state. Lying + // UX retired: the read-only `
` no longer claims a value + // can be changed when the saveEdit handler ignores the + // change. + groups_claim_path: trimmedPath, + groups_claim_format: editGroupsClaimFormat, fetch_userinfo: editFetchUserinfo, - scopes: provider.scopes, + scopes, // Audit 2026-05-11 A-3 — wire the chip-list value into the PUT // body. Backend persists [] as no-gate; the field is honest now. allowed_email_domains: editAllowedEmailDomains, - iat_window_seconds: provider.iat_window_seconds, - jwks_cache_ttl_seconds: provider.jwks_cache_ttl_seconds, + iat_window_seconds: editIATWindow, + jwks_cache_ttl_seconds: editJWKSCacheTTL, }; if (editClientSecret) req.client_secret = editClientSecret; await updateOIDCProvider(provider.id, req); @@ -270,6 +340,11 @@ export default function OIDCProviderDetailPage() {
IAT window
{provider.iat_window_seconds}s
+ {/* Audit 2026-05-11 A-7 — JWKS cache TTL surfaced in + read-only view too (pre-fix the value was persisted but + invisible). */} +
JWKS cache TTL
+
{provider.jwks_cache_ttl_seconds}s
) : (
@@ -415,6 +490,121 @@ export default function OIDCProviderDetailPage() {

)}
+ + {/* Audit 2026-05-11 A-7 — Advanced section. Five fields the + read-only
claimed were editable but the saveEdit + handler was passing through unchanged from the loaded + provider object. Each input has an inline help line that + links the operator's mental model to the backend + semantic (`internal/auth/oidc/domain/types.go::Validate` + rules). The section is collapsed by default — most + edits don't touch these fields, so they shouldn't + clutter the primary form. */} +
+ + Advanced (scopes, groups claim, IAT / JWKS TTL) + +
+
+ + setEditScopesInput(e.target.value)} + placeholder="openid profile email" + className="w-full px-3 py-1.5 text-sm border border-surface-border rounded bg-page text-ink font-mono" + data-testid="oidc-provider-edit-scopes" + /> +

+ Default openid profile email. Some IdPs need groups for + the group-claim path; Auth0 namespaces groups under a custom claim. Must include{' '} + openid. +

+
+
+
+ + setEditGroupsClaimPath(e.target.value)} + placeholder="groups" + className="w-full px-3 py-1.5 text-sm border border-surface-border rounded bg-page text-ink font-mono" + data-testid="oidc-provider-edit-groups-claim-path" + /> +

+ JSON path within the ID token (or userinfo if fallback enabled) that holds the + group list. Common: groups, realm_access.roles + {' '}(Keycloak), namespaced URLs (Auth0). +

+
+
+ + +

+ How the IdP encodes the group list. Most IdPs emit a JSON array — keep the + default. Use json-path when the claim is a nested object the + path needs to traverse. +

+
+
+
+
+ + setEditIATWindow(Number(e.target.value))} + className="w-full px-3 py-1.5 text-sm border border-surface-border rounded bg-page text-ink" + data-testid="oidc-provider-edit-iat-window-seconds" + /> +

+ Maximum ID-token age at consume time (RFC 7519 §4.1.6). Default 300. Range + 1–600. Tighter = more replay-resistant; looser = more clock-skew-tolerant. +

+
+
+ + setEditJWKSCacheTTL(Number(e.target.value))} + className="w-full px-3 py-1.5 text-sm border border-surface-border rounded bg-page text-ink" + data-testid="oidc-provider-edit-jwks-cache-ttl-seconds" + /> +

+ How long to cache the IdP's signing-key set before re-fetching. Default 3600 + (1h); floor 60. MED-6 auto-refresh-on-cache-miss covers most rotation events; + this knob is for slow-rotation IdPs that want longer caching. +

+
+
+
+
)}