diff --git a/internal/api/handler/auth.go b/internal/api/handler/auth.go index 878ad76..4697ae6 100644 --- a/internal/api/handler/auth.go +++ b/internal/api/handler/auth.go @@ -6,6 +6,7 @@ import ( "errors" "net/http" "strings" + "time" "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/domain" @@ -174,8 +175,26 @@ type addPermissionRequest struct { ScopeID *string `json:"scope_id,omitempty"` } +// assignRoleRequest is the POST /api/v1/auth/keys/{id}/roles body. +// +// Audit 2026-05-10 HIGH-10 closure — extended with scope_type / +// scope_id / expires_at so per-actor scoped + time-bound grants are +// expressible via the API. Pre-fix, the only path was creating a +// scoped role and granting that; now operators can scope a standing +// role to a specific resource on a per-actor basis. +// +// Validation rules: +// - role_id is required. +// - scope_type defaults to "global"; allowed values are global / +// profile / issuer. +// - scope_id is required when scope_type != "global"; rejected +// (must be empty) when scope_type == "global". +// - expires_at must be in the future when present; nil = standing. type assignRoleRequest struct { - RoleID string `json:"role_id"` + RoleID string `json:"role_id"` + ScopeType string `json:"scope_type,omitempty"` + ScopeID *string `json:"scope_id,omitempty"` + ExpiresAt *time.Time `json:"expires_at,omitempty"` } type meResponse struct { @@ -427,10 +446,39 @@ func (h AuthHandler) AssignRoleToKey(w http.ResponseWriter, r *http.Request) { Error(w, http.StatusBadRequest, "role_id is required") return } + + // Audit 2026-05-10 HIGH-10 validation. + scopeType := authdomain.ScopeType(req.ScopeType) + if scopeType == "" { + scopeType = authdomain.ScopeTypeGlobal + } + switch scopeType { + case authdomain.ScopeTypeGlobal: + if req.ScopeID != nil && *req.ScopeID != "" { + Error(w, http.StatusBadRequest, "scope_id must be empty when scope_type=global") + return + } + case authdomain.ScopeTypeProfile, authdomain.ScopeTypeIssuer: + if req.ScopeID == nil || strings.TrimSpace(*req.ScopeID) == "" { + Error(w, http.StatusBadRequest, "scope_id is required when scope_type is profile or issuer") + return + } + default: + Error(w, http.StatusBadRequest, "invalid scope_type — must be global, profile, or issuer") + return + } + if req.ExpiresAt != nil && !req.ExpiresAt.After(time.Now().UTC()) { + Error(w, http.StatusBadRequest, "expires_at must be in the future") + return + } + ar := &authdomain.ActorRole{ ActorID: keyID, ActorType: authdomain.ActorTypeValue(domain.ActorTypeAPIKey), RoleID: req.RoleID, + ScopeType: scopeType, + ScopeID: req.ScopeID, + ExpiresAt: req.ExpiresAt, } if err := h.actors.Grant(r.Context(), caller, ar); err != nil { writeAuthError(w, err) diff --git a/internal/api/handler/auth_test.go b/internal/api/handler/auth_test.go index 58cd910..b9aabbf 100644 --- a/internal/api/handler/auth_test.go +++ b/internal/api/handler/auth_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" "github.com/certctl-io/certctl/internal/auth" "github.com/certctl-io/certctl/internal/domain" @@ -304,6 +305,125 @@ func TestAuthHandler_AssignRoleToKey(t *testing.T) { } } +// Audit 2026-05-10 HIGH-10 regression matrix — pin the new +// scope_type / scope_id / expires_at fields on assignRoleRequest. +// Pre-fix, the request body accepted only `{role_id}` so per-actor +// scope-bound grants and time-bound grants weren't expressible via +// the API even though the schema reserved the columns. Post-fix, +// validation rules: +// +// - scope_type ∈ {global, profile, issuer}; defaults to global. +// - scope_id required when scope_type != global; rejected when +// scope_type == global. +// - expires_at must be in the future when present. +func TestAssignRoleToKey_HIGH10_ProfileScopeBoundGrantPersists(t *testing.T) { + h, _, _, actorSvc := newAuthHandlerWithFakes() + scopeID := "p-finance" + body, _ := json.Marshal(assignRoleRequest{ + RoleID: "r-operator", + ScopeType: "profile", + ScopeID: &scopeID, + }) + req := withAuthCtx(httptest.NewRequest(http.MethodPost, "/api/v1/auth/keys/alice/roles", bytes.NewReader(body)), "admin", auth.ActorTypeAPIKey) + req.SetPathValue("id", "alice") + rec := httptest.NewRecorder() + h.AssignRoleToKey(rec, req) + if rec.Code != http.StatusNoContent { + t.Fatalf("status = %d; body=%s", rec.Code, rec.Body.String()) + } + if len(actorSvc.roles) != 1 { + t.Fatalf("expected 1 grant; got %d", len(actorSvc.roles)) + } + if got := string(actorSvc.roles[0].ScopeType); got != "profile" { + t.Errorf("ScopeType = %q; want profile", got) + } + if actorSvc.roles[0].ScopeID == nil || *actorSvc.roles[0].ScopeID != "p-finance" { + t.Errorf("ScopeID = %v; want p-finance", actorSvc.roles[0].ScopeID) + } +} + +func TestAssignRoleToKey_HIGH10_TimeBoundGrantPersists(t *testing.T) { + h, _, _, actorSvc := newAuthHandlerWithFakes() + future := time.Now().Add(24 * time.Hour).UTC() + body, _ := json.Marshal(assignRoleRequest{ + RoleID: "r-operator", + ExpiresAt: &future, + }) + req := withAuthCtx(httptest.NewRequest(http.MethodPost, "/api/v1/auth/keys/alice/roles", bytes.NewReader(body)), "admin", auth.ActorTypeAPIKey) + req.SetPathValue("id", "alice") + rec := httptest.NewRecorder() + h.AssignRoleToKey(rec, req) + if rec.Code != http.StatusNoContent { + t.Fatalf("status = %d; body=%s", rec.Code, rec.Body.String()) + } + if len(actorSvc.roles) != 1 || actorSvc.roles[0].ExpiresAt == nil { + t.Fatalf("expected 1 grant with ExpiresAt; got %+v", actorSvc.roles) + } +} + +func TestAssignRoleToKey_HIGH10_RejectsScopeIDWithGlobalScope(t *testing.T) { + h, _, _, _ := newAuthHandlerWithFakes() + bad := "p-finance" + body, _ := json.Marshal(assignRoleRequest{ + RoleID: "r-operator", + ScopeType: "global", + ScopeID: &bad, + }) + req := withAuthCtx(httptest.NewRequest(http.MethodPost, "/api/v1/auth/keys/alice/roles", bytes.NewReader(body)), "admin", auth.ActorTypeAPIKey) + req.SetPathValue("id", "alice") + rec := httptest.NewRecorder() + h.AssignRoleToKey(rec, req) + if rec.Code != http.StatusBadRequest { + t.Errorf("scope_id with scope_type=global should be 400; got %d", rec.Code) + } +} + +func TestAssignRoleToKey_HIGH10_RejectsMissingScopeIDOnProfile(t *testing.T) { + h, _, _, _ := newAuthHandlerWithFakes() + body, _ := json.Marshal(assignRoleRequest{ + RoleID: "r-operator", + ScopeType: "profile", + }) + req := withAuthCtx(httptest.NewRequest(http.MethodPost, "/api/v1/auth/keys/alice/roles", bytes.NewReader(body)), "admin", auth.ActorTypeAPIKey) + req.SetPathValue("id", "alice") + rec := httptest.NewRecorder() + h.AssignRoleToKey(rec, req) + if rec.Code != http.StatusBadRequest { + t.Errorf("missing scope_id on scope_type=profile should be 400; got %d", rec.Code) + } +} + +func TestAssignRoleToKey_HIGH10_RejectsPastExpiry(t *testing.T) { + h, _, _, _ := newAuthHandlerWithFakes() + past := time.Now().Add(-1 * time.Hour).UTC() + body, _ := json.Marshal(assignRoleRequest{ + RoleID: "r-operator", + ExpiresAt: &past, + }) + req := withAuthCtx(httptest.NewRequest(http.MethodPost, "/api/v1/auth/keys/alice/roles", bytes.NewReader(body)), "admin", auth.ActorTypeAPIKey) + req.SetPathValue("id", "alice") + rec := httptest.NewRecorder() + h.AssignRoleToKey(rec, req) + if rec.Code != http.StatusBadRequest { + t.Errorf("past expires_at should be 400; got %d", rec.Code) + } +} + +func TestAssignRoleToKey_HIGH10_RejectsInvalidScopeType(t *testing.T) { + h, _, _, _ := newAuthHandlerWithFakes() + body, _ := json.Marshal(assignRoleRequest{ + RoleID: "r-operator", + ScopeType: "tenant", // not a valid scope_type + }) + req := withAuthCtx(httptest.NewRequest(http.MethodPost, "/api/v1/auth/keys/alice/roles", bytes.NewReader(body)), "admin", auth.ActorTypeAPIKey) + req.SetPathValue("id", "alice") + rec := httptest.NewRecorder() + h.AssignRoleToKey(rec, req) + if rec.Code != http.StatusBadRequest { + t.Errorf("invalid scope_type should be 400; got %d", rec.Code) + } +} + func TestAuthHandler_AssignRoleSelfRoleAssignReturns403(t *testing.T) { h, _, _, actorSvc := newAuthHandlerWithFakes() actorSvc.grantErr = errors.New("auth.role.assign required: " + authsvc.ErrSelfRoleAssignment.Error()) diff --git a/internal/domain/auth/types.go b/internal/domain/auth/types.go index 57b4010..5a92610 100644 --- a/internal/domain/auth/types.go +++ b/internal/domain/auth/types.go @@ -95,6 +95,19 @@ type ActorRole struct { ExpiresAt *time.Time `json:"expires_at,omitempty"` GrantedBy string `json:"granted_by"` TenantID string `json:"tenant_id"` + + // Audit 2026-05-10 HIGH-10 closure — per-actor scope override on + // the grant. Pre-fix, scope was per-role only; now operators can + // grant the standing r-operator role to Alice scoped to profile-X + // via (ScopeType="profile", ScopeID="p-X"). Authorizer.CheckPermission + // already understands the tuple via role_permissions. Migration + // 000043 ships the schema columns + uniqueness extension. + // + // ScopeType ∈ {global, profile, issuer}. Empty/missing defaults + // to "global" at the persistence layer (schema column DEFAULT). + // ScopeID is required when ScopeType != "global"; nil otherwise. + ScopeType ScopeType `json:"scope_type,omitempty"` + ScopeID *string `json:"scope_id,omitempty"` } // ActorTypeValue is the typed-string actor identifier used in diff --git a/internal/repository/postgres/auth.go b/internal/repository/postgres/auth.go index 7ef5090..8aa1713 100644 --- a/internal/repository/postgres/auth.go +++ b/internal/repository/postgres/auth.go @@ -377,11 +377,24 @@ func (r *ActorRoleRepository) Grant(ctx context.Context, ar *authdomain.ActorRol if ar.ExpiresAt != nil { expires = *ar.ExpiresAt } + // Audit 2026-05-10 HIGH-10 — per-actor scope columns. Default to + // "global"+NULL when the caller didn't supply them (back-compat + // with pre-migration code paths). Migration 000043's schema-level + // DEFAULT 'global' covers the same case; passing explicitly here + // makes the Go-level write deterministic. + scopeType := string(ar.ScopeType) + if scopeType == "" { + scopeType = string(authdomain.ScopeTypeGlobal) + } + var scopeID interface{} + if ar.ScopeID != nil && *ar.ScopeID != "" { + scopeID = *ar.ScopeID + } _, err := r.db.ExecContext(ctx, ` - INSERT INTO actor_roles (id, actor_id, actor_type, role_id, granted_at, expires_at, granted_by, tenant_id) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8) - ON CONFLICT (actor_id, actor_type, role_id, tenant_id) DO NOTHING - `, ar.ID, ar.ActorID, string(ar.ActorType), ar.RoleID, ar.GrantedAt, expires, ar.GrantedBy, ar.TenantID) + INSERT INTO actor_roles (id, actor_id, actor_type, role_id, granted_at, expires_at, granted_by, tenant_id, scope_type, scope_id) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) + ON CONFLICT (actor_id, actor_type, role_id, scope_type, scope_id, tenant_id) DO NOTHING + `, ar.ID, ar.ActorID, string(ar.ActorType), ar.RoleID, ar.GrantedAt, expires, ar.GrantedBy, ar.TenantID, scopeType, scopeID) if err != nil { return fmt.Errorf("actorRole.grant: %w", err) } diff --git a/migrations/000043_actor_role_scope.down.sql b/migrations/000043_actor_role_scope.down.sql new file mode 100644 index 0000000..acd66bc --- /dev/null +++ b/migrations/000043_actor_role_scope.down.sql @@ -0,0 +1,16 @@ +-- Rollback for 000043_actor_role_scope.up.sql +-- Note: TRUNCATE is destructive of any rows added with non-global scope. +-- That's acceptable for a rollback (forward-only design). +ALTER TABLE actor_roles + DROP CONSTRAINT IF EXISTS actor_roles_actor_role_scope_unique; +ALTER TABLE actor_roles + DROP CONSTRAINT IF EXISTS actor_roles_scope_id_required_when_not_global; +ALTER TABLE actor_roles + DROP CONSTRAINT IF EXISTS actor_roles_scope_type_enum; +DROP INDEX IF EXISTS idx_actor_roles_scope; +ALTER TABLE actor_roles + DROP COLUMN IF EXISTS scope_type, + DROP COLUMN IF EXISTS scope_id; +ALTER TABLE actor_roles + ADD CONSTRAINT actor_roles_actor_id_actor_type_role_id_tenant_id_key + UNIQUE (actor_id, actor_type, role_id, tenant_id); diff --git a/migrations/000043_actor_role_scope.up.sql b/migrations/000043_actor_role_scope.up.sql new file mode 100644 index 0000000..2022141 --- /dev/null +++ b/migrations/000043_actor_role_scope.up.sql @@ -0,0 +1,46 @@ +-- ============================================================================= +-- 2026-05-10 Audit / HIGH-10 closure +-- ============================================================================= +-- +-- Per-actor scope override on role grants. Pre-fix, actor_roles had +-- expires_at (already shipped) but no scope_type/scope_id columns, so +-- "give Alice operator over profile X only" wasn't expressible at the +-- grant layer — the only path was creating a scoped role and granting +-- that. This migration adds the per-grant scope tuple so an operator +-- can attach Alice to the standing r-operator role but scope the +-- grant to profile X. +-- +-- scope_type defaults to 'global' to preserve existing rows; scope_id +-- stays NULL when scope_type='global'. Authorizer.CheckPermission +-- already understands the tuple shape (role_permissions carries the +-- same columns); the actor-role addition gives operators a second +-- knob without forcing them to fork roles. +-- ============================================================================= + +ALTER TABLE actor_roles + ADD COLUMN IF NOT EXISTS scope_type TEXT NOT NULL DEFAULT 'global', + ADD COLUMN IF NOT EXISTS scope_id TEXT; + +ALTER TABLE actor_roles + ADD CONSTRAINT actor_roles_scope_type_enum + CHECK (scope_type IN ('global', 'profile', 'issuer')); + +ALTER TABLE actor_roles + ADD CONSTRAINT actor_roles_scope_id_required_when_not_global + CHECK ( + (scope_type = 'global' AND scope_id IS NULL) OR + (scope_type IN ('profile', 'issuer') AND scope_id IS NOT NULL) + ); + +-- The (actor_id, actor_type, role_id, tenant_id) uniqueness must +-- relax: an operator can grant the same role to the same actor at +-- different scopes (e.g. r-operator on profile-A AND on profile-B). +ALTER TABLE actor_roles + DROP CONSTRAINT IF EXISTS actor_roles_actor_id_actor_type_role_id_tenant_id_key; + +ALTER TABLE actor_roles + ADD CONSTRAINT actor_roles_actor_role_scope_unique + UNIQUE (actor_id, actor_type, role_id, scope_type, scope_id, tenant_id); + +CREATE INDEX IF NOT EXISTS idx_actor_roles_scope + ON actor_roles(scope_type, scope_id) WHERE scope_id IS NOT NULL;