Fix user deletion flows (#15614)

**Before** - any user with workpace_members permission was able to remove a user from their workspace. This triggered the deletion of workspaceMember + of userWorkspace, but did not delete the user (even if they had no workspace left) nor the roleTarget (acts as junction between role and userWorkspace) which was left with a userWorkspaceId pointing to nothing. This is because roleTarget points to userWorkspaceId but the foreign key constraint was not implemented - any user could delete their own account. This triggered the deletion of all their workspaceMembers, but not of their userWorkspace nor their user nor the roleTarget --> we have orphaned userWorkspace, not technically but product wise - a userWorkspace without a workspaceMember does not make sense So the problems are - we have some roleTargets pointing to non-existing userWorkspaceId (which caused https://github.com/twentyhq/twenty/issues/14608 ) - we have userWorkspaces that should not exist and that have no workspaceMember counterpart - it is not possible for a user to leave a workspace by themselves, they can only leave all workspaces at once, except if they are being removed from the workspace by another user **Now** - if a user has multiple workspaces, they are given the possibility to leave one workspace while remaining in the others (we show two buttons: Leave workspace and Delete account buttons). if a user has just one workspace, they only see Delete account - when a user leaves a workspace, we delete their workspaceMember, userWorkspace and roleTarget. If they don't belong to any other workspace we also soft-delete their user - soft-deleted users get hard deleted after 30 days thanks to a cron - we have two commands to clean the orphans roleTarget and userWorkspace (TODO: query db to see how many must be run) **Next** - once the commands have been run, we can implement and introduce the foreign key constraint on roleTarget Fixes https://github.com/twentyhq/twenty/issues/14608
2025-11-06 19:29:12 +01:00
parent bfe1f47065
commit 4ce93aee52
38 changed files with 980 additions and 173 deletions
@@ -2,9 +2,11 @@ import { Test, type TestingModule } from '@nestjs/testing';
 import { getRepositoryToken } from '@nestjs/typeorm';

 import { WorkspaceActivationStatus } from 'twenty-shared/workspace';
-import { type Repository } from 'typeorm';
+import { type Repository, type UpdateResult } from 'typeorm';

 import { AuthException } from 'src/engine/core-modules/auth/auth.exception';
+import { type UserWorkspaceEntity } from 'src/engine/core-modules/user-workspace/user-workspace.entity';
+import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service';
 import { UserService } from 'src/engine/core-modules/user/services/user.service';
 import { UserEntity } from 'src/engine/core-modules/user/user.entity';
 import { WorkspaceService } from 'src/engine/core-modules/workspace/services/workspace.service';
@@ -40,6 +42,7 @@ describe('UserService', () => {
          useValue: {
            findOne: jest.fn(),
            save: jest.fn(),
+            softDelete: jest.fn(),
          },
        },
        {
@@ -58,6 +61,12 @@ describe('UserService', () => {
            validateUserWorkspaceIsNotUniqueAdminOrThrow: jest.fn(),
          },
        },
+        {
+          provide: UserWorkspaceService,
+          useValue: {
+            deleteUserWorkspace: jest.fn(),
+          },
+        },
      ],
    }).compile();

@@ -311,9 +320,14 @@ describe('UserService', () => {
    });

    it('deletes workspace member and workspace when user is sole member', async () => {
+      const mockedUserWorkspace = {
+        id: 'uw2',
+        workspaceId: 'w2',
+      } as UserWorkspaceEntity;
+
      (userRepository.findOne as jest.Mock).mockResolvedValue({
        id: 'u2',
-        userWorkspaces: [{ id: 'uw2', workspaceId: 'w2' }],
+        userWorkspaces: [mockedUserWorkspace],
      });

      jest
@@ -323,11 +337,14 @@ describe('UserService', () => {
        .spyOn(twentyORMGlobalManager, 'getRepositoryForWorkspace')
        .mockResolvedValue(mockWorkspaceMemberRepo);

+      (userRepository.softDelete as jest.Mock).mockResolvedValue({
+        affected: 1,
+        raw: [],
+        generatedMaps: [],
+      } as UpdateResult);
+
      const res = await service.deleteUser('u2');

-      expect(mockWorkspaceMemberRepo.delete).toHaveBeenCalledWith({
-        userId: 'u2',
-      });
      expect(workspaceService.deleteWorkspace).toHaveBeenCalledWith('w2');
      expect(res).toMatchObject({ id: 'u2' });
    });
@@ -4,7 +4,7 @@ import assert from 'assert';

 import { msg } from '@lingui/core/macro';
 import { TypeOrmQueryService } from '@ptc-org/nestjs-query-typeorm';
-import { assertIsDefinedOrThrow } from 'twenty-shared/utils';
+import { assertIsDefinedOrThrow, isDefined } from 'twenty-shared/utils';
 import { isWorkspaceActiveOrSuspended } from 'twenty-shared/workspace';
 import { IsNull, Not, Repository } from 'typeorm';

@@ -12,6 +12,8 @@ import {
  AuthException,
  AuthExceptionCode,
 } from 'src/engine/core-modules/auth/auth.exception';
+import { UserWorkspaceEntity } from 'src/engine/core-modules/user-workspace/user-workspace.entity';
+import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service';
 import { UserEntity } from 'src/engine/core-modules/user/user.entity';
 import { userValidator } from 'src/engine/core-modules/user/user.validate';
 import { WorkspaceService } from 'src/engine/core-modules/workspace/services/workspace.service';
@@ -33,6 +35,7 @@ export class UserService extends TypeOrmQueryService<UserEntity> {
    private readonly workspaceService: WorkspaceService,
    private readonly twentyORMGlobalManager: TwentyORMGlobalManager,
    private readonly userRoleService: UserRoleService,
+    private readonly userWorkspaceService: UserWorkspaceService,
  ) {
    super(userRepository);
  }
@@ -89,7 +92,7 @@ export class UserService extends TypeOrmQueryService<UserEntity> {
    });
  }

-  async deleteUser(userId: string): Promise<UserEntity> {
+  async deleteUser(userId: string) {
    const user = await this.userRepository.findOne({
      where: {
        id: userId,
@@ -99,78 +102,116 @@ export class UserService extends TypeOrmQueryService<UserEntity> {

    userValidator.assertIsDefinedOrThrow(user);

-    const prepareForUserDeletionInWorkspaces = await Promise.all(
-      user.userWorkspaces.map(async (userWorkspace) => {
-        const { workspaceId } = userWorkspace;
+    for (const userWorkspace of user.userWorkspaces) {
+      await this.removeUserFromWorkspaceAndPotentiallyDeleteWorkspace(
+        userWorkspace,
+      );
+    }

-        const workspaceMemberRepository =
-          await this.twentyORMGlobalManager.getRepositoryForWorkspace<WorkspaceMemberWorkspaceEntity>(
+    await this.userRepository.softDelete({ id: userId });
+
+    return await this.userRepository.findOne({
+      where: {
+        id: userId,
+      },
+      withDeleted: true,
+    });
+  }
+
+  async deleteUserWorkspaceAndPotentiallyDeleteUser({
+    userId,
+    workspaceId,
+  }: {
+    userId: string;
+    workspaceId: string;
+  }) {
+    const user = await this.userRepository.findOne({
+      where: {
+        id: userId,
+      },
+      relations: { userWorkspaces: true },
+    });
+
+    userValidator.assertIsDefinedOrThrow(user);
+
+    const userWorkspace = user.userWorkspaces.find(
+      (userWorkspace) => userWorkspace.workspaceId === workspaceId,
+    );
+
+    if (!isDefined(userWorkspace)) {
+      throw new Error('User workspace not found.');
+    }
+
+    await this.removeUserFromWorkspaceAndPotentiallyDeleteWorkspace(
+      userWorkspace,
+    );
+
+    if (user.userWorkspaces.length === 1) {
+      await this.userRepository.softDelete(userId);
+    }
+
+    return userWorkspace;
+  }
+
+  async removeUserFromWorkspaceAndPotentiallyDeleteWorkspace(
+    userWorkspace: UserWorkspaceEntity,
+  ) {
+    const workspaceId = userWorkspace.workspaceId;
+
+    const workspaceMemberRepository =
+      await this.twentyORMGlobalManager.getRepositoryForWorkspace<WorkspaceMemberWorkspaceEntity>(
+        workspaceId,
+        'workspaceMember',
+        { shouldBypassPermissionChecks: true },
+      );
+
+    const workspaceMembers = await workspaceMemberRepository.find();
+
+    const userWorkspaceId = userWorkspace.id;
+
+    if (workspaceMembers.length === 1) {
+      await this.workspaceService.deleteWorkspace(workspaceId);
+
+      return;
+    }
+
+    if (workspaceMembers.length > 1) {
+      try {
+        await this.userRoleService.validateUserWorkspaceIsNotUniqueAdminOrThrow(
+          {
            workspaceId,
-            'workspaceMember',
-            { shouldBypassPermissionChecks: true },
-          );
-
-        const workspaceMembers = await workspaceMemberRepository.find();
-
-        if (workspaceMembers.length > 1) {
-          try {
-            await this.userRoleService.validateUserWorkspaceIsNotUniqueAdminOrThrow(
-              {
-                workspaceId,
-                userWorkspaceId: userWorkspace.id,
-              },
-            );
-          } catch (error) {
-            if (
-              error instanceof PermissionsException &&
-              error.code === PermissionsExceptionCode.CANNOT_UNASSIGN_LAST_ADMIN
-            ) {
-              throw new PermissionsException(
-                PermissionsExceptionMessage.CANNOT_DELETE_LAST_ADMIN_USER,
-                PermissionsExceptionCode.CANNOT_DELETE_LAST_ADMIN_USER,
-                {
-                  userFriendlyMessage: msg`Cannot delete account: you are the only admin. Assign another admin or delete the workspace(s) first.`,
-                },
-              );
-            }
-            throw error;
-          }
-        }
-
-        const workspaceMember = workspaceMembers.find(
-          (member: WorkspaceMemberWorkspaceEntity) => member.userId === userId,
+            userWorkspaceId: userWorkspace.id,
+          },
        );
+      } catch (error) {
+        if (
+          error instanceof PermissionsException &&
+          error.code === PermissionsExceptionCode.CANNOT_UNASSIGN_LAST_ADMIN
+        ) {
+          throw new PermissionsException(
+            PermissionsExceptionMessage.CANNOT_DELETE_LAST_ADMIN_USER,
+            PermissionsExceptionCode.CANNOT_DELETE_LAST_ADMIN_USER,
+            {
+              userFriendlyMessage: msg`Cannot delete account: you are the only admin. Assign another admin or delete the workspace(s) first.`,
+            },
+          );
+        }
+        throw error;
+      }
+    }

-        assert(workspaceMember, 'WorkspaceMember not found');
-
-        return {
-          workspaceId,
-          workspaceMemberRepository,
-          workspaceMembers,
-          workspaceMember,
-        };
-      }),
+    const workspaceMember = workspaceMembers.find(
+      (member: WorkspaceMemberWorkspaceEntity) =>
+        member.userId === userWorkspace.userId,
    );

-    await Promise.all(
-      prepareForUserDeletionInWorkspaces.map(
-        async ({
-          workspaceId,
-          workspaceMemberRepository,
-          workspaceMembers,
-        }) => {
-          await workspaceMemberRepository.delete({ userId });
+    assert(workspaceMember, 'WorkspaceMember not found');

-          if (workspaceMembers.length === 1) {
-            await this.workspaceService.deleteWorkspace(workspaceId);
+    await workspaceMemberRepository.delete({ userId: userWorkspace.userId });

-            return;
-          }
-        },
-      ),
-    );
-
-    return user;
+    await this.userWorkspaceService.deleteUserWorkspace({
+      userWorkspaceId,
+    });
  }

  async hasUserAccessToWorkspaceOrThrow(userId: string, workspaceId: string) {
@@ -1,4 +1,4 @@
-import { UseFilters, UseGuards } from '@nestjs/common';
+import { BadRequestException, UseFilters, UseGuards } from '@nestjs/common';
 import {
  Args,
  Mutation,
@@ -11,6 +11,7 @@ import { InjectRepository } from '@nestjs/typeorm';

 import crypto from 'crypto';

+import { msg } from '@lingui/core/macro';
 import { GraphQLJSONObject } from 'graphql-type-json';
 import { FileUpload, GraphQLUpload } from 'graphql-upload';
 import { isDefined } from 'twenty-shared/utils';
@@ -48,17 +49,27 @@ import { UserEntity } from 'src/engine/core-modules/user/user.entity';
 import { userValidator } from 'src/engine/core-modules/user/user.validate';
 import { AuthProviderEnum } from 'src/engine/core-modules/workspace/types/workspace.type';
 import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity';
+import { AuthApiKey } from 'src/engine/decorators/auth/auth-api-key.decorator';
 import { AuthProvider } from 'src/engine/decorators/auth/auth-provider.decorator';
+import { AuthUserWorkspaceId } from 'src/engine/decorators/auth/auth-user-workspace-id.decorator';
 import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator';
 import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator';
 import { UserAuthGuard } from 'src/engine/guards/user-auth.guard';
 import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
+import { PermissionFlagType } from 'src/engine/metadata-modules/permissions/constants/permission-flag-type.constants';
+import {
+  PermissionsException,
+  PermissionsExceptionCode,
+  PermissionsExceptionMessage,
+} from 'src/engine/metadata-modules/permissions/permissions.exception';
 import { PermissionsService } from 'src/engine/metadata-modules/permissions/permissions.service';
 import { type UserWorkspacePermissions } from 'src/engine/metadata-modules/permissions/types/user-workspace-permissions';
 import { PermissionsGraphqlApiExceptionFilter } from 'src/engine/metadata-modules/permissions/utils/permissions-graphql-api-exception.filter';
 import { fromUserWorkspacePermissionsToUserWorkspacePermissionsDto } from 'src/engine/metadata-modules/role/utils/fromUserWorkspacePermissionsToUserWorkspacePermissionsDto';
 import { UserRoleService } from 'src/engine/metadata-modules/user-role/user-role.service';
+import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
 import { AccountsToReconnectKeys } from 'src/modules/connected-account/types/accounts-to-reconnect-key-value.type';
+import { WorkspaceMemberWorkspaceEntity } from 'src/modules/workspace-member/standard-objects/workspace-member.workspace-entity';
 import { streamToBuffer } from 'src/utils/stream-to-buffer';

 const getHMACKey = (email?: string, key?: string | null) => {
@@ -87,6 +98,7 @@ export class UserResolver {

    private readonly workspaceMemberTranspiler: WorkspaceMemberTranspiler,
    private readonly userWorkspaceService: UserWorkspaceService,
+    private readonly twentyORMGlobalManager: TwentyORMGlobalManager,
  ) {}

  private async getUserWorkspacePermissions({
@@ -391,6 +403,70 @@ export class UserResolver {
    return this.userService.deleteUser(userId);
  }

+  @Mutation(() => UserWorkspaceEntity)
+  @UseGuards(UserAuthGuard)
+  async deleteUserFromWorkspace(
+    @Args('workspaceMemberIdToDelete') workspaceMemberIdToDelete: string,
+    @AuthUser() { id: userId }: UserEntity,
+    @AuthUserWorkspaceId() userWorkspaceId: string,
+    @AuthWorkspace()
+    workspace: WorkspaceEntity,
+    @AuthApiKey() apiKey?: string,
+  ) {
+    if (!workspace) {
+      throw new AuthException(
+        'Workspace not found',
+        AuthExceptionCode.WORKSPACE_NOT_FOUND,
+      );
+    }
+
+    const workspaceMemberRepository =
+      await this.twentyORMGlobalManager.getRepositoryForWorkspace<WorkspaceMemberWorkspaceEntity>(
+        workspace.id,
+        'workspaceMember',
+        { shouldBypassPermissionChecks: true },
+      );
+
+    const workspaceMemberToDelete = await workspaceMemberRepository.findOne({
+      where: {
+        id: workspaceMemberIdToDelete,
+      },
+    });
+
+    if (!isDefined(workspaceMemberToDelete)) {
+      throw new BadRequestException(
+        'Workspace member to delete not found in workspace',
+      );
+    }
+
+    const workspaceMemberToDeleteIsAuthenticatedUser =
+      workspaceMemberToDelete.userId === userId;
+
+    const canDeleteUserFromWorkspace =
+      workspaceMemberToDeleteIsAuthenticatedUser ||
+      (await this.permissionsService.userHasWorkspaceSettingPermission({
+        userWorkspaceId,
+        workspaceId: workspace.id,
+        setting: PermissionFlagType.WORKSPACE_MEMBERS,
+        apiKeyId: apiKey ?? undefined,
+      }));
+
+    if (!canDeleteUserFromWorkspace) {
+      throw new PermissionsException(
+        PermissionsExceptionMessage.PERMISSION_DENIED,
+        PermissionsExceptionCode.PERMISSION_DENIED,
+        {
+          userFriendlyMessage: msg`You do not have permission to delete this user from the workspace. Please contact your workspace administrator for access.`,
+        },
+      );
+    }
+
+    return this.userService.deleteUserWorkspaceAndPotentiallyDeleteUser({
+      userId: workspaceMemberToDelete.userId,
+      workspaceId: workspace.id,
+    });
+  }
+
  @ResolveField(() => OnboardingStatus, {
    nullable: true,
  })