2 Commits

Author	SHA1	Message	Date
shankar0123	34d5200904	fix(auth): ARCH-002 — relax OIDC runtime guard, full Bundle-2 stack ships ... Sprint 4 unified-master-audit closure. The README has advertised OIDC SSO as a v2.1 feature (L18, L74) but cmd/server/main.go retained a Bundle-2-Phase-0 runtime guard that os.Exit(1)'d the moment any operator set CERTCTL_AUTH_TYPE=oidc: CERTCTL_AUTH_TYPE=oidc: the OIDC auth chain is not yet wired in this build (Auth Bundle 2 Phase 6 ships the session middleware that consumes this auth-type literal). That message was true when Phase 0 landed (the literal got reserved in ValidAuthTypes ahead of the handler chain). It's been stale since Phase 6 shipped. As of 2026-05-16 the full stack is live: - session.NewService at cmd/server/main.go:394 - oidcsvc.NewService at cmd/server/main.go:436 - ChainAuthSessionThenBearer at cmd/server/main.go:2012 - csrfMiddleware at cmd/server/main.go:2017 - /auth/oidc/{login,callback,back-channel-logout} routes at router.go - 6 OIDC handler files in internal/api/handler/ - 2,852 LOC in internal/auth/oidc/ + 1,632 LOC in internal/auth/session/ Fix: - Introduce config.IsRuntimeSupportedAuthType(AuthType) as the single source of truth for which auth-type literals the cmd/server runtime guard accepts. The set is {api-key, none, oidc} — every entry in ValidAuthTypes(). The helper exists so the test suite can pin the invariant 'ValidAuthTypes ⊆ runtime-supported' without grepping cmd/server source. - cmd/server/main.go's switch collapses to a single IsRuntimeSupportedAuthType check; the dedicated AuthTypeOIDC fail-loud case is gone. The G-1 silent-auth-downgrade invariant stays intact — 'jwt' is still rejected at config.Validate() time (never made it into ValidAuthTypes()). - internal/config/auth.go AuthTypeOIDC comment updated to reflect the post-Phase-6 reality (it was prescriptive pre-fix: 'Once Bundle 2's session middleware + OIDC service ship, the runtime guard relaxes' — that condition is met). Regression coverage: - TestIsRuntimeSupportedAuthType_AcceptsAllValidEntries — every valid type is runtime-supported (catches future drift). - TestIsRuntimeSupportedAuthType_AcceptsOIDC — explicit pin on the ARCH-002 invariant. - TestIsRuntimeSupportedAuthType_RejectsUnknown — 'jwt', empty, 'saml', 'mtls', 'API-KEY' all rejected. (Also lands the ARCH-003 keygen-mode tests in the same file — contiguous hunk in config_test.go.) Closes ARCH-002.	2026-05-16 04:53:36 +00:00
shankar0123	51f9cf13dc	refactor(config): extract Auth family + 2 exported + 1 unexported helpers (Phase 9, 5 of N) ... The biggest single-sprint cut so far (-502 lines) and the FIRST split that moves EXPORTED helpers. Public-surface invariant verified end-to- end via broader-importer build (cmd/server + internal/auth + internal/api/...). What moved ========== internal/config/auth.go (new, 601 lines including BSL header + Phase 9 doc-comment + 4 imports + 5 types + 3 helpers) Five types: - NamedAPIKey (one named API-key entry; admin flag for actor attribution in audit trail) - AuthType (+ 3 consts: AuthTypeAPIKey / AuthTypeNone / AuthTypeOIDC — the typed enum that replaced the pre-G-1 string-literal map. "jwt" stays out forever per ValidAuthTypes() invariant pinned by config_test.go's property test) - AuthConfig (top-level: Type, Secret, NamedKeys, AgentBootstrapToken + DenyEmpty flag, Session, TrustedProxies, DemoModeAck + TS + ResidualStrict, OIDC pre-login binding knobs, Breakglass, BootstrapAdminGroups + ProviderID + BootstrapToken) - SessionConfig (Auth Bundle 2 Phase 4: IdleTimeout, AbsoluteTimeout, SigningKeyRetention, GCInterval, SameSite, BindIP, BindUserAgent) - BreakglassConfig (Auth Bundle 2 Phase 7.5: Enabled + LockoutThreshold + Duration + Reset) Three helpers (TWO exported — first sprint to move public-API): - ValidAuthTypes() — single source of truth for the allowed CERTCTL_AUTH_TYPE set. EXPORTED. External callers (verified clean via broader-importer build): cmd/server/main.go:115 internal/auth/middleware.go (doc ref) internal/api/handler/health.go (doc ref) - ParseNamedAPIKeys() — parses CERTCTL_API_KEYS_NAMED with L-004 rotation-aware duplicate-name handling + slog.Info "rotation window active" observability. EXPORTED. Test caller in config_test.go + production caller in Load() in config.go (intra-package, resolves via same-package lookup after move). - isValidKeyName() — alphanumeric + hyphen + underscore validator. Unexported; only called from ParseNamedAPIKeys (intra-file edge after the move — one fewer cross-file edge). External-importer surface (verified resolves clean post-move) ============================================================== The package name stays `config`, so every external reference continues to resolve. Live grep confirms the surface: cmd/server/main.go: - config.AuthType(...) (cast) - config.AuthTypeNone (const) - config.AuthTypeAPIKey (const) - config.AuthTypeOIDC (const) - config.ValidAuthTypes() (func) cmd/server/auth_backfill.go: - config.AuthType(...) (cast) - config.AuthTypeNone (const) internal/auth/middleware.go: - config.AuthType (doc reference + field-comment) - config.AuthTypeConsts (doc reference) internal/api/handler/health.go: - config.AuthType + config.ValidAuthTypes() (doc references) Verification (the critical broader-importer build): go build ./cmd/server/... ./internal/auth/... ./internal/api/router/... ./internal/api/handler/... ./internal/scheduler/... → clean If the move had accidentally renamed a symbol or changed a package boundary, that broader build would have failed loud. What stayed in config.go (intentionally) ======================================== - ErrAgentBootstrapTokenRequired sentinel (top-of-file Phase-2 sentinel block) — tied to Validate()'s fail-closed behavior, not to AuthConfig's struct shape. Same precedent as Sprint 2's ErrACMEInsecureWithoutAck and Sprint 3's leaving ErrDemoModeAckExpired in place. - demoModeAckMaxAge const (top-of-file) — tied to Validate()'s 24h TS-freshness check, not to struct shape. - The Validate() body that branches on AuthType / DemoModeAck / AgentBootstrapTokenDenyEmpty / DemoModeResidualStrict — cross- cutting validation logic that stays where the other Validate() branches live. - The Load() body that calls ParseNamedAPIKeys() during initial cfg.Auth.NamedKeys construction; same-package resolution. - Shared getEnv / getEnvBool / getEnvInt / getEnvDuration + splitComma + trimSpace helpers (splitComma + trimSpace are used by ParseNamedAPIKeys via same-package lookup). Edit shape ========== Two sed passes (the now-standard Sprint-3-onward pattern): 1. sed -i '847,1204d' — deleted the 358-line struct + enum + ValidAuthTypes block. 2. sed -i '1925,2068d' — deleted the 144-line helper block (positions shifted by Sprint 5's struct removal already applied; ParseNamedAPIKeys' new doc-comment start moved from 2283 → 1925). Then gofmt -w. No residual double-blank-line at either join — both removals happened mid-blank-separated regions cleanly. Public-surface invariant ======================== Every type, exported function, exported constant, exported field, and doc-comment is byte-identical to pre-split. Package stays `config`. Every external caller path is preserved. Verification (all clean): gofmt -l internal/config/ → clean go build ./internal/config/... → clean go test ./internal/config/... -count=1 → ok (0.70s) staticcheck ./internal/config/... → clean go build ./cmd/server/... ./internal/auth/... ./internal/api/router/... ./internal/api/handler/... ./internal/scheduler/... → clean grep -nE '^type (AuthConfig|SessionConfig|BreakglassConfig|NamedAPIKey|AuthType)|^func (ValidAuthTypes|ParseNamedAPIKeys|isValidKeyName)' internal/config/config.go → empty (none remain in config.go) grep -nE '^type (AuthConfig|SessionConfig|BreakglassConfig|NamedAPIKey|AuthType)|^func (ValidAuthTypes|ParseNamedAPIKeys|isValidKeyName)' internal/config/auth.go → 5 types + 3 funcs (correct) LOC delta: config.go: 2467 → 1963 (-504 lines: -358 struct block, -144 helper block, -2 from misc cleanup collapse) auth.go: new, 601 lines (incl. 101-line Phase 9 doc-comment + BSL header + package decl + 4 imports) Notable milestone: config.go is now BELOW 2000 LOC for the first time since the original audit. From 3403 → 1963 = -42.3% across Sprints 1+2+3+4+5. Cumulative Phase 9 progress (Sprints 1+2+3+4+5 from config.go): Pre-Phase-9: 3403 LOC After Sprint 1 (Notifier): 3335 LOC (-68) After Sprint 2 (ACME): 3108 LOC (-227) After Sprint 3 (SCEP): 2774 LOC (-334) After Sprint 4 (EST): 2467 LOC (-307) After Sprint 5 (Auth): 1963 LOC (-504) Total Sprint 1+2+3+4+5: -1440 LOC (-42.3%) Pattern lesson — exported-helper move ===================================== Pre-move check: enumerate every external caller via `grep -rnE 'config\.<Symbol>'`. If the symbol's external callers ARE all inside the same package, the move is trivial. If they're external, the move is still safe IFF the package name doesn't change — only the file the symbol lives IN changes. Same-package resolution at compile time guarantees the import-path that external code uses (`config.AuthType`, `config.ValidAuthTypes`) keeps working. The broader-importer build is the load-bearing verification: if it goes red, the move is wrong; green = safe. Next queued (Sprint 6): Server family from config.go → internal/config/server.go (~270 LOC). Includes ServerConfig + ServerTLSConfig + DatabaseConfig + SchedulerConfig + LogConfig + RateLimitConfig + CORSConfig + isLoopbackAddr (unexported HIGH-12 demo-mode helper). No exported helpers — back to the Sprint-3-style helper-bundle pattern, just bigger family. Closes: cowork/certctl-architecture-diligence-audit.html#fix-ARCH-M2 (partial — 5 of 12 — full ARCH-M2 closure is the aggregate)	2026-05-14 04:35:39 +00:00