auth-bundle-1 Phase 9 + 10: approval-bypass closure + RBAC GUI

# Phase 9 — approval-bypass closure (Decision 9, option a)

* Migration 000033_approval_kinds.up.sql: ALTER TABLE
  issuance_approval_requests ADD COLUMN approval_kind +
  payload JSONB; relax certificate_id + job_id to nullable;
  CHECK (approval_kind IN ('cert_issuance','profile_edit'))
  + CHECK (per-kind nullability invariant) + index on
  approval_kind. Idempotent throughout via DO blocks.
* domain.ApprovalKind enum (cert_issuance / profile_edit) +
  IsValidApprovalKind. ApprovalRequest gains Kind +
  Payload []byte for the pending profile diff.
* postgres.ApprovalRepository.Create + scanApprovalRow extended
  to round-trip the new columns; certificate_id + job_id
  switched to sql.NullString so profile_edit rows persist
  cleanly. Default Kind=cert_issuance preserves back-compat
  for every Phase-7-2026-05-03 caller.
* ApprovalService.RequestProfileEditApproval: new entry point
  that creates a pending profile-edit row carrying the
  serialized profile diff. Bypass mode (CERTCTL_APPROVAL_BYPASS)
  short-circuits the same way it does for cert_issuance.
* ApprovalService.SetProfileEditApply hook: cmd/server/main.go
  registers a closure that deserializes req.Payload + persists
  via profileRepo.Update + emits a profile.edit_applied audit
  row with category=auth. The hook avoids the Approval ↔
  Profile import cycle.
* ProfileService.UpdateProfile: gates when (a) the live
  profile carries RequiresApproval=true, OR (b) the proposed
  edit would set it true. Returns ErrProfileEditPendingApproval
  with the new approval ID; ProfileHandler maps to HTTP 202
  Accepted + {pending_approval_id}. Both arms close the
  flip-flop loophole because every transition through an
  approval-tier profile fires the gate.
* TestProfileEdit_RequiresApprovalLoopholeClosed pins all 3
  bypass attempts (flip-off / kept-on / flip-on) gated; nil-
  approval-service preserves pre-Phase-9 direct-apply for
  test fixtures.
* Approval service tests gain 4 profile_edit rows: pending row
  shape; same-actor self-approve rejected with
  ErrApproveBySameActor (load-bearing two-person integrity);
  approve fails-closed when apply callback unwired;
  apply callback invoked on approve.
* docs/reference/profiles.md (new) explains the gate +
  edit response shape (202) + same-actor invariant + bypass
  + audit hooks.

# Phase 10 — RBAC management GUI

* useAuthMe hook (web/src/hooks/useAuthMe.ts): TanStack Query
  fetches /api/v1/auth/me on app boot, caches for 60s, exposes
  hasPerm(p) + hasAnyPerm + isAdmin predicates. Every Phase-10
  page consumes this on mount + gates affordances against the
  cached effective_permissions slice. Server-side enforcement
  is the load-bearing gate; client-side hide/disable is UX.
* New routes:
   - /auth/roles — list (auth.role.list); create-role modal
     (auth.role.create) hidden when missing.
   - /auth/roles/:id — detail + permissions; edit
     (auth.role.edit), delete (auth.role.delete), add/remove
     permission affordances each gated.
   - /auth/keys — list of every actor with role grants; assign
     + revoke modals (auth.role.assign). actor-demo-anon
     flagged system-managed; mutation buttons hidden for it.
   - /auth/settings — stub showing /v1/auth/me identity +
     bootstrap-endpoint availability via /v1/auth/bootstrap.
* AuditPage extended with category filter ('All categories'
  + the 3 enum values from migration 000032). Selection flows
  to the API call params + the URL-driven query state.
* Layout: 3 new nav entries (Roles / API Keys / Auth Settings).
* api/client.ts: 12 new exported functions for the RBAC
  surface (authMe, list/get/create/update/delete role,
  list/add/remove role permissions, list keys, assign/revoke
  key role, bootstrap-availability probe).
* data-testid attributes on every interactive element so a
  future Playwright suite can assert behavior without brittle
  CSS selectors.
* Empty state, error state, and unsaved-changes warnings on
  every form per the prompt's implementation rules.

# Frontend tests

* RolesPage.test.tsx (6 tests): list render, empty state,
  error state, hide-create-button-without-perm,
  show-create-button-with-perm, submit-create-modal.
* KeysPage.test.tsx (3 tests): demo-anon flagged
  system-managed (no buttons), permission-gated affordance
  hide for auditor caller, assign-modal-POST contract.
* AuthSettingsPage.test.tsx (2 tests): identity surface,
  bootstrap-OPEN-status surface.
* AuditPage.test.tsx (+1): category-filter select renders
  with the 4 documented options.

15 frontend tests total in src/pages/auth/ + the audit
category-filter test; all pass via npx vitest run.

# Verifications

