mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 21:51:30 +00:00
7ff2e2de08
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.
236 lines
8.2 KiB
Go
236 lines
8.2 KiB
Go
package handler
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"encoding/json"
|
|
"fmt"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
|
|
"github.com/certctl-io/certctl/internal/api/middleware"
|
|
"github.com/certctl-io/certctl/internal/auth"
|
|
"github.com/certctl-io/certctl/internal/domain"
|
|
)
|
|
|
|
// mockBulkRevocationService is a test implementation of BulkRevocationService
|
|
type mockBulkRevocationService struct {
|
|
BulkRevokeFn func(ctx context.Context, criteria domain.BulkRevocationCriteria, reason string, actor string) (*domain.BulkRevocationResult, error)
|
|
}
|
|
|
|
func (m *mockBulkRevocationService) BulkRevoke(ctx context.Context, criteria domain.BulkRevocationCriteria, reason string, actor string) (*domain.BulkRevocationResult, error) {
|
|
if m.BulkRevokeFn != nil {
|
|
return m.BulkRevokeFn(ctx, criteria, reason, actor)
|
|
}
|
|
return &domain.BulkRevocationResult{}, nil
|
|
}
|
|
|
|
// adminContext returns a context carrying the admin flag, mimicking what the
|
|
// auth middleware sets for named-key callers whose entry is admin-tagged.
|
|
// M-003: bulk revocation handler requires admin context to reach the service.
|
|
func adminContext() context.Context {
|
|
ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id-bulk")
|
|
ctx = context.WithValue(ctx, auth.AdminKey{}, true)
|
|
return ctx
|
|
}
|
|
|
|
func TestBulkRevoke_Success_WithIDs(t *testing.T) {
|
|
svc := &mockBulkRevocationService{
|
|
BulkRevokeFn: func(ctx context.Context, criteria domain.BulkRevocationCriteria, reason string, actor string) (*domain.BulkRevocationResult, error) {
|
|
if len(criteria.CertificateIDs) != 2 {
|
|
t.Errorf("expected 2 IDs, got %d", len(criteria.CertificateIDs))
|
|
}
|
|
if reason != "keyCompromise" {
|
|
t.Errorf("expected reason keyCompromise, got %s", reason)
|
|
}
|
|
return &domain.BulkRevocationResult{
|
|
TotalMatched: 2,
|
|
TotalRevoked: 2,
|
|
}, nil
|
|
},
|
|
}
|
|
h := NewBulkRevocationHandler(svc)
|
|
|
|
body := `{"reason":"keyCompromise","certificate_ids":["mc-1","mc-2"]}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-revoke", bytes.NewBufferString(body))
|
|
req.Header.Set("Content-Type", "application/json")
|
|
req = req.WithContext(adminContext())
|
|
w := httptest.NewRecorder()
|
|
|
|
h.BulkRevoke(w, req)
|
|
|
|
if w.Code != http.StatusOK {
|
|
t.Errorf("expected 200, got %d", w.Code)
|
|
}
|
|
|
|
var result domain.BulkRevocationResult
|
|
if err := json.NewDecoder(w.Body).Decode(&result); err != nil {
|
|
t.Fatalf("failed to decode response: %v", err)
|
|
}
|
|
if result.TotalMatched != 2 {
|
|
t.Errorf("expected TotalMatched=2, got %d", result.TotalMatched)
|
|
}
|
|
if result.TotalRevoked != 2 {
|
|
t.Errorf("expected TotalRevoked=2, got %d", result.TotalRevoked)
|
|
}
|
|
}
|
|
|
|
func TestBulkRevoke_Success_WithProfile(t *testing.T) {
|
|
svc := &mockBulkRevocationService{
|
|
BulkRevokeFn: func(ctx context.Context, criteria domain.BulkRevocationCriteria, reason string, actor string) (*domain.BulkRevocationResult, error) {
|
|
if criteria.ProfileID != "prof-tls" {
|
|
t.Errorf("expected profile prof-tls, got %s", criteria.ProfileID)
|
|
}
|
|
return &domain.BulkRevocationResult{
|
|
TotalMatched: 5,
|
|
TotalRevoked: 4,
|
|
TotalSkipped: 1,
|
|
}, nil
|
|
},
|
|
}
|
|
h := NewBulkRevocationHandler(svc)
|
|
|
|
body := `{"reason":"keyCompromise","profile_id":"prof-tls"}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-revoke", bytes.NewBufferString(body))
|
|
req.Header.Set("Content-Type", "application/json")
|
|
req = req.WithContext(adminContext())
|
|
w := httptest.NewRecorder()
|
|
|
|
h.BulkRevoke(w, req)
|
|
|
|
if w.Code != http.StatusOK {
|
|
t.Errorf("expected 200, got %d", w.Code)
|
|
}
|
|
}
|
|
|
|
func TestBulkRevoke_MissingReason_400(t *testing.T) {
|
|
h := NewBulkRevocationHandler(&mockBulkRevocationService{})
|
|
|
|
body := `{"certificate_ids":["mc-1"]}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-revoke", bytes.NewBufferString(body))
|
|
req.Header.Set("Content-Type", "application/json")
|
|
req = req.WithContext(adminContext())
|
|
w := httptest.NewRecorder()
|
|
|
|
h.BulkRevoke(w, req)
|
|
|
|
if w.Code != http.StatusBadRequest {
|
|
t.Errorf("expected 400, got %d", w.Code)
|
|
}
|
|
}
|
|
|
|
func TestBulkRevoke_EmptyCriteria_400(t *testing.T) {
|
|
h := NewBulkRevocationHandler(&mockBulkRevocationService{})
|
|
|
|
body := `{"reason":"keyCompromise"}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-revoke", bytes.NewBufferString(body))
|
|
req.Header.Set("Content-Type", "application/json")
|
|
req = req.WithContext(adminContext())
|
|
w := httptest.NewRecorder()
|
|
|
|
h.BulkRevoke(w, req)
|
|
|
|
if w.Code != http.StatusBadRequest {
|
|
t.Errorf("expected 400, got %d", w.Code)
|
|
}
|
|
}
|
|
|
|
func TestBulkRevoke_InvalidReason_400(t *testing.T) {
|
|
h := NewBulkRevocationHandler(&mockBulkRevocationService{})
|
|
|
|
body := `{"reason":"totallyBogus","certificate_ids":["mc-1"]}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-revoke", bytes.NewBufferString(body))
|
|
req.Header.Set("Content-Type", "application/json")
|
|
req = req.WithContext(adminContext())
|
|
w := httptest.NewRecorder()
|
|
|
|
h.BulkRevoke(w, req)
|
|
|
|
if w.Code != http.StatusBadRequest {
|
|
t.Errorf("expected 400, got %d", w.Code)
|
|
}
|
|
}
|
|
|
|
func TestBulkRevoke_MethodNotAllowed_405(t *testing.T) {
|
|
h := NewBulkRevocationHandler(&mockBulkRevocationService{})
|
|
|
|
// Method check fires before the admin gate, so 405 must hold even for a
|
|
// non-admin caller — asserting this keeps the ordering explicit.
|
|
req := httptest.NewRequest(http.MethodGet, "/api/v1/certificates/bulk-revoke", nil)
|
|
w := httptest.NewRecorder()
|
|
|
|
h.BulkRevoke(w, req)
|
|
|
|
if w.Code != http.StatusMethodNotAllowed {
|
|
t.Errorf("expected 405, got %d", w.Code)
|
|
}
|
|
}
|
|
|
|
func TestBulkRevoke_ServiceError_500(t *testing.T) {
|
|
svc := &mockBulkRevocationService{
|
|
BulkRevokeFn: func(ctx context.Context, criteria domain.BulkRevocationCriteria, reason string, actor string) (*domain.BulkRevocationResult, error) {
|
|
return nil, fmt.Errorf("database connection failed")
|
|
},
|
|
}
|
|
h := NewBulkRevocationHandler(svc)
|
|
|
|
body := `{"reason":"keyCompromise","certificate_ids":["mc-1"]}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-revoke", bytes.NewBufferString(body))
|
|
req.Header.Set("Content-Type", "application/json")
|
|
req = req.WithContext(adminContext())
|
|
w := httptest.NewRecorder()
|
|
|
|
h.BulkRevoke(w, req)
|
|
|
|
if w.Code != http.StatusInternalServerError {
|
|
t.Errorf("expected 500, got %d", w.Code)
|
|
}
|
|
}
|
|
|
|
// --- M-003: admin-only gate on bulk revocation ---
|
|
|
|
// TestBulkRevoke_NonAdmin_Returns403 is the central authorization regression
|
|
// for M-003. A caller without an admin-tagged context must be rejected with
|
|
// HTTP 403, regardless of how well-formed its body is, and the service layer
|
|
// must never see the request.
|
|
|
|
// TestBulkRevoke_AdminExplicitFalse_Returns403 pins the specific case where the
|
|
// AdminKey exists but is set to false — e.g., a non-admin named-key caller.
|
|
// Without this we could regress to "key missing == deny, key present == allow"
|
|
// which would silently grant a false flag.
|
|
|
|
// TestBulkRevoke_AdminPermitted_ForwardsActor confirms the happy path:
|
|
// an admin-tagged context reaches the service and the actor (from the auth
|
|
// UserKey) is propagated through to BulkRevoke. This keeps the admin gate and
|
|
// the M-002 actor-propagation wired together in a single regression.
|
|
func TestBulkRevoke_AdminPermitted_ForwardsActor(t *testing.T) {
|
|
var capturedActor string
|
|
svc := &mockBulkRevocationService{
|
|
BulkRevokeFn: func(ctx context.Context, criteria domain.BulkRevocationCriteria, reason string, actor string) (*domain.BulkRevocationResult, error) {
|
|
capturedActor = actor
|
|
return &domain.BulkRevocationResult{TotalMatched: 1, TotalRevoked: 1}, nil
|
|
},
|
|
}
|
|
h := NewBulkRevocationHandler(svc)
|
|
|
|
body := `{"reason":"keyCompromise","certificate_ids":["mc-1"]}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/certificates/bulk-revoke", bytes.NewBufferString(body))
|
|
req.Header.Set("Content-Type", "application/json")
|
|
|
|
ctx := context.WithValue(context.Background(), middleware.RequestIDKey{}, "test-request-id")
|
|
ctx = context.WithValue(ctx, auth.AdminKey{}, true)
|
|
ctx = context.WithValue(ctx, auth.UserKey{}, "ops-admin")
|
|
req = req.WithContext(ctx)
|
|
w := httptest.NewRecorder()
|
|
|
|
h.BulkRevoke(w, req)
|
|
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("expected status 200 for admin caller, got %d (body=%q)", w.Code, w.Body.String())
|
|
}
|
|
if capturedActor != "ops-admin" {
|
|
t.Errorf("expected actor ops-admin, got %q", capturedActor)
|
|
}
|
|
}
|