Skip to content

DeleteUser cascade is incomplete: orphaned FGA tuples, org memberships, authenticators, passkeys and federated identities (SSO lockout) #747

Description

@lakhansamani

Summary

StorageProvider.DeleteUser cascades to sessions only. Every other row keyed on the user id survives the delete, so _delete_user leaves orphaned records across six tables. One of them causes a permanent SSO lockout.

Current behaviour

internal/storage/db/sql/user.go:

func (p *provider) DeleteUser(ctx context.Context, user *schemas.User) error {
	err := p.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
		if err := tx.Where("user_id = ?", user.ID).Delete(&schemas.Session{}).Error; err != nil {
			return err
		}
		return tx.Delete(&user).Error
	})
	...
}

The admin service path (internal/service/admin_users.go DeleteUser) additionally cleans up OTPs and verification requests, asynchronously and best-effort. Nothing cleans up the rest.

What is orphaned

Table / store Schema Impact
FGA tuples (OpenFGA store) user:<dead-id> grants persist forever. Not cleaned by any cascade; ReadTuples/ListUsers return a dead subject.
authorizer_federated_identities schemas.FederatedIdentity Permanent SSO lockout — see below.
authorizer_org_memberships schemas.OrgMembership Phantom members; inflated org member counts.
authorizer_authenticators schemas.Authenticator Orphaned TOTP secrets and recovery codes at rest.
authorizer_webauthn_credentials schemas.WebauthnCredential Orphaned passkeys.
authorizer_session_tokens, authorizer_mfa_sessions schemas.SessionToken, schemas.MfaSession Orphaned rows.

The federated-identity orphan is a hard lockout

jitProvisionFederatedUser (internal/http_handlers/oauth_sso.go) resolves a returning SSO principal through the (org_id, issuer, subject) row:

if fi, err := h.StorageProvider.GetFederatedIdentity(ctx, orgID, issuer, subject); err == nil && fi != nil {
	user, err := h.StorageProvider.GetUserByID(ctx, fi.UserID)
	if err != nil || user == nil {
		return nil, false, fmt.Errorf("federated identity references an unknown user")
	}
	...
}

After the user row is deleted, the FI row survives and points at a dead id. Every subsequent SSO login for that principal fails closed on this branch. Re-provisioning cannot recover it either: the (org_id, issuer, subject) triple is unique, so AddFederatedIdentity would collide. The principal is locked out of SSO permanently, and deleting the row is the only fix.

Proposed fix

Extend the storage-level DeleteUser cascade to cover every user-keyed table, inside the existing transaction:

  • authorizer_federated_identities
  • authorizer_org_memberships
  • authorizer_authenticators
  • authorizer_webauthn_credentials
  • authorizer_session_tokens
  • authorizer_mfa_sessions

FGA tuples live outside storage, so they need a separate step in the service layer (ReadTuples filtered by user:<id>, then DeleteTuples), best-effort and logged — a deleted user must not be left holding grants.

Per AGENTS.md, this must be implemented identically across all six backends (SQL, MongoDB, ArangoDB, Cassandra/ScyllaDB, DynamoDB, Couchbase) — a cascade present in one backend and missing in another is a parity bug. Verify with at least one non-SQL backend, not just SQLite.

Test coverage to add

  • Create a user with an org membership, a passkey, a TOTP authenticator, a federated identity and an FGA grant; delete; assert every row is gone and no FGA tuple remains for that subject.
  • Regression for the lockout: create a federated identity, delete the user, re-run the SSO login for the same (org, issuer, subject) — it must provision a fresh account rather than fail closed.
  • Cross-backend parity, mirroring TestNotFoundContractIsUniform.

Context

Found while bounding the OAuth pre-hijack delete during the 2.4.0 pre-release security audit. That path now refuses to delete an account holding any state (accountHasState, internal/http_handlers/oauth_account_state.go) precisely because this cascade is incomplete — but that only bounds the one caller. _delete_user still has the full problem.

Noted in docs/email-verification-contract.md under "The pre-hijack delete is now bounded".

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions