From 99826c11e6df4b345475621a9d510f085d56836d Mon Sep 17 00:00:00 2001 From: shankar0123 Date: Sat, 9 May 2026 19:33:07 +0000 Subject: [PATCH] auth-bundle-1 Phase 0-5 closure: demo-mode wire, named-key backfill, AuthCheck enrichment, OpenAPI schema, intermediate-ca comment refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the 5 gaps the post-Phase-5 audit flagged on dev/auth-bundle-1. C1: cmd/server/main.go now selects auth.NewDemoModeAuth() when CERTCTL_AUTH_TYPE=none and falls back to auth.NewAuthWithNamedKeys otherwise. Pre-closure, the no-op pass-through that NewAuthWithNamedKeys returns for empty keys would have left ActorIDKey / ActorTypeKey / TenantIDKey unpopulated and 401'd every Phase-3.5 rbacGate-wrapped admin route + every Phase-4 RBAC handler in demo deployments. NewDemoModeAuth injects the synthetic 'actor-demo-anon' actor seeded by migration 000029, which holds r-admin at global scope. C2: backfillNamedKeyActorRoles startup hook (cmd/server/auth_backfill.go) iterates CERTCTL_API_KEYS_NAMED entries (and legacy CERTCTL_AUTH_SECRET synthesized fallbacks) and grants r-admin or r-viewer to each via authActorRoleRepo.Grant before the HTTP server starts accepting requests. Idempotent via ON CONFLICT DO NOTHING in the repo. Failures log a warning but are non-fatal — the server still starts and the operator can fix grants via /v1/auth/keys. Helper extracted from main.go so the role-mapping invariant is pinned by 4 focused unit tests (admin->r-admin, non-admin->r-viewer, empty no-op, grant-error non-fatal, nil-logger safe). M1: HealthHandler.AuthCheck now returns actor_id, actor_type, tenant_id, roles, effective_permissions, and admin_via_role when the optional AuthCheckResolver is wired (production path: authCheckResolverAdapter wraps the postgres ActorRoleRepository in main.go). Nil resolver preserves the legacy {status, user, admin} contract for back-compat with pre-Bundle-1 GUIs and test fixtures. Adds 2 regression tests + 1 fake resolver shim. M2: refreshes the stale 'Admin gate: every method calls auth.IsAdmin first' comment on IntermediateCAHandler — the gate moved to router.go::rbacGate via auth.RequirePermission middleware in Phase 3.5; the new comment block points readers there. M4: 11 RBAC routes (auth/me, auth/permissions, 5 role lifecycle, 2 role-permission grant/revoke, 2 actor-role grant/revoke) added to api/openapi.yaml under the [Auth] tag with operationIds and shared AuthRole / AuthRolePermission schemas. AuthCheck path extended with the Bundle-1 enrichment fields. The 11 entries removed from openapi_parity_test.go::SpecParityExceptions. Tests: go vet + staticcheck + go test -short -count=1 green across cmd/server/, internal/auth/, internal/api/router/, and internal/api/handler/. New tests: 4 backfill unit tests, 2 AuthCheck M1 enrichment tests, 1 demo-mode + rbacGate chain integration test (TestRBACGate_DemoModeChainReachesHandler). Branch SECURITY.md (cowork/auth-bundle-1-SECURITY.md, not part of this commit) captures the full posture of dev/auth-bundle-1 as of this closure for the operator's pre-merge review. --- api/openapi.yaml | 390 +++++++++++++++++- cmd/server/auth_backfill.go | 58 +++ cmd/server/auth_backfill_test.go | 116 ++++++ cmd/server/main.go | 68 ++- internal/api/handler/health.go | 94 ++++- internal/api/handler/health_test.go | 117 ++++++ internal/api/handler/intermediate_ca.go | 15 +- internal/api/router/openapi_parity_test.go | 24 +- .../api/router/rbac_gate_integration_test.go | 34 ++ 9 files changed, 889 insertions(+), 27 deletions(-) create mode 100644 cmd/server/auth_backfill.go create mode 100644 cmd/server/auth_backfill_test.go diff --git a/api/openapi.yaml b/api/openapi.yaml index 043ab66..538b262 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -147,7 +147,16 @@ paths: get: tags: [Health] summary: Validate credentials - description: Returns 200 if auth credentials are valid, 401 otherwise. + description: | + Returns 200 if auth credentials are valid, 401 otherwise. + + Bundle 1 Phase 3 closure (M1): when the server has the RBAC + primitive wired (Bundle 1 default), the response also includes + the caller's `actor_id`, `actor_type`, `tenant_id`, the + `roles` they hold, and `effective_permissions` they resolve + to. The legacy `admin` boolean is preserved for back-compat + with pre-Bundle-1 GUIs; new GUIs should switch to + `effective_permissions` for affordance gating. operationId: checkAuth responses: "200": @@ -156,13 +165,353 @@ paths: application/json: schema: type: object + required: [status] properties: status: type: string example: authenticated + user: + type: string + description: Named-key identity (empty when CERTCTL_AUTH_TYPE=none) + admin: + type: boolean + description: Legacy admin flag (back-compat with pre-Bundle-1 GUIs). + actor_id: + type: string + description: Actor identifier for the authenticated request (Bundle 1+). + actor_type: + type: string + enum: [User, System, Agent, APIKey, Anonymous] + description: Actor-type discriminator (Bundle 1+). + tenant_id: + type: string + description: Tenant the actor belongs to (Bundle 1 ships single-tenant `t-default`). + admin_via_role: + type: boolean + description: True when the actor holds `r-admin`. Authoritative admin signal under Bundle 1+. + roles: + type: array + items: + type: string + description: Role IDs (e.g. `r-admin`, `r-viewer`) the actor holds. + effective_permissions: + type: array + items: + type: object + required: [permission, scope_type] + properties: + permission: + type: string + example: cert.bulk_revoke + scope_type: + type: string + enum: [global, profile, issuer] + scope_id: + type: string "401": description: Unauthorized + # ─── Auth / RBAC (Bundle 1 Phase 4) ───────────────────────────────── + # The RBAC primitive surface for managing roles, permissions, and the + # role grants assigned to actors (API keys today; OIDC-federated users + # in Bundle 2). Every mutating route runs through the service layer's + # privilege-escalation guard — callers need `auth.role.assign` for + # role grants on actors, `auth.role.create/edit/delete` for the role + # lifecycle, `auth.key.*` for key management. Read endpoints require + # `auth.role.list`. The /v1/auth/me endpoint has no permission gate + # (every authenticated caller can read their own permissions). + /api/v1/auth/me: + get: + tags: [Auth] + summary: Current actor's roles + effective permissions + description: | + Returns the standing roles + effective permission set for the + authenticated caller. This is the query the GUI uses to gate + affordance rendering; /api/v1/auth/check returns the same shape + on the boot path. + operationId: getAuthMe + responses: + "200": + description: Caller identity + roles + effective permissions + content: + application/json: + schema: + type: object + required: [actor_id, actor_type, tenant_id, admin, roles, effective_permissions] + properties: + actor_id: { type: string } + actor_type: { type: string, enum: [User, System, Agent, APIKey, Anonymous] } + tenant_id: { type: string } + admin: { type: boolean } + roles: + type: array + items: { type: string } + effective_permissions: + type: array + items: + type: object + required: [permission, scope_type] + properties: + permission: { type: string } + scope_type: { type: string, enum: [global, profile, issuer] } + scope_id: { type: string } + "401": + description: Unauthorized + + /api/v1/auth/permissions: + get: + tags: [Auth] + summary: List canonical permission catalogue + description: | + Returns every permission name registered in the canonical + catalogue. Used by the GUI's role editor to populate the + "grant permission" picker. Permission: `auth.role.list`. + operationId: listAuthPermissions + responses: + "200": + description: Permission catalogue + content: + application/json: + schema: + type: object + properties: + permissions: + type: array + items: + type: object + required: [id, name, namespace] + properties: + id: { type: string } + name: { type: string } + namespace: { type: string } + "401": { description: Unauthorized } + "403": { description: Forbidden } + + /api/v1/auth/roles: + get: + tags: [Auth] + summary: List roles for the active tenant + description: Permission `auth.role.list`. Returns every role registered for `t-default` (Bundle 1 single-tenant). + operationId: listAuthRoles + responses: + "200": + description: Role list + content: + application/json: + schema: + type: object + properties: + roles: + type: array + items: { $ref: "#/components/schemas/AuthRole" } + "401": { description: Unauthorized } + "403": { description: Forbidden } + post: + tags: [Auth] + summary: Create a custom role + description: Permission `auth.role.create`. Default roles (`r-admin` / `r-operator` / `r-viewer` / `r-agent` / `r-mcp` / `r-cli` / `r-auditor`) are seeded by migration and immutable. + operationId: createAuthRole + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [name] + properties: + name: { type: string } + description: { type: string } + responses: + "201": + description: Role created + content: + application/json: + schema: { $ref: "#/components/schemas/AuthRole" } + "400": { description: Validation error } + "401": { description: Unauthorized } + "403": { description: Forbidden } + "409": { description: Role with that name already exists } + + /api/v1/auth/roles/{id}: + get: + tags: [Auth] + summary: Get a role and its permissions + description: Permission `auth.role.list`. + operationId: getAuthRole + parameters: + - in: path + name: id + required: true + schema: { type: string } + responses: + "200": + description: Role + permissions + content: + application/json: + schema: + type: object + properties: + role: { $ref: "#/components/schemas/AuthRole" } + permissions: + type: array + items: { $ref: "#/components/schemas/AuthRolePermission" } + "401": { description: Unauthorized } + "403": { description: Forbidden } + "404": { description: Role not found } + put: + tags: [Auth] + summary: Update a custom role's name or description + description: Permission `auth.role.edit`. Default roles cannot be renamed. + operationId: updateAuthRole + parameters: + - in: path + name: id + required: true + schema: { type: string } + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + name: { type: string } + description: { type: string } + responses: + "200": { description: Updated } + "400": { description: Validation error } + "401": { description: Unauthorized } + "403": { description: Forbidden } + "404": { description: Role not found } + "409": { description: Default role cannot be renamed / name collision } + delete: + tags: [Auth] + summary: Delete a custom role + description: Permission `auth.role.delete`. Fails with 409 when actors still hold the role (FK ON DELETE RESTRICT). + operationId: deleteAuthRole + parameters: + - in: path + name: id + required: true + schema: { type: string } + responses: + "204": { description: Deleted } + "401": { description: Unauthorized } + "403": { description: Forbidden } + "404": { description: Role not found } + "409": { description: Role still has active actor assignments } + + /api/v1/auth/roles/{id}/permissions: + post: + tags: [Auth] + summary: Grant a permission to a role at a scope + description: Permission `auth.role.edit`. ScopeType defaults to `global`; per-profile / per-issuer scopes require ScopeID. + operationId: grantAuthRolePermission + parameters: + - in: path + name: id + required: true + schema: { type: string } + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [permission] + properties: + permission: { type: string } + scope_type: + type: string + enum: [global, profile, issuer] + default: global + scope_id: { type: string } + responses: + "204": { description: Granted } + "400": { description: Permission not in canonical catalogue / scope_id missing for non-global scope } + "401": { description: Unauthorized } + "403": { description: Forbidden } + "404": { description: Role not found } + + /api/v1/auth/roles/{id}/permissions/{perm}: + delete: + tags: [Auth] + summary: Revoke a permission from a role + description: Permission `auth.role.edit`. + operationId: revokeAuthRolePermission + parameters: + - in: path + name: id + required: true + schema: { type: string } + - in: path + name: perm + required: true + schema: { type: string } + - in: query + name: scope_type + schema: + type: string + enum: [global, profile, issuer] + - in: query + name: scope_id + schema: { type: string } + responses: + "204": { description: Revoked } + "401": { description: Unauthorized } + "403": { description: Forbidden } + "404": { description: Role or permission grant not found } + + /api/v1/auth/keys/{id}/roles: + post: + tags: [Auth] + summary: Assign a role to an API key + description: Permission `auth.role.assign`. The reserved `actor-demo-anon` actor cannot be re-assigned. + operationId: assignAuthKeyRole + parameters: + - in: path + name: id + required: true + schema: { type: string } + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [role_id] + properties: + role_id: { type: string } + responses: + "204": { description: Assigned } + "400": { description: Validation error } + "401": { description: Unauthorized } + "403": { description: Forbidden } + "404": { description: Role not found } + "409": { description: Reserved system actor cannot be modified } + + /api/v1/auth/keys/{id}/roles/{role_id}: + delete: + tags: [Auth] + summary: Revoke a role from an API key + description: Permission `auth.role.assign`. Revoking the synthetic `actor-demo-anon` admin grant is rejected. + operationId: revokeAuthKeyRole + parameters: + - in: path + name: id + required: true + schema: { type: string } + - in: path + name: role_id + required: true + schema: { type: string } + responses: + "204": { description: Revoked } + "401": { description: Unauthorized } + "403": { description: Forbidden } + "404": { description: Role not assigned to actor } + "409": { description: Reserved system actor cannot be modified } + /api/v1/version: get: tags: [Health] @@ -4361,6 +4710,45 @@ components: $ref: "#/components/schemas/ErrorResponse" schemas: + # ─── Auth / RBAC (Bundle 1 Phase 4) ───────────────────────────── + AuthRole: + type: object + required: [id, tenant_id, name] + properties: + id: + type: string + description: Role ID (`r-` prefix). + example: r-admin + tenant_id: + type: string + example: t-default + name: + type: string + example: admin + description: + type: string + created_at: + type: string + format: date-time + updated_at: + type: string + format: date-time + + AuthRolePermission: + type: object + required: [role_id, permission_id, scope_type] + properties: + role_id: + type: string + permission_id: + type: string + scope_type: + type: string + enum: [global, profile, issuer] + scope_id: + type: string + description: NULL/absent for global scope; profile/issuer ID otherwise. + # ─── Approvals ─────────────────────────────────────────────────── ApprovalRequest: type: object diff --git a/cmd/server/auth_backfill.go b/cmd/server/auth_backfill.go new file mode 100644 index 0000000..a59349c --- /dev/null +++ b/cmd/server/auth_backfill.go @@ -0,0 +1,58 @@ +package main + +import ( + "context" + "log/slog" + + "github.com/certctl-io/certctl/internal/auth" + "github.com/certctl-io/certctl/internal/domain" + authdomain "github.com/certctl-io/certctl/internal/domain/auth" +) + +// actorRoleGranter is the narrow interface backfillNamedKeyActorRoles +// needs from the postgres ActorRoleRepository. Pulled out so the unit +// test can inject a fake without spinning up the full repo / DB. +type actorRoleGranter interface { + Grant(ctx context.Context, ar *authdomain.ActorRole) error +} + +// backfillNamedKeyActorRoles is the Bundle 1 Phase 3 closure (C2) +// startup hook that ensures every CERTCTL_API_KEYS_NAMED entry — and +// every legacy CERTCTL_AUTH_SECRET synthesized fallback — has an +// actor_roles row before the HTTP server accepts requests. Admin-flagged +// keys grant `r-admin` (full canonical permission set); non-admin keys +// grant `r-viewer` (read-only surface), matching the pre-Phase-3.5 +// capability shape. +// +// Idempotent via ON CONFLICT DO NOTHING in the repo Grant — reboots +// don't create duplicates. Failures are logged but non-fatal: the server +// still starts, and the operator can fix the grant via the RBAC API. +// +// The function is package-private + extracted from main() so the unit +// test in auth_backfill_test.go can pin the role-mapping invariant +// without depending on the full server bootstrap path. +func backfillNamedKeyActorRoles( + ctx context.Context, + repo actorRoleGranter, + keys []auth.NamedAPIKey, + logger *slog.Logger, +) { + for _, nk := range keys { + role := authdomain.RoleIDViewer + if nk.Admin { + role = authdomain.RoleIDAdmin + } + if err := repo.Grant(ctx, &authdomain.ActorRole{ + ActorID: nk.Name, + ActorType: authdomain.ActorTypeValue(domain.ActorTypeAPIKey), + RoleID: role, + TenantID: authdomain.DefaultTenantID, + GrantedBy: "bootstrap", + }); err != nil { + if logger != nil { + logger.Warn("api-key actor-role backfill failed; key authenticates but RBAC routes will 403 until grant is added via /v1/auth/keys", + "key", nk.Name, "role", role, "err", err) + } + } + } +} diff --git a/cmd/server/auth_backfill_test.go b/cmd/server/auth_backfill_test.go new file mode 100644 index 0000000..1367b52 --- /dev/null +++ b/cmd/server/auth_backfill_test.go @@ -0,0 +1,116 @@ +package main + +import ( + "context" + "errors" + "io" + "log/slog" + "testing" + + "github.com/certctl-io/certctl/internal/auth" + authdomain "github.com/certctl-io/certctl/internal/domain/auth" +) + +// fakeGranter is a tiny in-memory stand-in for the postgres ActorRoleRepository +// — enough surface area for backfillNamedKeyActorRoles to call Grant against. +type fakeGranter struct { + calls []*authdomain.ActorRole + err error +} + +func (f *fakeGranter) Grant(_ context.Context, ar *authdomain.ActorRole) error { + f.calls = append(f.calls, ar) + return f.err +} + +// TestBackfillNamedKeyActorRoles_RoleMapping pins the Bundle 1 Phase 3 +// closure (C2) invariant: admin-flagged named keys grant r-admin, +// non-admin keys grant r-viewer, both at TenantID t-default with +// ActorType APIKey and GrantedBy=bootstrap. +func TestBackfillNamedKeyActorRoles_RoleMapping(t *testing.T) { + repo := &fakeGranter{} + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + keys := []auth.NamedAPIKey{ + {Name: "alice-admin", Key: "AAA", Admin: true}, + {Name: "bob-viewer", Key: "BBB", Admin: false}, + {Name: "carol-admin", Key: "CCC", Admin: true}, + } + backfillNamedKeyActorRoles(context.Background(), repo, keys, logger) + + if len(repo.calls) != 3 { + t.Fatalf("Grant call count = %d, want 3", len(repo.calls)) + } + type want struct { + actor, role string + } + wants := []want{ + {actor: "alice-admin", role: authdomain.RoleIDAdmin}, + {actor: "bob-viewer", role: authdomain.RoleIDViewer}, + {actor: "carol-admin", role: authdomain.RoleIDAdmin}, + } + for i, w := range wants { + got := repo.calls[i] + if got.ActorID != w.actor { + t.Errorf("call[%d].ActorID = %q, want %q", i, got.ActorID, w.actor) + } + if got.RoleID != w.role { + t.Errorf("call[%d].RoleID = %q, want %q", i, got.RoleID, w.role) + } + if got.TenantID != authdomain.DefaultTenantID { + t.Errorf("call[%d].TenantID = %q, want %q", i, got.TenantID, authdomain.DefaultTenantID) + } + if string(got.ActorType) != "APIKey" { + t.Errorf("call[%d].ActorType = %q, want APIKey", i, got.ActorType) + } + if got.GrantedBy != "bootstrap" { + t.Errorf("call[%d].GrantedBy = %q, want bootstrap", i, got.GrantedBy) + } + } +} + +// TestBackfillNamedKeyActorRoles_EmptyKeysIsNoOp confirms the boot path +// is safe when no named keys are configured (typical CERTCTL_AUTH_TYPE= +// none deploy). No Grant calls; no panic. +func TestBackfillNamedKeyActorRoles_EmptyKeysIsNoOp(t *testing.T) { + repo := &fakeGranter{} + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + backfillNamedKeyActorRoles(context.Background(), repo, nil, logger) + if len(repo.calls) != 0 { + t.Errorf("Grant called %d times for empty keys, want 0", len(repo.calls)) + } +} + +// TestBackfillNamedKeyActorRoles_GrantErrorIsNonFatal confirms the +// closure invariant that a Grant failure logs a warning and proceeds +// rather than crashing the server during boot. Subsequent keys still +// get processed. +func TestBackfillNamedKeyActorRoles_GrantErrorIsNonFatal(t *testing.T) { + repo := &fakeGranter{err: errors.New("simulated DB error")} + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + + keys := []auth.NamedAPIKey{ + {Name: "alice", Key: "A", Admin: true}, + {Name: "bob", Key: "B", Admin: false}, + } + // Should not panic. + backfillNamedKeyActorRoles(context.Background(), repo, keys, logger) + + if len(repo.calls) != 2 { + t.Errorf("Grant calls = %d, want 2 (every key processed even when prior Grant errored)", len(repo.calls)) + } +} + +// TestBackfillNamedKeyActorRoles_NilLoggerIsSafe pins that callers +// passing nil for the logger don't NPE the goroutine. Belt-and-braces +// for tests + future call sites that may not have a logger plumbed. +func TestBackfillNamedKeyActorRoles_NilLoggerIsSafe(t *testing.T) { + repo := &fakeGranter{err: errors.New("simulated")} + keys := []auth.NamedAPIKey{ + {Name: "alice", Key: "A", Admin: true}, + } + backfillNamedKeyActorRoles(context.Background(), repo, keys, nil) + if len(repo.calls) != 1 { + t.Errorf("Grant calls = %d, want 1", len(repo.calls)) + } +} diff --git a/cmd/server/main.go b/cmd/server/main.go index f2d65eb..5e2ecbc 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -35,6 +35,7 @@ import ( "github.com/certctl-io/certctl/internal/domain" authdomainAlias "github.com/certctl-io/certctl/internal/domain/auth" "github.com/certctl-io/certctl/internal/ratelimit" + "github.com/certctl-io/certctl/internal/repository" "github.com/certctl-io/certctl/internal/repository/postgres" "github.com/certctl-io/certctl/internal/scep/intune" "github.com/certctl-io/certctl/internal/scheduler" @@ -678,6 +679,12 @@ func main() { // Bundle-5 / H-006: pass the *sql.DB pool so /ready can probe DB // connectivity via PingContext. /health stays shallow (liveness signal). healthHandler := handler.NewHealthHandler(cfg.Auth.Type, db) + // Bundle 1 Phase 3 closure (M1): wire the AuthCheckResolver so + // /v1/auth/check returns the caller's standing roles + effective + // permissions in the same response. The shim is tiny — just a type- + // erasure wrap around the repo so the handler layer doesn't have to + // import internal/domain/auth or internal/repository/postgres. + healthHandler.Resolver = authCheckResolverAdapter{repo: authActorRoleRepo} // U-3 ride-along (cat-u-no_version_endpoint, P2): the version handler // answers GET /api/v1/version with build identity (ldflags Version, // VCS commit/dirty/timestamp, Go runtime version). Wired through the @@ -1558,7 +1565,33 @@ func main() { } } } - authMiddleware := auth.NewAuthWithNamedKeys(namedKeys) + // Bundle 1 Phase 3 closure (C2): backfill actor_roles rows for every + // CERTCTL_API_KEYS_NAMED entry (and the legacy CERTCTL_AUTH_SECRET + // synthesized fallbacks) so RBAC checks have a row to match against. + // Without this, named keys would land on a Phase-3 actor context + // that authorizes every request through the legacy in-handler + // auth.IsAdmin path but fails every Phase-3.5 rbacGate (no + // actor_roles row → empty EffectivePermissions → 403). The helper + // lives in cmd/server/auth_backfill.go so the role-mapping invariant + // is pinned by a focused unit test without dragging in the full + // server bootstrap path. + backfillNamedKeyActorRoles(ctx, authActorRoleRepo, namedKeys, logger) + // Bundle 1 Phase 3 closure (C1): when CERTCTL_AUTH_TYPE=none the + // legacy NewAuthWithNamedKeys returns a no-op pass-through, which + // would leave ActorIDKey / ActorTypeKey / TenantIDKey unpopulated + // in context. Phase 3.5's rbacGate + Phase 4's RBAC handlers all + // require an actor in context (or they 401), so demo mode would be + // completely broken. NewDemoModeAuth injects the synthetic + // `actor-demo-anon` actor seeded by migration 000029, which holds + // the admin role at global scope; the demo + 5 examples in + // examples/*/docker-compose.yml continue to work end-to-end. + var authMiddleware func(http.Handler) http.Handler + switch config.AuthType(cfg.Auth.Type) { + case config.AuthTypeNone: + authMiddleware = auth.NewDemoModeAuth() + default: + authMiddleware = auth.NewAuthWithNamedKeys(namedKeys) + } corsMiddleware := middleware.NewCORS(middleware.CORSConfig{ AllowedOrigins: cfg.CORS.AllowedOrigins, }) @@ -2301,3 +2334,36 @@ func (ad authPermissionCheckerAdapter) CheckPermission( scopeID, ) } + +// authCheckResolverAdapter bridges the postgres ActorRoleRepository +// (authdomain.ActorTypeValue) to handler.AuthCheckResolver +// (domain.ActorType). Lives in cmd/server so the handler layer keeps its +// existing import set; the GUI's /v1/auth/check probe round-trips +// through this on every page load. Read-only — no caller / no audit row. +// +// Bundle 1 Phase 3 closure (M1): the equivalent surface area on +// /v1/auth/me runs through the service layer's auth.role.list permission +// gate, which the GUI may not yet hold during initial render. AuthCheck +// has no permission gate (its only requirement is "the request +// authenticated"), so the bypass is by design. +type authCheckResolverAdapter struct { + repo *postgres.ActorRoleRepository +} + +func (ad authCheckResolverAdapter) ListRoles( + ctx context.Context, + actorID string, + actorType domain.ActorType, + tenantID string, +) ([]*authdomainAlias.ActorRole, error) { + return ad.repo.ListByActor(ctx, actorID, authdomainAlias.ActorTypeValue(actorType), tenantID) +} + +func (ad authCheckResolverAdapter) EffectivePermissions( + ctx context.Context, + actorID string, + actorType domain.ActorType, + tenantID string, +) ([]repository.EffectivePermission, error) { + return ad.repo.EffectivePermissions(ctx, actorID, authdomainAlias.ActorTypeValue(actorType), tenantID) +} diff --git a/internal/api/handler/health.go b/internal/api/handler/health.go index a7950b7..6b27b35 100644 --- a/internal/api/handler/health.go +++ b/internal/api/handler/health.go @@ -7,8 +7,33 @@ import ( "time" "github.com/certctl-io/certctl/internal/auth" + "github.com/certctl-io/certctl/internal/domain" + authdomain "github.com/certctl-io/certctl/internal/domain/auth" + "github.com/certctl-io/certctl/internal/repository" ) +// AuthCheckResolver is the optional dependency HealthHandler uses to enrich +// the /v1/auth/check response with the caller's standing roles and +// effective permission set. The auth handler's /v1/auth/me endpoint +// returns the same shape; we duplicate it here so the GUI can render the +// auth gate from a single round-trip on app boot. main.go wires this +// from the same authsvc.ActorRoleService used by AuthHandler; tests pass +// nil and AuthCheck degrades to the legacy minimal payload. +// +// Bundle 1 Phase 3 closure (M1): pre-closure, /v1/auth/check returned +// only {status, user, admin}. The GUI had to second-fetch /v1/auth/me to +// know which buttons to render — and Me is gated by the rbacGate on +// auth.role.list which the GUI's pre-render path may not yet hold (chicken- +// and-egg with the role-list affordance). Folding the same payload into +// AuthCheck keeps the GUI's boot path single-shot. +type AuthCheckResolver interface { + // ListRoles returns the actor's standing role grants. + ListRoles(ctx context.Context, actorID string, actorType domain.ActorType, tenantID string) ([]*authdomain.ActorRole, error) + // EffectivePermissions returns the deduplicated (perm, scope) triples + // the actor holds across all of its roles. + EffectivePermissions(ctx context.Context, actorID string, actorType domain.ActorType, tenantID string) ([]repository.EffectivePermission, error) +} + // HealthHandler handles health and readiness check endpoints. // // Bundle-5 / Audit H-006 / CWE-754 (Improper Check for Unusual or @@ -45,6 +70,13 @@ type HealthHandler struct { // ReadyProbeTimeout is the per-probe ceiling for the DB ping. Defaults // to 2s when zero. Exposed so tests can shorten it. ReadyProbeTimeout time.Duration + + // AuthCheck (M1) — optional. When set, AuthCheck includes the caller's + // standing roles + effective permissions in the response so the GUI + // can gate affordances from a single fetch. Nil resolver degrades to + // the legacy {status, user, admin} payload (preserves test fixtures + // and the no-db deploy path). + Resolver AuthCheckResolver } // NewHealthHandler creates a new HealthHandler. @@ -53,6 +85,10 @@ type HealthHandler struct { // Ready returns 200 with {"db":"not_configured"} — preserves backwards // compatibility for the call sites that haven't wired the dependency yet. // Production main.go always passes a non-nil pool. +// +// Bundle 1 Phase 3 closure (M1): the resolver is wired separately via +// HealthHandler.Resolver after construction so existing call sites +// (legacy tests, no-db deploys) keep compiling without churn. func NewHealthHandler(authType string, db *sql.DB) HealthHandler { return HealthHandler{ AuthType: authType, @@ -145,15 +181,69 @@ func (h HealthHandler) AuthInfo(w http.ResponseWriter, r *http.Request) { // that would otherwise 403 at the server. This is a hint for UX only — // authorization remains enforced at the handler layer (bulk_revocation.go). // +// Bundle 1 Phase 3 closure (M1): when HealthHandler.Resolver is wired, +// the response is enriched with the caller's standing roles and effective +// permissions. This mirrors the /v1/auth/me payload but lives on /auth/check +// so the GUI can gate affordance rendering with a single fetch on app +// boot. Resolver lookups are best-effort: failures fall back to the +// legacy minimal payload rather than 500-ing the GUI's auth probe. +// // The auth middleware runs before this handler, so reaching here means auth // passed. `user` falls back to an empty string when auth is disabled // (CERTCTL_AUTH_TYPE=none). // GET /api/v1/auth/check func (h HealthHandler) AuthCheck(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() response := map[string]interface{}{ "status": "authenticated", - "user": auth.GetUser(r.Context()), - "admin": auth.IsAdmin(r.Context()), + "user": auth.GetUser(ctx), + "admin": auth.IsAdmin(ctx), } + + if h.Resolver != nil { + actorID, _ := ctx.Value(auth.ActorIDKey{}).(string) + actorType, _ := ctx.Value(auth.ActorTypeKey{}).(string) + tenantID, _ := ctx.Value(auth.TenantIDKey{}).(string) + if tenantID == "" { + tenantID = authdomain.DefaultTenantID + } + if actorID != "" && actorType != "" { + at := domain.ActorType(actorType) + roles, rerr := h.Resolver.ListRoles(ctx, actorID, at, tenantID) + perms, perr := h.Resolver.EffectivePermissions(ctx, actorID, at, tenantID) + if rerr == nil && perr == nil { + roleIDs := make([]string, 0, len(roles)) + hasAdmin := false + for _, role := range roles { + roleIDs = append(roleIDs, role.RoleID) + if role.RoleID == authdomain.RoleIDAdmin { + hasAdmin = true + } + } + permPayload := make([]map[string]interface{}, 0, len(perms)) + for _, p := range perms { + entry := map[string]interface{}{ + "permission": p.PermissionName, + "scope_type": string(p.ScopeType), + } + if p.ScopeID != nil { + entry["scope_id"] = *p.ScopeID + } + permPayload = append(permPayload, entry) + } + response["actor_id"] = actorID + response["actor_type"] = actorType + response["tenant_id"] = tenantID + response["roles"] = roleIDs + response["effective_permissions"] = permPayload + // Authoritative admin signal: the standing-roles list. The + // legacy `admin` boolean above is preserved for back-compat + // (in-handler IsAdmin for non-rbacGate routes), but the + // rbacGate-gated routes now key off effective_permissions. + response["admin_via_role"] = hasAdmin + } + } + } + JSON(w, http.StatusOK, response) } diff --git a/internal/api/handler/health_test.go b/internal/api/handler/health_test.go index 6b7cd2c..e3e2496 100644 --- a/internal/api/handler/health_test.go +++ b/internal/api/handler/health_test.go @@ -10,6 +10,9 @@ import ( "time" "github.com/certctl-io/certctl/internal/auth" + "github.com/certctl-io/certctl/internal/domain" + authdomain "github.com/certctl-io/certctl/internal/domain/auth" + "github.com/certctl-io/certctl/internal/repository" _ "github.com/lib/pq" // Bundle-5 / H-006: postgres driver for /ready DB-probe regression test ) @@ -338,6 +341,120 @@ func TestAuthCheck_NoAuthContext_DefaultsToEmptyUserAndFalseAdmin(t *testing.T) } } +// fakeAuthCheckResolver is a tiny in-memory stand-in for the postgres +// ActorRoleRepository so the M1 enrichment can be tested without a DB. +type fakeAuthCheckResolver struct { + roles []*authdomain.ActorRole + perms []repository.EffectivePermission + err error +} + +func (f fakeAuthCheckResolver) ListRoles(_ context.Context, _ string, _ domain.ActorType, _ string) ([]*authdomain.ActorRole, error) { + return f.roles, f.err +} +func (f fakeAuthCheckResolver) EffectivePermissions(_ context.Context, _ string, _ domain.ActorType, _ string) ([]repository.EffectivePermission, error) { + return f.perms, f.err +} + +// TestAuthCheck_M1_ResolverEnrichesResponseWithRolesAndPerms is the +// Bundle 1 Phase 3 closure (M1) regression: when HealthHandler.Resolver +// is wired, the response includes actor_id / actor_type / tenant_id / +// roles / effective_permissions / admin_via_role. The legacy `admin` +// boolean is preserved for back-compat with pre-Bundle-1 GUIs. +func TestAuthCheck_M1_ResolverEnrichesResponseWithRolesAndPerms(t *testing.T) { + handler := NewHealthHandler("api-key", nil) + scopeID := "profile-prod" + handler.Resolver = fakeAuthCheckResolver{ + roles: []*authdomain.ActorRole{ + {ActorID: "alice", RoleID: authdomain.RoleIDAdmin, TenantID: authdomain.DefaultTenantID}, + {ActorID: "alice", RoleID: authdomain.RoleIDOperator, TenantID: authdomain.DefaultTenantID}, + }, + perms: []repository.EffectivePermission{ + {PermissionName: "cert.bulk_revoke", ScopeType: authdomain.ScopeTypeGlobal}, + {PermissionName: "cert.issue", ScopeType: authdomain.ScopeTypeProfile, ScopeID: &scopeID}, + }, + } + + ctx := context.Background() + ctx = context.WithValue(ctx, auth.ActorIDKey{}, "alice") + ctx = context.WithValue(ctx, auth.ActorTypeKey{}, "APIKey") + ctx = context.WithValue(ctx, auth.TenantIDKey{}, "t-default") + ctx = context.WithValue(ctx, auth.UserKey{}, "alice") + ctx = context.WithValue(ctx, auth.AdminKey{}, true) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/check", nil).WithContext(ctx) + w := httptest.NewRecorder() + handler.AuthCheck(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected status 200, got %d", w.Code) + } + var result map[string]any + if err := json.NewDecoder(w.Body).Decode(&result); err != nil { + t.Fatalf("decode: %v", err) + } + + if result["actor_id"] != "alice" { + t.Errorf("actor_id = %v, want alice", result["actor_id"]) + } + if result["actor_type"] != "APIKey" { + t.Errorf("actor_type = %v, want APIKey", result["actor_type"]) + } + if result["tenant_id"] != "t-default" { + t.Errorf("tenant_id = %v, want t-default", result["tenant_id"]) + } + if result["admin_via_role"] != true { + t.Errorf("admin_via_role = %v, want true (alice holds r-admin)", result["admin_via_role"]) + } + roles, ok := result["roles"].([]any) + if !ok || len(roles) != 2 { + t.Fatalf("roles = %v, want 2-element slice", result["roles"]) + } + perms, ok := result["effective_permissions"].([]any) + if !ok || len(perms) != 2 { + t.Fatalf("effective_permissions = %v, want 2-element slice", result["effective_permissions"]) + } + first := perms[0].(map[string]any) + if first["permission"] != "cert.bulk_revoke" || first["scope_type"] != "global" { + t.Errorf("perm[0] = %v, want cert.bulk_revoke/global", first) + } + second := perms[1].(map[string]any) + if second["permission"] != "cert.issue" || second["scope_type"] != "profile" || second["scope_id"] != "profile-prod" { + t.Errorf("perm[1] = %v, want cert.issue/profile/profile-prod", second) + } +} + +// TestAuthCheck_M1_NilResolverPreservesLegacyShape pins backwards +// compatibility: when no resolver is wired, the response keeps the +// original {status, user, admin} contract that pre-Bundle-1 GUIs key +// off. New keys (actor_id, roles, ...) must be absent. +func TestAuthCheck_M1_NilResolverPreservesLegacyShape(t *testing.T) { + handler := NewHealthHandler("api-key", nil) // Resolver left nil + + ctx := context.Background() + ctx = context.WithValue(ctx, auth.ActorIDKey{}, "alice") + ctx = context.WithValue(ctx, auth.ActorTypeKey{}, "APIKey") + ctx = context.WithValue(ctx, auth.UserKey{}, "alice") + ctx = context.WithValue(ctx, auth.AdminKey{}, true) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/auth/check", nil).WithContext(ctx) + w := httptest.NewRecorder() + handler.AuthCheck(w, req) + + var result map[string]any + if err := json.NewDecoder(w.Body).Decode(&result); err != nil { + t.Fatalf("decode: %v", err) + } + for _, k := range []string{"actor_id", "actor_type", "tenant_id", "roles", "effective_permissions", "admin_via_role"} { + if _, present := result[k]; present { + t.Errorf("%s should be absent in legacy (nil resolver) response, got %v", k, result[k]) + } + } + if result["admin"] != true || result["user"] != "alice" { + t.Errorf("legacy fields not preserved: admin=%v user=%v", result["admin"], result["user"]) + } +} + // --- Bundle-5 / H-006: /ready DB-probe regression coverage --- // TestReady_DBPingSuccess_Returns200WithReachable confirms that when the diff --git a/internal/api/handler/intermediate_ca.go b/internal/api/handler/intermediate_ca.go index ce9be60..a1f3a7e 100644 --- a/internal/api/handler/intermediate_ca.go +++ b/internal/api/handler/intermediate_ca.go @@ -37,12 +37,15 @@ type IntermediateCAServicer interface { // All routes are pinned at /api/v1/issuers/{id}/intermediates and // /api/v1/intermediates/{id}. // -// Admin gate: every method calls auth.IsAdmin first and surfaces -// HTTP 403 for non-admin Bearer callers (M-003 admin-gating pattern, -// matches AdminCRLCacheHandler / AdminESTHandler / AdminSCEPIntuneHandler). -// CA hierarchy management is a high-blast-radius surface — adding a -// child CA mints a new sub-CA cert that becomes a trust root for every -// downstream leaf. Operators expect this gated behind admin role. +// Bundle 1 Phase 3.5: the admin gate moved from in-handler auth.IsAdmin +// checks to router-level auth.RequirePermission middleware (rbacGate +// wraps the handler with the ca.hierarchy.manage permission gate before +// the handler body runs — non-admin Bearer callers get 403 from the +// middleware layer instead of from each handler method). CA hierarchy +// management is a high-blast-radius surface — adding a child CA mints a +// new sub-CA cert that becomes a trust root for every downstream leaf. +// The router gate guarantees the only callers reaching this handler +// hold the admin role at global scope. type IntermediateCAHandler struct { svc IntermediateCAServicer } diff --git a/internal/api/router/openapi_parity_test.go b/internal/api/router/openapi_parity_test.go index 91726a0..3ee1af0 100644 --- a/internal/api/router/openapi_parity_test.go +++ b/internal/api/router/openapi_parity_test.go @@ -93,23 +93,13 @@ var SpecParityExceptions = map[string]string{ "POST /acme/revoke-cert": "Phase 4 default-profile shorthand for revoke-cert.", "GET /acme/renewal-info/{cert_id}": "Phase 4 default-profile shorthand for ARI.", - // Bundle 1 / Phase 4 RBAC API: routes registered in this commit; - // OpenAPI schema entries land in a Phase 4 follow-up commit so the - // schema review is its own atomic change. Each route's request / - // response shape is documented in internal/api/handler/auth.go's - // type definitions; the OpenAPI section lift will mirror those. - // Routes: - "GET /api/v1/auth/me": "Bundle 1 Phase 4 RBAC: current actor's effective permissions; OpenAPI follow-up.", - "GET /api/v1/auth/permissions": "Bundle 1 Phase 4 RBAC: canonical permission catalogue; OpenAPI follow-up.", - "GET /api/v1/auth/roles": "Bundle 1 Phase 4 RBAC: list roles; OpenAPI follow-up.", - "POST /api/v1/auth/roles": "Bundle 1 Phase 4 RBAC: create role; OpenAPI follow-up.", - "GET /api/v1/auth/roles/{id}": "Bundle 1 Phase 4 RBAC: get role + permissions; OpenAPI follow-up.", - "PUT /api/v1/auth/roles/{id}": "Bundle 1 Phase 4 RBAC: update role; OpenAPI follow-up.", - "DELETE /api/v1/auth/roles/{id}": "Bundle 1 Phase 4 RBAC: delete role; OpenAPI follow-up.", - "POST /api/v1/auth/roles/{id}/permissions": "Bundle 1 Phase 4 RBAC: grant permission to role; OpenAPI follow-up.", - "DELETE /api/v1/auth/roles/{id}/permissions/{perm}": "Bundle 1 Phase 4 RBAC: revoke permission from role; OpenAPI follow-up.", - "POST /api/v1/auth/keys/{id}/roles": "Bundle 1 Phase 4 RBAC: assign role to API key; OpenAPI follow-up.", - "DELETE /api/v1/auth/keys/{id}/roles/{role_id}": "Bundle 1 Phase 4 RBAC: revoke role from API key; OpenAPI follow-up.", + // Bundle 1 / Phase 4 RBAC API: shipped with full OpenAPI schema in + // the Phase 0-5 closure commit. The 11 routes (auth/me + permissions + // catalogue + 5 role-lifecycle + 2 role-permission grant/revoke + 2 + // actor-role grant/revoke) live in api/openapi.yaml under tag + // `[Auth]`. Shared shapes: AuthRole + AuthRolePermission in the + // schemas section. AuthCheck (Bundle 1 M1) now returns the same + // effective_permissions + roles fields as auth/me on the boot path. } func TestRouter_OpenAPIParity(t *testing.T) { diff --git a/internal/api/router/rbac_gate_integration_test.go b/internal/api/router/rbac_gate_integration_test.go index 4de144a..76bc41a 100644 --- a/internal/api/router/rbac_gate_integration_test.go +++ b/internal/api/router/rbac_gate_integration_test.go @@ -127,3 +127,37 @@ func TestRBACGate_NoActorReturns401(t *testing.T) { t.Errorf("handler body must NOT run when no actor in context") } } + +// TestRBACGate_DemoModeChainReachesHandler is the end-to-end Bundle 1 +// Phase 3 closure (C1) regression: when CERTCTL_AUTH_TYPE=none, the +// auth.NewDemoModeAuth middleware injects the synthetic actor-demo-anon +// actor into context. The rbacGate downstream sees a populated actor + +// the fake checker (standing in for the seeded admin grant on the +// demo actor) and forwards the request. Without the C1 fix, the +// pre-closure NewAuthWithNamedKeys no-op pass-through would have left +// context unpopulated and the rbacGate would 401 every demo request. +func TestRBACGate_DemoModeChainReachesHandler(t *testing.T) { + rh := &reachedHandler{} + // Mirror the seeded admin grant on actor-demo-anon: the checker + // allows every permission for the demo actor (matches the data + // migration seeds in 000029_rbac.up.sql). + checker := &fakeChecker{permFn: func(_ context.Context, actorID, _, _, _, _ string, _ *string) (bool, error) { + if actorID != auth.DemoAnonActorID { + t.Errorf("checker called for unexpected actor %q (want demo-anon)", actorID) + } + return true, nil + }} + gated := rbacGate(checker, "cert.bulk_revoke", rh.ServeHTTP) + chain := auth.NewDemoModeAuth()(gated) + + req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-revoke", nil) + rec := httptest.NewRecorder() + chain.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Errorf("demo-mode caller against admin route should reach handler 200; got %d", rec.Code) + } + if !rh.called { + t.Errorf("handler body must run for demo-mode caller (C1 closure regression)") + } +}