From f604c48686cc0e73ebad86ea3efface71de4f97d Mon Sep 17 00:00:00 2001
From: Sacha Al Himdani <sacha@probo.com>
Date: Wed, 15 Jul 2026 14:30:34 +0200
Subject: [PATCH] Archive SCIM users with in-use profiles instead of 500ing

When a SCIM hard delete targets a profile that is still referenced
(e.g. completed document version signatures, FK RESTRICT), profile.Delete
fails with 23503 and poisons the surrounding transaction. The existing
deactivate fallback then ran on the aborted transaction and failed with
25P02, surfacing to the connector as an opaque 500 and eventually
disabling the bridge.

Wrap profile.Delete in a savepoint so the FK violation only rolls back
the delete attempt, leaving the outer transaction healthy for the
deactivate/archive fallback. Also map FK violations in Membership.Delete
to ErrResourceInUse for consistency with MembershipProfile.Delete.

Signed-off-by: Sacha Al Himdani <sacha@probo.com>
---
 e2e/console/scim_test.go        | 47 +++++++++++++++++++++++++++++++++
 e2e/internal/testutil/client.go |  3 +++
 pkg/coredata/membership.go      |  6 +++++
 pkg/iam/scim/service.go         | 16 ++++++++---
 4 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/e2e/console/scim_test.go b/e2e/console/scim_test.go
index d67d1b9b7..84bda0c4b 100644
--- a/e2e/console/scim_test.go
+++ b/e2e/console/scim_test.go
@@ -270,6 +270,53 @@ func TestSCIM_DeleteUser(t *testing.T) {
 	assert.Equal(t, http.StatusNotFound, status)
 }
 
+// TestSCIM_DeleteUser_ArchivesWhenProfileInUse verifies that deleting a SCIM
+// user whose profile is still referenced by a completed document version
+// signature archives the profile (state INACTIVE) instead of failing. The
+// signature's RESTRICT foreign key makes the hard delete fail; the service must
+// fall back to deactivation and return 204, not surface an opaque 500.
+func TestSCIM_DeleteUser_ArchivesWhenProfileInUse(t *testing.T) {
+	t.Parallel()
+
+	owner := testutil.NewClient(t, testutil.RoleOwner)
+	signer := testutil.NewClientInOrg(t, testutil.RoleEmployee, owner)
+	sc := newSCIMClient(t, owner)
+
+	// The signer signs a published document version, creating a completed
+	// signature that references the signer's profile via a RESTRICT FK.
+	docID, _ := createTestDocument(t, owner)
+	approveTestDocument(t, owner, docID)
+	versionID := latestDocumentVersionID(t, owner, docID)
+
+	requestDocumentSignature(t, owner, versionID, signer.GetProfileID().String())
+
+	_, state, _ := signDocumentVersion(t, signer, versionID)
+	require.Equal(t, "SIGNED", state)
+
+	// Enroll the signer's existing profile into SCIM so it becomes SCIM-managed
+	// (same underlying profile ID, source flipped to SCIM).
+	body, status := sc.createUser(signer.GetEmail(), "Signer User", "ext-signed-1", true)
+	require.Equal(t, http.StatusCreated, status, body)
+
+	var enrolled map[string]any
+	require.NoError(t, json.Unmarshal([]byte(body), &enrolled))
+	scimUserID := enrolled["id"].(string)
+	require.Equal(t, signer.GetProfileID().String(), scimUserID)
+
+	// Deleting the in-use profile must archive it, not 500.
+	body, status = sc.deleteUser(scimUserID)
+	require.Equal(t, http.StatusNoContent, status, body)
+
+	// The profile is archived (deactivated), not hard-deleted: it is still
+	// present but inactive, and the completed signature is preserved.
+	body, status = sc.getUser(scimUserID)
+	require.Equal(t, http.StatusOK, status, body)
+
+	var fetched map[string]any
+	require.NoError(t, json.Unmarshal([]byte(body), &fetched))
+	assert.Equal(t, false, fetched["active"], "profile should be archived (inactive), not deleted")
+}
+
 func TestSCIM_Unauthorized(t *testing.T) {
 	t.Parallel()
 
diff --git a/e2e/internal/testutil/client.go b/e2e/internal/testutil/client.go
index ff03874a5..16f0ef351 100644
--- a/e2e/internal/testutil/client.go
+++ b/e2e/internal/testutil/client.go
@@ -141,6 +141,9 @@ func (c *Client) SetupTestUserInOrg(ownerClient *Client) {
 	password := "TestPassword123!"
 	fullName := fmt.Sprintf("Test User %s", uniqueID)
 
+	c.email = email
+	c.password = password
+
 	// Owner invites user to organization
 	profileID, identityID := ownerClient.createUser(email, fullName, coredata.MembershipRole(c.role))
 	c.userID = identityID
diff --git a/pkg/coredata/membership.go b/pkg/coredata/membership.go
index 221770bc5..366859be0 100644
--- a/pkg/coredata/membership.go
+++ b/pkg/coredata/membership.go
@@ -370,6 +370,12 @@ WHERE
 
 	_, err := conn.Exec(ctx, query, args)
 	if err != nil {
+		if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok {
+			if pgErr.Code == "23503" || pgErr.Code == "23001" {
+				return ErrResourceInUse
+			}
+		}
+
 		return fmt.Errorf("cannot delete membership: %w", err)
 	}
 
diff --git a/pkg/iam/scim/service.go b/pkg/iam/scim/service.go
index 3efb11458..c45d4bc1e 100644
--- a/pkg/iam/scim/service.go
+++ b/pkg/iam/scim/service.go
@@ -892,8 +892,18 @@ func (s *Service) DeleteUser(
 				membership = m
 			}
 
-			if err := profile.Delete(ctx, tx, scope, profile.ID); err != nil {
-				if errors.Is(err, coredata.ErrResourceInUse) {
+			deleteErr := tx.Savepoint(
+				ctx,
+				func(ctx context.Context, sp pg.Tx) error {
+					if err := profile.Delete(ctx, sp, scope, profile.ID); err != nil {
+						return fmt.Errorf("cannot delete profile: %w", err)
+					}
+
+					return nil
+				},
+			)
+			if deleteErr != nil {
+				if errors.Is(deleteErr, coredata.ErrResourceInUse) {
 					s.logger.WarnCtx(
 						ctx,
 						"SCIM user delete skipped, profile is in use",
@@ -907,7 +917,7 @@ func (s *Service) DeleteUser(
 					return nil
 				}
 
-				return fmt.Errorf("cannot delete profile: %w", err)
+				return fmt.Errorf("cannot delete profile in savepoint: %w", deleteErr)
 			}
 
 			invitations := &coredata.Invitations{}