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 <cursoragent@cursor.com>
This commit is contained in:
committed by
Bryan Frimin
parent
ffa3db3cd2
commit
b50bbc8d6a
@@ -131,6 +131,7 @@ func TestUser_RemoveUser(t *testing.T) {
|
|||||||
edges {
|
edges {
|
||||||
node {
|
node {
|
||||||
id
|
id
|
||||||
|
state
|
||||||
membership {
|
membership {
|
||||||
role
|
role
|
||||||
}
|
}
|
||||||
@@ -148,6 +149,7 @@ func TestUser_RemoveUser(t *testing.T) {
|
|||||||
Edges []struct {
|
Edges []struct {
|
||||||
Node struct {
|
Node struct {
|
||||||
ID string `json:"id"`
|
ID string `json:"id"`
|
||||||
|
State string `json:"state"`
|
||||||
Membership struct {
|
Membership struct {
|
||||||
Role string `json:"role"`
|
Role string `json:"role"`
|
||||||
} `json:"membership"`
|
} `json:"membership"`
|
||||||
@@ -198,6 +200,24 @@ func TestUser_RemoveUser(t *testing.T) {
|
|||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
assert.Equal(t, userID, mutationResult.RemoveUser.DeletedProfileID)
|
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) {
|
func TestUser_RemoveOwner(t *testing.T) {
|
||||||
|
|||||||
@@ -169,18 +169,6 @@ func (e ErrLastActiveOwner) Error() string {
|
|||||||
return fmt.Sprintf("cannot remove profile %q: last active owner of the organization", e.MembershipID)
|
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 }
|
type ErrOrganizationNotFound struct{ OrganizationID gid.GID }
|
||||||
|
|
||||||
func NewOrganizationNotFoundError(organizationID gid.GID) error {
|
func NewOrganizationNotFoundError(organizationID gid.GID) error {
|
||||||
|
|||||||
@@ -21,7 +21,6 @@ import (
|
|||||||
"io"
|
"io"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/jackc/pgx/v5/pgconn"
|
|
||||||
"go.gearno.de/crypto/uuid"
|
"go.gearno.de/crypto/uuid"
|
||||||
"go.gearno.de/kit/pg"
|
"go.gearno.de/kit/pg"
|
||||||
"go.probo.inc/probo/packages/emails"
|
"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)
|
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
|
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(
|
func (s *OrganizationService) InviteUser(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
req *CreateInvitationRequest,
|
req *CreateInvitationRequest,
|
||||||
|
|||||||
@@ -1,92 +0,0 @@
|
|||||||
// Copyright (c) 2026 Probo Inc <hello@getprobo.com>.
|
|
||||||
//
|
|
||||||
// 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(),
|
|
||||||
)
|
|
||||||
}
|
|
||||||
@@ -120,10 +120,6 @@ func (r *mutationResolver) RemoveUser(ctx context.Context, input types.RemoveUse
|
|||||||
return nil, gqlutils.Conflict(ctx, err)
|
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) {
|
if errors.Is(err, coredata.ErrResourceInUse) {
|
||||||
return nil, gqlutils.Conflict(ctx, err)
|
return nil, gqlutils.Conflict(ctx, err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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)
|
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) {
|
if errors.Is(err, coredata.ErrResourceInUse) {
|
||||||
return nil, types.RemoveUserOutput{}, fmt.Errorf("cannot remove user: %w", err)
|
return nil, types.RemoveUserOutput{}, fmt.Errorf("cannot remove user: %w", err)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user