fix: lock role count check

apps/server/src/core/space/services/space-member.service.ts

@@ -10,7 +10,7 @@ import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo';
 import { GroupUserRepo } from '@docmost/db/repos/group/group-user.repo';
 import { AddSpaceMembersDto } from '../dto/add-space-members.dto';
 import { InjectKysely } from 'nestjs-kysely';
-import { Space, SpaceMember, User } from '@docmost/db/types/entity.types';
+import { Space, User } from '@docmost/db/types/entity.types';
 import { SpaceRepo } from '@docmost/db/repos/space/space.repo';
 import { RemoveSpaceMemberDto } from '../dto/remove-space-member.dto';
 import { UpdateSpaceMemberRoleDto } from '../dto/update-space-member-role.dto';
@@ -218,41 +218,18 @@ export class SpaceMemberService {
     dto: RemoveSpaceMemberDto,
     workspaceId: string,
   ): Promise<void> {
-    const space = await this.spaceRepo.findById(dto.spaceId, workspaceId);
+    const memberTypeId = dto.userId
-    if (!space) {
+      ? { userId: dto.userId }
-      throw new NotFoundException('Space not found');
+      : dto.groupId
-    }
+        ? { groupId: dto.groupId }
+        : null;
 
-    let spaceMember: SpaceMember = null;
+    if (!memberTypeId) {
-
-    if (dto.userId) {
-      spaceMember = await this.spaceMemberRepo.getSpaceMemberByTypeId(
-        dto.spaceId,
-        {
-          userId: dto.userId,
-        },
-      );
-    } else if (dto.groupId) {
-      spaceMember = await this.spaceMemberRepo.getSpaceMemberByTypeId(
-        dto.spaceId,
-        {
-          groupId: dto.groupId,
-        },
-      );
-    } else {
       throw new BadRequestException(
         'Please provide a valid userId or groupId to remove',
       );
     }
 
-    if (!spaceMember) {
-      throw new NotFoundException('Space membership not found');
-    }
-
-    if (spaceMember.role === SpaceRole.ADMIN) {
-      await this.validateLastAdmin(dto.spaceId);
-    }
-
     let affectedUserIds: string[] = [];
     if (dto.userId) {
       affectedUserIds = [dto.userId];
@@ -262,7 +239,29 @@ export class SpaceMemberService {
       );
     }
 
-    await executeTx(this.db, async (trx) => {
+    const { space, spaceMember } = await executeTx(this.db, async (trx) => {
+      const space = await this.spaceRepo.findById(
+        dto.spaceId,
+        workspaceId,
+        { withLock: true, trx },
+      );
+      if (!space) {
+        throw new NotFoundException('Space not found');
+      }
+
+      const spaceMember = await this.spaceMemberRepo.getSpaceMemberByTypeId(
+        dto.spaceId,
+        memberTypeId,
+        trx,
+      );
+      if (!spaceMember) {
+        throw new NotFoundException('Space membership not found');
+      }
+
+      if (spaceMember.role === SpaceRole.ADMIN) {
+        await this.validateLastAdmin(dto.spaceId, trx);
+      }
+
       await this.spaceMemberRepo.removeSpaceMemberById(
         spaceMember.id,
         dto.spaceId,
@@ -280,6 +279,8 @@ export class SpaceMemberService {
         dto.spaceId,
         { trx },
       );
+
+      return { space, spaceMember };
     });
 
     this.auditService.log({
@@ -304,48 +305,40 @@ export class SpaceMemberService {
     dto: UpdateSpaceMemberRoleDto,
     workspaceId: string,
   ): Promise<void> {
-    const space = await this.spaceRepo.findById(dto.spaceId, workspaceId);
+    const memberTypeId = dto.userId
-    if (!space) {
+      ? { userId: dto.userId }
-      throw new NotFoundException('Space not found');
+      : dto.groupId
-    }
+        ? { groupId: dto.groupId }
+        : null;
 
-    let spaceMember: SpaceMember = null;
+    if (!memberTypeId) {
-
-    if (dto.userId) {
-      spaceMember = await this.spaceMemberRepo.getSpaceMemberByTypeId(
-        dto.spaceId,
-        {
-          userId: dto.userId,
-        },
-      );
-    } else if (dto.groupId) {
-      spaceMember = await this.spaceMemberRepo.getSpaceMemberByTypeId(
-        dto.spaceId,
-        {
-          groupId: dto.groupId,
-        },
-      );
-    } else {
       throw new BadRequestException(
         'Please provide a valid userId or groupId to remove',
       );
     }
 
-    if (!spaceMember) {
+    const result = await executeTx(this.db, async (trx) => {
-      throw new NotFoundException('Space membership not found');
+      const space = await this.spaceRepo.findById(
-    }
+        dto.spaceId,
+        workspaceId,
+        { withLock: true, trx },
+      );
+      if (!space) {
+        throw new NotFoundException('Space not found');
+      }
 
-    if (spaceMember.role === dto.role) {
+      const spaceMember = await this.spaceMemberRepo.getSpaceMemberByTypeId(
-      return;
+        dto.spaceId,
-    }
+        memberTypeId,
+        trx,
+      );
+      if (!spaceMember) {
+        throw new NotFoundException('Space membership not found');
+      }
 
-    await executeTx(this.db, async (trx) => {
+      if (spaceMember.role === dto.role) {
-      await trx
+        return { changed: false, space, spaceMember };
-        .selectFrom('spaces')
+      }
-        .select('id')
-        .where('id', '=', dto.spaceId)
-        .forUpdate()
-        .executeTakeFirst();
 
       if (spaceMember.role === SpaceRole.ADMIN) {
         await this.validateLastAdmin(dto.spaceId, trx);
@@ -357,8 +350,16 @@ export class SpaceMemberService {
         dto.spaceId,
         trx,
       );
+
+      return { changed: true, space, spaceMember };
     });
 
+    if (!result.changed) {
+      return;
+    }
+
+    const { space, spaceMember } = result;
+
     this.auditService.log({
       event: AuditEvent.SPACE_MEMBER_ROLE_CHANGED,
       resourceType: AuditResource.SPACE_MEMBER,
@@ -387,7 +388,7 @@ export class SpaceMemberService {
       spaceId,
       trx,
     );
-    if (spaceOwnerCount === 1) {
+    if (spaceOwnerCount <= 1) {
       throw new BadRequestException(
         'There must be at least one space admin with full access',
       );

apps/server/src/core/workspace/services/workspace.service.ts

@@ -747,44 +747,61 @@ export class WorkspaceService {
     userRoleDto: UpdateWorkspaceUserRoleDto,
     workspaceId: string,
   ) {
-    const user = await this.userRepo.findById(userRoleDto.userId, workspaceId);
-
     const newRole = userRoleDto.role.toLowerCase();
+    const result = await executeTx(this.db, async (trx) => {
+      const workspace = await this.workspaceRepo.findById(workspaceId, {
+        withLock: true,
+        trx,
+      });
+      if (!workspace) {
+        throw new NotFoundException('Workspace not found');
+      }
 
-    if (!user) {
+      const user = await this.userRepo.findById(
-      throw new BadRequestException('Workspace member not found');
+        userRoleDto.userId,
-    }
+        workspaceId,
-
+        { trx },
-    // prevent ADMIN from managing OWNER role
-    if (
-      isAdminActingOnOwner(authUser.role, newRole) ||
-      isAdminActingOnOwner(authUser.role, user.role)
-    ) {
-      throw new ForbiddenException();
-    }
-
-    if (user.role === newRole) {
-      return user;
-    }
-
-    const workspaceOwnerCount = await this.userRepo.roleCountByWorkspaceId(
-      UserRole.OWNER,
-      workspaceId,
-    );
-
-    if (user.role === UserRole.OWNER && workspaceOwnerCount === 1) {
-      throw new BadRequestException(
-        'There must be at least one workspace owner',
       );
+      if (!user) {
+        throw new BadRequestException('Workspace member not found');
+      }
+
+      if (
+        isAdminActingOnOwner(authUser.role, newRole) ||
+        isAdminActingOnOwner(authUser.role, user.role)
+      ) {
+        throw new ForbiddenException();
+      }
+
+      if (user.role === newRole) {
+        return { changed: false, user };
+      }
+
+      if (
+        user.role === UserRole.OWNER &&
+        !user.deletedAt &&
+        !user.deactivatedAt
+      ) {
+        await this.validateLastWorkspaceOwner(workspaceId, trx);
+      }
+
+      await this.userRepo.updateUser(
+        {
+          role: newRole,
+        },
+        user.id,
+        workspaceId,
+        trx,
+      );
+
+      return { changed: true, user };
+    });
+
+    if (!result.changed) {
+      return result.user;
     }
 
-    await this.userRepo.updateUser(
+    const { user } = result;
-      {
-        role: newRole,
-      },
-      user.id,
-      workspaceId,
-    );
 
     this.auditService.log({
       event: AuditEvent.USER_ROLE_CHANGED,
@@ -848,40 +865,38 @@ export class WorkspaceService {
     userId: string,
     workspaceId: string,
   ): Promise<void> {
-    const user = await this.userRepo.findById(userId, workspaceId);
+    const user = await executeTx(this.db, async (trx) => {
+      const workspace = await this.workspaceRepo.findById(workspaceId, {
+        withLock: true,
+        trx,
+      });
+      if (!workspace) {
+        throw new NotFoundException('Workspace not found');
+      }
 
-    if (!user || user.deletedAt) {
+      const user = await this.userRepo.findById(userId, workspaceId, { trx });
-      throw new BadRequestException('Workspace member not found');
+      if (!user || user.deletedAt) {
-    }
+        throw new BadRequestException('Workspace member not found');
+      }
 
-    if (user.deactivatedAt) {
+      if (user.deactivatedAt) {
-      throw new BadRequestException('User is already deactivated');
+        throw new BadRequestException('User is already deactivated');
-    }
+      }
 
-    if (authUser.id === userId) {
+      if (authUser.id === userId) {
-      throw new BadRequestException('You cannot deactivate yourself');
+        throw new BadRequestException('You cannot deactivate yourself');
-    }
+      }
 
-    if (isAdminActingOnOwner(authUser.role, user.role)) {
+      if (isAdminActingOnOwner(authUser.role, user.role)) {
-      throw new BadRequestException(
-        'You cannot deactivate a user with owner role',
-      );
-    }
-
-    if (user.role === UserRole.OWNER) {
-      const workspaceOwnerCount = await this.userRepo.roleCountByWorkspaceId(
-        UserRole.OWNER,
-        workspaceId,
-      );
-
-      if (workspaceOwnerCount === 1) {
         throw new BadRequestException(
-          'There must be at least one workspace owner',
+          'You cannot deactivate a user with owner role',
         );
       }
-    }
 
-    await executeTx(this.db, async (trx) => {
+      if (user.role === UserRole.OWNER) {
+        await this.validateLastWorkspaceOwner(workspaceId, trx);
+      }
+
       await this.userRepo.updateUser(
         { deactivatedAt: new Date() },
         userId,
@@ -889,6 +904,8 @@ export class WorkspaceService {
         trx,
       );
       await this.userSessionRepo.revokeByUserId(userId, workspaceId, trx);
+
+      return user;
     });
 
     this.auditService.log({
@@ -951,32 +968,34 @@ export class WorkspaceService {
     userId: string,
     workspaceId: string,
   ): Promise<void> {
-    const user = await this.userRepo.findById(userId, workspaceId);
+    const user = await executeTx(this.db, async (trx) => {
+      const workspace = await this.workspaceRepo.findById(workspaceId, {
+        withLock: true,
+        trx,
+      });
+      if (!workspace) {
+        throw new NotFoundException('Workspace not found');
+      }
 
-    if (!user || user.deletedAt) {
+      const user = await this.userRepo.findById(userId, workspaceId, { trx });
-      throw new BadRequestException('Workspace member not found');
+      if (!user || user.deletedAt) {
-    }
+        throw new BadRequestException('Workspace member not found');
+      }
 
-    const workspaceOwnerCount = await this.userRepo.roleCountByWorkspaceId(
+      if (authUser.id === userId) {
-      UserRole.OWNER,
+        throw new BadRequestException('You cannot delete yourself');
-      workspaceId,
+      }
-    );
 
-    if (user.role === UserRole.OWNER && workspaceOwnerCount === 1) {
+      if (isAdminActingOnOwner(authUser.role, user.role)) {
-      throw new BadRequestException(
+        throw new BadRequestException(
-        'There must be at least one workspace owner',
+          'You cannot delete a user with owner role',
-      );
+        );
-    }
+      }
 
-    if (authUser.id === userId) {
+      if (user.role === UserRole.OWNER && !user.deactivatedAt) {
-      throw new BadRequestException('You cannot delete yourself');
+        await this.validateLastWorkspaceOwner(workspaceId, trx);
-    }
+      }
 
-    if (isAdminActingOnOwner(authUser.role, user.role)) {
-      throw new BadRequestException('You cannot delete a user with owner role');
-    }
-
-    await executeTx(this.db, async (trx) => {
       await this.userRepo.updateUser(
         {
           name: 'Deleted user',
@@ -1009,6 +1028,8 @@ export class WorkspaceService {
       });
 
       await this.userSessionRepo.revokeByUserId(userId, workspaceId, trx);
+
+      return user;
     });
 
     this.auditService.log({
@@ -1030,4 +1051,20 @@ export class WorkspaceService {
       // empty
     }
   }
+
+  private async validateLastWorkspaceOwner(
+    workspaceId: string,
+    trx: KyselyTransaction,
+  ): Promise<void> {
+    const workspaceOwnerCount = await this.userRepo.roleCountByWorkspaceId(
+      UserRole.OWNER,
+      workspaceId,
+      trx,
+    );
+    if (workspaceOwnerCount <= 1) {
+      throw new BadRequestException(
+        'There must be at least one workspace owner',
+      );
+    }
+  }
 }

apps/server/src/database/repos/space/space.repo.ts

@@ -25,7 +25,11 @@ export class SpaceRepo {
   async findById(
     spaceId: string,
     workspaceId: string,
-    opts?: { includeMemberCount?: boolean; trx?: KyselyTransaction },
+    opts?: {
+      includeMemberCount?: boolean;
+      withLock?: boolean;
+      trx?: KyselyTransaction;
+    },
   ): Promise<Space> {
     const db = dbOrTx(this.db, opts?.trx);
 
@@ -41,6 +45,11 @@ export class SpaceRepo {
     } else {
       query = query.where(sql`LOWER(slug)`, '=', sql`LOWER(${spaceId})`);
     }
+
+    if (opts?.withLock && opts?.trx) {
+      query = query.forUpdate();
+    }
+
     return query.executeTakeFirst();
   }
 

apps/server/src/database/repos/user/user.repo.ts

@@ -145,12 +145,16 @@ export class UserRepo {
   async roleCountByWorkspaceId(
     role: string,
     workspaceId: string,
+    trx?: KyselyTransaction,
   ): Promise<number> {
-    const { count } = await this.db
+    const db = dbOrTx(this.db, trx);
+    const { count } = await db
       .selectFrom('users')
       .select((eb) => eb.fn.count('role').as('count'))
       .where('role', '=', role)
       .where('workspaceId', '=', workspaceId)
+      .where('deletedAt', 'is', null)
+      .where('deactivatedAt', 'is', null)
       .executeTakeFirst();
 
     return count as number;