mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-09 19:58:51 +00:00
0152bdf567
HIGH-10's UNIQUE (actor, role, scope_type, scope_id, tenant) uniqueness
extension lets an operator grant the same role to the same actor at
multiple scopes (e.g. r-operator on profile=p-acme AND profile=p-globex).
But ActorRoleRepository.Revoke's WHERE clause omitted (scope_type,
scope_id) — a single call deleted every variant. Selective revoke was
unrepresentable; operators had to drop all and re-grant N-1, opening
a race window where the actor's access was briefly different.
Closure across all layers (handler → service → repo → MCP → GUI client),
preserving the legacy "revoke all variants" contract for unmodified
callers:
internal/repository/auth.go
- New ActorRoleRevokeOptions struct. Zero value = legacy semantic;
non-empty ScopeType narrows to one variant.
- New ErrActorRoleNotFound sentinel for scoped no-match (HTTP 404).
internal/repository/postgres/auth.go
- Revoke signature extended with opts. Empty opts.ScopeType uses
the legacy SQL (no scope WHERE), zero-row delete = no error.
- Non-empty narrows with `scope_type = $5 AND scope_id IS NOT
DISTINCT FROM $6` — the IS-NOT-DISTINCT-FROM is load-bearing,
vanilla `=` would silently miss the (global, NULL) case because
NULL ≠ NULL in standard SQL.
- Selective revoke with zero matching rows returns
ErrActorRoleNotFound; operators get feedback on typos.
internal/service/auth/actor_role_service.go
- Revoke takes opts. Audit row's details map records the scope so
SIEMs can distinguish wide-vs-selective revokes:
`scope: "all_variants"` for the legacy path, or
`scope_type` + `scope_id` for selective. Privilege check
(auth.role.assign) and reserved-actor guard unchanged.
internal/api/handler/auth.go
- RevokeRoleFromKey parses optional `?scope_type=` / `?scope_id=`
query params via new parseRevokeScope helper.
- Validation mirrors AssignRoleToKey: scope_id forbidden with
scope_type=global, required with profile/issuer, invalid
scope_type → 400. scope_id without scope_type also → 400.
- writeAuthError maps ErrActorRoleNotFound to 404.
internal/mcp/tools_auth.go + types.go
- AuthRevokeKeyRoleInput gains optional ScopeType + ScopeID with
jsonschema descriptions explaining the dual-mode contract.
- Tool call site appends URL-encoded query params when ScopeType
is set; legacy callers (no scope_type) emit the bare DELETE
path unchanged.
web/src/api/client.ts
- authRevokeKeyRole signature: optional 3rd argument
`{ scope_type?, scope_id? }`. Pre-A-4 call sites (no opts arg)
keep firing the bare DELETE — fully backward compatible. The
GUI KeysPage's per-row revoke button (still one row per role,
pre-Fix-12) continues to use the legacy shape; future GUI work
can pass scope params for per-variant rows.
docs/operator/rbac.md
- New "Revoke: legacy 'all variants' vs scope-selective" subsection
under "From the HTTP API" with curl examples for both modes plus
the audit-row payload shape that lets SOC/SIEM tell them apart.
Regression coverage:
Repository (testcontainers, skipped under -short — 6 tests in
internal/repository/postgres/auth_revoke_scope_test.go):
TestRevokeActorRole_NoOpts_RemovesAllVariants
TestRevokeActorRole_WithScope_RemovesOnlyMatching
TestRevokeActorRole_WithGlobalScope_RemovesOnlyGlobal — pins the
IS-NOT-DISTINCT-FROM branch (global, NULL)
TestRevokeActorRole_NoMatch_ReturnsNotFound — pins the new sentinel
TestRevokeActorRole_NoOpts_NoMatch_IsNoOp — pins the legacy
idempotence contract
TestRevokeActorRole_IssuerScope_RemovesOnlyMatching — pin the
issuer-scope half (profile + issuer are symmetric scope types)
Handler (7 new tests in auth_test.go):
TestAuthHandler_RevokeRoleFromKey — extended to assert no scope
filter is forwarded when query string is empty (legacy behaviour)
TestAuthHandler_RevokeRoleFromKey_A4_ScopedProfile
TestAuthHandler_RevokeRoleFromKey_A4_ScopedGlobal
TestAuthHandler_RevokeRoleFromKey_A4_RejectsScopeIDWithGlobal
TestAuthHandler_RevokeRoleFromKey_A4_RejectsMissingScopeID
TestAuthHandler_RevokeRoleFromKey_A4_RejectsScopeIDWithoutScopeType
TestAuthHandler_RevokeRoleFromKey_A4_RejectsInvalidScopeType
TestAuthHandler_RevokeRoleFromKey_A4_ScopedNotFoundReturns404
MCP (2 new table rows in tools_per_tool_test.go):
Scoped revoke with scope_type=profile + scope_id=p-acme →
`?scope_type=profile&scope_id=p-acme`
Scoped revoke with scope_type=global (no scope_id) →
`?scope_type=global`
Service-layer test plumbing (service_test.go) updated for new opts
arg: 4 existing call sites pass repository.ActorRoleRevokeOptions{}
to keep their pre-A-4 semantics; the fakeActorRoleRepo.Revoke
implementation now mirrors the postgres scope-aware behaviour
(legacy zero-value vs scoped narrowing + ErrActorRoleNotFound on
no-match).
Verify gate green: gofmt clean, go vet clean, go test -short across
repository/postgres, service/auth, api/handler, and mcp. The
pre-existing KeysPage.test.tsx failure observed on the baseline
commit (reproduced via `git stash` earlier in Fix 03) is unrelated;
my client.ts change adds an optional third argument and is fully
backward-compatible.
Spec at cowork/auth-bundles-fixes-2026-05-11/04-high-actor-role-revoke-scope.md.
Audit doc updated: new row A-4 (2026-05-11) CLOSED appended to the
status table at the bottom of cowork/auth-bundles-audit-2026-05-10.md.
Operator-visible advisory in CHANGELOG.md v2.1.0 release notes under
Security (non-BREAKING — legacy callers are unchanged).
Depends on Fix 01 (the scope-aware EffectivePermissions read path on
branch fix/audit-2026-05-11/crit-actor-role-scope-reads). This fix
makes the inverse op selectively reversible; without Fix 01 the read
side would mis-evaluate scoped grants anyway, making selective revoke
moot at runtime.
212 lines
7.9 KiB
Go
212 lines
7.9 KiB
Go
package auth
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"log/slog"
|
|
|
|
"github.com/certctl-io/certctl/internal/domain"
|
|
authdomain "github.com/certctl-io/certctl/internal/domain/auth"
|
|
"github.com/certctl-io/certctl/internal/repository"
|
|
)
|
|
|
|
// ActorRoleService grants / revokes roles to actors and exposes the
|
|
// effective-permissions query the Phase 3 middleware uses on the hot
|
|
// path.
|
|
type ActorRoleService struct {
|
|
repo repository.ActorRoleRepository
|
|
roleRepo repository.RoleRepository
|
|
authorizer *Authorizer
|
|
audit AuditService
|
|
}
|
|
|
|
// NewActorRoleService constructs an ActorRoleService.
|
|
func NewActorRoleService(
|
|
repo repository.ActorRoleRepository,
|
|
roleRepo repository.RoleRepository,
|
|
authorizer *Authorizer,
|
|
audit AuditService,
|
|
) *ActorRoleService {
|
|
return &ActorRoleService{
|
|
repo: repo,
|
|
roleRepo: roleRepo,
|
|
authorizer: authorizer,
|
|
audit: audit,
|
|
}
|
|
}
|
|
|
|
// Grant assigns a role to an actor. Privilege-escalation guard: the
|
|
// caller must hold `auth.role.assign` (globally). System callers
|
|
// bypass. Reserved actor `actor-demo-anon` is rejected.
|
|
func (s *ActorRoleService) Grant(ctx context.Context, caller *Caller, ar *authdomain.ActorRole) error {
|
|
if caller == nil {
|
|
return ErrUnauthenticated
|
|
}
|
|
if !caller.IsSystem {
|
|
ok, err := s.authorizer.HoldsAnyOf(ctx, caller.ActorID, authdomain.ActorTypeValue(caller.ActorType), s.tenantOf(caller), "auth.role.assign")
|
|
if err != nil {
|
|
return err
|
|
}
|
|
if !ok {
|
|
return fmt.Errorf("%w: auth.role.assign required", ErrSelfRoleAssignment)
|
|
}
|
|
}
|
|
if ar.ActorID == authdomain.DemoAnonActorID {
|
|
return fmt.Errorf("%w: actor-demo-anon is reserved", repository.ErrAuthReservedActor)
|
|
}
|
|
if ar.TenantID == "" {
|
|
ar.TenantID = authdomain.DefaultTenantID
|
|
}
|
|
if err := s.repo.Grant(ctx, ar); err != nil {
|
|
return err
|
|
}
|
|
s.recordAudit(ctx, caller, "actor_role.grant", "actor_role", ar.ID, map[string]interface{}{
|
|
"actor_id": ar.ActorID,
|
|
"actor_type": string(ar.ActorType),
|
|
"role_id": ar.RoleID,
|
|
})
|
|
return nil
|
|
}
|
|
|
|
// Revoke removes a previously-granted role from an actor. Same
|
|
// privilege guard as Grant: caller needs `auth.role.assign` to mutate
|
|
// role membership. Reserved actor `actor-demo-anon` is rejected so the
|
|
// demo path stays alive even after a misclick.
|
|
//
|
|
// Audit 2026-05-11 A-4 — opts narrows the revoke to a specific
|
|
// (scope_type, scope_id) variant. Zero value preserves the legacy
|
|
// "revoke all variants" behaviour. When opts.ScopeType is set the
|
|
// repository returns repository.ErrActorRoleNotFound if no row matches;
|
|
// the handler maps it to HTTP 404. The audit row records the scope so
|
|
// operators can distinguish "wide revoke" from "selective revoke" in
|
|
// the SIEM.
|
|
func (s *ActorRoleService) Revoke(ctx context.Context, caller *Caller, actorID string, actorType domain.ActorType, roleID string, opts repository.ActorRoleRevokeOptions) error {
|
|
if caller == nil {
|
|
return ErrUnauthenticated
|
|
}
|
|
if !caller.IsSystem {
|
|
ok, err := s.authorizer.HoldsAnyOf(ctx, caller.ActorID, authdomain.ActorTypeValue(caller.ActorType), s.tenantOf(caller), "auth.role.assign")
|
|
if err != nil {
|
|
return err
|
|
}
|
|
if !ok {
|
|
return fmt.Errorf("%w: auth.role.assign required", ErrSelfRoleAssignment)
|
|
}
|
|
}
|
|
if actorID == authdomain.DemoAnonActorID {
|
|
return fmt.Errorf("%w: actor-demo-anon is reserved", repository.ErrAuthReservedActor)
|
|
}
|
|
tenantID := s.tenantOf(caller)
|
|
if err := s.repo.Revoke(ctx, actorID, authdomain.ActorTypeValue(actorType), roleID, tenantID, opts); err != nil {
|
|
return err
|
|
}
|
|
details := map[string]interface{}{
|
|
"actor_id": actorID,
|
|
"actor_type": string(actorType),
|
|
"role_id": roleID,
|
|
}
|
|
if opts.ScopeType != "" {
|
|
details["scope_type"] = string(opts.ScopeType)
|
|
if opts.ScopeID != nil {
|
|
details["scope_id"] = *opts.ScopeID
|
|
}
|
|
} else {
|
|
details["scope"] = "all_variants"
|
|
}
|
|
s.recordAudit(ctx, caller, "actor_role.revoke", "actor_role", roleID, details)
|
|
return nil
|
|
}
|
|
|
|
// ListForActor returns the roles held by the named actor.
|
|
func (s *ActorRoleService) ListForActor(ctx context.Context, caller *Caller, actorID string, actorType domain.ActorType) ([]*authdomain.ActorRole, error) {
|
|
if caller == nil {
|
|
return nil, ErrUnauthenticated
|
|
}
|
|
if !caller.IsSystem && caller.ActorID != actorID {
|
|
ok, err := s.authorizer.HoldsAnyOf(ctx, caller.ActorID, authdomain.ActorTypeValue(caller.ActorType), s.tenantOf(caller), "auth.role.list")
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
if !ok {
|
|
return nil, fmt.Errorf("%w: auth.role.list required to view another actor's roles", ErrForbidden)
|
|
}
|
|
}
|
|
return s.repo.ListByActor(ctx, actorID, authdomain.ActorTypeValue(actorType), s.tenantOf(caller))
|
|
}
|
|
|
|
// EffectivePermissions returns the deduplicated (permission, scope)
|
|
// pairs granted to the actor across all roles. Phase 3 middleware
|
|
// (auth.RequirePermission) calls this on every gated request via the
|
|
// Authorizer; that hot path skips RBAC self-checks. The service-level
|
|
// method here is for handler / GUI callers (the /v1/auth/me endpoint).
|
|
func (s *ActorRoleService) EffectivePermissions(ctx context.Context, caller *Caller, actorID string, actorType domain.ActorType) ([]repository.EffectivePermission, error) {
|
|
if caller == nil {
|
|
return nil, ErrUnauthenticated
|
|
}
|
|
if !caller.IsSystem && caller.ActorID != actorID {
|
|
ok, err := s.authorizer.HoldsAnyOf(ctx, caller.ActorID, authdomain.ActorTypeValue(caller.ActorType), s.tenantOf(caller), "auth.role.list")
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
if !ok {
|
|
return nil, fmt.Errorf("%w: auth.role.list required to view another actor's permissions", ErrForbidden)
|
|
}
|
|
}
|
|
return s.repo.EffectivePermissions(ctx, actorID, authdomain.ActorTypeValue(actorType), s.tenantOf(caller))
|
|
}
|
|
|
|
// ListKeys (Bundle 1 Phase 7) returns every actor in the tenant that
|
|
// holds at least one role grant. Permission `auth.role.list` is
|
|
// required (or the caller must be system). The CLI's `auth keys list`
|
|
// + scope-down helper consume this to enumerate the operator-key
|
|
// population without a separate /v1/auth/keys-by-name surface.
|
|
func (s *ActorRoleService) ListKeys(ctx context.Context, caller *Caller) ([]repository.ActorWithRoles, error) {
|
|
if caller == nil {
|
|
return nil, ErrUnauthenticated
|
|
}
|
|
if !caller.IsSystem {
|
|
ok, err := s.authorizer.HoldsAnyOf(ctx, caller.ActorID, authdomain.ActorTypeValue(caller.ActorType), s.tenantOf(caller), "auth.role.list")
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
if !ok {
|
|
return nil, fmt.Errorf("%w: auth.role.list required to list keys", ErrForbidden)
|
|
}
|
|
}
|
|
return s.repo.ListDistinctActors(ctx, s.tenantOf(caller))
|
|
}
|
|
|
|
func (s *ActorRoleService) tenantOf(caller *Caller) string {
|
|
if caller != nil && caller.TenantID != "" {
|
|
return caller.TenantID
|
|
}
|
|
return authdomain.DefaultTenantID
|
|
}
|
|
|
|
func (s *ActorRoleService) recordAudit(ctx context.Context, caller *Caller, action, resourceType, resourceID string, details map[string]interface{}) {
|
|
if s.audit == nil || caller == nil {
|
|
return
|
|
}
|
|
// Bundle 1 Phase 8: every actor-role grant/revoke is an
|
|
// authentication / authorization event. The auditor role queries
|
|
// /v1/audit?category=auth to surface this slice without
|
|
// also pulling in cert.* events.
|
|
//
|
|
// Audit 2026-05-10 HIGH-6 partial closure: the audit emit is still
|
|
// best-effort relative to the action transaction (the transactional-
|
|
// leg WithinTx refactor is a v3 follow-on; see
|
|
// cowork/auth-bundles-fixes-2026-05-10/10-high-6-atomic-audit-commit.md).
|
|
// What this commit closes is the *silence* leg — swap the discarded
|
|
// `_ = ...` pattern for an explicit WARN log so a DB hiccup or
|
|
// connection reset between action and audit is observable to the
|
|
// operator instead of going unnoticed (CWE-778).
|
|
if err := s.audit.RecordEventWithCategory(ctx, caller.ActorID, caller.ActorType, action, domain.EventCategoryAuth, resourceType, resourceID, details); err != nil {
|
|
slog.WarnContext(ctx, "audit write failed (action committed; audit row may be missing)",
|
|
"action", action,
|
|
"resource_type", resourceType,
|
|
"resource_id", resourceID,
|
|
"actor_id", caller.ActorID,
|
|
"err", err)
|
|
}
|
|
}
|