mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 15:11:29 +00:00
auth-bundle-1 Phase 8 follow-up: classify issuer/target audit rows + auditor end-to-end tests + gofmt drift
Self-audit caught five real gaps in 3ef45e2; this commit closes them. # Phase 8 — issuer/target audit rows now classified as 'config' The Phase 8 prompt explicitly required existing config-mutation calls (issuer config, target config, etc.) to write event_category=config. The3ef45e2commit only migrated the auth service callers; the 6 issuer/target call-sites (internal/service/issuer.go: create/update/delete_issuer + internal/service/target.go: create/update/delete_target) still defaulted to cert_lifecycle. They now pass through RecordEventWithCategory(..., domain.EventCategoryConfig, ...) so auditors filtering /v1/audit?category=config see the slice the migration's docstring promised. # Auditor exit-criterion test Phase 8's exit criteria pin 'a user with the auditor role can list / export audit events but gets 403 on every other endpoint.' Bundle 1 unit invariants (auditor permission set, rbacGate behaviour) were in place but no end-to-end test walked the full set of admin perms with an auditor actor. internal/api/router/rbac_gate_integration_test.go gains TestRBACGate_AuditorRole_403sOnAdminRoutes (table-driven across all 5 admin perms — cert.bulk_revoke / crl.admin / scep.admin / est.admin / ca.hierarchy.manage) plus TestRBACGate_AuditorRole_PassesAuditReadGate (positive case for audit.read). # gofmt drift3ef45e2left two cosmetic struct-field-alignment diffs in internal/cli/auth.go and internal/api/handler/audit_handler_test.go that gofmt -l flagged. CI's gofmt step would have failed; gofmt -w applied; gofmt -l now clean across the repo. # CHANGELOG path-prefix CHANGELOG.md v2.1.0 used '/v1/auth/bootstrap' shorthand in the operator-facing flow examples. The actual route is '/api/v1/auth/bootstrap'; an operator copy-pasting the curl would 404. All five hits replaced. Verifications: gofmt clean, go vet ./internal/service/ ./internal/api/router/ clean, go test -short -count=1 green across internal/service + internal/api/router, including the 6 new auditor sub-tests (PASS).
This commit is contained in:
+1
-1
@@ -41,7 +41,7 @@ What else changed in v2.1.0:
|
||||
`auth.RequirePermission` middleware (replaces the legacy
|
||||
`IsAdmin` boolean check on the 5 admin-only handlers).
|
||||
- **Day-0 admin bootstrap.** Set `CERTCTL_BOOTSTRAP_TOKEN` on a fresh
|
||||
deploy and POST a single curl call against `/v1/auth/bootstrap` to
|
||||
deploy and POST a single curl call against `/api/v1/auth/bootstrap` to
|
||||
mint the first admin API key; one-shot, never logged, and locks
|
||||
closed once any admin actor exists. Migration 000031 ships the
|
||||
`api_keys` table that stores the SHA-256 hash; the plaintext is
|
||||
|
||||
@@ -15,9 +15,9 @@ import (
|
||||
|
||||
// mockAuditService implements AuditService for testing.
|
||||
type mockAuditService struct {
|
||||
listFunc func(page, perPage int) ([]domain.AuditEvent, int64, error)
|
||||
listByCatFunc func(category string, page, perPage int) ([]domain.AuditEvent, int64, error)
|
||||
getFunc func(id string) (*domain.AuditEvent, error)
|
||||
listFunc func(page, perPage int) ([]domain.AuditEvent, int64, error)
|
||||
listByCatFunc func(category string, page, perPage int) ([]domain.AuditEvent, int64, error)
|
||||
getFunc func(id string) (*domain.AuditEvent, error)
|
||||
}
|
||||
|
||||
func (m *mockAuditService) ListAuditEvents(_ context.Context, page, perPage int) ([]domain.AuditEvent, int64, error) {
|
||||
|
||||
@@ -128,6 +128,76 @@ func TestRBACGate_NoActorReturns401(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestRBACGate_AuditorRole_403sOnAdminRoutes is the Bundle 1 Phase 8
|
||||
// exit-criterion test: an actor holding only the auditor role
|
||||
// (audit.read + audit.export) gets 403 on every rbacGate-wrapped admin
|
||||
// route. This pins the prompt's "auditor user can list/export audit
|
||||
// events but gets 403 on every other endpoint" requirement.
|
||||
//
|
||||
// We exercise every admin perm name registered in router.go's rbacGate
|
||||
// calls (cert.bulk_revoke / crl.admin / scep.admin / est.admin /
|
||||
// ca.hierarchy.manage). The checker simulates the auditor's permission
|
||||
// matrix — only audit.read + audit.export return true; every admin
|
||||
// permission returns false. The handler MUST NOT be reached for any
|
||||
// admin perm; the wrapper MUST emit 403.
|
||||
func TestRBACGate_AuditorRole_403sOnAdminRoutes(t *testing.T) {
|
||||
auditorPerms := map[string]bool{
|
||||
"audit.read": true,
|
||||
"audit.export": true,
|
||||
}
|
||||
checker := &fakeChecker{permFn: func(_ context.Context, _, _, _, perm, _ string, _ *string) (bool, error) {
|
||||
return auditorPerms[perm], nil
|
||||
}}
|
||||
for _, adminPerm := range []string{
|
||||
"cert.bulk_revoke",
|
||||
"crl.admin",
|
||||
"scep.admin",
|
||||
"est.admin",
|
||||
"ca.hierarchy.manage",
|
||||
} {
|
||||
t.Run(adminPerm, func(t *testing.T) {
|
||||
rh := &reachedHandler{}
|
||||
gated := rbacGate(checker, adminPerm, rh.ServeHTTP)
|
||||
req := withActor(httptest.NewRequest(http.MethodPost, "/api/v1/", nil), "audrey", auth.ActorTypeAPIKey)
|
||||
rec := httptest.NewRecorder()
|
||||
gated.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusForbidden {
|
||||
t.Errorf("auditor on %q route should get 403; got %d", adminPerm, rec.Code)
|
||||
}
|
||||
if rh.called {
|
||||
t.Errorf("handler body must NOT run for auditor on admin route %q", adminPerm)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestRBACGate_AuditorRole_PassesAuditReadGate confirms the positive
|
||||
// half of the auditor invariant: a route gated on audit.read does
|
||||
// reach the handler when the auditor calls it. (Bundle 1 doesn't
|
||||
// currently wrap any audit route via rbacGate at the router level —
|
||||
// /v1/audit relies on auth.role.list at the service layer instead;
|
||||
// this test simulates a future wrap to pin the symmetric path.)
|
||||
func TestRBACGate_AuditorRole_PassesAuditReadGate(t *testing.T) {
|
||||
auditorPerms := map[string]bool{
|
||||
"audit.read": true,
|
||||
"audit.export": true,
|
||||
}
|
||||
checker := &fakeChecker{permFn: func(_ context.Context, _, _, _, perm, _ string, _ *string) (bool, error) {
|
||||
return auditorPerms[perm], nil
|
||||
}}
|
||||
rh := &reachedHandler{}
|
||||
gated := rbacGate(checker, "audit.read", rh.ServeHTTP)
|
||||
req := withActor(httptest.NewRequest(http.MethodGet, "/api/v1/audit", nil), "audrey", auth.ActorTypeAPIKey)
|
||||
rec := httptest.NewRecorder()
|
||||
gated.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Errorf("auditor on audit.read route should reach handler 200; got %d", rec.Code)
|
||||
}
|
||||
if !rh.called {
|
||||
t.Errorf("handler body must run for auditor on audit-read gate")
|
||||
}
|
||||
}
|
||||
|
||||
// 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
|
||||
|
||||
@@ -19,10 +19,10 @@ import (
|
||||
// authMeResponse mirrors handler.meResponse without importing the
|
||||
// handler package (would couple CLI build to the server tree).
|
||||
type authMeResponse struct {
|
||||
ActorID string `json:"actor_id"`
|
||||
ActorType string `json:"actor_type"`
|
||||
TenantID string `json:"tenant_id"`
|
||||
Admin bool `json:"admin"`
|
||||
ActorID string `json:"actor_id"`
|
||||
ActorType string `json:"actor_type"`
|
||||
TenantID string `json:"tenant_id"`
|
||||
Admin bool `json:"admin"`
|
||||
Roles []string `json:"roles"`
|
||||
EffectivePermissions []struct {
|
||||
Permission string `json:"permission"`
|
||||
|
||||
@@ -191,7 +191,7 @@ func (s *IssuerService) Create(ctx context.Context, iss *domain.Issuer, actor st
|
||||
}
|
||||
|
||||
if s.auditService != nil {
|
||||
if auditErr := s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser, "create_issuer", "issuer", iss.ID, nil); auditErr != nil {
|
||||
if auditErr := s.auditService.RecordEventWithCategory(ctx, actor, domain.ActorTypeUser, "create_issuer", domain.EventCategoryConfig, "issuer", iss.ID, nil); auditErr != nil {
|
||||
s.logger.Error("failed to record audit event", "error", auditErr)
|
||||
}
|
||||
}
|
||||
@@ -232,7 +232,7 @@ func (s *IssuerService) Update(ctx context.Context, id string, iss *domain.Issue
|
||||
s.rebuildRegistryQuiet(ctx)
|
||||
|
||||
if s.auditService != nil {
|
||||
if auditErr := s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser, "update_issuer", "issuer", id, nil); auditErr != nil {
|
||||
if auditErr := s.auditService.RecordEventWithCategory(ctx, actor, domain.ActorTypeUser, "update_issuer", domain.EventCategoryConfig, "issuer", id, nil); auditErr != nil {
|
||||
s.logger.Error("failed to record audit event", "error", auditErr)
|
||||
}
|
||||
}
|
||||
@@ -252,7 +252,7 @@ func (s *IssuerService) Delete(ctx context.Context, id string, actor string) err
|
||||
}
|
||||
|
||||
if s.auditService != nil {
|
||||
if auditErr := s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser, "delete_issuer", "issuer", id, nil); auditErr != nil {
|
||||
if auditErr := s.auditService.RecordEventWithCategory(ctx, actor, domain.ActorTypeUser, "delete_issuer", domain.EventCategoryConfig, "issuer", id, nil); auditErr != nil {
|
||||
s.logger.Error("failed to record audit event", "error", auditErr)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -153,7 +153,7 @@ func (s *TargetService) Create(ctx context.Context, target *domain.DeploymentTar
|
||||
}
|
||||
|
||||
if s.auditService != nil {
|
||||
if auditErr := s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser, "create_target", "target", target.ID, nil); auditErr != nil {
|
||||
if auditErr := s.auditService.RecordEventWithCategory(ctx, actor, domain.ActorTypeUser, "create_target", domain.EventCategoryConfig, "target", target.ID, nil); auditErr != nil {
|
||||
s.logger.Error("failed to record audit event", "error", auditErr)
|
||||
}
|
||||
}
|
||||
@@ -191,7 +191,7 @@ func (s *TargetService) Update(ctx context.Context, id string, target *domain.De
|
||||
}
|
||||
|
||||
if s.auditService != nil {
|
||||
if auditErr := s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser, "update_target", "target", id, nil); auditErr != nil {
|
||||
if auditErr := s.auditService.RecordEventWithCategory(ctx, actor, domain.ActorTypeUser, "update_target", domain.EventCategoryConfig, "target", id, nil); auditErr != nil {
|
||||
s.logger.Error("failed to record audit event", "error", auditErr)
|
||||
}
|
||||
}
|
||||
@@ -206,7 +206,7 @@ func (s *TargetService) Delete(ctx context.Context, id string, actor string) err
|
||||
}
|
||||
|
||||
if s.auditService != nil {
|
||||
if auditErr := s.auditService.RecordEvent(ctx, actor, domain.ActorTypeUser, "delete_target", "target", id, nil); auditErr != nil {
|
||||
if auditErr := s.auditService.RecordEventWithCategory(ctx, actor, domain.ActorTypeUser, "delete_target", domain.EventCategoryConfig, "target", id, nil); auditErr != nil {
|
||||
s.logger.Error("failed to record audit event", "error", auditErr)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user