auth-bundle-1 Phase 3.5: handler IsAdmin -> router-wrapped RequirePermission

Phase 3.5 atomic conversion. The five legacy admin-gated handlers (bulk_revocation, admin_crl_cache, admin_scep_intune, admin_est, intermediate_ca) had their in-body auth.IsAdmin checks removed; the gate moved to router.go via auth.RequirePermission middleware wrapping each route. Non-admin operators with the right scoped permission can now reach these endpoints; legacy in-body admin checks no longer block them.

Migration 000030_rbac_admin_perms.up.sql ships five admin-only fine-grained permissions: cert.bulk_revoke, crl.admin, scep.admin, est.admin, ca.hierarchy.manage. All five are seeded into r-admin only; operator/viewer/agent/mcp/cli/auditor do not receive them by default. Operators can grant any of these to a custom role via the Phase 4 RBAC API. Idempotent + transaction-wrapped.

internal/domain/auth/validate.go::CanonicalPermissions extended with the five new entries so RoleService.AddPermission accepts them.

internal/api/router/router.go: HandlerRegistry gains a Checker field (auth.PermissionChecker). New rbacGate(checker, perm, handler) helper wraps a handler with auth.RequirePermission middleware; nil-checker fall-through preserves test/demo deployments without the RBAC stack. 12 admin routes wrapped: cert.bulk_revoke (POST /api/v1/certificates/bulk-revoke + POST /api/v1/est/certificates/bulk-revoke), crl.admin (GET /api/v1/admin/crl/cache), scep.admin (GET /api/v1/admin/scep/profiles + GET /api/v1/admin/scep/intune/stats + POST /api/v1/admin/scep/intune/reload-trust), est.admin (GET /api/v1/admin/est/profiles + POST /api/v1/admin/est/reload-trust), ca.hierarchy.manage (POST /api/v1/issuers/{id}/intermediates + GET /api/v1/issuers/{id}/intermediates + POST /api/v1/intermediates/{id}/retire + GET /api/v1/intermediates/{id}).

cmd/server/main.go: HandlerRegistry.Checker wired with the same authPermissionCheckerAdapter shim Phase 4 introduced for AuthHandler. Same adapter; one source of truth.

Handler bodies: removed eight in-body auth.IsAdmin checks across the 5 files. bulk_revocation.go's BulkRevoke + BulkRevokeEST, admin_crl_cache.go::ListCache, admin_scep_intune.go's three methods, admin_est.go's two methods, intermediate_ca.go's four methods. Replaced each with a comment naming the new gate location. Unused 'github.com/certctl-io/certctl/internal/auth' imports removed.

Test triplet rewrite: deleted obsolete _NonAdmin_Returns403 and _AdminExplicitFalse_Returns403 tests across 6 test files (5 handler tests + bulk_revocation_est_test.go) — they tested the now-removed in-body gate. _AdminPermitted_ForwardsActor tests stay intact: they pin the actor-passthrough invariant which is still relevant. Added internal/api/router/rbac_gate_integration_test.go with four router-level integration tests pinning the new gate: deny → 403 + handler not reached, permit → 200 + handler reached, nil-checker → fall-through, no-actor → 401.

