From 39aa416450b0425272413cbbd3b88421fb7e1eaf Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Wed, 9 Oct 2024 16:28:41 +0200 Subject: [PATCH 1/5] update course sync --- .../service/course-do.service.spec.ts | 51 ++++++++++++++++--- .../learnroom/service/course-do.service.ts | 19 ++++++- .../learnroom/uc/course-sync.uc.spec.ts | 36 +++++++++++-- .../modules/learnroom/uc/course-sync.uc.ts | 27 +++++++--- 4 files changed, 111 insertions(+), 22 deletions(-) diff --git a/apps/server/src/modules/learnroom/service/course-do.service.spec.ts b/apps/server/src/modules/learnroom/service/course-do.service.spec.ts index a6fae948299..9f9591c5c19 100644 --- a/apps/server/src/modules/learnroom/service/course-do.service.spec.ts +++ b/apps/server/src/modules/learnroom/service/course-do.service.spec.ts @@ -1,6 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; -import { Group } from '@modules/group'; +import { Group, GroupUser } from '@modules/group'; import { Test, TestingModule } from '@nestjs/testing'; import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { Page } from '@shared/domain/domainobject'; @@ -185,43 +185,78 @@ describe(CourseDoService.name, () => { const setup = () => { const course: Course = courseFactory.build(); const group: Group = groupFactory.build(); + const students: GroupUser[] = [{ roleId: 'student-role-id', userId: 'student-user-id' }]; + const teachers: GroupUser[] = [{ roleId: 'teacher-role-id', userId: 'teacher-user-id' }]; return { course, group, + students, + teachers, }; }; - it('should save a course with a synchronized group', async () => { - const { course, group } = setup(); + it('should save a course with synchronized group, students, and teachers', async () => { + const { course, group, students, teachers } = setup(); - await service.startSynchronization(course, group); + await service.startSynchronization(course, group, students, teachers); expect(courseRepo.save).toHaveBeenCalledWith( new Course({ ...course.getProps(), syncedWithGroup: group.id, + name: group.name, + startDate: group.validPeriod?.from, + untilDate: group.validPeriod?.until, + studentIds: students.map((student) => student.userId), + teacherIds: teachers.map((teacher) => teacher.userId), + }) + ); + }); + + it('should set an empty list of students if no teachers are present', async () => { + const { course, group, students } = setup(); + const teachers: GroupUser[] = []; // No teachers + + await service.startSynchronization(course, group, students, teachers); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + syncedWithGroup: group.id, + name: group.name, + startDate: group.validPeriod?.from, + untilDate: group.validPeriod?.until, + studentIds: [], + teacherIds: [], }) ); }); }); - describe('when a course is synchronized with a group', () => { + describe('when a course is already synchronized with a group', () => { const setup = () => { const course: Course = courseFactory.build({ syncedWithGroup: new ObjectId().toHexString() }); const group: Group = groupFactory.build(); + const students: GroupUser[] = [{ roleId: 'student-role-id', userId: 'student-user-id' }]; + const teachers: GroupUser[] = [{ roleId: 'teacher-role-id', userId: 'teacher-user-id' }]; return { course, group, + students, + teachers, }; }; - it('should throw an unprocessable entity exception', async () => { - const { course, group } = setup(); - await expect(service.startSynchronization(course, group)).rejects.toThrow( + it('should throw a CourseAlreadySynchronizedLoggableException', async () => { + const { course, group, students, teachers } = setup(); + + await expect(service.startSynchronization(course, group, students, teachers)).rejects.toThrow( CourseAlreadySynchronizedLoggableException ); + + expect(courseRepo.save).not.toHaveBeenCalled(); }); }); }); diff --git a/apps/server/src/modules/learnroom/service/course-do.service.ts b/apps/server/src/modules/learnroom/service/course-do.service.ts index 951693dee25..c5590db6bee 100644 --- a/apps/server/src/modules/learnroom/service/course-do.service.ts +++ b/apps/server/src/modules/learnroom/service/course-do.service.ts @@ -1,5 +1,5 @@ import { AuthorizationLoaderServiceGeneric } from '@modules/authorization'; -import { type Group } from '@modules/group'; +import { GroupUser, type Group } from '@modules/group'; import { Inject, Injectable } from '@nestjs/common'; import { Page } from '@shared/domain/domainobject'; import { IFindOptions } from '@shared/domain/interface'; @@ -45,12 +45,27 @@ export class CourseDoService implements AuthorizationLoaderServiceGeneric { + public async startSynchronization( + course: Course, + group: Group, + students: GroupUser[], + teachers: GroupUser[] + ): Promise { if (course.syncedWithGroup) { throw new CourseAlreadySynchronizedLoggableException(course.id); } course.syncedWithGroup = group.id; + course.name = group.name; + course.startDate = group.validPeriod?.from; + course.untilDate = group.validPeriod?.until; + + if (teachers.length >= 1) { + course.students = students.map((groupUser: GroupUser): EntityId => groupUser.userId); + course.teachers = teachers.map((groupUser: GroupUser): EntityId => groupUser.userId); + } else { + course.students = []; + } await this.courseRepo.save(course); } diff --git a/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts b/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts index 60366ab3065..ad386434cd5 100644 --- a/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts +++ b/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts @@ -1,9 +1,10 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; import { GroupService } from '@modules/group'; +import { RoleService } from '@modules/role'; import { Test, TestingModule } from '@nestjs/testing'; -import { Permission } from '@shared/domain/interface'; -import { groupFactory, setupEntities, userFactory } from '@shared/testing'; +import { Permission, RoleName } from '@shared/domain/interface'; +import { groupFactory, roleDtoFactory, setupEntities, userFactory } from '@shared/testing'; import { CourseDoService } from '../service'; import { courseFactory } from '../testing'; import { CourseSyncUc } from './course-sync.uc'; @@ -15,6 +16,7 @@ describe(CourseSyncUc.name, () => { let authorizationService: DeepMocked; let courseService: DeepMocked; let groupService: DeepMocked; + let roleService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -32,6 +34,10 @@ describe(CourseSyncUc.name, () => { provide: GroupService, useValue: createMock(), }, + { + provide: RoleService, + useValue: createMock(), + }, ], }).compile(); @@ -39,6 +45,7 @@ describe(CourseSyncUc.name, () => { authorizationService = module.get(AuthorizationService); courseService = module.get(CourseDoService); groupService = module.get(GroupService); + roleService = module.get(RoleService); await setupEntities(); }); @@ -93,15 +100,30 @@ describe(CourseSyncUc.name, () => { const user = userFactory.buildWithId(); const course = courseFactory.build(); const group = groupFactory.build(); + const studentRole = roleDtoFactory.build({ id: 'student-role-id' }); + const teacherRole = roleDtoFactory.build({ id: 'teacher-role-id' }); + + group.users = [ + { roleId: 'student-role-id', userId: 'student-user-id' }, + { roleId: 'teacher-role-id', userId: 'teacher-user-id' }, + ]; + + const students = group.users.filter((groupUser) => groupUser.roleId === studentRole.id); + const teachers = group.users.filter((groupUser) => groupUser.roleId === teacherRole.id); courseService.findById.mockResolvedValueOnce(course); groupService.findById.mockResolvedValueOnce(group); authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + roleService.findByName.mockResolvedValueOnce(studentRole).mockResolvedValueOnce(teacherRole); return { user, course, group, + studentRole, + teacherRole, + students, + teachers, }; }; @@ -119,14 +141,18 @@ describe(CourseSyncUc.name, () => { ); }); - it('should start the synchronization', async () => { - const { user, course, group } = setup(); + it('should start the synchronization with correct roles', async () => { + const { user, course, group, students, teachers } = setup(); await uc.startSynchronization(user.id, course.id, group.id); + expect(courseService.findById).toHaveBeenCalledWith(course.id); expect(groupService.findById).toHaveBeenCalledWith(group.id); - expect(courseService.startSynchronization).toHaveBeenCalledWith(course, group); + expect(roleService.findByName).toHaveBeenCalledWith(RoleName.STUDENT); + expect(roleService.findByName).toHaveBeenCalledWith(RoleName.TEACHER); + + expect(courseService.startSynchronization).toHaveBeenCalledWith(course, group, students, teachers); }); }); }); diff --git a/apps/server/src/modules/learnroom/uc/course-sync.uc.ts b/apps/server/src/modules/learnroom/uc/course-sync.uc.ts index 53a4fe2e173..770268fe882 100644 --- a/apps/server/src/modules/learnroom/uc/course-sync.uc.ts +++ b/apps/server/src/modules/learnroom/uc/course-sync.uc.ts @@ -1,8 +1,10 @@ import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; -import { Group, GroupService } from '@modules/group'; +import { GroupService, GroupUser } from '@modules/group'; +import { RoleService } from '@modules/role'; import { Injectable } from '@nestjs/common'; + import { type User as UserEntity } from '@shared/domain/entity'; -import { Permission } from '@shared/domain/interface'; +import { Permission, RoleName } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { Course } from '../domain'; import { CourseDoService } from '../service'; @@ -12,7 +14,8 @@ export class CourseSyncUc { constructor( private readonly authorizationService: AuthorizationService, private readonly courseService: CourseDoService, - private readonly groupService: GroupService + private readonly groupService: GroupService, + private readonly roleService: RoleService ) {} public async stopSynchronization(userId: EntityId, courseId: EntityId): Promise { @@ -29,9 +32,11 @@ export class CourseSyncUc { } public async startSynchronization(userId: string, courseId: string, groupId: string) { - const course: Course = await this.courseService.findById(courseId); - const group: Group = await this.groupService.findById(groupId); - const user: UserEntity = await this.authorizationService.getUserWithPermissions(userId); + const [course, group, user] = await Promise.all([ + this.courseService.findById(courseId), + this.groupService.findById(groupId), + this.authorizationService.getUserWithPermissions(userId), + ]); this.authorizationService.checkPermission( user, @@ -39,6 +44,14 @@ export class CourseSyncUc { AuthorizationContextBuilder.write([Permission.COURSE_EDIT]) ); - await this.courseService.startSynchronization(course, group); + const [studentRole, teacherRole] = await Promise.all([ + this.roleService.findByName(RoleName.STUDENT), + this.roleService.findByName(RoleName.TEACHER), + ]); + + const students = group.users.filter((groupUser: GroupUser) => groupUser.roleId === studentRole.id); + const teachers = group.users.filter((groupUser: GroupUser) => groupUser.roleId === teacherRole.id); + + await this.courseService.startSynchronization(course, group, students, teachers); } } From 691acabf21022ac9448f2ad00d09e287f7a0cbe4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= <103562092+MarvinOehlerkingCap@users.noreply.github.com> Date: Thu, 10 Oct 2024 10:26:39 +0200 Subject: [PATCH 2/5] N21-2200 Move logout to nest (#5278) --- apps/server/src/imports-from-feathers.ts | 8 ++ .../authentication-api.module.ts | 9 +- .../controllers/api-test/logout.api.spec.ts | 72 +++++++++++++++ .../authentication/controllers/index.ts | 2 + .../controllers/logout.controller.ts | 20 +++++ .../helper/jwt-whitelist.adapter.spec.ts | 90 +++++++++---------- .../helper/jwt-whitelist.adapter.ts | 24 +++-- .../src/modules/authentication/uc/index.ts | 1 + .../authentication/uc/logout.uc.spec.ts | 47 ++++++++++ .../modules/authentication/uc/logout.uc.ts | 11 +++ .../testing/factory/jwt.test.factory.ts | 3 +- 11 files changed, 222 insertions(+), 65 deletions(-) create mode 100644 apps/server/src/modules/authentication/controllers/api-test/logout.api.spec.ts create mode 100644 apps/server/src/modules/authentication/controllers/index.ts create mode 100644 apps/server/src/modules/authentication/controllers/logout.controller.ts create mode 100644 apps/server/src/modules/authentication/uc/logout.uc.spec.ts create mode 100644 apps/server/src/modules/authentication/uc/logout.uc.ts diff --git a/apps/server/src/imports-from-feathers.ts b/apps/server/src/imports-from-feathers.ts index 0804901a53b..57cf4ad7ea8 100644 --- a/apps/server/src/imports-from-feathers.ts +++ b/apps/server/src/imports-from-feathers.ts @@ -4,5 +4,13 @@ export { addTokenToWhitelist, createRedisIdentifierFromJwtData, ensureTokenIsWhitelisted, + getRedisData, } from '../../../src/services/authentication/logic/whitelist.js'; export * as feathersRedis from '../../../src/utils/redis.js'; +export type JwtRedisData = { + IP: string; + Browser: string; + Device: string; + privateDevice: boolean; + expirationInSeconds: number; +}; diff --git a/apps/server/src/modules/authentication/authentication-api.module.ts b/apps/server/src/modules/authentication/authentication-api.module.ts index 6780e5fe11c..07780ddbe4c 100644 --- a/apps/server/src/modules/authentication/authentication-api.module.ts +++ b/apps/server/src/modules/authentication/authentication-api.module.ts @@ -1,12 +1,11 @@ import { Module } from '@nestjs/common'; import { AuthenticationModule } from './authentication.module'; -import { LoginController } from './controllers/login.controller'; -import { LoginUc } from './uc/login.uc'; +import { LoginController, LogoutController } from './controllers'; +import { LoginUc, LogoutUc } from './uc'; @Module({ imports: [AuthenticationModule], - providers: [LoginUc], - controllers: [LoginController], - exports: [], + providers: [LoginUc, LogoutUc], + controllers: [LoginController, LogoutController], }) export class AuthenticationApiModule {} diff --git a/apps/server/src/modules/authentication/controllers/api-test/logout.api.spec.ts b/apps/server/src/modules/authentication/controllers/api-test/logout.api.spec.ts new file mode 100644 index 00000000000..0ea7fd6bd2b --- /dev/null +++ b/apps/server/src/modules/authentication/controllers/api-test/logout.api.spec.ts @@ -0,0 +1,72 @@ +import { EntityManager } from '@mikro-orm/mongodb'; +import { ServerTestModule } from '@modules/server/server.module'; +import { CACHE_MANAGER } from '@nestjs/cache-manager'; +import { HttpStatus, INestApplication } from '@nestjs/common'; +import { Test, TestingModule } from '@nestjs/testing'; +import { cleanupCollections, TestApiClient, UserAndAccountTestFactory } from '@shared/testing'; +import { Cache } from 'cache-manager'; +import { Response } from 'supertest'; + +describe('Logout Controller (api)', () => { + const baseRouteName = '/logout'; + + let app: INestApplication; + let em: EntityManager; + let cacheManager: Cache; + let testApiClient: TestApiClient; + + beforeAll(async () => { + const moduleFixture: TestingModule = await Test.createTestingModule({ + imports: [ServerTestModule], + }).compile(); + + app = moduleFixture.createNestApplication(); + await app.init(); + em = app.get(EntityManager); + cacheManager = app.get(CACHE_MANAGER); + testApiClient = new TestApiClient(app, baseRouteName); + }); + + beforeEach(async () => { + await cleanupCollections(em); + }); + + afterAll(async () => { + await app.close(); + }); + + describe('logout', () => { + describe('when a valid jwt is provided', () => { + const setup = async () => { + const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent(); + + await em.persistAndFlush([studentAccount, studentUser]); + em.clear(); + + const loggedInClient = await testApiClient.login(studentAccount); + + return { + loggedInClient, + studentAccount, + }; + }; + + it('should log out the user', async () => { + const { loggedInClient, studentAccount } = await setup(); + + const response: Response = await loggedInClient.post(''); + + expect(response.status).toEqual(HttpStatus.OK); + expect(await cacheManager.store.keys(`jwt:${studentAccount.id}:*`)).toHaveLength(0); + }); + }); + + describe('when the user is not logged in', () => { + it('should return unauthorized', async () => { + const response: Response = await testApiClient.post(''); + + expect(response.status).toEqual(HttpStatus.UNAUTHORIZED); + }); + }); + }); +}); diff --git a/apps/server/src/modules/authentication/controllers/index.ts b/apps/server/src/modules/authentication/controllers/index.ts new file mode 100644 index 00000000000..94b9a1dc3aa --- /dev/null +++ b/apps/server/src/modules/authentication/controllers/index.ts @@ -0,0 +1,2 @@ +export { LogoutController } from './logout.controller'; +export { LoginController } from './login.controller'; diff --git a/apps/server/src/modules/authentication/controllers/logout.controller.ts b/apps/server/src/modules/authentication/controllers/logout.controller.ts new file mode 100644 index 00000000000..a483a9a1bec --- /dev/null +++ b/apps/server/src/modules/authentication/controllers/logout.controller.ts @@ -0,0 +1,20 @@ +import { JWT, JwtAuthentication } from '@infra/auth-guard'; +import { Controller, HttpCode, HttpStatus, Post } from '@nestjs/common'; +import { ApiOkResponse, ApiOperation, ApiTags, ApiUnauthorizedResponse } from '@nestjs/swagger'; +import { LogoutUc } from '../uc'; + +@ApiTags('Authentication') +@Controller('logout') +export class LogoutController { + constructor(private readonly logoutUc: LogoutUc) {} + + @JwtAuthentication() + @Post() + @HttpCode(HttpStatus.OK) + @ApiOperation({ summary: 'Logs out a user.' }) + @ApiOkResponse({ description: 'Logout was successful.' }) + @ApiUnauthorizedResponse({ description: 'There has been an error while logging out.' }) + async logout(@JWT() jwt: string): Promise { + await this.logoutUc.logout(jwt); + } +} diff --git a/apps/server/src/modules/authentication/helper/jwt-whitelist.adapter.spec.ts b/apps/server/src/modules/authentication/helper/jwt-whitelist.adapter.spec.ts index 2d2662e1dde..471584ea96c 100644 --- a/apps/server/src/modules/authentication/helper/jwt-whitelist.adapter.spec.ts +++ b/apps/server/src/modules/authentication/helper/jwt-whitelist.adapter.spec.ts @@ -1,47 +1,30 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { JwtValidationAdapter } from '@infra/auth-guard/'; -import { CacheService } from '@infra/cache'; -import { CacheStoreType } from '@infra/cache/interface/cache-store-type.enum'; +import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { ObjectId } from '@mikro-orm/mongodb'; import { CACHE_MANAGER } from '@nestjs/cache-manager'; import { Test, TestingModule } from '@nestjs/testing'; -import { feathersRedis } from '@src/imports-from-feathers'; import { Cache } from 'cache-manager'; import { JwtWhitelistAdapter } from './jwt-whitelist.adapter'; -import RedisMock = require('../../../../../../test/utils/redis/redisMock'); describe('jwt strategy', () => { let module: TestingModule; let jwtWhitelistAdapter: JwtWhitelistAdapter; - let jwtValidationAdapter: JwtValidationAdapter; let cacheManager: DeepMocked; - let cacheService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ providers: [ - JwtValidationAdapter, JwtWhitelistAdapter, { provide: CACHE_MANAGER, useValue: createMock(), }, - { - provide: CacheService, - useValue: createMock(), - }, ], }).compile(); - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-member-access - const redisClientMock = new RedisMock(); - // eslint-disable-next-line @typescript-eslint/no-unsafe-call,@typescript-eslint/no-unsafe-member-access - feathersRedis.setRedisClient(redisClientMock); cacheManager = module.get(CACHE_MANAGER); - cacheService = module.get(CacheService); jwtWhitelistAdapter = module.get(JwtWhitelistAdapter); - jwtValidationAdapter = module.get(JwtValidationAdapter); }); afterAll(async () => { @@ -52,41 +35,58 @@ describe('jwt strategy', () => { jest.resetAllMocks(); }); - describe('when authenticate a user with jwt', () => { - it('should fail without whitelisted jwt', async () => { - const accountId = new ObjectId().toHexString(); - const jti = new ObjectId().toHexString(); - await expect(jwtValidationAdapter.isWhitelisted(accountId, jti)).rejects.toThrow( - 'Session was expired due to inactivity - autologout.' - ); - }); - it('should pass when jwt has been whitelisted', async () => { - const accountId = new ObjectId().toHexString(); - const jti = new ObjectId().toHexString(); - await jwtWhitelistAdapter.addToWhitelist(accountId, jti); - // might fail when we would wait more than JWT_TIMEOUT_SECONDS - await jwtValidationAdapter.isWhitelisted(accountId, jti); - }); - }); + describe('addToWhitelist', () => { + describe('when adding jwt to the whitelist', () => { + const setup = () => { + const accountId = new ObjectId().toHexString(); + const jti = new ObjectId().toHexString(); + const expirationInSeconds = Configuration.get('JWT_TIMEOUT_SECONDS') as number; - describe('removeFromWhitelist is called', () => { - describe('when redis is used as cache store', () => { - it('should call the cache manager to delete the entry from the cache', async () => { - cacheService.getStoreType.mockReturnValue(CacheStoreType.REDIS); + return { + accountId, + jti, + expirationInSeconds, + }; + }; - await jwtWhitelistAdapter.removeFromWhitelist('accountId', 'jti'); + it('should call the cache manager to set the jwt from the cache', async () => { + const { accountId, jti, expirationInSeconds } = setup(); - expect(cacheManager.del).toHaveBeenCalledWith('jwt:accountId:jti'); + await jwtWhitelistAdapter.addToWhitelist(accountId, jti); + + expect(cacheManager.set).toHaveBeenCalledWith( + `jwt:${accountId}:${jti}`, + { + IP: 'NONE', + Browser: 'NONE', + Device: 'NONE', + privateDevice: false, + expirationInSeconds, + }, + expirationInSeconds * 1000 + ); }); }); + }); + + describe('removeFromWhitelist', () => { + describe('when removing a token from the whitelist', () => { + const setup = () => { + const accountId = new ObjectId().toHexString(); + const jti = new ObjectId().toHexString(); + + return { + accountId, + jti, + }; + }; - describe('when a memory store is used', () => { - it('should do nothing', async () => { - cacheService.getStoreType.mockReturnValue(CacheStoreType.MEMORY); + it('should call the cache manager to jwt the entry from the cache', async () => { + const { accountId, jti } = setup(); - await jwtWhitelistAdapter.removeFromWhitelist('accountId', 'jti'); + await jwtWhitelistAdapter.removeFromWhitelist(accountId, jti); - expect(cacheManager.del).not.toHaveBeenCalled(); + expect(cacheManager.del).toHaveBeenCalledWith(`jwt:${accountId}:${jti}`); }); }); }); diff --git a/apps/server/src/modules/authentication/helper/jwt-whitelist.adapter.ts b/apps/server/src/modules/authentication/helper/jwt-whitelist.adapter.ts index 9a7b9d81f26..c7568545906 100644 --- a/apps/server/src/modules/authentication/helper/jwt-whitelist.adapter.ts +++ b/apps/server/src/modules/authentication/helper/jwt-whitelist.adapter.ts @@ -1,27 +1,23 @@ -import { CacheService } from '@infra/cache'; -import { CacheStoreType } from '@infra/cache/interface/cache-store-type.enum'; import { CACHE_MANAGER } from '@nestjs/cache-manager'; import { Inject, Injectable } from '@nestjs/common'; -import { addTokenToWhitelist, createRedisIdentifierFromJwtData } from '@src/imports-from-feathers'; +import { createRedisIdentifierFromJwtData, getRedisData, JwtRedisData } from '@src/imports-from-feathers'; import { Cache } from 'cache-manager'; @Injectable() export class JwtWhitelistAdapter { - constructor( - @Inject(CACHE_MANAGER) private readonly cacheManager: Cache, - private readonly cacheService: CacheService - ) {} + constructor(@Inject(CACHE_MANAGER) private readonly cacheManager: Cache) {} async addToWhitelist(accountId: string, jti: string): Promise { - const redisIdentifier = createRedisIdentifierFromJwtData(accountId, jti); - // eslint-disable-next-line @typescript-eslint/no-unsafe-call - await addTokenToWhitelist(redisIdentifier); + const redisIdentifier: string = createRedisIdentifierFromJwtData(accountId, jti); + const redisData: JwtRedisData = getRedisData({}); + const expirationInMilliseconds: number = redisData.expirationInSeconds * 1000; + + await this.cacheManager.set(redisIdentifier, redisData, expirationInMilliseconds); } async removeFromWhitelist(accountId: string, jti: string): Promise { - if (this.cacheService.getStoreType() === CacheStoreType.REDIS) { - const redisIdentifier: string = createRedisIdentifierFromJwtData(accountId, jti); - await this.cacheManager.del(redisIdentifier); - } + const redisIdentifier: string = createRedisIdentifierFromJwtData(accountId, jti); + + await this.cacheManager.del(redisIdentifier); } } diff --git a/apps/server/src/modules/authentication/uc/index.ts b/apps/server/src/modules/authentication/uc/index.ts index 8616d4505f9..bd541002fd4 100644 --- a/apps/server/src/modules/authentication/uc/index.ts +++ b/apps/server/src/modules/authentication/uc/index.ts @@ -1,2 +1,3 @@ export { LoginDto } from './dto'; export { LoginUc } from './login.uc'; +export { LogoutUc } from './logout.uc'; diff --git a/apps/server/src/modules/authentication/uc/logout.uc.spec.ts b/apps/server/src/modules/authentication/uc/logout.uc.spec.ts new file mode 100644 index 00000000000..fc06ce13cf6 --- /dev/null +++ b/apps/server/src/modules/authentication/uc/logout.uc.spec.ts @@ -0,0 +1,47 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { Test, TestingModule } from '@nestjs/testing'; +import { JwtTestFactory } from '@shared/testing'; +import { AuthenticationService } from '../services'; +import { LogoutUc } from './logout.uc'; + +describe(LogoutUc.name, () => { + let module: TestingModule; + let logoutUc: LogoutUc; + + let authenticationService: DeepMocked; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [ + LogoutUc, + { + provide: AuthenticationService, + useValue: createMock(), + }, + ], + }).compile(); + + logoutUc = await module.get(LogoutUc); + authenticationService = await module.get(AuthenticationService); + }); + + describe('logout', () => { + describe('when a jwt is given', () => { + const setup = () => { + const jwt = JwtTestFactory.createJwt(); + + return { + jwt, + }; + }; + + it('should remove the user from the whitelist', async () => { + const { jwt } = setup(); + + await logoutUc.logout(jwt); + + expect(authenticationService.removeJwtFromWhitelist).toHaveBeenCalledWith(jwt); + }); + }); + }); +}); diff --git a/apps/server/src/modules/authentication/uc/logout.uc.ts b/apps/server/src/modules/authentication/uc/logout.uc.ts new file mode 100644 index 00000000000..63c792626cb --- /dev/null +++ b/apps/server/src/modules/authentication/uc/logout.uc.ts @@ -0,0 +1,11 @@ +import { Injectable } from '@nestjs/common'; +import { AuthenticationService } from '../services'; + +@Injectable() +export class LogoutUc { + constructor(private readonly authenticationService: AuthenticationService) {} + + async logout(jwt: string): Promise { + await this.authenticationService.removeJwtFromWhitelist(jwt); + } +} diff --git a/apps/server/src/shared/testing/factory/jwt.test.factory.ts b/apps/server/src/shared/testing/factory/jwt.test.factory.ts index 6d63fada5c2..54658c5abeb 100644 --- a/apps/server/src/shared/testing/factory/jwt.test.factory.ts +++ b/apps/server/src/shared/testing/factory/jwt.test.factory.ts @@ -1,5 +1,5 @@ -import jwt from 'jsonwebtoken'; import crypto, { KeyPairKeyObjectResult } from 'crypto'; +import jwt from 'jsonwebtoken'; const keyPair: KeyPairKeyObjectResult = crypto.generateKeyPairSync('rsa', { modulusLength: 4096 }); const publicKey: string | Buffer = keyPair.publicKey.export({ type: 'pkcs1', format: 'pem' }); @@ -36,6 +36,7 @@ export class JwtTestFactory { algorithm: 'RS256', } ); + return validJwt; } } From 943220fbe3b168c6ed0248b78dd340fc81c0aba9 Mon Sep 17 00:00:00 2001 From: mrikallab <93978883+mrikallab@users.noreply.github.com> Date: Thu, 10 Oct 2024 10:57:24 +0200 Subject: [PATCH 3/5] N21-2236 fix lti encryption (#5281) --- .../templates/configmap_file_init.yml.j2 | 4 +- .../service/external-tool.service.spec.ts | 31 +-- .../service/external-tool.service.ts | 12 +- .../external-tool/uc/external-tool.uc.spec.ts | 247 +++++++++++++++++- .../tool/external-tool/uc/external-tool.uc.ts | 36 ++- .../src/modules/tool/tool-api.module.ts | 2 + 6 files changed, 289 insertions(+), 43 deletions(-) diff --git a/ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2 b/ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2 index f191ce69ca8..238b44226e4 100644 --- a/ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2 +++ b/ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2 @@ -519,8 +519,8 @@ data: echo "Inserting ctl seed data secrets to external-tools..." # Encrypt secrets of external tools that contain an lti11 config. - $CTL_SEED_SECRET_ONLINE_DIA_MATHE=$(node scripts/secret.js -s $AES_KEY -e $CTL_SEED_SECRET_ONLINE_DIA_MATHE) - $CTL_SEED_SECRET_ONLINE_DIA_DEUTSCH=$(node scripts/secret.js -s $AES_KEY -e $CTL_SEED_SECRET_ONLINE_DIA_DEUTSCH) + CTL_SEED_SECRET_ONLINE_DIA_MATHE=$(node scripts/secret.js -s $AES_KEY -e $CTL_SEED_SECRET_ONLINE_DIA_MATHE) + CTL_SEED_SECRET_ONLINE_DIA_DEUTSCH=$(node scripts/secret.js -s $AES_KEY -e $CTL_SEED_SECRET_ONLINE_DIA_DEUTSCH) mongosh $DATABASE__URL --quiet --eval 'db.getCollection("external-tools").updateOne( { diff --git a/apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts b/apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts index 5e08193da2c..38a14509f9d 100644 --- a/apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts +++ b/apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts @@ -1,5 +1,4 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { DefaultEncryptionService, EncryptionService } from '@infra/encryption'; import { ProviderOauthClient } from '@modules/oauth-provider/domain'; import { UnprocessableEntityException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; @@ -24,7 +23,6 @@ describe(ExternalToolService.name, () => { let oauthProviderService: DeepMocked; let commonToolDeleteService: DeepMocked; let mapper: DeepMocked; - let encryptionService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -42,10 +40,6 @@ describe(ExternalToolService.name, () => { provide: ExternalToolServiceMapper, useValue: createMock(), }, - { - provide: DefaultEncryptionService, - useValue: createMock(), - }, { provide: LegacyLogger, useValue: createMock(), @@ -62,7 +56,6 @@ describe(ExternalToolService.name, () => { oauthProviderService = module.get(OauthProviderService); mapper = module.get(ExternalToolServiceMapper); commonToolDeleteService = module.get(CommonToolDeleteService); - encryptionService = module.get(DefaultEncryptionService); }); afterAll(async () => { @@ -160,29 +153,14 @@ describe(ExternalToolService.name, () => { describe('when lti11 config is set', () => { const setup = () => { - const encryptedSecret = 'encryptedSecret'; const { externalTool, lti11ToolConfig } = createTools(); externalTool.config = lti11ToolConfig; - const lti11ToolConfigDOEncrypted: Lti11ToolConfig = { ...lti11ToolConfig, secret: encryptedSecret }; - const externalToolDOEncrypted: ExternalTool = externalToolFactory.build({ - ...externalTool, - config: lti11ToolConfigDOEncrypted, - }); - encryptionService.encrypt.mockReturnValue(encryptedSecret); - externalToolRepo.save.mockResolvedValue(externalToolDOEncrypted); + externalToolRepo.save.mockResolvedValue(externalTool); - return { externalTool, lti11ToolConfig, encryptedSecret, externalToolDOEncrypted }; + return { externalTool, lti11ToolConfig }; }; - it('should encrypt the secret', async () => { - const { externalTool } = setup(); - - await service.createExternalTool(externalTool); - - expect(encryptionService.encrypt).toHaveBeenCalledWith('secret'); - }); - it('should call the repo to save a tool', async () => { const { externalTool } = setup(); @@ -192,11 +170,11 @@ describe(ExternalToolService.name, () => { }); it('should save DO', async () => { - const { externalTool, externalToolDOEncrypted } = setup(); + const { externalTool } = setup(); const result: ExternalTool = await service.createExternalTool(externalTool); - expect(result).toEqual(externalToolDOEncrypted); + expect(result).toEqual(externalTool); }); }); }); @@ -512,7 +490,6 @@ describe(ExternalToolService.name, () => { describe('updateExternalTool', () => { describe('when external tool with lti11 config is given', () => { const setup = () => { - encryptionService.encrypt.mockReturnValue('newEncryptedSecret'); const changedTool: ExternalTool = externalToolFactory .withLti11Config({ secret: 'newEncryptedSecret' }) .build({ name: 'newName' }); diff --git a/apps/server/src/modules/tool/external-tool/service/external-tool.service.ts b/apps/server/src/modules/tool/external-tool/service/external-tool.service.ts index 784e64c5643..1d320179cd6 100644 --- a/apps/server/src/modules/tool/external-tool/service/external-tool.service.ts +++ b/apps/server/src/modules/tool/external-tool/service/external-tool.service.ts @@ -1,7 +1,6 @@ -import { DefaultEncryptionService, EncryptionService } from '@infra/encryption'; import { ProviderOauthClient } from '@modules/oauth-provider/domain'; import { OauthProviderService } from '@modules/oauth-provider/domain/service/oauth-provider.service'; -import { Inject, Injectable, UnprocessableEntityException } from '@nestjs/common'; +import { Injectable, UnprocessableEntityException } from '@nestjs/common'; import { Page } from '@shared/domain/domainobject'; import { IFindOptions } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; @@ -19,15 +18,12 @@ export class ExternalToolService { private readonly externalToolRepo: ExternalToolRepo, private readonly oauthProviderService: OauthProviderService, private readonly mapper: ExternalToolServiceMapper, - @Inject(DefaultEncryptionService) private readonly encryptionService: EncryptionService, private readonly legacyLogger: LegacyLogger, private readonly commonToolDeleteService: CommonToolDeleteService ) {} public async createExternalTool(externalTool: ExternalTool): Promise { - if (ExternalTool.isLti11Config(externalTool.config) && externalTool.config.secret) { - externalTool.config.secret = this.encryptionService.encrypt(externalTool.config.secret); - } else if (ExternalTool.isOauth2Config(externalTool.config)) { + if (ExternalTool.isOauth2Config(externalTool.config)) { const oauthClient: Partial = this.mapper.mapDoToProviderOauthClient( externalTool.name, externalTool.config @@ -42,10 +38,6 @@ export class ExternalToolService { } public async updateExternalTool(toUpdate: ExternalTool): Promise { - if (ExternalTool.isLti11Config(toUpdate.config) && toUpdate.config.secret) { - toUpdate.config.secret = this.encryptionService.encrypt(toUpdate.config.secret); - } - await this.updateOauth2ToolConfig(toUpdate); const externalTool: ExternalTool = await this.externalToolRepo.save(toUpdate); diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts index b3af5f0e83e..625d60eee53 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts @@ -1,5 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Configuration } from '@hpi-schul-cloud/commons/lib'; +import { DefaultEncryptionService, EncryptionService } from '@infra/encryption'; import { ObjectId } from '@mikro-orm/mongodb'; import { Action, AuthorizationService } from '@modules/authorization'; import { School, SchoolService } from '@modules/school'; @@ -13,6 +14,7 @@ import { Role, User } from '@shared/domain/entity'; import { IFindOptions, Permission, SortOrder } from '@shared/domain/interface'; import { currentUserFactory, roleFactory, setupEntities, userFactory } from '@shared/testing'; import { CustomParameter } from '../../common/domain'; +import { LtiMessageType, LtiPrivacyPermission, ToolConfigType } from '../../common/enum'; import { ExternalToolSearchQuery } from '../../common/interface'; import { CommonToolMetadataService } from '../../common/service/common-tool-metadata.service'; import { schoolExternalToolFactory } from '../../school-external-tool/testing'; @@ -22,6 +24,7 @@ import { ExternalToolMetadata, ExternalToolParameterDatasheetTemplateProperty, ExternalToolProps, + Lti11ToolConfig, Oauth2ToolConfig, } from '../domain'; import { @@ -36,9 +39,10 @@ import { externalToolDatasheetTemplateDataFactory, externalToolFactory, fileRecordRefFactory, + lti11ToolConfigFactory, oauth2ToolConfigFactory, } from '../testing'; -import { ExternalToolCreate, ExternalToolImportResult, ExternalToolUpdate } from './dto'; +import { ExternalToolCreate, ExternalToolImportResult, ExternalToolUpdate, Lti11ToolConfigUpdate } from './dto'; import { ExternalToolUc } from './external-tool.uc'; describe(ExternalToolUc.name, () => { @@ -54,6 +58,7 @@ describe(ExternalToolUc.name, () => { let commonToolMetadataService: DeepMocked; let pdfService: DeepMocked; let externalToolImageService: DeepMocked; + let encryptionService: DeepMocked; beforeAll(async () => { await setupEntities(); @@ -99,6 +104,10 @@ describe(ExternalToolUc.name, () => { provide: ExternalToolImageService, useValue: createMock(), }, + { + provide: DefaultEncryptionService, + useValue: createMock(), + }, ], }).compile(); @@ -112,6 +121,7 @@ describe(ExternalToolUc.name, () => { commonToolMetadataService = module.get(CommonToolMetadataService); pdfService = module.get(DatasheetPdfService); externalToolImageService = module.get(ExternalToolImageService); + encryptionService = module.get(DefaultEncryptionService); }); afterAll(async () => { @@ -140,6 +150,7 @@ describe(ExternalToolUc.name, () => { const externalTool: ExternalTool = externalToolFactory.withCustomParameters(1).buildWithId(); const oauth2ConfigWithoutExternalData: Oauth2ToolConfig = oauth2ToolConfigFactory.build(); + const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); const query: ExternalToolSearchQuery = { name: externalTool.name, @@ -171,6 +182,7 @@ describe(ExternalToolUc.name, () => { query, toolId, mockLogoBase64, + lti11ToolConfig, }; }; @@ -334,6 +346,44 @@ describe(ExternalToolUc.name, () => { ); }); }); + + describe('when external tool with lti11 config is given', () => { + const setupLTI = () => { + const { currentUser } = setupAuthorization(); + const { externalTool, lti11ToolConfig } = setupDefault(); + externalTool.config = lti11ToolConfig; + + encryptionService.encrypt.mockReturnValue('encrypted'); + + return { + currentUser, + externalTool, + }; + }; + it('should call the encryption service', async () => { + const { currentUser, externalTool } = setupLTI(); + + await uc.createExternalTool(currentUser.userId, externalTool.getProps(), 'jwt'); + + expect(encryptionService.encrypt).toHaveBeenCalledWith('secret'); + }); + + it('should call the service to save a tool', async () => { + const { currentUser, externalTool } = setupLTI(); + + await uc.createExternalTool(currentUser.userId, externalTool.getProps(), 'jwt'); + + expect(externalToolService.createExternalTool).toHaveBeenNthCalledWith( + 1, + new ExternalTool({ + ...externalTool.getProps(), + logo: 'base64LogoString', + config: { ...externalTool.config, secret: 'encrypted' }, + id: expect.any(String), + }) + ); + }); + }); }); describe('importExternalTools', () => { @@ -382,6 +432,14 @@ describe(ExternalToolUc.name, () => { }; }; + it('should not call encryption service', async () => { + const { user, externalTool1, externalTool2 } = setup(); + + await uc.importExternalTools(user.id, [externalTool1.getProps(), externalTool2.getProps()], 'jwt'); + + expect(encryptionService.encrypt).not.toHaveBeenCalled(); + }); + it('should check the users permission', async () => { const { user, externalTool1, externalTool2 } = setup(); @@ -477,6 +535,67 @@ describe(ExternalToolUc.name, () => { }); }); + describe('when importing lti tool', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const externalTool1 = externalToolFactory.build({ + name: 'tool1', + medium: { + mediumId: 'medium1', + mediaSourceId: 'mediumSource1', + }, + }); + + const ltiConfig = lti11ToolConfigFactory.build(); + externalTool1.config = ltiConfig; + + const externalToolCreate1: ExternalToolCreate = { + ...externalTool1.getProps(), + thumbnailUrl: 'https://thumbnail.url1', + }; + + const jwt = 'jwt'; + + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + logoService.fetchLogo.mockResolvedValueOnce(undefined); + const thumbnailFileRecordRef = fileRecordRefFactory.build(); + externalToolImageService.uploadImageFileFromUrl.mockResolvedValueOnce(thumbnailFileRecordRef); + externalToolService.createExternalTool.mockResolvedValueOnce(externalTool1); + encryptionService.encrypt.mockReturnValue('encrypted'); + + return { + user, + externalTool1, + externalToolCreate1, + thumbnailFileRecordRef, + jwt, + }; + }; + + it('should call encryption service', async () => { + const { user, externalTool1 } = setup(); + + await uc.importExternalTools(user.id, [externalTool1.getProps()], 'jwt'); + + expect(encryptionService.encrypt).toHaveBeenCalled(); + }); + + it('should save tool', async () => { + const { user, externalTool1 } = setup(); + + await uc.importExternalTools(user.id, [externalTool1.getProps()], 'jwt'); + + expect(externalToolService.createExternalTool).toHaveBeenNthCalledWith( + 1, + new ExternalTool({ + ...externalTool1.getProps(), + config: lti11ToolConfigFactory.build({ ...externalTool1.config, secret: 'encrypted' }), + id: expect.any(String), + }) + ); + }); + }); + describe('when an external tools fails the validation', () => { const setup = () => { const user = userFactory.buildWithId(); @@ -643,6 +762,8 @@ describe(ExternalToolUc.name, () => { url: undefined, }); + const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); + externalToolService.updateExternalTool.mockResolvedValue(updatedExternalTool); externalToolService.findById.mockResolvedValue(new ExternalTool(externalToolToUpdate)); @@ -652,6 +773,7 @@ describe(ExternalToolUc.name, () => { externalToolDOtoUpdate: externalToolToUpdate, toolId, mockLogoBase64, + lti11ToolConfig, }; }; @@ -868,6 +990,129 @@ describe(ExternalToolUc.name, () => { ); }); }); + + describe('when lti11 config is given and secret is not encrypted', () => { + const setupLTI = () => { + const { externalTool, toolId, mockLogoBase64 } = setupDefault(); + + const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); + const externalToolToUpdate: ExternalToolUpdate = { + ...externalTool.getProps(), + name: 'newName', + config: lti11ToolConfig, + url: undefined, + logo: mockLogoBase64, + }; + + const currentTestTool = new ExternalTool({ ...externalToolToUpdate }); + const expectedLtiConfig = lti11ToolConfigFactory.buildWithId({ secret: 'encrypted' }); + currentTestTool.config = expectedLtiConfig; + + const updatedExternalTool: ExternalTool = externalToolFactory.build({ + ...externalTool.getProps(), + name: 'newName', + config: expectedLtiConfig, + url: undefined, + logo: mockLogoBase64, + }); + + externalToolService.findById.mockResolvedValue(currentTestTool); + encryptionService.encrypt.mockReturnValue(expectedLtiConfig.secret); + externalToolService.updateExternalTool.mockResolvedValue(updatedExternalTool); + + return { + externalTool, + updatedExternalToolDO: updatedExternalTool, + externalToolDOtoUpdate: externalToolToUpdate, + toolId, + mockLogoBase64, + lti11ToolConfig, + }; + }; + + it('should call encryption service', async () => { + const { currentUser } = setupAuthorization(); + const { toolId, externalToolDOtoUpdate, lti11ToolConfig } = setupLTI(); + + await uc.updateExternalTool(currentUser.userId, toolId, externalToolDOtoUpdate, 'jwt'); + + expect(encryptionService.encrypt).toHaveBeenNthCalledWith(1, lti11ToolConfig.secret); + }); + + it('should call the service to update the tool', async () => { + const { currentUser } = setupAuthorization(); + const { toolId, externalToolDOtoUpdate, updatedExternalToolDO } = setupLTI(); + + await uc.updateExternalTool(currentUser.userId, toolId, externalToolDOtoUpdate, 'jwt'); + + expect(externalToolService.updateExternalTool).toHaveBeenCalledWith(updatedExternalToolDO); + }); + }); + + describe('when lti11 config is given and secret is already encrypted', () => { + const setupLTI = () => { + const { externalTool, toolId, mockLogoBase64 } = setupDefault(); + + const lti11ToolConfig: Lti11ToolConfigUpdate = { + type: ToolConfigType.LTI11, + baseUrl: 'https://www.basic-baseUrl.com/', + key: 'key', + privacy_permission: LtiPrivacyPermission.PSEUDONYMOUS, + lti_message_type: LtiMessageType.BASIC_LTI_LAUNCH_REQUEST, + launch_presentation_locale: 'de-DE', + }; + + const externalToolToUpdate: ExternalToolUpdate = { + ...externalTool.getProps(), + config: lti11ToolConfig, + name: 'newName', + url: undefined, + }; + + const updatedExternalTool: ExternalTool = externalToolFactory.build({ + ...externalTool.getProps(), + config: lti11ToolConfigFactory.build({ ...externalTool.config, secret: 'encrypted' }), + name: 'newName', + url: undefined, + logo: mockLogoBase64, + }); + + externalToolService.findById.mockResolvedValue( + new ExternalTool({ + ...externalToolToUpdate, + config: lti11ToolConfigFactory.build({ ...externalToolToUpdate.config, secret: 'encrypted' }), + }) + ); + externalToolService.updateExternalTool.mockResolvedValue(updatedExternalTool); + + return { + externalTool, + updatedExternalToolDO: updatedExternalTool, + externalToolDOtoUpdate: externalToolToUpdate, + toolId, + mockLogoBase64, + lti11ToolConfig, + }; + }; + + it('should not call encryption service', async () => { + const { currentUser } = setupAuthorization(); + const { toolId, externalToolDOtoUpdate } = setupLTI(); + + await uc.updateExternalTool(currentUser.userId, toolId, externalToolDOtoUpdate, 'jwt'); + + expect(encryptionService.encrypt).not.toHaveBeenCalledWith(); + }); + + it('should call the service to update the tool', async () => { + const { currentUser } = setupAuthorization(); + const { toolId, externalToolDOtoUpdate, updatedExternalToolDO } = setupLTI(); + + await uc.updateExternalTool(currentUser.userId, toolId, externalToolDOtoUpdate, 'jwt'); + + expect(externalToolService.updateExternalTool).toHaveBeenCalledWith(updatedExternalToolDO); + }); + }); }); describe('deleteExternalTool', () => { diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts index bc79c03f090..f9396b3e299 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts @@ -1,16 +1,25 @@ +import { DefaultEncryptionService, EncryptionService } from '@infra/encryption'; import { ObjectId } from '@mikro-orm/mongodb'; import { AuthorizationService } from '@modules/authorization'; import { School, SchoolService } from '@modules/school'; import { SchoolExternalTool } from '@modules/tool/school-external-tool/domain'; import { SchoolExternalToolService } from '@modules/tool/school-external-tool/service'; -import { Injectable } from '@nestjs/common'; +import { Inject, Injectable } from '@nestjs/common'; import { Page } from '@shared/domain/domainobject'; import { User } from '@shared/domain/entity'; import { IFindOptions, Permission } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { ExternalToolSearchQuery } from '../../common/interface'; import { CommonToolMetadataService } from '../../common/service/common-tool-metadata.service'; -import { ExternalTool, ExternalToolConfig, ExternalToolDatasheetTemplateData, ExternalToolMetadata } from '../domain'; +import { + BasicToolConfig, + ExternalTool, + ExternalToolConfig, + ExternalToolDatasheetTemplateData, + ExternalToolMetadata, + Lti11ToolConfig, + Oauth2ToolConfig, +} from '../domain'; import { ExternalToolDatasheetMapper } from '../mapper/external-tool-datasheet.mapper'; import { DatasheetPdfService, @@ -32,7 +41,8 @@ export class ExternalToolUc { private readonly externalToolLogoService: ExternalToolLogoService, private readonly commonToolMetadataService: CommonToolMetadataService, private readonly datasheetPdfService: DatasheetPdfService, - private readonly externalToolImageService: ExternalToolImageService + private readonly externalToolImageService: ExternalToolImageService, + @Inject(DefaultEncryptionService) private readonly encryptionService: EncryptionService ) {} public async createExternalTool( @@ -42,6 +52,8 @@ export class ExternalToolUc { ): Promise { await this.ensurePermission(userId, Permission.TOOL_ADMIN); + externalToolCreate.config = this.encryptLtiSecret(externalToolCreate); + const tool: ExternalTool = await this.validateAndSaveExternalTool(externalToolCreate, jwt); return tool; @@ -64,6 +76,8 @@ export class ExternalToolUc { }); try { + externalTool.config = this.encryptLtiSecret(externalTool); + // eslint-disable-next-line no-await-in-loop const savedTool: ExternalTool = await this.validateAndSaveExternalTool(externalTool, jwt); @@ -118,6 +132,8 @@ export class ExternalToolUc { ): Promise { await this.ensurePermission(userId, Permission.TOOL_ADMIN); + externalToolUpdate.config = this.encryptLtiSecret(externalToolUpdate); + const { thumbnailUrl, ...externalToolUpdateProps } = externalToolUpdate; const currentExternalTool: ExternalTool = await this.externalToolService.findById(toolId); @@ -248,4 +264,18 @@ export class ExternalToolUc { return fileName; } + + private encryptLtiSecret( + externalTool: ExternalToolCreate | ExternalToolUpdate + ): BasicToolConfig | Lti11ToolConfig | Oauth2ToolConfig { + if (ExternalTool.isLti11Config(externalTool.config) && externalTool.config.secret) { + const encrypted = this.encryptionService.encrypt(externalTool.config.secret); + + const updatedConfig = new Lti11ToolConfig({ ...externalTool.config, secret: encrypted }); + + return updatedConfig; + } + + return externalTool.config; + } } diff --git a/apps/server/src/modules/tool/tool-api.module.ts b/apps/server/src/modules/tool/tool-api.module.ts index 12d33bd3d90..51eec1332ff 100644 --- a/apps/server/src/modules/tool/tool-api.module.ts +++ b/apps/server/src/modules/tool/tool-api.module.ts @@ -1,3 +1,4 @@ +import { EncryptionModule } from '@infra/encryption'; import { AuthorizationModule } from '@modules/authorization'; import { BoardModule } from '@modules/board'; import { LegacySchoolModule } from '@modules/legacy-school'; @@ -35,6 +36,7 @@ import { ToolModule } from './tool.module'; BoardModule, SchoolModule, UserLicenseModule, + EncryptionModule, ], controllers: [ ToolLaunchController, From 0022fc459b218c14fdcc4358637e7570c79e77f1 Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Wed, 9 Oct 2024 16:28:41 +0200 Subject: [PATCH 4/5] update course sync --- .../service/course-do.service.spec.ts | 51 ++++++++++++++++--- .../learnroom/service/course-do.service.ts | 19 ++++++- .../learnroom/uc/course-sync.uc.spec.ts | 36 +++++++++++-- .../modules/learnroom/uc/course-sync.uc.ts | 27 +++++++--- 4 files changed, 111 insertions(+), 22 deletions(-) diff --git a/apps/server/src/modules/learnroom/service/course-do.service.spec.ts b/apps/server/src/modules/learnroom/service/course-do.service.spec.ts index a6fae948299..9f9591c5c19 100644 --- a/apps/server/src/modules/learnroom/service/course-do.service.spec.ts +++ b/apps/server/src/modules/learnroom/service/course-do.service.spec.ts @@ -1,6 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; -import { Group } from '@modules/group'; +import { Group, GroupUser } from '@modules/group'; import { Test, TestingModule } from '@nestjs/testing'; import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { Page } from '@shared/domain/domainobject'; @@ -185,43 +185,78 @@ describe(CourseDoService.name, () => { const setup = () => { const course: Course = courseFactory.build(); const group: Group = groupFactory.build(); + const students: GroupUser[] = [{ roleId: 'student-role-id', userId: 'student-user-id' }]; + const teachers: GroupUser[] = [{ roleId: 'teacher-role-id', userId: 'teacher-user-id' }]; return { course, group, + students, + teachers, }; }; - it('should save a course with a synchronized group', async () => { - const { course, group } = setup(); + it('should save a course with synchronized group, students, and teachers', async () => { + const { course, group, students, teachers } = setup(); - await service.startSynchronization(course, group); + await service.startSynchronization(course, group, students, teachers); expect(courseRepo.save).toHaveBeenCalledWith( new Course({ ...course.getProps(), syncedWithGroup: group.id, + name: group.name, + startDate: group.validPeriod?.from, + untilDate: group.validPeriod?.until, + studentIds: students.map((student) => student.userId), + teacherIds: teachers.map((teacher) => teacher.userId), + }) + ); + }); + + it('should set an empty list of students if no teachers are present', async () => { + const { course, group, students } = setup(); + const teachers: GroupUser[] = []; // No teachers + + await service.startSynchronization(course, group, students, teachers); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + syncedWithGroup: group.id, + name: group.name, + startDate: group.validPeriod?.from, + untilDate: group.validPeriod?.until, + studentIds: [], + teacherIds: [], }) ); }); }); - describe('when a course is synchronized with a group', () => { + describe('when a course is already synchronized with a group', () => { const setup = () => { const course: Course = courseFactory.build({ syncedWithGroup: new ObjectId().toHexString() }); const group: Group = groupFactory.build(); + const students: GroupUser[] = [{ roleId: 'student-role-id', userId: 'student-user-id' }]; + const teachers: GroupUser[] = [{ roleId: 'teacher-role-id', userId: 'teacher-user-id' }]; return { course, group, + students, + teachers, }; }; - it('should throw an unprocessable entity exception', async () => { - const { course, group } = setup(); - await expect(service.startSynchronization(course, group)).rejects.toThrow( + it('should throw a CourseAlreadySynchronizedLoggableException', async () => { + const { course, group, students, teachers } = setup(); + + await expect(service.startSynchronization(course, group, students, teachers)).rejects.toThrow( CourseAlreadySynchronizedLoggableException ); + + expect(courseRepo.save).not.toHaveBeenCalled(); }); }); }); diff --git a/apps/server/src/modules/learnroom/service/course-do.service.ts b/apps/server/src/modules/learnroom/service/course-do.service.ts index 951693dee25..c5590db6bee 100644 --- a/apps/server/src/modules/learnroom/service/course-do.service.ts +++ b/apps/server/src/modules/learnroom/service/course-do.service.ts @@ -1,5 +1,5 @@ import { AuthorizationLoaderServiceGeneric } from '@modules/authorization'; -import { type Group } from '@modules/group'; +import { GroupUser, type Group } from '@modules/group'; import { Inject, Injectable } from '@nestjs/common'; import { Page } from '@shared/domain/domainobject'; import { IFindOptions } from '@shared/domain/interface'; @@ -45,12 +45,27 @@ export class CourseDoService implements AuthorizationLoaderServiceGeneric { + public async startSynchronization( + course: Course, + group: Group, + students: GroupUser[], + teachers: GroupUser[] + ): Promise { if (course.syncedWithGroup) { throw new CourseAlreadySynchronizedLoggableException(course.id); } course.syncedWithGroup = group.id; + course.name = group.name; + course.startDate = group.validPeriod?.from; + course.untilDate = group.validPeriod?.until; + + if (teachers.length >= 1) { + course.students = students.map((groupUser: GroupUser): EntityId => groupUser.userId); + course.teachers = teachers.map((groupUser: GroupUser): EntityId => groupUser.userId); + } else { + course.students = []; + } await this.courseRepo.save(course); } diff --git a/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts b/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts index 60366ab3065..ad386434cd5 100644 --- a/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts +++ b/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts @@ -1,9 +1,10 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; import { GroupService } from '@modules/group'; +import { RoleService } from '@modules/role'; import { Test, TestingModule } from '@nestjs/testing'; -import { Permission } from '@shared/domain/interface'; -import { groupFactory, setupEntities, userFactory } from '@shared/testing'; +import { Permission, RoleName } from '@shared/domain/interface'; +import { groupFactory, roleDtoFactory, setupEntities, userFactory } from '@shared/testing'; import { CourseDoService } from '../service'; import { courseFactory } from '../testing'; import { CourseSyncUc } from './course-sync.uc'; @@ -15,6 +16,7 @@ describe(CourseSyncUc.name, () => { let authorizationService: DeepMocked; let courseService: DeepMocked; let groupService: DeepMocked; + let roleService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -32,6 +34,10 @@ describe(CourseSyncUc.name, () => { provide: GroupService, useValue: createMock(), }, + { + provide: RoleService, + useValue: createMock(), + }, ], }).compile(); @@ -39,6 +45,7 @@ describe(CourseSyncUc.name, () => { authorizationService = module.get(AuthorizationService); courseService = module.get(CourseDoService); groupService = module.get(GroupService); + roleService = module.get(RoleService); await setupEntities(); }); @@ -93,15 +100,30 @@ describe(CourseSyncUc.name, () => { const user = userFactory.buildWithId(); const course = courseFactory.build(); const group = groupFactory.build(); + const studentRole = roleDtoFactory.build({ id: 'student-role-id' }); + const teacherRole = roleDtoFactory.build({ id: 'teacher-role-id' }); + + group.users = [ + { roleId: 'student-role-id', userId: 'student-user-id' }, + { roleId: 'teacher-role-id', userId: 'teacher-user-id' }, + ]; + + const students = group.users.filter((groupUser) => groupUser.roleId === studentRole.id); + const teachers = group.users.filter((groupUser) => groupUser.roleId === teacherRole.id); courseService.findById.mockResolvedValueOnce(course); groupService.findById.mockResolvedValueOnce(group); authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + roleService.findByName.mockResolvedValueOnce(studentRole).mockResolvedValueOnce(teacherRole); return { user, course, group, + studentRole, + teacherRole, + students, + teachers, }; }; @@ -119,14 +141,18 @@ describe(CourseSyncUc.name, () => { ); }); - it('should start the synchronization', async () => { - const { user, course, group } = setup(); + it('should start the synchronization with correct roles', async () => { + const { user, course, group, students, teachers } = setup(); await uc.startSynchronization(user.id, course.id, group.id); + expect(courseService.findById).toHaveBeenCalledWith(course.id); expect(groupService.findById).toHaveBeenCalledWith(group.id); - expect(courseService.startSynchronization).toHaveBeenCalledWith(course, group); + expect(roleService.findByName).toHaveBeenCalledWith(RoleName.STUDENT); + expect(roleService.findByName).toHaveBeenCalledWith(RoleName.TEACHER); + + expect(courseService.startSynchronization).toHaveBeenCalledWith(course, group, students, teachers); }); }); }); diff --git a/apps/server/src/modules/learnroom/uc/course-sync.uc.ts b/apps/server/src/modules/learnroom/uc/course-sync.uc.ts index 53a4fe2e173..770268fe882 100644 --- a/apps/server/src/modules/learnroom/uc/course-sync.uc.ts +++ b/apps/server/src/modules/learnroom/uc/course-sync.uc.ts @@ -1,8 +1,10 @@ import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; -import { Group, GroupService } from '@modules/group'; +import { GroupService, GroupUser } from '@modules/group'; +import { RoleService } from '@modules/role'; import { Injectable } from '@nestjs/common'; + import { type User as UserEntity } from '@shared/domain/entity'; -import { Permission } from '@shared/domain/interface'; +import { Permission, RoleName } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { Course } from '../domain'; import { CourseDoService } from '../service'; @@ -12,7 +14,8 @@ export class CourseSyncUc { constructor( private readonly authorizationService: AuthorizationService, private readonly courseService: CourseDoService, - private readonly groupService: GroupService + private readonly groupService: GroupService, + private readonly roleService: RoleService ) {} public async stopSynchronization(userId: EntityId, courseId: EntityId): Promise { @@ -29,9 +32,11 @@ export class CourseSyncUc { } public async startSynchronization(userId: string, courseId: string, groupId: string) { - const course: Course = await this.courseService.findById(courseId); - const group: Group = await this.groupService.findById(groupId); - const user: UserEntity = await this.authorizationService.getUserWithPermissions(userId); + const [course, group, user] = await Promise.all([ + this.courseService.findById(courseId), + this.groupService.findById(groupId), + this.authorizationService.getUserWithPermissions(userId), + ]); this.authorizationService.checkPermission( user, @@ -39,6 +44,14 @@ export class CourseSyncUc { AuthorizationContextBuilder.write([Permission.COURSE_EDIT]) ); - await this.courseService.startSynchronization(course, group); + const [studentRole, teacherRole] = await Promise.all([ + this.roleService.findByName(RoleName.STUDENT), + this.roleService.findByName(RoleName.TEACHER), + ]); + + const students = group.users.filter((groupUser: GroupUser) => groupUser.roleId === studentRole.id); + const teachers = group.users.filter((groupUser: GroupUser) => groupUser.roleId === teacherRole.id); + + await this.courseService.startSynchronization(course, group, students, teachers); } } From 4bb020779a131f68a2218063ccae00c011c5c6e0 Mon Sep 17 00:00:00 2001 From: Steliyan Dinkov Date: Thu, 10 Oct 2024 17:53:18 +0200 Subject: [PATCH 5/5] add course-sync service; clean up courseDo and schulconnex sync service; update tests + index --- apps/server/src/modules/learnroom/index.ts | 1 + .../src/modules/learnroom/learnroom.module.ts | 5 + .../service/course-do.service.spec.ts | 133 +------ .../learnroom/service/course-do.service.ts | 46 +-- .../service/course-sync.service.spec.ts | 366 ++++++++++++++++++ .../learnroom/service/course-sync.service.ts | 76 ++++ .../src/modules/learnroom/service/index.ts | 5 +- .../learnroom/uc/course-sync.uc.spec.ts | 58 ++- .../modules/learnroom/uc/course-sync.uc.ts | 32 +- .../schulconnex-course-sync.service.spec.ts | 188 +-------- .../schulconnex-course-sync.service.ts | 42 +- 11 files changed, 512 insertions(+), 440 deletions(-) create mode 100644 apps/server/src/modules/learnroom/service/course-sync.service.spec.ts create mode 100644 apps/server/src/modules/learnroom/service/course-sync.service.ts diff --git a/apps/server/src/modules/learnroom/index.ts b/apps/server/src/modules/learnroom/index.ts index 20d6d0f7508..4135d6d3677 100644 --- a/apps/server/src/modules/learnroom/index.ts +++ b/apps/server/src/modules/learnroom/index.ts @@ -6,6 +6,7 @@ export { CourseGroupService, CourseService, CourseDoService, + CourseSyncService, DashboardService, CourseRoomsService, } from './service'; diff --git a/apps/server/src/modules/learnroom/learnroom.module.ts b/apps/server/src/modules/learnroom/learnroom.module.ts index 0adf76cacd0..a8bdcbeba4c 100644 --- a/apps/server/src/modules/learnroom/learnroom.module.ts +++ b/apps/server/src/modules/learnroom/learnroom.module.ts @@ -3,6 +3,7 @@ import { ClassModule } from '@modules/class'; import { CopyHelperModule } from '@modules/copy-helper'; import { GroupModule } from '@modules/group'; import { LessonModule } from '@modules/lesson'; +import { RoleModule } from '@modules/role'; import { SchoolModule } from '@modules/school'; import { TaskModule } from '@modules/task'; import { ContextExternalToolModule } from '@modules/tool/context-external-tool'; @@ -34,6 +35,7 @@ import { CourseGroupService, CourseRoomsService, CourseService, + CourseSyncService, DashboardService, GroupDeletedHandlerService, } from './service'; @@ -56,6 +58,7 @@ import { CommonCartridgeFileValidatorPipe } from './utils'; ClassModule, SchoolModule, GroupModule, + RoleModule, ], providers: [ { @@ -79,6 +82,7 @@ import { CommonCartridgeFileValidatorPipe } from './utils'; }, CourseService, CourseDoService, + CourseSyncService, DashboardElementRepo, DashboardModelMapper, DashboardService, @@ -92,6 +96,7 @@ import { CommonCartridgeFileValidatorPipe } from './utils'; CourseCopyService, CourseService, CourseDoService, + CourseSyncService, CourseRoomsService, CommonCartridgeExportService, CommonCartridgeImportService, diff --git a/apps/server/src/modules/learnroom/service/course-do.service.spec.ts b/apps/server/src/modules/learnroom/service/course-do.service.spec.ts index 9f9591c5c19..4710ecc4aaa 100644 --- a/apps/server/src/modules/learnroom/service/course-do.service.spec.ts +++ b/apps/server/src/modules/learnroom/service/course-do.service.spec.ts @@ -1,20 +1,13 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; -import { Group, GroupUser } from '@modules/group'; +import { Group } from '@modules/group'; import { Test, TestingModule } from '@nestjs/testing'; import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { Page } from '@shared/domain/domainobject'; import { IFindOptions, SortOrder } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { groupFactory } from '@shared/testing'; -import { - Course, - COURSE_REPO, - CourseAlreadySynchronizedLoggableException, - CourseFilter, - CourseNotSynchronizedLoggableException, - CourseRepo, -} from '../domain'; +import { Course, COURSE_REPO, CourseFilter, CourseRepo } from '../domain'; import { courseFactory } from '../testing'; import { CourseDoService } from './course-do.service'; @@ -139,128 +132,6 @@ describe(CourseDoService.name, () => { }); }); - describe('stopSynchronization', () => { - describe('when a course is synchronized with a group', () => { - const setup = () => { - const course: Course = courseFactory.build({ syncedWithGroup: new ObjectId().toHexString() }); - - return { - course, - }; - }; - - it('should save a course without a synchronized group', async () => { - const { course } = setup(); - - await service.stopSynchronization(course); - - expect(courseRepo.save).toHaveBeenCalledWith( - new Course({ - ...course.getProps(), - syncedWithGroup: undefined, - }) - ); - }); - }); - - describe('when a course is not synchronized with a group', () => { - const setup = () => { - const course: Course = courseFactory.build(); - - return { - course, - }; - }; - - it('should throw an unprocessable entity exception', async () => { - const { course } = setup(); - - await expect(service.stopSynchronization(course)).rejects.toThrow(CourseNotSynchronizedLoggableException); - }); - }); - }); - - describe('startSynchronization', () => { - describe('when a course is not synchronized with a group', () => { - const setup = () => { - const course: Course = courseFactory.build(); - const group: Group = groupFactory.build(); - const students: GroupUser[] = [{ roleId: 'student-role-id', userId: 'student-user-id' }]; - const teachers: GroupUser[] = [{ roleId: 'teacher-role-id', userId: 'teacher-user-id' }]; - - return { - course, - group, - students, - teachers, - }; - }; - - it('should save a course with synchronized group, students, and teachers', async () => { - const { course, group, students, teachers } = setup(); - - await service.startSynchronization(course, group, students, teachers); - - expect(courseRepo.save).toHaveBeenCalledWith( - new Course({ - ...course.getProps(), - syncedWithGroup: group.id, - name: group.name, - startDate: group.validPeriod?.from, - untilDate: group.validPeriod?.until, - studentIds: students.map((student) => student.userId), - teacherIds: teachers.map((teacher) => teacher.userId), - }) - ); - }); - - it('should set an empty list of students if no teachers are present', async () => { - const { course, group, students } = setup(); - const teachers: GroupUser[] = []; // No teachers - - await service.startSynchronization(course, group, students, teachers); - - expect(courseRepo.save).toHaveBeenCalledWith( - new Course({ - ...course.getProps(), - syncedWithGroup: group.id, - name: group.name, - startDate: group.validPeriod?.from, - untilDate: group.validPeriod?.until, - studentIds: [], - teacherIds: [], - }) - ); - }); - }); - - describe('when a course is already synchronized with a group', () => { - const setup = () => { - const course: Course = courseFactory.build({ syncedWithGroup: new ObjectId().toHexString() }); - const group: Group = groupFactory.build(); - const students: GroupUser[] = [{ roleId: 'student-role-id', userId: 'student-user-id' }]; - const teachers: GroupUser[] = [{ roleId: 'teacher-role-id', userId: 'teacher-user-id' }]; - - return { - course, - group, - students, - teachers, - }; - }; - - it('should throw a CourseAlreadySynchronizedLoggableException', async () => { - const { course, group, students, teachers } = setup(); - - await expect(service.startSynchronization(course, group, students, teachers)).rejects.toThrow( - CourseAlreadySynchronizedLoggableException - ); - - expect(courseRepo.save).not.toHaveBeenCalled(); - }); - }); - }); - describe('findCourses', () => { describe('when course are found', () => { const setup = () => { diff --git a/apps/server/src/modules/learnroom/service/course-do.service.ts b/apps/server/src/modules/learnroom/service/course-do.service.ts index c5590db6bee..76c73fa9ea7 100644 --- a/apps/server/src/modules/learnroom/service/course-do.service.ts +++ b/apps/server/src/modules/learnroom/service/course-do.service.ts @@ -1,17 +1,10 @@ import { AuthorizationLoaderServiceGeneric } from '@modules/authorization'; -import { GroupUser, type Group } from '@modules/group'; +import { type Group } from '@modules/group'; import { Inject, Injectable } from '@nestjs/common'; import { Page } from '@shared/domain/domainobject'; import { IFindOptions } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; -import { - type Course, - COURSE_REPO, - CourseAlreadySynchronizedLoggableException, - CourseFilter, - CourseNotSynchronizedLoggableException, - CourseRepo, -} from '../domain'; +import { COURSE_REPO, CourseFilter, CourseRepo, type Course } from '../domain'; @Injectable() export class CourseDoService implements AuthorizationLoaderServiceGeneric { @@ -35,41 +28,6 @@ export class CourseDoService implements AuthorizationLoaderServiceGeneric { - if (!course.syncedWithGroup) { - throw new CourseNotSynchronizedLoggableException(course.id); - } - - course.syncedWithGroup = undefined; - - await this.courseRepo.save(course); - } - - public async startSynchronization( - course: Course, - group: Group, - students: GroupUser[], - teachers: GroupUser[] - ): Promise { - if (course.syncedWithGroup) { - throw new CourseAlreadySynchronizedLoggableException(course.id); - } - - course.syncedWithGroup = group.id; - course.name = group.name; - course.startDate = group.validPeriod?.from; - course.untilDate = group.validPeriod?.until; - - if (teachers.length >= 1) { - course.students = students.map((groupUser: GroupUser): EntityId => groupUser.userId); - course.teachers = teachers.map((groupUser: GroupUser): EntityId => groupUser.userId); - } else { - course.students = []; - } - - await this.courseRepo.save(course); - } - public async getCourseInfo(filter: CourseFilter, options?: IFindOptions): Promise> { const courses = await this.courseRepo.getCourseInfo(filter, options); diff --git a/apps/server/src/modules/learnroom/service/course-sync.service.spec.ts b/apps/server/src/modules/learnroom/service/course-sync.service.spec.ts new file mode 100644 index 00000000000..07a6140d7ac --- /dev/null +++ b/apps/server/src/modules/learnroom/service/course-sync.service.spec.ts @@ -0,0 +1,366 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { ObjectId } from '@mikro-orm/mongodb'; +import { Group, GroupPeriod, GroupUser } from '@modules/group'; +import { RoleDto, RoleService } from '@modules/role'; +import { Test, TestingModule } from '@nestjs/testing'; +import { groupFactory, roleDtoFactory } from '@shared/testing'; +import { + Course, + COURSE_REPO, + CourseAlreadySynchronizedLoggableException, + CourseNotSynchronizedLoggableException, + CourseRepo, +} from '../domain'; +import { courseFactory } from '../testing'; +import { CourseSyncService } from './course-sync.service'; + +describe(CourseSyncService.name, () => { + let module: TestingModule; + let service: CourseSyncService; + let roleService: DeepMocked; + + let courseRepo: DeepMocked; + + beforeAll(async () => { + module = await Test.createTestingModule({ + providers: [ + CourseSyncService, + { + provide: RoleService, + useValue: createMock(), + }, + { + provide: COURSE_REPO, + useValue: createMock(), + }, + ], + }).compile(); + + service = module.get(CourseSyncService); + roleService = module.get(RoleService); + courseRepo = module.get(COURSE_REPO); + }); + + afterAll(async () => { + await module.close(); + }); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('stopSynchronization', () => { + describe('when a course is synchronized with a group', () => { + const setup = () => { + const course: Course = courseFactory.build({ syncedWithGroup: new ObjectId().toHexString() }); + + return { + course, + }; + }; + + it('should save a course without a synchronized group', async () => { + const { course } = setup(); + + await service.stopSynchronization(course); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + syncedWithGroup: undefined, + }) + ); + }); + }); + + describe('when a course is not synchronized with a group', () => { + const setup = () => { + const course: Course = courseFactory.build(); + + return { + course, + }; + }; + + it('should throw an unprocessable entity exception', async () => { + const { course } = setup(); + + await expect(service.stopSynchronization(course)).rejects.toThrow(CourseNotSynchronizedLoggableException); + }); + }); + }); + + describe('startSynchronization', () => { + describe('when a course is not synchronized with a group', () => { + const setup = () => { + const course: Course = courseFactory.build(); + + const studentRole = roleDtoFactory.build({ id: 'student-role-id' }); + const teacherRole = roleDtoFactory.build({ id: 'teacher-role-id' }); + const students: GroupUser[] = [{ roleId: 'student-role-id', userId: 'student-user-id' }]; + const teachers: GroupUser[] = [{ roleId: 'teacher-role-id', userId: 'teacher-user-id' }]; + const group: Group = groupFactory.build({ users: [...students, ...teachers] }); + const groupWithoutTeachers: Group = groupFactory.build({ users: [...students] }); + roleService.findByName.mockResolvedValueOnce(studentRole).mockResolvedValueOnce(teacherRole); + + return { + course, + group, + students, + teachers, + groupWithoutTeachers, + }; + }; + + it('should save a course with synchronized group, students, and teachers', async () => { + const { course, group, students, teachers } = setup(); + + await service.startSynchronization(course, group); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + syncedWithGroup: group.id, + name: course.name, + startDate: group.validPeriod?.from, + untilDate: group.validPeriod?.until, + studentIds: students.map((student) => student.userId), + teacherIds: teachers.map((teacher) => teacher.userId), + }) + ); + }); + + it('should set an empty list of students if no teachers are present', async () => { + const { course, groupWithoutTeachers } = setup(); + + await service.startSynchronization(course, groupWithoutTeachers); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + syncedWithGroup: groupWithoutTeachers.id, + name: course.name, + startDate: groupWithoutTeachers.validPeriod?.from, + untilDate: groupWithoutTeachers.validPeriod?.until, + studentIds: [], + teacherIds: [], + }) + ); + }); + }); + + describe('when a course is already synchronized with a group', () => { + const setup = () => { + const course: Course = courseFactory.build({ syncedWithGroup: new ObjectId().toHexString() }); + const group: Group = groupFactory.build(); + const students: GroupUser[] = [{ roleId: 'student-role-id', userId: 'student-user-id' }]; + const teachers: GroupUser[] = [{ roleId: 'teacher-role-id', userId: 'teacher-user-id' }]; + + return { + course, + group, + students, + teachers, + }; + }; + + it('should throw a CourseAlreadySynchronizedLoggableException', async () => { + const { course, group } = setup(); + + await expect(service.startSynchronization(course, group)).rejects.toThrow( + CourseAlreadySynchronizedLoggableException + ); + + expect(courseRepo.save).not.toHaveBeenCalled(); + }); + }); + }); + + describe('synchronizeCourseWithGroup', () => { + describe('when synchronizing with a new group', () => { + const setup = () => { + const course: Course = courseFactory.build(); + const studentId: string = new ObjectId().toHexString(); + const teacherId: string = new ObjectId().toHexString(); + const studentRoleId: string = new ObjectId().toHexString(); + const teacherRoleId: string = new ObjectId().toHexString(); + const studentRole: RoleDto = roleDtoFactory.build({ id: studentRoleId }); + const teacherRole: RoleDto = roleDtoFactory.build({ id: teacherRoleId }); + const newGroup: Group = groupFactory.build({ + users: [ + { + userId: studentId, + roleId: studentRoleId, + }, + { + userId: teacherId, + roleId: teacherRoleId, + }, + ], + }); + + courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + + return { + course, + newGroup, + studentId, + teacherId, + }; + }; + + it('should synchronize with the group', async () => { + const { course, newGroup, studentId, teacherId } = setup(); + + await service.synchronizeCourseWithGroup(newGroup); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + name: newGroup.name, + syncedWithGroup: newGroup.id, + startDate: newGroup.validPeriod?.from, + untilDate: newGroup.validPeriod?.until, + studentIds: [studentId], + teacherIds: [teacherId], + }) + ); + }); + }); + + describe('when the course name is the same as the old group name', () => { + const setup = () => { + const course: Course = courseFactory.build({ name: 'Course Name' }); + const studentRole: RoleDto = roleDtoFactory.build(); + const teacherRole: RoleDto = roleDtoFactory.build(); + const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); + const newGroup: Group = groupFactory.build({ + name: 'New Group Name', + users: [], + }); + + courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + + return { + course, + newGroup, + oldGroup, + }; + }; + + it('should synchronize the group name', async () => { + const { course, newGroup, oldGroup } = setup(); + + await service.synchronizeCourseWithGroup(newGroup, oldGroup); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + name: newGroup.name, + syncedWithGroup: newGroup.id, + startDate: newGroup.validPeriod?.from, + untilDate: newGroup.validPeriod?.until, + studentIds: [], + teacherIds: [], + }) + ); + }); + }); + + describe('when the course name is different from the old group name', () => { + const setup = () => { + const course: Course = courseFactory.build({ name: 'Custom Course Name' }); + const studentRole: RoleDto = roleDtoFactory.build(); + const teacherRole: RoleDto = roleDtoFactory.build(); + const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); + const newGroup: Group = groupFactory.build({ + name: 'New Group Name', + users: [], + }); + + courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + + return { + course, + newGroup, + oldGroup, + }; + }; + + it('should keep the old course name', async () => { + const { course, newGroup, oldGroup } = setup(); + + await service.synchronizeCourseWithGroup(newGroup, oldGroup); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + name: course.name, + syncedWithGroup: newGroup.id, + startDate: newGroup.validPeriod?.from, + untilDate: newGroup.validPeriod?.until, + studentIds: [], + teacherIds: [], + }) + ); + }); + }); + + describe('when the last teacher gets removed from a synced group', () => { + const setup = () => { + const studentUserId = new ObjectId().toHexString(); + const teacherUserId = new ObjectId().toHexString(); + const studentRoleId: string = new ObjectId().toHexString(); + const studentRole: RoleDto = roleDtoFactory.build({ id: studentRoleId }); + const teacherRole: RoleDto = roleDtoFactory.build(); + const newGroup: Group = groupFactory.build({ + users: [ + new GroupUser({ + userId: studentUserId, + roleId: studentRoleId, + }), + ], + }); + + const course: Course = courseFactory.build({ + teacherIds: [teacherUserId], + studentIds: [studentUserId], + syncedWithGroup: newGroup.id, + }); + courseRepo.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); + roleService.findByName.mockResolvedValueOnce(studentRole); + roleService.findByName.mockResolvedValueOnce(teacherRole); + + return { + course, + newGroup, + teacherUserId, + }; + }; + + it('should keep the last teacher, remove all students', async () => { + const { course, newGroup, teacherUserId } = setup(); + + await service.synchronizeCourseWithGroup(newGroup, newGroup); + + expect(courseRepo.save).toHaveBeenCalledWith( + new Course({ + ...course.getProps(), + name: course.name, + startDate: newGroup.validPeriod?.from, + untilDate: newGroup.validPeriod?.until, + studentIds: [], + teacherIds: [teacherUserId], + syncedWithGroup: course.syncedWithGroup, + }) + ); + }); + }); + }); +}); diff --git a/apps/server/src/modules/learnroom/service/course-sync.service.ts b/apps/server/src/modules/learnroom/service/course-sync.service.ts new file mode 100644 index 00000000000..10be631edc8 --- /dev/null +++ b/apps/server/src/modules/learnroom/service/course-sync.service.ts @@ -0,0 +1,76 @@ +import { Group, GroupUser } from '@modules/group'; +import { RoleService } from '@modules/role'; +import { Inject, Injectable } from '@nestjs/common'; +import { RoleName } from '@shared/domain/interface'; +import { EntityId } from '@shared/domain/types'; +import { + Course, + COURSE_REPO, + CourseAlreadySynchronizedLoggableException, + CourseNotSynchronizedLoggableException, + CourseRepo, +} from '../domain'; + +@Injectable() +export class CourseSyncService { + constructor( + @Inject(COURSE_REPO) private readonly courseRepo: CourseRepo, + private readonly roleService: RoleService + ) {} + + public async startSynchronization(course: Course, group: Group): Promise { + if (course.syncedWithGroup) { + throw new CourseAlreadySynchronizedLoggableException(course.id); + } + + await this.synchronize(course, group); + } + + public async stopSynchronization(course: Course): Promise { + if (!course.syncedWithGroup) { + throw new CourseNotSynchronizedLoggableException(course.id); + } + + course.syncedWithGroup = undefined; + + await this.courseRepo.save(course); + } + + public async synchronizeCourseWithGroup(newGroup: Group, oldGroup?: Group): Promise { + const courses: Course[] = await this.courseRepo.findBySyncedGroup(newGroup); + + if (courses.length) { + await Promise.all( + courses.map(async (course) => { + await this.synchronize(course, newGroup, oldGroup); + }) + ); + } + } + + private async synchronize(course: Course, group: Group, oldGroup?: Group): Promise { + course.syncedWithGroup = group.id; + if (!oldGroup || oldGroup.name === course.name) { + course.name = group.name; + } + course.startDate = group.validPeriod?.from; + course.untilDate = group.validPeriod?.until; + + const [studentRole, teacherRole] = await Promise.all([ + this.roleService.findByName(RoleName.STUDENT), + this.roleService.findByName(RoleName.TEACHER), + ]); + + const students = group.users.filter((groupUser: GroupUser) => groupUser.roleId === studentRole.id); + const teachers = group.users.filter((groupUser: GroupUser) => groupUser.roleId === teacherRole.id); + + if (teachers.length >= 1) { + course.students = students.map((groupUser: GroupUser): EntityId => groupUser.userId); + course.teachers = teachers.map((groupUser: GroupUser): EntityId => groupUser.userId); + } else { + course.students = []; + } + + await this.courseRepo.save(course); + } +} diff --git a/apps/server/src/modules/learnroom/service/index.ts b/apps/server/src/modules/learnroom/service/index.ts index 61cc6b2d068..b91fcdc556f 100644 --- a/apps/server/src/modules/learnroom/service/index.ts +++ b/apps/server/src/modules/learnroom/service/index.ts @@ -2,9 +2,10 @@ export * from './board-copy.service'; export * from './common-cartridge-export.service'; export * from './common-cartridge-import.service'; export * from './course-copy.service'; -export * from './course.service'; export { CourseDoService } from './course-do.service'; +export * from './course-rooms.service'; +export { CourseSyncService } from './course-sync.service'; +export * from './course.service'; export * from './coursegroup.service'; export * from './dashboard.service'; -export * from './course-rooms.service'; export { GroupDeletedHandlerService } from './group-deleted-handler.service'; diff --git a/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts b/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts index ad386434cd5..fa0cdda3b27 100644 --- a/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts +++ b/apps/server/src/modules/learnroom/uc/course-sync.uc.spec.ts @@ -1,11 +1,11 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; import { GroupService } from '@modules/group'; -import { RoleService } from '@modules/role'; import { Test, TestingModule } from '@nestjs/testing'; -import { Permission, RoleName } from '@shared/domain/interface'; -import { groupFactory, roleDtoFactory, setupEntities, userFactory } from '@shared/testing'; +import { Permission } from '@shared/domain/interface'; +import { groupFactory, setupEntities, userFactory } from '@shared/testing'; import { CourseDoService } from '../service'; +import { CourseSyncService } from '../service/course-sync.service'; import { courseFactory } from '../testing'; import { CourseSyncUc } from './course-sync.uc'; @@ -16,7 +16,7 @@ describe(CourseSyncUc.name, () => { let authorizationService: DeepMocked; let courseService: DeepMocked; let groupService: DeepMocked; - let roleService: DeepMocked; + let courseSyncService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -31,12 +31,12 @@ describe(CourseSyncUc.name, () => { useValue: createMock(), }, { - provide: GroupService, - useValue: createMock(), + provide: CourseSyncService, + useValue: createMock(), }, { - provide: RoleService, - useValue: createMock(), + provide: GroupService, + useValue: createMock(), }, ], }).compile(); @@ -45,7 +45,8 @@ describe(CourseSyncUc.name, () => { authorizationService = module.get(AuthorizationService); courseService = module.get(CourseDoService); groupService = module.get(GroupService); - roleService = module.get(RoleService); + courseSyncService = module.get(CourseSyncService); + await setupEntities(); }); @@ -89,7 +90,7 @@ describe(CourseSyncUc.name, () => { await uc.stopSynchronization(user.id, course.id); - expect(courseService.stopSynchronization).toHaveBeenCalledWith(course); + expect(courseSyncService.stopSynchronization).toHaveBeenCalledWith(course); }); }); }); @@ -100,30 +101,15 @@ describe(CourseSyncUc.name, () => { const user = userFactory.buildWithId(); const course = courseFactory.build(); const group = groupFactory.build(); - const studentRole = roleDtoFactory.build({ id: 'student-role-id' }); - const teacherRole = roleDtoFactory.build({ id: 'teacher-role-id' }); - - group.users = [ - { roleId: 'student-role-id', userId: 'student-user-id' }, - { roleId: 'teacher-role-id', userId: 'teacher-user-id' }, - ]; - - const students = group.users.filter((groupUser) => groupUser.roleId === studentRole.id); - const teachers = group.users.filter((groupUser) => groupUser.roleId === teacherRole.id); courseService.findById.mockResolvedValueOnce(course); groupService.findById.mockResolvedValueOnce(group); authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); - roleService.findByName.mockResolvedValueOnce(studentRole).mockResolvedValueOnce(teacherRole); return { user, course, group, - studentRole, - teacherRole, - students, - teachers, }; }; @@ -132,8 +118,6 @@ describe(CourseSyncUc.name, () => { await uc.startSynchronization(user.id, course.id, group.id); - expect(courseService.findById).toHaveBeenCalledWith(course.id); - expect(groupService.findById).toHaveBeenCalledWith(group.id); expect(authorizationService.checkPermission).toHaveBeenCalledWith( user, course, @@ -141,18 +125,28 @@ describe(CourseSyncUc.name, () => { ); }); - it('should start the synchronization with correct roles', async () => { - const { user, course, group, students, teachers } = setup(); + it('should call course do service with the correct course id', async () => { + const { user, course, group } = setup(); await uc.startSynchronization(user.id, course.id, group.id); expect(courseService.findById).toHaveBeenCalledWith(course.id); + }); + + it('should call group service with the correct group id', async () => { + const { user, course, group } = setup(); + + await uc.startSynchronization(user.id, course.id, group.id); + expect(groupService.findById).toHaveBeenCalledWith(group.id); + }); - expect(roleService.findByName).toHaveBeenCalledWith(RoleName.STUDENT); - expect(roleService.findByName).toHaveBeenCalledWith(RoleName.TEACHER); + it('should start the synchronization', async () => { + const { user, course, group } = setup(); + + await uc.startSynchronization(user.id, course.id, group.id); - expect(courseService.startSynchronization).toHaveBeenCalledWith(course, group, students, teachers); + expect(courseSyncService.startSynchronization).toHaveBeenCalledWith(course, group); }); }); }); diff --git a/apps/server/src/modules/learnroom/uc/course-sync.uc.ts b/apps/server/src/modules/learnroom/uc/course-sync.uc.ts index 770268fe882..4e018e3bedb 100644 --- a/apps/server/src/modules/learnroom/uc/course-sync.uc.ts +++ b/apps/server/src/modules/learnroom/uc/course-sync.uc.ts @@ -1,34 +1,32 @@ import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; -import { GroupService, GroupUser } from '@modules/group'; -import { RoleService } from '@modules/role'; +import { GroupService } from '@modules/group'; import { Injectable } from '@nestjs/common'; - -import { type User as UserEntity } from '@shared/domain/entity'; -import { Permission, RoleName } from '@shared/domain/interface'; +import { Permission } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; -import { Course } from '../domain'; -import { CourseDoService } from '../service'; +import { CourseDoService, CourseSyncService } from '../service'; @Injectable() export class CourseSyncUc { constructor( private readonly authorizationService: AuthorizationService, private readonly courseService: CourseDoService, - private readonly groupService: GroupService, - private readonly roleService: RoleService + private readonly courseSyncService: CourseSyncService, + private readonly groupService: GroupService ) {} public async stopSynchronization(userId: EntityId, courseId: EntityId): Promise { - const course: Course = await this.courseService.findById(courseId); + const [course, user] = await Promise.all([ + this.courseService.findById(courseId), + this.authorizationService.getUserWithPermissions(userId), + ]); - const user: UserEntity = await this.authorizationService.getUserWithPermissions(userId); this.authorizationService.checkPermission( user, course, AuthorizationContextBuilder.write([Permission.COURSE_EDIT]) ); - await this.courseService.stopSynchronization(course); + await this.courseSyncService.stopSynchronization(course); } public async startSynchronization(userId: string, courseId: string, groupId: string) { @@ -44,14 +42,6 @@ export class CourseSyncUc { AuthorizationContextBuilder.write([Permission.COURSE_EDIT]) ); - const [studentRole, teacherRole] = await Promise.all([ - this.roleService.findByName(RoleName.STUDENT), - this.roleService.findByName(RoleName.TEACHER), - ]); - - const students = group.users.filter((groupUser: GroupUser) => groupUser.roleId === studentRole.id); - const teachers = group.users.filter((groupUser: GroupUser) => groupUser.roleId === teacherRole.id); - - await this.courseService.startSynchronization(course, group, students, teachers); + await this.courseSyncService.startSynchronization(course, group); } } diff --git a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts index b83a48f9d27..9ef64798b77 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.spec.ts @@ -1,39 +1,28 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { ObjectId } from '@mikro-orm/mongodb'; -import { Group, GroupUser } from '@modules/group'; -import { CourseDoService } from '@modules/learnroom'; -import { Course } from '@modules/learnroom/domain'; -import { courseFactory } from '@modules/learnroom/testing'; -import { RoleDto, RoleService } from '@modules/role'; +import { Group } from '@modules/group'; +import { CourseSyncService } from '@modules/learnroom'; import { Test, TestingModule } from '@nestjs/testing'; -import { groupFactory, roleDtoFactory } from '@shared/testing'; +import { groupFactory } from '@shared/testing'; import { SchulconnexCourseSyncService } from './schulconnex-course-sync.service'; describe(SchulconnexCourseSyncService.name, () => { let module: TestingModule; let service: SchulconnexCourseSyncService; - - let courseService: DeepMocked; - let roleService: DeepMocked; + let courseSyncService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ providers: [ SchulconnexCourseSyncService, { - provide: CourseDoService, - useValue: createMock(), - }, - { - provide: RoleService, - useValue: createMock(), + provide: CourseSyncService, + useValue: createMock(), }, ], }).compile(); service = module.get(SchulconnexCourseSyncService); - courseService = module.get(CourseDoService); - roleService = module.get(RoleService); + courseSyncService = module.get(CourseSyncService); }); afterAll(async () => { @@ -47,184 +36,39 @@ describe(SchulconnexCourseSyncService.name, () => { describe('synchronizeCourseWithGroup', () => { describe('when synchronizing with a new group', () => { const setup = () => { - const course: Course = courseFactory.build(); - const studentId: string = new ObjectId().toHexString(); - const teacherId: string = new ObjectId().toHexString(); - const studentRoleId: string = new ObjectId().toHexString(); - const teacherRoleId: string = new ObjectId().toHexString(); - const studentRole: RoleDto = roleDtoFactory.build({ id: studentRoleId }); - const teacherRole: RoleDto = roleDtoFactory.build({ id: teacherRoleId }); - const newGroup: Group = groupFactory.build({ - users: [ - { - userId: studentId, - roleId: studentRoleId, - }, - { - userId: teacherId, - roleId: teacherRoleId, - }, - ], - }); - - courseService.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); - roleService.findByName.mockResolvedValueOnce(studentRole); - roleService.findByName.mockResolvedValueOnce(teacherRole); + const newGroup: Group = groupFactory.build(); return { - course, newGroup, - studentId, - teacherId, }; }; it('should synchronize with the group', async () => { - const { course, newGroup, studentId, teacherId } = setup(); + const { newGroup } = setup(); await service.synchronizeCourseWithGroup(newGroup); - expect(courseService.saveAll).toHaveBeenCalledWith<[Course[]]>([ - new Course({ - ...course.getProps(), - name: newGroup.name, - startDate: newGroup.validPeriod?.from, - untilDate: newGroup.validPeriod?.until, - studentIds: [studentId], - teacherIds: [teacherId], - }), - ]); - }); - }); - - describe('when the course name is the same as the old group name', () => { - const setup = () => { - const course: Course = courseFactory.build({ name: 'Course Name' }); - const studentRole: RoleDto = roleDtoFactory.build(); - const teacherRole: RoleDto = roleDtoFactory.build(); - const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); - const newGroup: Group = groupFactory.build({ - name: 'New Group Name', - users: [], - }); - - courseService.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); - roleService.findByName.mockResolvedValueOnce(studentRole); - roleService.findByName.mockResolvedValueOnce(teacherRole); - - return { - course, - newGroup, - oldGroup, - }; - }; - - it('should synchronize the group name', async () => { - const { course, newGroup, oldGroup } = setup(); - - await service.synchronizeCourseWithGroup(newGroup, oldGroup); - - expect(courseService.saveAll).toHaveBeenCalledWith<[Course[]]>([ - new Course({ - ...course.getProps(), - name: newGroup.name, - startDate: newGroup.validPeriod?.from, - untilDate: newGroup.validPeriod?.until, - studentIds: [], - teacherIds: [], - }), - ]); + expect(courseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(newGroup, undefined); }); }); - describe('when the course name is different from the old group name', () => { + describe('when synchronizing with a new group and an old group', () => { const setup = () => { - const course: Course = courseFactory.build({ name: 'Custom Course Name' }); - const studentRole: RoleDto = roleDtoFactory.build(); - const teacherRole: RoleDto = roleDtoFactory.build(); - const oldGroup: Group = groupFactory.build({ name: 'Course Name' }); - const newGroup: Group = groupFactory.build({ - name: 'New Group Name', - users: [], - }); - - courseService.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); - roleService.findByName.mockResolvedValueOnce(studentRole); - roleService.findByName.mockResolvedValueOnce(teacherRole); + const newGroup: Group = groupFactory.build(); + const oldGroup: Group = groupFactory.build(); return { - course, newGroup, oldGroup, }; }; - it('should keep the old course name', async () => { - const { course, newGroup, oldGroup } = setup(); + it('should synchronize with the group', async () => { + const { newGroup, oldGroup } = setup(); await service.synchronizeCourseWithGroup(newGroup, oldGroup); - expect(courseService.saveAll).toHaveBeenCalledWith<[Course[]]>([ - new Course({ - ...course.getProps(), - name: course.name, - startDate: newGroup.validPeriod?.from, - untilDate: newGroup.validPeriod?.until, - studentIds: [], - teacherIds: [], - }), - ]); - }); - }); - - describe('when the last teacher gets removed from a synced group', () => { - const setup = () => { - const studentUserId = new ObjectId().toHexString(); - const teacherUserId = new ObjectId().toHexString(); - const course: Course = courseFactory.build({ - teacherIds: [teacherUserId], - studentIds: [studentUserId], - syncedWithGroup: new ObjectId().toHexString(), - }); - const studentRoleId: string = new ObjectId().toHexString(); - const studentRole: RoleDto = roleDtoFactory.build({ id: studentRoleId }); - const teacherRole: RoleDto = roleDtoFactory.build(); - const newGroup: Group = groupFactory.build({ - users: [ - new GroupUser({ - userId: studentUserId, - roleId: studentRoleId, - }), - ], - }); - - courseService.findBySyncedGroup.mockResolvedValueOnce([new Course(course.getProps())]); - roleService.findByName.mockResolvedValueOnce(studentRole); - roleService.findByName.mockResolvedValueOnce(teacherRole); - - return { - course, - newGroup, - teacherUserId, - }; - }; - - it('should keep the last teacher, remove all students', async () => { - const { course, newGroup, teacherUserId } = setup(); - - await service.synchronizeCourseWithGroup(newGroup, newGroup); - - expect(courseService.saveAll).toHaveBeenCalledWith<[Course[]]>([ - new Course({ - ...course.getProps(), - name: course.name, - startDate: newGroup.validPeriod?.from, - untilDate: newGroup.validPeriod?.until, - studentIds: [], - teacherIds: [teacherUserId], - syncedWithGroup: course.syncedWithGroup, - }), - ]); + expect(courseSyncService.synchronizeCourseWithGroup).toHaveBeenCalledWith(newGroup, oldGroup); }); }); }); diff --git a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts index c62fe52cb81..9f93d76325e 100644 --- a/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts +++ b/apps/server/src/modules/provisioning/strategy/oidc/service/schulconnex-course-sync.service.ts @@ -1,46 +1,12 @@ -import { Group, GroupUser } from '@modules/group'; -import { CourseDoService } from '@modules/learnroom'; -import { RoleDto, RoleService } from '@modules/role'; +import { Group } from '@modules/group'; +import { CourseSyncService } from '@modules/learnroom'; import { Injectable } from '@nestjs/common'; -import { RoleName } from '@shared/domain/interface'; -import { EntityId } from '@shared/domain/types'; -import { Course } from '@src/modules/learnroom/domain'; @Injectable() export class SchulconnexCourseSyncService { - constructor(private readonly courseService: CourseDoService, private readonly roleService: RoleService) {} + constructor(private readonly courseSyncService: CourseSyncService) {} async synchronizeCourseWithGroup(newGroup: Group, oldGroup?: Group): Promise { - const courses: Course[] = await this.courseService.findBySyncedGroup(newGroup); - - if (courses.length) { - const studentRole: RoleDto = await this.roleService.findByName(RoleName.STUDENT); - const teacherRole: RoleDto = await this.roleService.findByName(RoleName.TEACHER); - - courses.forEach((course: Course): void => { - if (!oldGroup || oldGroup.name === course.name) { - course.name = newGroup.name; - } - - course.startDate = newGroup.validPeriod?.from; - course.untilDate = newGroup.validPeriod?.until; - - const students: GroupUser[] = newGroup.users.filter( - (user: GroupUser): boolean => user.roleId === studentRole.id - ); - const teachers: GroupUser[] = newGroup.users.filter( - (user: GroupUser): boolean => user.roleId === teacherRole.id - ); - - if (teachers.length >= 1) { - course.students = students.map((user: GroupUser): EntityId => user.userId); - course.teachers = teachers.map((user: GroupUser): EntityId => user.userId); - } else { - course.students = []; - } - }); - - await this.courseService.saveAll(courses); - } + await this.courseSyncService.synchronizeCourseWithGroup(newGroup, oldGroup); } }