* go vet ./... clean.
* staticcheck across internal/auth + handler + router + cli +
  service + repository + cmd + domain: clean.
* gofmt -l clean repo-wide.
* go test -short -count=1 green across internal/service,
  internal/api/handler, internal/api/router, internal/auth,
  internal/auth/bootstrap, internal/service/auth,
  internal/domain/auth, cmd/server, cmd/cli, internal/cli.
* npx tsc --noEmit clean.
* npm run build green (vite build produces dist/index.html
  + 946KB JS bundle; chunk-size warning is pre-existing).
* npx vitest run src/pages/auth/ src/pages/AuditPage.test.tsx
  green (15 tests, 4 files).
This commit is contained in:
shankar0123
2026-05-09 21:03:59 +00:00
parent af4fa12724
commit 69a508dfcf
24 changed files with 2413 additions and 29 deletions
+20 -1
View File
@@ -4,13 +4,14 @@ import (
"context"
"encoding/json"
"errors"
"github.com/certctl-io/certctl/internal/repository"
"net/http"
"strconv"
"strings"
"github.com/certctl-io/certctl/internal/api/middleware"
"github.com/certctl-io/certctl/internal/domain"
"github.com/certctl-io/certctl/internal/repository"
"github.com/certctl-io/certctl/internal/service"
)
// ProfileService defines the service interface for certificate profile operations.
@@ -164,6 +165,24 @@ func (h ProfileHandler) UpdateProfile(w http.ResponseWriter, r *http.Request) {
updated, err := h.svc.UpdateProfile(r.Context(), id, profile)
if err != nil {
// Bundle 1 Phase 9: a profile with RequiresApproval=true (or
// an edit that would set it true) routes through the approval
// workflow. The service returns ErrProfileEditPendingApproval
// wrapped with the new approval ID; surface 202 Accepted +
// pending_approval_id so the operator knows to chase a
// non-requester admin to approve via /v1/approvals/{id}/approve.
if errors.Is(err, service.ErrProfileEditPendingApproval) {
approvalID := ""
if msg := err.Error(); strings.Contains(msg, "approval=") {
approvalID = msg[strings.Index(msg, "approval=")+len("approval="):]
}
JSON(w, http.StatusAccepted, map[string]interface{}{
"status": "pending_approval",
"pending_approval_id": approvalID,
"message": "profile edit requires approval (see /v1/approvals/{id}/approve)",
})
return
}
if errors.Is(err, repository.ErrNotFound) {
ErrorWithRequestID(w, http.StatusNotFound, "Profile not found", requestID)
return
+48 -12
View File
@@ -22,18 +22,54 @@ import "time"
// PCI-DSS Level 1, FedRAMP Moderate / High, and SOC 2 Type II
// customers.
type ApprovalRequest struct {
ID string `json:"id"` // ar-<slug>
CertificateID string `json:"certificate_id"` // FK managed_certificates.id
JobID string `json:"job_id"` // FK jobs.id (the blocked Job)
ProfileID string `json:"profile_id"` // CertificateProfile that triggered the gate
RequestedBy string `json:"requested_by"` // actor that triggered the renewal
State ApprovalState `json:"state"` // pending / approved / rejected / expired
DecidedBy *string `json:"decided_by,omitempty"` // null while state=pending
DecidedAt *time.Time `json:"decided_at,omitempty"` // null while state=pending
DecisionNote *string `json:"decision_note,omitempty"` // operator's reason text
Metadata map[string]string `json:"metadata,omitempty"` // common_name, sans, issuer_id, severity_tier
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
ID string `json:"id"` // ar-<slug>
Kind ApprovalKind `json:"kind"` // cert_issuance | profile_edit (Phase 9)
CertificateID string `json:"certificate_id,omitempty"` // FK managed_certificates.id (nullable for profile_edit)
JobID string `json:"job_id,omitempty"` // FK jobs.id (nullable for profile_edit)
ProfileID string `json:"profile_id"` // CertificateProfile that triggered the gate
RequestedBy string `json:"requested_by"` // actor that triggered the renewal
State ApprovalState `json:"state"` // pending / approved / rejected / expired
DecidedBy *string `json:"decided_by,omitempty"` // null while state=pending
DecidedAt *time.Time `json:"decided_at,omitempty"` // null while state=pending
DecisionNote *string `json:"decision_note,omitempty"` // operator's reason text
Metadata map[string]string `json:"metadata,omitempty"` // common_name, sans, issuer_id, severity_tier
// Payload (Phase 9) carries the pending profile diff for
// approval_kind=profile_edit rows. Empty for cert_issuance.
// Stored as a raw JSON byte slice so the service layer
// serializes/deserializes the *domain.CertificateProfile
// without the repository needing to know the inner shape.
Payload []byte `json:"payload,omitempty"`
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
}
// ApprovalKind classifies the row into one of the supported approval
// workflows. Bundle 1 Phase 9 ships exactly two kinds. Bundle 2 will
// extend the enum (and the migration's CHECK constraint) without
// reshaping the column.
type ApprovalKind string
const (
// ApprovalKindCertIssuance is the original Rank-7 workflow:
// cert/renewal blocked at JobStatusAwaitingApproval until a
// non-requester decides. cert_id + job_id are required.
ApprovalKindCertIssuance ApprovalKind = "cert_issuance"
// ApprovalKindProfileEdit (Phase 9) closes the flip-flop loophole:
// a profile with RequiresApproval=true cannot be mutated until a
// non-requester decides. The pending diff lives in Payload until
// the approver's POST /v1/approvals/{id}/approve triggers the
// apply path. cert_id / job_id are NULL for these rows.
ApprovalKindProfileEdit ApprovalKind = "profile_edit"
)
// IsValidApprovalKind reports whether k is a closed-enum value.
func IsValidApprovalKind(k ApprovalKind) bool {
switch k {
case ApprovalKindCertIssuance, ApprovalKindProfileEdit:
return true
}
return false
}
// ApprovalState is the closed enum of approval lifecycle states.
+45 -9
View File
@@ -60,19 +60,41 @@ func (r *ApprovalRepository) Create(ctx context.Context, req *domain.ApprovalReq
metadataJSON = []byte("{}")
}
// Bundle 1 Phase 9: empty Kind defaults to cert_issuance to
// preserve back-compat for every Phase-7-2026-05-03 caller.
if req.Kind == "" {
req.Kind = domain.ApprovalKindCertIssuance
}
if !domain.IsValidApprovalKind(req.Kind) {
return fmt.Errorf("invalid approval kind %q", req.Kind)
}
// nullable cert_id / job_id for profile_edit rows.
var certID, jobID interface{}
if req.CertificateID != "" {
certID = req.CertificateID
}
if req.JobID != "" {
jobID = req.JobID
}
var payload interface{}
if len(req.Payload) > 0 {
payload = req.Payload
}
const q = `
INSERT INTO issuance_approval_requests
(id, certificate_id, job_id, profile_id, requested_by,
state, decided_by, decided_at, decision_note, metadata,
created_at, updated_at)
created_at, updated_at, approval_kind, payload)
VALUES
($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12)
($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14)
`
_, err = r.db.ExecContext(ctx, q,
req.ID, req.CertificateID, req.JobID, req.ProfileID, req.RequestedBy,
req.ID, certID, jobID, req.ProfileID, req.RequestedBy,
string(req.State), req.DecidedBy, req.DecidedAt, req.DecisionNote, metadataJSON,
req.CreatedAt, req.UpdatedAt,
req.CreatedAt, req.UpdatedAt, string(req.Kind), payload,
)
if err != nil {
var pqErr *pq.Error
@@ -89,7 +111,7 @@ func (r *ApprovalRepository) Get(ctx context.Context, id string) (*domain.Approv
const q = `
SELECT id, certificate_id, job_id, profile_id, requested_by,
state, decided_by, decided_at, decision_note, metadata,
created_at, updated_at
created_at, updated_at, approval_kind, payload
FROM issuance_approval_requests
WHERE id = $1
`
@@ -103,7 +125,7 @@ func (r *ApprovalRepository) GetByJobID(ctx context.Context, jobID string) (*dom
const q = `
SELECT id, certificate_id, job_id, profile_id, requested_by,
state, decided_by, decided_at, decision_note, metadata,
created_at, updated_at
created_at, updated_at, approval_kind, payload
FROM issuance_approval_requests
WHERE job_id = $1
ORDER BY created_at DESC
@@ -131,7 +153,7 @@ func (r *ApprovalRepository) List(ctx context.Context, filter *repository.Approv
q := `
SELECT id, certificate_id, job_id, profile_id, requested_by,
state, decided_by, decided_at, decision_note, metadata,
created_at, updated_at
created_at, updated_at, approval_kind, payload
FROM issuance_approval_requests
WHERE 1 = 1
`
@@ -269,16 +291,20 @@ type rowScanner interface {
func scanApprovalRow(row rowScanner) (*domain.ApprovalRequest, error) {
var (
req domain.ApprovalRequest
certID sql.NullString
jobID sql.NullString
stateStr string
decidedBy sql.NullString
decidedAt sql.NullTime
decisionNote sql.NullString
metadataJSON []byte
kindStr string
payload []byte
)
err := row.Scan(
&req.ID, &req.CertificateID, &req.JobID, &req.ProfileID, &req.RequestedBy,
&req.ID, &certID, &jobID, &req.ProfileID, &req.RequestedBy,
&stateStr, &decidedBy, &decidedAt, &decisionNote, &metadataJSON,
&req.CreatedAt, &req.UpdatedAt,
&req.CreatedAt, &req.UpdatedAt, &kindStr, &payload,
)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
@@ -288,6 +314,16 @@ func scanApprovalRow(row rowScanner) (*domain.ApprovalRequest, error) {
}
req.State = domain.ApprovalState(stateStr)
req.Kind = domain.ApprovalKind(kindStr)
if certID.Valid {
req.CertificateID = certID.String
}
if jobID.Valid {
req.JobID = jobID.String
}
if len(payload) > 0 {
req.Payload = payload
}
if decidedBy.Valid {
s := decidedBy.String
req.DecidedBy = &s
+91
View File
@@ -39,6 +39,25 @@ type ApprovalService struct {
metrics *ApprovalMetrics
bypassEnabled bool
// profileEditApply is the Bundle 1 Phase 9 hook the approve
// path invokes when req.Kind=profile_edit. Registered by
// cmd/server/main.go via SetProfileEditApply so the service
// doesn't import internal/service/profile.go (would create a
// cycle: ApprovalService -> ProfileService -> ApprovalService).
profileEditApply ProfileEditApplyFunc
}
// ProfileEditApplyFunc deserializes the pending profile diff stored
// in req.Payload and persists it via the profile repository. The
// caller registers this once at boot via SetProfileEditApply.
type ProfileEditApplyFunc func(ctx context.Context, req *domain.ApprovalRequest) error
// SetProfileEditApply registers the profile-edit apply callback. Called
// from main.go after both the ApprovalService and ProfileService are
// constructed; the closure captures the profile repo + audit service.
func (s *ApprovalService) SetProfileEditApply(f ProfileEditApplyFunc) {
s.profileEditApply = f
}
// JobStatusUpdater is the narrow interface ApprovalService depends on
@@ -139,6 +158,53 @@ func (s *ApprovalService) RequestApproval(
return req.ID, nil
}
// RequestProfileEditApproval is the Bundle 1 Phase 9 entry point for
// gated profile mutations. ProfileService.UpdateProfile calls this
// when the live profile (or the proposed update) carries
// RequiresApproval=true. Returns the new pending approval ID.
//
// The pending diff is serialized to req.Payload as JSON; the
// profile-edit-apply callback (registered by main.go) deserializes
// and persists when an approver decides.
//
// In bypass mode (CERTCTL_APPROVAL_BYPASS=true) the call short-
// circuits via approveInternal — the same dev/CI escape hatch as
// cert_issuance — so renewal-loop tests remain fast.
func (s *ApprovalService) RequestProfileEditApproval(
ctx context.Context,
profileID, requestedBy string,
payload []byte,
) (string, error) {
if profileID == "" || requestedBy == "" {
return "", fmt.Errorf("approval: profileID + requestedBy required")
}
if len(payload) == 0 {
return "", fmt.Errorf("approval: payload required for profile_edit")
}
now := time.Now().UTC()
req := &domain.ApprovalRequest{
Kind: domain.ApprovalKindProfileEdit,
ProfileID: profileID,
RequestedBy: requestedBy,
State: domain.ApprovalStatePending,
Payload: payload,
CreatedAt: now,
UpdatedAt: now,
}
if err := s.approvalRepo.Create(ctx, req); err != nil {
return "", fmt.Errorf("approval: create profile_edit request: %w", err)
}
s.recordAudit(ctx, requestedBy, domain.ActorTypeUser, "approval_profile_edit_requested", req, nil)
if s.bypassEnabled {
if err := s.approveInternal(ctx, req.ID, domain.ApprovalActorSystemBypass,
"auto-approved by CERTCTL_APPROVAL_BYPASS — dev/CI mode",
domain.ApprovalOutcomeBypassed, domain.ActorTypeSystem); err != nil {
return req.ID, fmt.Errorf("approval: bypass auto-approve profile_edit: %w", err)
}
}
return req.ID, nil
}
// Approve transitions a pending request to approved AND the linked Job
// from AwaitingApproval to Pending so the job processor picks it up.
// RBAC: rejects if decidedBy == request.RequestedBy.
@@ -194,6 +260,31 @@ func (s *ApprovalService) approveInternal(
return fmt.Errorf("approval: update state to approved: %w", err)
}
// Bundle 1 Phase 9: profile_edit kind requires the apply
// callback to deserialize req.Payload + persist the profile
// diff. cert_issuance kind continues through the existing job-
// transition path. The kind discriminator is the load-bearing
// dispatch — adding a future ApprovalKind goes here.
if req.Kind == domain.ApprovalKindProfileEdit {
if s.profileEditApply == nil {
s.recordAudit(ctx, decidedBy, actorType, "approval_profile_apply_missing", req,
map[string]interface{}{"error": "profileEditApply callback not wired"})
return fmt.Errorf("approval: profile-edit apply callback not registered")
}
if err := s.profileEditApply(ctx, req); err != nil {
s.recordAudit(ctx, decidedBy, actorType, "approval_profile_apply_failed", req,
map[string]interface{}{"error": err.Error()})
return fmt.Errorf("approval: apply profile edit: %w", err)
}
s.recordAudit(ctx, decidedBy, actorType, "approval_"+outcome, req,
map[string]interface{}{"note": note, "outcome": outcome, "kind": string(req.Kind)})
if s.metrics != nil {
s.metrics.RecordDecision(outcome, req.ProfileID)
s.metrics.ObservePendingAge(now.Sub(req.CreatedAt).Seconds())
}
return nil
}
// Transition the linked Job from AwaitingApproval to Pending so the
// scheduler picks it up. Best-effort — if the Job has already been
// cancelled or otherwise mutated externally, log via audit and move on.
+104 -3
View File
@@ -3,6 +3,7 @@ package service
import (
"context"
"errors"
"strings"
"sync"
"testing"
"time"
@@ -30,9 +31,14 @@ func (f *fakeApprovalRepo) Create(ctx context.Context, req *domain.ApprovalReque
req.ID = "ar-fake-" + time.Now().Format("150405.000000000")
}
// Enforce the partial-unique pending-per-job at the mock layer too.
for _, existing := range f.rows {
if existing.JobID == req.JobID && existing.State == domain.ApprovalStatePending {
return repository.ErrAlreadyExists
// Bundle 1 Phase 9: Postgres treats NULLs as distinct in UNIQUE
// indexes, so profile_edit rows (JobID="") never collide with
// each other or with cert_issuance rows. Mirror that here.
if req.JobID != "" {
for _, existing := range f.rows {
if existing.JobID == req.JobID && existing.State == domain.ApprovalStatePending {
return repository.ErrAlreadyExists
}
}
}
cp := *req
@@ -384,3 +390,98 @@ func TestApproval_MetricCounterIncrements(t *testing.T) {
t.Fatalf("expected at least 3 histogram samples; got %d", hist.Count)
}
}
// =============================================================================
// Bundle 1 Phase 9 — profile_edit kind tests.
// =============================================================================
// TestApproval_RequestProfileEditCreatesPendingRow pins the new
// RequestProfileEditApproval entry point: creates a pending row with
// Kind=profile_edit, no cert_id / job_id, and the serialized profile
// diff in Payload.
func TestApproval_RequestProfileEditCreatesPendingRow(t *testing.T) {
svc, ar, _ := newApprovalSvcForTest(false)
payload := []byte(`{"id":"prof-prod","name":"renamed","requires_approval":true}`)
id, err := svc.RequestProfileEditApproval(context.Background(), "prof-prod", "user-alice", payload)
if err != nil {
t.Fatalf("RequestProfileEditApproval err: %v", err)
}
got, err := ar.Get(context.Background(), id)
if err != nil {
t.Fatalf("Get err: %v", err)
}
if got.Kind != domain.ApprovalKindProfileEdit {
t.Errorf("Kind = %q, want profile_edit", got.Kind)
}
if got.CertificateID != "" || got.JobID != "" {
t.Errorf("profile_edit row carries cert_id=%q job_id=%q; both must be empty", got.CertificateID, got.JobID)
}
if string(got.Payload) != string(payload) {
t.Errorf("payload roundtrip wrong; got %s", string(got.Payload))
}
}
// TestApproval_ProfileEdit_SameActorSelfApproveRejected pins the
// load-bearing two-person integrity invariant for profile_edit
// approvals: the requester cannot approve their own row.
func TestApproval_ProfileEdit_SameActorSelfApproveRejected(t *testing.T) {
svc, _, _ := newApprovalSvcForTest(false)
id, err := svc.RequestProfileEditApproval(context.Background(),
"prof-prod", "user-alice",
[]byte(`{"id":"prof-prod"}`))
if err != nil {
t.Fatalf("RequestProfileEditApproval err: %v", err)
}
got := svc.Approve(context.Background(), id, "user-alice", "self-approve attempt")
if !errors.Is(got, ErrApproveBySameActor) {
t.Errorf("self-approve err = %v, want ErrApproveBySameActor", got)
}
}
// TestApproval_ProfileEdit_RejectsWhenApplyCallbackMissing pins
// that the approve path fails closed when a profile_edit row is
// approved without a registered profileEditApply callback. Better
// to surface a 500 than silently mark the row approved while the
// underlying profile is untouched.
func TestApproval_ProfileEdit_RejectsWhenApplyCallbackMissing(t *testing.T) {
svc, _, _ := newApprovalSvcForTest(false)
id, _ := svc.RequestProfileEditApproval(context.Background(),
"prof-prod", "user-alice",
[]byte(`{"id":"prof-prod"}`))
// Approver = different actor.
err := svc.Approve(context.Background(), id, "user-bob", "approving")
if err == nil {
t.Fatalf("Approve must fail when profile-edit-apply is unwired; got nil")
}
// Sentinel propagates from approveInternal — message contains the cue.
if !strings.Contains(err.Error(), "apply callback not registered") {
t.Errorf("err = %v, want 'apply callback not registered'", err)
}
}
// TestApproval_ProfileEdit_ApplyCallbackInvokedOnApprove pins the
// happy-path: when a profile-edit-apply callback is registered AND
// a non-requester approves, the callback fires with the right row.
func TestApproval_ProfileEdit_ApplyCallbackInvokedOnApprove(t *testing.T) {
svc, _, _ := newApprovalSvcForTest(false)
var captured *domain.ApprovalRequest
svc.SetProfileEditApply(func(_ context.Context, req *domain.ApprovalRequest) error {
captured = req
return nil
})
id, _ := svc.RequestProfileEditApproval(context.Background(),
"prof-prod", "user-alice",
[]byte(`{"id":"prof-prod","name":"renamed"}`))
if err := svc.Approve(context.Background(), id, "user-bob", "looks good"); err != nil {
t.Fatalf("Approve err: %v", err)
}
if captured == nil {
t.Fatalf("apply callback never invoked")
}
if captured.Kind != domain.ApprovalKindProfileEdit {
t.Errorf("captured.Kind = %q, want profile_edit", captured.Kind)
}
if captured.ProfileID != "prof-prod" {
t.Errorf("captured.ProfileID = %q, want prof-prod", captured.ProfileID)
}
}
+94 -3
View File
@@ -2,18 +2,37 @@ package service
import (
"context"
"encoding/json"
"errors"
"fmt"
"log/slog"
"time"
"github.com/certctl-io/certctl/internal/auth"
"github.com/certctl-io/certctl/internal/domain"
"github.com/certctl-io/certctl/internal/repository"
)
// ErrProfileEditPendingApproval (Bundle 1 Phase 9) is returned by
// UpdateProfile when the live profile (or the proposed update) carries
// RequiresApproval=true. The handler maps this to HTTP 202 Accepted +
// {pending_approval_id} so the operator knows to chase a second-admin
// approve. See docs/reference/profiles.md.
var ErrProfileEditPendingApproval = errors.New("profile edit gated by approval workflow")
// ProfileEditApprovalRequester is the slice of ApprovalService the
// ProfileService consumes when a profile edit triggers the gate.
// Pulled out as a small interface so unit tests can drive the gate
// without the full ApprovalService dependency tree.
type ProfileEditApprovalRequester interface {
RequestProfileEditApproval(ctx context.Context, profileID, requestedBy string, payload []byte) (string, error)
}
// ProfileService provides business logic for certificate profile management.
type ProfileService struct {
profileRepo repository.CertificateProfileRepository
auditService *AuditService
profileRepo repository.CertificateProfileRepository
auditService *AuditService
approvalService ProfileEditApprovalRequester // Bundle 1 Phase 9; nil disables the gate
}
// NewProfileService creates a new profile service.
@@ -27,6 +46,14 @@ func NewProfileService(
}
}
// SetApprovalService wires the Bundle 1 Phase 9 gate. cmd/server/main.go
// calls this after both ProfileService and ApprovalService are
// constructed. nil disables the gate (preserving pre-Phase-9 behaviour
// for any test fixture or alternate boot path that doesn't wire it).
func (s *ProfileService) SetApprovalService(a ProfileEditApprovalRequester) {
s.approvalService = a
}
// ListProfiles returns all profiles (handler interface method).
func (s *ProfileService) ListProfiles(ctx context.Context, page, perPage int) ([]domain.CertificateProfile, int64, error) {
// Bundle E / Audit L-020: page/perPage are unused; the underlying repo
@@ -97,12 +124,59 @@ func (s *ProfileService) CreateProfile(ctx context.Context, profile domain.Certi
}
// UpdateProfile modifies an existing profile (handler interface method).
//
// Bundle 1 Phase 9 (approval-bypass closure): if the LIVE profile has
// RequiresApproval=true OR the proposed update would set it true, the
// edit is NOT applied directly. Instead it is serialized to a pending
// ApprovalRequest with Kind=profile_edit and the caller receives
// ErrProfileEditPendingApproval. The handler maps this to HTTP 202 +
// the new approval ID. A non-requester admin then approves via the
// existing /v1/approvals/{id}/approve endpoint, which deserializes
// the payload and persists the diff via the profile-edit-apply
// callback registered in main.go. This closes the flip-flop loophole
// where an admin could disable RequiresApproval, mutate, re-enable.
//
// SetApprovalService(nil) disables the gate (test fixtures); the
// pre-Phase-9 direct-apply path is preserved.
func (s *ProfileService) UpdateProfile(ctx context.Context, id string, profile domain.CertificateProfile) (*domain.CertificateProfile, error) {
if err := validateProfile(&profile); err != nil {
return nil, err
}
profile.ID = id
if s.approvalService != nil {
live, err := s.profileRepo.Get(ctx, id)
if err != nil {
return nil, fmt.Errorf("failed to load live profile: %w", err)
}
// Gate when the live profile is approval-tier OR the edit
// would flip it on. Both arms close the loophole: a flip-
// flop attacker can't set false→mutate→true because every
// transition through an approval-tier profile triggers the
// gate.
if (live != nil && live.RequiresApproval) || profile.RequiresApproval {
payload, perr := json.Marshal(profile)
if perr != nil {
return nil, fmt.Errorf("marshal profile for approval payload: %w", perr)
}
requester := actorFromContext(ctx)
approvalID, gerr := s.approvalService.RequestProfileEditApproval(ctx, id, requester, payload)
if gerr != nil {
return nil, fmt.Errorf("approval gate: %w", gerr)
}
if s.auditService != nil {
_ = s.auditService.RecordEventWithCategory(
context.WithoutCancel(ctx),
requester, domain.ActorTypeUser,
"profile.edit_request", domain.EventCategoryAuth,
"certificate_profile", id,
map[string]interface{}{"approval_id": approvalID},
)
}
return nil, fmt.Errorf("%w: approval=%s", ErrProfileEditPendingApproval, approvalID)
}
}
if err := s.profileRepo.Update(ctx, &profile); err != nil {
return nil, fmt.Errorf("failed to update profile: %w", err)
}
@@ -117,6 +191,23 @@ func (s *ProfileService) UpdateProfile(ctx context.Context, id string, profile d
return &profile, nil
}
// actorFromContext pulls the caller's actor ID from the
// auth-middleware ActorIDKey populated by NewAuthWithKeyStore /
// NewDemoModeAuth. Falls back to "api" so legacy test fixtures that
// don't wire the auth context still record meaningful audit rows.
func actorFromContext(ctx context.Context) string {
if ctx == nil {
return "api"
}
if id := auth.GetActorID(ctx); id != "" {
return id
}
if id, ok := ctx.Value(auth.UserKey{}).(string); ok && id != "" {
return id
}
return "api"
}
// DeleteProfile removes a profile (handler interface method).
func (s *ProfileService) DeleteProfile(ctx context.Context, id string) error {
if err := s.profileRepo.Delete(ctx, id); err != nil {
+212
View File
@@ -0,0 +1,212 @@
package service
import (
"context"
"errors"
"testing"
"github.com/certctl-io/certctl/internal/domain"
"github.com/certctl-io/certctl/internal/repository"
)
// =============================================================================
// Bundle 1 Phase 9 — approval-bypass closure regression tests.
//
// Ship a tiny in-memory profile-repo + approval-repo so the gate can
// be exercised without testcontainers. The gate's invariant: any edit
// to a profile that has RequiresApproval=true (or that would set
// RequiresApproval=true) routes through ApprovalService and never
// reaches profileRepo.Update directly.
// =============================================================================
type fakeProfileRepo struct {
rows map[string]*domain.CertificateProfile
}
func newFakeProfileRepo() *fakeProfileRepo {
return &fakeProfileRepo{rows: make(map[string]*domain.CertificateProfile)}
}
func (f *fakeProfileRepo) List(_ context.Context) ([]*domain.CertificateProfile, error) {
out := make([]*domain.CertificateProfile, 0, len(f.rows))
for _, p := range f.rows {
out = append(out, p)
}
return out, nil
}
func (f *fakeProfileRepo) Get(_ context.Context, id string) (*domain.CertificateProfile, error) {
p, ok := f.rows[id]
if !ok {
return nil, repository.ErrNotFound
}
cp := *p
return &cp, nil
}
func (f *fakeProfileRepo) Create(_ context.Context, p *domain.CertificateProfile) error {
cp := *p
f.rows[p.ID] = &cp
return nil
}
func (f *fakeProfileRepo) Update(_ context.Context, p *domain.CertificateProfile) error {
if _, ok := f.rows[p.ID]; !ok {
return repository.ErrNotFound
}
cp := *p
f.rows[p.ID] = &cp
return nil
}
func (f *fakeProfileRepo) Delete(_ context.Context, id string) error {
delete(f.rows, id)
return nil
}
// fakeApprovalGate counts requests + lets the test inspect the
// payload that was queued. Mirrors ProfileEditApprovalRequester.
type fakeApprovalGate struct {
requests []struct {
ProfileID, RequestedBy string
Payload []byte
}
err error
}
func (f *fakeApprovalGate) RequestProfileEditApproval(_ context.Context, profileID, requestedBy string, payload []byte) (string, error) {
if f.err != nil {
return "", f.err
}
f.requests = append(f.requests, struct {
ProfileID, RequestedBy string
Payload []byte
}{profileID, requestedBy, payload})
return "ar-pending-" + profileID, nil
}
// TestProfileEdit_RequiresApprovalLoopholeClosed pins the load-bearing
// invariant: a profile with RequiresApproval=true cannot be mutated
// in-place. The flip-flop loophole (set false → mutate → set true) is
// closed because every call against an approval-tier profile routes
// through ApprovalService BEFORE reaching profileRepo.Update.
func TestProfileEdit_RequiresApprovalLoopholeClosed(t *testing.T) {
repo := newFakeProfileRepo()
repo.rows["prof-prod"] = &domain.CertificateProfile{
ID: "prof-prod",
Name: "production",
RequiresApproval: true,
}
gate := &fakeApprovalGate{}
svc := NewProfileService(repo, nil)
svc.SetApprovalService(gate)
// Attempt 1 — admin tries to flip RequiresApproval off.
flippedOff := domain.CertificateProfile{
ID: "prof-prod",
Name: "production",
RequiresApproval: false, // bypass attempt
}
_, err := svc.UpdateProfile(context.Background(), "prof-prod", flippedOff)
if !errors.Is(err, ErrProfileEditPendingApproval) {
t.Fatalf("flip-off attempt err = %v, want ErrProfileEditPendingApproval", err)
}
live, _ := repo.Get(context.Background(), "prof-prod")
if !live.RequiresApproval {
t.Errorf("flip-off attempt mutated live profile (RequiresApproval = false) — loophole NOT closed")
}
if len(gate.requests) != 1 {
t.Fatalf("gate not called for flip-off attempt: %d requests", len(gate.requests))
}
// Attempt 2 — admin tries to mutate other fields (RequiresApproval still true).
keptOn := domain.CertificateProfile{
ID: "prof-prod",
Name: "renamed",
RequiresApproval: true,
}
_, err = svc.UpdateProfile(context.Background(), "prof-prod", keptOn)
if !errors.Is(err, ErrProfileEditPendingApproval) {
t.Errorf("kept-on attempt err = %v, want ErrProfileEditPendingApproval", err)
}
live2, _ := repo.Get(context.Background(), "prof-prod")
if live2.Name == "renamed" {
t.Errorf("kept-on attempt mutated profile name without approval — loophole NOT closed")
}
// Attempt 3 — admin tries to flip a NON-approval profile to approval-tier.
repo.rows["prof-staging"] = &domain.CertificateProfile{
ID: "prof-staging",
Name: "staging",
RequiresApproval: false,
}
flippedOn := domain.CertificateProfile{
ID: "prof-staging",
Name: "staging",
RequiresApproval: true, // operator wants to enable approvals
}
_, err = svc.UpdateProfile(context.Background(), "prof-staging", flippedOn)
if !errors.Is(err, ErrProfileEditPendingApproval) {
t.Errorf("flip-on attempt err = %v, want ErrProfileEditPendingApproval (gate fires when target state is approval-tier)", err)
}
live3, _ := repo.Get(context.Background(), "prof-staging")
if live3.RequiresApproval {
t.Errorf("flip-on attempt enabled approval without an approval — gate must fire BEFORE the persistence")
}
if len(gate.requests) != 3 {
t.Errorf("gate request count = %d, want 3 (one per attempt)", len(gate.requests))
}
}
// TestProfileEdit_NonApprovalProfileApplyDirectly confirms the gate
// is dormant for profiles that have RequiresApproval=false AND the
// edit doesn't flip it on. Pre-Phase-9 behaviour preserved.
func TestProfileEdit_NonApprovalProfileApplyDirectly(t *testing.T) {
repo := newFakeProfileRepo()
repo.rows["prof-dev"] = &domain.CertificateProfile{
ID: "prof-dev",
Name: "development",
RequiresApproval: false,
}
gate := &fakeApprovalGate{}
svc := NewProfileService(repo, nil)
svc.SetApprovalService(gate)
updated := domain.CertificateProfile{
ID: "prof-dev",
Name: "development-renamed",
RequiresApproval: false,
}
got, err := svc.UpdateProfile(context.Background(), "prof-dev", updated)
if err != nil {
t.Fatalf("non-approval update err = %v", err)
}
if got.Name != "development-renamed" {
t.Errorf("name not updated; got %q", got.Name)
}
if len(gate.requests) != 0 {
t.Errorf("gate fired for non-approval profile: %d requests", len(gate.requests))
}
}
// TestProfileEdit_NilApprovalService_PreservesLegacyBehaviour confirms
// that a nil-ApprovalService wiring (test fixtures, alternate boot
// paths) preserves the pre-Phase-9 direct-apply path even on
// approval-tier profiles. The gate is opt-in.
func TestProfileEdit_NilApprovalService_PreservesLegacyBehaviour(t *testing.T) {
repo := newFakeProfileRepo()
repo.rows["prof-prod"] = &domain.CertificateProfile{
ID: "prof-prod",
Name: "production",
RequiresApproval: true,
}
svc := NewProfileService(repo, nil) // approvalService not wired
updated := domain.CertificateProfile{
ID: "prof-prod",
Name: "renamed",
RequiresApproval: true,
}
if _, err := svc.UpdateProfile(context.Background(), "prof-prod", updated); err != nil {
t.Fatalf("nil-gate err = %v", err)
}
live, _ := repo.Get(context.Background(), "prof-prod")
if live.Name != "renamed" {
t.Errorf("nil-gate did not fall through to direct apply; got %q", live.Name)
}
}