fix(storage): cascade DeleteUser to all user-keyed tables

internal/integration_tests/delete_user_cascade_test.go

@@ -0,0 +1,111 @@
+package integration_tests
+
+import (
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/authorizerdev/authorizer/internal/authorization/engine"
+	"github.com/authorizerdev/authorizer/internal/graph/model"
+	"github.com/authorizerdev/authorizer/internal/storage/schemas"
+)
+
+// TestDeleteUserCascade proves a hard delete (_delete_user) removes every piece
+// of state the account owned: the six user-keyed tables and its FGA grants.
+//
+// Before this, only sessions were cascaded. The federated-identity orphan was
+// the worst of it — it points at a dead user id, jitProvisionFederatedUser fails
+// closed on that branch, and the unique (org_id, issuer, subject) triple blocks
+// re-provisioning, so the SSO principal is locked out permanently.
+func TestDeleteUserCascade(t *testing.T) {
+	cfg := getTestConfig()
+	ts, eng := initFGATestSetup(t, cfg)
+	_, ctx := createContext(ts)
+
+	email := "delete_cascade_" + uuid.New().String() + "@authorizer.dev"
+	password := "Password@123"
+	signupRes, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{
+		Email: &email, Password: password, ConfirmPassword: password,
+	})
+	require.NoError(t, err)
+	require.NotNil(t, signupRes.User)
+	userID := signupRes.User.ID
+
+	sp := ts.StorageProvider
+	orgID := "org_" + uuid.New().String()
+	issuer := "https://idp.example.com"
+	subject := "upstream_" + uuid.New().String()
+
+	require.NoError(t, sp.AddSession(ctx, &schemas.Session{UserID: userID}))
+	_, err = sp.AddFederatedIdentity(ctx, &schemas.FederatedIdentity{
+		OrgID: orgID, Issuer: issuer, Subject: subject, UserID: userID,
+	})
+	require.NoError(t, err)
+	_, err = sp.AddOrgMembership(ctx, &schemas.OrgMembership{OrgID: orgID, UserID: userID, Roles: "member"})
+	require.NoError(t, err)
+	_, err = sp.AddAuthenticator(ctx, &schemas.Authenticator{UserID: userID, Method: "totp", Secret: "s3cret"})
+	require.NoError(t, err)
+	_, err = sp.AddWebauthnCredential(ctx, &schemas.WebauthnCredential{
+		UserID: userID, CredentialID: "cred_" + uuid.New().String(), PublicKey: "pk", Name: "laptop",
+	})
+	require.NoError(t, err)
+	require.NoError(t, sp.AddSessionToken(ctx, &schemas.SessionToken{UserID: userID, KeyName: "access", Token: "t"}))
+	require.NoError(t, sp.AddMFASession(ctx, &schemas.MFASession{UserID: userID, KeyName: "mfa"}))
+
+	// An FGA grant held by this user, which lives outside StorageProvider.
+	setAdminCookie(t, ts)
+	_, err = ts.GraphQLProvider.FgaWriteModel(ctx, &model.FgaWriteModelInput{Dsl: fgaTestModel})
+	require.NoError(t, err)
+	require.NoError(t, eng.WriteTuples(ctx, []engine.TupleKey{
+		{User: "user:" + userID, Relation: "viewer", Object: "document:secret"},
+	}))
+	clearCookies(ts)
+
+	setAdminCookie(t, ts)
+	deleteRes, err := ts.GraphQLProvider.DeleteUser(ctx, &model.DeleteUserRequest{Email: email})
+	require.NoError(t, err)
+	require.NotNil(t, deleteRes)
+
+	t.Run("federated identity is gone and the principal can be re-provisioned", func(t *testing.T) {
+		_, err := sp.GetFederatedIdentity(ctx, orgID, issuer, subject)
+		require.Error(t, err, "orphaned federated identity is a permanent SSO lockout")
+
+		fresh, err := sp.AddUser(ctx, &schemas.User{
+			Email:         &[]string{"reprovision_" + uuid.New().String() + "@authorizer.dev"}[0],
+			SignupMethods: "basic_auth",
+		})
+		require.NoError(t, err)
+		_, err = sp.AddFederatedIdentity(ctx, &schemas.FederatedIdentity{
+			OrgID: orgID, Issuer: issuer, Subject: subject, UserID: fresh.ID,
+		})
+		require.NoError(t, err, "the (org, issuer, subject) triple must be free again")
+	})
+
+	t.Run("org membership, authenticator and passkey are gone", func(t *testing.T) {
+		_, err := sp.GetOrgMembership(ctx, orgID, userID)
+		assert.Error(t, err)
+		_, err = sp.GetAuthenticatorDetailsByUserId(ctx, userID, "totp")
+		assert.Error(t, err)
+		creds, err := sp.ListWebauthnCredentialsByUserID(ctx, userID)
+		require.NoError(t, err)
+		assert.Empty(t, creds)
+	})
+
+	t.Run("session tokens and mfa sessions are gone", func(t *testing.T) {
+		_, err := sp.GetSessionTokenByUserIDAndKey(ctx, userID, "access")
+		assert.Error(t, err)
+		sessions, err := sp.GetAllMFASessionsByUserID(ctx, userID)
+		require.NoError(t, err)
+		assert.Empty(t, sessions)
+	})
+
+	t.Run("fga tuples are gone", func(t *testing.T) {
+		res, err := eng.ReadTuples(ctx, engine.ReadTuplesFilter{Object: "document:secret"})
+		require.NoError(t, err)
+		for _, tk := range res.Tuples {
+			assert.NotEqual(t, "user:"+userID, tk.User, "deleted user must not keep holding grants")
+		}
+	})
+}

