From 0586e64f64dca39d8c1287a3b5686170ba04f806 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 2 Jun 2026 01:12:20 -0400 Subject: [PATCH] plan(phase1): record Task 1.2 review findings + LdapOptionsValidator 0.1.1 question --- ...6-06-02-auth-audit-normalization-phase1.md | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/docs/plans/2026-06-02-auth-audit-normalization-phase1.md b/docs/plans/2026-06-02-auth-audit-normalization-phase1.md index 1b028b4..59c3088 100644 --- a/docs/plans/2026-06-02-auth-audit-normalization-phase1.md +++ b/docs/plans/2026-06-02-auth-audit-normalization-phase1.md @@ -84,6 +84,32 @@ Cookie `ZB.MOM.WW.ScadaBridge.Auth`; JWT-in-cookie via `JwtTokenService`. 5. **MxGateway ApiKeys cutover is the donor path — lowest risk** (delete locals, re-point to library; keep `ConstraintEnforcer`/gRPC/scopes on top). Confirms the GAPS sequencing (gateway first). +## Task 1.2 (LDAP cutover) — implemented + reviewed (2026-06-02) + +Commits: OtOpcUa `257caa7`, MxGateway `c3b466e`, ScadaBridge `ac34dac`. All targeted tests green. +Security review verdict: **sound, no credential-leak regression** in any repo (insecure-transport +guards fire correctly; DevStubMode cannot leak to prod; claim shapes preserved). All three returned +CHANGES-REQUESTED for fixable issues: + +- **OtOpcUa** (no Critical): (I1) insecure-transport guard is login-time only — add startup + validation gated on `Enabled` for defense-in-depth, verify prod overlays still boot; (I2) integration + stub pre-populates `Roles` so the Groups→mapper path isn't actually exercised — fix the stub; (I3) + document/test the zero-role fail-closed fallback. +- **MxGateway** (2 Critical): (C1) library strips group DNs to short RDN names before the + `LdapGroupClaimType` claim → verify prior behaviour, document, drop the now-dead full-DN branch in the + mapper, add a claim-value assertion; (C2) gateway's local `LdapOptions` is now a shadow copy (validated + but unused at runtime) → fold to the shared type or document the drift. (I1) shared `LdapOptionsValidator` + has **no `Enabled=false` guard** → validates even when LDAP is disabled (real for MxGateway, which can + disable dashboard LDAP). +- **ScadaBridge** (2 Critical): (C1) `ConfigSecretsTests` still checks the OLD flat key → passes + vacuously, no longer guards secret-in-config — repoint to nested key; (C2) `production-checklist.md` + still lists deleted flat keys → update; (I) unsafe `(RoleMappingResult)Scope!` cast → null-guard. + +**Cross-cutting decision — shared library `LdapOptionsValidator` `Enabled` guard:** the validator runs +regardless of `Enabled`, requiring Server/SearchBase/ServiceAccountDn even when LDAP is off. Correct fix = +add an `if (!Enabled) return Success` guard to the shared validator and republish `0.1.1`, re-pinning all +consumers. (Alternative: each consumer always supplies those fields. The library fix is the principled one.) + ## Resolved decisions (2026-06-02) - **Decision A — ScadaBridge inbound API keys depth → (a) FULL ADOPT.** Re-architect inbound-API auth to the