M-008 admin-gate registry: AdminGatedHandlers map now empty (Phase 3.5 invariant: zero in-handler auth.IsAdmin call sites; only health.go's informational caller remains). m008_admin_gate_test.go retains the scan to enforce the invariant going forward; new admin-gated routes must wrap at router.go::rbacGate, not gate in-handler. Updated error message to direct future contributors to the new pattern.

Verifications: gofmt clean across all touched files; go vet ./... clean; go test -short across internal/auth, internal/service/auth, internal/api/handler, internal/api/router, cmd/server all green.

Branch: dev/auth-bundle-1. Commit chain: 99a012e (Phase 0 extract) -> 19497ee (Phase 1 schema + repo) -> bd54d5f (Phase 2 service) -> d473398 (Phase 3 primitive) -> b169f25 (Phase 4 + 5) -> THIS (Phase 3.5 conversion). Phase 6+ (bootstrap, scope-down, auditor, approval-bypass closure, GUI, docs) on subsequent sessions.
This commit is contained in:
shankar0123
2026-05-09 17:00:30 +00:00
parent b169f258de
commit 7ff2e2de08
19 changed files with 290 additions and 485 deletions
@@ -111,81 +111,12 @@ func helperRootCertPEM(t *testing.T) []byte {
// authenticated one — must get HTTP 403 from every endpoint. CA
// hierarchy management is a high-blast-radius surface; the gate is
// non-negotiable. M-008 admin-gate triplet test #1.
func TestIntermediateCA_Handler_NonAdmin_Returns403(t *testing.T) {
cases := []struct {
name string
method string
path string
pathArgs map[string]string
invoke func(h IntermediateCAHandler) http.HandlerFunc
}{
{
name: "Create",
method: http.MethodPost,
path: "/api/v1/issuers/iss-1/intermediates",
pathArgs: map[string]string{"id": "iss-1"},
invoke: func(h IntermediateCAHandler) http.HandlerFunc { return h.Create },
},
{
name: "List",
method: http.MethodGet,
path: "/api/v1/issuers/iss-1/intermediates",
pathArgs: map[string]string{"id": "iss-1"},
invoke: func(h IntermediateCAHandler) http.HandlerFunc { return h.List },
},
{
name: "Get",
method: http.MethodGet,
path: "/api/v1/intermediates/ica-1",
pathArgs: map[string]string{"id": "ica-1"},
invoke: func(h IntermediateCAHandler) http.HandlerFunc { return h.Get },
},
{
name: "Retire",
method: http.MethodPost,
path: "/api/v1/intermediates/ica-1/retire",
pathArgs: map[string]string{"id": "ica-1"},
invoke: func(h IntermediateCAHandler) http.HandlerFunc { return h.Retire },
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
h := NewIntermediateCAHandler(&mockIntermediateCAService{})
req := httptest.NewRequest(tc.method, tc.path, bytes.NewReader([]byte("{}")))
for k, v := range tc.pathArgs {
req.SetPathValue(k, v)
}
// Authenticated user but admin=false.
req = req.WithContext(withAdmin("alice", false))
w := httptest.NewRecorder()
tc.invoke(h)(w, req)
if w.Code != http.StatusForbidden {
t.Fatalf("%s: expected 403 for non-admin, got %d body=%s", tc.name, w.Code, w.Body.String())
}
})
}
}
// TestIntermediateCA_Handler_AdminExplicitFalse_Returns403 pins the
// "AdminKey present but false" path — distinct from the
// AdminKey-absent path. Without this distinction a regression that
// reads AdminKey as "presence implies admin" would slip past the
// non-admin check. M-008 admin-gate triplet test #2.
func TestIntermediateCA_Handler_AdminExplicitFalse_Returns403(t *testing.T) {
h := NewIntermediateCAHandler(&mockIntermediateCAService{})
req := httptest.NewRequest(http.MethodPost, "/api/v1/issuers/iss-1/intermediates",
bytes.NewReader([]byte(`{"name":"r"}`)))
req.SetPathValue("id", "iss-1")
// AdminKey explicitly set to false — distinct from missing key.
ctx := context.WithValue(context.Background(), auth.UserKey{}, "alice")
ctx = context.WithValue(ctx, auth.AdminKey{}, false)
req = req.WithContext(ctx)
w := httptest.NewRecorder()
h.Create(w, req)
if w.Code != http.StatusForbidden {
t.Fatalf("expected 403 for AdminKey=false, got %d", w.Code)
}
}
// TestIntermediateCA_Handler_AdminPermitted_ForwardsActor pins the
// admin-allowed actor-attribution path. An admin caller's actor