Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions internal/handler/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ func mapErr(err error) error {
if errors.Is(err, domain.ErrIdentityExpired) || errors.Is(err, domain.ErrIdentityNotUsable) {
return huma.Error400BadRequest(err.Error())
}
// Malformed owner/account on the owner-scoped destructive paths: a 400
// tells the SCIM outbox worker the event is a data bug to surface, not a
// transient fault to retry forever.
if errors.Is(err, service.ErrInvalidOwnerArgument) {
return huma.Error400BadRequest(err.Error())
}
msg := err.Error()
switch {
case strings.Contains(msg, "no rows in result set"), strings.Contains(msg, "not found"):
Expand Down
67 changes: 67 additions & 0 deletions internal/handler/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,23 @@ type IdentityIDInput struct {
ID string `path:"id" doc:"Identity UUID"`
}

// OffboardByOwnerInput is the request for POST /identities/offboard-by-owner.
// account_id comes from tenant context, never the body — the payload cannot
// steer tenancy (INV-IDN-002 applied to the admin surface).
type OffboardByOwnerInput struct {
Body struct {
OwnerUserID string `json:"owner_user_id" required:"true" minLength:"1" doc:"Stable user ID of the offboarded human (identities.owner_user_id)"`
Reason string `json:"reason,omitempty" maxLength:"256" doc:"Audit reason recorded on the revocations (defaults to owner_deactivated)"`
}
}

type OffboardByOwnerOutput struct {
Body struct {
IdentitiesDeactivated int `json:"identities_deactivated" doc:"Identities deactivated this call — the field to key offboarding evidence and zero-alerts on"`
CredentialsRevoked int64 `json:"credentials_revoked" doc:"Stragglers caught by the final owner-scoped sweep only; ~0 on a healthy run because each identity's deactivation already cascade-revoked its credentials and descendants"`
}
}

// GetIdentityByWIMSEInput is the query for GET /identities/by-wimse.
// The URI is supplied as a query param (rather than a path segment) because
// SPIFFE URIs contain slashes that would conflict with route segmentation
Expand Down Expand Up @@ -152,6 +169,21 @@ func (a *API) registerIdentityRoutes(api huma.API) {
Tags: []string{"Identities"},
}, a.getIdentityByWIMSEOp)

// Literal segment, registered before /identities/{id} like by-wimse above.
huma.Register(api, huma.Operation{
OperationID: "offboard-identities-by-owner",
Method: http.MethodPost,
Path: "/identities/offboard-by-owner",
Summary: "Offboard a human: deactivate every identity they own and cascade-revoke credentials",
Description: "Human-offboarding composition (INV-IDN-010 / ADR 0028): deactivates every " +
"identity owned by the given user in the caller's account — across ALL projects; the " +
"X-Project-ID header is required by the admin surface but not used to scope this sweep — " +
"sweeping each identity's API keys and credentials and emitting retirement CAE signals, " +
"then runs the owner-scoped credential cascade to catch delegated descendants. Idempotent: " +
"safe to retry until 200. Intended caller: admin's SCIM deactivation outbox worker.",
Tags: []string{"Identities"},
}, a.offboardByOwnerOp)

huma.Register(api, huma.Operation{
OperationID: "get-identity",
Method: http.MethodGet,
Expand Down Expand Up @@ -529,6 +561,41 @@ func (a *API) deleteIdentityOp(ctx context.Context, input *IdentityIDInput) (*Id
return &IdentityOutput{Body: identity}, nil
}

// offboardByOwnerOp drives IdentityService.OffboardOwner. Failure mapping is
// retry-oriented for the SCIM outbox worker: a partial application (some
// identities failed to deactivate, or the credential cascade errored after
// some deactivations) returns 502 so the worker retries the idempotent
// operation; only a fully-applied offboarding returns 200.
func (a *API) offboardByOwnerOp(ctx context.Context, input *OffboardByOwnerInput) (*OffboardByOwnerOutput, error) {
tenant, err := internalMiddleware.GetTenant(ctx)
if err != nil {
return nil, huma.Error401Unauthorized("missing tenant context")
}

result, err := a.identitySvc.OffboardOwner(ctx, input.Body.OwnerUserID, tenant.AccountID, input.Body.Reason)
if err != nil {
log.Error().Err(err).
Str("owner_user_id", input.Body.OwnerUserID).
Str("account_id", tenant.AccountID).
Msg("owner offboarding failed or partially applied")
if result == nil {
// Nothing applied: argument guard or list failure.
return nil, mapErr(err)
}
// Partially applied — the worker must retry until it gets a 200.
// Fixed message: the wrapped chain can carry DB driver text, and this
// surface's convention (mapErr) is generic client messages with
// details logged server-side.
return nil, huma.Error502BadGateway("offboarding partially applied; retry (the operation is idempotent)")
}

out := &OffboardByOwnerOutput{}
out.Body.IdentitiesDeactivated = result.IdentitiesDeactivated
out.Body.CredentialsRevoked = result.CredentialsRevoked

return out, nil
}

func (a *API) expireIdentityOp(ctx context.Context, input *IdentityIDInput) (*IdentityOutput, error) {
tenant, err := internalMiddleware.GetTenant(ctx)
if err != nil {
Expand Down
14 changes: 10 additions & 4 deletions internal/service/credential.go
Original file line number Diff line number Diff line change
Expand Up @@ -656,11 +656,17 @@ func (s *CredentialService) RevokeAllActiveForIdentity(ctx context.Context, iden
// relying on that accident holding for a TEXT column. Mirrored in SQL by
// migration 042 so it survives a caller that bypasses this service.
func (s *CredentialService) RevokeAllActiveForOwner(ctx context.Context, ownerUserID, accountID, reason string) (int64, error) {
if ownerUserID == "" || accountID == "" {
// Trim-aware (not just non-empty): a whitespace-only or padded owner
// passes == "" but matches NO stored row (ownerless identities store "";
// VARCHAR equality is exact), so the cascade would "succeed" with zero
// revocations and a broken offboarding integration would look healthy
// indefinitely. Malformed input fails loudly instead.
if strings.TrimSpace(ownerUserID) == "" || strings.TrimSpace(ownerUserID) != ownerUserID ||
strings.TrimSpace(accountID) == "" || strings.TrimSpace(accountID) != accountID {
return 0, fmt.Errorf(
"RevokeAllActiveForOwner requires a non-empty owner_user_id and account_id (got owner=%q account=%q): "+
"an empty owner matches every ownerless identity in the account",
ownerUserID, accountID)
"%w: RevokeAllActiveForOwner requires trimmed, non-empty owner_user_id and account_id (got owner=%q account=%q): "+
"an empty owner matches every ownerless identity in the account, and a padded one silently matches nothing",
ErrInvalidOwnerArgument, ownerUserID, accountID)
}

if reason == "" {
Expand Down
7 changes: 7 additions & 0 deletions internal/service/credential_revoke_owner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ func TestRevokeAllActiveForOwner_RejectsEmptyOwner(t *testing.T) {
{"empty owner", "", "acct-123"},
{"empty account", "user-42", ""},
{"both empty", "", ""},
// Whitespace variants: pass == "" but match no stored row (ownerless
// identities store exactly ""), so the cascade would "succeed" with
// zero revocations — a broken offboarding that looks healthy. Guard
// is trim-aware; migration 043 mirrors it in SQL.
{"whitespace-only owner", " ", "acct-123"},
{"padded owner", " user-42 ", "acct-123"},
{"padded account", "user-42", " acct-123"},
}

for _, tc := range tests {
Expand Down
8 changes: 8 additions & 0 deletions internal/service/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ func isDuplicateKeyError(err error) bool {
return errors.As(err, &pgErr) && pgErr.Field('C') == "23505"
}

// ErrInvalidOwnerArgument marks a caller-fixable owner/account argument on the
// owner-scoped destructive paths (OffboardOwner, RevokeAllActiveForOwner): an
// empty, whitespace-only, or unpadded-vs-stored owner is a malformed request —
// typically a broken SCIM event — not a transient fault. Handlers map it to
// 400 so a retry-driven worker surfaces the event as a data bug instead of
// retrying a "500" forever.
var ErrInvalidOwnerArgument = errors.New("invalid owner-scoped revocation argument")

// IdentityDeactivatedConflictError is returned by RegisterIdentity when the
// external_id collides with an existing identity that is DEACTIVATED (soft
// deleted). Because deletes are soft, the deactivated row keeps the
Expand Down
125 changes: 125 additions & 0 deletions internal/service/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"encoding/pem"
"errors"
"fmt"
"strings"
"time"

"github.com/google/uuid"
Expand Down Expand Up @@ -973,6 +974,130 @@ func (s *IdentityService) DeactivateIdentity(ctx context.Context, id, accountID,
return updated, nil
}

// OffboardResult reports what an owner offboarding actually did, so the caller
// (admin's SCIM outbox worker) can log evidence and decide whether to retry.
type OffboardResult struct {
// IdentitiesDeactivated counts identities the loop successfully drove to
// deactivated. The list excludes already-deactivated rows, so this is the
// fresh count in practice; a row deactivated concurrently between list and
// loop is still counted (DeactivateIdentity no-ops idempotently). This is
// the field the SCIM worker should key evidence and zero-alerts on.
IdentitiesDeactivated int
// FailedIdentityIDs lists identities whose deactivation errored. Non-empty
// means the offboarding is INCOMPLETE and the caller must retry (the whole
// operation is idempotent).
FailedIdentityIDs []string
// CredentialsRevoked is the count from the FINAL owner-scoped cascade
// (revoke_credentials_by_owner) only. Expect it to be ~0 on a healthy run:
// each identity's deactivation cleanup already cascade-revokes that
// identity's credentials AND their delegated descendants
// (revoke_credentials_cascade, migration 031), so the final sweep usually
// finds nothing left. A non-zero value here means the sweep caught
// stragglers — descendants of identities that failed to deactivate, or
// credentials issued mid-loop. Do NOT alert on this being zero.
CredentialsRevoked int64
}

// OffboardOwner is the human-offboarding composition (INV-IDN-010 / ADR 0028
// D5): when the org IdP deactivates a person, every agent identity they own in
// the account — across all projects — is deactivated, and the owner-scoped
// credential cascade sweeps anything delegation reached beyond them.
//
// Order matters and both halves are required:
//
// 1. Deactivate each owned identity via the shared DeactivateIdentity path
// (sweeps linked API keys, cascade-revokes that identity's credentials,
// emits the retirement CAE signal). Credential revocation alone would be
// cosmetic — a surviving zid_sk_* service key just re-exchanges for a
// fresh token via the api_key grant.
// 2. RevokeAllActiveForOwner — the single-statement atomic sweep that also
// catches delegated descendants owned by other humans (parent_jti chain)
// and any credential issued while step 1 was looping.
//
// Failure semantics are retry-oriented, not fail-fast: a per-identity
// deactivation error is recorded and the loop continues, and the credential
// cascade runs regardless, so one bad row cannot shield the rest of the fleet.
// A non-nil error alongside a non-nil result means "partially applied, retry
// me" — every step is idempotent (already-deactivated identities no-op,
// already-revoked credentials emit no duplicate revocation events).
//
// Re-activation is deliberately NOT offered here: a SCIM active:true after an
// offboarding restores org membership only (ADR 0028 D6); agents are never
// silently resurrected.
func (s *IdentityService) OffboardOwner(ctx context.Context, ownerUserID, accountID, reason string) (*OffboardResult, error) {
// Trim-aware, not just non-empty: " " passes a == "" check, matches NO row
// (ownerless identities store "", and Postgres VARCHAR equality is exact),
// and would turn the offboarding into a silent 200 no-op — the SCIM worker
// records success while the departing human's fleet keeps running. A
// padded owner (e.g. "alice ") likewise matches nothing stored as "alice".
// Both are malformed events that must fail loudly (ErrInvalidOwnerArgument
// → 400), not read as healthy.
if strings.TrimSpace(ownerUserID) == "" || strings.TrimSpace(ownerUserID) != ownerUserID ||
strings.TrimSpace(accountID) == "" || strings.TrimSpace(accountID) != accountID {
return nil, fmt.Errorf(
"%w: OffboardOwner requires trimmed, non-empty owner_user_id and account_id (got owner=%q account=%q): "+
"an empty owner matches every ownerless identity in the account, and a padded one silently matches nothing",
ErrInvalidOwnerArgument, ownerUserID, accountID)
}

if reason == "" {
reason = "owner_deactivated"
}

identities, err := s.repo.ListByOwnerForOffboard(ctx, ownerUserID, accountID)
if err != nil {
return nil, err
}

result := &OffboardResult{}
for _, identity := range identities {
// A canceled caller must not burn one error log + one failure append
// per remaining identity on a large fleet; stop and report partial so
// the worker retries.
if ctxErr := ctx.Err(); ctxErr != nil {
result.FailedIdentityIDs = append(result.FailedIdentityIDs, identity.ID)

return result, fmt.Errorf("offboard interrupted after %d deactivations: %w", result.IdentitiesDeactivated, ctxErr)
}

if _, err := s.DeactivateIdentity(ctx, identity.ID, identity.AccountID, identity.ProjectID); err != nil {
log.Error().Err(err).
Str("identity_id", identity.ID).
Str("owner_user_id", ownerUserID).
Str("account_id", accountID).
Msg("offboard: failed to deactivate owned identity")
result.FailedIdentityIDs = append(result.FailedIdentityIDs, identity.ID)
continue
}
result.IdentitiesDeactivated++
}

// The owner-scoped cascade runs even when some deactivations failed — it
// does not depend on identity status and every credential it can reach
// must die on this call, not on the retry.
revoked, err := s.credentialSvc.RevokeAllActiveForOwner(ctx, ownerUserID, accountID, reason)
if err != nil {
return result, fmt.Errorf("offboard: owner-scoped credential cascade failed (deactivated %d identities first): %w",
result.IdentitiesDeactivated, err)
}
result.CredentialsRevoked = revoked

if len(result.FailedIdentityIDs) > 0 {
return result, fmt.Errorf("offboard incomplete: %d of %d identities failed to deactivate — retry (idempotent)",
len(result.FailedIdentityIDs), len(identities))
}

log.Info().
Str("owner_user_id", ownerUserID).
Str("account_id", accountID).
Str("reason", reason).
Int("identities_deactivated", result.IdentitiesDeactivated).
Int64("credentials_revoked", result.CredentialsRevoked).
Msg("offboard: owner offboarding complete")

return result, nil
}

// ExpireIdentity transitions an active identity to expired. It runs the same
// cleanup cascade as deactivation (revoke keys, credentials, emit CAE signal).
func (s *IdentityService) ExpireIdentity(ctx context.Context, id, accountID, projectID string) (*domain.Identity, error) {
Expand Down
108 changes: 108 additions & 0 deletions internal/service/identity_offboard_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package service

import (
"context"
"errors"
"strings"
"testing"
)

// TestOffboardOwner_RejectsEmptyOwnerAndAccount pins the argument guard on the
// human-offboarding composition (INV-IDN-010 / ADR 0028 D5), mirroring the
// guard on RevokeAllActiveForOwner: identities.owner_user_id is NOT NULL, so
// ownerless identities store the empty string, and a blank owner would match
// every ownerless identity in the account — turning "offboard one departing
// human" into a tenant-wide deactivation of exactly the workloads nobody
// watches. The realistic trigger is the intended caller: a SCIM deactivation
// event whose user-id field arrives blank.
//
// The receiver is built with nil repo and nil credentialSvc ON PURPOSE. The
// guard must short-circuit before any repository or credential-service call,
// so a nil-pointer panic here is a real failure signal: it means the guard did
// not fire and execution reached a dependency.
func TestOffboardOwner_RejectsEmptyOwnerAndAccount(t *testing.T) {
t.Parallel()

svc := &IdentityService{} // deps intentionally nil — see doc comment
ctx := context.Background()

tests := []struct {
name string
ownerUser string
accountID string
}{
{"empty owner", "", "acct-123"},
{"empty account", "user-42", ""},
{"both empty", "", ""},
// Whitespace variants pass a bare == "" check but match NO stored row
// (ownerless identities store ""; VARCHAR equality is exact), which
// would turn the offboarding into a silent 200 no-op — the worst
// outcome for an irreversible revocation. They must be rejected too.
{"whitespace-only owner", " ", "acct-123"},
{"tab owner", "\t", "acct-123"},
{"padded owner", " user-42 ", "acct-123"},
{"padded account", "user-42", " acct-123"},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

result, err := svc.OffboardOwner(ctx, tc.ownerUser, tc.accountID, "owner_deactivated")

if err == nil {
t.Fatalf("expected an error for owner=%q account=%q; a blank owner must never "+
"reach the offboarding sweep — it matches every ownerless identity in the account",
tc.ownerUser, tc.accountID)
}
if result != nil {
t.Errorf("result = %+v, want nil — nothing may be reported as applied on the reject path", result)
}
})
}
}

// TestOffboardOwner_GuardPrecedesReasonDefault documents ordering: the argument
// guard runs before the reason default, so a caller passing an empty reason AND
// an empty owner still gets the argument error rather than having the blank
// owner silently carried forward with a defaulted reason.
func TestOffboardOwner_GuardPrecedesReasonDefault(t *testing.T) {
t.Parallel()

svc := &IdentityService{} // nil deps: reaching them would panic
if _, err := svc.OffboardOwner(context.Background(), "", "", ""); err == nil {
t.Fatal("expected an error when owner, account and reason are all empty")
}
}

// TestOffboardOwner_GuardErrorIsTyped pins that guard rejections carry
// ErrInvalidOwnerArgument, so mapErr returns 400 (caller-fixable data bug the
// SCIM worker must surface) rather than 500 (transient, retry forever).
func TestOffboardOwner_GuardErrorIsTyped(t *testing.T) {
t.Parallel()

svc := &IdentityService{}
for _, owner := range []string{"", " ", " alice "} {
_, err := svc.OffboardOwner(context.Background(), owner, "acct-123", "owner_deactivated")
if !errors.Is(err, ErrInvalidOwnerArgument) {
t.Errorf("owner=%q: error must wrap ErrInvalidOwnerArgument; got: %v", owner, err)
}
}
}

// TestOffboardOwner_GuardErrorNamesTheFootgun pins that the guard error
// explains WHY a blank owner is rejected, so an operator staring at a SCIM
// worker's retry log understands the failure is a malformed event, not a
// transient outage to wait out.
func TestOffboardOwner_GuardErrorNamesTheFootgun(t *testing.T) {
t.Parallel()

svc := &IdentityService{}
_, err := svc.OffboardOwner(context.Background(), "", "acct-123", "owner_deactivated")
if err == nil {
t.Fatal("expected an error for empty owner")
}
if !strings.Contains(err.Error(), "ownerless") {
t.Errorf("guard error should name the ownerless-identity footgun; got: %v", err)
}
}
Loading