From 46788a4dff6f1f7aa7ed377efe9b2cc8cc1f087c Mon Sep 17 00:00:00 2001 From: mamutmk5 <3045922+mamutmk5@users.noreply.github.com> Date: Thu, 2 Nov 2023 16:59:41 +0100 Subject: [PATCH 1/3] BC-5705 - Remove fwu ingress (#4515) --- .../schulcloud-server-core/tasks/main.yml | 8 ++-- .../templates/api-fwu-ingress.yml.j2 | 41 ------------------- 2 files changed, 5 insertions(+), 44 deletions(-) delete mode 100644 ansible/roles/schulcloud-server-core/templates/api-fwu-ingress.yml.j2 diff --git a/ansible/roles/schulcloud-server-core/tasks/main.yml b/ansible/roles/schulcloud-server-core/tasks/main.yml index 1b58c8a5413..64e257c5f59 100644 --- a/ansible/roles/schulcloud-server-core/tasks/main.yml +++ b/ansible/roles/schulcloud-server-core/tasks/main.yml @@ -92,12 +92,14 @@ template: api-fwu-deployment.yml.j2 when: FEATURE_FWU_CONTENT_ENABLED is defined and FEATURE_FWU_CONTENT_ENABLED|bool - - name: Fwu Learning Contents Ingress + - name: Fwu Learning Contents Ingress Remove kubernetes.core.k8s: kubeconfig: ~/.kube/config namespace: "{{ NAMESPACE }}" - template: api-fwu-ingress.yml.j2 - apply: yes + state: absent + api_version: networking.k8s.io/v1 + kind: Ingress + name: "{{ NAMESPACE }}-api-fwu-ingress" when: FEATURE_FWU_CONTENT_ENABLED is defined and FEATURE_FWU_CONTENT_ENABLED|bool - name: Delete Files CronJob diff --git a/ansible/roles/schulcloud-server-core/templates/api-fwu-ingress.yml.j2 b/ansible/roles/schulcloud-server-core/templates/api-fwu-ingress.yml.j2 deleted file mode 100644 index f42c322e45b..00000000000 --- a/ansible/roles/schulcloud-server-core/templates/api-fwu-ingress.yml.j2 +++ /dev/null @@ -1,41 +0,0 @@ -apiVersion: networking.k8s.io/v1 -kind: Ingress -metadata: - name: {{ NAMESPACE }}-api-fwu-ingress - namespace: {{ NAMESPACE }} - annotations: - nginx.ingress.kubernetes.io/ssl-redirect: "{{ TLS_ENABELD|default("false") }}" - nginx.ingress.kubernetes.io/proxy-body-size: "{{ INGRESS_MAX_BODY_SIZE|default("2560") }}m" - nginx.org/client-max-body-size: "{{ INGRESS_MAX_BODY_SIZE|default("2560") }}m" - # The following properties added with BC-3606. - # The header size of the request is too big. For e.g. state and the permanent growing jwt. - # Nginx throws away the Location header, resulting in the 502 Bad Gateway. - nginx.ingress.kubernetes.io/client-header-buffer-size: 100k - nginx.ingress.kubernetes.io/http2-max-header-size: 96k - nginx.ingress.kubernetes.io/large-client-header-buffers: 4 100k - nginx.ingress.kubernetes.io/proxy-buffer-size: 96k -{% if CLUSTER_ISSUER is defined %} - cert-manager.io/cluster-issuer: {{ CLUSTER_ISSUER }} -{% endif %} - -spec: - ingressClassName: nginx -{% if CLUSTER_ISSUER is defined or (TLS_ENABELD is defined and TLS_ENABELD|bool) %} - tls: - - hosts: - - {{ DOMAIN }} -{% if CLUSTER_ISSUER is defined %} - secretName: {{ DOMAIN }}-tls -{% endif %} -{% endif %} - rules: - - host: {{ DOMAIN }} - http: - paths: - - path: /api/v3/fwu/ - backend: - service: - name: api-fwu-svc - port: - number: {{ PORT_FWU_LEARNING_CONTENTS }} - pathType: Prefix From 0f9dee5a3196425623fc3b881c605e311df821ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= <103562092+MarvinOehlerkingCap@users.noreply.github.com> Date: Fri, 3 Nov 2023 07:21:07 +0100 Subject: [PATCH 2/3] N21-1296 Delete ContextExternalTool when deleting a ExternalToolElement on boards (#4507) --- apps/server/src/modules/board/board.module.ts | 10 +++--- .../modules/board/repo/board-do.repo.spec.ts | 4 ++- .../repo/recursive-delete.visitor.spec.ts | 31 ++++++++++++++++--- .../board/repo/recursive-delete.vistor.ts | 16 ++++++++-- .../src/modules/pseudonym/pseudonym.module.ts | 9 +++--- apps/server/src/modules/task/task.module.ts | 7 ++--- 6 files changed, 55 insertions(+), 22 deletions(-) diff --git a/apps/server/src/modules/board/board.module.ts b/apps/server/src/modules/board/board.module.ts index fb04364b6c3..09d00c46bbd 100644 --- a/apps/server/src/modules/board/board.module.ts +++ b/apps/server/src/modules/board/board.module.ts @@ -1,12 +1,12 @@ +import { FilesStorageClientModule } from '@modules/files-storage-client'; +import { ContextExternalToolModule } from '@modules/tool/context-external-tool'; +import { UserModule } from '@modules/user'; import { Module } from '@nestjs/common'; import { ContentElementFactory } from '@shared/domain'; import { ConsoleWriterModule } from '@shared/infra/console'; import { CourseRepo } from '@shared/repo'; import { LoggerModule } from '@src/core/logger'; -import { FilesStorageClientModule } from '../files-storage-client'; -import { UserModule } from '../user'; -import { BoardDoRepo, BoardNodeRepo } from './repo'; -import { RecursiveDeleteVisitor } from './repo/recursive-delete.vistor'; +import { BoardDoRepo, BoardNodeRepo, RecursiveDeleteVisitor } from './repo'; import { BoardDoAuthorizableService, BoardDoService, @@ -21,7 +21,7 @@ import { BoardDoCopyService, SchoolSpecificFileCopyServiceFactory } from './serv import { ColumnBoardCopyService } from './service/column-board-copy.service'; @Module({ - imports: [ConsoleWriterModule, FilesStorageClientModule, LoggerModule, UserModule], + imports: [ConsoleWriterModule, FilesStorageClientModule, LoggerModule, UserModule, ContextExternalToolModule], providers: [ BoardDoAuthorizableService, BoardDoRepo, diff --git a/apps/server/src/modules/board/repo/board-do.repo.spec.ts b/apps/server/src/modules/board/repo/board-do.repo.spec.ts index aa1c49224fe..3874e9301ba 100644 --- a/apps/server/src/modules/board/repo/board-do.repo.spec.ts +++ b/apps/server/src/modules/board/repo/board-do.repo.spec.ts @@ -1,6 +1,8 @@ import { createMock } from '@golevelup/ts-jest'; import { NotFoundError } from '@mikro-orm/core'; import { EntityManager } from '@mikro-orm/mongodb'; +import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; +import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; import { NotFoundException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { @@ -26,7 +28,6 @@ import { richTextElementFactory, richTextElementNodeFactory, } from '@shared/testing'; -import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; import { BoardDoRepo } from './board-do.repo'; import { BoardNodeRepo } from './board-node.repo'; import { RecursiveDeleteVisitor } from './recursive-delete.vistor'; @@ -46,6 +47,7 @@ describe(BoardDoRepo.name, () => { BoardNodeRepo, RecursiveDeleteVisitor, { provide: FilesStorageClientAdapterService, useValue: createMock() }, + { provide: ContextExternalToolService, useValue: createMock() }, ], }).compile(); repo = module.get(BoardDoRepo); diff --git a/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts b/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts index 9142cb33553..6236d5de8bb 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.visitor.spec.ts @@ -1,10 +1,13 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { EntityManager } from '@mikro-orm/mongodb'; +import { FileDto, FilesStorageClientAdapterService } from '@modules/files-storage-client'; +import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; import { Test, TestingModule } from '@nestjs/testing'; import { FileRecordParentType } from '@shared/infra/rabbitmq'; import { columnBoardFactory, columnFactory, + contextExternalToolFactory, externalToolElementFactory, fileElementFactory, linkElementFactory, @@ -12,14 +15,15 @@ import { submissionContainerElementFactory, submissionItemFactory, } from '@shared/testing'; -import { FileDto, FilesStorageClientAdapterService } from '@modules/files-storage-client'; import { RecursiveDeleteVisitor } from './recursive-delete.vistor'; describe(RecursiveDeleteVisitor.name, () => { let module: TestingModule; + let service: RecursiveDeleteVisitor; + let em: DeepMocked; let filesStorageClientAdapterService: DeepMocked; - let service: RecursiveDeleteVisitor; + let contextExternalToolService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -27,12 +31,15 @@ describe(RecursiveDeleteVisitor.name, () => { RecursiveDeleteVisitor, { provide: EntityManager, useValue: createMock() }, { provide: FilesStorageClientAdapterService, useValue: createMock() }, + { provide: ContextExternalToolService, useValue: createMock() }, ], }).compile(); + service = module.get(RecursiveDeleteVisitor); em = module.get(EntityManager); filesStorageClientAdapterService = module.get(FilesStorageClientAdapterService); - service = module.get(RecursiveDeleteVisitor); + contextExternalToolService = module.get(ContextExternalToolService); + await setupEntities(); }); @@ -212,14 +219,30 @@ describe(RecursiveDeleteVisitor.name, () => { describe('visitExternalToolElementAsync', () => { const setup = () => { + const contextExternalTool = contextExternalToolFactory.buildWithId(); const childExternalToolElement = externalToolElementFactory.build(); const externalToolElement = externalToolElementFactory.build({ children: [childExternalToolElement], + contextExternalToolId: contextExternalTool.id, }); - return { externalToolElement, childExternalToolElement }; + contextExternalToolService.findById.mockResolvedValue(contextExternalTool); + + return { + externalToolElement, + childExternalToolElement, + contextExternalTool, + }; }; + it('should delete the context external tool that is linked to the element', async () => { + const { externalToolElement, contextExternalTool } = setup(); + + await service.visitExternalToolElementAsync(externalToolElement); + + expect(contextExternalToolService.deleteContextExternalTool).toHaveBeenCalledWith(contextExternalTool); + }); + it('should call entity remove', async () => { const { externalToolElement, childExternalToolElement } = setup(); diff --git a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts index 1c407391da4..0a1b08e663a 100644 --- a/apps/server/src/modules/board/repo/recursive-delete.vistor.ts +++ b/apps/server/src/modules/board/repo/recursive-delete.vistor.ts @@ -1,4 +1,7 @@ import { EntityManager } from '@mikro-orm/mongodb'; +import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; +import { ContextExternalTool } from '@modules/tool/context-external-tool/domain'; +import { ContextExternalToolService } from '@modules/tool/context-external-tool/service'; import { Injectable } from '@nestjs/common'; import { AnyBoardDo, @@ -14,13 +17,13 @@ import { SubmissionItem, } from '@shared/domain'; import { LinkElement } from '@shared/domain/domainobject/board/link-element.do'; -import { FilesStorageClientAdapterService } from '@modules/files-storage-client'; @Injectable() export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { constructor( private readonly em: EntityManager, - private readonly filesStorageClientAdapterService: FilesStorageClientAdapterService + private readonly filesStorageClientAdapterService: FilesStorageClientAdapterService, + private readonly contextExternalToolService: ContextExternalToolService ) {} async visitColumnBoardAsync(columnBoard: ColumnBoard): Promise { @@ -67,7 +70,14 @@ export class RecursiveDeleteVisitor implements BoardCompositeVisitorAsync { } async visitExternalToolElementAsync(externalToolElement: ExternalToolElement): Promise { - // TODO N21-1296: Delete linked ContextExternalTool + if (externalToolElement.contextExternalToolId) { + const linkedTool: ContextExternalTool = await this.contextExternalToolService.findById( + externalToolElement.contextExternalToolId + ); + + await this.contextExternalToolService.deleteContextExternalTool(linkedTool); + } + this.deleteNode(externalToolElement); await this.visitChildrenAsync(externalToolElement); diff --git a/apps/server/src/modules/pseudonym/pseudonym.module.ts b/apps/server/src/modules/pseudonym/pseudonym.module.ts index d282c5dd9fe..3a8bcdacbd1 100644 --- a/apps/server/src/modules/pseudonym/pseudonym.module.ts +++ b/apps/server/src/modules/pseudonym/pseudonym.module.ts @@ -1,14 +1,13 @@ -import { forwardRef, Module } from '@nestjs/common'; -import { LegacyLogger } from '@src/core/logger'; import { LearnroomModule } from '@modules/learnroom'; -import { UserModule } from '@modules/user'; import { ToolModule } from '@modules/tool'; -import { AuthorizationModule } from '@modules/authorization'; +import { UserModule } from '@modules/user'; +import { forwardRef, Module } from '@nestjs/common'; +import { LegacyLogger } from '@src/core/logger'; import { ExternalToolPseudonymRepo, PseudonymsRepo } from './repo'; import { FeathersRosterService, PseudonymService } from './service'; @Module({ - imports: [UserModule, LearnroomModule, forwardRef(() => ToolModule), forwardRef(() => AuthorizationModule)], + imports: [UserModule, LearnroomModule, forwardRef(() => ToolModule)], providers: [PseudonymService, PseudonymsRepo, ExternalToolPseudonymRepo, LegacyLogger, FeathersRosterService], exports: [PseudonymService, FeathersRosterService], }) diff --git a/apps/server/src/modules/task/task.module.ts b/apps/server/src/modules/task/task.module.ts index 696d608d0a3..45a0fdb720a 100644 --- a/apps/server/src/modules/task/task.module.ts +++ b/apps/server/src/modules/task/task.module.ts @@ -1,12 +1,11 @@ -import { forwardRef, Module } from '@nestjs/common'; -import { CourseRepo, LessonRepo, SubmissionRepo, TaskRepo } from '@shared/repo'; -import { AuthorizationModule } from '@modules/authorization'; import { CopyHelperModule } from '@modules/copy-helper'; import { FilesStorageClientModule } from '@modules/files-storage-client'; +import { Module } from '@nestjs/common'; +import { CourseRepo, LessonRepo, SubmissionRepo, TaskRepo } from '@shared/repo'; import { SubmissionService, TaskCopyService, TaskService } from './service'; @Module({ - imports: [forwardRef(() => AuthorizationModule), FilesStorageClientModule, CopyHelperModule], + imports: [FilesStorageClientModule, CopyHelperModule], providers: [TaskService, TaskCopyService, SubmissionService, TaskRepo, LessonRepo, CourseRepo, SubmissionRepo], exports: [TaskService, TaskCopyService, SubmissionService], }) From f4e73cd1520caf08beab6ec0b83ed06228982f8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marvin=20=C3=96hlerking?= <103562092+MarvinOehlerkingCap@users.noreply.github.com> Date: Fri, 3 Nov 2023 07:43:55 +0100 Subject: [PATCH 3/3] N21-1397 Restrict access to combined class list for teachers (#4506) --- .../src/modules/class/domain/testing/index.ts | 1 + .../src/modules/class/entity/testing/index.ts | 1 + .../modules/class/repo/classes.repo.spec.ts | 38 ++- .../src/modules/class/repo/classes.repo.ts | 32 +- .../class/repo/mapper/class.mapper.spec.ts | 2 +- .../modules/class/repo/mapper/class.mapper.ts | 2 +- .../class/service/class.service.spec.ts | 29 +- .../modules/class/service/class.service.ts | 7 + .../controller/api-test/group.api.spec.ts | 29 +- .../group/controller/group.controller.ts | 8 +- .../mapper/group-response.mapper.ts | 2 +- .../modules/group/uc/dto/class-info.dto.ts | 4 +- .../src/modules/group/uc/group.uc.spec.ts | 300 ++++++++++++++++-- apps/server/src/modules/group/uc/group.uc.ts | 101 ++++-- .../group/uc/mapper/group-uc.mapper.ts | 6 +- .../domain/interface/permission.enum.ts | 1 + .../shared/testing/user-role-permissions.ts | 1 + backup/setup/migrations.json | 11 + backup/setup/roles.json | 6 +- ...5587322-add-group-full-admin-permission.js | 74 +++++ 20 files changed, 554 insertions(+), 101 deletions(-) create mode 100644 apps/server/src/modules/class/domain/testing/index.ts create mode 100644 apps/server/src/modules/class/entity/testing/index.ts create mode 100644 migrations/1698325587322-add-group-full-admin-permission.js diff --git a/apps/server/src/modules/class/domain/testing/index.ts b/apps/server/src/modules/class/domain/testing/index.ts new file mode 100644 index 00000000000..3c5809ece1b --- /dev/null +++ b/apps/server/src/modules/class/domain/testing/index.ts @@ -0,0 +1 @@ +export * from './factory/class.factory'; diff --git a/apps/server/src/modules/class/entity/testing/index.ts b/apps/server/src/modules/class/entity/testing/index.ts new file mode 100644 index 00000000000..45893909755 --- /dev/null +++ b/apps/server/src/modules/class/entity/testing/index.ts @@ -0,0 +1 @@ +export * from './factory/class.entity.factory'; diff --git a/apps/server/src/modules/class/repo/classes.repo.spec.ts b/apps/server/src/modules/class/repo/classes.repo.spec.ts index 302a8f2de8e..7801045aff0 100644 --- a/apps/server/src/modules/class/repo/classes.repo.spec.ts +++ b/apps/server/src/modules/class/repo/classes.repo.spec.ts @@ -1,10 +1,11 @@ import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; +import { classEntityFactory } from '@modules/class/entity/testing/factory/class.entity.factory'; import { Test } from '@nestjs/testing'; import { TestingModule } from '@nestjs/testing/testing-module'; +import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { SchoolEntity } from '@shared/domain'; import { MongoMemoryDatabaseModule } from '@shared/infra/database'; import { cleanupCollections, schoolFactory } from '@shared/testing'; -import { classEntityFactory } from '@modules/class/entity/testing/factory/class.entity.factory'; import { Class } from '../domain'; import { ClassEntity } from '../entity'; import { ClassesRepo } from './classes.repo'; @@ -48,6 +49,7 @@ describe(ClassesRepo.name, () => { const classes: ClassEntity[] = classEntityFactory.buildListWithId(3, { schoolId: school.id }); await em.persistAndFlush(classes); + em.clear(); return { school, @@ -78,9 +80,13 @@ describe(ClassesRepo.name, () => { const setup = async () => { const testUser = new ObjectId(); const class1: ClassEntity = classEntityFactory.withUserIds([testUser, new ObjectId()]).buildWithId(); - const class2: ClassEntity = classEntityFactory.withUserIds([testUser, new ObjectId()]).buildWithId(); + const class2: ClassEntity = classEntityFactory + .withUserIds([new ObjectId()]) + .buildWithId({ teacherIds: [testUser] }); const class3: ClassEntity = classEntityFactory.withUserIds([new ObjectId(), new ObjectId()]).buildWithId(); + await em.persistAndFlush([class1, class2, class3]); + em.clear(); return { class1, @@ -104,7 +110,7 @@ describe(ClassesRepo.name, () => { }); describe('updateMany', () => { - describe('When deleting user data from classes', () => { + describe('when deleting user data from classes', () => { const setup = async () => { const testUser1 = new ObjectId(); const testUser2 = new ObjectId(); @@ -112,7 +118,9 @@ describe(ClassesRepo.name, () => { const class1: ClassEntity = classEntityFactory.withUserIds([testUser1, testUser2]).buildWithId(); const class2: ClassEntity = classEntityFactory.withUserIds([testUser1, testUser3]).buildWithId(); const class3: ClassEntity = classEntityFactory.withUserIds([testUser2, testUser3]).buildWithId(); + await em.persistAndFlush([class1, class2, class3]); + em.clear(); return { class1, @@ -144,5 +152,29 @@ describe(ClassesRepo.name, () => { expect(result3).toHaveLength(2); }); }); + + describe('when updating a class that does not exist', () => { + const setup = async () => { + const class1: ClassEntity = classEntityFactory.buildWithId(); + const class2: ClassEntity = classEntityFactory.buildWithId(); + + await em.persistAndFlush([class1]); + em.clear(); + + return { + class1, + class2, + }; + }; + + it('should throw an error', async () => { + const { class1, class2 } = await setup(); + + const updatedArray: ClassEntity[] = [class1, class2]; + const domainObjectsArray: Class[] = ClassMapper.mapToDOs(updatedArray); + + await expect(repo.updateMany(domainObjectsArray)).rejects.toThrow(NotFoundLoggableException); + }); + }); }); }); diff --git a/apps/server/src/modules/class/repo/classes.repo.ts b/apps/server/src/modules/class/repo/classes.repo.ts index 378b3de9716..24300bbe673 100644 --- a/apps/server/src/modules/class/repo/classes.repo.ts +++ b/apps/server/src/modules/class/repo/classes.repo.ts @@ -1,5 +1,6 @@ import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; import { Injectable } from '@nestjs/common'; +import { NotFoundLoggableException } from '@shared/common/loggable-exception'; import { EntityId } from '@shared/domain'; import { Class } from '../domain'; import { ClassEntity } from '../entity'; @@ -18,7 +19,9 @@ export class ClassesRepo { } async findAllByUserId(userId: EntityId): Promise { - const classes: ClassEntity[] = await this.em.find(ClassEntity, { userIds: new ObjectId(userId) }); + const classes: ClassEntity[] = await this.em.find(ClassEntity, { + $or: [{ userIds: new ObjectId(userId) }, { teacherIds: new ObjectId(userId) }], + }); const mapped: Class[] = ClassMapper.mapToDOs(classes); @@ -26,9 +29,30 @@ export class ClassesRepo { } async updateMany(classes: Class[]): Promise { - const classesEntities = ClassMapper.mapToEntities(classes); - const referencedEntities = classesEntities.map((classEntity) => this.em.getReference(ClassEntity, classEntity.id)); + const classMap: Map = new Map( + classes.map((clazz: Class): [string, Class] => [clazz.id, clazz]) + ); - await this.em.persistAndFlush(referencedEntities); + const existingEntities: ClassEntity[] = await this.em.find(ClassEntity, { + id: { $in: Array.from(classMap.keys()) }, + }); + + if (existingEntities.length < classes.length) { + const missingEntityIds: string[] = Array.from(classMap.keys()).filter( + (classId) => !existingEntities.find((entity) => entity.id === classId) + ); + + throw new NotFoundLoggableException(Class.name, 'id', missingEntityIds.toString()); + } + + existingEntities.forEach((entity) => { + const updatedDomainObject: Class | undefined = classMap.get(entity.id); + + const updatedEntity: ClassEntity = ClassMapper.mapToEntity(updatedDomainObject as Class); + + this.em.assign(entity, updatedEntity); + }); + + await this.em.persistAndFlush(existingEntities); } } diff --git a/apps/server/src/modules/class/repo/mapper/class.mapper.spec.ts b/apps/server/src/modules/class/repo/mapper/class.mapper.spec.ts index 53d0ecd4360..f1e71da5c4d 100644 --- a/apps/server/src/modules/class/repo/mapper/class.mapper.spec.ts +++ b/apps/server/src/modules/class/repo/mapper/class.mapper.spec.ts @@ -3,7 +3,7 @@ import { Class } from '../../domain'; import { ClassSourceOptions } from '../../domain/class-source-options.do'; import { classFactory } from '../../domain/testing/factory/class.factory'; import { ClassEntity } from '../../entity'; -import { classEntityFactory } from '../../entity/testing/factory/class.entity.factory'; +import { classEntityFactory } from '../../entity/testing'; import { ClassMapper } from './class.mapper'; describe(ClassMapper.name, () => { diff --git a/apps/server/src/modules/class/repo/mapper/class.mapper.ts b/apps/server/src/modules/class/repo/mapper/class.mapper.ts index 6340ffce7b0..8ae5e3b79b9 100644 --- a/apps/server/src/modules/class/repo/mapper/class.mapper.ts +++ b/apps/server/src/modules/class/repo/mapper/class.mapper.ts @@ -23,7 +23,7 @@ export class ClassMapper { }); } - private static mapToEntity(domainObject: Class): ClassEntity { + static mapToEntity(domainObject: Class): ClassEntity { return new ClassEntity({ id: domainObject.id, name: domainObject.name, diff --git a/apps/server/src/modules/class/service/class.service.spec.ts b/apps/server/src/modules/class/service/class.service.spec.ts index 850eaf655a6..5ba4b367b59 100644 --- a/apps/server/src/modules/class/service/class.service.spec.ts +++ b/apps/server/src/modules/class/service/class.service.spec.ts @@ -4,9 +4,9 @@ import { InternalServerErrorException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { EntityId } from '@shared/domain'; import { setupEntities } from '@shared/testing'; -import { classEntityFactory } from '@modules/class/entity/testing/factory/class.entity.factory'; import { Class } from '../domain'; -import { classFactory } from '../domain/testing/factory/class.factory'; +import { classFactory } from '../domain/testing'; +import { classEntityFactory } from '../entity/testing'; import { ClassesRepo } from '../repo'; import { ClassMapper } from '../repo/mapper'; import { ClassService } from './class.service'; @@ -74,6 +74,31 @@ describe(ClassService.name, () => { }); }); + describe('findAllByUserId', () => { + describe('when the user has classes', () => { + const setup = () => { + const userId: string = new ObjectId().toHexString(); + + const classes: Class[] = classFactory.buildList(3); + + classesRepo.findAllByUserId.mockResolvedValueOnce(classes); + + return { + userId, + classes, + }; + }; + + it('should return the classes', async () => { + const { userId, classes } = setup(); + + const result: Class[] = await service.findAllByUserId(userId); + + expect(result).toEqual(classes); + }); + }); + }); + describe('deleteUserDataFromClasses', () => { describe('when user is missing', () => { const setup = () => { diff --git a/apps/server/src/modules/class/service/class.service.ts b/apps/server/src/modules/class/service/class.service.ts index 9671c456912..772b9f0c4d4 100644 --- a/apps/server/src/modules/class/service/class.service.ts +++ b/apps/server/src/modules/class/service/class.service.ts @@ -13,6 +13,13 @@ export class ClassService { return classes; } + public async findAllByUserId(userId: EntityId): Promise { + const classes: Class[] = await this.classesRepo.findAllByUserId(userId); + + return classes; + } + + // FIXME There is no usage of this method public async deleteUserDataFromClasses(userId: EntityId): Promise { if (!userId) { throw new InternalServerErrorException('User id is missing'); diff --git a/apps/server/src/modules/group/controller/api-test/group.api.spec.ts b/apps/server/src/modules/group/controller/api-test/group.api.spec.ts index 34a49c03a35..2d9f4105f80 100644 --- a/apps/server/src/modules/group/controller/api-test/group.api.spec.ts +++ b/apps/server/src/modules/group/controller/api-test/group.api.spec.ts @@ -1,4 +1,7 @@ import { EntityManager } from '@mikro-orm/mongodb'; +import { ClassEntity } from '@modules/class/entity'; +import { classEntityFactory } from '@modules/class/entity/testing'; +import { ServerTestModule } from '@modules/server'; import { HttpStatus, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { Role, RoleName, SchoolEntity, SchoolYearEntity, SortOrder, SystemEntity, User } from '@shared/domain'; @@ -12,9 +15,6 @@ import { UserAndAccountTestFactory, userFactory, } from '@shared/testing'; -import { ClassEntity } from '@modules/class/entity'; -import { classEntityFactory } from '@modules/class/entity/testing/factory/class.entity.factory'; -import { ServerTestModule } from '@modules/server'; import { ObjectId } from 'bson'; import { GroupEntity, GroupEntityTypes } from '../../entity'; import { ClassRootType } from '../../uc/dto/class-root-type'; @@ -135,29 +135,6 @@ describe('Group (API)', () => { }); }); }); - - describe('when an invalid user requests a list of classes', () => { - const setup = async () => { - const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent(); - - await em.persistAndFlush([studentAccount, studentUser]); - em.clear(); - - const studentClient = await testApiClient.login(studentAccount); - - return { - studentClient, - }; - }; - - it('should return forbidden', async () => { - const { studentClient } = await setup(); - - const response = await studentClient.get(`/class`); - - expect(response.status).toEqual(HttpStatus.FORBIDDEN); - }); - }); }); describe('[GET] /groups/:groupId', () => { diff --git a/apps/server/src/modules/group/controller/group.controller.ts b/apps/server/src/modules/group/controller/group.controller.ts index 9e5f4b3b51a..a7dc0c77563 100644 --- a/apps/server/src/modules/group/controller/group.controller.ts +++ b/apps/server/src/modules/group/controller/group.controller.ts @@ -1,9 +1,9 @@ +import { Authenticate, CurrentUser, ICurrentUser } from '@modules/authentication'; import { Controller, Get, HttpStatus, Param, Query } from '@nestjs/common'; import { ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; import { PaginationParams } from '@shared/controller'; import { Page } from '@shared/domain'; import { ErrorResponse } from '@src/core/error/dto'; -import { ICurrentUser, Authenticate, CurrentUser } from '@modules/authentication'; import { GroupUc } from '../uc'; import { ClassInfoDto, ResolvedGroupDto } from '../uc/dto'; import { ClassInfoSearchListResponse, ClassSortParams, GroupIdParams, GroupResponse } from './dto'; @@ -15,17 +15,17 @@ import { GroupResponseMapper } from './mapper'; export class GroupController { constructor(private readonly groupUc: GroupUc) {} - @ApiOperation({ summary: 'Get a list of classes and groups of type class for the current users school.' }) + @ApiOperation({ summary: 'Get a list of classes and groups of type class for the current user.' }) @ApiResponse({ status: HttpStatus.OK, type: ClassInfoSearchListResponse }) @ApiResponse({ status: '4XX', type: ErrorResponse }) @ApiResponse({ status: '5XX', type: ErrorResponse }) @Get('/class') - public async findClassesForSchool( + public async findClasses( @Query() pagination: PaginationParams, @Query() sortingQuery: ClassSortParams, @CurrentUser() currentUser: ICurrentUser ): Promise { - const board: Page = await this.groupUc.findAllClassesForSchool( + const board: Page = await this.groupUc.findAllClasses( currentUser.userId, currentUser.schoolId, pagination.skip, diff --git a/apps/server/src/modules/group/controller/mapper/group-response.mapper.ts b/apps/server/src/modules/group/controller/mapper/group-response.mapper.ts index 6efd02d899d..8c990cbd44a 100644 --- a/apps/server/src/modules/group/controller/mapper/group-response.mapper.ts +++ b/apps/server/src/modules/group/controller/mapper/group-response.mapper.ts @@ -40,7 +40,7 @@ export class GroupResponseMapper { type: classInfo.type, name: classInfo.name, externalSourceName: classInfo.externalSourceName, - teachers: classInfo.teachers, + teachers: classInfo.teacherNames, schoolYear: classInfo.schoolYear, isUpgradable: classInfo.isUpgradable, }); diff --git a/apps/server/src/modules/group/uc/dto/class-info.dto.ts b/apps/server/src/modules/group/uc/dto/class-info.dto.ts index 8c564d9e106..611275e3bcd 100644 --- a/apps/server/src/modules/group/uc/dto/class-info.dto.ts +++ b/apps/server/src/modules/group/uc/dto/class-info.dto.ts @@ -9,7 +9,7 @@ export class ClassInfoDto { externalSourceName?: string; - teachers: string[]; + teacherNames: string[]; schoolYear?: string; @@ -20,7 +20,7 @@ export class ClassInfoDto { this.type = props.type; this.name = props.name; this.externalSourceName = props.externalSourceName; - this.teachers = props.teachers; + this.teacherNames = props.teacherNames; this.schoolYear = props.schoolYear; this.isUpgradable = props.isUpgradable; } diff --git a/apps/server/src/modules/group/uc/group.uc.spec.ts b/apps/server/src/modules/group/uc/group.uc.spec.ts index 34cb55a1354..d5236826def 100644 --- a/apps/server/src/modules/group/uc/group.uc.spec.ts +++ b/apps/server/src/modules/group/uc/group.uc.spec.ts @@ -1,5 +1,14 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; +import { Action, AuthorizationContext, AuthorizationService } from '@modules/authorization'; +import { ClassService } from '@modules/class'; +import { Class } from '@modules/class/domain'; +import { classFactory } from '@modules/class/domain/testing/factory/class.factory'; +import { LegacySchoolService, SchoolYearService } from '@modules/legacy-school'; +import { RoleService } from '@modules/role'; +import { RoleDto } from '@modules/role/service/dto/role.dto'; +import { SystemDto, SystemService } from '@modules/system'; +import { UserService } from '@modules/user'; import { ForbiddenException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { NotFoundLoggableException } from '@shared/common/loggable-exception'; @@ -14,15 +23,6 @@ import { userDoFactory, userFactory, } from '@shared/testing'; -import { Action, AuthorizationContext, AuthorizationService } from '@modules/authorization'; -import { ClassService } from '@modules/class'; -import { Class } from '@modules/class/domain'; -import { classFactory } from '@modules/class/domain/testing/factory/class.factory'; -import { LegacySchoolService, SchoolYearService } from '@modules/legacy-school'; -import { RoleService } from '@modules/role'; -import { RoleDto } from '@modules/role/service/dto/role.dto'; -import { SystemDto, SystemService } from '@modules/system'; -import { UserService } from '@modules/user'; import { Group, GroupTypes } from '../domain'; import { GroupService } from '../service'; import { ClassInfoDto, ResolvedGroupDto } from './dto'; @@ -102,7 +102,7 @@ describe('GroupUc', () => { jest.resetAllMocks(); }); - describe('findClassesForSchool', () => { + describe('findAllClasses', () => { describe('when the user has no permission', () => { const setup = () => { const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); @@ -114,6 +114,7 @@ describe('GroupUc', () => { authorizationService.checkPermission.mockImplementation(() => { throw error; }); + authorizationService.hasAllPermissions.mockReturnValueOnce(false); return { user, @@ -124,13 +125,13 @@ describe('GroupUc', () => { it('should throw forbidden', async () => { const { user, error } = setup(); - const func = () => uc.findAllClassesForSchool(user.id, user.school.id); + const func = () => uc.findAllClasses(user.id, user.school.id); await expect(func).rejects.toThrow(error); }); }); - describe('when the school has classes', () => { + describe('when accessing as a normal user', () => { const setup = () => { const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); const { studentUser } = UserAndAccountTestFactory.buildStudent(); @@ -181,8 +182,9 @@ describe('GroupUc', () => { schoolService.getSchoolById.mockResolvedValueOnce(school); authorizationService.getUserWithPermissions.mockResolvedValueOnce(teacherUser); - classService.findClassesForSchool.mockResolvedValueOnce([clazz]); - groupService.findClassesForSchool.mockResolvedValueOnce([group, groupWithSystem]); + authorizationService.hasAllPermissions.mockReturnValueOnce(false); + classService.findAllByUserId.mockResolvedValueOnce([clazz]); + groupService.findByUser.mockResolvedValueOnce([group, groupWithSystem]); systemService.findById.mockResolvedValue(system); userService.findById.mockImplementation((userId: string): Promise => { if (userId === teacherUser.id) { @@ -222,23 +224,34 @@ describe('GroupUc', () => { it('should check the required permissions', async () => { const { teacherUser, school } = setup(); - await uc.findAllClassesForSchool(teacherUser.id, teacherUser.school.id); + await uc.findAllClasses(teacherUser.id, teacherUser.school.id); expect(authorizationService.checkPermission).toHaveBeenCalledWith<[User, LegacySchoolDo, AuthorizationContext]>( teacherUser, school, { action: Action.read, - requiredPermissions: [Permission.CLASS_LIST, Permission.GROUP_LIST], + requiredPermissions: [Permission.CLASS_VIEW, Permission.GROUP_VIEW], } ); }); + it('should check the access to the full list', async () => { + const { teacherUser } = setup(); + + await uc.findAllClasses(teacherUser.id, teacherUser.school.id); + + expect(authorizationService.hasAllPermissions).toHaveBeenCalledWith<[User, string[]]>(teacherUser, [ + Permission.CLASS_FULL_ADMIN, + Permission.GROUP_FULL_ADMIN, + ]); + }); + describe('when no pagination is given', () => { it('should return all classes sorted by name', async () => { const { teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); - const result: Page = await uc.findAllClassesForSchool(teacherUser.id, teacherUser.school.id); + const result: Page = await uc.findAllClasses(teacherUser.id, teacherUser.school.id); expect(result).toEqual>({ data: [ @@ -247,7 +260,7 @@ describe('GroupUc', () => { name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, type: ClassRootType.CLASS, externalSourceName: clazz.source, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], schoolYear: schoolYear.name, isUpgradable: false, }, @@ -255,14 +268,14 @@ describe('GroupUc', () => { id: group.id, name: group.name, type: ClassRootType.GROUP, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], }, { id: groupWithSystem.id, name: groupWithSystem.name, type: ClassRootType.GROUP, externalSourceName: system.displayName, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], }, ], total: 3, @@ -274,7 +287,7 @@ describe('GroupUc', () => { it('should return all classes sorted by external source name in descending order', async () => { const { teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); - const result: Page = await uc.findAllClassesForSchool( + const result: Page = await uc.findAllClasses( teacherUser.id, teacherUser.school.id, undefined, @@ -290,7 +303,7 @@ describe('GroupUc', () => { name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, type: ClassRootType.CLASS, externalSourceName: clazz.source, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], schoolYear: schoolYear.name, isUpgradable: false, }, @@ -299,13 +312,13 @@ describe('GroupUc', () => { name: groupWithSystem.name, type: ClassRootType.GROUP, externalSourceName: system.displayName, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], }, { id: group.id, name: group.name, type: ClassRootType.GROUP, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], }, ], total: 3, @@ -317,7 +330,7 @@ describe('GroupUc', () => { it('should return the selected page', async () => { const { teacherUser, group } = setup(); - const result: Page = await uc.findAllClassesForSchool( + const result: Page = await uc.findAllClasses( teacherUser.id, teacherUser.school.id, 1, @@ -332,7 +345,242 @@ describe('GroupUc', () => { id: group.id, name: group.name, type: ClassRootType.GROUP, - teachers: [teacherUser.lastName], + teacherNames: [teacherUser.lastName], + }, + ], + total: 3, + }); + }); + }); + }); + + describe('when accessing as a user with elevated permission', () => { + const setup = () => { + const school: LegacySchoolDo = legacySchoolDoFactory.buildWithId(); + const { studentUser } = UserAndAccountTestFactory.buildStudent(); + const { teacherUser } = UserAndAccountTestFactory.buildTeacher(); + const { adminUser } = UserAndAccountTestFactory.buildAdmin(); + const teacherRole: RoleDto = roleDtoFactory.buildWithId({ + id: teacherUser.roles[0].id, + name: teacherUser.roles[0].name, + }); + const studentRole: RoleDto = roleDtoFactory.buildWithId({ + id: studentUser.roles[0].id, + name: studentUser.roles[0].name, + }); + const adminUserDo: UserDO = userDoFactory.buildWithId({ + id: adminUser.id, + lastName: adminUser.lastName, + roles: [{ id: adminUser.roles[0].id, name: adminUser.roles[0].name }], + }); + const teacherUserDo: UserDO = userDoFactory.buildWithId({ + id: teacherUser.id, + lastName: teacherUser.lastName, + roles: [{ id: teacherUser.roles[0].id, name: teacherUser.roles[0].name }], + }); + const studentUserDo: UserDO = userDoFactory.buildWithId({ + id: studentUser.id, + lastName: studentUser.lastName, + roles: [{ id: studentUser.roles[0].id, name: studentUser.roles[0].name }], + }); + const schoolYear: SchoolYearEntity = schoolYearFactory.buildWithId(); + const clazz: Class = classFactory.build({ + name: 'A', + teacherIds: [teacherUser.id], + source: 'LDAP', + year: schoolYear.id, + }); + const system: SystemDto = new SystemDto({ + id: new ObjectId().toHexString(), + displayName: 'External System', + type: 'oauth2', + }); + const group: Group = groupFactory.build({ + name: 'B', + users: [{ userId: teacherUser.id, roleId: teacherUser.roles[0].id }], + externalSource: undefined, + }); + const groupWithSystem: Group = groupFactory.build({ + name: 'C', + externalSource: { externalId: 'externalId', systemId: system.id }, + users: [ + { userId: teacherUser.id, roleId: teacherUser.roles[0].id }, + { userId: studentUser.id, roleId: studentUser.roles[0].id }, + ], + }); + + schoolService.getSchoolById.mockResolvedValueOnce(school); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(adminUser); + authorizationService.hasAllPermissions.mockReturnValueOnce(true); + classService.findClassesForSchool.mockResolvedValueOnce([clazz]); + groupService.findClassesForSchool.mockResolvedValueOnce([group, groupWithSystem]); + systemService.findById.mockResolvedValue(system); + + userService.findById.mockImplementation((userId: string): Promise => { + if (userId === teacherUser.id) { + return Promise.resolve(teacherUserDo); + } + + if (userId === studentUser.id) { + return Promise.resolve(studentUserDo); + } + + if (userId === adminUser.id) { + return Promise.resolve(adminUserDo); + } + + throw new Error(); + }); + roleService.findById.mockImplementation((roleId: string): Promise => { + if (roleId === teacherUser.roles[0].id) { + return Promise.resolve(teacherRole); + } + + if (roleId === studentUser.roles[0].id) { + return Promise.resolve(studentRole); + } + + throw new Error(); + }); + schoolYearService.findById.mockResolvedValue(schoolYear); + + return { + adminUser, + teacherUser, + school, + clazz, + group, + groupWithSystem, + system, + schoolYear, + }; + }; + + it('should check the required permissions', async () => { + const { adminUser, school } = setup(); + + await uc.findAllClasses(adminUser.id, adminUser.school.id); + + expect(authorizationService.checkPermission).toHaveBeenCalledWith<[User, LegacySchoolDo, AuthorizationContext]>( + adminUser, + school, + { + action: Action.read, + requiredPermissions: [Permission.CLASS_VIEW, Permission.GROUP_VIEW], + } + ); + }); + + it('should check the access to the full list', async () => { + const { adminUser } = setup(); + + await uc.findAllClasses(adminUser.id, adminUser.school.id); + + expect(authorizationService.hasAllPermissions).toHaveBeenCalledWith<[User, string[]]>(adminUser, [ + Permission.CLASS_FULL_ADMIN, + Permission.GROUP_FULL_ADMIN, + ]); + }); + + describe('when no pagination is given', () => { + it('should return all classes sorted by name', async () => { + const { adminUser, teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); + + const result: Page = await uc.findAllClasses(adminUser.id, adminUser.school.id); + + expect(result).toEqual>({ + data: [ + { + id: clazz.id, + name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, + type: ClassRootType.CLASS, + externalSourceName: clazz.source, + teacherNames: [teacherUser.lastName], + schoolYear: schoolYear.name, + isUpgradable: false, + }, + { + id: group.id, + name: group.name, + type: ClassRootType.GROUP, + teacherNames: [teacherUser.lastName], + }, + { + id: groupWithSystem.id, + name: groupWithSystem.name, + type: ClassRootType.GROUP, + externalSourceName: system.displayName, + teacherNames: [teacherUser.lastName], + }, + ], + total: 3, + }); + }); + }); + + describe('when sorting by external source name in descending order', () => { + it('should return all classes sorted by external source name in descending order', async () => { + const { adminUser, teacherUser, clazz, group, groupWithSystem, system, schoolYear } = setup(); + + const result: Page = await uc.findAllClasses( + adminUser.id, + adminUser.school.id, + undefined, + undefined, + 'externalSourceName', + SortOrder.desc + ); + + expect(result).toEqual>({ + data: [ + { + id: clazz.id, + name: clazz.gradeLevel ? `${clazz.gradeLevel}${clazz.name}` : clazz.name, + type: ClassRootType.CLASS, + externalSourceName: clazz.source, + teacherNames: [teacherUser.lastName], + schoolYear: schoolYear.name, + isUpgradable: false, + }, + { + id: groupWithSystem.id, + name: groupWithSystem.name, + type: ClassRootType.GROUP, + externalSourceName: system.displayName, + teacherNames: [teacherUser.lastName], + }, + { + id: group.id, + name: group.name, + type: ClassRootType.GROUP, + teacherNames: [teacherUser.lastName], + }, + ], + total: 3, + }); + }); + }); + + describe('when using pagination', () => { + it('should return the selected page', async () => { + const { adminUser, teacherUser, group } = setup(); + + const result: Page = await uc.findAllClasses( + adminUser.id, + adminUser.school.id, + 1, + 1, + 'name', + SortOrder.asc + ); + + expect(result).toEqual>({ + data: [ + { + id: group.id, + name: group.name, + type: ClassRootType.GROUP, + teacherNames: [teacherUser.lastName], }, ], total: 3, diff --git a/apps/server/src/modules/group/uc/group.uc.ts b/apps/server/src/modules/group/uc/group.uc.ts index 2421e444e73..f40750fc852 100644 --- a/apps/server/src/modules/group/uc/group.uc.ts +++ b/apps/server/src/modules/group/uc/group.uc.ts @@ -1,5 +1,3 @@ -import { Injectable } from '@nestjs/common'; -import { EntityId, LegacySchoolDo, Page, Permission, SchoolYearEntity, SortOrder, User, UserDO } from '@shared/domain'; import { AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization'; import { ClassService } from '@modules/class'; import { Class } from '@modules/class/domain'; @@ -8,6 +6,8 @@ import { RoleService } from '@modules/role'; import { RoleDto } from '@modules/role/service/dto/role.dto'; import { SystemDto, SystemService } from '@modules/system'; import { UserService } from '@modules/user'; +import { Injectable } from '@nestjs/common'; +import { EntityId, LegacySchoolDo, Page, Permission, SchoolYearEntity, SortOrder, User, UserDO } from '@shared/domain'; import { Group, GroupUser } from '../domain'; import { GroupService } from '../service'; import { SortHelper } from '../util'; @@ -27,7 +27,7 @@ export class GroupUc { private readonly schoolYearService: SchoolYearService ) {} - public async findAllClassesForSchool( + public async findAllClasses( userId: EntityId, schoolId: EntityId, skip = 0, @@ -41,10 +41,20 @@ export class GroupUc { this.authorizationService.checkPermission( user, school, - AuthorizationContextBuilder.read([Permission.CLASS_LIST, Permission.GROUP_LIST]) + AuthorizationContextBuilder.read([Permission.CLASS_VIEW, Permission.GROUP_VIEW]) ); - const combinedClassInfo: ClassInfoDto[] = await this.findCombinedClassListForSchool(schoolId); + const canSeeFullList: boolean = this.authorizationService.hasAllPermissions(user, [ + Permission.CLASS_FULL_ADMIN, + Permission.GROUP_FULL_ADMIN, + ]); + + let combinedClassInfo: ClassInfoDto[]; + if (canSeeFullList) { + combinedClassInfo = await this.findCombinedClassListForSchool(schoolId); + } else { + combinedClassInfo = await this.findCombinedClassListForUser(userId); + } combinedClassInfo.sort((a: ClassInfoDto, b: ClassInfoDto): number => SortHelper.genericSortFunction(a[sortBy], b[sortBy], sortOrder) @@ -57,7 +67,7 @@ export class GroupUc { return page; } - private async findCombinedClassListForSchool(schoolId: string): Promise { + private async findCombinedClassListForSchool(schoolId: EntityId): Promise { const [classInfosFromClasses, classInfosFromGroups] = await Promise.all([ await this.findClassesForSchool(schoolId), await this.findGroupsOfTypeClassForSchool(schoolId), @@ -68,52 +78,91 @@ export class GroupUc { return combinedClassInfo; } + private async findCombinedClassListForUser(userId: EntityId): Promise { + const [classInfosFromClasses, classInfosFromGroups] = await Promise.all([ + await this.findClassesForUser(userId), + await this.findGroupsOfTypeClassForUser(userId), + ]); + + const combinedClassInfo: ClassInfoDto[] = [...classInfosFromClasses, ...classInfosFromGroups]; + + return combinedClassInfo; + } + private async findClassesForSchool(schoolId: EntityId): Promise { const classes: Class[] = await this.classService.findClassesForSchool(schoolId); const classInfosFromClasses: ClassInfoDto[] = await Promise.all( - classes.map(async (clazz: Class): Promise => { - const teachers: UserDO[] = await Promise.all( - clazz.teacherIds.map((teacherId: EntityId) => this.userService.findById(teacherId)) - ); + classes.map((clazz) => this.getClassInfoFromClass(clazz)) + ); - let schoolYear: SchoolYearEntity | undefined; - if (clazz.year) { - schoolYear = await this.schoolYearService.findById(clazz.year); - } + return classInfosFromClasses; + } - const mapped: ClassInfoDto = GroupUcMapper.mapClassToClassInfoDto(clazz, teachers, schoolYear); + private async findClassesForUser(userId: EntityId): Promise { + const classes: Class[] = await this.classService.findAllByUserId(userId); - return mapped; - }) + const classInfosFromClasses: ClassInfoDto[] = await Promise.all( + classes.map((clazz) => this.getClassInfoFromClass(clazz)) ); return classInfosFromClasses; } + private async getClassInfoFromClass(clazz: Class): Promise { + const teachers: UserDO[] = await Promise.all( + clazz.teacherIds.map((teacherId: EntityId) => this.userService.findById(teacherId)) + ); + + let schoolYear: SchoolYearEntity | undefined; + if (clazz.year) { + schoolYear = await this.schoolYearService.findById(clazz.year); + } + + const mapped: ClassInfoDto = GroupUcMapper.mapClassToClassInfoDto(clazz, teachers, schoolYear); + + return mapped; + } + private async findGroupsOfTypeClassForSchool(schoolId: EntityId): Promise { const groupsOfTypeClass: Group[] = await this.groupService.findClassesForSchool(schoolId); const systemMap: Map = await this.findSystemNamesForGroups(groupsOfTypeClass); const classInfosFromGroups: ClassInfoDto[] = await Promise.all( - groupsOfTypeClass.map(async (group: Group): Promise => { - let system: SystemDto | undefined; - if (group.externalSource) { - system = systemMap.get(group.externalSource.systemId); - } + groupsOfTypeClass.map(async (group: Group): Promise => this.getClassInfoFromGroup(group, systemMap)) + ); - const resolvedUsers: ResolvedGroupUser[] = await this.findUsersForGroup(group); + return classInfosFromGroups; + } - const mapped: ClassInfoDto = GroupUcMapper.mapGroupToClassInfoDto(group, resolvedUsers, system); + private async findGroupsOfTypeClassForUser(userId: EntityId): Promise { + const user: UserDO = await this.userService.findById(userId); - return mapped; - }) + const groupsOfTypeClass: Group[] = await this.groupService.findByUser(user); + + const systemMap: Map = await this.findSystemNamesForGroups(groupsOfTypeClass); + + const classInfosFromGroups: ClassInfoDto[] = await Promise.all( + groupsOfTypeClass.map(async (group: Group): Promise => this.getClassInfoFromGroup(group, systemMap)) ); return classInfosFromGroups; } + private async getClassInfoFromGroup(group: Group, systemMap: Map): Promise { + let system: SystemDto | undefined; + if (group.externalSource) { + system = systemMap.get(group.externalSource.systemId); + } + + const resolvedUsers: ResolvedGroupUser[] = await this.findUsersForGroup(group); + + const mapped: ClassInfoDto = GroupUcMapper.mapGroupToClassInfoDto(group, resolvedUsers, system); + + return mapped; + } + private async findSystemNamesForGroups(groups: Group[]): Promise> { const systemIds: EntityId[] = groups .map((group: Group): string | undefined => group.externalSource?.systemId) diff --git a/apps/server/src/modules/group/uc/mapper/group-uc.mapper.ts b/apps/server/src/modules/group/uc/mapper/group-uc.mapper.ts index 5ac11f0e0b6..f65e8cca602 100644 --- a/apps/server/src/modules/group/uc/mapper/group-uc.mapper.ts +++ b/apps/server/src/modules/group/uc/mapper/group-uc.mapper.ts @@ -1,6 +1,6 @@ -import { RoleName, SchoolYearEntity, UserDO } from '@shared/domain'; import { Class } from '@modules/class/domain'; import { SystemDto } from '@modules/system'; +import { RoleName, SchoolYearEntity, UserDO } from '@shared/domain'; import { Group } from '../../domain'; import { ClassInfoDto, ResolvedGroupDto, ResolvedGroupUser } from '../dto'; import { ClassRootType } from '../dto/class-root-type'; @@ -16,7 +16,7 @@ export class GroupUcMapper { type: ClassRootType.GROUP, name: group.name, externalSourceName: system?.displayName, - teachers: resolvedUsers + teacherNames: resolvedUsers .filter((groupUser: ResolvedGroupUser) => groupUser.role.name === RoleName.TEACHER) .map((groupUser: ResolvedGroupUser) => groupUser.user.lastName), }); @@ -33,7 +33,7 @@ export class GroupUcMapper { type: ClassRootType.CLASS, name, externalSourceName: clazz.source, - teachers: teachers.map((user: UserDO) => user.lastName), + teacherNames: teachers.map((user: UserDO) => user.lastName), schoolYear: schoolYear?.name, isUpgradable, }); diff --git a/apps/server/src/shared/domain/interface/permission.enum.ts b/apps/server/src/shared/domain/interface/permission.enum.ts index 4512b95de7f..c3f880101b5 100644 --- a/apps/server/src/shared/domain/interface/permission.enum.ts +++ b/apps/server/src/shared/domain/interface/permission.enum.ts @@ -55,6 +55,7 @@ export enum Permission { FOLDER_CREATE = 'FOLDER_CREATE', FOLDER_DELETE = 'FOLDER_DELETE', GROUP_LIST = 'GROUP_LIST', + GROUP_FULL_ADMIN = 'GROUP_FULL_ADMIN', GROUP_VIEW = 'GROUP_VIEW', HELPDESK_CREATE = 'HELPDESK_CREATE', HELPDESK_EDIT = 'HELPDESK_EDIT', diff --git a/apps/server/src/shared/testing/user-role-permissions.ts b/apps/server/src/shared/testing/user-role-permissions.ts index cfd38cea3a3..6c38287a37e 100644 --- a/apps/server/src/shared/testing/user-role-permissions.ts +++ b/apps/server/src/shared/testing/user-role-permissions.ts @@ -139,4 +139,5 @@ export const adminPermissions = [ Permission.IMPORT_USER_UPDATE, Permission.IMPORT_USER_VIEW, Permission.SCHOOL_TOOL_ADMIN, + Permission.GROUP_FULL_ADMIN, ] as Permission[]; diff --git a/backup/setup/migrations.json b/backup/setup/migrations.json index 8292d8fc62f..90fcee0baa3 100644 --- a/backup/setup/migrations.json +++ b/backup/setup/migrations.json @@ -328,5 +328,16 @@ "$date": "2023-10-17T14:38:44.886Z" }, "__v": 0 + }, + { + "_id": { + "$oid": "653a645338f94b0ea8e3173d" + }, + "state": "up", + "name": "add-group-full-admin-permission", + "createdAt": { + "$date": "2023-10-26T13:06:27.322Z" + }, + "__v": 0 } ] diff --git a/backup/setup/roles.json b/backup/setup/roles.json index 0ad460fc526..9ceaa532f1d 100644 --- a/backup/setup/roles.json +++ b/backup/setup/roles.json @@ -134,7 +134,8 @@ "USER_LOGIN_MIGRATION_ADMIN", "START_MEETING", "JOIN_MEETING", - "GROUP_LIST" + "GROUP_LIST", + "GROUP_FULL_ADMIN" ], "__v": 2 }, @@ -191,7 +192,8 @@ "TOOL_CREATE", "TOOL_EDIT", "YEARS_EDIT", - "GROUP_LIST" + "GROUP_LIST", + "GROUP_FULL_ADMIN" ], "__v": 2 }, diff --git a/migrations/1698325587322-add-group-full-admin-permission.js b/migrations/1698325587322-add-group-full-admin-permission.js new file mode 100644 index 00000000000..cb7a10a642a --- /dev/null +++ b/migrations/1698325587322-add-group-full-admin-permission.js @@ -0,0 +1,74 @@ +const mongoose = require('mongoose'); +// eslint-disable-next-line no-unused-vars +const { info } = require('winston'); +const { alert } = require('../src/logger'); + +const { connect, close } = require('../src/utils/database'); + +const Roles = mongoose.model( + 'roles202310261524', + new mongoose.Schema( + { + name: { type: String, required: true }, + permissions: [{ type: String }], + }, + { + timestamps: true, + } + ), + 'roles' +); + +module.exports = { + up: async function up() { + // eslint-disable-next-line no-process-env + if (process.env.SC_THEME !== 'n21') { + info('Permission GROUP_FULL_ADMIN will not be added for this instance.'); + return; + } + + await connect(); + + const adminAndSuperheroRole = await Roles.updateMany( + { name: { $in: ['administrator', 'superhero'] } }, + { + $addToSet: { + permissions: { + $each: ['GROUP_FULL_ADMIN'], + }, + }, + } + ).exec(); + + if (adminAndSuperheroRole) { + alert('Permission GROUP_FULL_ADMIN added to role superhero and administrator'); + } + + await close(); + }, + + down: async function down() { + // eslint-disable-next-line no-process-env + if (process.env.SC_THEME !== 'n21') { + info('Permission GROUP_FULL_ADMIN will not be removed for this instance.'); + return; + } + + await connect(); + + const adminAndSuperheroRole = await Roles.updateMany( + { name: { $in: ['administrator', 'superhero'] } }, + { + $pull: { + permissions: 'GROUP_FULL_ADMIN', + }, + } + ).exec(); + + if (adminAndSuperheroRole) { + alert('Rollback: Removed permission GROUP_FULL_ADMIN from roles superhero and administrator'); + } + + await close(); + }, +};