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>
This commit is contained in:
@@ -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()
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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{}
|
||||
|
||||
Reference in New Issue
Block a user