diff --git a/cmd/server/main.go b/cmd/server/main.go index 24078cf..f5e014e 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -5,6 +5,7 @@ import ( "crypto" "crypto/tls" "crypto/x509" + "encoding/json" "encoding/pem" "fmt" "log/slog" @@ -559,6 +560,36 @@ func main() { defer issuerRegistry.StopLifecycles() targetService := service.NewTargetService(targetRepo, auditService, agentRepo, encryptionKey, logger) profileService := service.NewProfileService(profileRepo, auditService) + // Bundle 1 Phase 9 — approval-bypass closure. Wire the profile + // service's gate to the existing ApprovalService so edits to a + // RequiresApproval=true profile route through the four-eyes + // workflow. The profile-edit-apply callback registered on the + // ApprovalService closes the loop: when an approver decides, + // the callback deserializes req.Payload and persists the diff. + profileService.SetApprovalService(approvalService) + approvalService.SetProfileEditApply(func(ctx context.Context, req *domain.ApprovalRequest) error { + var pendingProfile domain.CertificateProfile + if err := json.Unmarshal(req.Payload, &pendingProfile); err != nil { + return fmt.Errorf("decode profile-edit payload: %w", err) + } + pendingProfile.ID = req.ProfileID + if err := profileRepo.Update(ctx, &pendingProfile); err != nil { + return fmt.Errorf("apply profile-edit diff: %w", err) + } + // Audit row category=auth so the auditor surface keeps the + // approval-decision history grouped with the request side. + if auditService != nil { + _ = auditService.RecordEventWithCategory(ctx, "approval-system", + domain.ActorTypeSystem, "profile.edit_applied", + domain.EventCategoryAuth, "certificate_profile", + req.ProfileID, + map[string]interface{}{ + "approval_id": req.ID, + "requested_by": req.RequestedBy, + }) + } + return nil + }) teamService := service.NewTeamService(teamRepo, auditService) ownerService := service.NewOwnerService(ownerRepo, auditService) agentGroupRepo := postgres.NewAgentGroupRepository(db) diff --git a/docs/reference/profiles.md b/docs/reference/profiles.md new file mode 100644 index 0000000..2b865db --- /dev/null +++ b/docs/reference/profiles.md @@ -0,0 +1,113 @@ +# Certificate profiles + +Last reviewed: 2026-05-09 + +A `CertificateProfile` is the policy object that groups every cert with +the same shape: which issuer mints it, which key algorithm + size are +allowed, what EKUs and SANs the issuer should emit, what renewal +window the scheduler uses, what targets get the cert deployed to. Every +managed certificate references exactly one profile; changing a +profile's policy retroactively affects renewal of every cert pointing +at it. + +This file documents the profile lifecycle as it stands after Bundle 1. +For the schema, see `migrations/000003_certificate_profiles.up.sql` + +`migrations/000027_approval_workflow.up.sql` + +`migrations/000033_approval_kinds.up.sql`. For the API surface, +see `api/openapi.yaml` under `/api/v1/profiles`. + +## Anatomy + +| Field | Default | Purpose | +|---|---|---| +| `id` | autogenerated `prof-` | Stable opaque identifier; used by every other resource. | +| `name` | required | Human-readable label; rendered in the GUI's profile picker. | +| `issuer_id` | required | Which issuer (Local / Vault / EJBCA / ACME / SCEP / EST / ADCS / etc.) mints certs against this profile. | +| `default_validity_days` | 90 | Rendered into the issuer call as the requested NotAfter delta. | +| `renewal_window_days` | 30 | Scheduler enqueues a renewal Job when `cert.NotAfter - now < renewal_window_days`. | +| `allowed_key_algorithms` | RSA 2048+, ECDSA P-256+ | Validates incoming CSRs at issuance time. | +| `allowed_ekus` | server, client | RFC 5280 §4.2.1.12 EKU set. | +| `must_staple` | false | Per-profile RFC 7633 `id-pe-tlsfeature` extension toggle (Phase 5.6 of the SCEP master bundle). | +| `requires_approval` | false | Bundle 1 Phase 9 — gates issuance + renewal AND profile edits behind a four-eyes approval workflow. See below. | + +## RequiresApproval and the approval workflow + +Setting `requires_approval=true` on a profile does two things: + +1. **Issuance + renewal of every cert pointing at the profile gates + on a non-requester admin's approval.** The scheduler enqueues a + `Job` at status `AwaitingApproval`; the linked + `issuance_approval_requests` row stays at `pending` until either + approved (job → `Pending`, scheduler dispatches) or rejected (job + → `Cancelled`). Same actor cannot self-approve. +2. **Edits to the profile itself gate on a non-requester admin's + approval.** This is the Bundle 1 Phase 9 closure for the flip-flop + loophole — without it an admin could set `requires_approval=false`, + mutate any other field, set `requires_approval=true`, and the + approval workflow would only have been bypassed during the + "off" window. The Phase 9 gate fires under three conditions: + - The live profile has `requires_approval=true` AND the operator + submits any edit (regardless of whether the edit changes the + flag). + - The live profile has `requires_approval=false` AND the operator + submits an edit that would set it to `true` (the flag-flip + direction is gated too because otherwise the gate could be + enabled by anyone and have no review). + - Both arms route through `ApprovalService.RequestProfileEditApproval` + which writes a row to `issuance_approval_requests` with + `approval_kind=profile_edit`. The pending profile diff is + serialized to `payload` (JSONB). + +**Edit response shape.** When the gate fires, `PUT /api/v1/profiles/{id}` +returns HTTP 202 Accepted with body +`{"status":"pending_approval","pending_approval_id":"ar-…"}`. +The operator copies the approval ID, hands it to a peer admin, and +the peer POSTs `/api/v1/approvals/{id}/approve` with their own +credentials. On approve, the server deserializes `payload`, applies +the diff against the live profile, and emits a +`profile.edit_applied` audit row with `event_category=auth`. On +reject, the pending row is dropped; the live profile is unchanged. + +**Same-actor self-approve is rejected** with HTTP 403 and the existing +`ErrApproveBySameActor` sentinel. This is the load-bearing +two-person-integrity invariant that satisfies SOC 2 CC6.3 + NIST +SSDF PO.5.2. + +**Bypass mode.** `CERTCTL_APPROVAL_BYPASS=true` short-circuits both +issuance approvals and profile-edit approvals; every request +auto-approves with `actor=system-bypass`. Used by dev / CI for fast +iteration; production deploys MUST leave it unset. A single SQL +query (`SELECT FROM audit_events WHERE actor='system-bypass'`) +confirms zero rows. + +## Operator workflows + +**Enable approval for an existing profile.** Edit the profile, set +`requires_approval=true`. The first time you do this, the edit +itself is gated (the live profile is non-approval but the proposed +state is approval-tier, so the flip-on direction still routes through +the workflow). Hand the approval ID to a peer; once approved, every +subsequent edit and every renewal of every cert pointing at the +profile gates on the workflow. + +**Disable approval.** Edit the profile, set `requires_approval=false`. +This edit is gated because the live profile is currently +approval-tier. A peer must approve the disable. Once disabled, +subsequent edits flow through the direct-apply path again. + +**Audit who approved what.** The audit trail records every approval +request + decision under `event_category=auth`. Filter via +`GET /api/v1/audit?category=auth` or the `auditor` role's +audit-only view. Each row carries the approval ID + the requester ++ the decider; the WORM trigger prevents tampering. + +## Related + +- `migrations/000027_approval_workflow.up.sql` (initial approval + schema, Rank 7 of the 2026-05-03 deep-research deliverable) +- `migrations/000033_approval_kinds.up.sql` (Phase 9 — adds + `approval_kind` + `payload` + nullable cert/job FKs) +- `internal/service/approval.go::RequestProfileEditApproval` +- `internal/service/profile.go::UpdateProfile` (gate) +- `internal/api/handler/profiles.go::UpdateProfile` (202 mapping) +- `cowork/auth-bundle-1-prompt.md` (Phase 9 spec) diff --git a/internal/api/handler/profiles.go b/internal/api/handler/profiles.go index 3be8c15..4b8f8dd 100644 --- a/internal/api/handler/profiles.go +++ b/internal/api/handler/profiles.go @@ -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 diff --git a/internal/domain/approval.go b/internal/domain/approval.go index f66382f..b1a1b63 100644 --- a/internal/domain/approval.go +++ b/internal/domain/approval.go @@ -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- - 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- + 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. diff --git a/internal/repository/postgres/approval.go b/internal/repository/postgres/approval.go index 775b9cd..02fca6a 100644 --- a/internal/repository/postgres/approval.go +++ b/internal/repository/postgres/approval.go @@ -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 diff --git a/internal/service/approval.go b/internal/service/approval.go index d12a994..f36d6d6 100644 --- a/internal/service/approval.go +++ b/internal/service/approval.go @@ -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. diff --git a/internal/service/approval_test.go b/internal/service/approval_test.go index 2f18159..802f317 100644 --- a/internal/service/approval_test.go +++ b/internal/service/approval_test.go @@ -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) + } +} diff --git a/internal/service/profile.go b/internal/service/profile.go index b71591d..f11889a 100644 --- a/internal/service/profile.go +++ b/internal/service/profile.go @@ -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 { diff --git a/internal/service/profile_approval_test.go b/internal/service/profile_approval_test.go new file mode 100644 index 0000000..cbc796a --- /dev/null +++ b/internal/service/profile_approval_test.go @@ -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) + } +} diff --git a/migrations/000033_approval_kinds.down.sql b/migrations/000033_approval_kinds.down.sql new file mode 100644 index 0000000..79f02f7 --- /dev/null +++ b/migrations/000033_approval_kinds.down.sql @@ -0,0 +1,12 @@ +-- Bundle 1 Phase 9 down: drop the kind/payload columns + constraints. +-- Destructive — any pending profile-edit approval rows are lost. +BEGIN; +DROP INDEX IF EXISTS idx_approval_kind; +ALTER TABLE issuance_approval_requests DROP CONSTRAINT IF EXISTS approval_kind_consistency; +ALTER TABLE issuance_approval_requests DROP CONSTRAINT IF EXISTS approval_kind_check; +ALTER TABLE issuance_approval_requests DROP COLUMN IF EXISTS payload; +ALTER TABLE issuance_approval_requests DROP COLUMN IF EXISTS approval_kind; +-- Down-migration intentionally does NOT restore NOT NULL on cert_id +-- and job_id even though the up-migration relaxed them — old data +-- might already include profile_edit rows that violate it. +COMMIT; diff --git a/migrations/000033_approval_kinds.up.sql b/migrations/000033_approval_kinds.up.sql new file mode 100644 index 0000000..7ee6f83 --- /dev/null +++ b/migrations/000033_approval_kinds.up.sql @@ -0,0 +1,87 @@ +-- Bundle 1 Phase 9 — approval kinds (Decision 9, option a). +-- +-- Closes the flip-flop loophole: an admin can NO LONGER flip a +-- profile's RequiresApproval=false → mutate → flip back. Profile +-- edits to a profile that has (or would have) RequiresApproval=true +-- now route through ApprovalService just like cert issuance. +-- +-- Schema changes: +-- +-- 1. New `approval_kind` column (cert_issuance | profile_edit). +-- Default cert_issuance preserves back-compat for every existing +-- row created by Phase 7 of the 2026-05-03 deep-research bundle. +-- +-- 2. `certificate_id` and `job_id` become nullable so profile-edit +-- approvals (no associated cert / job) can share the table. +-- The CHECK constraint below pins per-kind nullability so +-- cert_issuance rows must have both, profile_edit rows must +-- have neither and instead carry payload. +-- +-- 3. New `payload` JSONB column captures the pending profile diff +-- for profile_edit approvals. The approver's POST +-- /v1/approvals/{id}/approve triggers the diff to be applied +-- against the live profile row. +-- +-- Idempotent throughout via IF NOT EXISTS / DO blocks. + +BEGIN; + +ALTER TABLE issuance_approval_requests + ADD COLUMN IF NOT EXISTS approval_kind TEXT NOT NULL DEFAULT 'cert_issuance'; + +ALTER TABLE issuance_approval_requests + ADD COLUMN IF NOT EXISTS payload JSONB; + +-- Drop NOT NULL on cert_id + job_id so profile_edit rows can omit +-- both. The CHECK below restores per-kind invariants. Idempotent +-- via DO block (Postgres doesn't expose ALTER COLUMN ... IF NOT +-- NULL natively). +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM information_schema.columns + WHERE table_name = 'issuance_approval_requests' + AND column_name = 'certificate_id' + AND is_nullable = 'NO') THEN + ALTER TABLE issuance_approval_requests + ALTER COLUMN certificate_id DROP NOT NULL; + END IF; + IF EXISTS (SELECT 1 FROM information_schema.columns + WHERE table_name = 'issuance_approval_requests' + AND column_name = 'job_id' + AND is_nullable = 'NO') THEN + ALTER TABLE issuance_approval_requests + ALTER COLUMN job_id DROP NOT NULL; + END IF; +END$$; + +-- Per-kind invariant. cert_issuance rows must have cert_id + job_id. +-- profile_edit rows must have payload (the pending diff). +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_constraint + WHERE conname = 'approval_kind_consistency') THEN + ALTER TABLE issuance_approval_requests + ADD CONSTRAINT approval_kind_consistency CHECK ( + (approval_kind = 'cert_issuance' + AND certificate_id IS NOT NULL AND job_id IS NOT NULL) + OR (approval_kind = 'profile_edit' + AND payload IS NOT NULL) + ); + END IF; +END$$; + +DO $$ +BEGIN + IF NOT EXISTS (SELECT 1 FROM pg_constraint + WHERE conname = 'approval_kind_check') THEN + ALTER TABLE issuance_approval_requests + ADD CONSTRAINT approval_kind_check CHECK ( + approval_kind IN ('cert_issuance', 'profile_edit') + ); + END IF; +END$$; + +CREATE INDEX IF NOT EXISTS idx_approval_kind + ON issuance_approval_requests(approval_kind); + +COMMIT; diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 9503390..e80c44c 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -103,6 +103,126 @@ export const checkAuth = (key: string) => return r.json() as Promise; }); +// ============================================================================= +// Bundle 1 Phase 10 — RBAC management API surface. +// +// Backs the Roles / Keys / Auth Settings GUI pages (web/src/pages/auth/*). +// Every function maps 1:1 to a Phase-4 / Phase-7 server endpoint; +// permission gates fire server-side, the GUI's permission-aware +// renders are a UX layer on top. +// ============================================================================= + +export interface AuthRole { + id: string; + tenant_id: string; + name: string; + description?: string; + created_at?: string; + updated_at?: string; +} + +export interface AuthRolePermission { + role_id: string; + permission_id: string; + scope_type: 'global' | 'profile' | 'issuer'; + scope_id?: string; +} + +export interface AuthPermission { + id: string; + name: string; + namespace: string; +} + +export interface AuthEffectivePermission { + permission: string; + scope_type: 'global' | 'profile' | 'issuer'; + scope_id?: string; +} + +export interface AuthMeResponse { + actor_id: string; + actor_type: string; + tenant_id: string; + admin: boolean; + roles: string[]; + effective_permissions: AuthEffectivePermission[]; +} + +export interface AuthKeyEntry { + actor_id: string; + actor_type: string; + tenant_id: string; + role_ids: string[]; +} + +export const authMe = () => fetchJSON(`${BASE}/auth/me`); + +export const authListRoles = () => + fetchJSON<{ roles: AuthRole[] }>(`${BASE}/auth/roles`).then(r => r.roles); + +export const authGetRole = (id: string) => + fetchJSON<{ role: AuthRole; permissions: AuthRolePermission[] }>( + `${BASE}/auth/roles/${id}`, + ); + +export const authCreateRole = (body: { name: string; description?: string }) => + fetchJSON(`${BASE}/auth/roles`, { + method: 'POST', + body: JSON.stringify(body), + }); + +export const authUpdateRole = (id: string, body: { name: string; description?: string }) => + fetchJSON(`${BASE}/auth/roles/${id}`, { + method: 'PUT', + body: JSON.stringify(body), + }); + +export const authDeleteRole = (id: string) => + fetchJSON(`${BASE}/auth/roles/${id}`, { method: 'DELETE' }); + +export const authListPermissions = () => + fetchJSON<{ permissions: AuthPermission[] }>(`${BASE}/auth/permissions`).then( + r => r.permissions, + ); + +export const authAddRolePermission = ( + roleId: string, + body: { permission: string; scope_type?: string; scope_id?: string }, +) => + fetchJSON(`${BASE}/auth/roles/${roleId}/permissions`, { + method: 'POST', + body: JSON.stringify(body), + }); + +export const authRemoveRolePermission = (roleId: string, perm: string) => + fetchJSON(`${BASE}/auth/roles/${roleId}/permissions/${perm}`, { + method: 'DELETE', + }); + +export const authListKeys = () => + fetchJSON<{ keys: AuthKeyEntry[] }>(`${BASE}/auth/keys`).then(r => r.keys); + +export const authAssignKeyRole = (keyId: string, roleId: string) => + fetchJSON(`${BASE}/auth/keys/${keyId}/roles`, { + method: 'POST', + body: JSON.stringify({ role_id: roleId }), + }); + +export const authRevokeKeyRole = (keyId: string, roleId: string) => + fetchJSON(`${BASE}/auth/keys/${keyId}/roles/${roleId}`, { + method: 'DELETE', + }); + +export interface BootstrapAvailability { + available: boolean; +} + +export const authBootstrapAvailable = () => + fetch(`${BASE}/auth/bootstrap`, { + headers: { 'Content-Type': 'application/json' }, + }).then(r => r.json() as Promise); + // Certificates export const getCertificates = (params: Record = {}) => { const qs = new URLSearchParams({ page: '1', per_page: '50', ...params }).toString(); diff --git a/web/src/components/Layout.tsx b/web/src/components/Layout.tsx index 9363ad5..3ee757b 100644 --- a/web/src/components/Layout.tsx +++ b/web/src/components/Layout.tsx @@ -26,6 +26,10 @@ const nav = [ { to: '/scep', label: 'SCEP Admin', icon: 'M12 15v2m-6 4h12a2 2 0 002-2v-6a2 2 0 00-2-2H6a2 2 0 00-2 2v6a2 2 0 002 2zm10-10V7a4 4 0 00-8 0v4h8z' }, { to: '/est', label: 'EST Admin', icon: 'M9 12l2 2 4-4m5.618-4.016A11.955 11.955 0 0112 2.944a11.955 11.955 0 01-8.618 3.04A12.02 12.02 0 003 9c0 5.591 3.824 10.29 9 11.622 5.176-1.332 9-6.03 9-11.622 0-1.042-.133-2.052-.382-3.016z' }, { to: '/audit', label: 'Audit Trail', icon: 'M12 8v4l3 3m6-3a9 9 0 11-18 0 9 9 0 0118 0z' }, + // Bundle 1 Phase 10 — RBAC management (Roles / Keys / Settings). + { to: '/auth/roles', label: 'Roles', icon: 'M16 7a4 4 0 11-8 0 4 4 0 018 0zM12 14a7 7 0 00-7 7h14a7 7 0 00-7-7z' }, + { to: '/auth/keys', label: 'API Keys', icon: 'M15 7a2 2 0 012 2m4 0a6 6 0 01-7.743 5.743L11 17H9v2H7v2H4a1 1 0 01-1-1v-2.586a1 1 0 01.293-.707l5.964-5.964A6 6 0 1121 9z' }, + { to: '/auth/settings', label: 'Auth Settings', icon: 'M10.325 4.317c.426-1.756 2.924-1.756 3.35 0a1.724 1.724 0 002.573 1.066c1.543-.94 3.31.826 2.37 2.37a1.724 1.724 0 001.066 2.573c1.756.426 1.756 2.924 0 3.35a1.724 1.724 0 00-1.066 2.573c.94 1.543-.826 3.31-2.37 2.37a1.724 1.724 0 00-2.573 1.066c-.426 1.756-2.924 1.756-3.35 0a1.724 1.724 0 00-2.573-1.066c-1.543.94-3.31-.826-2.37-2.37a1.724 1.724 0 00-1.066-2.573c-1.756-.426-1.756-2.924 0-3.35a1.724 1.724 0 001.066-2.573c-.94-1.543.826-3.31 2.37-2.37.996.608 2.296.07 2.572-1.065z M15 12a3 3 0 11-6 0 3 3 0 016 0z' }, ]; function Icon({ d }: { d: string }) { diff --git a/web/src/hooks/useAuthMe.ts b/web/src/hooks/useAuthMe.ts new file mode 100644 index 0000000..0eab345 --- /dev/null +++ b/web/src/hooks/useAuthMe.ts @@ -0,0 +1,58 @@ +import { useQuery } from '@tanstack/react-query'; +import { authMe, type AuthMeResponse } from '../api/client'; + +// ============================================================================= +// Bundle 1 Phase 10 — `useAuthMe` is the GUI's single source of truth for +// "what can the current actor do?" Every Phase-10 auth page (Roles, +// Keys, Auth Settings, Audit category filter) consumes this hook on +// mount + caches via TanStack Query, so toggling between pages doesn't +// re-fetch the permission set every navigation. +// +// The hook returns three things: +// +// - data: the raw AuthMeResponse from /v1/auth/me (or undefined while +// loading / on error). +// - hasPerm(p): predicate the caller uses to gate buttons / links. +// Reads the cached effective_permissions slice. +// - isLoading + error: standard TanStack Query surface. +// +// The permission check is intentionally a string-equality match against +// the canonical permission names. Scope semantics (global / profile / +// issuer) are NOT applied client-side — the server is the load-bearing +// gate. The client uses hasPerm purely for "show or hide the button" +// UX; the server returns 403 if a missing perm gets through anyway. +// ============================================================================= + +const STALE_TIME_MS = 60_000; + +export function useAuthMe() { + const query = useQuery({ + queryKey: ['auth', 'me'], + queryFn: authMe, + staleTime: STALE_TIME_MS, + retry: 0, + }); + + const hasPerm = (perm: string): boolean => { + if (!query.data) return false; + return query.data.effective_permissions.some(p => p.permission === perm); + }; + + const hasAnyPerm = (perms: string[]): boolean => { + if (!query.data) return false; + return perms.some(p => hasPerm(p)); + }; + + const isAdmin = (): boolean => { + return Boolean(query.data?.roles?.includes('r-admin') || query.data?.admin); + }; + + return { + data: query.data, + isLoading: query.isLoading, + error: query.error, + hasPerm, + hasAnyPerm, + isAdmin, + }; +} diff --git a/web/src/main.tsx b/web/src/main.tsx index 2341c36..9e9cbee 100644 --- a/web/src/main.tsx +++ b/web/src/main.tsx @@ -35,6 +35,11 @@ import IssuerHierarchyPage from './pages/IssuerHierarchyPage'; import TargetDetailPage from './pages/TargetDetailPage'; import SCEPAdminPage from './pages/SCEPAdminPage'; import ESTAdminPage from './pages/ESTAdminPage'; +// Bundle 1 Phase 10 — RBAC management pages. +import RolesPage from './pages/auth/RolesPage'; +import RoleDetailPage from './pages/auth/RoleDetailPage'; +import KeysPage from './pages/auth/KeysPage'; +import AuthSettingsPage from './pages/auth/AuthSettingsPage'; import './index.css'; const queryClient = new QueryClient({ @@ -105,6 +110,16 @@ createRoot(document.getElementById('root')!).render( required" banner for non-admin callers and skips the underlying API calls so the server never sees a 403. */} } /> + {/* Bundle 1 Phase 10 — RBAC management surface. + Every page reads /api/v1/auth/me on mount via the + useAuthMe hook and gates affordances against the + cached effective_permissions slice. Server-side + enforcement is the load-bearing layer; client-side + hide/disable is UX. */} + } /> + } /> + } /> + } /> diff --git a/web/src/pages/AuditPage.test.tsx b/web/src/pages/AuditPage.test.tsx index e2653ee..5c5da39 100644 --- a/web/src/pages/AuditPage.test.tsx +++ b/web/src/pages/AuditPage.test.tsx @@ -102,3 +102,28 @@ describe('AuditPage — render + XSS hardening (M-026 / M-029 Pass 3)', () => { }); }); }); + +// ============================================================================= +// Bundle 1 Phase 10 — category filter render test. Pins that the +// new setCategory(e.target.value)} + className="bg-surface border border-surface-border rounded px-3 py-1.5 text-xs text-ink focus:outline-none focus:border-brand-400" + data-testid="audit-category-filter" + > + {CATEGORIES.map((c) => ( + + ))} + setRoleID(e.target.value)} + className="w-full bg-white border border-surface-border rounded px-3 py-2 text-sm" + required + data-testid="assign-role-select" + > + + {roles + .filter(r => !actor.role_ids.includes(r.id)) + .map(r => ( + + ))} + +
+ + +
+ + + + ); +} diff --git a/web/src/pages/auth/RoleDetailPage.tsx b/web/src/pages/auth/RoleDetailPage.tsx new file mode 100644 index 0000000..9eca943 --- /dev/null +++ b/web/src/pages/auth/RoleDetailPage.tsx @@ -0,0 +1,341 @@ +import { useState } from 'react'; +import { Link, useParams, useNavigate } from 'react-router-dom'; +import { useQuery, useQueryClient } from '@tanstack/react-query'; +import { + authGetRole, + authListPermissions, + authUpdateRole, + authDeleteRole, + authAddRolePermission, + authRemoveRolePermission, + type AuthPermission, +} from '../../api/client'; +import { useAuthMe } from '../../hooks/useAuthMe'; +import PageHeader from '../../components/PageHeader'; +import ErrorState from '../../components/ErrorState'; + +// ============================================================================= +// Bundle 1 Phase 10 — RoleDetailPage. +// +// Shows a single role plus its current permission grants. Surfaces: +// +// - Edit role modal (auth.role.edit) +// - Delete role action (auth.role.delete) — disabled when actors hold +// the role (server returns 409; UX surfaces via ErrorState). +// - Add permission picker (auth.role.edit) populated from the +// canonical catalogue. +// - Remove permission action per row (auth.role.edit). +// +// Each action is HIDDEN when the caller lacks the permission. The +// server still 403s an end-run; client-side hide is UX, not security. +// ============================================================================= + +export default function RoleDetailPage() { + const { id = '' } = useParams<{ id: string }>(); + const me = useAuthMe(); + const qc = useQueryClient(); + const navigate = useNavigate(); + + const detailQuery = useQuery({ + queryKey: ['auth', 'role', id], + queryFn: () => authGetRole(id), + enabled: Boolean(id), + staleTime: 30_000, + }); + const permsCatalogue = useQuery({ + queryKey: ['auth', 'permissions'], + queryFn: authListPermissions, + staleTime: 5 * 60_000, + }); + + const [editOpen, setEditOpen] = useState(false); + const [submitting, setSubmitting] = useState(false); + const [actionError, setActionError] = useState(null); + + const canEdit = me.hasPerm('auth.role.edit') || me.isAdmin(); + const canDelete = me.hasPerm('auth.role.delete') || me.isAdmin(); + + if (detailQuery.isLoading) return ; + if (detailQuery.error || !detailQuery.data) + return ( +
+ + qc.invalidateQueries({ queryKey: ['auth', 'role', id] })} + /> +
+ ); + + const { role, permissions } = detailQuery.data; + + const handleDelete = async () => { + if (!window.confirm(`Delete role ${role.name}? This cannot be undone.`)) return; + setSubmitting(true); + setActionError(null); + try { + await authDeleteRole(role.id); + navigate('/auth/roles'); + } catch (err) { + setActionError(err instanceof Error ? err.message : String(err)); + } finally { + setSubmitting(false); + } + }; + + const handleAddPermission = async (perm: string) => { + setSubmitting(true); + setActionError(null); + try { + await authAddRolePermission(role.id, { permission: perm }); + qc.invalidateQueries({ queryKey: ['auth', 'role', role.id] }); + } catch (err) { + setActionError(err instanceof Error ? err.message : String(err)); + } finally { + setSubmitting(false); + } + }; + + const handleRemovePermission = async (perm: string) => { + setSubmitting(true); + setActionError(null); + try { + await authRemoveRolePermission(role.id, perm); + qc.invalidateQueries({ queryKey: ['auth', 'role', role.id] }); + } catch (err) { + setActionError(err instanceof Error ? err.message : String(err)); + } finally { + setSubmitting(false); + } + }; + + const grantedPermNames = new Set(permissions.map(p => p.permission_id)); + const availablePerms = (permsCatalogue.data ?? []).filter(p => !grantedPermNames.has(p.name)); + + return ( +
+ + + Back + + {canEdit && ( + + )} + {canDelete && ( + + )} +
+ } + /> + {actionError && ( +
+ {actionError} +
+ )} +
+
Description
+
{role.description || (none)}
+
+ +
+
+
+
Permissions ({permissions.length})
+
+ Permissions granted at the listed scope. Global wins over more-specific scopes. +
+
+ {canEdit && availablePerms.length > 0 && ( + + )} +
+ {permissions.length === 0 ? ( +
+ No permissions granted. {canEdit ? 'Use the picker above to add some.' : ''} +
+ ) : ( + + + + + + {canEdit && } + + + + {permissions.map(p => { + const permName = lookupPermNameByID(permsCatalogue.data ?? [], p.permission_id); + return ( + + + + {canEdit && ( + + )} + + ); + })} + +
PermissionScope
{permName} + {p.scope_type} + {p.scope_id ? ` (${p.scope_id})` : ''} + + +
+ )} +
+ + {editOpen && ( + setEditOpen(false)} + onSuccess={() => { + setEditOpen(false); + qc.invalidateQueries({ queryKey: ['auth', 'role', role.id] }); + qc.invalidateQueries({ queryKey: ['auth', 'roles'] }); + }} + /> + )} + + ); +} + +function lookupPermNameByID(catalogue: AuthPermission[], id: string): string { + // The role-permissions response uses permission_id which the server + // populates as the canonical permission NAME (the schema treats + // permission name as the row id surrogate). Belt-and-braces + // fallback: if the catalogue knows the id, return its display name. + const m = catalogue.find(p => p.id === id || p.name === id); + return m?.name ?? id; +} + +interface EditModalProps { + roleId: string; + initialName: string; + initialDescription: string; + onClose: () => void; + onSuccess: () => void; +} + +function EditRoleModal({ roleId, initialName, initialDescription, onClose, onSuccess }: EditModalProps) { + const [name, setName] = useState(initialName); + const [description, setDescription] = useState(initialDescription); + const [submitting, setSubmitting] = useState(false); + const [error, setError] = useState(null); + const dirty = name !== initialName || description !== initialDescription; + + const handleClose = () => { + if (dirty && !window.confirm('Discard unsaved changes?')) return; + onClose(); + }; + + const handleSubmit = async (e: React.FormEvent) => { + e.preventDefault(); + setSubmitting(true); + setError(null); + try { + await authUpdateRole(roleId, { name: name.trim(), description: description.trim() }); + onSuccess(); + } catch (err) { + setError(err instanceof Error ? err.message : String(err)); + } finally { + setSubmitting(false); + } + }; + + return ( +
+
e.stopPropagation()} + data-testid="edit-role-modal" + > +

Edit role

+ {error && ( +
{error}
+ )} +
+
+ + setName(e.target.value)} + className="w-full bg-white border border-surface-border rounded px-3 py-2 text-sm" + required + data-testid="edit-role-name" + /> +
+
+ +