internal/service/admin_users.go

@@ -132,6 +132,13 @@ func (p *provider) UpdateUser(ctx context.Context, meta RequestMetadata, params
 		params.Roles == nil &&
 		params.IsMultiFactorAuthEnabled == nil &&
 		params.ResetMfa == nil &&
+		// EmailVerified/PhoneNumberVerified were missing from this gate even
+		// though both are applied further down, so an admin force-verifying an
+		// address — the operator's escape hatch when a user cannot receive mail
+		// — was rejected with "please enter atleast one param to update" unless
+		// they padded the call with an unrelated field.
+		params.EmailVerified == nil &&
+		params.PhoneNumberVerified == nil &&
 		params.AppData == nil {
 		log.Debug().Msg("please enter atleast one param to update")
 		return nil, nil, InvalidArgument("please enter atleast one param to update")
@@ -393,6 +400,15 @@ func (p *provider) DeleteUser(ctx context.Context, meta RequestMetadata, params
 		return nil, nil, err
 	}
 
+	// FGA tuples live outside StorageProvider, so the storage cascade cannot
+	// reach them. Purge synchronously (this is security cleanup, and callers
+	// must not observe a deleted user still holding grants) but best-effort: a
+	// tuple-store failure is logged, not returned — the user row is already gone
+	// and failing here would report a delete that did happen as failed.
+	if err := p.purgeFgaTuplesForUser(ctx, user.ID); err != nil {
+		log.Warn().Err(err).Str("user_id", user.ID).Msg("Failed to purge FGA tuples for deleted user; grants may be orphaned")
+	}
+
 	res := &model.Response{
 		Message: `user deleted successfully`,
 	}

internal/service/fga.go

@@ -345,3 +345,55 @@ func (p *provider) enforceRequiredRelations(ctx context.Context, meta RequestMet
 	}
 	return nil
 }
+
+// fgaPurgePageSize is the ReadTuples page size used when scanning the store for
+// a deleted user's grants.
+const fgaPurgePageSize = 100
+
+// purgeFgaTuplesForUser removes every relationship tuple naming the given user,
+// so a hard-deleted account cannot keep holding grants. A no-op when FGA is not
+// enabled.
+//
+// ponytail: this pages through the WHOLE tuple store and matches client-side,
+// because OpenFGA's Read API rejects a user-only filter — it requires at least
+// an object type ("the 'tuple_key' field was provided but the object type field
+// is required"), and we do not know which types a user appears under. That is
+// fine for an admin delete-user call; the upgrade path, if the store grows big
+// enough to matter, is to enumerate the model's type definitions via ReadModel
+// and issue one filtered Read per type.
+func (p *provider) purgeFgaTuplesForUser(ctx context.Context, userID string) error {
+	if p.AuthzEngine == nil || strings.TrimSpace(userID) == "" {
+		return nil
+	}
+	subject := "user:" + userID
+	var stale []engine.TupleKey
+	contToken := ""
+	for {
+		res, err := p.AuthzEngine.ReadTuples(ctx, engine.ReadTuplesFilter{
+			PageSize:          fgaPurgePageSize,
+			ContinuationToken: contToken,
+		})
+		if err != nil {
+			return fmt.Errorf("read tuples: %w", err)
+		}
+		for _, t := range res.Tuples {
+			// Match the user as subject and as object: a model may name the
+			// account on either side, and leaving either behind is a live grant
+			// pointing at a dead id.
+			if t.User == subject || t.Object == subject {
+				stale = append(stale, t)
+			}
+		}
+		if res.ContinuationToken == "" {
+			break
+		}
+		contToken = res.ContinuationToken
+	}
+	if len(stale) == 0 {
+		return nil
+	}
+	if err := p.AuthzEngine.DeleteTuples(ctx, stale); err != nil {
+		return fmt.Errorf("delete %d tuples: %w", len(stale), err)
+	}
+	return nil
+}

internal/storage/db/arangodb/user.go

@@ -73,25 +73,27 @@ func (p *provider) UpdateUser(ctx context.Context, user *schemas.User) (*schemas
 
 // DeleteUser to delete user information from database
 func (p *provider) DeleteUser(ctx context.Context, user *schemas.User) error {
-	collection, _ := p.db.Collection(ctx, schemas.Collections.User)
-	_, err := collection.RemoveDocument(ctx, user.Key)
-	if err != nil {
-		return err
-	}
-	query := fmt.Sprintf(`FOR d IN %s FILTER d.user_id == @user_id REMOVE { _key: d._key } IN %s`, schemas.Collections.Session, schemas.Collections.Session)
+	// Children first, user row last: ArangoDB runs these as separate queries, so
+	// a partial failure must leave the user row intact and retryable rather than
+	// stranding orphans that point at a dead id (see
+	// schemas.UserOwnedCollections).
 	bindVars := map[string]interface{}{
-		// Session.UserID is stored as the full document handle (collection/key),
-		// which is what user.ID holds after Add/Get. Binding user.Key (bare key)
-		// would match zero session rows. This cascade is the only session-cleanup
-		// path on user deletion.
+		// user_id is stored as the full document handle (collection/key), which
+		// is what user.ID holds after Add/Get. Binding user.Key (bare key) would
+		// match zero rows. This cascade is the only cleanup path on user deletion.
 		"user_id": user.ID,
 	}
-	cursor, err := p.db.Query(ctx, query, bindVars)
-	if err != nil {
-		return err
+	for _, collectionName := range schemas.UserOwnedCollections {
+		query := fmt.Sprintf(`FOR d IN %s FILTER d.user_id == @user_id REMOVE { _key: d._key } IN %s`, collectionName, collectionName)
+		cursor, err := p.db.Query(ctx, query, bindVars)
+		if err != nil {
+			return err
+		}
+		_ = cursor.Close()
 	}
-	defer func() { _ = cursor.Close() }()
-	return nil
+	collection, _ := p.db.Collection(ctx, schemas.Collections.User)
+	_, err := collection.RemoveDocument(ctx, user.Key)
+	return err
 }
 
 // ListUsers to get list of users from database

internal/storage/db/cassandradb/user.go

@@ -116,37 +116,40 @@ func (p *provider) UpdateUser(ctx context.Context, user *schemas.User) (*schemas
 
 // DeleteUser to delete user information from database
 func (p *provider) DeleteUser(ctx context.Context, user *schemas.User) error {
-	query := fmt.Sprintf("DELETE FROM %s WHERE id = ?", KeySpace+"."+schemas.Collections.User)
-	err := p.db.Query(query, user.ID).Exec()
-	if err != nil {
-		return err
-	}
-	getSessionsQuery := fmt.Sprintf("SELECT id FROM %s WHERE user_id = ? ALLOW FILTERING", KeySpace+"."+schemas.Collections.Session)
-	scanner := p.db.Query(getSessionsQuery, user.ID).Iter().Scanner()
-	var sessionIDList []string
-	for scanner.Next() {
-		var wlID string
-		err = scanner.Scan(&wlID)
-		if err != nil {
+	// Children first, user row last: Cassandra has no cross-table transaction, so
+	// a partial failure must leave the user row intact and retryable rather than
+	// stranding orphans that point at a dead id (see
+	// schemas.UserOwnedCollections). Every one of these tables is keyed on `id`.
+	for _, table := range schemas.UserOwnedCollections {
+		selectQuery := fmt.Sprintf("SELECT id FROM %s WHERE user_id = ? ALLOW FILTERING", KeySpace+"."+table)
+		scanner := p.db.Query(selectQuery, user.ID).Iter().Scanner()
+		var ids []string
+		for scanner.Next() {
+			var id string
+			if err := scanner.Scan(&id); err != nil {
+				return err
+			}
+			ids = append(ids, id)
+		}
+		if err := scanner.Err(); err != nil {
 			return err
 		}
-		sessionIDList = append(sessionIDList, wlID)
-	}
-	if len(sessionIDList) > 0 {
-		placeholders := strings.Repeat("?,", len(sessionIDList))
-		placeholders = strings.TrimSuffix(placeholders, ",")
-		deleteValues := make([]interface{}, len(sessionIDList))
-		for i, id := range sessionIDList {
+		if len(ids) == 0 {
+			continue
+		}
+		placeholders := strings.TrimSuffix(strings.Repeat("?,", len(ids)), ",")
+		deleteValues := make([]interface{}, len(ids))
+		for i, id := range ids {
 			deleteValues[i] = id
 		}
-		deleteSessionQuery := fmt.Sprintf("DELETE FROM %s WHERE id IN (%s)", KeySpace+"."+schemas.Collections.Session, placeholders)
-		err = p.db.Query(deleteSessionQuery, deleteValues...).Exec()
-		if err != nil {
+		deleteQuery := fmt.Sprintf("DELETE FROM %s WHERE id IN (%s)", KeySpace+"."+table, placeholders)
+		if err := p.db.Query(deleteQuery, deleteValues...).Exec(); err != nil {
 			return err
 		}
 	}
 
-	return nil
+	query := fmt.Sprintf("DELETE FROM %s WHERE id = ?", KeySpace+"."+schemas.Collections.User)
+	return p.db.Query(query, user.ID).Exec()
 }
 
 // ListUsers to get list of users from database

internal/storage/db/couchbase/user.go

@@ -75,14 +75,26 @@ func (p *provider) UpdateUser(ctx context.Context, user *schemas.User) (*schemas
 
 // DeleteUser to delete user information from database
 func (p *provider) DeleteUser(ctx context.Context, user *schemas.User) error {
+	// Children first, user row last: Couchbase runs these as separate statements,
+	// so a partial failure must leave the user row intact and retryable rather
+	// than stranding orphans that point at a dead id (see
+	// schemas.UserOwnedCollections). Sessions were not cleaned up here at all
+	// before — every other backend did it, this one did not.
+	for _, collection := range schemas.UserOwnedCollections {
+		query := fmt.Sprintf("DELETE FROM %s.%s WHERE user_id = $1", p.scopeName, collection)
+		if _, err := p.db.Query(query, &gocb.QueryOptions{
+			ScanConsistency:      gocb.QueryScanConsistencyRequestPlus,
+			Context:              ctx,
+			PositionalParameters: []interface{}{user.ID},
+		}); err != nil {
+			return err
+		}
+	}
 	removeOpt := gocb.RemoveOptions{
 		Context: ctx,
 	}
 	_, err := p.db.Collection(schemas.Collections.User).Remove(user.ID, &removeOpt)
-	if err != nil {
-		return err
-	}
-	return nil
+	return err
 }
 
 // ListUsers to get list of users from database

internal/storage/db/dynamodb/user.go

@@ -141,23 +141,28 @@ func (p *provider) DeleteUser(ctx context.Context, user *schemas.User) error {
 	if user.ID == "" {
 		return nil
 	}
-	if err := p.deleteItemByHash(ctx, schemas.Collections.User, "id", user.ID); err != nil {
-		return err
-	}
-	items, err := p.queryEq(ctx, schemas.Collections.Session, "user_id", "user_id", user.ID, nil)
-	if err != nil {
-		return err
-	}
-	for _, it := range items {
-		var s schemas.Session
-		if err := unmarshalItem(it, &s); err != nil {
+	// Children first, user row last: DynamoDB has no transaction here, so a
+	// partial failure must leave the user row intact and retryable rather than
+	// stranding orphans that point at a dead id (see
+	// schemas.UserOwnedCollections). Every one of these tables hashes on "id" and
+	// carries a "user_id" GSI (see tables.go), so one query/delete loop serves
+	// them all.
+	for _, table := range schemas.UserOwnedCollections {
+		items, err := p.queryEq(ctx, table, "user_id", "user_id", user.ID, nil)
+		if err != nil {
 			return err
 		}
-		if err := p.deleteItemByHash(ctx, schemas.Collections.Session, "id", s.ID); err != nil {
-			return err
+		for _, it := range items {
+			id, ok := it["id"].(*types.AttributeValueMemberS)
+			if !ok || id.Value == "" {
+				continue
+			}
+			if err := p.deleteItemByHash(ctx, table, "id", id.Value); err != nil {
+				return err
+			}
 		}
 	}
-	return nil
+	return p.deleteItemByHash(ctx, schemas.Collections.User, "id", user.ID)
 }
 
 // ListUsers to get list of users from database