From 272a1fa64bca49f6f9927fc9e6381c2ab756103a Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Tue, 10 Dec 2024 17:22:53 +0100 Subject: [PATCH 01/19] add migrations --- .../mikro-orm/Migration20241113100535.ts | 4 +- .../mikro-orm/Migration20241209165812.ts | 40 +++++++++++++++++++ .../mikro-orm/Migration20241210152600.ts | 37 +++++++++++++++++ 3 files changed, 79 insertions(+), 2 deletions(-) create mode 100644 apps/server/src/migrations/mikro-orm/Migration20241209165812.ts create mode 100644 apps/server/src/migrations/mikro-orm/Migration20241210152600.ts diff --git a/apps/server/src/migrations/mikro-orm/Migration20241113100535.ts b/apps/server/src/migrations/mikro-orm/Migration20241113100535.ts index d233b984596..4bfb49bbbe2 100644 --- a/apps/server/src/migrations/mikro-orm/Migration20241113100535.ts +++ b/apps/server/src/migrations/mikro-orm/Migration20241113100535.ts @@ -46,7 +46,7 @@ export class Migration20241113100535 extends Migration { ); if (teacherRoleUpdate.modifiedCount > 0) { - console.info('Rollback: Permission ROOM_CREATE added to role teacher.'); + console.info('Rollback: Permission ROOM_CREATE removed from role teacher.'); } const roomEditorRoleUpdate = await this.getCollection('roles').updateOne( @@ -61,7 +61,7 @@ export class Migration20241113100535 extends Migration { ); if (roomEditorRoleUpdate.modifiedCount > 0) { - console.info('Rollback: Permission ROOM_DELETE added to role roomeditor.'); + console.info('Rollback: Permission ROOM_DELETE removed from role roomeditor.'); } } } diff --git a/apps/server/src/migrations/mikro-orm/Migration20241209165812.ts b/apps/server/src/migrations/mikro-orm/Migration20241209165812.ts new file mode 100644 index 00000000000..ffa54bbc778 --- /dev/null +++ b/apps/server/src/migrations/mikro-orm/Migration20241209165812.ts @@ -0,0 +1,40 @@ +import { Migration } from '@mikro-orm/migrations-mongodb'; + +export class Migration20241209165812 extends Migration { + async up(): Promise { + // Add ROOM_OWNER role + await this.getCollection('roles').insertOne({ + name: 'roomowner', + permissions: [ + 'ROOM_VIEW', + 'ROOM_EDIT', + 'ROOM_DELETE', + 'ROOM_MEMBERS_ADD', + 'ROOM_MEMBERS_REMOVE', + 'ROOM_CHANGE_OWNER', + ], + }); + console.info( + 'Added ROOM_OWNER role with ROOM_VIEW, -_EDIT, _DELETE, -_MEMBERS_ADD, -_MEMBERS_REMOVE AND -_CHANGE_OWNER permission' + ); + + // Add ROOM_ADMIN role + await this.getCollection('roles').insertOne({ + name: 'roomadmin', + permissions: ['ROOM_VIEW', 'ROOM_EDIT', 'ROOM_MEMBERS_ADD', 'ROOM_MEMBERS_REMOVE'], + }); + console.info( + 'Added ROOM_ADMIN role with ROOM_VIEW, ROOM_EDIT, ROOM_MEMBERS_ADD AND ROOM_MEMBERS_REMOVE permissions' + ); + } + + async down(): Promise { + // Remove ROOM_OWNER role + await this.getCollection('roles').deleteOne({ name: 'roomowner' }); + console.info('Rollback: Removed ROOM_OWNER role'); + + // Remove ROOM_ADMIN role + await this.getCollection('roles').deleteOne({ name: 'roomadmin' }); + console.info('Rollback: Removed ROOM_ADMIN role'); + } +} diff --git a/apps/server/src/migrations/mikro-orm/Migration20241210152600.ts b/apps/server/src/migrations/mikro-orm/Migration20241210152600.ts new file mode 100644 index 00000000000..5f88d698b99 --- /dev/null +++ b/apps/server/src/migrations/mikro-orm/Migration20241210152600.ts @@ -0,0 +1,37 @@ +import { Migration } from '@mikro-orm/migrations-mongodb'; + +export class Migration20241210152600 extends Migration { + async up(): Promise { + const roomEditorRoleUpdate = await this.getCollection('roles').updateOne( + { name: 'roomeditor' }, + { + $set: { + permissions: ['ROOM_VIEW', 'ROOM_EDIT', 'ROOM_MEMBERS_ADD', 'ROOM_MEMBERS_REMOVE'], + }, + } + ); + + if (roomEditorRoleUpdate.modifiedCount > 0) { + console.info( + 'Rollback: Permission ROOM_DELETE removed from and ROOM_MEMBERS_ADD and ROOM_MEMBERS_REMOVE added to role roomeditor.' + ); + } + } + + async down(): Promise { + const roomEditorRoleUpdate = await this.getCollection('roles').updateOne( + { name: 'roomeditor' }, + { + $set: { + permissions: ['ROOM_VIEW', 'ROOM_EDIT', 'ROOM_DELETE'], + }, + } + ); + + if (roomEditorRoleUpdate.modifiedCount > 0) { + console.info( + 'Permissions ROOM_DELETE added to and ROOM_MEMBERS_ADD and ROOM_MEMBERS_REMOVE from role roomeditor.' + ); + } + } +} From ad97fc0417d2a04835eff4a782a838cbdb0c338f Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Tue, 10 Dec 2024 17:44:58 +0100 Subject: [PATCH 02/19] separate adding owner to new room from regular member adding --- .../service/room-membership.service.ts | 26 ++++++++--------- apps/server/src/modules/room/api/room.uc.ts | 29 ++++++++++--------- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/apps/server/src/modules/room-membership/service/room-membership.service.ts b/apps/server/src/modules/room-membership/service/room-membership.service.ts index 8baa23dd643..1aeb909ba67 100644 --- a/apps/server/src/modules/room-membership/service/room-membership.service.ts +++ b/apps/server/src/modules/room-membership/service/room-membership.service.ts @@ -20,11 +20,7 @@ export class RoomMembershipService { private readonly userService: UserService ) {} - private async createNewRoomMembership( - roomId: EntityId, - userId: EntityId, - roleName: RoleName.ROOMEDITOR | RoleName.ROOMVIEWER - ): Promise { + public async createNewRoomMembership(roomId: EntityId, ownerUserId: EntityId): Promise { const room = await this.roomService.getSingleRoom(roomId); const group = await this.groupService.createGroup( @@ -32,7 +28,7 @@ export class RoomMembershipService { GroupTypes.ROOM, room.schoolId ); - await this.groupService.addUsersToGroup(group.id, [{ userId, roleName }]); + await this.groupService.addUsersToGroup(group.id, [{ userId: ownerUserId, roleName: RoleName.ROOMOWNER }]); const roomMembership = new RoomMembership({ id: new ObjectId().toHexString(), @@ -79,16 +75,14 @@ export class RoomMembershipService { public async addMembersToRoom( roomId: EntityId, - userIdsAndRoles: Array<{ userId: EntityId; roleName: RoleName.ROOMEDITOR | RoleName.ROOMVIEWER }> + userIdsAndRoles: Array<{ + userId: EntityId; + roleName: RoleName.ROOMADMIN | RoleName.ROOMEDITOR | RoleName.ROOMVIEWER; + }> ): Promise { const roomMembership = await this.roomMembershipRepo.findByRoomId(roomId); if (roomMembership === null) { - const firstUser = userIdsAndRoles.shift(); - if (firstUser === undefined) { - throw new BadRequestException('No user provided'); - } - const newRoomMembership = await this.createNewRoomMembership(roomId, firstUser.userId, firstUser.roleName); - return newRoomMembership.id; + throw new Error('Room membership not found'); } await this.groupService.addUsersToGroup(roomMembership.userGroupId, userIdsAndRoles); @@ -106,6 +100,12 @@ export class RoomMembershipService { } const group = await this.groupService.findById(roomMembership.userGroupId); + + // TODO: fail if trying to remove owner + // const hasOwner = group.users + // .filter((user) => userIds.includes(user.userId)) + // .some((groupUser) => groupUser.roleName === RoleName.ROOMOWNER); + await this.groupService.removeUsersFromGroup(group.id, userIds); await this.handleGuestRoleRemoval(userIds, roomMembership.schoolId); diff --git a/apps/server/src/modules/room/api/room.uc.ts b/apps/server/src/modules/room/api/room.uc.ts index a80e2838c66..cc44a4c68e4 100644 --- a/apps/server/src/modules/room/api/room.uc.ts +++ b/apps/server/src/modules/room/api/room.uc.ts @@ -5,7 +5,7 @@ import { Injectable } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { FeatureDisabledLoggableException } from '@shared/common/loggable-exception'; import { Page, UserDO } from '@shared/domain/domainobject'; -import { IFindOptions, Permission, RoleName, RoomRole } from '@shared/domain/interface'; +import { IFindOptions, Permission, RoleName } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { BoardExternalReferenceType, ColumnBoard, ColumnBoardService } from '@modules/board'; import { Room, RoomService } from '../domain'; @@ -40,14 +40,13 @@ export class RoomUc { this.authorizationService.checkOneOfPermissions(user, [Permission.ROOM_CREATE]); - await this.roomMembershipService - .addMembersToRoom(room.id, [{ userId: user.id, roleName: RoleName.ROOMEDITOR }]) - .catch(async (err) => { - await this.roomService.deleteRoom(room); - throw err; - }); - - return room; + try { + await this.roomMembershipService.createNewRoomMembership(room.id, userId); + return room; + } catch (err) { + await this.roomService.deleteRoom(room); + throw err; + } } public async getSingleRoom(userId: EntityId, roomId: EntityId): Promise<{ room: Room; permissions: Permission[] }> { @@ -129,14 +128,17 @@ export class RoomUc { public async addMembersToRoom( currentUserId: EntityId, roomId: EntityId, - userIdsAndRoles: Array<{ userId: EntityId; roleName: RoomRole }> + userIdsAndRoles: Array<{ + userId: EntityId; + roleName: RoleName.ROOMADMIN | RoleName.ROOMEDITOR | RoleName.ROOMVIEWER; + }> ): Promise { this.checkFeatureEnabled(); - await this.checkRoomAuthorization(currentUserId, roomId, Action.write); + await this.checkRoomAuthorization(currentUserId, roomId, Action.write, [Permission.ROOM_MEMBERS_ADD]); await this.roomMembershipService.addMembersToRoom(roomId, userIdsAndRoles); } - private mapToMember(member: UserWithRoomRoles, user: UserDO) { + private mapToMember(member: UserWithRoomRoles, user: UserDO): RoomMemberResponse { return new RoomMemberResponse({ userId: member.userId, firstName: user.firstName, @@ -148,7 +150,7 @@ export class RoomUc { public async removeMembersFromRoom(currentUserId: EntityId, roomId: EntityId, userIds: EntityId[]): Promise { this.checkFeatureEnabled(); - await this.checkRoomAuthorization(currentUserId, roomId, Action.write); + await this.checkRoomAuthorization(currentUserId, roomId, Action.write, [Permission.ROOM_MEMBERS_REMOVE]); await this.roomMembershipService.removeMembersFromRoom(roomId, userIds); } @@ -171,6 +173,7 @@ export class RoomUc { ): Promise { const roomMembershipAuthorizable = await this.roomMembershipService.getRoomMembershipAuthorizable(roomId); const user = await this.authorizationService.getUserWithPermissions(userId); + console.log('user.roles', user.roles); this.authorizationService.checkPermission(user, roomMembershipAuthorizable, { action, requiredPermissions }); return roomMembershipAuthorizable; From 99a4e9d76fab755f636459a9d8d67ee2d9ccc91b Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Tue, 10 Dec 2024 17:48:01 +0100 Subject: [PATCH 03/19] fix room membership rule to use room role to check permissions --- .../authorization/room-membership.rule.ts | 36 ++++++++++++++++--- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/apps/server/src/modules/room-membership/authorization/room-membership.rule.ts b/apps/server/src/modules/room-membership/authorization/room-membership.rule.ts index 3336e93892f..544a8bdfacf 100644 --- a/apps/server/src/modules/room-membership/authorization/room-membership.rule.ts +++ b/apps/server/src/modules/room-membership/authorization/room-membership.rule.ts @@ -10,18 +10,18 @@ export class RoomMembershipRule implements Rule { this.authorisationInjectionService.injectAuthorizationRule(this); } - public isApplicable(user: User, object: unknown): boolean { + public isApplicable(_: User, object: unknown): boolean { const isMatched = object instanceof RoomMembershipAuthorizable; return isMatched; } public hasPermission(user: User, object: RoomMembershipAuthorizable, context: AuthorizationContext): boolean { - const primarySchoolId = user.school.id; - const secondarySchools = user.secondarySchools ?? []; - const secondarySchoolIds = secondarySchools.map(({ school }) => school.id); + if (!this.hasAccessToSchool(user, object.schoolId)) { + return false; + } - if (![primarySchoolId, ...secondarySchoolIds].includes(object.schoolId)) { + if (!this.hasRequiredRoomPermissions(user, object, context.requiredPermissions)) { return false; } @@ -36,4 +36,30 @@ export class RoomMembershipRule implements Rule { } return permissionsThisUserHas.includes(Permission.ROOM_EDIT); } + + private hasAccessToSchool(user: User, schoolId: string): boolean { + const primarySchoolId = user.school.id; + const secondarySchools = user.secondarySchools ?? []; + const secondarySchoolIds = secondarySchools.map(({ school }) => school.id); + + return [primarySchoolId, ...secondarySchoolIds].includes(schoolId); + } + + private hasRequiredRoomPermissions( + user: User, + object: RoomMembershipAuthorizable, + requiredPermissions: string[] + ): boolean { + const roomPermissionsOfUser = this.resolveRoomPermissions(user, object); + const missingPermissions = requiredPermissions.filter((permission) => !roomPermissionsOfUser.includes(permission)); + return missingPermissions.length === 0; + } + + private resolveRoomPermissions(user: User, object: RoomMembershipAuthorizable): string[] { + const member = object.members.find((m) => m.userId === user.id); + if (!member) { + return []; + } + return member.roles.flatMap((role) => role.permissions ?? []); + } } From af26650b3d6b29ef2ff03c8f822e8a1fd744ad9d Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Tue, 10 Dec 2024 17:49:34 +0100 Subject: [PATCH 04/19] other changes --- .../authorization/room-membership.rule.spec.ts | 11 +++++++++++ .../api/dto/request/add-room-members.body.params.ts | 4 ++-- .../src/shared/domain/interface/permission.enum.ts | 3 +++ .../src/shared/domain/interface/rolename.enum.ts | 9 ++++++++- 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/apps/server/src/modules/room-membership/authorization/room-membership.rule.spec.ts b/apps/server/src/modules/room-membership/authorization/room-membership.rule.spec.ts index 0326bb2d02b..24384cd6b2b 100644 --- a/apps/server/src/modules/room-membership/authorization/room-membership.rule.spec.ts +++ b/apps/server/src/modules/room-membership/authorization/room-membership.rule.spec.ts @@ -116,6 +116,17 @@ describe(RoomMembershipRule.name, () => { expect(res).toBe(false); }); + + it('should return false for change owner action', () => { + const { user, roomMembershipAuthorizable } = setup(); + + const res = service.hasPermission(user, roomMembershipAuthorizable, { + action: Action.read, + requiredPermissions: [Permission.ROOM_CHANGE_OWNER], + }); + + expect(res).toBe(false); + }); }); describe('when user is not member of room', () => { diff --git a/apps/server/src/modules/room/api/dto/request/add-room-members.body.params.ts b/apps/server/src/modules/room/api/dto/request/add-room-members.body.params.ts index 93cb5556460..9980d106fb9 100644 --- a/apps/server/src/modules/room/api/dto/request/add-room-members.body.params.ts +++ b/apps/server/src/modules/room/api/dto/request/add-room-members.body.params.ts @@ -1,7 +1,7 @@ import { ApiProperty } from '@nestjs/swagger'; import { IsMongoId, IsString, ValidateNested } from 'class-validator'; import { Type } from 'class-transformer'; -import { RoomRole, RoomRoleArray } from '@shared/domain/interface'; +import { RoleName, RoomRoleArray } from '@shared/domain/interface'; class UserIdAndRole { @ApiProperty({ @@ -17,7 +17,7 @@ class UserIdAndRole { enum: RoomRoleArray, }) @IsString() - roleName!: RoomRole; + roleName!: RoleName.ROOMADMIN | RoleName.ROOMEDITOR | RoleName.ROOMVIEWER; } export class AddRoomMembersBodyParams { diff --git a/apps/server/src/shared/domain/interface/permission.enum.ts b/apps/server/src/shared/domain/interface/permission.enum.ts index c5bed37ad11..bd1c2b0d255 100644 --- a/apps/server/src/shared/domain/interface/permission.enum.ts +++ b/apps/server/src/shared/domain/interface/permission.enum.ts @@ -104,6 +104,9 @@ export enum Permission { ROOM_EDIT = 'ROOM_EDIT', ROOM_VIEW = 'ROOM_VIEW', ROOM_DELETE = 'ROOM_DELETE', + ROOM_MEMBERS_ADD = 'ROOM_MEMBERS_ADD', + ROOM_MEMBERS_REMOVE = 'ROOM_MEMBERS_REMOVE', + ROOM_CHANGE_OWNER = 'ROOM_CHANGE_OWNER', SCHOOL_CHAT_MANAGE = 'SCHOOL_CHAT_MANAGE', SCHOOL_CREATE = 'SCHOOL_CREATE', SCHOOL_EDIT = 'SCHOOL_EDIT', diff --git a/apps/server/src/shared/domain/interface/rolename.enum.ts b/apps/server/src/shared/domain/interface/rolename.enum.ts index e354109efd3..310f80cf84f 100644 --- a/apps/server/src/shared/domain/interface/rolename.enum.ts +++ b/apps/server/src/shared/domain/interface/rolename.enum.ts @@ -13,6 +13,8 @@ export enum RoleName { HELPDESK = 'helpdesk', ROOMVIEWER = 'roomviewer', ROOMEDITOR = 'roomeditor', + ROOMADMIN = 'roomadmin', + ROOMOWNER = 'roomowner', STUDENT = 'student', SUPERHERO = 'superhero', TEACHER = 'teacher', @@ -32,7 +34,12 @@ export type IUserRoleName = | RoleName.DEMOSTUDENT | RoleName.DEMOTEACHER; -export const RoomRoleArray = [RoleName.ROOMEDITOR, RoleName.ROOMVIEWER] as const; +export const RoomRoleArray = [ + RoleName.ROOMOWNER, + RoleName.ROOMADMIN, + RoleName.ROOMEDITOR, + RoleName.ROOMVIEWER, +] as const; export type RoomRole = typeof RoomRoleArray[number]; export const GuestRoleArray = [RoleName.GUESTSTUDENT, RoleName.GUESTTEACHER] as const; From 5f12a56e0b1cced33e7a83279030caa4260eadbb Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Wed, 11 Dec 2024 16:01:31 +0100 Subject: [PATCH 05/19] fix some tests --- .../service/room-membership.service.spec.ts | 2 +- .../api/test/room-add-members.api.spec.ts | 7 ++- .../room/api/test/room-create.api.spec.ts | 2 +- .../room/api/test/room-delete.api.spec.ts | 53 +++++++++++++++---- .../api/test/room-remove-members.api.spec.ts | 7 ++- .../tldraw.controller.401.api.spec.ts | 2 +- 6 files changed, 57 insertions(+), 16 deletions(-) diff --git a/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts b/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts index 98763e33358..a1cec042634 100644 --- a/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts +++ b/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts @@ -90,7 +90,7 @@ describe('RoomMembershipService', () => { it('should create new roomMembership when not exists', async () => { const { user, room } = setup(); - await service.addMembersToRoom(room.id, [{ userId: user.id, roleName: RoleName.ROOMEDITOR }]); + await service.addMembersToRoom(room.id, [{ userId: user.id, roleName: RoleName.ROOMA }]); expect(roomMembershipRepo.save).toHaveBeenCalled(); }); diff --git a/apps/server/src/modules/room/api/test/room-add-members.api.spec.ts b/apps/server/src/modules/room/api/test/room-add-members.api.spec.ts index ad8f5e6a3b7..ab1d9b64da4 100644 --- a/apps/server/src/modules/room/api/test/room-add-members.api.spec.ts +++ b/apps/server/src/modules/room/api/test/room-add-members.api.spec.ts @@ -55,7 +55,12 @@ describe('Room Controller (API)', () => { const studentGuestRole = roleFactory.buildWithId({ name: RoleName.GUESTSTUDENT }); const role = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR, - permissions: [Permission.ROOM_VIEW, Permission.ROOM_EDIT], + permissions: [ + Permission.ROOM_VIEW, + Permission.ROOM_EDIT, + Permission.ROOM_MEMBERS_ADD, + Permission.ROOM_MEMBERS_REMOVE, + ], }); // TODO: add more than one user const userGroupEntity = groupEntityFactory.buildWithId({ diff --git a/apps/server/src/modules/room/api/test/room-create.api.spec.ts b/apps/server/src/modules/room/api/test/room-create.api.spec.ts index eeca260725b..c0f2be3c2cd 100644 --- a/apps/server/src/modules/room/api/test/room-create.api.spec.ts +++ b/apps/server/src/modules/room/api/test/room-create.api.spec.ts @@ -70,7 +70,7 @@ describe('Room Controller (API)', () => { const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); const role = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR, - permissions: [Permission.ROOM_EDIT, Permission.ROOM_VIEW], + permissions: [Permission.ROOM_CREATE, Permission.ROOM_EDIT, Permission.ROOM_VIEW], }); await em.persistAndFlush([teacherAccount, teacherUser, role]); em.clear(); diff --git a/apps/server/src/modules/room/api/test/room-delete.api.spec.ts b/apps/server/src/modules/room/api/test/room-delete.api.spec.ts index 4e8be194dfe..9deef7a2ec5 100644 --- a/apps/server/src/modules/room/api/test/room-delete.api.spec.ts +++ b/apps/server/src/modules/room/api/test/room-delete.api.spec.ts @@ -96,32 +96,50 @@ describe('Room Controller (API)', () => { describe('when the user has the required permissions', () => { const setup = async () => { const room = roomEntityFactory.build(); - const role = roleFactory.buildWithId({ + const roomOwnerRole = roleFactory.buildWithId({ + name: RoleName.ROOMOWNER, + permissions: [Permission.ROOM_EDIT, Permission.ROOM_DELETE], + }); + const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR, permissions: [Permission.ROOM_EDIT], }); const school = schoolEntityFactory.buildWithId(); - const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({ school }); + const { teacherAccount: teacherOwnerAccount, teacherUser: teacherOwnerUser } = + UserAndAccountTestFactory.buildTeacher({ school }); + const { teacherAccount: teacherEditorAccount, teacherUser: teacherEditorUser } = + UserAndAccountTestFactory.buildTeacher({ school }); const userGroup = groupEntityFactory.buildWithId({ type: GroupEntityTypes.ROOM, - users: [{ role, user: teacherUser }], + users: [ + { role: roomOwnerRole, user: teacherOwnerUser }, + { role: roomEditorRole, user: teacherEditorUser }, + ], }); const roomMembership = roomMembershipEntityFactory.build({ roomId: room.id, userGroupId: userGroup.id, - schoolId: teacherUser.school.id, + schoolId: teacherOwnerUser.school.id, }); - await em.persistAndFlush([room, roomMembership, teacherAccount, teacherUser, userGroup, role]); + await em.persistAndFlush([ + room, + roomMembership, + teacherOwnerAccount, + teacherOwnerUser, + teacherEditorAccount, + teacherEditorUser, + userGroup, + roomOwnerRole, + ]); em.clear(); - const loggedInClient = await testApiClient.login(teacherAccount); - - return { loggedInClient, room }; + return { teacherOwnerAccount, teacherEditorAccount, room }; }; describe('when the room exists', () => { it('should delete the room', async () => { - const { loggedInClient, room } = await setup(); + const { teacherOwnerAccount, room } = await setup(); + const loggedInClient = await testApiClient.login(teacherOwnerAccount); const response = await loggedInClient.delete(room.id); expect(response.status).toBe(HttpStatus.NO_CONTENT); @@ -129,7 +147,8 @@ describe('Room Controller (API)', () => { }); it('should delete the roomMembership', async () => { - const { loggedInClient, room } = await setup(); + const { teacherOwnerAccount, room } = await setup(); + const loggedInClient = await testApiClient.login(teacherOwnerAccount); await expect(em.findOneOrFail(RoomMembershipEntity, { roomId: room.id })).resolves.not.toThrow(); @@ -137,11 +156,23 @@ describe('Room Controller (API)', () => { expect(response.status).toBe(HttpStatus.NO_CONTENT); await expect(em.findOneOrFail(RoomMembershipEntity, { roomId: room.id })).rejects.toThrow(NotFoundException); }); + + describe('when user is not the roomowner', () => { + it('should fail', async () => { + const { teacherEditorAccount, room } = await setup(); + const loggedInClient = await testApiClient.login(teacherEditorAccount); + + const response = await loggedInClient.delete(room.id); + + expect(response.status).toBe(HttpStatus.FORBIDDEN); + }); + }); }); describe('when the room does not exist', () => { it('should return a 404 error', async () => { - const { loggedInClient } = await setup(); + const { teacherOwnerAccount, room } = await setup(); + const loggedInClient = await testApiClient.login(teacherOwnerAccount); const someId = new ObjectId().toHexString(); const response = await loggedInClient.delete(someId); diff --git a/apps/server/src/modules/room/api/test/room-remove-members.api.spec.ts b/apps/server/src/modules/room/api/test/room-remove-members.api.spec.ts index 3810a9f4f39..f885a76b0ed 100644 --- a/apps/server/src/modules/room/api/test/room-remove-members.api.spec.ts +++ b/apps/server/src/modules/room/api/test/room-remove-members.api.spec.ts @@ -48,7 +48,12 @@ describe('Room Controller (API)', () => { const setupRoomRoles = () => { const editorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR, - permissions: [Permission.ROOM_VIEW, Permission.ROOM_EDIT], + permissions: [ + Permission.ROOM_VIEW, + Permission.ROOM_EDIT, + Permission.ROOM_MEMBERS_ADD, // for now room_editors have these two rights as room_admins are not yet available + Permission.ROOM_MEMBERS_REMOVE, + ], }); const viewerRole = roleFactory.buildWithId({ name: RoleName.ROOMVIEWER, diff --git a/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.401.api.spec.ts b/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.401.api.spec.ts index 88b24bce8b9..09d5d81cbb4 100644 --- a/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.401.api.spec.ts +++ b/apps/server/src/modules/tldraw/controller/api-test/tldraw.controller.401.api.spec.ts @@ -37,7 +37,7 @@ describe('tldraw controller (api)', () => { return { drawingItemData }; }; - it('should return status 204 for delete', async () => { + it('should return status 401 for delete', async () => { const { drawingItemData } = await setup(); const response = await testApiClient.delete(`${drawingItemData.docName}`); From e0f541fe9fd468e1a9bf29e1f756ecebc8cd8141 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Wed, 11 Dec 2024 16:49:33 +0100 Subject: [PATCH 06/19] fix tests --- .../service/room-membership.service.spec.ts | 20 ------------------- .../src/modules/room/api/room.uc.spec.ts | 2 +- .../room/api/test/room-create.api.spec.ts | 14 +++++++++++-- 3 files changed, 13 insertions(+), 23 deletions(-) diff --git a/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts b/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts index a1cec042634..249ae34ec06 100644 --- a/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts +++ b/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts @@ -87,26 +87,6 @@ describe('RoomMembershipService', () => { }; }; - it('should create new roomMembership when not exists', async () => { - const { user, room } = setup(); - - await service.addMembersToRoom(room.id, [{ userId: user.id, roleName: RoleName.ROOMA }]); - - expect(roomMembershipRepo.save).toHaveBeenCalled(); - }); - - it('should save the schoolId of the room in the roomMembership', async () => { - const { user, room } = setup(); - - await service.addMembersToRoom(room.id, [{ userId: user.id, roleName: RoleName.ROOMEDITOR }]); - - expect(roomMembershipRepo.save).toHaveBeenCalledWith( - expect.objectContaining({ - schoolId: room.schoolId, - }) - ); - }); - describe('when no user is provided', () => { it('should throw an exception', async () => { const { room } = setup(); diff --git a/apps/server/src/modules/room/api/room.uc.spec.ts b/apps/server/src/modules/room/api/room.uc.spec.ts index 8910130093e..95cd6f7f6cd 100644 --- a/apps/server/src/modules/room/api/room.uc.spec.ts +++ b/apps/server/src/modules/room/api/room.uc.spec.ts @@ -117,7 +117,7 @@ describe('RoomUc', () => { authorizationService.checkOneOfPermissions.mockReturnValue(undefined); const room = roomFactory.build(); roomService.createRoom.mockResolvedValue(room); - roomMembershipService.addMembersToRoom.mockRejectedValue(new Error('test')); + roomMembershipService.createNewRoomMembership.mockRejectedValue(new Error('test')); return { user, room }; }; diff --git a/apps/server/src/modules/room/api/test/room-create.api.spec.ts b/apps/server/src/modules/room/api/test/room-create.api.spec.ts index c0f2be3c2cd..47cecf68d32 100644 --- a/apps/server/src/modules/room/api/test/room-create.api.spec.ts +++ b/apps/server/src/modules/room/api/test/room-create.api.spec.ts @@ -69,10 +69,20 @@ describe('Room Controller (API)', () => { const setup = async () => { const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); const role = roleFactory.buildWithId({ - name: RoleName.ROOMEDITOR, + name: RoleName.TEACHER, permissions: [Permission.ROOM_CREATE, Permission.ROOM_EDIT, Permission.ROOM_VIEW], }); - await em.persistAndFlush([teacherAccount, teacherUser, role]); + const roomOwnerRole = roleFactory.buildWithId({ + name: RoleName.ROOMOWNER, + permissions: [ + Permission.ROOM_CREATE, + Permission.ROOM_EDIT, + Permission.ROOM_VIEW, + Permission.ROOM_MEMBERS_ADD, + Permission.ROOM_MEMBERS_REMOVE, + ], + }); + await em.persistAndFlush([teacherAccount, teacherUser, role, roomOwnerRole]); em.clear(); const loggedInClient = await testApiClient.login(teacherAccount); From 38107db58b4a3af2322142e46d3271affc585ec0 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Wed, 11 Dec 2024 16:49:44 +0100 Subject: [PATCH 07/19] update roles seed --- backup/setup/roles.json | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/backup/setup/roles.json b/backup/setup/roles.json index 81c1b5bc4af..833e9f68e1d 100644 --- a/backup/setup/roles.json +++ b/backup/setup/roles.json @@ -600,7 +600,8 @@ "permissions": [ "ROOM_VIEW", "ROOM_EDIT", - "ROOM_DELETE" + "ROOM_MEMBERS_ADD", + "ROOM_MEMBERS_REMOVE" ] }, { @@ -616,5 +617,31 @@ }, "name": "guestTeacher", "permissions": [] + }, + { + "_id": { + "$oid": "67599159f85fdde7abcda6be" + }, + "name": "roomowner", + "permissions": [ + "ROOM_VIEW", + "ROOM_EDIT", + "ROOM_DELETE", + "ROOM_MEMBERS_ADD", + "ROOM_MEMBERS_REMOVE", + "ROOM_CHANGE_OWNER" + ] + }, + { + "_id": { + "$oid": "67599159f85fdde7abcda6bf" + }, + "name": "roomadmin", + "permissions": [ + "ROOM_VIEW", + "ROOM_EDIT", + "ROOM_MEMBERS_ADD", + "ROOM_MEMBERS_REMOVE" + ] } ] From 71a8f522ea5892aa7c8bffb5d94929ff51c34b2f Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Wed, 11 Dec 2024 17:07:17 +0100 Subject: [PATCH 08/19] fix: test --- apps/server/src/modules/room/api/test/room-delete.api.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/server/src/modules/room/api/test/room-delete.api.spec.ts b/apps/server/src/modules/room/api/test/room-delete.api.spec.ts index 9deef7a2ec5..a088b76b872 100644 --- a/apps/server/src/modules/room/api/test/room-delete.api.spec.ts +++ b/apps/server/src/modules/room/api/test/room-delete.api.spec.ts @@ -171,7 +171,7 @@ describe('Room Controller (API)', () => { describe('when the room does not exist', () => { it('should return a 404 error', async () => { - const { teacherOwnerAccount, room } = await setup(); + const { teacherOwnerAccount } = await setup(); const loggedInClient = await testApiClient.login(teacherOwnerAccount); const someId = new ObjectId().toHexString(); From 0358d326346d2dff30126607fe6df88e5fd4ba1f Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Wed, 11 Dec 2024 17:47:29 +0100 Subject: [PATCH 09/19] chore: remove console.log --- apps/server/src/modules/room/api/room.uc.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/server/src/modules/room/api/room.uc.ts b/apps/server/src/modules/room/api/room.uc.ts index cc44a4c68e4..1de7fd11b5c 100644 --- a/apps/server/src/modules/room/api/room.uc.ts +++ b/apps/server/src/modules/room/api/room.uc.ts @@ -173,7 +173,6 @@ export class RoomUc { ): Promise { const roomMembershipAuthorizable = await this.roomMembershipService.getRoomMembershipAuthorizable(roomId); const user = await this.authorizationService.getUserWithPermissions(userId); - console.log('user.roles', user.roles); this.authorizationService.checkPermission(user, roomMembershipAuthorizable, { action, requiredPermissions }); return roomMembershipAuthorizable; From 1e141a9b1766699df94e2312c5fb11f286fba8d1 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Wed, 11 Dec 2024 18:06:56 +0100 Subject: [PATCH 10/19] chore: remove comment --- .../room-membership/service/room-membership.service.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/apps/server/src/modules/room-membership/service/room-membership.service.ts b/apps/server/src/modules/room-membership/service/room-membership.service.ts index 1aeb909ba67..4f7bd8889ee 100644 --- a/apps/server/src/modules/room-membership/service/room-membership.service.ts +++ b/apps/server/src/modules/room-membership/service/room-membership.service.ts @@ -101,11 +101,6 @@ export class RoomMembershipService { const group = await this.groupService.findById(roomMembership.userGroupId); - // TODO: fail if trying to remove owner - // const hasOwner = group.users - // .filter((user) => userIds.includes(user.userId)) - // .some((groupUser) => groupUser.roleName === RoleName.ROOMOWNER); - await this.groupService.removeUsersFromGroup(group.id, userIds); await this.handleGuestRoleRemoval(userIds, roomMembership.schoolId); From ba1b459f058396b08135742f33272ee1ea9aca8f Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Wed, 11 Dec 2024 18:14:33 +0100 Subject: [PATCH 11/19] update seed for roles-collation --- backup/setup/roles.json | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/backup/setup/roles.json b/backup/setup/roles.json index 833e9f68e1d..9e0288ae532 100644 --- a/backup/setup/roles.json +++ b/backup/setup/roles.json @@ -643,5 +643,31 @@ "ROOM_MEMBERS_ADD", "ROOM_MEMBERS_REMOVE" ] + }, + { + "_id": { + "$oid": "6759c7f97deafc47bbd8f808" + }, + "name": "roomowner", + "permissions": [ + "ROOM_VIEW", + "ROOM_EDIT", + "ROOM_DELETE", + "ROOM_MEMBERS_ADD", + "ROOM_MEMBERS_REMOVE", + "ROOM_CHANGE_OWNER" + ] + }, + { + "_id": { + "$oid": "6759c7f97deafc47bbd8f809" + }, + "name": "roomadmin", + "permissions": [ + "ROOM_VIEW", + "ROOM_EDIT", + "ROOM_MEMBERS_ADD", + "ROOM_MEMBERS_REMOVE" + ] } ] From d0b0fd28a0e07886c91d82bfea03df6e4c831b63 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 12 Dec 2024 11:03:42 +0100 Subject: [PATCH 12/19] fix: roles database seed file --- backup/setup/roles.json | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/backup/setup/roles.json b/backup/setup/roles.json index 9e0288ae532..16924e52e99 100644 --- a/backup/setup/roles.json +++ b/backup/setup/roles.json @@ -643,31 +643,5 @@ "ROOM_MEMBERS_ADD", "ROOM_MEMBERS_REMOVE" ] - }, - { - "_id": { - "$oid": "6759c7f97deafc47bbd8f808" - }, - "name": "roomowner", - "permissions": [ - "ROOM_VIEW", - "ROOM_EDIT", - "ROOM_DELETE", - "ROOM_MEMBERS_ADD", - "ROOM_MEMBERS_REMOVE", - "ROOM_CHANGE_OWNER" - ] - }, - { - "_id": { - "$oid": "6759c7f97deafc47bbd8f809" - }, - "name": "roomadmin", - "permissions": [ - "ROOM_VIEW", - "ROOM_EDIT", - "ROOM_MEMBERS_ADD", - "ROOM_MEMBERS_REMOVE" - ] } -] +] \ No newline at end of file From 6dbf63a585c32ed9bc69b8513958a77e9eaed3af Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 12 Dec 2024 11:43:11 +0100 Subject: [PATCH 13/19] fix: migrations --- backup/setup/migrations.json | 24 +++++++++++++++++++++--- backup/setup/roles.json | 6 +++--- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/backup/setup/migrations.json b/backup/setup/migrations.json index d99686e576e..9babec5e269 100644 --- a/backup/setup/migrations.json +++ b/backup/setup/migrations.json @@ -278,6 +278,15 @@ "$date": "2024-11-13T10:13:12.411Z" } }, + { + "_id": { + "$oid": "673fca34cc4a3264457c8ad1" + }, + "name": "Migration20241120100616", + "created_at": { + "$date": "2024-11-20T17:03:31.473Z" + } + }, { "_id": { "$oid": "674444262ba8186272dc8abd" @@ -298,11 +307,20 @@ }, { "_id": { - "$oid": "673fca34cc4a3264457c8ad1" + "$oid": "675abdb4e76b1142cd4c89e5" }, - "name": "Migration20241120100616", + "name": "Migration20241209165812", "created_at": { - "$date": "2024-11-20T17:03:31.473Z" + "$date": "2024-12-12T10:40:52.027Z" + } + }, + { + "_id": { + "$oid": "675abdb4e76b1142cd4c89e6" + }, + "name": "Migration20241210152600", + "created_at": { + "$date": "2024-12-12T10:40:52.029Z" } } ] diff --git a/backup/setup/roles.json b/backup/setup/roles.json index 16924e52e99..fab489dd12c 100644 --- a/backup/setup/roles.json +++ b/backup/setup/roles.json @@ -620,7 +620,7 @@ }, { "_id": { - "$oid": "67599159f85fdde7abcda6be" + "$oid": "675abdb4e76b1142cd4c89e3" }, "name": "roomowner", "permissions": [ @@ -634,7 +634,7 @@ }, { "_id": { - "$oid": "67599159f85fdde7abcda6bf" + "$oid": "675abdb4e76b1142cd4c89e4" }, "name": "roomadmin", "permissions": [ @@ -644,4 +644,4 @@ "ROOM_MEMBERS_REMOVE" ] } -] \ No newline at end of file +] From 0f3713eddc98c3fc8b3c97dc9895b86650a05061 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 12 Dec 2024 11:45:00 +0100 Subject: [PATCH 14/19] fix: migration messages --- .../src/migrations/mikro-orm/Migration20241210152600.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/server/src/migrations/mikro-orm/Migration20241210152600.ts b/apps/server/src/migrations/mikro-orm/Migration20241210152600.ts index 5f88d698b99..37933e2d0c1 100644 --- a/apps/server/src/migrations/mikro-orm/Migration20241210152600.ts +++ b/apps/server/src/migrations/mikro-orm/Migration20241210152600.ts @@ -13,7 +13,7 @@ export class Migration20241210152600 extends Migration { if (roomEditorRoleUpdate.modifiedCount > 0) { console.info( - 'Rollback: Permission ROOM_DELETE removed from and ROOM_MEMBERS_ADD and ROOM_MEMBERS_REMOVE added to role roomeditor.' + 'Permission ROOM_DELETE removed from and ROOM_MEMBERS_ADD and ROOM_MEMBERS_REMOVE added to role roomeditor.' ); } } @@ -30,7 +30,7 @@ export class Migration20241210152600 extends Migration { if (roomEditorRoleUpdate.modifiedCount > 0) { console.info( - 'Permissions ROOM_DELETE added to and ROOM_MEMBERS_ADD and ROOM_MEMBERS_REMOVE from role roomeditor.' + 'Rollback: Permissions ROOM_DELETE added to and ROOM_MEMBERS_ADD and ROOM_MEMBERS_REMOVE removed from role roomeditor.' ); } } From 42788ea0b0e366484fedbd6378b55bd720a652cc Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 12 Dec 2024 12:24:55 +0100 Subject: [PATCH 15/19] ensure that room owners can not be removed from a room --- .../service/room-membership.service.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/apps/server/src/modules/room-membership/service/room-membership.service.ts b/apps/server/src/modules/room-membership/service/room-membership.service.ts index 4f7bd8889ee..fd978721d04 100644 --- a/apps/server/src/modules/room-membership/service/room-membership.service.ts +++ b/apps/server/src/modules/room-membership/service/room-membership.service.ts @@ -101,6 +101,7 @@ export class RoomMembershipService { const group = await this.groupService.findById(roomMembership.userGroupId); + await this.ensureOwnerIsNotRemoved(group, userIds); await this.groupService.removeUsersFromGroup(group.id, userIds); await this.handleGuestRoleRemoval(userIds, roomMembership.schoolId); @@ -146,6 +147,17 @@ export class RoomMembershipService { return roomMembershipAuthorizable; } + private async ensureOwnerIsNotRemoved(group: Group, userIds: EntityId[]): Promise { + const role = await this.roleService.findByName(RoleName.ROOMOWNER); + const includedOwner = group.users + .filter((groupUser) => userIds.includes(groupUser.userId)) + .find((groupUser) => groupUser.roleId === role.id); + + if (includedOwner) { + throw new BadRequestException('Cannot remove owner from room'); + } + } + private async handleGuestRoleRemoval(userIds: EntityId[], schoolId: EntityId): Promise { const { data: groups } = await this.groupService.findGroups({ userIds, groupTypes: [GroupTypes.ROOM], schoolId }); From 6a74c751a3e3ca58607a9515f97713bbfd48d6eb Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 12 Dec 2024 12:29:45 +0100 Subject: [PATCH 16/19] fix tests --- .../service/room-membership.service.spec.ts | 179 +++++++++--------- 1 file changed, 94 insertions(+), 85 deletions(-) diff --git a/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts b/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts index 249ae34ec06..bfc45cf99dd 100644 --- a/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts +++ b/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts @@ -169,118 +169,127 @@ describe('RoomMembershipService', () => { }); describe('when roomMembership exists', () => { - const setup = () => { - const user = userFactory.buildWithId(); + const setupGroupAndRoom = (schoolId: string) => { const group = groupFactory.build({ type: GroupTypes.ROOM }); - const room = roomFactory.build(); - const roomMembership = roomMembershipFactory.build({ roomId: room.id, userGroupId: group.id }); - - roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); - groupService.findById.mockResolvedValue(group); - groupService.findGroups.mockResolvedValue({ total: 1, data: [group] }); + const room = roomFactory.build({ schoolId }); + const roomMembership = roomMembershipFactory.build({ + roomId: room.id, + userGroupId: group.id, + schoolId, + }); - return { - user, - room, - roomMembership, - group, - }; + return { group, room, roomMembership }; }; - it('should remove roomMembership', async () => { - const { user, room, group } = setup(); - - await service.removeMembersFromRoom(room.id, [user.id]); - - expect(groupService.removeUsersFromGroup).toHaveBeenCalledWith(group.id, [user.id]); - }); - }); + const mockGroupsAtSchoolAfterRemoval = (groups: Group[]) => { + groupService.findGroups.mockResolvedValue({ total: groups.length, data: groups }); + }; - const setupUserWithSecondarySchool = () => { - const secondarySchool = schoolFactory.build(); - const otherSchool = schoolFactory.build(); - const role = roleFactory.buildWithId({ name: RoleName.TEACHER }); - const guestTeacher = roleFactory.buildWithId({ name: RoleName.GUESTTEACHER }); - const externalUser = userDoFactory.buildWithId({ - roles: [role], - secondarySchools: [{ schoolId: secondarySchool.id, role: new RoleDto(guestTeacher) }], - }); + const setupUserWithSecondarySchool = () => { + const secondarySchool = schoolFactory.build(); + const otherSchool = schoolFactory.build(); + const role = roleFactory.buildWithId({ name: RoleName.TEACHER }); + const guestTeacher = roleFactory.buildWithId({ name: RoleName.GUESTTEACHER }); + const externalUser = userDoFactory.buildWithId({ + roles: [role], + secondarySchools: [{ schoolId: secondarySchool.id, role: new RoleDto(guestTeacher) }], + }); - return { secondarySchool, externalUser, otherSchool }; - }; + return { secondarySchool, externalUser, otherSchool }; + }; - const setupGroupAndRoom = (schoolId: string) => { - const group = groupFactory.build({ type: GroupTypes.ROOM }); - const room = roomFactory.build({ schoolId }); - const roomMembership = roomMembershipFactory.build({ - roomId: room.id, - userGroupId: group.id, - schoolId, - }); + it('should pass the schoolId of the room', async () => { + const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); - return { group, room, roomMembership }; - }; + const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); - const mockGroupsAtSchoolAfterRemoval = (groups: Group[]) => { - groupService.findGroups.mockResolvedValue({ total: groups.length, data: groups }); - }; + const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); + group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); - it('should pass the schoolId of the room', async () => { - const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); + roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); + groupService.findById.mockResolvedValue(group); + groupService.removeUsersFromGroup.mockResolvedValue(group); + mockGroupsAtSchoolAfterRemoval([]); - const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); + await service.removeMembersFromRoom(room.id, [externalUser.id as string]); - const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); - group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); + expect(groupService.findGroups).toHaveBeenCalledWith(expect.objectContaining({ schoolId: secondarySchool.id })); + }); - roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); - groupService.findById.mockResolvedValue(group); - groupService.removeUsersFromGroup.mockResolvedValue(group); - mockGroupsAtSchoolAfterRemoval([]); + it('should remove roomMembership', async () => { + const user = userFactory.buildWithId(); + const { room, group, roomMembership } = setupGroupAndRoom(user.school.id); + const roomOwnerRole = roleFactory.buildWithId({ name: RoleName.ROOMOWNER }); + groupService.findById.mockResolvedValue(group); + roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); + roleService.findByName.mockResolvedValue(roomOwnerRole); + mockGroupsAtSchoolAfterRemoval([group]); - await service.removeMembersFromRoom(room.id, [externalUser.id as string]); + await service.removeMembersFromRoom(room.id, [user.id]); - expect(groupService.findGroups).toHaveBeenCalledWith(expect.objectContaining({ schoolId: secondarySchool.id })); - }); + expect(groupService.removeUsersFromGroup).toHaveBeenCalledWith(group.id, [user.id]); + }); - describe('when after removal: user is not in any room of that secondary school', () => { - it('should remove user from secondary school', async () => { - const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); + describe('when after removal: user is not in any room of that secondary school', () => { + it('should remove user from secondary school', async () => { + const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); - const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); - const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); - group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); + const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); + const roomOwnerRole = roleFactory.buildWithId({ name: RoleName.ROOMOWNER }); + const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); + group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); - roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); - groupService.findById.mockResolvedValue(group); - groupService.removeUsersFromGroup.mockResolvedValue(group); - mockGroupsAtSchoolAfterRemoval([]); + roleService.findByName.mockResolvedValue(roomOwnerRole); + roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); + groupService.findById.mockResolvedValue(group); + groupService.removeUsersFromGroup.mockResolvedValue(group); + mockGroupsAtSchoolAfterRemoval([]); - await service.removeMembersFromRoom(room.id, [externalUser.id as string]); + await service.removeMembersFromRoom(room.id, [externalUser.id as string]); - expect(userService.removeSecondarySchoolFromUsers).toHaveBeenCalledWith([externalUser.id], secondarySchool.id); + expect(userService.removeSecondarySchoolFromUsers).toHaveBeenCalledWith( + [externalUser.id], + secondarySchool.id + ); + }); }); - }); - describe('when after removal: user is still in a room of that secondary school', () => { - it('should not remove user from secondary school', async () => { - const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); + describe('when after removal: user is still in a room of that secondary school', () => { + it('should not remove user from secondary school', async () => { + const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); - const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); + const roomOwnerRole = roleFactory.buildWithId({ name: RoleName.ROOMOWNER }); + const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); - const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); - group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); - const { group: group2 } = setupGroupAndRoom(secondarySchool.id); - group2.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); + const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); + group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); + const { group: group2 } = setupGroupAndRoom(secondarySchool.id); + group2.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); - roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); - groupService.findById.mockResolvedValue(group); - groupService.removeUsersFromGroup.mockResolvedValue(group); - mockGroupsAtSchoolAfterRemoval([group2]); + roleService.findByName.mockResolvedValue(roomOwnerRole); + roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); + groupService.findById.mockResolvedValue(group); + groupService.removeUsersFromGroup.mockResolvedValue(group); + mockGroupsAtSchoolAfterRemoval([group2]); - await service.removeMembersFromRoom(room.id, [externalUser.id as string]); + await service.removeMembersFromRoom(room.id, [externalUser.id as string]); - expect(userService.removeSecondarySchoolFromUsers).not.toHaveBeenCalled(); + expect(userService.removeSecondarySchoolFromUsers).not.toHaveBeenCalled(); + }); + }); + + describe('when owner should be removed', () => { + it('should throw a badrequest exception', async () => { + const user = userFactory.buildWithId(); + const { room, group, roomMembership } = setupGroupAndRoom(user.school.id); + roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); + const roomOwnerRole = roleFactory.buildWithId({ name: RoleName.ROOMOWNER }); + group.addUser({ userId: user.id, roleId: roomOwnerRole.id }); + groupService.findById.mockResolvedValue(group); + roleService.findByName.mockResolvedValue(roomOwnerRole); + + await expect(service.removeMembersFromRoom(room.id, [user.id])).rejects.toThrowError(BadRequestException); + }); }); }); }); From 07514ec221c096dae99d909697923cc1faf3ce74 Mon Sep 17 00:00:00 2001 From: "hoeppner.dataport" Date: Thu, 12 Dec 2024 13:06:02 +0100 Subject: [PATCH 17/19] fix api-test --- .../api/test/room-remove-members.api.spec.ts | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/apps/server/src/modules/room/api/test/room-remove-members.api.spec.ts b/apps/server/src/modules/room/api/test/room-remove-members.api.spec.ts index f885a76b0ed..0ecae7d548b 100644 --- a/apps/server/src/modules/room/api/test/room-remove-members.api.spec.ts +++ b/apps/server/src/modules/room/api/test/room-remove-members.api.spec.ts @@ -46,6 +46,16 @@ describe('Room Controller (API)', () => { describe('PATCH /rooms/:roomId/members/remove', () => { const setupRoomRoles = () => { + const ownerRole = roleFactory.buildWithId({ + name: RoleName.ROOMOWNER, + permissions: [ + Permission.ROOM_VIEW, + Permission.ROOM_EDIT, + Permission.ROOM_DELETE, + Permission.ROOM_MEMBERS_ADD, // for now room_editors have these two rights as room_admins are not yet available + Permission.ROOM_MEMBERS_REMOVE, + ], + }); const editorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR, permissions: [ @@ -59,7 +69,7 @@ describe('Room Controller (API)', () => { name: RoleName.ROOMVIEWER, permissions: [Permission.ROOM_VIEW], }); - return { editorRole, viewerRole }; + return { ownerRole, editorRole, viewerRole }; }; const setupRoomWithMembers = async () => { @@ -74,7 +84,7 @@ describe('Room Controller (API)', () => { const users = { teacherUser, inRoomEditor2, inRoomEditor3, inRoomViewer, outTeacher }; - const { editorRole, viewerRole } = setupRoomRoles(); + const { ownerRole, editorRole, viewerRole } = setupRoomRoles(); const roomUsers = [teacherUser, inRoomEditor2, inRoomEditor3].map((user) => { return { role: editorRole, user }; @@ -94,7 +104,14 @@ describe('Room Controller (API)', () => { schoolId: school.id, }); - await em.persistAndFlush([...Object.values(users), room, roomMemberships, teacherAccount, userGroupEntity]); + await em.persistAndFlush([ + ...Object.values(users), + room, + roomMemberships, + teacherAccount, + userGroupEntity, + ownerRole, + ]); em.clear(); const loggedInClient = await testApiClient.login(teacherAccount); From 89bd3b90011104cd38dc2fc0ad0d50a7fc83eb8c Mon Sep 17 00:00:00 2001 From: Thomas Feldtkeller Date: Fri, 13 Dec 2024 10:36:24 +0100 Subject: [PATCH 18/19] refactor tests --- .../service/room-membership.service.spec.ts | 139 ++++++++++-------- 1 file changed, 80 insertions(+), 59 deletions(-) diff --git a/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts b/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts index bfc45cf99dd..c6249661dc4 100644 --- a/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts +++ b/apps/server/src/modules/room-membership/service/room-membership.service.spec.ts @@ -178,6 +178,9 @@ describe('RoomMembershipService', () => { schoolId, }); + groupService.findById.mockResolvedValue(group); + roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); + return { group, room, roomMembership }; }; @@ -185,6 +188,14 @@ describe('RoomMembershipService', () => { groupService.findGroups.mockResolvedValue({ total: groups.length, data: groups }); }; + const setupRoomRoles = () => { + const roomOwnerRole = roleFactory.buildWithId({ name: RoleName.ROOMOWNER }); + const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); + roleService.findByName.mockResolvedValue(roomOwnerRole); + + return { roomOwnerRole, roomEditorRole }; + }; + const setupUserWithSecondarySchool = () => { const secondarySchool = schoolFactory.build(); const otherSchool = schoolFactory.build(); @@ -194,99 +205,109 @@ describe('RoomMembershipService', () => { roles: [role], secondarySchools: [{ schoolId: secondarySchool.id, role: new RoleDto(guestTeacher) }], }); + const externalUserId = externalUser.id as string; - return { secondarySchool, externalUser, otherSchool }; + return { secondarySchool, externalUser, externalUserId, otherSchool }; }; - it('should pass the schoolId of the room', async () => { - const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); - - const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); + describe('when removing user from a different school, with no further groups on host school', () => { + const setup = () => { + const { secondarySchool, externalUserId } = setupUserWithSecondarySchool(); + const { roomEditorRole } = setupRoomRoles(); - const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); - group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); + const { room, group } = setupGroupAndRoom(secondarySchool.id); + group.addUser({ userId: externalUserId, roleId: roomEditorRole.id }); - roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); - groupService.findById.mockResolvedValue(group); - groupService.removeUsersFromGroup.mockResolvedValue(group); - mockGroupsAtSchoolAfterRemoval([]); + mockGroupsAtSchoolAfterRemoval([]); - await service.removeMembersFromRoom(room.id, [externalUser.id as string]); + return { secondarySchool, externalUserId, room, group }; + }; - expect(groupService.findGroups).toHaveBeenCalledWith(expect.objectContaining({ schoolId: secondarySchool.id })); - }); + it('should pass the schoolId of the room', async () => { + const { secondarySchool, externalUserId, room } = setup(); - it('should remove roomMembership', async () => { - const user = userFactory.buildWithId(); - const { room, group, roomMembership } = setupGroupAndRoom(user.school.id); - const roomOwnerRole = roleFactory.buildWithId({ name: RoleName.ROOMOWNER }); - groupService.findById.mockResolvedValue(group); - roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); - roleService.findByName.mockResolvedValue(roomOwnerRole); - mockGroupsAtSchoolAfterRemoval([group]); + await service.removeMembersFromRoom(room.id, [externalUserId]); - await service.removeMembersFromRoom(room.id, [user.id]); + expect(groupService.findGroups).toHaveBeenCalledWith( + expect.objectContaining({ schoolId: secondarySchool.id }) + ); + }); - expect(groupService.removeUsersFromGroup).toHaveBeenCalledWith(group.id, [user.id]); - }); + it('should remove user from room', async () => { + const { group, externalUserId, room } = setup(); - describe('when after removal: user is not in any room of that secondary school', () => { - it('should remove user from secondary school', async () => { - const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); + await service.removeMembersFromRoom(room.id, [externalUserId]); - const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); - const roomOwnerRole = roleFactory.buildWithId({ name: RoleName.ROOMOWNER }); - const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); - group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); + expect(groupService.removeUsersFromGroup).toHaveBeenCalledWith(group.id, [externalUserId]); + }); - roleService.findByName.mockResolvedValue(roomOwnerRole); - roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); - groupService.findById.mockResolvedValue(group); - groupService.removeUsersFromGroup.mockResolvedValue(group); - mockGroupsAtSchoolAfterRemoval([]); + it('should remove user from secondary school', async () => { + const { secondarySchool, externalUserId, room } = setup(); - await service.removeMembersFromRoom(room.id, [externalUser.id as string]); + await service.removeMembersFromRoom(room.id, [externalUserId]); - expect(userService.removeSecondarySchoolFromUsers).toHaveBeenCalledWith( - [externalUser.id], - secondarySchool.id - ); + expect(userService.removeSecondarySchoolFromUsers).toHaveBeenCalledWith([externalUserId], secondarySchool.id); }); }); - describe('when after removal: user is still in a room of that secondary school', () => { - it('should not remove user from secondary school', async () => { + describe('when removing user from a different school, with further groups on host school', () => { + const setup = () => { const { secondarySchool, externalUser } = setupUserWithSecondarySchool(); + const { roomEditorRole } = setupRoomRoles(); - const roomOwnerRole = roleFactory.buildWithId({ name: RoleName.ROOMOWNER }); - const roomEditorRole = roleFactory.buildWithId({ name: RoleName.ROOMEDITOR }); - - const { room, group, roomMembership } = setupGroupAndRoom(secondarySchool.id); + const { room, group } = setupGroupAndRoom(secondarySchool.id); group.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); const { group: group2 } = setupGroupAndRoom(secondarySchool.id); group2.addUser({ userId: externalUser.id as string, roleId: roomEditorRole.id }); - roleService.findByName.mockResolvedValue(roomOwnerRole); - roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); - groupService.findById.mockResolvedValue(group); - groupService.removeUsersFromGroup.mockResolvedValue(group); mockGroupsAtSchoolAfterRemoval([group2]); + return { externalUser, room }; + }; + + it('should not remove user from secondary school', async () => { + const { externalUser, room } = setup(); + await service.removeMembersFromRoom(room.id, [externalUser.id as string]); expect(userService.removeSecondarySchoolFromUsers).not.toHaveBeenCalled(); }); }); - describe('when owner should be removed', () => { - it('should throw a badrequest exception', async () => { + describe('when removing user from the same school', () => { + const setup = () => { const user = userFactory.buildWithId(); - const { room, group, roomMembership } = setupGroupAndRoom(user.school.id); - roomMembershipRepo.findByRoomId.mockResolvedValue(roomMembership); - const roomOwnerRole = roleFactory.buildWithId({ name: RoleName.ROOMOWNER }); + const { roomEditorRole } = setupRoomRoles(); + const { room, group } = setupGroupAndRoom(user.school.id); + group.addUser({ userId: user.id, roleId: roomEditorRole.id }); + + mockGroupsAtSchoolAfterRemoval([group]); + + return { user, room, group }; + }; + + it('should remove user from room', async () => { + const { user, group, room } = setup(); + + await service.removeMembersFromRoom(room.id, [user.id]); + + expect(groupService.removeUsersFromGroup).toHaveBeenCalledWith(group.id, [user.id]); + }); + }); + + describe('when removing the owner of the room', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const { room, group } = setupGroupAndRoom(user.school.id); + const { roomOwnerRole } = setupRoomRoles(); + group.addUser({ userId: user.id, roleId: roomOwnerRole.id }); - groupService.findById.mockResolvedValue(group); - roleService.findByName.mockResolvedValue(roomOwnerRole); + + return { user, room }; + }; + + it('should throw a badrequest exception', async () => { + const { user, room } = setup(); await expect(service.removeMembersFromRoom(room.id, [user.id])).rejects.toThrowError(BadRequestException); }); From 9868323a83ce415584740abe8aa8f916896eeea8 Mon Sep 17 00:00:00 2001 From: Thomas Feldtkeller Date: Fri, 13 Dec 2024 15:10:20 +0100 Subject: [PATCH 19/19] take admin rights away from editors --- .../mikro-orm/Migration20241210152600.ts | 6 ++-- .../api/test/room-add-members.api.spec.ts | 7 ++++- .../api/test/room-remove-members.api.spec.ts | 30 +++++++++---------- backup/setup/roles.json | 4 +-- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/apps/server/src/migrations/mikro-orm/Migration20241210152600.ts b/apps/server/src/migrations/mikro-orm/Migration20241210152600.ts index 37933e2d0c1..4bd331b5057 100644 --- a/apps/server/src/migrations/mikro-orm/Migration20241210152600.ts +++ b/apps/server/src/migrations/mikro-orm/Migration20241210152600.ts @@ -6,15 +6,13 @@ export class Migration20241210152600 extends Migration { { name: 'roomeditor' }, { $set: { - permissions: ['ROOM_VIEW', 'ROOM_EDIT', 'ROOM_MEMBERS_ADD', 'ROOM_MEMBERS_REMOVE'], + permissions: ['ROOM_VIEW', 'ROOM_EDIT'], }, } ); if (roomEditorRoleUpdate.modifiedCount > 0) { - console.info( - 'Permission ROOM_DELETE removed from and ROOM_MEMBERS_ADD and ROOM_MEMBERS_REMOVE added to role roomeditor.' - ); + console.info('Permission ROOM_DELETE removed from role roomeditor.'); } } diff --git a/apps/server/src/modules/room/api/test/room-add-members.api.spec.ts b/apps/server/src/modules/room/api/test/room-add-members.api.spec.ts index ab1d9b64da4..d4d5761ad5f 100644 --- a/apps/server/src/modules/room/api/test/room-add-members.api.spec.ts +++ b/apps/server/src/modules/room/api/test/room-add-members.api.spec.ts @@ -54,7 +54,7 @@ describe('Room Controller (API)', () => { const teacherGuestRole = roleFactory.buildWithId({ name: RoleName.GUESTTEACHER }); const studentGuestRole = roleFactory.buildWithId({ name: RoleName.GUESTSTUDENT }); const role = roleFactory.buildWithId({ - name: RoleName.ROOMEDITOR, + name: RoleName.ROOMADMIN, permissions: [ Permission.ROOM_VIEW, Permission.ROOM_EDIT, @@ -62,6 +62,10 @@ describe('Room Controller (API)', () => { Permission.ROOM_MEMBERS_REMOVE, ], }); + const roomEditorRole = roleFactory.buildWithId({ + name: RoleName.ROOMEDITOR, + permissions: [Permission.ROOM_VIEW, Permission.ROOM_EDIT], + }); // TODO: add more than one user const userGroupEntity = groupEntityFactory.buildWithId({ users: [{ role, user: teacherUser }], @@ -82,6 +86,7 @@ describe('Room Controller (API)', () => { teacherUser, teacherGuestRole, studentGuestRole, + roomEditorRole, otherTeacherUser, otherTeacherAccount, userGroupEntity, diff --git a/apps/server/src/modules/room/api/test/room-remove-members.api.spec.ts b/apps/server/src/modules/room/api/test/room-remove-members.api.spec.ts index 0ecae7d548b..f52dfc0bf2d 100644 --- a/apps/server/src/modules/room/api/test/room-remove-members.api.spec.ts +++ b/apps/server/src/modules/room/api/test/room-remove-members.api.spec.ts @@ -52,16 +52,16 @@ describe('Room Controller (API)', () => { Permission.ROOM_VIEW, Permission.ROOM_EDIT, Permission.ROOM_DELETE, - Permission.ROOM_MEMBERS_ADD, // for now room_editors have these two rights as room_admins are not yet available + Permission.ROOM_MEMBERS_ADD, Permission.ROOM_MEMBERS_REMOVE, ], }); - const editorRole = roleFactory.buildWithId({ - name: RoleName.ROOMEDITOR, + const adminRole = roleFactory.buildWithId({ + name: RoleName.ROOMADMIN, permissions: [ Permission.ROOM_VIEW, Permission.ROOM_EDIT, - Permission.ROOM_MEMBERS_ADD, // for now room_editors have these two rights as room_admins are not yet available + Permission.ROOM_MEMBERS_ADD, Permission.ROOM_MEMBERS_REMOVE, ], }); @@ -69,7 +69,7 @@ describe('Room Controller (API)', () => { name: RoleName.ROOMVIEWER, permissions: [Permission.ROOM_VIEW], }); - return { ownerRole, editorRole, viewerRole }; + return { ownerRole, adminRole, viewerRole }; }; const setupRoomWithMembers = async () => { @@ -77,17 +77,17 @@ describe('Room Controller (API)', () => { const room = roomEntityFactory.buildWithId({ schoolId: school.id }); const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({ school }); - const { teacherUser: inRoomEditor2 } = UserAndAccountTestFactory.buildTeacher({ school: teacherUser.school }); - const { teacherUser: inRoomEditor3 } = UserAndAccountTestFactory.buildTeacher({ school: teacherUser.school }); + const { teacherUser: inRoomAdmin2 } = UserAndAccountTestFactory.buildTeacher({ school: teacherUser.school }); + const { teacherUser: inRoomAdmin3 } = UserAndAccountTestFactory.buildTeacher({ school: teacherUser.school }); const { teacherUser: inRoomViewer } = UserAndAccountTestFactory.buildTeacher({ school: teacherUser.school }); const { teacherUser: outTeacher } = UserAndAccountTestFactory.buildTeacher({ school: teacherUser.school }); - const users = { teacherUser, inRoomEditor2, inRoomEditor3, inRoomViewer, outTeacher }; + const users = { teacherUser, inRoomAdmin2, inRoomAdmin3, inRoomViewer, outTeacher }; - const { ownerRole, editorRole, viewerRole } = setupRoomRoles(); + const { ownerRole, adminRole, viewerRole } = setupRoomRoles(); - const roomUsers = [teacherUser, inRoomEditor2, inRoomEditor3].map((user) => { - return { role: editorRole, user }; + const roomUsers = [teacherUser, inRoomAdmin2, inRoomAdmin3].map((user) => { + return { role: adminRole, user }; }); roomUsers.push({ role: viewerRole, user: inRoomViewer }); @@ -159,9 +159,9 @@ describe('Room Controller (API)', () => { describe('when the user has the required permissions', () => { describe('when removing a user from the room', () => { it('should return OK', async () => { - const { loggedInClient, room, inRoomEditor2 } = await setupRoomWithMembers(); + const { loggedInClient, room, inRoomAdmin2 } = await setupRoomWithMembers(); - const userIds = [inRoomEditor2.id]; + const userIds = [inRoomAdmin2.id]; const response = await loggedInClient.patch(`/${room.id}/members/remove`, { userIds }); expect(response.status).toBe(HttpStatus.OK); @@ -170,9 +170,9 @@ describe('Room Controller (API)', () => { describe('when removing several users from the room', () => { it('should return OK', async () => { - const { loggedInClient, room, inRoomEditor2, inRoomEditor3 } = await setupRoomWithMembers(); + const { loggedInClient, room, inRoomAdmin2, inRoomAdmin3 } = await setupRoomWithMembers(); - const userIds = [inRoomEditor2.id, inRoomEditor3.id]; + const userIds = [inRoomAdmin2.id, inRoomAdmin3.id]; const response = await loggedInClient.patch(`/${room.id}/members/remove`, { userIds }); expect(response.status).toBe(HttpStatus.OK); diff --git a/backup/setup/roles.json b/backup/setup/roles.json index fab489dd12c..0c494cb441f 100644 --- a/backup/setup/roles.json +++ b/backup/setup/roles.json @@ -599,9 +599,7 @@ "name": "roomeditor", "permissions": [ "ROOM_VIEW", - "ROOM_EDIT", - "ROOM_MEMBERS_ADD", - "ROOM_MEMBERS_REMOVE" + "ROOM_EDIT" ] }, {