2 Commits

Author	SHA1	Message	Date
shankar0123	9cce2ab043	harden(auth): LOW + Nit batch — bootstrap audit, crypto/rand, XFF trust, CSRF check, protocol-prefix unify (Batch 1) ... Audit 2026-05-10 — close 8 LOWs + 2 Nits in-bundle. Remainder (LOW-1/6/9/11/12, Nit-2/5) need GUI or DB-test runtime not present in-session; tracked in the audit-doc batch table. LOW-2: bootstrap.ValidateAndMint now emits 'bootstrap.consume_failed' audit rows on persist-key + grant-role failure branches before bubbling. Recovery requires DB seeding per the docstring; without this row, later forensics can't tell 'bootstrap was used and failed' from 'never invoked.' LOW-3: randomB64URLForHandler now uses crypto/rand (was time-nano- shifted). Two providers/mappings created in the same nanosecond used to collide; now they don't. Time-nano fallback retained for the unlikely crypto/rand-broken path. LOW-4: breakglass.verifyDummy uses s.readRand(salt) for the dummy Argon2id verify. Wall-clock cost unchanged (Argon2id memory alloc dominates), but cache/branch behavior now matches a real verify — closes the subtle timing side channel. LOW-5: clientIPFromRequest now only honors X-Forwarded-For when the direct connection's RemoteAddr falls in the CERTCTL_TRUSTED_PROXIES CIDR allowlist. Default-deny: empty list means XFF is ignored. SetTrustedProxies wired in cmd/server/main.go from cfg.Auth.TrustedProxies. LOW-7: internal/auth/protocol_endpoints.go::ProtocolEndpointPrefixes now carries /scep-mtls + /.well-known/est-mtls (previously only in router.AuthExemptDispatchPrefixes; the two lists had drifted). The canonical-prefix coverage test in Phase 12 still pins the set. LOW-8: docs/operator/rbac.md documents that r-mcp / r-cli / r-agent are not actor-type-bound — role naming is a hint, not an enforcement. Operators wanting hard binding must apply periodic audit queries. Native binding is on the v2 roadmap. LOW-10: Session.Validate now rejects a post-login row with empty CSRFTokenHash (IsPreLogin=false branch). validSession test fixture updated with a valid 64-hex CSRF hash. Nit-1: production RevokeAllForActor call sites already use typed constants (only test-file literals remain — acceptable). Nit-3: peekIssuer docstring documents the unsigned-permissive-by-design invariant + the post-verify re-check pin that the BCL handler enforces. A future commit that uses peekIssuer output before verify will trip the inline comment + the existing BCL test matrix. Status table updated in cowork/auth-bundles-audit-2026-05-10.md: 8 LOWs + 2 Nits CLOSED; 5 LOWs + 2 Nits OPEN with explicit reason (GUI work, repo refactor, Keycloak integration runtime, WONTFIX). Refs: cowork/auth-bundles-audit-2026-05-10.md LOW-2/3/4/5/7/8/10 cowork/auth-bundles-audit-2026-05-10.md Nit-1/3	2026-05-10 22:26:12 +00:00
shankar0123	d473398aba	auth-bundle-1 Phase 3 (primitive): RequirePermission middleware + demo-mode + protocol allowlist ... Bundle 1 / Phase 3 (primitive ship): the load-bearing RBAC middleware factory plus its dependencies. Handler conversion sweep (5 admin files: bulk_revocation.go, admin_crl_cache.go, admin_scep_intune.go, admin_est.go, intermediate_ca.go) + m008_admin_gate_test.go registry update is Phase 3.5 follow-on; this commit ships the primitive so 3.5 is mechanical. New context keys (internal/auth/context.go): ActorIDKey, ActorTypeKey, TenantIDKey alongside the legacy UserKey + AdminKey. New helpers GetActorID / GetActorType / GetTenantID with safe fallbacks (UserKey for actor id, ActorTypeAPIKey for missing type, DefaultTenantID for missing tenant). Constants DemoAnonActorID + ActorTypeAPIKey + ActorTypeAnonymous mirror internal/domain/auth without an import cycle. RequirePermission factory (internal/auth/require_permission.go): wraps a handler and gates it behind a named permission. 401 when no actor, 403 when actor lacks permission, 500 on repository error. Skips the gate entirely for protocol endpoints (ACME / SCEP / EST / OCSP / CRL) per the audit's Category F do-not-gate allowlist. PermissionChecker is an interface so internal/auth doesn't depend on internal/service/auth (cmd/server wires the concrete Authorizer at startup). HasPermission is the imperative variant for handlers that branch behaviour rather than 403'ing. ScopeFunc closure extracts the scope type + id from the request for per-resource gating. Protocol-endpoint allowlist (internal/auth/protocol_endpoints.go): IsProtocolEndpoint matches /acme, /scep, /.well-known/est, /.well-known/pki/ocsp, /.well-known/pki/crl prefixes. Adding a new protocol endpoint MUST update this list and add a parallel test. Demo-mode synthetic admin (internal/auth/middleware.go::NewDemoModeAuth): when CERTCTL_AUTH_TYPE=none is configured, this middleware injects ActorID=actor-demo-anon, ActorType=Anonymous, TenantID=t-default, plus the legacy UserKey + AdminKey for back-compat with existing handlers. The synthetic actor's admin-role grant is seeded by migration 000029 so RequirePermission resolves through the JOIN like any other actor. cmd/server startup wires this middleware only when none-mode is configured. API-key middleware extension: NewAuthWithNamedKeys now populates the new keys (ActorIDKey, ActorTypeKey=APIKey, TenantIDKey=t-default) alongside UserKey + AdminKey on every successful Bearer match. Existing handlers continue to read UserKey / IsAdmin until the Phase 3.5 sweep converts them to RequirePermission. Test coverage: TestRequirePermission_NoActorReturns401, TestRequirePermission_GrantedActorReaches200, TestRequirePermission_DeniedActorReturns403, TestRequirePermission_CheckerErrorReturns500, TestRequirePermission_ProtocolEndpointBypassesGate (covers all 5 prefixes), TestRequirePermission_ScopeFnExtractsResourceID, TestIsProtocolEndpoint_PrefixesOnly, TestNewDemoModeAuth_InjectsSyntheticActor, TestNewAuthWithNamedKeys_PopulatesPhase3ContextKeys. fakeChecker pins the contract without a database. Phase 3.5 follow-on (NOT in this commit): convert each of the 5 admin handlers from auth.IsAdmin checks to auth.RequirePermission middleware in router.go; update internal/api/handler/m008_admin_gate_test.go to track auth.RequirePermission call sites instead of (or alongside) auth.IsAdmin; pick the right permission per handler (cert.revoke for bulk_revocation, etc.). Each handler conversion needs the 3-test triplet (_NonAdmin_Returns403 / _AdminExplicitFalse_Returns403 / _AdminPermitted_ForwardsActor) per M-008. Branch: dev/auth-bundle-1. Phase 2 was prior commit (service layer). Phase 3.5 (handler conversion) + Phase 4 (HTTP API) on the next session.	2026-05-09 16:20:04 +00:00