feat(gui/oidc): editable Advanced form on OIDCProviderDetailPage (A-7 / MED-4)

The 2026-05-10 audit tagged MED-4 as DEFERRED to v3 with the rationale
"backend already accepts the five fields." The 2026-05-11 adversarial
review verified the deferral framing was inaccurate — the read-only
`<dl>` rendered scopes / groups_claim_path / groups_claim_format /
iat_window_seconds (and persisted but invisible jwks_cache_ttl_seconds),
which gave operators the impression those fields were editable.
Switching to edit mode revealed no inputs but the saveEdit handler at
OIDCProviderDetailPage.tsx:107-134 silently passed `provider.scopes` /
`provider.groups_claim_path` / etc. through to the PUT body unchanged
from the loaded provider object.

Result: a "lying UX" anti-pattern. The page collected updates to other
fields (display name, issuer URL, client secret, redirect URI,
fetch_userinfo), the PUT succeeded with HTTP 204, and no error fired —
but the displayed Advanced values were whatever the create form
persisted or curl last set. A second operator bumping `iat_window_seconds`
from 60 to 300 had to drop to curl. The "DEFERRED to v3" framing hid
the gap from acquisition reviewers who only inspect the GUI.

Closure (frontend-only — backend already accepts all 5 fields on
`PUT /api/v1/auth/oidc/providers/{id}`):

  OIDCProviderDetailPage.tsx
    - New `<details data-testid="oidc-provider-edit-advanced">` section
      collapsed by default inside the edit form. Most edits don't
      touch these fields, so they shouldn't clutter the primary form.
    - Five new inputs wired through component state:
      * `editScopesInput` — text input rendered as space-separated
        string per OIDC convention (every IdP docs page shows scopes
        that way). Submit splits on whitespace + filters empty strings.
      * `editGroupsClaimPath` — text input with `groups` default.
      * `editGroupsClaimFormat` — select with the actual backend enum
        `string-array` | `json-path` (NOT `string_array` /
        `space_separated` / `comma_separated` as the spec mistakenly
        proposed — those values don't exist in
        `internal/auth/oidc/domain/types.go::GroupsClaimFormat*`).
      * `editIATWindow` — number input with `min=1, max=600` matching
        `MaxIATWindowSeconds=600` from the domain validator.
      * `editJWKSCacheTTL` — number input with `min=60` matching
        `MinJWKSCacheTTLSeconds=60`.
    - `startEdit` pre-populates all five from the live provider so
      operators see current values when expanding the section.
    - `saveEdit` validates client-side mirroring the backend
      `Validate` rules (empty scopes / empty path / invalid format /
      IAT out of (0, 600] / JWKS < 60) → inline error + does NOT
      POST. Server is still source-of-truth; any 400 surfaces via
      the existing error UI.
    - Read-only `<dl>` gained the previously-invisible
      `jwks_cache_ttl_seconds` row so all five values are visible
      without entering edit mode.

  Each input carries a help paragraph linking the operator mental
  model to the backend semantic (e.g. Keycloak's
  `realm_access.roles`, Auth0's namespaced claims; RFC 7519 §4.1.6
  for IAT; MED-6 auto-refresh-on-cache-miss for the JWKS TTL).

Tests (9 new + 5 pre-existing, all passing under vitest):

  A-7 Advanced details section is collapsed by default and visible
    in edit mode — pin <details> has no `open` attribute initially.
  A-7 Advanced fields pre-populate from the live provider — start
    edit with a non-default provider (Keycloak shape: realm_access.roles,
    json-path, IAT=120, JWKS TTL=600); assert each input carries the
    live value.
  A-7 all five Advanced fields round-trip into the PUT body — change
    every field, submit, assert the PUT body carries the parsed shapes
    (whitespace-normalized scopes array, trimmed groups_claim_path,
    enum value, numeric values).
  A-7 IAT window above 600 rejects with inline error and does NOT POST
    — operator types 601, save handler rejects before reaching
    updateOIDCProvider.
  A-7 IAT window <= 0 rejects with inline error.
  A-7 JWKS cache TTL below 60 rejects with inline error.
  A-7 empty scopes input rejects — guards against operator
    accidentally wiping the array via whitespace.
  A-7 empty groups-claim-path rejects.
  A-7 unchanged Advanced fields still round-trip as the existing
    values — pin that a name-only edit still carries the live
    advanced config (no regression to the pass-through behavior;
    operators don't lose their config when editing other fields).

Verify gate green: tsc --noEmit clean; vitest passes all 14 tests
in OIDCProviderDetailPage.test.tsx (5 pre-existing + 9 new A-7
cases).

Spec at cowork/auth-bundles-fixes-2026-05-11/07-high-oidc-provider-advanced-form.md.
Audit doc: MED-4 section in cowork/auth-bundles-audit-2026-05-10.md
appended with the A-7 follow-up closure annotation correcting the
"DEFERRED to v3" framing and explaining the lying-UX pattern;
status table row updated from "CLOSED" (incorrectly tagged on the
pass-through behavior) to "CLOSED 2026-05-11 (A-7)" with the
5-field enumeration. Operator-visible CHANGELOG.md entry under
Security retires the lying-UX caveat.
This commit is contained in:
shankar0123
2026-05-11 11:14:49 +00:00
parent 191384c1d2
commit 9af5dad2b0
3 changed files with 389 additions and 5 deletions
+20
View File
@@ -17,6 +17,26 @@
### Security
- **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 1600; 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 `<dl>`
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