mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 18:11:32 +00:00
Merge Fix 04 (HIGH A-4): scope-aware ActorRole revoke
This commit is contained in:
@@ -2,6 +2,28 @@
|
||||
|
||||
## Unreleased
|
||||
|
||||
### Security
|
||||
|
||||
- **Scope-aware actor-role revoke (Audit 2026-05-11 A-4).**
|
||||
HIGH-10 made it possible to grant the same role to the same actor at
|
||||
multiple scopes (e.g. `r-operator` on `profile=p-acme` AND `profile=p-globex`)
|
||||
via the unique constraint extension on `actor_roles`, but
|
||||
`ActorRoleRepository.Revoke` ignored `(scope_type, scope_id)` and
|
||||
unconditionally deleted every variant. Operators who wanted to drop
|
||||
one scoped grant had to nuke them all and re-grant the remainder —
|
||||
a race window where the actor's access was briefly different. The
|
||||
`DELETE /v1/auth/keys/{id}/roles/{role_id}` endpoint now accepts
|
||||
optional `?scope_type=` / `?scope_id=` query params that narrow the
|
||||
revoke to a single variant; no-match returns 404. The legacy "revoke
|
||||
every variant" semantic is preserved when the query params are
|
||||
absent, so existing CLI / GUI buttons keep working unchanged. The
|
||||
audit row's `details` payload records which mode fired so SOC / SIEM
|
||||
can distinguish wide cleanups from targeted demotions. MCP tool
|
||||
`certctl_auth_revoke_role_from_key` gains optional `scope_type` +
|
||||
`scope_id` input fields with matching semantics. Documented in
|
||||
`docs/operator/rbac.md` under "Revoke: legacy 'all variants' vs
|
||||
scope-selective."
|
||||
|
||||
### Security (BREAKING — silent-elevation closure)
|
||||
|
||||
- **HIGH-10 actor-role scope is now enforced (Audit 2026-05-11 A-1).**
|
||||
|
||||
+38
-1
@@ -209,10 +209,47 @@ tag. Quick reference:
|
||||
| `DELETE /v1/auth/roles/{id}/permissions/{perm}` | `auth.role.edit` |
|
||||
| `GET /v1/auth/keys` | `auth.role.list` |
|
||||
| `POST /v1/auth/keys/{id}/roles` | `auth.role.assign` |
|
||||
| `DELETE /v1/auth/keys/{id}/roles/{role_id}` | `auth.role.assign` |
|
||||
| `DELETE /v1/auth/keys/{id}/roles/{role_id}` (+ optional `?scope_type=` / `?scope_id=`) | `auth.role.assign` |
|
||||
| `GET /v1/auth/check` | (authenticated; surfaces effective perms) |
|
||||
| `GET /v1/auth/bootstrap` + `POST /v1/auth/bootstrap` | (auth-exempt; gated by env-var token) |
|
||||
|
||||
#### Revoke: legacy "all variants" vs scope-selective (Audit 2026-05-11 A-4)
|
||||
|
||||
`DELETE /v1/auth/keys/{id}/roles/{role_id}` runs in one of two modes,
|
||||
selected by presence of the optional query parameters:
|
||||
|
||||
- **No query params (legacy "revoke all variants")** — every scoped grant of
|
||||
this role held by this actor is dropped. Idempotent: zero-row deletes
|
||||
return 204 (no error). This is the pre-A-4 behaviour and remains the
|
||||
default for the CLI / GUI buttons that don't know about scope.
|
||||
|
||||
```bash
|
||||
# Drop EVERY variant of r-operator from alice (global, profile-scoped,
|
||||
# issuer-scoped — all gone).
|
||||
curl -X DELETE https://certctl.example.com/api/v1/auth/keys/alice/roles/r-operator
|
||||
```
|
||||
|
||||
- **`?scope_type=` (+ optional `?scope_id=`)** — drop ONE variant. Used
|
||||
when an actor holds the same role at multiple scopes (HIGH-10 made
|
||||
that representable; A-4 makes it selectively revocable).
|
||||
`scope_type=global` requires `scope_id` to be absent; `scope_type=profile`
|
||||
/ `issuer` require `scope_id`. No match returns 404 so operators get
|
||||
feedback when they target a scope variant the actor doesn't hold.
|
||||
|
||||
```bash
|
||||
# Alice holds r-operator scoped to p-acme AND p-globex.
|
||||
# Drop ONLY the p-acme grant; the p-globex grant stays.
|
||||
curl -X DELETE 'https://certctl.example.com/api/v1/auth/keys/alice/roles/r-operator?scope_type=profile&scope_id=p-acme'
|
||||
|
||||
# Drop ONLY the global grant of r-operator (keeps any profile / issuer variants):
|
||||
curl -X DELETE 'https://certctl.example.com/api/v1/auth/keys/alice/roles/r-operator?scope_type=global'
|
||||
```
|
||||
|
||||
The audit row's `details` payload records which mode fired —
|
||||
`scope: "all_variants"` for the legacy path, or the explicit
|
||||
`scope_type` + `scope_id` for selective revoke — so SOC / SIEM can
|
||||
distinguish wide cleanups from targeted demotions in the access log.
|
||||
|
||||
### From the MCP server
|
||||
|
||||
Bundle 1 Phase 11 ships 12 RBAC tools:
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -72,7 +73,11 @@ type AuthPermissionService interface {
|
||||
// effective-permissions query the GUI's /v1/auth/me handler uses.
|
||||
type AuthActorRoleService interface {
|
||||
Grant(ctx context.Context, caller *authsvc.Caller, ar *authdomain.ActorRole) error
|
||||
Revoke(ctx context.Context, caller *authsvc.Caller, actorID string, actorType domain.ActorType, roleID string) error
|
||||
// Audit 2026-05-11 A-4 — Revoke takes optional scope filtering so
|
||||
// callers that hold multiple scoped variants of the same role can
|
||||
// drop one variant selectively. opts.ScopeType == "" preserves the
|
||||
// legacy "revoke all" semantic.
|
||||
Revoke(ctx context.Context, caller *authsvc.Caller, actorID string, actorType domain.ActorType, roleID string, opts repository.ActorRoleRevokeOptions) error
|
||||
ListForActor(ctx context.Context, caller *authsvc.Caller, actorID string, actorType domain.ActorType) ([]*authdomain.ActorRole, error)
|
||||
EffectivePermissions(ctx context.Context, caller *authsvc.Caller, actorID string, actorType domain.ActorType) ([]repository.EffectivePermission, error)
|
||||
// ListKeys (Bundle 1 Phase 7) returns every actor in the tenant
|
||||
@@ -496,6 +501,22 @@ func (h AuthHandler) AssignRoleToKey(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
|
||||
// RevokeRoleFromKey handles DELETE /api/v1/auth/keys/{id}/roles/{role_id}.
|
||||
//
|
||||
// Audit 2026-05-11 A-4 — two operating modes selected by presence of
|
||||
// the optional `?scope_type=` / `?scope_id=` query parameters:
|
||||
//
|
||||
// - No query params: legacy "revoke every scope variant of this role
|
||||
// from this actor" semantic. Preserves pre-A-4 GUI behaviour
|
||||
// (KeysPage before Fix 12 fires plain DELETE with no scope; one
|
||||
// button per role row).
|
||||
//
|
||||
// - `scope_type=global` (no scope_id) or
|
||||
// `scope_type=profile&scope_id=<id>` /
|
||||
// `scope_type=issuer&scope_id=<id>`: drop ONLY the matching variant.
|
||||
// Returns HTTP 404 when no row matches the scope (operator
|
||||
// feedback for typos). Validation mirrors AssignRoleToKey:
|
||||
// `scope_id` MUST be empty with `scope_type=global`, MUST be
|
||||
// present with `profile` / `issuer`, anything else → 400.
|
||||
func (h AuthHandler) RevokeRoleFromKey(w http.ResponseWriter, r *http.Request) {
|
||||
caller, err := callerFromRequest(r)
|
||||
if err != nil {
|
||||
@@ -504,7 +525,19 @@ func (h AuthHandler) RevokeRoleFromKey(w http.ResponseWriter, r *http.Request) {
|
||||
}
|
||||
keyID := r.PathValue("id")
|
||||
roleID := r.PathValue("role_id")
|
||||
if err := h.actors.Revoke(r.Context(), caller, keyID, domain.ActorTypeAPIKey, roleID); err != nil {
|
||||
|
||||
// Parse + validate optional scope filter. Empty query string is
|
||||
// the legacy path; mismatched filter is rejected before the call
|
||||
// reaches the service.
|
||||
scopeTypeRaw := r.URL.Query().Get("scope_type")
|
||||
scopeIDRaw := r.URL.Query().Get("scope_id")
|
||||
opts, derr := parseRevokeScope(scopeTypeRaw, scopeIDRaw)
|
||||
if derr != nil {
|
||||
Error(w, http.StatusBadRequest, derr.Error())
|
||||
return
|
||||
}
|
||||
|
||||
if err := h.actors.Revoke(r.Context(), caller, keyID, domain.ActorTypeAPIKey, roleID, opts); err != nil {
|
||||
writeAuthError(w, err)
|
||||
return
|
||||
}
|
||||
@@ -515,6 +548,40 @@ func (h AuthHandler) RevokeRoleFromKey(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
}
|
||||
|
||||
// parseRevokeScope translates the (scope_type, scope_id) query string
|
||||
// into an ActorRoleRevokeOptions. Empty inputs → legacy "revoke all"
|
||||
// option (zero value); any combination missing required halves →
|
||||
// validation error. Audit 2026-05-11 A-4 — mirrors AssignRoleToKey's
|
||||
// scope validation so the assign / revoke pair stays symmetric.
|
||||
func parseRevokeScope(scopeType, scopeID string) (repository.ActorRoleRevokeOptions, error) {
|
||||
scopeType = strings.TrimSpace(scopeType)
|
||||
scopeID = strings.TrimSpace(scopeID)
|
||||
if scopeType == "" {
|
||||
if scopeID != "" {
|
||||
return repository.ActorRoleRevokeOptions{}, fmt.Errorf("scope_id requires scope_type")
|
||||
}
|
||||
return repository.ActorRoleRevokeOptions{}, nil
|
||||
}
|
||||
switch authdomain.ScopeType(scopeType) {
|
||||
case authdomain.ScopeTypeGlobal:
|
||||
if scopeID != "" {
|
||||
return repository.ActorRoleRevokeOptions{}, fmt.Errorf("scope_id must be empty when scope_type=global")
|
||||
}
|
||||
return repository.ActorRoleRevokeOptions{ScopeType: authdomain.ScopeTypeGlobal}, nil
|
||||
case authdomain.ScopeTypeProfile, authdomain.ScopeTypeIssuer:
|
||||
if scopeID == "" {
|
||||
return repository.ActorRoleRevokeOptions{}, fmt.Errorf("scope_id is required when scope_type is profile or issuer")
|
||||
}
|
||||
sid := scopeID
|
||||
return repository.ActorRoleRevokeOptions{
|
||||
ScopeType: authdomain.ScopeType(scopeType),
|
||||
ScopeID: &sid,
|
||||
}, nil
|
||||
default:
|
||||
return repository.ActorRoleRevokeOptions{}, fmt.Errorf("invalid scope_type — must be global, profile, or issuer")
|
||||
}
|
||||
}
|
||||
|
||||
// Me handles GET /api/v1/auth/me. Returns the current actor's effective
|
||||
// permissions plus admin flag (back-compat with /v1/auth/check). No
|
||||
// permission required: every authenticated caller can read their own.
|
||||
@@ -596,7 +663,7 @@ func writeAuthError(w http.ResponseWriter, err error) {
|
||||
Error(w, http.StatusForbidden, err.Error())
|
||||
case errors.Is(err, authsvc.ErrInvalidPermission):
|
||||
Error(w, http.StatusBadRequest, err.Error())
|
||||
case errors.Is(err, repository.ErrAuthNotFound):
|
||||
case errors.Is(err, repository.ErrAuthNotFound), errors.Is(err, repository.ErrActorRoleNotFound):
|
||||
Error(w, http.StatusNotFound, "Not found")
|
||||
case errors.Is(err, repository.ErrAuthDuplicateName), errors.Is(err, repository.ErrAuthRoleInUse), errors.Is(err, repository.ErrAuthReservedActor):
|
||||
Error(w, http.StatusConflict, err.Error())
|
||||
|
||||
@@ -122,6 +122,13 @@ type fakeAuthActorSvc struct {
|
||||
revokeErr error
|
||||
roles []*authdomain.ActorRole
|
||||
effective []repository.EffectivePermission
|
||||
// Audit 2026-05-11 A-4 — capture Revoke opts so tests can assert
|
||||
// that the handler forwards scope_type / scope_id correctly.
|
||||
revokeOpts repository.ActorRoleRevokeOptions
|
||||
revokeCall struct {
|
||||
actorID, roleID string
|
||||
called bool
|
||||
}
|
||||
}
|
||||
|
||||
func newFakeAuthActorSvc() *fakeAuthActorSvc {
|
||||
@@ -134,7 +141,11 @@ func (f *fakeAuthActorSvc) Grant(_ context.Context, _ *authsvc.Caller, ar *authd
|
||||
f.roles = append(f.roles, ar)
|
||||
return nil
|
||||
}
|
||||
func (f *fakeAuthActorSvc) Revoke(_ context.Context, _ *authsvc.Caller, _ string, _ domain.ActorType, _ string) error {
|
||||
func (f *fakeAuthActorSvc) Revoke(_ context.Context, _ *authsvc.Caller, actorID string, _ domain.ActorType, roleID string, opts repository.ActorRoleRevokeOptions) error {
|
||||
f.revokeCall.called = true
|
||||
f.revokeCall.actorID = actorID
|
||||
f.revokeCall.roleID = roleID
|
||||
f.revokeOpts = opts
|
||||
return f.revokeErr
|
||||
}
|
||||
func (f *fakeAuthActorSvc) ListForActor(_ context.Context, _ *authsvc.Caller, _ string, _ domain.ActorType) ([]*authdomain.ActorRole, error) {
|
||||
@@ -440,7 +451,7 @@ func TestAuthHandler_AssignRoleSelfRoleAssignReturns403(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestAuthHandler_RevokeRoleFromKey(t *testing.T) {
|
||||
h, _, _, _ := newAuthHandlerWithFakes()
|
||||
h, _, _, actorSvc := newAuthHandlerWithFakes()
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodDelete, "/api/v1/auth/keys/alice/roles/r-viewer", nil), "admin", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "alice")
|
||||
req.SetPathValue("role_id", "r-viewer")
|
||||
@@ -449,6 +460,136 @@ func TestAuthHandler_RevokeRoleFromKey(t *testing.T) {
|
||||
if rec.Code != http.StatusNoContent {
|
||||
t.Errorf("revoke should be 204; got %d", rec.Code)
|
||||
}
|
||||
// Audit 2026-05-11 A-4 — no scope params → legacy "revoke all
|
||||
// variants" semantic propagates as the zero-value
|
||||
// ActorRoleRevokeOptions to the service layer.
|
||||
if actorSvc.revokeOpts.ScopeType != "" {
|
||||
t.Errorf("legacy DELETE forwarded a scope filter: ScopeType=%q", actorSvc.revokeOpts.ScopeType)
|
||||
}
|
||||
if actorSvc.revokeOpts.ScopeID != nil {
|
||||
t.Errorf("legacy DELETE forwarded a scope_id: %v", actorSvc.revokeOpts.ScopeID)
|
||||
}
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// Audit 2026-05-11 A-4 — scope-aware revoke handler tests.
|
||||
// =============================================================================
|
||||
|
||||
func TestAuthHandler_RevokeRoleFromKey_A4_ScopedProfile(t *testing.T) {
|
||||
h, _, _, actorSvc := newAuthHandlerWithFakes()
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodDelete,
|
||||
"/api/v1/auth/keys/alice/roles/r-operator?scope_type=profile&scope_id=p-acme", nil),
|
||||
"admin", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "alice")
|
||||
req.SetPathValue("role_id", "r-operator")
|
||||
rec := httptest.NewRecorder()
|
||||
h.RevokeRoleFromKey(rec, req)
|
||||
if rec.Code != http.StatusNoContent {
|
||||
t.Fatalf("scoped revoke should be 204; got %d body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if actorSvc.revokeOpts.ScopeType != authdomain.ScopeTypeProfile {
|
||||
t.Errorf("ScopeType = %q; want profile", actorSvc.revokeOpts.ScopeType)
|
||||
}
|
||||
if actorSvc.revokeOpts.ScopeID == nil || *actorSvc.revokeOpts.ScopeID != "p-acme" {
|
||||
t.Errorf("ScopeID = %v; want p-acme", actorSvc.revokeOpts.ScopeID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_RevokeRoleFromKey_A4_ScopedGlobal(t *testing.T) {
|
||||
h, _, _, actorSvc := newAuthHandlerWithFakes()
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodDelete,
|
||||
"/api/v1/auth/keys/alice/roles/r-operator?scope_type=global", nil),
|
||||
"admin", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "alice")
|
||||
req.SetPathValue("role_id", "r-operator")
|
||||
rec := httptest.NewRecorder()
|
||||
h.RevokeRoleFromKey(rec, req)
|
||||
if rec.Code != http.StatusNoContent {
|
||||
t.Fatalf("scoped revoke (global) should be 204; got %d body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if actorSvc.revokeOpts.ScopeType != authdomain.ScopeTypeGlobal {
|
||||
t.Errorf("ScopeType = %q; want global", actorSvc.revokeOpts.ScopeType)
|
||||
}
|
||||
if actorSvc.revokeOpts.ScopeID != nil {
|
||||
t.Errorf("ScopeID must be nil for scope_type=global; got %v", actorSvc.revokeOpts.ScopeID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_RevokeRoleFromKey_A4_RejectsScopeIDWithGlobal(t *testing.T) {
|
||||
h, _, _, actorSvc := newAuthHandlerWithFakes()
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodDelete,
|
||||
"/api/v1/auth/keys/alice/roles/r-operator?scope_type=global&scope_id=p-acme", nil),
|
||||
"admin", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "alice")
|
||||
req.SetPathValue("role_id", "r-operator")
|
||||
rec := httptest.NewRecorder()
|
||||
h.RevokeRoleFromKey(rec, req)
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Errorf("global+scope_id should be 400; got %d body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if actorSvc.revokeCall.called {
|
||||
t.Error("service should NOT have been called on validation error")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_RevokeRoleFromKey_A4_RejectsMissingScopeID(t *testing.T) {
|
||||
h, _, _, actorSvc := newAuthHandlerWithFakes()
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodDelete,
|
||||
"/api/v1/auth/keys/alice/roles/r-operator?scope_type=profile", nil),
|
||||
"admin", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "alice")
|
||||
req.SetPathValue("role_id", "r-operator")
|
||||
rec := httptest.NewRecorder()
|
||||
h.RevokeRoleFromKey(rec, req)
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Errorf("profile-without-scope_id should be 400; got %d body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if actorSvc.revokeCall.called {
|
||||
t.Error("service should NOT have been called on validation error")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_RevokeRoleFromKey_A4_RejectsScopeIDWithoutScopeType(t *testing.T) {
|
||||
h, _, _, _ := newAuthHandlerWithFakes()
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodDelete,
|
||||
"/api/v1/auth/keys/alice/roles/r-operator?scope_id=p-acme", nil),
|
||||
"admin", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "alice")
|
||||
req.SetPathValue("role_id", "r-operator")
|
||||
rec := httptest.NewRecorder()
|
||||
h.RevokeRoleFromKey(rec, req)
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Errorf("scope_id-without-scope_type should be 400; got %d body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_RevokeRoleFromKey_A4_RejectsInvalidScopeType(t *testing.T) {
|
||||
h, _, _, _ := newAuthHandlerWithFakes()
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodDelete,
|
||||
"/api/v1/auth/keys/alice/roles/r-operator?scope_type=bogus", nil),
|
||||
"admin", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "alice")
|
||||
req.SetPathValue("role_id", "r-operator")
|
||||
rec := httptest.NewRecorder()
|
||||
h.RevokeRoleFromKey(rec, req)
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Errorf("bogus scope_type should be 400; got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_RevokeRoleFromKey_A4_ScopedNotFoundReturns404(t *testing.T) {
|
||||
h, _, _, actorSvc := newAuthHandlerWithFakes()
|
||||
actorSvc.revokeErr = repository.ErrActorRoleNotFound
|
||||
req := withAuthCtx(httptest.NewRequest(http.MethodDelete,
|
||||
"/api/v1/auth/keys/alice/roles/r-operator?scope_type=profile&scope_id=p-globex", nil),
|
||||
"admin", auth.ActorTypeAPIKey)
|
||||
req.SetPathValue("id", "alice")
|
||||
req.SetPathValue("role_id", "r-operator")
|
||||
rec := httptest.NewRecorder()
|
||||
h.RevokeRoleFromKey(rec, req)
|
||||
if rec.Code != http.StatusNotFound {
|
||||
t.Errorf("ErrActorRoleNotFound should be 404; got %d", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthHandler_RevokeReservedActorReturns409(t *testing.T) {
|
||||
|
||||
@@ -190,9 +190,21 @@ func registerAuthTools(s *gomcp.Server, c *Client) {
|
||||
|
||||
gomcp.AddTool(s, &gomcp.Tool{
|
||||
Name: "certctl_auth_revoke_role_from_key",
|
||||
Description: "Revoke a role from an API key actor (DELETE /v1/auth/keys/{id}/roles/{role_id}). Rejects revocations against the reserved actor-demo-anon (HTTP 409). Permission: auth.role.assign.",
|
||||
Description: "Revoke a role from an API key actor (DELETE /v1/auth/keys/{id}/roles/{role_id}). Rejects revocations against the reserved actor-demo-anon (HTTP 409). Audit 2026-05-11 A-4: pass scope_type=global / profile / issuer (with scope_id for the latter two) to selectively revoke ONE variant when the actor holds the same role at multiple scopes; omit both for the legacy 'revoke every variant' behaviour. Permission: auth.role.assign.",
|
||||
}, func(ctx context.Context, req *gomcp.CallToolRequest, input AuthRevokeKeyRoleInput) (*gomcp.CallToolResult, any, error) {
|
||||
data, err := c.Delete("/api/v1/auth/keys/" + input.KeyID + "/roles/" + input.RoleID)
|
||||
// Audit 2026-05-11 A-4 — append the optional scope filter when
|
||||
// the caller supplied scope_type. The handler validates the
|
||||
// pair shape (scope_id required vs forbidden) so we don't
|
||||
// duplicate that here.
|
||||
path := "/api/v1/auth/keys/" + input.KeyID + "/roles/" + input.RoleID
|
||||
if input.ScopeType != "" {
|
||||
q := "?scope_type=" + url.QueryEscape(input.ScopeType)
|
||||
if input.ScopeID != "" {
|
||||
q += "&scope_id=" + url.QueryEscape(input.ScopeID)
|
||||
}
|
||||
path += q
|
||||
}
|
||||
data, err := c.Delete(path)
|
||||
if err != nil {
|
||||
return errorResult(err)
|
||||
}
|
||||
|
||||
@@ -439,6 +439,10 @@ var allHappyPathCases = []toolCase{
|
||||
{"certctl_auth_list_keys", map[string]any{}, http.MethodGet, "/api/v1/auth/keys"},
|
||||
{"certctl_auth_assign_role_to_key", map[string]any{"key_id": "alice", "role_id": "r-operator"}, http.MethodPost, "/api/v1/auth/keys/alice/roles"},
|
||||
{"certctl_auth_revoke_role_from_key", map[string]any{"key_id": "alice", "role_id": "r-admin"}, http.MethodDelete, "/api/v1/auth/keys/alice/roles/r-admin"},
|
||||
// Audit 2026-05-11 A-4 — scoped revoke append both query params.
|
||||
{"certctl_auth_revoke_role_from_key", map[string]any{"key_id": "alice", "role_id": "r-operator", "scope_type": "profile", "scope_id": "p-acme"}, http.MethodDelete, "/api/v1/auth/keys/alice/roles/r-operator?scope_type=profile&scope_id=p-acme"},
|
||||
// Audit 2026-05-11 A-4 — scoped revoke with global emits only scope_type.
|
||||
{"certctl_auth_revoke_role_from_key", map[string]any{"key_id": "alice", "role_id": "r-operator", "scope_type": "global"}, http.MethodDelete, "/api/v1/auth/keys/alice/roles/r-operator?scope_type=global"},
|
||||
|
||||
// Bundle 2 Phase 9 — OIDC + session tools (11 tools).
|
||||
{"certctl_auth_list_oidc_providers", map[string]any{}, http.MethodGet, "/api/v1/auth/oidc/providers"},
|
||||
|
||||
+11
-2
@@ -602,9 +602,18 @@ type AuthAssignKeyRoleInput struct {
|
||||
|
||||
// AuthRevokeKeyRoleInput is the input for
|
||||
// certctl_auth_revoke_role_from_key.
|
||||
//
|
||||
// Audit 2026-05-11 A-4 — optional scope_type / scope_id fields narrow
|
||||
// the revoke to a single (scope_type, scope_id) variant when multiple
|
||||
// scoped grants of the same role exist. Empty scope_type fires the
|
||||
// legacy "revoke every variant" semantic. The handler enforces:
|
||||
// `scope_id` empty when scope_type=global; non-empty when
|
||||
// scope_type=profile / issuer.
|
||||
type AuthRevokeKeyRoleInput struct {
|
||||
KeyID string `json:"key_id" jsonschema:"API-key actor ID. Reserved actor-demo-anon is rejected server-side"`
|
||||
RoleID string `json:"role_id" jsonschema:"Role ID to revoke"`
|
||||
KeyID string `json:"key_id" jsonschema:"API-key actor ID. Reserved actor-demo-anon is rejected server-side"`
|
||||
RoleID string `json:"role_id" jsonschema:"Role ID to revoke"`
|
||||
ScopeType string `json:"scope_type,omitempty" jsonschema:"Optional scope filter: global / profile / issuer. Empty = revoke every scope variant of this role (legacy behaviour)"`
|
||||
ScopeID string `json:"scope_id,omitempty" jsonschema:"Required when scope_type=profile or issuer; must be empty when scope_type=global"`
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
|
||||
@@ -32,8 +32,46 @@ var (
|
||||
// references a permission name not in the canonical catalog.
|
||||
// Maps to HTTP 400.
|
||||
ErrAuthUnknownPermission = errors.New("auth: permission not in canonical catalog")
|
||||
|
||||
// ErrActorRoleNotFound is returned by ActorRoleRepository.Revoke
|
||||
// when the caller passes a non-empty `RevokeOptions.ScopeType` that
|
||||
// doesn't match any persisted (actor, role, scope_type, scope_id)
|
||||
// tuple. The legacy no-opts "revoke all variants" call never
|
||||
// returns this — pre-A-4 callers cannot start seeing the error.
|
||||
// Maps to HTTP 404. Audit 2026-05-11 A-4.
|
||||
ErrActorRoleNotFound = errors.New("auth: no actor_role row matches the requested scope")
|
||||
)
|
||||
|
||||
// ActorRoleRevokeOptions narrows ActorRoleRepository.Revoke to a
|
||||
// specific (scope_type, scope_id) variant when set. Audit 2026-05-11
|
||||
// A-4 — HIGH-10's UNIQUE (actor, role, scope_type, scope_id, tenant)
|
||||
// uniqueness extension allows multiple scoped grants of the same role
|
||||
// to the same actor; without scope plumbing on Revoke an operator who
|
||||
// granted Alice `r-operator` against both `profile=p-acme` and
|
||||
// `profile=p-globex` cannot selectively revoke one.
|
||||
//
|
||||
// Semantics:
|
||||
//
|
||||
// - Zero value (ScopeType="") preserves the legacy "revoke all
|
||||
// variants" behaviour. Every actor_roles row matching
|
||||
// (actor_id, actor_type, role_id, tenant_id) is deleted regardless
|
||||
// of scope. Pre-A-4 callers that don't pass options stay correct.
|
||||
//
|
||||
// - Non-empty ScopeType filters to that one variant. `global`
|
||||
// requires ScopeID==nil; `profile` / `issuer` require
|
||||
// ScopeID!=nil. The SQL uses `scope_type = $5 AND scope_id IS NOT
|
||||
// DISTINCT FROM $6` so the NULL case matches cleanly.
|
||||
//
|
||||
// - If the filter doesn't match any row, the repository returns
|
||||
// ErrActorRoleNotFound — the caller (service / handler) maps it
|
||||
// to HTTP 404. The legacy "revoke all" semantic stays best-effort
|
||||
// (deleting zero rows is not an error) because the GUI used to
|
||||
// fire it as a clean-up and operators rely on the idempotence.
|
||||
type ActorRoleRevokeOptions struct {
|
||||
ScopeType authdomain.ScopeType
|
||||
ScopeID *string
|
||||
}
|
||||
|
||||
// TenantRepository wraps the tenants table. Bundle 1 ships single-tenant
|
||||
// (one seeded `t-default`); the future managed-service offering activates
|
||||
// multi-tenant by inserting additional tenants.
|
||||
@@ -89,10 +127,15 @@ type ActorRoleRepository interface {
|
||||
// only if the operator explicitly wires that, which the API
|
||||
// layer rejects.
|
||||
Grant(ctx context.Context, ar *authdomain.ActorRole) error
|
||||
// Revoke deletes an actor_roles row by (actor_id, actor_type,
|
||||
// role_id, tenant_id). The API layer must reject revocations
|
||||
// targeting `actor-demo-anon` to preserve the demo path.
|
||||
Revoke(ctx context.Context, actorID string, actorType authdomain.ActorTypeValue, roleID, tenantID string) error
|
||||
// Revoke deletes actor_roles row(s). Without opts (the legacy
|
||||
// no-options call shape) every variant matching
|
||||
// (actor_id, actor_type, role_id, tenant_id) is deleted regardless
|
||||
// of (scope_type, scope_id). With opts.ScopeType set, only the
|
||||
// matching variant is deleted; no-match returns
|
||||
// ErrActorRoleNotFound. The API layer must reject revocations
|
||||
// targeting `actor-demo-anon` to preserve the demo path. Audit
|
||||
// 2026-05-11 A-4 — see ActorRoleRevokeOptions for semantics.
|
||||
Revoke(ctx context.Context, actorID string, actorType authdomain.ActorTypeValue, roleID, tenantID string, opts ActorRoleRevokeOptions) error
|
||||
|
||||
// EffectivePermissions returns the deduplicated set of
|
||||
// (permission_name, scope_type, scope_id) triples granted to the
|
||||
|
||||
@@ -406,14 +406,59 @@ func (r *ActorRoleRepository) Grant(ctx context.Context, ar *authdomain.ActorRol
|
||||
return nil
|
||||
}
|
||||
|
||||
func (r *ActorRoleRepository) Revoke(ctx context.Context, actorID string, actorType authdomain.ActorTypeValue, roleID, tenantID string) error {
|
||||
_, err := r.db.ExecContext(ctx, `
|
||||
// Audit 2026-05-11 A-4 — scope-aware revoke. The pre-fix SQL omitted
|
||||
// (scope_type, scope_id) from the WHERE clause; combined with HIGH-10's
|
||||
// UNIQUE (actor_id, actor_type, role_id, scope_type, scope_id, tenant_id)
|
||||
// uniqueness extension, an operator who granted the same role to the
|
||||
// same actor at two different scopes had no selective-revoke path —
|
||||
// every Revoke call nuked both rows. The new behaviour:
|
||||
//
|
||||
// - opts.ScopeType == "" (legacy call shape): drop the scope from the
|
||||
// WHERE clause; delete every variant. Zero-row delete is NOT an
|
||||
// error (preserves the GUI's pre-A-4 idempotence contract).
|
||||
//
|
||||
// - opts.ScopeType != "": narrow WHERE with
|
||||
// `scope_type = $5 AND scope_id IS NOT DISTINCT FROM $6` (the
|
||||
// IS-NOT-DISTINCT-FROM handles the `global → scope_id IS NULL`
|
||||
// case cleanly — Postgres `= NULL` would silently match nothing).
|
||||
// Zero-row delete IS an error (ErrActorRoleNotFound, mapped to
|
||||
// HTTP 404 upstream) so operators get feedback when they target a
|
||||
// scope variant that doesn't exist.
|
||||
func (r *ActorRoleRepository) Revoke(ctx context.Context, actorID string, actorType authdomain.ActorTypeValue, roleID, tenantID string, opts repository.ActorRoleRevokeOptions) error {
|
||||
if opts.ScopeType == "" {
|
||||
// Legacy "revoke all variants" path. Zero-row delete = no-op.
|
||||
_, err := r.db.ExecContext(ctx, `
|
||||
DELETE FROM actor_roles
|
||||
WHERE actor_id = $1 AND actor_type = $2 AND role_id = $3 AND tenant_id = $4
|
||||
`, actorID, string(actorType), roleID, tenantID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("actorRole.revoke: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
// Scoped path. `scope_id IS NOT DISTINCT FROM $6` makes
|
||||
// (global, NULL) match (global, NULL) cleanly — vanilla `=` would
|
||||
// drop on NULL ≠ NULL.
|
||||
var scopeID interface{}
|
||||
if opts.ScopeID != nil && *opts.ScopeID != "" {
|
||||
scopeID = *opts.ScopeID
|
||||
}
|
||||
res, err := r.db.ExecContext(ctx, `
|
||||
DELETE FROM actor_roles
|
||||
WHERE actor_id = $1 AND actor_type = $2 AND role_id = $3 AND tenant_id = $4
|
||||
`, actorID, string(actorType), roleID, tenantID)
|
||||
WHERE actor_id = $1
|
||||
AND actor_type = $2
|
||||
AND role_id = $3
|
||||
AND tenant_id = $4
|
||||
AND scope_type = $5
|
||||
AND scope_id IS NOT DISTINCT FROM $6
|
||||
`, actorID, string(actorType), roleID, tenantID, string(opts.ScopeType), scopeID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("actorRole.revoke: %w", err)
|
||||
}
|
||||
n, _ := res.RowsAffected()
|
||||
if n == 0 {
|
||||
return repository.ErrActorRoleNotFound
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,231 @@
|
||||
package postgres_test
|
||||
|
||||
// Audit 2026-05-11 A-4 closure — scope-aware ActorRoleRepository.Revoke
|
||||
// regression matrix. Pre-fix, Revoke deleted every (actor,role,tenant)
|
||||
// row regardless of (scope_type, scope_id), so an operator who held
|
||||
// the same role at two different profile scopes could only nuke them
|
||||
// both — selective revoke was unrepresentable. The new semantic:
|
||||
//
|
||||
// opts.ScopeType == "" → legacy "revoke all variants"
|
||||
// opts.ScopeType == global → drop only the (global, NULL) row
|
||||
// opts.ScopeType == profile|issuer + scope_id → drop only that variant
|
||||
// no match for a scoped revoke → ErrActorRoleNotFound (HTTP 404)
|
||||
//
|
||||
// Helpers (seedRoleWithPerm, grantActorRoleAtScope, ptrStr) live in
|
||||
// auth_scope_test.go in this same _test package.
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
authdomain "github.com/certctl-io/certctl/internal/domain/auth"
|
||||
"github.com/certctl-io/certctl/internal/repository"
|
||||
"github.com/certctl-io/certctl/internal/repository/postgres"
|
||||
)
|
||||
|
||||
// listActorRoleScopes returns every (scope_type, scope_id-or-empty)
|
||||
// tuple held by the actor for the role. Lets tests assert which
|
||||
// variants are still standing after a Revoke.
|
||||
func listActorRoleScopes(t *testing.T, ctx context.Context, repo *postgres.ActorRoleRepository, actorID, roleID string) []string {
|
||||
t.Helper()
|
||||
rows, err := repo.ListByActor(ctx, actorID, authdomain.ActorTypeValue("APIKey"), authdomain.DefaultTenantID)
|
||||
if err != nil {
|
||||
t.Fatalf("ListByActor: %v", err)
|
||||
}
|
||||
var out []string
|
||||
for _, r := range rows {
|
||||
if r.RoleID != roleID {
|
||||
continue
|
||||
}
|
||||
st := string(r.ScopeType)
|
||||
sid := ""
|
||||
if r.ScopeID != nil {
|
||||
sid = *r.ScopeID
|
||||
}
|
||||
out = append(out, st+"|"+sid)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// TestRevokeActorRole_NoOpts_RemovesAllVariants — pre-A-4 legacy
|
||||
// semantic. Actor holds the same role at two profile scopes; Revoke
|
||||
// with zero-value opts must wipe both.
|
||||
func TestRevokeActorRole_NoOpts_RemovesAllVariants(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("integration test in short mode")
|
||||
}
|
||||
db := getTestDB(t).freshSchema(t)
|
||||
ctx := context.Background()
|
||||
roleRepo := postgres.NewRoleRepository(db)
|
||||
permRepo := postgres.NewPermissionRepository(db)
|
||||
actorRepo := postgres.NewActorRoleRepository(db)
|
||||
|
||||
roleID := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "rev-noopts", "cert.read", authdomain.ScopeTypeGlobal, nil)
|
||||
grantActorRoleAtScope(t, ctx, actorRepo, "alice", roleID, authdomain.ScopeTypeProfile, ptrStr("p-acme"))
|
||||
grantActorRoleAtScope(t, ctx, actorRepo, "alice", roleID, authdomain.ScopeTypeProfile, ptrStr("p-globex"))
|
||||
|
||||
got := listActorRoleScopes(t, ctx, actorRepo, "alice", roleID)
|
||||
if len(got) != 2 {
|
||||
t.Fatalf("pre-revoke variants = %v; want 2", got)
|
||||
}
|
||||
|
||||
if err := actorRepo.Revoke(ctx, "alice", authdomain.ActorTypeValue("APIKey"), roleID, authdomain.DefaultTenantID, repository.ActorRoleRevokeOptions{}); err != nil {
|
||||
t.Fatalf("Revoke (no-opts): %v", err)
|
||||
}
|
||||
got = listActorRoleScopes(t, ctx, actorRepo, "alice", roleID)
|
||||
if len(got) != 0 {
|
||||
t.Errorf("legacy revoke left variants: %v; want []", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRevokeActorRole_WithScope_RemovesOnlyMatching — the A-4
|
||||
// selective-revoke happy path. Two scoped grants; revoke one by
|
||||
// (profile=p-acme); the other (profile=p-globex) stays.
|
||||
func TestRevokeActorRole_WithScope_RemovesOnlyMatching(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("integration test in short mode")
|
||||
}
|
||||
db := getTestDB(t).freshSchema(t)
|
||||
ctx := context.Background()
|
||||
roleRepo := postgres.NewRoleRepository(db)
|
||||
permRepo := postgres.NewPermissionRepository(db)
|
||||
actorRepo := postgres.NewActorRoleRepository(db)
|
||||
|
||||
roleID := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "rev-scoped", "cert.read", authdomain.ScopeTypeGlobal, nil)
|
||||
grantActorRoleAtScope(t, ctx, actorRepo, "alice", roleID, authdomain.ScopeTypeProfile, ptrStr("p-acme"))
|
||||
grantActorRoleAtScope(t, ctx, actorRepo, "alice", roleID, authdomain.ScopeTypeProfile, ptrStr("p-globex"))
|
||||
|
||||
err := actorRepo.Revoke(ctx, "alice", authdomain.ActorTypeValue("APIKey"), roleID, authdomain.DefaultTenantID, repository.ActorRoleRevokeOptions{
|
||||
ScopeType: authdomain.ScopeTypeProfile,
|
||||
ScopeID: ptrStr("p-acme"),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("scoped Revoke: %v", err)
|
||||
}
|
||||
got := listActorRoleScopes(t, ctx, actorRepo, "alice", roleID)
|
||||
if len(got) != 1 || got[0] != "profile|p-globex" {
|
||||
t.Errorf("post-revoke variants = %v; want [profile|p-globex]", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRevokeActorRole_WithGlobalScope_RemovesOnlyGlobal — pin the
|
||||
// `scope_id IS NOT DISTINCT FROM $6` branch. Actor holds global +
|
||||
// profile-scoped grants of the same role; revoke `scope_type=global`
|
||||
// drops only the global row.
|
||||
func TestRevokeActorRole_WithGlobalScope_RemovesOnlyGlobal(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("integration test in short mode")
|
||||
}
|
||||
db := getTestDB(t).freshSchema(t)
|
||||
ctx := context.Background()
|
||||
roleRepo := postgres.NewRoleRepository(db)
|
||||
permRepo := postgres.NewPermissionRepository(db)
|
||||
actorRepo := postgres.NewActorRoleRepository(db)
|
||||
|
||||
roleID := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "rev-global", "cert.read", authdomain.ScopeTypeGlobal, nil)
|
||||
grantActorRoleAtScope(t, ctx, actorRepo, "alice", roleID, authdomain.ScopeTypeGlobal, nil)
|
||||
grantActorRoleAtScope(t, ctx, actorRepo, "alice", roleID, authdomain.ScopeTypeProfile, ptrStr("p-acme"))
|
||||
|
||||
err := actorRepo.Revoke(ctx, "alice", authdomain.ActorTypeValue("APIKey"), roleID, authdomain.DefaultTenantID, repository.ActorRoleRevokeOptions{
|
||||
ScopeType: authdomain.ScopeTypeGlobal,
|
||||
ScopeID: nil,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("global-scope Revoke: %v", err)
|
||||
}
|
||||
got := listActorRoleScopes(t, ctx, actorRepo, "alice", roleID)
|
||||
if len(got) != 1 || got[0] != "profile|p-acme" {
|
||||
t.Errorf("post-revoke variants = %v; want [profile|p-acme]", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRevokeActorRole_NoMatch_ReturnsNotFound — operator passes a
|
||||
// (scope_type, scope_id) the actor doesn't actually hold for this
|
||||
// role. Selective revoke must return ErrActorRoleNotFound; existing
|
||||
// rows must be untouched.
|
||||
func TestRevokeActorRole_NoMatch_ReturnsNotFound(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("integration test in short mode")
|
||||
}
|
||||
db := getTestDB(t).freshSchema(t)
|
||||
ctx := context.Background()
|
||||
roleRepo := postgres.NewRoleRepository(db)
|
||||
permRepo := postgres.NewPermissionRepository(db)
|
||||
actorRepo := postgres.NewActorRoleRepository(db)
|
||||
|
||||
roleID := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "rev-nomatch", "cert.read", authdomain.ScopeTypeGlobal, nil)
|
||||
grantActorRoleAtScope(t, ctx, actorRepo, "alice", roleID, authdomain.ScopeTypeProfile, ptrStr("p-acme"))
|
||||
|
||||
err := actorRepo.Revoke(ctx, "alice", authdomain.ActorTypeValue("APIKey"), roleID, authdomain.DefaultTenantID, repository.ActorRoleRevokeOptions{
|
||||
ScopeType: authdomain.ScopeTypeProfile,
|
||||
ScopeID: ptrStr("p-globex"),
|
||||
})
|
||||
if !errors.Is(err, repository.ErrActorRoleNotFound) {
|
||||
t.Errorf("err = %v; want ErrActorRoleNotFound", err)
|
||||
}
|
||||
got := listActorRoleScopes(t, ctx, actorRepo, "alice", roleID)
|
||||
if len(got) != 1 || got[0] != "profile|p-acme" {
|
||||
t.Errorf("untargeted revoke mutated existing rows: %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRevokeActorRole_NoOpts_NoMatch_IsNoOp — pin the legacy
|
||||
// idempotence contract. Empty opts + zero matching rows must NOT
|
||||
// return an error (the GUI's pre-A-4 revoke button fires this when
|
||||
// the operator clicks "remove" on a stale row).
|
||||
func TestRevokeActorRole_NoOpts_NoMatch_IsNoOp(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("integration test in short mode")
|
||||
}
|
||||
db := getTestDB(t).freshSchema(t)
|
||||
ctx := context.Background()
|
||||
roleRepo := postgres.NewRoleRepository(db)
|
||||
permRepo := postgres.NewPermissionRepository(db)
|
||||
actorRepo := postgres.NewActorRoleRepository(db)
|
||||
|
||||
// Seed an unrelated role + grant so the table isn't completely
|
||||
// empty (more realistic baseline).
|
||||
otherRole := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "rev-noop-other", "cert.read", authdomain.ScopeTypeGlobal, nil)
|
||||
grantActorRoleAtScope(t, ctx, actorRepo, "bob", otherRole, authdomain.ScopeTypeGlobal, nil)
|
||||
|
||||
if err := actorRepo.Revoke(ctx, "alice", authdomain.ActorTypeValue("APIKey"), "r-nonexistent", authdomain.DefaultTenantID, repository.ActorRoleRevokeOptions{}); err != nil {
|
||||
t.Errorf("no-match no-opts Revoke should be nil; got %v", err)
|
||||
}
|
||||
// Sanity: the unrelated grant is untouched.
|
||||
got := listActorRoleScopes(t, ctx, actorRepo, "bob", otherRole)
|
||||
if len(got) != 1 || got[0] != "global|" {
|
||||
t.Errorf("unrelated grant mutated: %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRevokeActorRole_IssuerScope_RemovesOnlyMatching — pin the
|
||||
// issuer-scope variant (the other half of the {profile, issuer}
|
||||
// scope-type pair). Actor holds the same role across profile + issuer
|
||||
// scopes; revoke `issuer=iss-x` drops only the issuer row.
|
||||
func TestRevokeActorRole_IssuerScope_RemovesOnlyMatching(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("integration test in short mode")
|
||||
}
|
||||
db := getTestDB(t).freshSchema(t)
|
||||
ctx := context.Background()
|
||||
roleRepo := postgres.NewRoleRepository(db)
|
||||
permRepo := postgres.NewPermissionRepository(db)
|
||||
actorRepo := postgres.NewActorRoleRepository(db)
|
||||
|
||||
roleID := seedRoleWithPerm(t, ctx, roleRepo, permRepo, "rev-issuer", "cert.read", authdomain.ScopeTypeGlobal, nil)
|
||||
grantActorRoleAtScope(t, ctx, actorRepo, "alice", roleID, authdomain.ScopeTypeProfile, ptrStr("p-acme"))
|
||||
grantActorRoleAtScope(t, ctx, actorRepo, "alice", roleID, authdomain.ScopeTypeIssuer, ptrStr("iss-x"))
|
||||
|
||||
err := actorRepo.Revoke(ctx, "alice", authdomain.ActorTypeValue("APIKey"), roleID, authdomain.DefaultTenantID, repository.ActorRoleRevokeOptions{
|
||||
ScopeType: authdomain.ScopeTypeIssuer,
|
||||
ScopeID: ptrStr("iss-x"),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("issuer-scope Revoke: %v", err)
|
||||
}
|
||||
got := listActorRoleScopes(t, ctx, actorRepo, "alice", roleID)
|
||||
if len(got) != 1 || got[0] != "profile|p-acme" {
|
||||
t.Errorf("post-revoke variants = %v; want [profile|p-acme]", got)
|
||||
}
|
||||
}
|
||||
@@ -72,7 +72,15 @@ func (s *ActorRoleService) Grant(ctx context.Context, caller *Caller, ar *authdo
|
||||
// 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.
|
||||
func (s *ActorRoleService) Revoke(ctx context.Context, caller *Caller, actorID string, actorType domain.ActorType, roleID string) error {
|
||||
//
|
||||
// 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
|
||||
}
|
||||
@@ -89,14 +97,23 @@ func (s *ActorRoleService) Revoke(ctx context.Context, caller *Caller, actorID s
|
||||
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); err != nil {
|
||||
if err := s.repo.Revoke(ctx, actorID, authdomain.ActorTypeValue(actorType), roleID, tenantID, opts); err != nil {
|
||||
return err
|
||||
}
|
||||
s.recordAudit(ctx, caller, "actor_role.revoke", "actor_role", roleID, map[string]interface{}{
|
||||
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
|
||||
}
|
||||
|
||||
|
||||
@@ -159,15 +159,53 @@ func (f *fakeActorRoleRepo) Grant(_ context.Context, ar *authdomain.ActorRole) e
|
||||
f.grants = append(f.grants, ar)
|
||||
return nil
|
||||
}
|
||||
func (f *fakeActorRoleRepo) Revoke(_ context.Context, actorID string, actorType authdomain.ActorTypeValue, roleID, _ string) error {
|
||||
func (f *fakeActorRoleRepo) Revoke(_ context.Context, actorID string, actorType authdomain.ActorTypeValue, roleID, _ string, opts repository.ActorRoleRevokeOptions) error {
|
||||
// Audit 2026-05-11 A-4 — mirror the postgres semantics: when
|
||||
// opts.ScopeType is empty, remove every (actor,type,role) match
|
||||
// regardless of scope (legacy). When set, narrow to the single
|
||||
// matching scope variant; zero-match returns ErrActorRoleNotFound.
|
||||
wantScopedSpecific := opts.ScopeType != ""
|
||||
matched := 0
|
||||
out := f.grants[:0]
|
||||
for _, g := range f.grants {
|
||||
if g.ActorID == actorID && g.ActorType == actorType && g.RoleID == roleID {
|
||||
if wantScopedSpecific {
|
||||
// Match by (scope_type, scope_id).
|
||||
gScope := g.ScopeType
|
||||
if gScope == "" {
|
||||
gScope = authdomain.ScopeTypeGlobal
|
||||
}
|
||||
if gScope != opts.ScopeType {
|
||||
out = append(out, g)
|
||||
continue
|
||||
}
|
||||
// scope_id IS NOT DISTINCT FROM:
|
||||
gSID := ""
|
||||
if g.ScopeID != nil {
|
||||
gSID = *g.ScopeID
|
||||
}
|
||||
wSID := ""
|
||||
if opts.ScopeID != nil {
|
||||
wSID = *opts.ScopeID
|
||||
}
|
||||
if gSID != wSID {
|
||||
out = append(out, g)
|
||||
continue
|
||||
}
|
||||
// Drop this row.
|
||||
matched++
|
||||
continue
|
||||
}
|
||||
// Legacy "revoke all variants" path.
|
||||
matched++
|
||||
continue
|
||||
}
|
||||
out = append(out, g)
|
||||
}
|
||||
f.grants = out
|
||||
if wantScopedSpecific && matched == 0 {
|
||||
return repository.ErrActorRoleNotFound
|
||||
}
|
||||
return nil
|
||||
}
|
||||
func (f *fakeActorRoleRepo) AdminExists(_ context.Context, _ string) (bool, error) {
|
||||
@@ -439,7 +477,7 @@ func TestActorRoleService_GrantRejectsReservedDemoActor(t *testing.T) {
|
||||
|
||||
func TestActorRoleService_RevokeRejectsReservedDemoActor(t *testing.T) {
|
||||
svc, _, _ := newActorRoleServiceWithFakes()
|
||||
err := svc.Revoke(context.Background(), AsSystemCaller(), authdomain.DemoAnonActorID, domain.ActorTypeAnonymous, "r-admin")
|
||||
err := svc.Revoke(context.Background(), AsSystemCaller(), authdomain.DemoAnonActorID, domain.ActorTypeAnonymous, "r-admin", repository.ActorRoleRevokeOptions{})
|
||||
if !errors.Is(err, repository.ErrAuthReservedActor) {
|
||||
t.Errorf("Revoke against actor-demo-anon should be rejected; got %v", err)
|
||||
}
|
||||
@@ -848,19 +886,19 @@ func TestActorRoleService_ListKeysGates(t *testing.T) {
|
||||
func TestActorRoleService_RevokeGatesAndSucceeds(t *testing.T) {
|
||||
svc, repo, audit := newActorRoleServiceWithFakes()
|
||||
// nil caller
|
||||
if err := svc.Revoke(context.Background(), nil, "alice", domain.ActorTypeAPIKey, "r-admin"); !errors.Is(err, ErrUnauthenticated) {
|
||||
if err := svc.Revoke(context.Background(), nil, "alice", domain.ActorTypeAPIKey, "r-admin", repository.ActorRoleRevokeOptions{}); !errors.Is(err, ErrUnauthenticated) {
|
||||
t.Errorf("Revoke(nil) err = %v; want ErrUnauthenticated", err)
|
||||
}
|
||||
// caller without auth.role.assign
|
||||
caller := &Caller{ActorID: "bob", ActorType: domain.ActorTypeAPIKey}
|
||||
if err := svc.Revoke(context.Background(), caller, "alice", domain.ActorTypeAPIKey, "r-admin"); !errors.Is(err, ErrSelfRoleAssignment) {
|
||||
if err := svc.Revoke(context.Background(), caller, "alice", domain.ActorTypeAPIKey, "r-admin", repository.ActorRoleRevokeOptions{}); !errors.Is(err, ErrSelfRoleAssignment) {
|
||||
t.Errorf("Revoke(no perm) err = %v; want ErrSelfRoleAssignment", err)
|
||||
}
|
||||
// system caller success
|
||||
repo.grants = []*authdomain.ActorRole{
|
||||
{ID: "ar-1", ActorID: "alice", ActorType: authdomain.ActorTypeValue(domain.ActorTypeAPIKey), RoleID: "r-admin", TenantID: authdomain.DefaultTenantID},
|
||||
}
|
||||
if err := svc.Revoke(context.Background(), AsSystemCaller(), "alice", domain.ActorTypeAPIKey, "r-admin"); err != nil {
|
||||
if err := svc.Revoke(context.Background(), AsSystemCaller(), "alice", domain.ActorTypeAPIKey, "r-admin", repository.ActorRoleRevokeOptions{}); err != nil {
|
||||
t.Fatalf("Revoke(system) err: %v", err)
|
||||
}
|
||||
if len(audit.calls) != 1 || audit.calls[0].Action != "actor_role.revoke" {
|
||||
@@ -878,7 +916,7 @@ func TestActorRoleService_RevokeSucceedsWithAuthRoleAssign(t *testing.T) {
|
||||
{ID: "ar-1", ActorID: "carol", ActorType: authdomain.ActorTypeValue(domain.ActorTypeAPIKey), RoleID: "r-viewer", TenantID: authdomain.DefaultTenantID},
|
||||
}
|
||||
caller := &Caller{ActorID: "alice", ActorType: domain.ActorTypeAPIKey}
|
||||
if err := svc.Revoke(context.Background(), caller, "carol", domain.ActorTypeAPIKey, "r-viewer"); err != nil {
|
||||
if err := svc.Revoke(context.Background(), caller, "carol", domain.ActorTypeAPIKey, "r-viewer", repository.ActorRoleRevokeOptions{}); err != nil {
|
||||
t.Fatalf("Revoke(perm) err: %v", err)
|
||||
}
|
||||
if len(audit.calls) != 1 || audit.calls[0].Action != "actor_role.revoke" {
|
||||
|
||||
+21
-4
@@ -387,10 +387,27 @@ export const authOIDCTestProvider = (body: {
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
|
||||
export const authRevokeKeyRole = (keyId: string, roleId: string) =>
|
||||
fetchJSON<unknown>(`${BASE}/auth/keys/${keyId}/roles/${roleId}`, {
|
||||
method: 'DELETE',
|
||||
});
|
||||
// Audit 2026-05-11 A-4 — optional scope filter. When opts is omitted
|
||||
// or scope_type is empty, the server runs the legacy "revoke every
|
||||
// scope variant of this role" semantic (preserves pre-A-4 GUI
|
||||
// behaviour). When scope_type is set, only the matching variant is
|
||||
// dropped; server enforces scope_id presence vs absence per
|
||||
// scope_type. Useful when one actor holds the same role scoped to
|
||||
// multiple profiles / issuers and the operator wants to drop one
|
||||
// without touching the others.
|
||||
export const authRevokeKeyRole = (
|
||||
keyId: string,
|
||||
roleId: string,
|
||||
opts?: { scope_type?: string; scope_id?: string },
|
||||
) => {
|
||||
let path = `${BASE}/auth/keys/${keyId}/roles/${roleId}`;
|
||||
if (opts?.scope_type) {
|
||||
const params = new URLSearchParams({ scope_type: opts.scope_type });
|
||||
if (opts.scope_id) params.set('scope_id', opts.scope_id);
|
||||
path += `?${params.toString()}`;
|
||||
}
|
||||
return fetchJSON<unknown>(path, { method: 'DELETE' });
|
||||
};
|
||||
|
||||
export interface BootstrapAvailability {
|
||||
available: boolean;
|
||||
|
||||
Reference in New Issue
Block a user