From b50bbc8d6ad458b84ba5f7c7d2b342d78282a5a1 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 27 May 2026 00:32:16 +0000 Subject: [PATCH] Archive manual users on remove Switch remove-user behavior for manually managed profiles from hard\ndelete to archival by deactivating the profile. This matches the\nrequested SCIM-like lifecycle while avoiding dependency errors for\nlinked records such as signatures and assets.\n\nThe remove flow now updates profile state to INACTIVE, updates\nmembership timestamps, and emits a user-updated webhook event instead of\ndelete events. E2E coverage now asserts that remove keeps the profile\nand marks it inactive. Signed-off-by: Cursor Agent --- e2e/console/user_test.go | 20 ++++ pkg/iam/errors.go | 12 --- pkg/iam/organization_service.go | 47 ++++------ pkg/iam/organization_service_test.go | 92 ------------------- .../api/connect/v1/profile_resolvers.go | 4 - pkg/server/api/mcp/v1/schema.resolvers.go | 4 - 6 files changed, 36 insertions(+), 143 deletions(-) delete mode 100644 pkg/iam/organization_service_test.go diff --git a/e2e/console/user_test.go b/e2e/console/user_test.go index d5e199261..ada50e79d 100644 --- a/e2e/console/user_test.go +++ b/e2e/console/user_test.go @@ -131,6 +131,7 @@ func TestUser_RemoveUser(t *testing.T) { edges { node { id + state membership { role } @@ -148,6 +149,7 @@ func TestUser_RemoveUser(t *testing.T) { Edges []struct { Node struct { ID string `json:"id"` + State string `json:"state"` Membership struct { Role string `json:"role"` } `json:"membership"` @@ -198,6 +200,24 @@ func TestUser_RemoveUser(t *testing.T) { require.NoError(t, err) assert.Equal(t, userID, mutationResult.RemoveUser.DeletedProfileID) + + // Remove archives the user instead of hard-deleting them. + err = owner.ExecuteConnect(query, map[string]any{ + "id": owner.GetOrganizationID().String(), + }, &result) + require.NoError(t, err) + + var removedUserState string + + for _, edge := range result.Node.Profiles.Edges { + if edge.Node.ID == userID { + removedUserState = edge.Node.State + break + } + } + + require.NotEmpty(t, removedUserState, "Should still find archived user") + assert.Equal(t, "INACTIVE", removedUserState) } func TestUser_RemoveOwner(t *testing.T) { diff --git a/pkg/iam/errors.go b/pkg/iam/errors.go index fe398b67f..571e18c76 100644 --- a/pkg/iam/errors.go +++ b/pkg/iam/errors.go @@ -169,18 +169,6 @@ func (e ErrLastActiveOwner) Error() string { return fmt.Sprintf("cannot remove profile %q: last active owner of the organization", e.MembershipID) } -type ErrUserReferencedByRecords struct { - ProfileID gid.GID -} - -func NewUserReferencedByRecordsError(profileID gid.GID) error { - return &ErrUserReferencedByRecords{ProfileID: profileID} -} - -func (e ErrUserReferencedByRecords) Error() string { - return "cannot remove user because they are referenced by existing records (for example signatures, tasks, assets, or risks)" -} - type ErrOrganizationNotFound struct{ OrganizationID gid.GID } func NewOrganizationNotFoundError(organizationID gid.GID) error { diff --git a/pkg/iam/organization_service.go b/pkg/iam/organization_service.go index 99619c042..07df36fb9 100644 --- a/pkg/iam/organization_service.go +++ b/pkg/iam/organization_service.go @@ -21,7 +21,6 @@ import ( "io" "time" - "github.com/jackc/pgx/v5/pgconn" "go.gearno.de/crypto/uuid" "go.gearno.de/kit/pg" "go.probo.inc/probo/packages/emails" @@ -350,44 +349,30 @@ func (s *OrganizationService) RemoveUser( } } - if err := webhook.InsertData(ctx, tx, scope, organizationID, coredata.WebhookEventTypeUserDeleted, webhooktypes.NewUser(&profile, membership)); err != nil { + now := time.Now() + if profile.State != coredata.ProfileStateInactive { + profile.State = coredata.ProfileStateInactive + profile.UpdatedAt = now + + if err := profile.Update(ctx, tx, scope); err != nil { + return fmt.Errorf("cannot update profile state: %w", err) + } + } + + membership.UpdatedAt = now + if err := membership.Update(ctx, tx, scope); err != nil { + return fmt.Errorf("cannot update membership: %w", err) + } + + if err := webhook.InsertData(ctx, tx, scope, organizationID, coredata.WebhookEventTypeUserUpdated, webhooktypes.NewUser(&profile, membership)); err != nil { return fmt.Errorf("cannot insert webhook event: %w", err) } - if err := profile.Delete(ctx, tx, scope, profileID); err != nil { - if isUserRemovalDependencyError(err) { - return NewUserReferencedByRecordsError(profileID) - } - - return fmt.Errorf("cannot delete profile: %w", err) - } - - if err := membership.Delete(ctx, tx, scope, membership.ID); err != nil { - if isUserRemovalDependencyError(err) { - return NewUserReferencedByRecordsError(profileID) - } - - return fmt.Errorf("cannot delete membership: %w", err) - } - return nil }, ) } -func isUserRemovalDependencyError(err error) bool { - if errors.Is(err, coredata.ErrResourceInUse) { - return true - } - - pgErr, ok := errors.AsType[*pgconn.PgError](err) - if !ok { - return false - } - - return pgErr.Code == "23503" -} - func (s *OrganizationService) InviteUser( ctx context.Context, req *CreateInvitationRequest, diff --git a/pkg/iam/organization_service_test.go b/pkg/iam/organization_service_test.go deleted file mode 100644 index 940c62457..000000000 --- a/pkg/iam/organization_service_test.go +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright (c) 2026 Probo Inc . -// -// Permission to use, copy, modify, and/or distribute this software for any -// purpose with or without fee is hereby granted, provided that the above -// copyright notice and this permission notice appear in all copies. -// -// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH -// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY -// AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, -// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM -// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR -// OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR -// PERFORMANCE OF THIS SOFTWARE. - -package iam - -import ( - "errors" - "fmt" - "testing" - - "github.com/jackc/pgx/v5/pgconn" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "go.probo.inc/probo/pkg/coredata" - "go.probo.inc/probo/pkg/gid" -) - -func TestIsUserRemovalDependencyError(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - err error - want bool - }{ - { - name: "returns true for sentinel error", - err: coredata.ErrResourceInUse, - want: true, - }, - { - name: "returns true for wrapped sentinel error", - err: fmt.Errorf("wrapped: %w", coredata.ErrResourceInUse), - want: true, - }, - { - name: "returns true for wrapped postgres foreign key error", - err: fmt.Errorf( - "wrapped: %w", - &pgconn.PgError{Code: "23503"}, - ), - want: true, - }, - { - name: "returns false for non foreign key postgres error", - err: fmt.Errorf( - "wrapped: %w", - &pgconn.PgError{Code: "23505"}, - ), - want: false, - }, - { - name: "returns false for unrelated error", - err: errors.New("boom"), - want: false, - }, - } - - for _, tt := range tests { - tt := tt - - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - assert.Equal(t, tt.want, isUserRemovalDependencyError(tt.err)) - }) - } -} - -func TestNewUserReferencedByRecordsError_Message(t *testing.T) { - t.Parallel() - - err := NewUserReferencedByRecordsError(gid.Nil) - resourceErr, ok := errors.AsType[*ErrUserReferencedByRecords](err) - require.True(t, ok) - assert.Equal( - t, - "cannot remove user because they are referenced by existing records (for example signatures, tasks, assets, or risks)", - resourceErr.Error(), - ) -} diff --git a/pkg/server/api/connect/v1/profile_resolvers.go b/pkg/server/api/connect/v1/profile_resolvers.go index ad1839287..26fe2caa7 100644 --- a/pkg/server/api/connect/v1/profile_resolvers.go +++ b/pkg/server/api/connect/v1/profile_resolvers.go @@ -120,10 +120,6 @@ func (r *mutationResolver) RemoveUser(ctx context.Context, input types.RemoveUse return nil, gqlutils.Conflict(ctx, err) } - if _, ok := errors.AsType[*iam.ErrUserReferencedByRecords](err); ok { - return nil, gqlutils.Conflict(ctx, err) - } - if errors.Is(err, coredata.ErrResourceInUse) { return nil, gqlutils.Conflict(ctx, err) } diff --git a/pkg/server/api/mcp/v1/schema.resolvers.go b/pkg/server/api/mcp/v1/schema.resolvers.go index 7022795bc..1063e8fef 100644 --- a/pkg/server/api/mcp/v1/schema.resolvers.go +++ b/pkg/server/api/mcp/v1/schema.resolvers.go @@ -2930,10 +2930,6 @@ func (r *Resolver) RemoveUserTool(ctx context.Context, req *mcp.CallToolRequest, return nil, types.RemoveUserOutput{}, fmt.Errorf("cannot remove last active owner: %w", err) } - if _, ok := errors.AsType[*iam.ErrUserReferencedByRecords](err); ok { - return nil, types.RemoveUserOutput{}, fmt.Errorf("cannot remove user: %w", err) - } - if errors.Is(err, coredata.ErrResourceInUse) { return nil, types.RemoveUserOutput{}, fmt.Errorf("cannot remove user: %w", err) }