mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 19:11:30 +00:00
feat(auth/rbac): scope_type+scope_id+expires_at on role grants (HIGH-10)
Audit 2026-05-10 — close HIGH-10 from the HANDOFF.md backend batch
(item 1). Per-actor scoped + time-bound role grants are now
expressible via the API.
Migration 000043: adds scope_type TEXT NOT NULL DEFAULT 'global' +
scope_id TEXT to actor_roles. Constraints:
- actor_roles_scope_type_enum: scope_type ∈ {global, profile, issuer}
- actor_roles_scope_id_required_when_not_global: scope_id is NULL
iff scope_type='global'
- Uniqueness extended: (actor_id, actor_type, role_id, scope_type,
scope_id, tenant_id) — so an operator can grant the same role to
the same actor scoped to multiple profiles/issuers (e.g.
r-operator on p-finance AND on p-engineering).
Index idx_actor_roles_scope for non-global lookup hot paths.
Domain: ActorRole.ScopeType (ScopeType enum) + ScopeID (*string).
Authorizer.CheckPermission already understands the tuple via the
parallel role_permissions columns; this addition gives operators a
per-actor knob without forking roles.
Postgres repo: Grant writes scope_type+scope_id with ON CONFLICT keyed
on the new uniqueness tuple. Defaults to (global, NULL) when caller
omits.
Handler: assignRoleRequest extended with scope_type / scope_id /
expires_at. Validation:
- role_id required (unchanged)
- scope_type defaults to 'global'; allowed values global/profile/
issuer; anything else → 400
- scope_id required when scope_type ∈ {profile, issuer}; rejected
(must be empty) when scope_type='global'
- expires_at must be in the future when present; nil = standing
Regression matrix in internal/api/handler/auth_test.go (6 cases):
- TestAssignRoleToKey_HIGH10_ProfileScopeBoundGrantPersists
- TestAssignRoleToKey_HIGH10_TimeBoundGrantPersists
- TestAssignRoleToKey_HIGH10_RejectsScopeIDWithGlobalScope
- TestAssignRoleToKey_HIGH10_RejectsMissingScopeIDOnProfile
- TestAssignRoleToKey_HIGH10_RejectsPastExpiry
- TestAssignRoleToKey_HIGH10_RejectsInvalidScopeType
HIGH-10 marked CLOSED in audit-doc — the v3 deferral from the prior
session is reversed; everything lands in v2.
Refs: cowork/auth-bundles-fixes-2026-05-10/HANDOFF.md item 1
cowork/auth-bundles-audit-2026-05-10.md HIGH-10
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
@@ -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;
|
||||
Reference in New Issue
Block a user