diff --git a/e2e/console/user_test.go b/e2e/console/user_test.go index 1d45caed4..47c5cec72 100644 --- a/e2e/console/user_test.go +++ b/e2e/console/user_test.go @@ -20,6 +20,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.probo.inc/probo/e2e/internal/factory" "go.probo.inc/probo/e2e/internal/testutil" ) @@ -324,6 +325,67 @@ func TestUser_RemoveUser(t *testing.T) { assert.False(t, removedUserFound, "Should not find removed user") } +func TestUser_RemoveUser_ProfileInUse(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + profileID := factory.CreateUser(owner) + + const createAssetMutation = ` + mutation($input: CreateAssetInput!) { + createAsset(input: $input) { + assetEdge { + node { + id + } + } + } + } + ` + + var createAssetResult struct { + CreateAsset struct { + AssetEdge struct { + Node struct { + ID string `json:"id"` + } `json:"node"` + } `json:"assetEdge"` + } `json:"createAsset"` + } + + err := owner.Execute(createAssetMutation, map[string]any{ + "input": map[string]any{ + "organizationId": owner.GetOrganizationID().String(), + "name": "Production Database Server", + "amount": 1, + "ownerId": profileID, + "assetType": "VIRTUAL", + "dataTypesStored": "Customer PII", + }, + }, &createAssetResult) + require.NoError(t, err) + require.NotEmpty(t, createAssetResult.CreateAsset.AssetEdge.Node.ID) + + const removeUserMutation = ` + mutation($input: RemoveUserInput!) { + removeUser(input: $input) { + deletedProfileId + } + } + ` + + err = owner.ExecuteConnect(removeUserMutation, map[string]any{ + "input": map[string]any{ + "organizationId": owner.GetOrganizationID().String(), + "profileId": profileID, + }, + }, nil) + testutil.RequireErrorCode(t, err, "CONFLICT") + + var gqlErrors testutil.GraphQLErrors + require.ErrorAs(t, err, &gqlErrors) + assert.Contains(t, gqlErrors[0].Message, "referenced by other resources") +} + func TestUser_ArchiveUser(t *testing.T) { t.Parallel() owner := testutil.NewClient(t, testutil.RoleOwner) diff --git a/pkg/coredata/connector.go b/pkg/coredata/connector.go index 12ee477f3..3dfd0a4b7 100644 --- a/pkg/coredata/connector.go +++ b/pkg/coredata/connector.go @@ -323,7 +323,7 @@ WHERE %s AND id = @id _, err := conn.Exec(ctx, q, args) if err != nil { if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok { - if pgErr.Code == "23503" { + if pgErr.Code == "23503" || pgErr.Code == "23001" { return ErrResourceInUse } } diff --git a/pkg/coredata/membership_profile.go b/pkg/coredata/membership_profile.go index c35777f4d..d1c090268 100644 --- a/pkg/coredata/membership_profile.go +++ b/pkg/coredata/membership_profile.go @@ -1263,7 +1263,7 @@ WHERE _, err := conn.Exec(ctx, q, args) if err != nil { if pgErr, ok := errors.AsType[*pgconn.PgError](err); ok { - if pgErr.Code == "23503" { + if pgErr.Code == "23503" || pgErr.Code == "23001" { return ErrResourceInUse } } diff --git a/pkg/iam/errors.go b/pkg/iam/errors.go index f9257c1e1..8a9d9f2d7 100644 --- a/pkg/iam/errors.go +++ b/pkg/iam/errors.go @@ -180,6 +180,18 @@ func (e ErrLastActiveOwner) Error() string { return fmt.Sprintf("cannot remove profile %q: last active owner of the organization", e.MembershipID) } +type ErrProfileInUse struct { + ProfileID gid.GID +} + +func NewProfileInUseError(profileID gid.GID) error { + return &ErrProfileInUse{ProfileID: profileID} +} + +func (e ErrProfileInUse) Error() string { + return fmt.Sprintf("cannot remove profile %q: referenced by other resources", e.ProfileID) +} + 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 de8664b2b..4d6dede31 100644 --- a/pkg/iam/organization_service.go +++ b/pkg/iam/organization_service.go @@ -354,6 +354,10 @@ func (s *OrganizationService) RemoveUser( } if err := profile.Delete(ctx, tx, scope, profileID); err != nil { + if errors.Is(err, coredata.ErrResourceInUse) { + return NewProfileInUseError(profileID) + } + return fmt.Errorf("cannot delete profile: %w", err) } diff --git a/pkg/server/api/connect/v1/profile_resolvers.go b/pkg/server/api/connect/v1/profile_resolvers.go index 8a609184e..c87fbad53 100644 --- a/pkg/server/api/connect/v1/profile_resolvers.go +++ b/pkg/server/api/connect/v1/profile_resolvers.go @@ -145,6 +145,10 @@ func (r *mutationResolver) RemoveUser(ctx context.Context, input types.RemoveUse return nil, gqlutils.Conflictf(ctx, "cannot remove last active owner") } + if _, ok := errors.AsType[*iam.ErrProfileInUse](err); ok { + return nil, gqlutils.Conflictf(ctx, "cannot remove person: referenced by other resources") + } + r.logger.ErrorCtx(ctx, "cannot remove user from organization", log.Error(err)) return nil, gqlutils.Internal(ctx) diff --git a/pkg/server/api/mcp/v1/schema.resolvers.go b/pkg/server/api/mcp/v1/schema.resolvers.go index 1afa93592..d16660187 100644 --- a/pkg/server/api/mcp/v1/schema.resolvers.go +++ b/pkg/server/api/mcp/v1/schema.resolvers.go @@ -2954,6 +2954,10 @@ 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.ErrProfileInUse](err); ok { + return nil, types.RemoveUserOutput{}, fmt.Errorf("cannot remove person: referenced by other resources: %w", err) + } + return nil, types.RemoveUserOutput{}, fmt.Errorf("remove user: %w", err) }