mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 20: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.
362 lines
13 KiB
Go
362 lines
13 KiB
Go
package handler
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"crypto/ecdsa"
|
|
"crypto/elliptic"
|
|
"crypto/rand"
|
|
"crypto/x509"
|
|
"crypto/x509/pkix"
|
|
"encoding/pem"
|
|
"errors"
|
|
"math/big"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/certctl-io/certctl/internal/auth"
|
|
"github.com/certctl-io/certctl/internal/domain"
|
|
"github.com/certctl-io/certctl/internal/service"
|
|
)
|
|
|
|
// mockIntermediateCAService is the minimal IntermediateCAServicer for
|
|
// handler-layer tests. Captures the arguments each method was called
|
|
// with so tests can assert dispatch + RBAC behavior.
|
|
type mockIntermediateCAService struct {
|
|
createRootCalled bool
|
|
createChildCalled bool
|
|
retireCalled bool
|
|
createRootErr error
|
|
createChildErr error
|
|
retireErr error
|
|
retireConfirm bool
|
|
|
|
// Get returns this row when nonzero; otherwise the
|
|
// IntermediateCANotFound sentinel.
|
|
getResult *domain.IntermediateCA
|
|
|
|
// LoadHierarchy returns this slice if non-nil.
|
|
loadHierarchyResult []*domain.IntermediateCA
|
|
}
|
|
|
|
func (m *mockIntermediateCAService) CreateRoot(ctx context.Context, issuerID, name, decidedBy string,
|
|
rootCertPEM []byte, keyDriverID string, opts *service.CreateRootOptions) (string, error) {
|
|
m.createRootCalled = true
|
|
if m.createRootErr != nil {
|
|
return "", m.createRootErr
|
|
}
|
|
return "ica-root-mock", nil
|
|
}
|
|
|
|
func (m *mockIntermediateCAService) CreateChild(ctx context.Context, parentCAID, name, decidedBy string,
|
|
opts *service.CreateChildOptions) (string, error) {
|
|
m.createChildCalled = true
|
|
if m.createChildErr != nil {
|
|
return "", m.createChildErr
|
|
}
|
|
return "ica-child-mock", nil
|
|
}
|
|
|
|
func (m *mockIntermediateCAService) Retire(ctx context.Context, caID, decidedBy, note string, confirm bool) error {
|
|
m.retireCalled = true
|
|
m.retireConfirm = confirm
|
|
return m.retireErr
|
|
}
|
|
|
|
func (m *mockIntermediateCAService) Get(ctx context.Context, id string) (*domain.IntermediateCA, error) {
|
|
if m.getResult != nil {
|
|
return m.getResult, nil
|
|
}
|
|
return nil, service.ErrIntermediateCANotFound
|
|
}
|
|
|
|
func (m *mockIntermediateCAService) LoadHierarchy(ctx context.Context, issuerID string) ([]*domain.IntermediateCA, error) {
|
|
return m.loadHierarchyResult, nil
|
|
}
|
|
|
|
// withAdmin returns a context with the admin flag set + a non-empty
|
|
// authenticated user — the standard "admin caller" shape for these
|
|
// tests.
|
|
func withAdmin(actor string, admin bool) context.Context {
|
|
ctx := context.WithValue(context.Background(), auth.UserKey{}, actor)
|
|
ctx = context.WithValue(ctx, auth.AdminKey{}, admin)
|
|
return ctx
|
|
}
|
|
|
|
// helperRootCertPEM returns a freshly-minted self-signed root cert
|
|
// PEM for the body of CreateRoot tests.
|
|
func helperRootCertPEM(t *testing.T) []byte {
|
|
t.Helper()
|
|
priv, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
|
|
serial, _ := rand.Int(rand.Reader, new(big.Int).Lsh(big.NewInt(1), 128))
|
|
subj := pkix.Name{CommonName: "Test Root"}
|
|
tmpl := &x509.Certificate{
|
|
SerialNumber: serial,
|
|
Subject: subj,
|
|
Issuer: subj,
|
|
NotBefore: time.Now().Add(-time.Hour),
|
|
NotAfter: time.Now().Add(time.Hour),
|
|
KeyUsage: x509.KeyUsageCertSign,
|
|
BasicConstraintsValid: true,
|
|
IsCA: true,
|
|
}
|
|
der, _ := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &priv.PublicKey, priv)
|
|
return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der})
|
|
}
|
|
|
|
// TestIntermediateCA_Handler_NonAdmin_Returns403 pins the
|
|
// admin-gating contract. Any non-admin Bearer caller — even a valid
|
|
// 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.
|
|
|
|
// 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.
|
|
|
|
// TestIntermediateCA_Handler_AdminPermitted_ForwardsActor pins the
|
|
// admin-allowed actor-attribution path. An admin caller's actor
|
|
// (UserKey context value) must be forwarded to the service so the
|
|
// audit trail records who registered the CA. M-008 admin-gate
|
|
// triplet test #3.
|
|
func TestIntermediateCA_Handler_AdminPermitted_ForwardsActor(t *testing.T) {
|
|
mock := &mockIntermediateCAService{
|
|
getResult: &domain.IntermediateCA{ID: "ica-mock"},
|
|
}
|
|
h := NewIntermediateCAHandler(mock)
|
|
rootPEM := helperRootCertPEM(t)
|
|
body := `{"name":"Acme Root","root_cert_pem":` + jsonString(string(rootPEM)) +
|
|
`,"key_driver_id":"/etc/certctl/keys/root.pem"}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/issuers/iss-1/intermediates",
|
|
bytes.NewReader([]byte(body)))
|
|
req.SetPathValue("id", "iss-1")
|
|
req = req.WithContext(withAdmin("admin-actor", true))
|
|
w := httptest.NewRecorder()
|
|
h.Create(w, req)
|
|
if w.Code != http.StatusCreated {
|
|
t.Fatalf("expected 201, got %d body=%s", w.Code, w.Body.String())
|
|
}
|
|
if !mock.createRootCalled {
|
|
t.Fatalf("expected service dispatch with admin actor")
|
|
}
|
|
}
|
|
|
|
// TestIntermediateCA_HandlerCreate_RootDispatch pins the body
|
|
// discriminator: empty parent_ca_id + root_cert_pem + key_driver_id
|
|
// → CreateRoot (not CreateChild). The mock service captures which
|
|
// method was called.
|
|
func TestIntermediateCA_HandlerCreate_RootDispatch(t *testing.T) {
|
|
mock := &mockIntermediateCAService{
|
|
getResult: &domain.IntermediateCA{ID: "ica-root-mock"},
|
|
}
|
|
h := NewIntermediateCAHandler(mock)
|
|
rootPEM := helperRootCertPEM(t)
|
|
body := `{
|
|
"name": "Acme Root",
|
|
"root_cert_pem": ` + jsonString(string(rootPEM)) + `,
|
|
"key_driver_id": "/etc/certctl/keys/root.pem"
|
|
}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/issuers/iss-1/intermediates",
|
|
bytes.NewReader([]byte(body)))
|
|
req.SetPathValue("id", "iss-1")
|
|
req = req.WithContext(withAdmin("admin-actor", true))
|
|
w := httptest.NewRecorder()
|
|
h.Create(w, req)
|
|
if w.Code != http.StatusCreated {
|
|
t.Fatalf("expected 201, got %d body=%s", w.Code, w.Body.String())
|
|
}
|
|
if !mock.createRootCalled {
|
|
t.Fatalf("expected CreateRoot dispatch, got CreateChild=%v", mock.createChildCalled)
|
|
}
|
|
if mock.createChildCalled {
|
|
t.Fatalf("expected only CreateRoot, but CreateChild was also called")
|
|
}
|
|
}
|
|
|
|
// TestIntermediateCA_HandlerCreate_ChildDispatch pins the
|
|
// discriminator's other half: parent_ca_id present → CreateChild.
|
|
func TestIntermediateCA_HandlerCreate_ChildDispatch(t *testing.T) {
|
|
mock := &mockIntermediateCAService{
|
|
getResult: &domain.IntermediateCA{ID: "ica-child-mock"},
|
|
}
|
|
h := NewIntermediateCAHandler(mock)
|
|
body := `{
|
|
"name": "Acme Policy",
|
|
"parent_ca_id": "ica-root-1",
|
|
"subject": {"common_name": "Acme Policy CA", "organization": ["Acme"]},
|
|
"algorithm": "ECDSA-P256",
|
|
"ttl_days": 1825
|
|
}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/issuers/iss-1/intermediates",
|
|
bytes.NewReader([]byte(body)))
|
|
req.SetPathValue("id", "iss-1")
|
|
req = req.WithContext(withAdmin("admin-actor", true))
|
|
w := httptest.NewRecorder()
|
|
h.Create(w, req)
|
|
if w.Code != http.StatusCreated {
|
|
t.Fatalf("expected 201, got %d body=%s", w.Code, w.Body.String())
|
|
}
|
|
if !mock.createChildCalled {
|
|
t.Fatalf("expected CreateChild dispatch")
|
|
}
|
|
if mock.createRootCalled {
|
|
t.Fatalf("expected only CreateChild, but CreateRoot was also called")
|
|
}
|
|
}
|
|
|
|
// TestIntermediateCA_HandlerCreate_BadRequestOnMissingRootBundle pins
|
|
// the validation: empty parent_ca_id + missing root_cert_pem →
|
|
// HTTP 400.
|
|
func TestIntermediateCA_HandlerCreate_BadRequestOnMissingRootBundle(t *testing.T) {
|
|
h := NewIntermediateCAHandler(&mockIntermediateCAService{})
|
|
body := `{"name": "Some Name"}` // no parent, no root bundle
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/issuers/iss-1/intermediates",
|
|
bytes.NewReader([]byte(body)))
|
|
req.SetPathValue("id", "iss-1")
|
|
req = req.WithContext(withAdmin("admin-actor", true))
|
|
w := httptest.NewRecorder()
|
|
h.Create(w, req)
|
|
if w.Code != http.StatusBadRequest {
|
|
t.Fatalf("expected 400, got %d body=%s", w.Code, w.Body.String())
|
|
}
|
|
}
|
|
|
|
// TestIntermediateCA_HandlerCreate_ServiceErrorMappings pins the
|
|
// error → HTTP code dispatch table.
|
|
func TestIntermediateCA_HandlerCreate_ServiceErrorMappings(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
err error
|
|
wantCode int
|
|
isRootCmd bool
|
|
}{
|
|
{"NotSelfSigned->400", service.ErrCANotSelfSigned, http.StatusBadRequest, true},
|
|
{"KeyMismatch->400", service.ErrCAKeyMismatch, http.StatusBadRequest, true},
|
|
{"PathLenExceeded->400", service.ErrPathLenExceeded, http.StatusBadRequest, false},
|
|
{"NameConstraintExceeded->400", service.ErrNameConstraintExceeded, http.StatusBadRequest, false},
|
|
{"ParentNotActive->409", service.ErrParentCANotActive, http.StatusConflict, false},
|
|
{"NotFound->404", service.ErrIntermediateCANotFound, http.StatusNotFound, false},
|
|
{"Other->500", errors.New("unexpected"), http.StatusInternalServerError, false},
|
|
}
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
mock := &mockIntermediateCAService{}
|
|
if tc.isRootCmd {
|
|
mock.createRootErr = tc.err
|
|
} else {
|
|
mock.createChildErr = tc.err
|
|
}
|
|
h := NewIntermediateCAHandler(mock)
|
|
var body string
|
|
if tc.isRootCmd {
|
|
rootPEM := helperRootCertPEM(t)
|
|
body = `{"name":"Root","root_cert_pem":` + jsonString(string(rootPEM)) + `,"key_driver_id":"/k"}`
|
|
} else {
|
|
body = `{"name":"Child","parent_ca_id":"ica-root-1"}`
|
|
}
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/issuers/iss-1/intermediates",
|
|
bytes.NewReader([]byte(body)))
|
|
req.SetPathValue("id", "iss-1")
|
|
req = req.WithContext(withAdmin("admin-actor", true))
|
|
w := httptest.NewRecorder()
|
|
h.Create(w, req)
|
|
if w.Code != tc.wantCode {
|
|
t.Fatalf("expected %d, got %d body=%s", tc.wantCode, w.Code, w.Body.String())
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestIntermediateCA_HandlerRetire_TwoPhaseConfirm pins the body's
|
|
// confirm flag passes through to the service. First call confirm=false;
|
|
// second call confirm=true (the operator explicitly terminalizes).
|
|
func TestIntermediateCA_HandlerRetire_TwoPhaseConfirm(t *testing.T) {
|
|
mock := &mockIntermediateCAService{}
|
|
h := NewIntermediateCAHandler(mock)
|
|
|
|
// First call — confirm omitted (defaults to false).
|
|
body1 := `{"note": "drain start"}`
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/intermediates/ica-1/retire",
|
|
bytes.NewReader([]byte(body1)))
|
|
req.SetPathValue("id", "ica-1")
|
|
req = req.WithContext(withAdmin("admin-actor", true))
|
|
w := httptest.NewRecorder()
|
|
h.Retire(w, req)
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("first retire: expected 200, got %d body=%s", w.Code, w.Body.String())
|
|
}
|
|
if mock.retireConfirm {
|
|
t.Fatalf("first retire: expected confirm=false, got true")
|
|
}
|
|
|
|
// Second call — confirm=true.
|
|
mock.retireCalled = false
|
|
body2 := `{"note":"terminalize","confirm":true}`
|
|
req = httptest.NewRequest(http.MethodPost, "/api/v1/intermediates/ica-1/retire",
|
|
bytes.NewReader([]byte(body2)))
|
|
req.SetPathValue("id", "ica-1")
|
|
req = req.WithContext(withAdmin("admin-actor", true))
|
|
w = httptest.NewRecorder()
|
|
h.Retire(w, req)
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("second retire: expected 200, got %d body=%s", w.Code, w.Body.String())
|
|
}
|
|
if !mock.retireConfirm {
|
|
t.Fatalf("second retire: expected confirm=true, got false")
|
|
}
|
|
}
|
|
|
|
// TestIntermediateCA_HandlerRetire_StillHasActiveChildren_Returns409
|
|
// pins the drain-first contract: ErrCAStillHasActiveChildren maps
|
|
// to HTTP 409.
|
|
func TestIntermediateCA_HandlerRetire_StillHasActiveChildren_Returns409(t *testing.T) {
|
|
mock := &mockIntermediateCAService{retireErr: service.ErrCAStillHasActiveChildren}
|
|
h := NewIntermediateCAHandler(mock)
|
|
req := httptest.NewRequest(http.MethodPost, "/api/v1/intermediates/ica-1/retire",
|
|
bytes.NewReader([]byte(`{"confirm": true}`)))
|
|
req.SetPathValue("id", "ica-1")
|
|
req = req.WithContext(withAdmin("admin-actor", true))
|
|
w := httptest.NewRecorder()
|
|
h.Retire(w, req)
|
|
if w.Code != http.StatusConflict {
|
|
t.Fatalf("expected 409, got %d body=%s", w.Code, w.Body.String())
|
|
}
|
|
}
|
|
|
|
// jsonString returns a JSON-quoted Go string suitable for embedding
|
|
// in a test JSON body literal. Standard library encoding/json's
|
|
// Marshal does the same thing but the test assertions are clearer
|
|
// when we control the wrapping.
|
|
func jsonString(s string) string {
|
|
return string(mustMarshalJSONString(s))
|
|
}
|
|
|
|
func mustMarshalJSONString(s string) []byte {
|
|
// Trivial: wrap in quotes and escape \ and " — sufficient for
|
|
// PEM bodies (which contain newlines but no quotes).
|
|
out := make([]byte, 0, len(s)+2)
|
|
out = append(out, '"')
|
|
for _, r := range []byte(s) {
|
|
switch r {
|
|
case '"':
|
|
out = append(out, '\\', '"')
|
|
case '\\':
|
|
out = append(out, '\\', '\\')
|
|
case '\n':
|
|
out = append(out, '\\', 'n')
|
|
case '\r':
|
|
out = append(out, '\\', 'r')
|
|
case '\t':
|
|
out = append(out, '\\', 't')
|
|
default:
|
|
out = append(out, r)
|
|
}
|
|
}
|
|
out = append(out, '"')
|
|
return out
|
|
}
|