plan(phase1): record Task 1.2 review findings + LdapOptionsValidator 0.1.1 question

This commit is contained in:
Joseph Doherty
2026-06-02 01:12:20 -04:00
parent 37c03e5fc2
commit 0586e64f64
@@ -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