Feat: allow to manage personal groups from the guest profile dialog

backend/src/applications/users/dto/create-or-update-user.dto.ts

@@ -47,7 +47,7 @@ export class OptionalUserDto {
   @IsInt()
   storageQuota?: number
 
-  // for users
+  // for users and guests
   @IsOptional()
   @IsArray()
   @IsInt({ each: true })

backend/src/applications/users/dto/search-members.dto.ts

@@ -26,6 +26,14 @@ export class SearchMembersDto {
   @IsBoolean()
   excludePersonalGroups?: boolean = false
 
+  @IsOptional()
+  @IsBoolean()
+  onlyPersonalGroups?: boolean = false
+
+  @IsOptional()
+  @IsBoolean()
+  isGroupManager?: boolean = false
+
   @IsOptional()
   @IsInt()
   usersRole?: number

backend/src/applications/users/interfaces/guest-user.interface.ts

@@ -4,4 +4,5 @@ import type { Member } from './member.interface'
 export type GuestUser = Partial<User> & {
   fullName: string
   managers?: Member[]
+  groups?: Member[]
 }

backend/src/applications/users/services/admin-users-manager.service.spec.ts

@@ -292,11 +292,41 @@ describe(AdminUsersManager.name, () => {
       expect(res).toEqual(updatedGuest)
     })
 
+    it('update guest + groups diff', async () => {
+      const guest = {
+        id: 33,
+        login: 'g',
+        email: 'g@x',
+        managers: [{ id: 2 }],
+        groups: [{ id: 1 }, { id: 3 }],
+        role: USER_ROLE.GUEST
+      }
+      setGuest(guest as any)
+      const updatedGuest = { ...guest, groups: [{ id: 3 }, { id: 5 }] }
+      setGuest(updatedGuest as any)
+
+      adminQueriesMock.updateUserGroups.mockResolvedValueOnce(undefined)
+
+      const res = await service.updateUserOrGuest(guest.id, { groups: [3, 5] } as UpdateUserDto, USER_ROLE.GUEST)
+      expect(adminQueriesMock.usersQueries.updateUserOrGuest).not.toHaveBeenCalled()
+      expect(adminQueriesMock.updateUserGroups).toHaveBeenCalledWith(guest.id, { add: [5], delete: [1] })
+      expect(adminQueriesMock.updateGuestManagers).not.toHaveBeenCalled()
+      expect(res).toEqual(updatedGuest)
+    })
+
     it('validations updateGuest', async () => {
       expect(() => service.updateGuest(1, {} as any)).toThrow(/no changes to update/i)
       expect(() => service.updateGuest(1, { managers: [] } as any)).toThrow(/guest must have at least one manager/i)
     })
 
+    it('updateUserGroups échoue pour guest', async () => {
+      const guest = { id: 33, login: 'g', email: 'g@x', managers: [{ id: 2 }], groups: [{ id: 1 }], role: USER_ROLE.GUEST }
+      setGuest(guest as any)
+      adminQueriesMock.updateUserGroups.mockRejectedValueOnce(new Error('group error'))
+      await expect(service.updateUserOrGuest(guest.id, { groups: [2] } as any, USER_ROLE.GUEST)).rejects.toThrow('Unable to update user groups')
+      expect(adminQueriesMock.updateGuestManagers).not.toHaveBeenCalled()
+    })
+
     it('updateGuestManagers échoue', async () => {
       const guest = { id: 33, login: 'g', email: 'g@x', managers: [{ id: 2 }], role: USER_ROLE.GUEST }
       setGuest(guest as any)

backend/src/applications/users/services/admin-users-manager.service.ts

@@ -62,7 +62,7 @@ export class AdminUsersManager {
         tag: this.createUserOrGuest.name,
         msg: `${USER_ROLE[userRole]} (${userId}) was created : ${JSON.stringify(anonymizePassword(createUserDto))}`
       })
-      this.adminQueries.usersQueries.clearWhiteListCaches('*')
+      void this.adminQueries.usersQueries.clearWhiteListCaches('*')
       await user.makePaths()
       if (userRole <= USER_ROLE.USER) {
         return asAdmin ? this.getUser(user.id) : user
@@ -86,6 +86,7 @@ export class AdminUsersManager {
     const updateUser: Partial<User> = {}
     const updateUserGroups: { add: number[]; delete: number[] } = { add: [], delete: [] }
     const updateGuestManagers: { add: number[]; delete: number[] } = { add: [], delete: [] }
+
     for (const [k, v] of Object.entries(updateUserDto)) {
       switch (k as keyof UpdateUserDto) {
         case 'login':
@@ -120,13 +121,13 @@ export class AdminUsersManager {
         case 'password':
           updateUser.password = await hashPassword(updateUserDto.password)
           break
-        case 'groups':
-          if (userRole === USER_ROLE.USER) {
-            const currentGroups: number[] = user.groups?.length ? user.groups.map((g) => g.id) : []
-            updateUserGroups.add = v.filter((id: number) => currentGroups.indexOf(id) === -1)
-            updateUserGroups.delete = currentGroups.filter((id: number) => v.indexOf(id) === -1)
-          }
+        case 'groups': {
+          // allowed for users and guests
+          const currentGroups: number[] = user.groups?.length ? user.groups.map((g) => g.id) : []
+          updateUserGroups.add = v.filter((id: number) => currentGroups.indexOf(id) === -1)
+          updateUserGroups.delete = currentGroups.filter((id: number) => v.indexOf(id) === -1)
           break
+        }
         case 'managers':
           if (userRole === USER_ROLE.GUEST) {
             const currentManagers: number[] = user.managers?.length ? user.managers.map((m) => m.id) : []
@@ -138,32 +139,32 @@ export class AdminUsersManager {
           updateUser[k] = v
       }
     }
+
     if (Object.keys(updateUser).length) {
       // force the type for security reason
       const forceRole = userRole === USER_ROLE.GUEST ? USER_ROLE.GUEST : undefined
       if (!(await this.adminQueries.usersQueries.updateUserOrGuest(user.id, updateUser, forceRole))) {
         throw new HttpException('Unable to update user', HttpStatus.INTERNAL_SERVER_ERROR)
       }
     }
-    if (userRole === USER_ROLE.USER) {
-      if (updateUserGroups.add.length || updateUserGroups.delete.length) {
-        try {
-          await this.adminQueries.updateUserGroups(user.id, updateUserGroups)
-        } catch {
-          throw new HttpException('Unable to update user groups', HttpStatus.INTERNAL_SERVER_ERROR)
-        }
+
+    if (updateUserGroups.add.length || updateUserGroups.delete.length) {
+      try {
+        await this.adminQueries.updateUserGroups(user.id, updateUserGroups)
+      } catch {
+        throw new HttpException('Unable to update user groups', HttpStatus.INTERNAL_SERVER_ERROR)
       }
-      return this.getUser(userId)
-    } else {
-      if (updateGuestManagers.add.length || updateGuestManagers.delete.length) {
-        try {
-          await this.adminQueries.updateGuestManagers(user.id, updateGuestManagers)
-        } catch {
-          throw new HttpException('Unable to update guest managers', HttpStatus.INTERNAL_SERVER_ERROR)
-        }
+    }
+
+    if (userRole === USER_ROLE.GUEST && (updateGuestManagers.add.length || updateGuestManagers.delete.length)) {
+      try {
+        await this.adminQueries.updateGuestManagers(user.id, updateGuestManagers)
+      } catch {
+        throw new HttpException('Unable to update guest managers', HttpStatus.INTERNAL_SERVER_ERROR)
       }
-      return this.getGuest(userId)
     }
+
+    return userRole === USER_ROLE.USER ? this.getUser(userId) : this.getGuest(userId)
   }
 
   async deleteUserOrGuest(userId: number, userLogin: string, deleteUserDto: DeleteUserDto): Promise<void> {
@@ -258,7 +259,7 @@ export class AdminUsersManager {
     }
     // Clear whitelist caches when the group’s visibility is changed
     if (updateGroupDto.visibility !== undefined) {
-      this.adminQueries.usersQueries.clearWhiteListCaches('*')
+      void this.adminQueries.usersQueries.clearWhiteListCaches('*')
     }
     return this.adminQueries.groupFromId(groupId)
   }

backend/src/applications/users/services/admin-users-queries.service.ts

@@ -204,7 +204,7 @@ export class AdminUsersQueries {
           groups.add.length
         )
         this.logger.log({ tag: this.updateUserGroups.name, msg: `user (${userId}) groups ${JSON.stringify(groups.add)} was added` })
-        this.usersQueries.clearWhiteListCaches([userId])
+        void this.usersQueries.clearWhiteListCaches([userId])
       } catch (e) {
         this.logger.error({ tag: this.updateUserGroups.name, msg: `user (${userId}) groups ${JSON.stringify(groups.add)} was not added: ${e}` })
         throw new Error('User groups was not added')
@@ -217,7 +217,7 @@ export class AdminUsersQueries {
           groups.delete.length
         )
         this.logger.log({ tag: this.updateUserGroups.name, msg: `user (${userId}) groups ${JSON.stringify(groups.delete)} was deleted` })
-        this.usersQueries.clearWhiteListCaches([userId])
+        void this.usersQueries.clearWhiteListCaches([userId])
       } catch (e) {
         this.logger.error({ tag: this.updateUserGroups.name, msg: `user (${userId}) groups ${JSON.stringify(groups.delete)} was not deleted: ${e}` })
         throw new Error('User groups was not deleted')
@@ -273,7 +273,7 @@ export class AdminUsersQueries {
         userIdsWithRequiredRole.length
       )
       this.logger.log({ tag: this.addUsersToGroup.name, msg: `users (${userIds}) was added to group (${groupId})` })
-      this.usersQueries.clearWhiteListCaches(userIds)
+      void this.usersQueries.clearWhiteListCaches(userIds)
     } catch (e) {
       this.logger.error({ tag: this.addUsersToGroup.name, msg: `unable to add users (${userIds}) to group (${groupId}) : ${e}` })
       throw new Error('Unable to add users to group')
@@ -310,7 +310,7 @@ export class AdminUsersQueries {
         1
       )
       this.logger.log({ tag: this.removeUserFromGroup.name, msg: `user (${userId}) was removed from group (${groupId})` })
-      this.usersQueries.clearWhiteListCaches([userId])
+      void this.usersQueries.clearWhiteListCaches([userId])
     } catch (e) {
       this.logger.error({ tag: this.removeUserFromGroup.name, msg: `user (${userId}) or group (${groupId}) does not exist : ${e}` })
       throw new Error('Unable to remove user from group')

backend/src/applications/users/services/users-manager.service.spec.ts

@@ -11,6 +11,7 @@ import { DB_TOKEN_PROVIDER } from '../../../infrastructure/database/constants'
 import * as filesUtilsModule from '../../files/utils/files'
 import { fileName, isPathExists } from '../../files/utils/files'
 import { NotificationsManager } from '../../notifications/services/notifications-manager.service'
+import { GROUP_TYPE } from '../constants/group'
 import { MEMBER_TYPE } from '../constants/member'
 import { USER_GROUP_ROLE, USER_MAX_PASSWORD_ATTEMPTS, USER_ROLE } from '../constants/user'
 import { CreateUserDto } from '../dto/create-or-update-user.dto'
@@ -514,21 +515,23 @@ describe(UsersManager.name, () => {
     expect(lgSpy).toHaveBeenCalledWith(expect.objectContaining({ msg: expect.stringMatching(/was deleted/) }))
   })
 
-  it('guests + proxies', async () => {
+  it('guests list + get', async () => {
     usersQueriesService.listGuests = jest.fn().mockResolvedValue([{ id: 1 }])
     await expect(usersManager.listGuests(userTest)).resolves.toEqual([{ id: 1 }])
+
     const checkSpy = jest.spyOn(adminUsersManager, 'checkUser').mockImplementation(() => undefined)
     usersQueriesService.listGuests = jest.fn().mockResolvedValue({ id: 9 })
     await expect(usersManager.getGuest(userTest, 9)).resolves.toEqual({ id: 9 })
     expect(checkSpy).toHaveBeenCalled()
+  })
 
+  it('createGuest adds current user as manager only once', async () => {
     usersQueriesService.usersWhitelist = jest.fn().mockResolvedValue([userTest.id, 100])
-    usersQueriesService.clearWhiteListCaches = jest.fn()
     const createSpy = jest.spyOn(adminUsersManager, 'createUserOrGuest').mockResolvedValue({ id: 55 } as any)
+
     const dto1: CreateUserDto = { ...userTest, managers: [100], password: 'x' }
     const r = await usersManager.createGuest(userTest, dto1)
     expect(createSpy).toHaveBeenCalled()
-    expect(usersQueriesService.clearWhiteListCaches).toHaveBeenCalledWith([userTest.id])
     expect(r).toEqual({ id: 55 })
     const args1 = (createSpy as jest.Mock).mock.calls[0][0]
     expect(args1.managers).toEqual(expect.arrayContaining([userTest.id]))
@@ -537,9 +540,40 @@ describe(UsersManager.name, () => {
     await usersManager.createGuest(userTest, dto2)
     const args2 = (createSpy as jest.Mock).mock.calls[0][0]
     expect((args2.managers as number[]).filter((m: number) => m === userTest.id)).toHaveLength(1)
+  })
+
+  it('createGuest groups filtering keeps only allowed groups and logs warning', async () => {
+    const warnSpy = jest.spyOn((usersManager as any)['logger'], 'warn').mockImplementation(() => undefined as any)
+    usersQueriesService.usersWhitelist = jest.fn().mockResolvedValue([userTest.id, 100])
+    usersQueriesService.groupsWhitelist = jest.fn().mockResolvedValue([10])
+    const createSpy = jest.spyOn(adminUsersManager, 'createUserOrGuest').mockResolvedValue({ id: 55 } as any)
 
+    const dto: CreateUserDto = { ...userTest, managers: [100], groups: [10, 11], password: 'x' }
+    await expect(usersManager.createGuest(userTest, dto)).resolves.toEqual({ id: 55 })
+    expect(usersQueriesService.groupsWhitelist).toHaveBeenCalledWith(userTest.id, GROUP_TYPE.PERSONAL, USER_GROUP_ROLE.MANAGER)
+    const args = (createSpy as jest.Mock).mock.calls[0][0]
+    expect(args.groups).toEqual([10])
+    expect(warnSpy).toHaveBeenCalledWith(expect.objectContaining({ msg: 'Some groups were not allowed' }))
+  })
+
+  it('createGuest groups keeps all allowed groups without warning', async () => {
+    const warnSpy = jest.spyOn((usersManager as any)['logger'], 'warn').mockImplementation(() => undefined as any)
+    usersQueriesService.usersWhitelist = jest.fn().mockResolvedValue([userTest.id, 100])
+    usersQueriesService.groupsWhitelist = jest.fn().mockResolvedValue([10, 11])
+    const createSpy = jest.spyOn(adminUsersManager, 'createUserOrGuest').mockResolvedValue({ id: 55 } as any)
+
+    const dto: CreateUserDto = { ...userTest, managers: [100], groups: [10, 11], password: 'x' }
+    await expect(usersManager.createGuest(userTest, dto)).resolves.toEqual({ id: 55 })
+    expect(usersQueriesService.groupsWhitelist).toHaveBeenCalledWith(userTest.id, GROUP_TYPE.PERSONAL, USER_GROUP_ROLE.MANAGER)
+    const args = (createSpy as jest.Mock).mock.calls[0][0]
+    expect(args.groups).toEqual([10, 11])
+    expect(warnSpy).not.toHaveBeenCalled()
+  })
+
+  it('updateGuest checks ownership and manager whitelist', async () => {
     await expect(usersManager.updateGuest(userTest, 9, {} as any)).rejects.toThrow('No changes to update')
     usersQueriesService.usersWhitelist = jest.fn().mockResolvedValue([1])
+    usersQueriesService.isGuestManager = jest.fn().mockResolvedValue(true)
     await expect(usersManager.updateGuest(userTest, 9, { managers: [2] } as any)).rejects.toThrow('Guest must have at least one manager')
     usersQueriesService.isGuestManager = jest.fn().mockResolvedValue(false)
     await expect(usersManager.updateGuest(userTest, 9, { email: 'a' } as any)).rejects.toThrow('You are not allowed to do this action')
@@ -554,14 +588,58 @@ describe(UsersManager.name, () => {
     await expect(usersManager.updateGuest(userTest, 9, { managers: [userTest.id, 77] } as any)).resolves.toEqual({
       managers: [{ id: userTest.id }, { id: 77 }]
     })
+  })
+
+  it('updateGuest groups forbidden when current user is not manager', async () => {
+    const updateSpy = jest.spyOn(adminUsersManager, 'updateUserOrGuest')
+    usersQueriesService.isGuestManager = jest.fn().mockResolvedValue(false)
+    usersQueriesService.groupsWhitelist = jest.fn()
+    await expect(usersManager.updateGuest(userTest, 9, { groups: [10] } as any)).rejects.toThrow('You are not allowed to do this action')
+    expect(usersQueriesService.groupsWhitelist).not.toHaveBeenCalled()
+    expect(updateSpy).not.toHaveBeenCalled()
+  })
 
+  it('updateGuest groups filtering logs warning when some groups are rejected', async () => {
+    const warnSpy = jest.spyOn((usersManager as any)['logger'], 'warn').mockImplementation(() => undefined as any)
+    const updateSpy = jest.spyOn(adminUsersManager, 'updateUserOrGuest')
+    usersQueriesService.isGuestManager = jest.fn().mockResolvedValue(true)
+    usersQueriesService.groupsWhitelist = jest.fn().mockResolvedValue([10])
+    updateSpy.mockResolvedValue({ managers: [{ id: userTest.id }], groups: [{ id: 10 }] } as any)
+
+    await expect(usersManager.updateGuest(userTest, 9, { groups: [10, 11] } as any)).resolves.toEqual({
+      managers: [{ id: userTest.id }],
+      groups: [{ id: 10 }]
+    })
+    expect(usersQueriesService.groupsWhitelist).toHaveBeenCalledWith(userTest.id, GROUP_TYPE.PERSONAL, USER_GROUP_ROLE.MANAGER)
+    expect(updateSpy).toHaveBeenLastCalledWith(9, { groups: [10] }, USER_ROLE.GUEST)
+    expect(warnSpy).toHaveBeenCalledWith(expect.objectContaining({ msg: 'Some groups were not allowed' }))
+  })
+
+  it('updateGuest groups does not log warning when all groups are allowed', async () => {
+    const warnSpy = jest.spyOn((usersManager as any)['logger'], 'warn').mockImplementation(() => undefined as any)
+    const updateSpy = jest.spyOn(adminUsersManager, 'updateUserOrGuest')
+    usersQueriesService.isGuestManager = jest.fn().mockResolvedValue(true)
+    usersQueriesService.groupsWhitelist = jest.fn().mockResolvedValue([10, 11])
+    updateSpy.mockResolvedValue({ managers: [{ id: userTest.id }], groups: [{ id: 10 }, { id: 11 }] } as any)
+
+    await expect(usersManager.updateGuest(userTest, 9, { groups: [10, 11] } as any)).resolves.toEqual({
+      managers: [{ id: userTest.id }],
+      groups: [{ id: 10 }, { id: 11 }]
+    })
+    expect(updateSpy).toHaveBeenLastCalledWith(9, { groups: [10, 11] }, USER_ROLE.GUEST)
+    expect(warnSpy).not.toHaveBeenCalled()
+  })
+
+  it('deleteGuest checks ownership then deletes guest', async () => {
     usersQueriesService.isGuestManager = jest.fn().mockResolvedValue(null)
     await expect(usersManager.deleteGuest(userTest, 9)).rejects.toThrow('You are not allowed to do this action')
     usersQueriesService.isGuestManager = jest.fn().mockResolvedValue({ id: 9, login: 'guest' })
     const delSpy = jest.spyOn(adminUsersManager, 'deleteUserOrGuest').mockResolvedValue(undefined)
     await expect(usersManager.deleteGuest(userTest, 9)).resolves.toBeUndefined()
     expect(delSpy).toHaveBeenCalledWith(9, 'guest', { deleteSpace: true, isGuest: true })
+  })
 
+  it('proxies forward search + online + whitelist', async () => {
     usersQueriesService.searchUsersOrGroups = jest.fn().mockResolvedValue([{ id: 1 }])
     await expect(usersManager.searchMembers(userTest, { search: '' } as any)).resolves.toEqual([{ id: 1 }])