From 1e08a23ddc7ad59e5b9d367fecc865a706a6ef81 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 27 May 2026 18:26:33 +0000 Subject: [PATCH] Split user remove and archive actions Restore RemoveUser as a hard delete operation and surface dependency\nconflicts with a dedicated IAM error.\n\nAdd a new ArchiveUser flow that deactivates profiles while keeping the\nmember in the organization, then expose both actions across Connect, MCP,\nCLI, n8n, console UI, and e2e coverage. Signed-off-by: Cursor Agent Co-authored-by: Bryan FRIMIN --- .../iam/organizations/people/PersonPage.tsx | 84 ++++++++++--- .../people/_components/PeopleListItem.tsx | 67 ++++++++-- e2e/console/user_test.go | 111 ++++++++++++++++- .../actions/user/archiveUser.operation.ts | 71 +++++++++++ .../nodes/Probo/actions/user/index.ts | 11 +- .../actions/user/removeUser.operation.ts | 4 +- pkg/cmd/user/archive/archive.go | 6 +- pkg/cmd/user/remove/remove.go | 114 ++++++++++++++++++ pkg/cmd/user/user.go | 2 + pkg/iam/errors.go | 12 ++ pkg/iam/organization_service.go | 81 +++++++++++++ .../api/connect/v1/graphql/profile.graphql | 11 ++ .../api/connect/v1/profile_resolvers.go | 34 +++++- pkg/server/api/mcp/v1/schema.resolvers.go | 33 ++++- pkg/server/api/mcp/v1/specification.yaml | 35 +++++- 15 files changed, 628 insertions(+), 48 deletions(-) create mode 100644 packages/n8n-node/nodes/Probo/actions/user/archiveUser.operation.ts create mode 100644 pkg/cmd/user/remove/remove.go diff --git a/apps/console/src/pages/iam/organizations/people/PersonPage.tsx b/apps/console/src/pages/iam/organizations/people/PersonPage.tsx index fd73eed68..23498dc42 100644 --- a/apps/console/src/pages/iam/organizations/people/PersonPage.tsx +++ b/apps/console/src/pages/iam/organizations/people/PersonPage.tsx @@ -14,7 +14,7 @@ import { sprintf } from "@probo/helpers"; import { useTranslate } from "@probo/i18n"; -import { ActionDropdown, Avatar, Badge, Breadcrumb, Card, DropdownItem, IconTrashCan, useConfirm } from "@probo/ui"; +import { ActionDropdown, Avatar, Badge, Breadcrumb, Card, DropdownItem, IconArchive, IconTrashCan, useConfirm } from "@probo/ui"; import { type PreloadedQuery, usePreloadedQuery } from "react-relay"; import { useNavigate } from "react-router"; import { graphql } from "relay-runtime"; @@ -34,6 +34,7 @@ export const personPageQuery = graphql` fullName emailAddress source + state canDelete: permission(action: "iam:membership-profile:delete") ...PersonFormFragment } @@ -51,6 +52,16 @@ const removeUserMutation = graphql` } `; +const archiveUserMutation = graphql` + mutation PersonPage_archiveMutation( + $input: ArchiveUserInput! + ) { + archiveUser(input: $input) { + archivedProfileId + } + } +`; + export function PersonPage(props: { queryRef: PreloadedQuery }) { const { queryRef } = props; @@ -64,18 +75,25 @@ export function PersonPage(props: { queryRef: PreloadedQuery }) throw new Error("invalid type for node"); } - const [removeUser, isRemoving] = useMutationWithToasts( - removeUserMutation, + const [archiveUser, isArchiving] = useMutationWithToasts( + archiveUserMutation, { successMessage: __("Person archived successfully"), errorMessage: __("Failed to archive person"), }, ); + const [removeUser, isRemoving] = useMutationWithToasts( + removeUserMutation, + { + successMessage: __("Person removed successfully"), + errorMessage: __("Failed to remove person"), + }, + ); - const handleRemove = () => { + const handleArchive = () => { confirm( () => { - return removeUser({ + return archiveUser({ variables: { input: { profileId: person.id, @@ -96,7 +114,32 @@ export function PersonPage(props: { queryRef: PreloadedQuery }) ); }; - const canArchive = person.canDelete && person.source !== "SCIM"; + const handleRemove = () => { + confirm( + () => { + return removeUser({ + variables: { + input: { + profileId: person.id, + organizationId: organizationId, + }, + }, + onCompleted: () => { + void navigate(`/organizations/${organizationId}/people`); + }, + }); + }, + { + message: sprintf( + __("Are you sure you want to remove %s?"), + person.fullName, + ), + }, + ); + }; + + const canArchive = person.canDelete && person.source !== "SCIM" && person.state !== "INACTIVE"; + const canRemove = person.canDelete && person.source !== "SCIM"; return (
@@ -122,16 +165,27 @@ export function PersonPage(props: { queryRef: PreloadedQuery })
{person.emailAddress}
- {canArchive && ( + {(canArchive || canRemove) && ( - - {__("Archive")} - + {canArchive && ( + + {__("Archive")} + + )} + {canRemove && ( + + {__("Remove")} + + )} )} diff --git a/apps/console/src/pages/iam/organizations/people/_components/PeopleListItem.tsx b/apps/console/src/pages/iam/organizations/people/_components/PeopleListItem.tsx index e6ec90de9..9688a075d 100644 --- a/apps/console/src/pages/iam/organizations/people/_components/PeopleListItem.tsx +++ b/apps/console/src/pages/iam/organizations/people/_components/PeopleListItem.tsx @@ -18,6 +18,7 @@ import { ActionDropdown, Badge, DropdownItem, + IconArchive, IconMail, IconTrashCan, Option, @@ -107,6 +108,14 @@ const removeUserMutation = graphql` } `; +const archiveUserMutation = graphql` + mutation PeopleListItem_archiveMutation($input: ArchiveUserInput!) { + archiveUser(input: $input) { + archivedProfileId + } + } +`; + export function PeopleListItem(props: { connectionId: DataID; fKey: PeopleListItemFragment$key; @@ -128,6 +137,7 @@ export function PeopleListItem(props: { const canSendActivationMail = isInactive && profile.source !== "SCIM" && profile.canInvite; const canArchive = profile.canDelete && profile.source !== "SCIM" && profile.state !== "INACTIVE"; + const canRemove = profile.canDelete && profile.source !== "SCIM"; const [inviteUser] = useMutationWithToasts(inviteUserMutation, { @@ -141,13 +151,21 @@ export function PeopleListItem(props: { errorMessage: __("Failed to update role"), }, ); - const [removeUser, isRemoving] = useMutationWithToasts( - removeUserMutation, + const [archiveUser, isArchiving] = useMutationWithToasts( + archiveUserMutation, { successMessage: __("Person archived successfully"), errorMessage: __("Failed to archive person"), }, ); + const [removeUser, isRemoving] = useMutationWithToasts( + removeUserMutation, + { + successMessage: __("Person removed successfully"), + errorMessage: __("Failed to remove person"), + }, + ); + const isMutating = isArchiving || isRemoving; const handleInvite = () => { confirm( @@ -183,6 +201,29 @@ export function PeopleListItem(props: { }, }); }; + const handleArchive = () => { + confirm( + () => { + return archiveUser({ + variables: { + input: { + profileId: profile.id, + organizationId: organizationId, + }, + }, + onCompleted: () => { + onRefetch(); + }, + }); + }, + { + message: sprintf( + __("Are you sure you want to archive %s?"), + profile.fullName, + ), + }, + ); + }; const handleRemove = () => { confirm( () => { @@ -201,7 +242,7 @@ export function PeopleListItem(props: { }, { message: sprintf( - __("Are you sure you want to archive %s?"), + __("Are you sure you want to remove %s?"), profile.fullName, ), }, @@ -211,7 +252,7 @@ export function PeopleListItem(props: { return ( @@ -221,7 +262,7 @@ export function PeopleListItem(props: { {profile.state} @@ -235,7 +276,7 @@ export function PeopleListItem(props: { noLink className={clsx( "pr-4", - isRemoving && "opacity-60 pointer-events-none", + isMutating && "opacity-60 pointer-events-none", isInactive && "opacity-50", )} > @@ -263,14 +304,14 @@ export function PeopleListItem(props: { )} {new Date(profile.createdAt).toLocaleDateString()} - {(canSendActivationMail || canArchive) && ( + {(canSendActivationMail || canArchive || canRemove) && ( {canSendActivationMail && ( )} {canArchive && ( + + {__("Archive person")} + + )} + {canRemove && ( - {__("Archive person")} + {__("Remove person")} )} diff --git a/e2e/console/user_test.go b/e2e/console/user_test.go index ada50e79d..7a76b0d2c 100644 --- a/e2e/console/user_test.go +++ b/e2e/console/user_test.go @@ -201,23 +201,124 @@ func TestUser_RemoveUser(t *testing.T) { assert.Equal(t, userID, mutationResult.RemoveUser.DeletedProfileID) - // Remove archives the user instead of hard-deleting them. + // Removed user should no longer be returned. err = owner.ExecuteConnect(query, map[string]any{ "id": owner.GetOrganizationID().String(), }, &result) require.NoError(t, err) - var removedUserState string + var removedUserFound bool for _, edge := range result.Node.Profiles.Edges { if edge.Node.ID == userID { - removedUserState = edge.Node.State + removedUserFound = true break } } - require.NotEmpty(t, removedUserState, "Should still find archived user") - assert.Equal(t, "INACTIVE", removedUserState) + assert.False(t, removedUserFound, "Should not find removed user") +} + +func TestUser_ArchiveUser(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + + // Create a user to archive. + userToArchive := testutil.NewClientInOrg(t, testutil.RoleViewer, owner) + _ = userToArchive + + query := ` + query($id: ID!) { + node(id: $id) { + ... on Organization { + profiles(first: 50) { + edges { + node { + id + state + membership { + role + } + } + } + } + } + } + } + ` + + var result struct { + Node struct { + Profiles struct { + Edges []struct { + Node struct { + ID string `json:"id"` + State string `json:"state"` + Membership struct { + Role string `json:"role"` + } `json:"membership"` + } `json:"node"` + } `json:"edges"` + } `json:"profiles"` + } `json:"node"` + } + + err := owner.ExecuteConnect(query, map[string]any{ + "id": owner.GetOrganizationID().String(), + }, &result) + require.NoError(t, err) + + var userID string + + for _, edge := range result.Node.Profiles.Edges { + if edge.Node.Membership.Role == "VIEWER" { + userID = edge.Node.ID + break + } + } + + require.NotEmpty(t, userID, "Should find viewer member") + + mutation := ` + mutation($input: ArchiveUserInput!) { + archiveUser(input: $input) { + archivedProfileId + } + } + ` + + var mutationResult struct { + ArchiveUser struct { + ArchivedProfileID string `json:"archivedProfileId"` + } `json:"archiveUser"` + } + + err = owner.ExecuteConnect(mutation, map[string]any{ + "input": map[string]any{ + "organizationId": owner.GetOrganizationID().String(), + "profileId": userID, + }, + }, &mutationResult) + require.NoError(t, err) + + assert.Equal(t, userID, mutationResult.ArchiveUser.ArchivedProfileID) + + err = owner.ExecuteConnect(query, map[string]any{ + "id": owner.GetOrganizationID().String(), + }, &result) + require.NoError(t, err) + + var archivedUserState string + + for _, edge := range result.Node.Profiles.Edges { + if edge.Node.ID == userID { + archivedUserState = edge.Node.State + break + } + } + + require.NotEmpty(t, archivedUserState, "Should still find archived user") + assert.Equal(t, "INACTIVE", archivedUserState) } func TestUser_RemoveOwner(t *testing.T) { diff --git a/packages/n8n-node/nodes/Probo/actions/user/archiveUser.operation.ts b/packages/n8n-node/nodes/Probo/actions/user/archiveUser.operation.ts new file mode 100644 index 000000000..8576ef193 --- /dev/null +++ b/packages/n8n-node/nodes/Probo/actions/user/archiveUser.operation.ts @@ -0,0 +1,71 @@ +// 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. + +import type { INodeProperties, IExecuteFunctions, INodeExecutionData } from 'n8n-workflow'; +import { proboConnectApiRequest } from '../../GenericFunctions'; + +export const description: INodeProperties[] = [ + { + displayName: 'Organization ID', + name: 'organizationId', + type: 'string', + displayOptions: { + show: { + resource: ['user'], + operation: ['archiveUser'], + }, + }, + default: '', + description: 'The ID of the organization', + required: true, + }, + { + displayName: 'User ID', + name: 'userId', + type: 'string', + displayOptions: { + show: { + resource: ['user'], + operation: ['archiveUser'], + }, + }, + default: '', + description: 'The ID of the user (profile) to archive in the organization', + required: true, + }, +]; + +export async function execute( + this: IExecuteFunctions, + itemIndex: number, +): Promise { + const organizationId = this.getNodeParameter('organizationId', itemIndex) as string; + const userId = this.getNodeParameter('userId', itemIndex) as string; + + const query = ` + mutation ArchiveUser($input: ArchiveUserInput!) { + archiveUser(input: $input) { + archivedProfileId + } + } + `; + + const input = { organizationId, profileId: userId }; + const responseData = await proboConnectApiRequest.call(this, query, { input }); + + return { + json: responseData, + pairedItem: { item: itemIndex }, + }; +} diff --git a/packages/n8n-node/nodes/Probo/actions/user/index.ts b/packages/n8n-node/nodes/Probo/actions/user/index.ts index 12147831b..222a7f017 100644 --- a/packages/n8n-node/nodes/Probo/actions/user/index.ts +++ b/packages/n8n-node/nodes/Probo/actions/user/index.ts @@ -13,6 +13,7 @@ // PERFORMANCE OF THIS SOFTWARE. import type { INodeProperties } from 'n8n-workflow'; +import * as archiveUserOp from './archiveUser.operation'; import * as listUsersOp from './listUsers.operation'; import * as getUserOp from './getUser.operation'; import * as createUserOp from './createUser.operation'; @@ -35,7 +36,7 @@ export const description: INodeProperties[] = [ options: [ { name: 'Archive', - value: 'removeUser', + value: 'archiveUser', description: 'Archive a user in the organization', action: 'Archive a user', }, @@ -63,6 +64,12 @@ export const description: INodeProperties[] = [ description: 'List all users in the organization', action: 'List users', }, + { + name: 'Remove', + value: 'removeUser', + description: 'Remove a user from the organization', + action: 'Remove a user', + }, { name: 'Update', value: 'updateUser', @@ -78,6 +85,7 @@ export const description: INodeProperties[] = [ ], default: 'listUsers', }, + ...archiveUserOp.description, ...listUsersOp.description, ...getUserOp.description, ...createUserOp.description, @@ -88,6 +96,7 @@ export const description: INodeProperties[] = [ ]; export { + archiveUserOp as archiveUser, listUsersOp as listUsers, getUserOp as getUser, createUserOp as createUser, diff --git a/packages/n8n-node/nodes/Probo/actions/user/removeUser.operation.ts b/packages/n8n-node/nodes/Probo/actions/user/removeUser.operation.ts index d8d756ecb..5ce261d9b 100644 --- a/packages/n8n-node/nodes/Probo/actions/user/removeUser.operation.ts +++ b/packages/n8n-node/nodes/Probo/actions/user/removeUser.operation.ts @@ -41,7 +41,7 @@ export const description: INodeProperties[] = [ }, }, default: '', - description: 'The ID of the user (profile) to archive in the organization', + description: 'The ID of the user (profile) to remove from the organization', required: true, }, ]; @@ -56,7 +56,7 @@ export async function execute( const query = ` mutation RemoveUser($input: RemoveUserInput!) { removeUser(input: $input) { - archivedProfileId: deletedProfileId + deletedProfileId } } `; diff --git a/pkg/cmd/user/archive/archive.go b/pkg/cmd/user/archive/archive.go index 346a8fe23..f22f6cfb9 100644 --- a/pkg/cmd/user/archive/archive.go +++ b/pkg/cmd/user/archive/archive.go @@ -24,9 +24,9 @@ import ( ) const archiveMutation = ` -mutation($input: RemoveUserInput!) { - removeUser(input: $input) { - deletedProfileId +mutation($input: ArchiveUserInput!) { + archiveUser(input: $input) { + archivedProfileId } } ` diff --git a/pkg/cmd/user/remove/remove.go b/pkg/cmd/user/remove/remove.go new file mode 100644 index 000000000..dd6b26b51 --- /dev/null +++ b/pkg/cmd/user/remove/remove.go @@ -0,0 +1,114 @@ +// 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 remove + +import ( + "fmt" + + "github.com/charmbracelet/huh" + "github.com/spf13/cobra" + "go.probo.inc/probo/pkg/cli/api" + "go.probo.inc/probo/pkg/cmd/cmdutil" +) + +const removeMutation = ` +mutation($input: RemoveUserInput!) { + removeUser(input: $input) { + deletedProfileId + } +} +` + +func NewCmdRemove(f *cmdutil.Factory) *cobra.Command { + var ( + flagOrg string + flagYes bool + ) + + cmd := &cobra.Command{ + Use: "remove ", + Short: "Remove a user", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if !flagYes { + if !f.IOStreams.IsInteractive() { + return fmt.Errorf("cannot remove user: confirmation required, use --yes to confirm") + } + + var confirmed bool + + err := huh.NewConfirm(). + Title(fmt.Sprintf("Remove user %s?", args[0])). + Value(&confirmed). + Run() + if err != nil { + return err + } + + if !confirmed { + return nil + } + } + + cfg, err := f.Config() + if err != nil { + return err + } + + host, hc, err := cfg.DefaultHost() + if err != nil { + return err + } + + if flagOrg == "" { + flagOrg = hc.Organization + } + + if flagOrg == "" { + return fmt.Errorf("organization is required; pass --org or set a default with 'prb auth login'") + } + + client := api.NewClient( + host, + hc.Token, + "/api/console/v1/graphql", + cfg.HTTPTimeoutDuration(), + cmdutil.TokenRefreshOption(cfg, host, hc), + ) + + _, err = client.Do( + removeMutation, + map[string]any{ + "input": map[string]any{ + "organizationId": flagOrg, + "profileId": args[0], + }, + }, + ) + if err != nil { + return err + } + + _, _ = fmt.Fprintf(f.IOStreams.Out, "Removed user %s\n", args[0]) + + return nil + }, + } + + cmd.Flags().StringVar(&flagOrg, "org", "", "Organization ID") + cmd.Flags().BoolVarP(&flagYes, "yes", "y", false, "Skip confirmation prompt") + + return cmd +} diff --git a/pkg/cmd/user/user.go b/pkg/cmd/user/user.go index 6d23fc312..3a76b30f5 100644 --- a/pkg/cmd/user/user.go +++ b/pkg/cmd/user/user.go @@ -19,6 +19,7 @@ import ( "go.probo.inc/probo/pkg/cmd/cmdutil" "go.probo.inc/probo/pkg/cmd/user/archive" "go.probo.inc/probo/pkg/cmd/user/list" + "go.probo.inc/probo/pkg/cmd/user/remove" "go.probo.inc/probo/pkg/cmd/user/view" ) @@ -31,6 +32,7 @@ func NewCmdUser(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(list.NewCmdList(f)) cmd.AddCommand(view.NewCmdView(f)) cmd.AddCommand(archive.NewCmdArchive(f)) + cmd.AddCommand(remove.NewCmdRemove(f)) return cmd } diff --git a/pkg/iam/errors.go b/pkg/iam/errors.go index 571e18c76..356e56a03 100644 --- a/pkg/iam/errors.go +++ b/pkg/iam/errors.go @@ -157,6 +157,18 @@ func (e ErrUserManagedBySCIM) Error() string { return fmt.Sprintf("user %q is managed by SCIM and cannot be deleted manually", e.ProfileID) } +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 ErrLastActiveOwner struct { MembershipID gid.GID } diff --git a/pkg/iam/organization_service.go b/pkg/iam/organization_service.go index f0cca4e9b..3327f4f4c 100644 --- a/pkg/iam/organization_service.go +++ b/pkg/iam/organization_service.go @@ -21,6 +21,7 @@ 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" @@ -314,6 +315,73 @@ func (s *OrganizationService) RemoveUser( ) error { scope := coredata.NewScopeFromObjectID(organizationID) + return s.pg.WithTx( + ctx, + func(ctx context.Context, tx pg.Tx) error { + profile := coredata.MembershipProfile{} + + if err := profile.LoadByID(ctx, tx, scope, profileID); err != nil { + if err == coredata.ErrResourceNotFound { + return NewProfileNotFoundError(profileID) + } + + return fmt.Errorf("cannot load profile: %w", err) + } + + if profile.Source == coredata.ProfileSourceSCIM { + return NewUserManagedBySCIMError(profileID) + } + + membership := &coredata.Membership{} + if err := membership.LoadByIdentityIDAndOrganizationID(ctx, tx, scope, profile.IdentityID, profile.OrganizationID); err != nil { + return fmt.Errorf("cannot load membership: %w", err) + } + + if membership.Role == coredata.MembershipRoleOwner && profile.State == coredata.ProfileStateActive { + profiles := coredata.MembershipProfiles{} + + count, err := profiles.CountActiveOwnerByOrganizationID(ctx, tx, scope, organizationID) + if err != nil { + return fmt.Errorf("cannot count active owners: %w", err) + } + + if count <= 1 { + return NewLastActiveOwnerError(profileID) + } + } + + 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) + } + + if err := webhook.InsertData(ctx, tx, scope, organizationID, coredata.WebhookEventTypeUserDeleted, webhooktypes.NewUser(&profile, membership)); err != nil { + return fmt.Errorf("cannot insert webhook event: %w", err) + } + + return nil + }, + ) +} + +func (s *OrganizationService) ArchiveUser( + ctx context.Context, + organizationID gid.GID, + profileID gid.GID, +) error { + scope := coredata.NewScopeFromObjectID(organizationID) + return s.pg.WithTx( ctx, func(ctx context.Context, tx pg.Tx) error { @@ -374,6 +442,19 @@ func (s *OrganizationService) RemoveUser( ) } +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/server/api/connect/v1/graphql/profile.graphql b/pkg/server/api/connect/v1/graphql/profile.graphql index cd762ee30..522412ee6 100644 --- a/pkg/server/api/connect/v1/graphql/profile.graphql +++ b/pkg/server/api/connect/v1/graphql/profile.graphql @@ -96,6 +96,8 @@ extend type Mutation { @session(required: PRESENT) deactivateUser(input: DeactivateUserInput!): DeactivateUserPayload updateUser(input: UpdateUserInput!): UpdateUserPayload! + archiveUser(input: ArchiveUserInput!): ArchiveUserPayload + @session(required: PRESENT) removeUser(input: RemoveUserInput!): RemoveUserPayload @session(required: PRESENT) } @@ -137,6 +139,11 @@ input RemoveUserInput { profileId: ID! } +input ArchiveUserInput { + organizationId: ID! + profileId: ID! +} + type CreateUserPayload { profileEdge: ProfileEdge! } @@ -152,3 +159,7 @@ type UpdateUserPayload { type RemoveUserPayload { deletedProfileId: ID! } + +type ArchiveUserPayload { + archivedProfileId: ID! +} diff --git a/pkg/server/api/connect/v1/profile_resolvers.go b/pkg/server/api/connect/v1/profile_resolvers.go index 89861421c..4a9b60c08 100644 --- a/pkg/server/api/connect/v1/profile_resolvers.go +++ b/pkg/server/api/connect/v1/profile_resolvers.go @@ -104,13 +104,13 @@ func (r *mutationResolver) UpdateUser(ctx context.Context, input types.UpdateUse }, nil } -// RemoveUser is the resolver for the removeUser field. -func (r *mutationResolver) RemoveUser(ctx context.Context, input types.RemoveUserInput) (*types.RemoveUserPayload, error) { +// ArchiveUser is the resolver for the archiveUser field. +func (r *mutationResolver) ArchiveUser(ctx context.Context, input types.ArchiveUserInput) (*types.ArchiveUserPayload, error) { if _, err := r.authorize(ctx, input.ProfileID, iam.ActionMembershipProfileDelete); err != nil { return nil, err } - err := r.iam.OrganizationService.RemoveUser(ctx, input.OrganizationID, input.ProfileID) + err := r.iam.OrganizationService.ArchiveUser(ctx, input.OrganizationID, input.ProfileID) if err != nil { if _, ok := errors.AsType[*iam.ErrUserManagedBySCIM](err); ok { return nil, gqlutils.Conflictf(ctx, "user is managed by SCIM and cannot be archived") @@ -120,8 +120,32 @@ func (r *mutationResolver) RemoveUser(ctx context.Context, input types.RemoveUse return nil, gqlutils.Conflictf(ctx, "cannot archive last active owner") } - if errors.Is(err, coredata.ErrResourceInUse) { - return nil, gqlutils.Conflictf(ctx, "cannot archive user") + r.logger.ErrorCtx(ctx, "cannot archive user from organization", log.Error(err)) + + return nil, gqlutils.Internal(ctx) + } + + return &types.ArchiveUserPayload{ArchivedProfileID: input.ProfileID}, nil +} + +// RemoveUser is the resolver for the removeUser field. +func (r *mutationResolver) RemoveUser(ctx context.Context, input types.RemoveUserInput) (*types.RemoveUserPayload, error) { + if _, err := r.authorize(ctx, input.ProfileID, iam.ActionMembershipProfileDelete); err != nil { + return nil, err + } + + err := r.iam.OrganizationService.RemoveUser(ctx, input.OrganizationID, input.ProfileID) + if err != nil { + if _, ok := errors.AsType[*iam.ErrUserManagedBySCIM](err); ok { + return nil, gqlutils.Conflictf(ctx, "user is managed by SCIM and cannot be removed") + } + + if _, ok := errors.AsType[*iam.ErrLastActiveOwner](err); ok { + return nil, gqlutils.Conflictf(ctx, "cannot remove last active owner") + } + + if _, ok := errors.AsType[*iam.ErrUserReferencedByRecords](err); ok { + return nil, gqlutils.Conflictf(ctx, "cannot remove user because they are referenced by existing records (for example signatures, tasks, assets, or risks)") } r.logger.ErrorCtx(ctx, "cannot remove user from organization", log.Error(err)) diff --git a/pkg/server/api/mcp/v1/schema.resolvers.go b/pkg/server/api/mcp/v1/schema.resolvers.go index f8242a83d..6e9aad701 100644 --- a/pkg/server/api/mcp/v1/schema.resolvers.go +++ b/pkg/server/api/mcp/v1/schema.resolvers.go @@ -2923,21 +2923,42 @@ func (r *Resolver) RemoveUserTool(ctx context.Context, req *mcp.CallToolRequest, err := r.iamSvc.OrganizationService.RemoveUser(ctx, input.OrganizationID, input.ProfileID) if err != nil { if _, ok := errors.AsType[*iam.ErrUserManagedBySCIM](err); ok { - return nil, types.RemoveUserOutput{}, fmt.Errorf("user is managed by SCIM and cannot be archived: %w", err) + return nil, types.RemoveUserOutput{}, fmt.Errorf("user is managed by SCIM and cannot be removed: %w", err) } if _, ok := errors.AsType[*iam.ErrLastActiveOwner](err); ok { - return nil, types.RemoveUserOutput{}, fmt.Errorf("cannot archive last active owner: %w", err) + return nil, types.RemoveUserOutput{}, fmt.Errorf("cannot remove last active owner: %w", err) } - if errors.Is(err, coredata.ErrResourceInUse) { - return nil, types.RemoveUserOutput{}, fmt.Errorf("cannot archive user: %w", err) + if _, ok := errors.AsType[*iam.ErrUserReferencedByRecords](err); ok { + return nil, types.RemoveUserOutput{}, fmt.Errorf("cannot remove user because they are referenced by existing records: %w", err) } - return nil, types.RemoveUserOutput{}, fmt.Errorf("archive user: %w", err) + return nil, types.RemoveUserOutput{}, fmt.Errorf("remove user: %w", err) } - return nil, types.RemoveUserOutput{ArchivedUserID: input.ProfileID}, nil + return nil, types.RemoveUserOutput{DeletedUserID: input.ProfileID}, nil +} + +func (r *Resolver) ArchiveUserTool(ctx context.Context, req *mcp.CallToolRequest, input *types.ArchiveUserInput) (*mcp.CallToolResult, types.ArchiveUserOutput, error) { + if _, err := r.Authorize(ctx, input.ProfileID, iam.ActionMembershipProfileDelete); err != nil { + return nil, types.ArchiveUserOutput{}, err + } + + err := r.iamSvc.OrganizationService.ArchiveUser(ctx, input.OrganizationID, input.ProfileID) + if err != nil { + if _, ok := errors.AsType[*iam.ErrUserManagedBySCIM](err); ok { + return nil, types.ArchiveUserOutput{}, fmt.Errorf("user is managed by SCIM and cannot be archived: %w", err) + } + + if _, ok := errors.AsType[*iam.ErrLastActiveOwner](err); ok { + return nil, types.ArchiveUserOutput{}, fmt.Errorf("cannot archive last active owner: %w", err) + } + + return nil, types.ArchiveUserOutput{}, fmt.Errorf("archive user: %w", err) + } + + return nil, types.ArchiveUserOutput{ArchivedUserID: input.ProfileID}, nil } func (r *Resolver) DeleteDataProtectionImpactAssessmentTool(ctx context.Context, req *mcp.CallToolRequest, input *types.DeleteDataProtectionImpactAssessmentInput) (*mcp.CallToolResult, types.DeleteDataProtectionImpactAssessmentOutput, error) { diff --git a/pkg/server/api/mcp/v1/specification.yaml b/pkg/server/api/mcp/v1/specification.yaml index 6ae919cda..4b7174278 100644 --- a/pkg/server/api/mcp/v1/specification.yaml +++ b/pkg/server/api/mcp/v1/specification.yaml @@ -1709,9 +1709,31 @@ components: description: Organization ID profile_id: $ref: "#/components/schemas/GID" - description: User (profile) ID to archive + description: User (profile) ID to remove RemoveUserOutput: + type: object + required: + - deleted_user_id + properties: + deleted_user_id: + $ref: "#/components/schemas/GID" + description: Deleted user (profile) ID + + ArchiveUserInput: + type: object + required: + - organization_id + - profile_id + properties: + organization_id: + $ref: "#/components/schemas/GID" + description: Organization ID + profile_id: + $ref: "#/components/schemas/GID" + description: User (profile) ID to archive + + ArchiveUserOutput: type: object required: - archived_user_id @@ -11934,13 +11956,22 @@ tools: outputSchema: $ref: "#/components/schemas/UpdateMembershipOutput" - name: removeUser - description: Archive a user in the organization + description: Remove a user from the organization hints: readonly: false + destructive: true inputSchema: $ref: "#/components/schemas/RemoveUserInput" outputSchema: $ref: "#/components/schemas/RemoveUserOutput" + - name: archiveUser + description: Archive a user in the organization + hints: + readonly: false + inputSchema: + $ref: "#/components/schemas/ArchiveUserInput" + outputSchema: + $ref: "#/components/schemas/ArchiveUserOutput" - name: addThirdParty description: Add a new thirdParty to the organization hints: