diff --git a/apps/server/src/modules/authorization/README.md b/apps/server/src/modules/authorization/README.md index 645feed2e64..7d2b69d209b 100644 --- a/apps/server/src/modules/authorization/README.md +++ b/apps/server/src/modules/authorization/README.md @@ -132,17 +132,7 @@ When calling other internal micro service for already authorized operations plea // next orchestration steps ``` -### Example 2 - Execute a Single Operation with Loading Resources - -```javascript -// If you don't have an entity but an entity type and id, you can check permission by reference -await this.authorizationService.checkPermissionByReferences(userId, AllowedEntity.course, courseId, AuthorizationContextBuilder.read([])); -// or -await this.authorizationService.hasPermissionByReferences(userId, AllowedEntity.course, courseId, AuthorizationContextBuilder.read([])); -// next orchestration steps -``` - -### Example 3 - Set Permission(s) of User as Required +### Example 2 - Set Permission(s) of User as Required ```javascript // Multiple permissions can be added. For a successful authorization, the user need all of them. @@ -173,14 +163,13 @@ this.authorizationService.hasPermission(userId, course, PermissionContexts.creat ```ts async createSchoolBySuperhero(userId: EntityId, params: { name: string }) { - const user = this.authorizationService.getUserWithPermissions(userId); - this.authorizationService.hasAllPermissions(user, [Permission.SCHOOL_CREATE]); - - const school = new School(params); + const user = this.authorizationService.getUserWithPermissions(userId); + this.authorizationService.hasAllPermissions(user, [Permission.SCHOOL_CREATE]); - await this.schoolService.save(school); + const school = new School(params); + await this.schoolService.save(school); - return true; + return true; } ``` @@ -191,15 +180,15 @@ async createSchoolBySuperhero(userId: EntityId, params: { name: string }) { async createUserByAdmin(userId: EntityId, params: { email: string, firstName: string, lastName: string, schoolId: EntityId }) { - const user = this.authorizationService.getUserWithPermissions(userId); - - await this.authorizationService.checkPermissionByReferences(userId, AllowedEntity.school, schoolId, AuthorizationContextBuilder.write([Permission.INSTANCE, Permission.CREATE_USER])); - - const newUser = new User(params) + const user = this.authorizationService.getUserWithPermissions(userId); + + const context = AuthorizationContextBuilder.write([Permission.INSTANCE, Permission.CREATE_USER]) + await this.authorizationService.checkPermission(user, school, context); - await this.userService.save(newUser); + const newUser = new User(params) + await this.userService.save(newUser); - return true; + return true; } ``` @@ -210,18 +199,17 @@ async createUserByAdmin(userId: EntityId, params: { email: string, firstName: st // admin async editCourseByAdmin(userId: EntityId, params: { courseId: EntityId, description: string }) { - const course = this.courseService.getCourse(params.courseId); - const user = this.authorizationService.getUserWithPermissions(userId); - - const school = course.school - - this.authorizationService.hasPermissions(user, school, [Permission.INSTANCE, Permission.COURSE_EDIT]); + const course = this.courseService.getCourse(params.courseId); + const user = this.authorizationService.getUserWithPermissions(userId); + const school = course.school; - course.description = params.description; + const context = AuthorizationContextBuilder.write([Permission.INSTANCE, Permission.CREATE_USER]); + this.authorizationService.checkPermissions(user, school, context); - await this.courseService.save(course); + course.description = params.description; + await this.courseService.save(course); - return true; + return true; } ``` @@ -234,18 +222,17 @@ async createCourse(userId: EntityId, params: { schoolId: EntityId }) { const user = this.authorizationService.getUserWithPermissions(userId); const school = this.schoolService.getSchool(params.schoolId); - this.authorizationService.checkPermission(user, school - { - action: Actions.write, - requiredPermissions: [Permission.COURSE_CREATE], - } - ); + this.authorizationService.checkPermission(user, school + { + action: Actions.write, + requiredPermissions: [Permission.COURSE_CREATE], + } + ); - const course = new Course({ school }); + const course = new Course({ school }); + await this.courseService.saveCourse(course); - await this.courseService.saveCourse(course); - - return course; + return course; } ``` @@ -255,21 +242,20 @@ async createCourse(userId: EntityId, params: { schoolId: EntityId }) { ```ts // User can create a lesson to course, so you have a courseId async createLesson(userId: EntityId, params: { courseId: EntityId }) { - const course = this.courseService.getCourse(params.courseId); - const user = this.authorizationService.getUserWithPermissions(userId); + const course = this.courseService.getCourse(params.courseId); + const user = this.authorizationService.getUserWithPermissions(userId); // check authorization for user and course - this.authorizationService.checkPermission(user, course - { - action: Actions.write, - requiredPermissions: [Permission.COURSE_EDIT], - } - ); - - const lesson = new Lesson({course}); + this.authorizationService.checkPermission(user, course + { + action: Actions.write, + requiredPermissions: [Permission.COURSE_EDIT], + } + ); - await this.lessonService.saveLesson(lesson); + const lesson = new Lesson({course}); + await this.lessonService.saveLesson(lesson); - return true; + return true; } ``` @@ -345,8 +331,9 @@ The authorization module is the core of authorization. It collects all needed in ### Reference.loader -For situations where only the id and the domain object (string) type is known, it is possible to use the \*ByReferences methods. -They load the reference directly. +It should be use only inside of the authorization module. +It is use to load registrated ressouces by the id and name of the ressource. +This is needed to solve the API requests from external services. (API implementation is missing for now) > Please keep in mind that it can have an impact on the performance if you use it wrongly. > We keep it as a seperate method to avoid the usage in areas where the domain object should exist, because we see the risk that a developer could be tempted by the ease of only passing the id. diff --git a/apps/server/src/modules/authorization/authorization-reference.module.ts b/apps/server/src/modules/authorization/authorization-reference.module.ts new file mode 100644 index 00000000000..7346f2178dd --- /dev/null +++ b/apps/server/src/modules/authorization/authorization-reference.module.ts @@ -0,0 +1,43 @@ +import { forwardRef, Module } from '@nestjs/common'; +import { + CourseGroupRepo, + CourseRepo, + LessonRepo, + SchoolExternalToolRepo, + LegacySchoolRepo, + SubmissionRepo, + TaskRepo, + TeamsRepo, + UserRepo, +} from '@shared/repo'; +import { ToolModule } from '@src/modules/tool'; +import { LoggerModule } from '@src/core/logger'; +import { BoardModule } from '@src/modules/board'; +import { ReferenceLoader, AuthorizationReferenceService, AuthorizationHelper } from './domain'; +import { AuthorizationModule } from './authorization.module'; + +/** + * This module is part of an intermediate state. In the future it should be replaced by an AuthorizationApiModule. + * For now it is used where the authorization itself needs to load data from the database. + * Avoid using this module and load the needed data in your use cases and then use the normal AuthorizationModule! + */ +@Module({ + // TODO: remove forwardRef to TooModule N21-1055 + imports: [AuthorizationModule, forwardRef(() => ToolModule), forwardRef(() => BoardModule), LoggerModule], + providers: [ + AuthorizationHelper, + ReferenceLoader, + UserRepo, + CourseRepo, + CourseGroupRepo, + TaskRepo, + LegacySchoolRepo, + LessonRepo, + TeamsRepo, + SubmissionRepo, + SchoolExternalToolRepo, + AuthorizationReferenceService, + ], + exports: [AuthorizationReferenceService], +}) +export class AuthorizationReferenceModule {} diff --git a/apps/server/src/modules/authorization/authorization.module.ts b/apps/server/src/modules/authorization/authorization.module.ts index c983ee187fd..37ca0a2b229 100644 --- a/apps/server/src/modules/authorization/authorization.module.ts +++ b/apps/server/src/modules/authorization/authorization.module.ts @@ -1,53 +1,46 @@ -import { forwardRef, Module } from '@nestjs/common'; -import { ALL_RULES } from '@shared/domain/rules'; +import { Module } from '@nestjs/common'; +import { UserRepo } from '@shared/repo'; +import { LoggerModule } from '@src/core/logger'; import { FeathersModule } from '@shared/infra/feathers'; import { - CourseGroupRepo, - CourseRepo, - LessonRepo, - SchoolExternalToolRepo, - LegacySchoolRepo, - SubmissionRepo, - TaskRepo, - TeamsRepo, - UserRepo, -} from '@shared/repo'; -import { LoggerModule } from '@src/core/logger'; -import { LegacySchoolModule } from '@src/modules/legacy-school'; -import { ToolModule } from '@src/modules/tool'; -import { BoardModule } from '../board'; -import { AuthorizationHelper } from './authorization.helper'; -import { AuthorizationService } from './authorization.service'; + BoardDoRule, + ContextExternalToolRule, + CourseGroupRule, + CourseRule, + LessonRule, + SchoolExternalToolRule, + SubmissionRule, + TaskRule, + TeamRule, + UserRule, + UserLoginMigrationRule, + LegacySchoolRule, +} from './domain/rules'; +import { AuthorizationHelper, AuthorizationService, RuleManager } from './domain'; import { FeathersAuthorizationService, FeathersAuthProvider } from './feathers'; -import { ReferenceLoader } from './reference.loader'; -import { RuleManager } from './rule-manager'; @Module({ - // TODO: remove forwardRef to TooModule N21-1055 - imports: [ - FeathersModule, - LoggerModule, - LegacySchoolModule, - forwardRef(() => ToolModule), - forwardRef(() => BoardModule), - ], + imports: [FeathersModule, LoggerModule], providers: [ FeathersAuthorizationService, FeathersAuthProvider, AuthorizationService, - ...ALL_RULES, - ReferenceLoader, UserRepo, - CourseRepo, - CourseGroupRepo, - TaskRepo, - LegacySchoolRepo, - LessonRepo, - TeamsRepo, - SubmissionRepo, - SchoolExternalToolRepo, RuleManager, AuthorizationHelper, + // rules + BoardDoRule, + ContextExternalToolRule, + CourseGroupRule, + CourseRule, + LessonRule, + SchoolExternalToolRule, + SubmissionRule, + TaskRule, + TeamRule, + UserRule, + UserLoginMigrationRule, + LegacySchoolRule, ], exports: [FeathersAuthorizationService, AuthorizationService], }) diff --git a/apps/server/src/modules/authorization/authorization.service.ts b/apps/server/src/modules/authorization/authorization.service.ts deleted file mode 100644 index b89561f1c30..00000000000 --- a/apps/server/src/modules/authorization/authorization.service.ts +++ /dev/null @@ -1,100 +0,0 @@ -import { ForbiddenException, Injectable, UnauthorizedException } from '@nestjs/common'; -import { BaseDO, EntityId, User } from '@shared/domain'; -import { AuthorizableObject } from '@shared/domain/domain-object'; -import { ErrorUtils } from '@src/core/error/utils'; -import { AuthorizationHelper } from './authorization.helper'; -import { ForbiddenLoggableException } from './errors/forbidden.loggable-exception'; -import { ReferenceLoader } from './reference.loader'; -import { RuleManager } from './rule-manager'; -import { AuthorizableReferenceType, AuthorizationContext } from './types'; - -@Injectable() -export class AuthorizationService { - constructor( - private readonly ruleManager: RuleManager, - private readonly loader: ReferenceLoader, - private readonly authorizationHelper: AuthorizationHelper - ) {} - - public checkPermission(user: User, object: AuthorizableObject | BaseDO, context: AuthorizationContext): void { - if (!this.hasPermission(user, object, context)) { - throw new ForbiddenLoggableException(user.id, object.constructor.name, context); - } - } - - public hasPermission(user: User, object: AuthorizableObject | BaseDO, context: AuthorizationContext): boolean { - const rule = this.ruleManager.selectRule(user, object, context); - const hasPermission = rule.hasPermission(user, object, context); - - return hasPermission; - } - - /** - * @deprecated - */ - public async checkPermissionByReferences( - userId: EntityId, - entityName: AuthorizableReferenceType, - entityId: EntityId, - context: AuthorizationContext - ): Promise { - if (!(await this.hasPermissionByReferences(userId, entityName, entityId, context))) { - throw new ForbiddenLoggableException(userId, entityName, context); - } - } - - /** - * @deprecated - */ - public async hasPermissionByReferences( - userId: EntityId, - entityName: AuthorizableReferenceType, - entityId: EntityId, - context: AuthorizationContext - ): Promise { - // TODO: This try-catch-block should be removed. See ticket: https://ticketsystem.dbildungscloud.de/browse/BC-4023 - try { - const [user, object] = await Promise.all([ - this.getUserWithPermissions(userId), - this.loader.loadAuthorizableObject(entityName, entityId), - ]); - const rule = this.ruleManager.selectRule(user, object, context); - const hasPermission = rule.hasPermission(user, object, context); - - return hasPermission; - } catch (error) { - throw new ForbiddenException( - null, - ErrorUtils.createHttpExceptionOptions(error, 'AuthorizationService:hasPermissionByReferences') - ); - } - } - - public checkAllPermissions(user: User, requiredPermissions: string[]): void { - if (!this.authorizationHelper.hasAllPermissions(user, requiredPermissions)) { - // TODO: Should be ForbiddenException - throw new UnauthorizedException(); - } - } - - public hasAllPermissions(user: User, requiredPermissions: string[]): boolean { - return this.authorizationHelper.hasAllPermissions(user, requiredPermissions); - } - - public checkOneOfPermissions(user: User, requiredPermissions: string[]): void { - if (!this.authorizationHelper.hasOneOfPermissions(user, requiredPermissions)) { - // TODO: Should be ForbiddenException - throw new UnauthorizedException(); - } - } - - public hasOneOfPermissions(user: User, requiredPermissions: string[]): boolean { - return this.authorizationHelper.hasOneOfPermissions(user, requiredPermissions); - } - - public async getUserWithPermissions(userId: EntityId): Promise { - const userWithPermissions = await this.loader.getUserWithPermissions(userId); - - return userWithPermissions; - } -} diff --git a/apps/server/src/modules/authorization/errors/forbidden.loggable-exception.ts b/apps/server/src/modules/authorization/domain/error/forbidden.loggable-exception.ts similarity index 94% rename from apps/server/src/modules/authorization/errors/forbidden.loggable-exception.ts rename to apps/server/src/modules/authorization/domain/error/forbidden.loggable-exception.ts index f775fb903df..9557ed14ede 100644 --- a/apps/server/src/modules/authorization/errors/forbidden.loggable-exception.ts +++ b/apps/server/src/modules/authorization/domain/error/forbidden.loggable-exception.ts @@ -2,7 +2,7 @@ import { ForbiddenException } from '@nestjs/common'; import { EntityId } from '@shared/domain'; import { Loggable } from '@src/core/logger/interfaces'; import { ErrorLogMessage } from '@src/core/logger/types'; -import { AuthorizationContext } from '../types'; +import { AuthorizationContext } from '../type'; export class ForbiddenLoggableException extends ForbiddenException implements Loggable { constructor( diff --git a/apps/server/src/modules/authorization/domain/error/index.ts b/apps/server/src/modules/authorization/domain/error/index.ts new file mode 100644 index 00000000000..f2c782cbe56 --- /dev/null +++ b/apps/server/src/modules/authorization/domain/error/index.ts @@ -0,0 +1 @@ +export * from './forbidden.loggable-exception'; diff --git a/apps/server/src/modules/authorization/domain/index.ts b/apps/server/src/modules/authorization/domain/index.ts new file mode 100644 index 00000000000..0f5cfe67874 --- /dev/null +++ b/apps/server/src/modules/authorization/domain/index.ts @@ -0,0 +1,4 @@ +export * from './service'; +export * from './mapper'; +export * from './error'; +export * from './type'; diff --git a/apps/server/src/modules/authorization/authorization-context.builder.spec.ts b/apps/server/src/modules/authorization/domain/mapper/authorization-context.builder.spec.ts similarity index 96% rename from apps/server/src/modules/authorization/authorization-context.builder.spec.ts rename to apps/server/src/modules/authorization/domain/mapper/authorization-context.builder.spec.ts index 856ec92d4a6..5944d7f22e0 100644 --- a/apps/server/src/modules/authorization/authorization-context.builder.spec.ts +++ b/apps/server/src/modules/authorization/domain/mapper/authorization-context.builder.spec.ts @@ -1,6 +1,6 @@ import { Permission } from '@shared/domain'; import { AuthorizationContextBuilder } from './authorization-context.builder'; -import { Action } from './types'; +import { Action } from '../type'; describe('AuthorizationContextBuilder', () => { it('Should allow to set required permissions.', () => { diff --git a/apps/server/src/modules/authorization/authorization-context.builder.ts b/apps/server/src/modules/authorization/domain/mapper/authorization-context.builder.ts similarity index 91% rename from apps/server/src/modules/authorization/authorization-context.builder.ts rename to apps/server/src/modules/authorization/domain/mapper/authorization-context.builder.ts index 58259aa1b6f..86b16685b58 100644 --- a/apps/server/src/modules/authorization/authorization-context.builder.ts +++ b/apps/server/src/modules/authorization/domain/mapper/authorization-context.builder.ts @@ -1,5 +1,5 @@ import { Permission } from '@shared/domain'; -import { AuthorizationContext, Action } from './types'; +import { AuthorizationContext, Action } from '../type'; export class AuthorizationContextBuilder { private static build(requiredPermissions: Permission[], action: Action): AuthorizationContext { diff --git a/apps/server/src/modules/authorization/domain/mapper/index.ts b/apps/server/src/modules/authorization/domain/mapper/index.ts new file mode 100644 index 00000000000..6f21d79acad --- /dev/null +++ b/apps/server/src/modules/authorization/domain/mapper/index.ts @@ -0,0 +1 @@ +export * from './authorization-context.builder'; diff --git a/apps/server/src/shared/domain/rules/board-do.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/board-do.rule.spec.ts similarity index 95% rename from apps/server/src/shared/domain/rules/board-do.rule.spec.ts rename to apps/server/src/modules/authorization/domain/rules/board-do.rule.spec.ts index 3574250b67c..bda3680b460 100644 --- a/apps/server/src/shared/domain/rules/board-do.rule.spec.ts +++ b/apps/server/src/modules/authorization/domain/rules/board-do.rule.spec.ts @@ -1,10 +1,10 @@ import { Test, TestingModule } from '@nestjs/testing'; import { roleFactory, setupEntities, userFactory } from '@shared/testing'; -import { Action } from '@src/modules'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; import { ObjectId } from 'bson'; -import { BoardDoAuthorizable, BoardRoles, UserRoleEnum } from '../domainobject'; -import { Permission } from '../interface'; +import { BoardDoAuthorizable, BoardRoles, UserRoleEnum } from '@shared/domain/domainobject'; +import { Permission } from '@shared/domain/interface'; +import { Action } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; import { BoardDoRule } from './board-do.rule'; describe(BoardDoRule.name, () => { diff --git a/apps/server/src/shared/domain/rules/board-do.rule.ts b/apps/server/src/modules/authorization/domain/rules/board-do.rule.ts similarity index 81% rename from apps/server/src/shared/domain/rules/board-do.rule.ts rename to apps/server/src/modules/authorization/domain/rules/board-do.rule.ts index 575c9f0db5a..2042365e071 100644 --- a/apps/server/src/shared/domain/rules/board-do.rule.ts +++ b/apps/server/src/modules/authorization/domain/rules/board-do.rule.ts @@ -1,8 +1,8 @@ import { Injectable } from '@nestjs/common'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action, AuthorizationContext, Rule } from '@src/modules/authorization/types'; -import { BoardDoAuthorizable, BoardRoles } from '../domainobject'; -import { User } from '../entity'; +import { BoardDoAuthorizable, BoardRoles } from '@shared/domain/domainobject'; +import { User } from '@shared/domain/entity'; +import { Action, AuthorizationContext, Rule } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; @Injectable() export class BoardDoRule implements Rule { diff --git a/apps/server/src/shared/domain/rules/context-external-tool.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/context-external-tool.rule.spec.ts similarity index 93% rename from apps/server/src/shared/domain/rules/context-external-tool.rule.spec.ts rename to apps/server/src/modules/authorization/domain/rules/context-external-tool.rule.spec.ts index 90a25a1ca82..ddd458959ed 100644 --- a/apps/server/src/shared/domain/rules/context-external-tool.rule.spec.ts +++ b/apps/server/src/modules/authorization/domain/rules/context-external-tool.rule.spec.ts @@ -7,16 +7,15 @@ import { setupEntities, userFactory, } from '@shared/testing'; - -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action } from '@src/modules/authorization/types'; import { ContextExternalTool } from '@src/modules/tool/context-external-tool/domain'; import { ContextExternalToolEntity } from '@src/modules/tool/context-external-tool/entity'; import { SchoolExternalTool } from '@src/modules/tool/school-external-tool/domain'; import { SchoolExternalToolEntity } from '@src/modules/tool/school-external-tool/entity'; -import { Role, User } from '../entity'; -import { Permission } from '../interface'; +import { Role, User } from '@shared/domain/entity'; +import { Permission } from '@shared/domain/interface'; import { ContextExternalToolRule } from './context-external-tool.rule'; +import { Action } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; describe('ContextExternalToolRule', () => { let service: ContextExternalToolRule; diff --git a/apps/server/src/shared/domain/rules/context-external-tool.rule.ts b/apps/server/src/modules/authorization/domain/rules/context-external-tool.rule.ts similarity index 85% rename from apps/server/src/shared/domain/rules/context-external-tool.rule.ts rename to apps/server/src/modules/authorization/domain/rules/context-external-tool.rule.ts index 35be641e550..5d57c95a160 100644 --- a/apps/server/src/shared/domain/rules/context-external-tool.rule.ts +++ b/apps/server/src/modules/authorization/domain/rules/context-external-tool.rule.ts @@ -1,9 +1,9 @@ import { Injectable } from '@nestjs/common'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { AuthorizationContext, Rule } from '@src/modules/authorization/types'; import { ContextExternalTool } from '@src/modules/tool/context-external-tool/domain'; import { ContextExternalToolEntity } from '@src/modules/tool/context-external-tool/entity'; -import { User } from '../entity'; +import { User } from '@shared/domain/entity'; +import { AuthorizationContext, Rule } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; @Injectable() export class ContextExternalToolRule implements Rule { diff --git a/apps/server/src/shared/domain/rules/course-group.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/course-group.rule.spec.ts similarity index 97% rename from apps/server/src/shared/domain/rules/course-group.rule.spec.ts rename to apps/server/src/modules/authorization/domain/rules/course-group.rule.spec.ts index 8296f75f917..62c14baa138 100644 --- a/apps/server/src/shared/domain/rules/course-group.rule.spec.ts +++ b/apps/server/src/modules/authorization/domain/rules/course-group.rule.spec.ts @@ -2,10 +2,10 @@ import { Test, TestingModule } from '@nestjs/testing'; import { CourseGroup, User } from '@shared/domain/entity'; import { Permission } from '@shared/domain/interface'; import { courseFactory, courseGroupFactory, roleFactory, setupEntities, userFactory } from '@shared/testing'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action } from '@src/modules/authorization/types'; import { CourseGroupRule } from './course-group.rule'; import { CourseRule } from './course.rule'; +import { Action } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; describe('CourseGroupRule', () => { let service: CourseGroupRule; diff --git a/apps/server/src/shared/domain/rules/course-group.rule.ts b/apps/server/src/modules/authorization/domain/rules/course-group.rule.ts similarity index 84% rename from apps/server/src/shared/domain/rules/course-group.rule.ts rename to apps/server/src/modules/authorization/domain/rules/course-group.rule.ts index 14638862ba2..863d7072ec8 100644 --- a/apps/server/src/shared/domain/rules/course-group.rule.ts +++ b/apps/server/src/modules/authorization/domain/rules/course-group.rule.ts @@ -1,8 +1,8 @@ import { Injectable } from '@nestjs/common'; import { CourseGroup, User } from '@shared/domain/entity'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action, AuthorizationContext, Rule } from '@src/modules/authorization/types'; import { CourseRule } from './course.rule'; +import { Action, AuthorizationContext, Rule } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; @Injectable() export class CourseGroupRule implements Rule { diff --git a/apps/server/src/shared/domain/rules/course.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/course.rule.spec.ts similarity index 95% rename from apps/server/src/shared/domain/rules/course.rule.spec.ts rename to apps/server/src/modules/authorization/domain/rules/course.rule.spec.ts index eff0bd41fee..1c4dcc7d670 100644 --- a/apps/server/src/shared/domain/rules/course.rule.spec.ts +++ b/apps/server/src/modules/authorization/domain/rules/course.rule.spec.ts @@ -2,9 +2,9 @@ import { Test, TestingModule } from '@nestjs/testing'; import { Course, User } from '@shared/domain/entity'; import { Permission } from '@shared/domain/interface'; import { courseFactory, roleFactory, setupEntities, userFactory } from '@shared/testing'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action } from '@src/modules/authorization/types'; import { CourseRule } from './course.rule'; +import { Action } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; describe('CourseRule', () => { let service: CourseRule; diff --git a/apps/server/src/shared/domain/rules/course.rule.ts b/apps/server/src/modules/authorization/domain/rules/course.rule.ts similarity index 82% rename from apps/server/src/shared/domain/rules/course.rule.ts rename to apps/server/src/modules/authorization/domain/rules/course.rule.ts index 90183dbfd0f..e923e1ab967 100644 --- a/apps/server/src/shared/domain/rules/course.rule.ts +++ b/apps/server/src/modules/authorization/domain/rules/course.rule.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { Course, User } from '@shared/domain/entity'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action, AuthorizationContext, Rule } from '@src/modules/authorization/types'; +import { Action, AuthorizationContext, Rule } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; @Injectable() export class CourseRule implements Rule { diff --git a/apps/server/src/modules/authorization/domain/rules/index.ts b/apps/server/src/modules/authorization/domain/rules/index.ts new file mode 100644 index 00000000000..bd4ffe27a59 --- /dev/null +++ b/apps/server/src/modules/authorization/domain/rules/index.ts @@ -0,0 +1,16 @@ +/** + * Rules are currently placed in authorization module to avoid dependency cycles. + * In future they must be moved to the feature modules and register it in registration service. + */ +export * from './board-do.rule'; +export * from './context-external-tool.rule'; +export * from './course-group.rule'; +export * from './course.rule'; +export * from './legacy-school.rule'; +export * from './lesson.rule'; +export * from './school-external-tool.rule'; +export * from './submission.rule'; +export * from './task.rule'; +export * from './team.rule'; +export * from './user-login-migration.rule'; +export * from './user.rule'; diff --git a/apps/server/src/shared/domain/rules/legacy-school.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/legacy-school.rule.spec.ts similarity index 93% rename from apps/server/src/shared/domain/rules/legacy-school.rule.spec.ts rename to apps/server/src/modules/authorization/domain/rules/legacy-school.rule.spec.ts index c547f772de5..489def39318 100644 --- a/apps/server/src/shared/domain/rules/legacy-school.rule.spec.ts +++ b/apps/server/src/modules/authorization/domain/rules/legacy-school.rule.spec.ts @@ -1,9 +1,9 @@ import { Test, TestingModule } from '@nestjs/testing'; import { Permission } from '@shared/domain/interface'; import { roleFactory, legacySchoolDoFactory, setupEntities, userFactory } from '@shared/testing'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action } from '@src/modules/authorization/types'; import { ObjectID } from 'bson'; +import { Action } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; import { LegacySchoolRule } from './legacy-school.rule'; describe('LegacySchoolRule', () => { diff --git a/apps/server/src/shared/domain/rules/legacy-school.rule.ts b/apps/server/src/modules/authorization/domain/rules/legacy-school.rule.ts similarity index 78% rename from apps/server/src/shared/domain/rules/legacy-school.rule.ts rename to apps/server/src/modules/authorization/domain/rules/legacy-school.rule.ts index 5068d327c35..e115727091a 100644 --- a/apps/server/src/shared/domain/rules/legacy-school.rule.ts +++ b/apps/server/src/modules/authorization/domain/rules/legacy-school.rule.ts @@ -1,9 +1,9 @@ import { Injectable } from '@nestjs/common'; import { BaseDO, LegacySchoolDo } from '@shared/domain'; import { User } from '@shared/domain/entity'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { AuthorizationContext, Rule } from '@src/modules/authorization/types'; -import { AuthorizableObject } from '../domain-object'; +import { AuthorizableObject } from '@shared/domain/domain-object'; +import { AuthorizationContext, Rule } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; /** * @deprecated because it uses the deprecated LegacySchoolDo. diff --git a/apps/server/src/shared/domain/rules/lesson.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/lesson.rule.spec.ts similarity index 50% rename from apps/server/src/shared/domain/rules/lesson.rule.spec.ts rename to apps/server/src/modules/authorization/domain/rules/lesson.rule.spec.ts index 13c605f77e4..a8e4bdd8038 100644 --- a/apps/server/src/shared/domain/rules/lesson.rule.spec.ts +++ b/apps/server/src/modules/authorization/domain/rules/lesson.rule.spec.ts @@ -10,17 +10,20 @@ import { setupEntities, userFactory, } from '@shared/testing'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action } from '@src/modules/authorization/types'; -import { CourseGroupRule, CourseRule } from '.'; +import { NotImplementedException } from '@nestjs/common'; +import { Action, AuthorizationContext } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; +import { CourseGroupRule } from './course-group.rule'; +import { CourseRule } from './course.rule'; import { LessonRule } from './lesson.rule'; +import { AuthorizationContextBuilder } from '../mapper'; describe('LessonRule', () => { - let service: LessonRule; + let rule: LessonRule; let authorizationHelper: AuthorizationHelper; let courseRule: DeepPartial; let courseGroupRule: DeepPartial; - let user: User; + let globalUser: User; let entity: LessonEntity; const permissionA = 'a' as Permission; const permissionB = 'b' as Permission; @@ -33,7 +36,7 @@ describe('LessonRule', () => { providers: [AuthorizationHelper, LessonRule, CourseRule, CourseGroupRule], }).compile(); - service = await module.get(LessonRule); + rule = await module.get(LessonRule); authorizationHelper = await module.get(AuthorizationHelper); courseRule = await module.get(CourseRule); courseGroupRule = await module.get(CourseGroupRule); @@ -41,58 +44,117 @@ describe('LessonRule', () => { beforeEach(() => { const role = roleFactory.build({ permissions: [permissionA, permissionB] }); - user = userFactory.build({ roles: [role] }); + globalUser = userFactory.build({ roles: [role] }); }); it('should call hasAllPermissions on AuthorizationHelper', () => { entity = lessonFactory.build(); const spy = jest.spyOn(authorizationHelper, 'hasAllPermissions'); - service.hasPermission(user, entity, { action: Action.read, requiredPermissions: [] }); - expect(spy).toBeCalledWith(user, []); + rule.hasPermission(globalUser, entity, { action: Action.read, requiredPermissions: [] }); + expect(spy).toBeCalledWith(globalUser, []); }); it('should call courseRule.hasPermission', () => { - const course = courseFactory.build({ teachers: [user] }); + const course = courseFactory.build({ teachers: [globalUser] }); entity = lessonFactory.build({ course }); const spy = jest.spyOn(courseRule, 'hasPermission'); - service.hasPermission(user, entity, { action: Action.write, requiredPermissions: [permissionA] }); - expect(spy).toBeCalledWith(user, entity.course, { action: Action.write, requiredPermissions: [] }); + rule.hasPermission(globalUser, entity, { action: Action.write, requiredPermissions: [permissionA] }); + expect(spy).toBeCalledWith(globalUser, entity.course, { action: Action.write, requiredPermissions: [] }); }); it('should call courseGroupRule.hasPermission', () => { - const course = courseFactory.build({ teachers: [user] }); + const course = courseFactory.build({ teachers: [globalUser] }); const courseGroup = courseGroupFactory.build({ course }); entity = lessonFactory.build({ course: undefined, courseGroup }); const spy = jest.spyOn(courseGroupRule, 'hasPermission'); - service.hasPermission(user, entity, { action: Action.write, requiredPermissions: [permissionA] }); - expect(spy).toBeCalledWith(user, entity.courseGroup, { action: Action.write, requiredPermissions: [] }); + rule.hasPermission(globalUser, entity, { action: Action.write, requiredPermissions: [permissionA] }); + expect(spy).toBeCalledWith(globalUser, entity.courseGroup, { action: Action.write, requiredPermissions: [] }); + }); + + describe('Given user request not implemented action', () => { + const getContext = (): AuthorizationContext => { + const context: AuthorizationContext = { + requiredPermissions: [], + // @ts-expect-error Testcase + action: 'not_implemented', + }; + + return context; + }; + + describe('when valid data exists', () => { + const setup = () => { + const user = userFactory.build(); + const course = courseFactory.build({ teachers: [user] }); + const lesson = lessonFactory.build({ course }); + const context = getContext(); + + return { + user, + lesson, + context, + }; + }; + + it('should reject with NotImplementedException', () => { + const { user, lesson, context } = setup(); + + expect(() => rule.hasPermission(user, lesson, context)).toThrowError(NotImplementedException); + }); + }); + }); + + describe('Given user request Action.write', () => { + const getWriteContext = () => AuthorizationContextBuilder.write([]); + + describe('when lesson has no course or coursegroup', () => { + const setup = () => { + const user = userFactory.build(); + const lessonEntity = lessonFactory.build({ course: undefined }); + const context = getWriteContext(); + + return { + user, + lessonEntity, + context, + }; + }; + + it('should return false', () => { + const { user, lessonEntity, context } = setup(); + + const result = rule.hasPermission(user, lessonEntity, context); + + expect(result).toBe(false); + }); + }); }); describe('User [TEACHER]', () => { it('should return "true" if user in scope', () => { - const course = courseFactory.build({ teachers: [user] }); + const course = courseFactory.build({ teachers: [globalUser] }); entity = lessonFactory.build({ course }); - const res = service.hasPermission(user, entity, { action: Action.read, requiredPermissions: [permissionA] }); + const res = rule.hasPermission(globalUser, entity, { action: Action.read, requiredPermissions: [permissionA] }); expect(res).toBe(true); }); it('should return "true" if user has access to hidden entity', () => { - const course = courseFactory.build({ teachers: [user] }); + const course = courseFactory.build({ teachers: [globalUser] }); entity = lessonFactory.build({ course, hidden: true }); - const res = service.hasPermission(user, entity, { action: Action.read, requiredPermissions: [permissionA] }); + const res = rule.hasPermission(globalUser, entity, { action: Action.read, requiredPermissions: [permissionA] }); expect(res).toBe(true); }); it('should return "false" if user has not permission', () => { entity = lessonFactory.build(); - const res = service.hasPermission(user, entity, { action: Action.read, requiredPermissions: [permissionC] }); + const res = rule.hasPermission(globalUser, entity, { action: Action.read, requiredPermissions: [permissionC] }); expect(res).toBe(false); }); it('should return "false" if user has not access to entity', () => { entity = lessonFactory.build(); - const res = service.hasPermission(user, entity, { action: Action.read, requiredPermissions: [permissionC] }); + const res = rule.hasPermission(globalUser, entity, { action: Action.read, requiredPermissions: [permissionC] }); expect(res).toBe(false); }); }); @@ -106,14 +168,14 @@ describe('LessonRule', () => { it('should return "false" if user has access to entity', () => { const course = courseFactory.build({ students: [student] }); entity = lessonFactory.build({ course }); - const res = service.hasPermission(student, entity, { action: Action.read, requiredPermissions: [permissionA] }); + const res = rule.hasPermission(student, entity, { action: Action.read, requiredPermissions: [permissionA] }); expect(res).toBe(true); }); it('should return "false" if user has not access to hidden entity', () => { const course = courseFactory.build({ students: [student] }); entity = lessonFactory.build({ course, hidden: true }); - const res = service.hasPermission(student, entity, { action: Action.read, requiredPermissions: [permissionA] }); + const res = rule.hasPermission(student, entity, { action: Action.read, requiredPermissions: [permissionA] }); expect(res).toBe(false); }); }); diff --git a/apps/server/src/shared/domain/rules/lesson.rule.ts b/apps/server/src/modules/authorization/domain/rules/lesson.rule.ts similarity index 88% rename from apps/server/src/shared/domain/rules/lesson.rule.ts rename to apps/server/src/modules/authorization/domain/rules/lesson.rule.ts index ff264af13ae..1f59f98ad49 100644 --- a/apps/server/src/shared/domain/rules/lesson.rule.ts +++ b/apps/server/src/modules/authorization/domain/rules/lesson.rule.ts @@ -1,7 +1,7 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, NotImplementedException } from '@nestjs/common'; import { Course, CourseGroup, LessonEntity, User } from '@shared/domain/entity'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action, AuthorizationContext, Rule } from '@src/modules/authorization/types'; +import { Action, AuthorizationContext, Rule } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; import { CourseGroupRule } from './course-group.rule'; import { CourseRule } from './course.rule'; @@ -27,6 +27,8 @@ export class LessonRule implements Rule { hasLessonPermission = this.lessonReadPermission(user, entity); } else if (action === Action.write) { hasLessonPermission = this.lessonWritePermission(user, entity); + } else { + throw new NotImplementedException('Action is not supported.'); } const hasUserPermission = this.authorizationHelper.hasAllPermissions(user, requiredPermissions); @@ -55,12 +57,14 @@ export class LessonRule implements Rule { } private parentPermission(user: User, entity: LessonEntity, action: Action): boolean { - let result = false; + let result: boolean; if (entity.courseGroup) { result = this.courseGroupPermission(user, entity.courseGroup, action); } else if (entity.course) { result = this.coursePermission(user, entity.course, action); // ask course for student = read || teacher, sub-teacher = write + } else { + result = false; } return result; diff --git a/apps/server/src/shared/domain/rules/school-external-tool.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/school-external-tool.rule.spec.ts similarity index 92% rename from apps/server/src/shared/domain/rules/school-external-tool.rule.spec.ts rename to apps/server/src/modules/authorization/domain/rules/school-external-tool.rule.spec.ts index b24ed4d0ac8..d1781bb8576 100644 --- a/apps/server/src/shared/domain/rules/school-external-tool.rule.spec.ts +++ b/apps/server/src/modules/authorization/domain/rules/school-external-tool.rule.spec.ts @@ -7,13 +7,11 @@ import { userFactory, schoolExternalToolFactory, } from '@shared/testing'; - -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action } from '@src/modules/authorization/types'; import { SchoolExternalTool } from '@src/modules/tool/school-external-tool/domain'; import { SchoolExternalToolEntity } from '@src/modules/tool/school-external-tool/entity'; -import { Role, User } from '../entity'; -import { Permission } from '../interface'; +import { Role, User, Permission } from '@shared/domain'; +import { Action } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; import { SchoolExternalToolRule } from './school-external-tool.rule'; describe('SchoolExternalToolRule', () => { diff --git a/apps/server/src/shared/domain/rules/school-external-tool.rule.ts b/apps/server/src/modules/authorization/domain/rules/school-external-tool.rule.ts similarity index 85% rename from apps/server/src/shared/domain/rules/school-external-tool.rule.ts rename to apps/server/src/modules/authorization/domain/rules/school-external-tool.rule.ts index bd28502faa2..041c8b523e2 100644 --- a/apps/server/src/shared/domain/rules/school-external-tool.rule.ts +++ b/apps/server/src/modules/authorization/domain/rules/school-external-tool.rule.ts @@ -1,9 +1,9 @@ import { Injectable } from '@nestjs/common'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { AuthorizationContext, Rule } from '@src/modules/authorization/types'; import { SchoolExternalTool } from '@src/modules/tool/school-external-tool/domain'; import { SchoolExternalToolEntity } from '@src/modules/tool/school-external-tool/entity'; -import { User } from '../entity'; +import { User } from '@shared/domain/entity'; +import { AuthorizationContext, Rule } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; @Injectable() export class SchoolExternalToolRule implements Rule { diff --git a/apps/server/src/shared/domain/rules/submission.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/submission.rule.spec.ts similarity index 93% rename from apps/server/src/shared/domain/rules/submission.rule.spec.ts rename to apps/server/src/modules/authorization/domain/rules/submission.rule.spec.ts index 098f83547e8..8b054671970 100644 --- a/apps/server/src/shared/domain/rules/submission.rule.spec.ts +++ b/apps/server/src/modules/authorization/domain/rules/submission.rule.spec.ts @@ -9,9 +9,14 @@ import { taskFactory, userFactory, } from '@shared/testing'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action } from '@src/modules/authorization/types'; -import { CourseGroupRule, CourseRule, LessonRule, SubmissionRule, TaskRule } from '.'; +import { NotImplementedException } from '@nestjs/common'; +import { Action, AuthorizationContext } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; +import { SubmissionRule } from './submission.rule'; +import { TaskRule } from './task.rule'; +import { CourseRule } from './course.rule'; +import { LessonRule } from './lesson.rule'; +import { CourseGroupRule } from './course-group.rule'; const buildUserWithPermission = (permission) => { const role = roleFactory.buildWithId({ permissions: [permission] }); @@ -76,6 +81,38 @@ describe('SubmissionRule', () => { }); describe('hasPermission', () => { + describe('Given user request not implemented action', () => { + const getContext = (): AuthorizationContext => { + const context: AuthorizationContext = { + requiredPermissions: [], + // @ts-expect-error Testcase + action: 'not_implemented', + }; + + return context; + }; + + describe('when valid data exists', () => { + const setup = () => { + const user = userFactory.build(); + const submission = submissionFactory.build({ student: user }); + const context = getContext(); + + return { + user, + submission, + context, + }; + }; + + it('should reject with NotImplementedException', () => { + const { user, submission, context } = setup(); + + expect(() => submissionRule.hasPermission(user, submission, context)).toThrowError(NotImplementedException); + }); + }); + }); + describe('when user roles do not contain required permissions', () => { const setup = () => { const permission = 'a' as Permission; diff --git a/apps/server/src/shared/domain/rules/submission.rule.ts b/apps/server/src/modules/authorization/domain/rules/submission.rule.ts similarity index 88% rename from apps/server/src/shared/domain/rules/submission.rule.ts rename to apps/server/src/modules/authorization/domain/rules/submission.rule.ts index 3234f8e8cff..6bff9504f5c 100644 --- a/apps/server/src/shared/domain/rules/submission.rule.ts +++ b/apps/server/src/modules/authorization/domain/rules/submission.rule.ts @@ -1,7 +1,7 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, NotImplementedException } from '@nestjs/common'; import { Submission, User } from '@shared/domain/entity'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action, AuthorizationContext, Rule } from '@src/modules/authorization/types'; +import { Action, AuthorizationContext, Rule } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; import { TaskRule } from './task.rule'; @Injectable() @@ -31,6 +31,8 @@ export class SubmissionRule implements Rule { hasAccessToSubmission = this.hasWriteAccess(user, submission); } else if (action === Action.read) { hasAccessToSubmission = this.hasReadAccess(user, submission); + } else { + throw new NotImplementedException('Action is not supported.'); } return hasAccessToSubmission; diff --git a/apps/server/src/shared/domain/rules/task.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/task.rule.spec.ts similarity index 96% rename from apps/server/src/shared/domain/rules/task.rule.spec.ts rename to apps/server/src/modules/authorization/domain/rules/task.rule.spec.ts index 0fc886df84f..31d68661ff0 100644 --- a/apps/server/src/shared/domain/rules/task.rule.spec.ts +++ b/apps/server/src/modules/authorization/domain/rules/task.rule.spec.ts @@ -2,9 +2,12 @@ import { DeepPartial } from '@mikro-orm/core'; import { Test, TestingModule } from '@nestjs/testing'; import { Permission, RoleName } from '@shared/domain/interface'; import { courseFactory, lessonFactory, roleFactory, setupEntities, taskFactory, userFactory } from '@shared/testing'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { CourseGroupRule, CourseRule, LessonRule, TaskRule } from '.'; -import { Action } from '../../../modules/authorization/types/action.enum'; +import { Action } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; +import { CourseGroupRule } from './course-group.rule'; +import { TaskRule } from './task.rule'; +import { CourseRule } from './course.rule'; +import { LessonRule } from './lesson.rule'; describe('TaskRule', () => { let service: TaskRule; diff --git a/apps/server/src/shared/domain/rules/task.rule.ts b/apps/server/src/modules/authorization/domain/rules/task.rule.ts similarity index 90% rename from apps/server/src/shared/domain/rules/task.rule.ts rename to apps/server/src/modules/authorization/domain/rules/task.rule.ts index 4c358593109..3ebc04d9f71 100644 --- a/apps/server/src/shared/domain/rules/task.rule.ts +++ b/apps/server/src/modules/authorization/domain/rules/task.rule.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { Task, User } from '@shared/domain/entity'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action, AuthorizationContext, Rule } from '@src/modules/authorization/types'; +import { Action, AuthorizationContext, Rule } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; import { CourseRule } from './course.rule'; import { LessonRule } from './lesson.rule'; diff --git a/apps/server/src/shared/domain/rules/team.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/team.rule.spec.ts similarity index 91% rename from apps/server/src/shared/domain/rules/team.rule.spec.ts rename to apps/server/src/modules/authorization/domain/rules/team.rule.spec.ts index d29eaaaa6f8..da99354a49b 100644 --- a/apps/server/src/shared/domain/rules/team.rule.spec.ts +++ b/apps/server/src/modules/authorization/domain/rules/team.rule.spec.ts @@ -1,10 +1,9 @@ import { Test, TestingModule } from '@nestjs/testing'; import { Permission } from '@shared/domain/interface'; -import { roleFactory, setupEntities, userFactory } from '@shared/testing'; -import { teamFactory } from '@shared/testing/factory/team.factory'; -import { TeamRule } from '@shared/domain/rules/team.rule'; -import { AuthorizationContextBuilder } from '@src/modules/authorization/authorization-context.builder'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; +import { roleFactory, setupEntities, userFactory, teamFactory } from '@shared/testing'; +import { AuthorizationHelper } from '../service/authorization.helper'; +import { TeamRule } from './team.rule'; +import { AuthorizationContextBuilder } from '../mapper'; describe('TeamRule', () => { let rule: TeamRule; diff --git a/apps/server/src/shared/domain/rules/team.rule.ts b/apps/server/src/modules/authorization/domain/rules/team.rule.ts similarity index 81% rename from apps/server/src/shared/domain/rules/team.rule.ts rename to apps/server/src/modules/authorization/domain/rules/team.rule.ts index 23ad0d55cf7..2d8f5e90edf 100644 --- a/apps/server/src/shared/domain/rules/team.rule.ts +++ b/apps/server/src/modules/authorization/domain/rules/team.rule.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { TeamEntity, TeamUserEntity, User } from '@shared/domain/entity'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { AuthorizationContext, Rule } from '@src/modules/authorization/types'; +import { AuthorizationContext, Rule } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; @Injectable() export class TeamRule implements Rule { diff --git a/apps/server/src/shared/domain/rules/user-login-migration.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/user-login-migration.rule.spec.ts similarity index 94% rename from apps/server/src/shared/domain/rules/user-login-migration.rule.spec.ts rename to apps/server/src/modules/authorization/domain/rules/user-login-migration.rule.spec.ts index a7c6b1e5f7a..f7fe9d3c53f 100644 --- a/apps/server/src/shared/domain/rules/user-login-migration.rule.spec.ts +++ b/apps/server/src/modules/authorization/domain/rules/user-login-migration.rule.spec.ts @@ -2,10 +2,10 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { ObjectId } from '@mikro-orm/mongodb'; import { Test, TestingModule } from '@nestjs/testing'; import { schoolFactory, setupEntities, userFactory, userLoginMigrationDOFactory } from '@shared/testing'; -import { Action, AuthorizationContext } from '@src/modules/authorization'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { UserLoginMigrationDO } from '../domainobject'; -import { Permission } from '../interface'; +import { UserLoginMigrationDO } from '@shared/domain/domainobject'; +import { Permission } from '@shared/domain/interface'; +import { Action, AuthorizationContext } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; import { UserLoginMigrationRule } from './user-login-migration.rule'; describe('UserLoginMigrationRule', () => { diff --git a/apps/server/src/shared/domain/rules/user-login-migration.rule.ts b/apps/server/src/modules/authorization/domain/rules/user-login-migration.rule.ts similarity index 72% rename from apps/server/src/shared/domain/rules/user-login-migration.rule.ts rename to apps/server/src/modules/authorization/domain/rules/user-login-migration.rule.ts index 084e4d26372..3ae82d02505 100644 --- a/apps/server/src/shared/domain/rules/user-login-migration.rule.ts +++ b/apps/server/src/modules/authorization/domain/rules/user-login-migration.rule.ts @@ -1,8 +1,8 @@ import { Injectable } from '@nestjs/common'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { AuthorizationContext, Rule } from '@src/modules/authorization/types'; -import { UserLoginMigrationDO } from '../domainobject'; -import { User } from '../entity'; +import { UserLoginMigrationDO } from '@shared/domain/domainobject'; +import { User } from '@shared/domain/entity'; +import { AuthorizationContext, Rule } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; @Injectable() export class UserLoginMigrationRule implements Rule { diff --git a/apps/server/src/shared/domain/rules/user.rule.spec.ts b/apps/server/src/modules/authorization/domain/rules/user.rule.spec.ts similarity index 94% rename from apps/server/src/shared/domain/rules/user.rule.spec.ts rename to apps/server/src/modules/authorization/domain/rules/user.rule.spec.ts index bd771961ed3..85492348f75 100644 --- a/apps/server/src/shared/domain/rules/user.rule.spec.ts +++ b/apps/server/src/modules/authorization/domain/rules/user.rule.spec.ts @@ -2,8 +2,8 @@ import { Test, TestingModule } from '@nestjs/testing'; import { Role, User } from '@shared/domain/entity'; import { Permission } from '@shared/domain/interface'; import { roleFactory, setupEntities, userFactory } from '@shared/testing'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { Action } from '@src/modules/authorization/types'; +import { Action } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; import { UserRule } from './user.rule'; describe('UserRule', () => { diff --git a/apps/server/src/shared/domain/rules/user.rule.ts b/apps/server/src/modules/authorization/domain/rules/user.rule.ts similarity index 78% rename from apps/server/src/shared/domain/rules/user.rule.ts rename to apps/server/src/modules/authorization/domain/rules/user.rule.ts index 3dd9bc6d229..2a1365881e1 100644 --- a/apps/server/src/shared/domain/rules/user.rule.ts +++ b/apps/server/src/modules/authorization/domain/rules/user.rule.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { User } from '@shared/domain/entity'; -import { AuthorizationHelper } from '@src/modules/authorization/authorization.helper'; -import { AuthorizationContext, Rule } from '@src/modules/authorization/types'; +import { AuthorizationContext, Rule } from '../type'; +import { AuthorizationHelper } from '../service/authorization.helper'; @Injectable() export class UserRule implements Rule { diff --git a/apps/server/src/modules/authorization/domain/service/authorization-reference.service.spec.ts b/apps/server/src/modules/authorization/domain/service/authorization-reference.service.spec.ts new file mode 100644 index 00000000000..8ab1719a72d --- /dev/null +++ b/apps/server/src/modules/authorization/domain/service/authorization-reference.service.spec.ts @@ -0,0 +1,183 @@ +import { createMock, DeepMocked } from '@golevelup/ts-jest'; +import { NotFoundException } from '@nestjs/common'; +import { Test, TestingModule } from '@nestjs/testing'; +import { courseFactory, setupEntities, userFactory } from '@shared/testing'; +import { ObjectId } from 'bson'; +import { AuthorizableReferenceType } from '../type'; +import { AuthorizationService } from './authorization.service'; +import { ReferenceLoader } from './reference.loader'; +import { AuthorizationContextBuilder } from '../mapper'; +import { ForbiddenLoggableException } from '../error'; +import { AuthorizationReferenceService } from './authorization-reference.service'; + +describe('AuthorizationReferenceService', () => { + let service: AuthorizationReferenceService; + let authorizationService: DeepMocked; + let loader: DeepMocked; + + beforeAll(async () => { + await setupEntities(); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + AuthorizationReferenceService, + { + provide: AuthorizationService, + useValue: createMock(), + }, + { + provide: ReferenceLoader, + useValue: createMock(), + }, + ], + }).compile(); + + service = await module.get(AuthorizationReferenceService); + authorizationService = await module.get(AuthorizationService); + loader = await module.get(ReferenceLoader); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('checkPermissionByReferences', () => { + const setupData = () => { + const entityId = new ObjectId().toHexString(); + const userId = new ObjectId().toHexString(); + const context = AuthorizationContextBuilder.read([]); + const entityName = AuthorizableReferenceType.Course; + + return { context, entityId, userId, entityName }; + }; + + describe('when hasPermissionByReferences returns false', () => { + const setup = () => { + const { entityId, userId, context, entityName } = setupData(); + + const spy = jest.spyOn(service, 'hasPermissionByReferences').mockResolvedValueOnce(false); + + return { context, userId, entityId, entityName, spy }; + }; + + it('should reject with ForbiddenLoggableException', async () => { + const { context, userId, entityId, entityName, spy } = setup(); + + await expect(service.checkPermissionByReferences(userId, entityName, entityId, context)).rejects.toThrow( + new ForbiddenLoggableException(userId, entityName, context) + ); + + spy.mockRestore(); + }); + }); + + describe('when hasPermissionByReferences returns true', () => { + const setup = () => { + const { entityId, userId, context, entityName } = setupData(); + + const spy = jest.spyOn(service, 'hasPermissionByReferences').mockResolvedValueOnce(true); + + return { context, userId, entityId, entityName, spy }; + }; + + it('should resolve without error', async () => { + const { context, userId, entityId, entityName, spy } = setup(); + + await expect(service.checkPermissionByReferences(userId, entityName, entityId, context)).resolves.not.toThrow(); + + spy.mockRestore(); + }); + }); + }); + + describe('hasPermissionByReferences', () => { + const setupData = () => { + const entity = courseFactory.buildWithId(); + const user = userFactory.buildWithId(); + const context = AuthorizationContextBuilder.read([]); + const entityName = AuthorizableReferenceType.Course; + + return { context, entity, user, entityName }; + }; + + describe('when loader throws an error', () => { + const setup = () => { + const { entity, user, context, entityName } = setupData(); + + loader.loadAuthorizableObject.mockRejectedValueOnce(new NotFoundException()); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + authorizationService.hasPermission.mockReturnValueOnce(true); + + return { context, userId: user.id, entityId: entity.id, entityName }; + }; + + it('should reject with this error', async () => { + const { context, userId, entityId, entityName } = setup(); + + await expect(service.hasPermissionByReferences(userId, entityName, entityId, context)).rejects.toThrow( + new NotFoundException() + ); + }); + }); + + describe('when authorizationService throws an error', () => { + const setup = () => { + const { entity, user, context, entityName } = setupData(); + + loader.loadAuthorizableObject.mockRejectedValueOnce(entity); + authorizationService.getUserWithPermissions.mockRejectedValueOnce(new NotFoundException()); + authorizationService.hasPermission.mockReturnValueOnce(true); + + return { context, userId: user.id, entityId: entity.id, entityName }; + }; + + it('should reject with this error', async () => { + const { context, userId, entityId, entityName } = setup(); + + await expect(service.hasPermissionByReferences(userId, entityName, entityId, context)).rejects.toThrow( + new NotFoundException() + ); + }); + }); + + describe('when loader can load entites and authorization resolve with true', () => { + const setup = () => { + const { entity, user, context, entityName } = setupData(); + + loader.loadAuthorizableObject.mockResolvedValueOnce(entity); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + authorizationService.hasPermission.mockReturnValueOnce(true); + + return { context, userId: user.id, entityId: entity.id, entityName }; + }; + + it('should resolve to true', async () => { + const { context, userId, entityId, entityName } = setup(); + + const result = await service.hasPermissionByReferences(userId, entityName, entityId, context); + + expect(result).toBe(true); + }); + }); + + describe('when loader can load entities and authorization resolve with false', () => { + const setup = () => { + const { entity, user, context, entityName } = setupData(); + + loader.loadAuthorizableObject.mockResolvedValueOnce(entity); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + authorizationService.hasPermission.mockReturnValueOnce(false); + + return { context, userId: user.id, entityId: entity.id, entityName }; + }; + + it('should resolve to false', async () => { + const { context, userId, entityId, entityName } = setup(); + + const result = await service.hasPermissionByReferences(userId, entityName, entityId, context); + + expect(result).toBe(false); + }); + }); + }); +}); diff --git a/apps/server/src/modules/authorization/domain/service/authorization-reference.service.ts b/apps/server/src/modules/authorization/domain/service/authorization-reference.service.ts new file mode 100644 index 00000000000..814df9378da --- /dev/null +++ b/apps/server/src/modules/authorization/domain/service/authorization-reference.service.ts @@ -0,0 +1,41 @@ +import { Injectable } from '@nestjs/common'; +import { EntityId } from '@shared/domain'; +import { ReferenceLoader } from './reference.loader'; +import { AuthorizationContext, AuthorizableReferenceType } from '../type'; +import { ForbiddenLoggableException } from '../error'; +import { AuthorizationService } from './authorization.service'; + +/** + * Should by use only internal in authorization module. See ticket: BC-3990 + */ +@Injectable() +export class AuthorizationReferenceService { + constructor(private readonly loader: ReferenceLoader, private readonly authorizationService: AuthorizationService) {} + + public async checkPermissionByReferences( + userId: EntityId, + entityName: AuthorizableReferenceType, + entityId: EntityId, + context: AuthorizationContext + ): Promise { + if (!(await this.hasPermissionByReferences(userId, entityName, entityId, context))) { + throw new ForbiddenLoggableException(userId, entityName, context); + } + } + + public async hasPermissionByReferences( + userId: EntityId, + entityName: AuthorizableReferenceType, + entityId: EntityId, + context: AuthorizationContext + ): Promise { + const [user, object] = await Promise.all([ + this.authorizationService.getUserWithPermissions(userId), + this.loader.loadAuthorizableObject(entityName, entityId), + ]); + + const hasPermission = this.authorizationService.hasPermission(user, object, context); + + return hasPermission; + } +} diff --git a/apps/server/src/modules/authorization/authorization.helper.spec.ts b/apps/server/src/modules/authorization/domain/service/authorization.helper.spec.ts similarity index 100% rename from apps/server/src/modules/authorization/authorization.helper.spec.ts rename to apps/server/src/modules/authorization/domain/service/authorization.helper.spec.ts diff --git a/apps/server/src/modules/authorization/authorization.helper.ts b/apps/server/src/modules/authorization/domain/service/authorization.helper.ts similarity index 100% rename from apps/server/src/modules/authorization/authorization.helper.ts rename to apps/server/src/modules/authorization/domain/service/authorization.helper.ts diff --git a/apps/server/src/modules/authorization/authorization.service.spec.ts b/apps/server/src/modules/authorization/domain/service/authorization.service.spec.ts similarity index 60% rename from apps/server/src/modules/authorization/authorization.service.spec.ts rename to apps/server/src/modules/authorization/domain/service/authorization.service.spec.ts index 766c2c84d23..f113c64472c 100644 --- a/apps/server/src/modules/authorization/authorization.service.spec.ts +++ b/apps/server/src/modules/authorization/domain/service/authorization.service.spec.ts @@ -1,33 +1,34 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { ForbiddenException, InternalServerErrorException, UnauthorizedException } from '@nestjs/common'; +import { UnauthorizedException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { Permission } from '@shared/domain'; import { setupEntities, userFactory } from '@shared/testing'; -import { AuthorizationContextBuilder } from './authorization-context.builder'; +import { UserRepo } from '@shared/repo'; +import { AuthorizationContextBuilder } from '../mapper'; import { AuthorizationHelper } from './authorization.helper'; import { AuthorizationService } from './authorization.service'; -import { ForbiddenLoggableException } from './errors/forbidden.loggable-exception'; +import { ForbiddenLoggableException } from '../error'; import { ReferenceLoader } from './reference.loader'; import { RuleManager } from './rule-manager'; -import { AuthorizableReferenceType, Rule } from './types'; +import { Rule } from '../type'; -describe('AuthorizationService', () => { - class TestRule implements Rule { - constructor(private returnValueOfhasPermission: boolean) {} +class TestRule implements Rule { + constructor(private returnValueOfhasPermission: boolean) {} - isApplicable(): boolean { - return true; - } + isApplicable(): boolean { + return true; + } - hasPermission(): boolean { - return this.returnValueOfhasPermission; - } + hasPermission(): boolean { + return this.returnValueOfhasPermission; } +} +describe('AuthorizationService', () => { let service: AuthorizationService; let ruleManager: DeepMocked; - let loader: DeepMocked; let authorizationHelper: DeepMocked; + let userRepo: DeepMocked; const testPermission = 'CAN_TEST' as Permission; @@ -49,13 +50,17 @@ describe('AuthorizationService', () => { provide: AuthorizationHelper, useValue: createMock(), }, + { + provide: UserRepo, + useValue: createMock(), + }, ], }).compile(); service = await module.get(AuthorizationService); ruleManager = await module.get(RuleManager); - loader = await module.get(ReferenceLoader); authorizationHelper = await module.get(AuthorizationHelper); + userRepo = await module.get(UserRepo); }); afterEach(() => { @@ -66,7 +71,7 @@ describe('AuthorizationService', () => { describe('when hasPermission returns false', () => { const setup = () => { const context = AuthorizationContextBuilder.read([]); - const user = userFactory.build(); + const user = userFactory.buildWithId(); const spy = jest.spyOn(service, 'hasPermission').mockReturnValueOnce(false); @@ -85,7 +90,7 @@ describe('AuthorizationService', () => { describe('when hasPermission returns true', () => { const setup = () => { const context = AuthorizationContextBuilder.read([]); - const user = userFactory.build(); + const user = userFactory.buildWithId(); const spy = jest.spyOn(service, 'hasPermission').mockReturnValueOnce(true); @@ -106,7 +111,7 @@ describe('AuthorizationService', () => { describe('when the selected rule returns false', () => { const setup = () => { const context = AuthorizationContextBuilder.read([]); - const user = userFactory.build(); + const user = userFactory.buildWithId(); const testRule = new TestRule(false); ruleManager.selectRule.mockReturnValueOnce(testRule); @@ -126,7 +131,7 @@ describe('AuthorizationService', () => { describe('when the selected rule returns true', () => { const setup = () => { const context = AuthorizationContextBuilder.read([]); - const user = userFactory.build(); + const user = userFactory.buildWithId(); const testRule = new TestRule(true); ruleManager.selectRule.mockReturnValueOnce(testRule); @@ -144,123 +149,10 @@ describe('AuthorizationService', () => { }); }); - describe('checkPermissionByReferences', () => { - describe('when hasPermissionByReferences returns false', () => { - const setup = () => { - const context = AuthorizationContextBuilder.read([]); - const userId = 'test'; - const entityId = 'test'; - const entityName = AuthorizableReferenceType.Course; - - const spy = jest.spyOn(service, 'hasPermissionByReferences').mockResolvedValueOnce(false); - - return { context, userId, entityId, entityName, spy }; - }; - - it('should reject with ForbiddenLoggableException', async () => { - const { context, userId, entityId, entityName, spy } = setup(); - - await expect(service.checkPermissionByReferences(userId, entityName, entityId, context)).rejects.toThrow( - ForbiddenLoggableException - ); - - spy.mockRestore(); - }); - }); - - describe('when hasPermissionByReferences returns true', () => { - const setup = () => { - const context = AuthorizationContextBuilder.read([]); - const userId = 'test'; - const entityId = 'test'; - const entityName = AuthorizableReferenceType.Course; - - const spy = jest.spyOn(service, 'hasPermissionByReferences').mockResolvedValueOnce(true); - - return { context, userId, entityId, entityName, spy }; - }; - - it('should resolve', async () => { - const { context, userId, entityId, entityName, spy } = setup(); - - await expect(service.checkPermissionByReferences(userId, entityName, entityId, context)).resolves.not.toThrow(); - - spy.mockRestore(); - }); - }); - }); - - describe('hasPermissionByReferences', () => { - describe('when loader throws an error', () => { - const setup = () => { - const context = AuthorizationContextBuilder.read([]); - const userId = 'test'; - const entityId = 'test'; - const entityName = AuthorizableReferenceType.Course; - - loader.loadAuthorizableObject.mockRejectedValueOnce(InternalServerErrorException); - - return { context, userId, entityId, entityName }; - }; - - it('should reject with ForbiddenException', async () => { - const { context, userId, entityId, entityName } = setup(); - - await expect(service.hasPermissionByReferences(userId, entityName, entityId, context)).rejects.toThrow( - ForbiddenException - ); - }); - }); - - describe('when the selected rule returns true', () => { - const setup = () => { - const context = AuthorizationContextBuilder.read([]); - const userId = 'test'; - const entityId = 'test'; - const entityName = AuthorizableReferenceType.Course; - const testRule = new TestRule(true); - - ruleManager.selectRule.mockReturnValueOnce(testRule); - - return { context, userId, entityId, entityName }; - }; - - it('should resolve to true', async () => { - const { context, userId, entityId, entityName } = setup(); - - const result = await service.hasPermissionByReferences(userId, entityName, entityId, context); - - expect(result).toBe(true); - }); - }); - - describe('when the selected rule returns false', () => { - const setup = () => { - const context = AuthorizationContextBuilder.read([]); - const userId = 'test'; - const entityId = 'test'; - const entityName = AuthorizableReferenceType.Course; - const testRule = new TestRule(false); - - ruleManager.selectRule.mockReturnValueOnce(testRule); - - return { context, userId, entityId, entityName }; - }; - - it('should resolve to false', async () => { - const { context, userId, entityId, entityName } = setup(); - - const result = await service.hasPermissionByReferences(userId, entityName, entityId, context); - - expect(result).toBe(false); - }); - }); - }); - describe('checkAllPermissions', () => { describe('when helper method returns false', () => { const setup = () => { - const user = userFactory.build(); + const user = userFactory.buildWithId(); const requiredPermissions = [testPermission]; authorizationHelper.hasAllPermissions.mockReturnValueOnce(false); @@ -277,7 +169,7 @@ describe('AuthorizationService', () => { describe('when helper method returns true', () => { const setup = () => { - const user = userFactory.build(); + const user = userFactory.buildWithId(); const requiredPermissions = [testPermission]; authorizationHelper.hasAllPermissions.mockReturnValueOnce(true); @@ -296,7 +188,7 @@ describe('AuthorizationService', () => { describe('hasAllPermissions', () => { describe('when helper method returns false', () => { const setup = () => { - const user = userFactory.build(); + const user = userFactory.buildWithId(); const requiredPermissions = [testPermission]; authorizationHelper.hasAllPermissions.mockReturnValueOnce(false); @@ -315,7 +207,7 @@ describe('AuthorizationService', () => { describe('when helper method returns true', () => { const setup = () => { - const user = userFactory.build(); + const user = userFactory.buildWithId(); const requiredPermissions = [testPermission]; authorizationHelper.hasAllPermissions.mockReturnValueOnce(true); @@ -336,7 +228,7 @@ describe('AuthorizationService', () => { describe('checkOneOfPermissions', () => { describe('when helper method returns false', () => { const setup = () => { - const user = userFactory.build(); + const user = userFactory.buildWithId(); const requiredPermissions = [testPermission]; authorizationHelper.hasOneOfPermissions.mockReturnValueOnce(false); @@ -353,7 +245,7 @@ describe('AuthorizationService', () => { describe('when helper method returns true', () => { const setup = () => { - const user = userFactory.build(); + const user = userFactory.buildWithId(); const requiredPermissions = [testPermission]; authorizationHelper.hasOneOfPermissions.mockReturnValueOnce(true); @@ -372,7 +264,7 @@ describe('AuthorizationService', () => { describe('hasOneOfPermissions', () => { describe('when helper method returns false', () => { const setup = () => { - const user = userFactory.build(); + const user = userFactory.buildWithId(); const requiredPermissions = [testPermission]; authorizationHelper.hasOneOfPermissions.mockReturnValueOnce(false); @@ -391,7 +283,7 @@ describe('AuthorizationService', () => { describe('when helper method returns true', () => { const setup = () => { - const user = userFactory.build(); + const user = userFactory.buildWithId(); const requiredPermissions = [testPermission]; authorizationHelper.hasOneOfPermissions.mockReturnValueOnce(true); @@ -410,12 +302,18 @@ describe('AuthorizationService', () => { }); describe('getUserWithPermissions', () => { + const setup = () => { + const user = userFactory.buildWithId(); + + userRepo.findById.mockResolvedValueOnce(user); + + return { user }; + }; + it('should return user received from loader', async () => { - const userId = 'test'; - const user = userFactory.build(); - loader.getUserWithPermissions.mockResolvedValueOnce(user); + const { user } = setup(); - const result = await service.getUserWithPermissions(userId); + const result = await service.getUserWithPermissions(user.id); expect(result).toEqual(user); }); diff --git a/apps/server/src/modules/authorization/domain/service/authorization.service.ts b/apps/server/src/modules/authorization/domain/service/authorization.service.ts new file mode 100644 index 00000000000..5218dffda81 --- /dev/null +++ b/apps/server/src/modules/authorization/domain/service/authorization.service.ts @@ -0,0 +1,59 @@ +import { Injectable, UnauthorizedException } from '@nestjs/common'; +import { BaseDO, EntityId, User } from '@shared/domain'; +import { AuthorizableObject } from '@shared/domain/domain-object'; +import { UserRepo } from '@shared/repo'; +import { AuthorizationHelper } from './authorization.helper'; +import { ForbiddenLoggableException } from '../error'; +import { RuleManager } from './rule-manager'; +import { AuthorizationContext } from '../type'; + +@Injectable() +export class AuthorizationService { + constructor( + private readonly ruleManager: RuleManager, + private readonly authorizationHelper: AuthorizationHelper, + private readonly userRepo: UserRepo + ) {} + + public checkPermission(user: User, object: AuthorizableObject | BaseDO, context: AuthorizationContext): void { + if (!this.hasPermission(user, object, context)) { + throw new ForbiddenLoggableException(user.id, object.constructor.name, context); + } + } + + public hasPermission(user: User, object: AuthorizableObject | BaseDO, context: AuthorizationContext): boolean { + const rule = this.ruleManager.selectRule(user, object, context); + const hasPermission = rule.hasPermission(user, object, context); + + return hasPermission; + } + + public checkAllPermissions(user: User, requiredPermissions: string[]): void { + if (!this.authorizationHelper.hasAllPermissions(user, requiredPermissions)) { + // TODO: Should be ForbiddenLoggableException + throw new UnauthorizedException(); + } + } + + public hasAllPermissions(user: User, requiredPermissions: string[]): boolean { + return this.authorizationHelper.hasAllPermissions(user, requiredPermissions); + } + + public checkOneOfPermissions(user: User, requiredPermissions: string[]): void { + if (!this.authorizationHelper.hasOneOfPermissions(user, requiredPermissions)) { + // TODO: Should be ForbiddenLoggableException + throw new UnauthorizedException(); + } + } + + public hasOneOfPermissions(user: User, requiredPermissions: string[]): boolean { + return this.authorizationHelper.hasOneOfPermissions(user, requiredPermissions); + } + + public async getUserWithPermissions(userId: EntityId): Promise { + // replace with service method getUserWithPermissions BC-5069 + const userWithPopulatedRoles = await this.userRepo.findById(userId, true); + + return userWithPopulatedRoles; + } +} diff --git a/apps/server/src/modules/authorization/domain/service/index.ts b/apps/server/src/modules/authorization/domain/service/index.ts new file mode 100644 index 00000000000..4175cc4b7a7 --- /dev/null +++ b/apps/server/src/modules/authorization/domain/service/index.ts @@ -0,0 +1,5 @@ +export * from './authorization.service'; +export * from './authorization.helper'; +export * from './rule-manager'; +export * from './authorization-reference.service'; +export * from './reference.loader'; diff --git a/apps/server/src/modules/authorization/reference.loader.spec.ts b/apps/server/src/modules/authorization/domain/service/reference.loader.spec.ts similarity index 87% rename from apps/server/src/modules/authorization/reference.loader.spec.ts rename to apps/server/src/modules/authorization/domain/service/reference.loader.spec.ts index 9bba78b1880..0403ebcbfd5 100644 --- a/apps/server/src/modules/authorization/reference.loader.spec.ts +++ b/apps/server/src/modules/authorization/domain/service/reference.loader.spec.ts @@ -14,11 +14,11 @@ import { TeamsRepo, UserRepo, } from '@shared/repo'; -import { roleFactory, setupEntities, userFactory } from '@shared/testing'; +import { setupEntities, userFactory } from '@shared/testing'; import { BoardDoAuthorizableService } from '@src/modules/board'; import { ContextExternalToolAuthorizableService } from '@src/modules/tool/context-external-tool/service/context-external-tool-authorizable.service'; import { ReferenceLoader } from './reference.loader'; -import { AuthorizableReferenceType } from './types'; +import { AuthorizableReferenceType } from '../type'; describe('reference.loader', () => { let service: ReferenceLoader; @@ -138,7 +138,7 @@ describe('reference.loader', () => { it('should call userRepo.findById', async () => { await service.loadAuthorizableObject(AuthorizableReferenceType.User, entityId); - expect(userRepo.findById).toBeCalledWith(entityId, true); + expect(userRepo.findById).toBeCalledWith(entityId); }); it('should call lessonRepo.findById', async () => { @@ -192,33 +192,4 @@ describe('reference.loader', () => { ).rejects.toThrow(NotImplementedException); }); }); - - describe('getUserWithPermissions', () => { - describe('when user successfully', () => { - const setup = () => { - const roles = [roleFactory.build()]; - const user = userFactory.buildWithId({ roles }); - userRepo.findById.mockResolvedValue(user); - return { - user, - }; - }; - - it('should call userRepo.findById with specific arguments', async () => { - const { user } = setup(); - - await service.getUserWithPermissions(user.id); - - expect(userRepo.findById).toBeCalledWith(user.id, true); - }); - - it('should return user', async () => { - const { user } = setup(); - - const result = await service.getUserWithPermissions(user.id); - - expect(result).toBe(user); - }); - }); - }); }); diff --git a/apps/server/src/modules/authorization/reference.loader.ts b/apps/server/src/modules/authorization/domain/service/reference.loader.ts similarity index 88% rename from apps/server/src/modules/authorization/reference.loader.ts rename to apps/server/src/modules/authorization/domain/service/reference.loader.ts index 9afe013fd24..5c38963c6f5 100644 --- a/apps/server/src/modules/authorization/reference.loader.ts +++ b/apps/server/src/modules/authorization/domain/service/reference.loader.ts @@ -1,5 +1,5 @@ import { Injectable, NotImplementedException } from '@nestjs/common'; -import { BaseDO, EntityId, User } from '@shared/domain'; +import { BaseDO, EntityId } from '@shared/domain'; import { AuthorizableObject } from '@shared/domain/domain-object'; import { CourseGroupRepo, @@ -12,11 +12,10 @@ import { TeamsRepo, UserRepo, } from '@shared/repo'; -import { BoardDoAuthorizableService } from '@src/modules/board/service'; +import { BoardDoAuthorizableService } from '@src/modules/board'; import { ContextExternalToolAuthorizableService } from '@src/modules/tool/context-external-tool/service'; -import { AuthorizableReferenceType } from './types'; +import { AuthorizableReferenceType } from '../type'; -// replace later with general "base" do-repo type RepoType = | TaskRepo | CourseRepo @@ -55,7 +54,7 @@ export class ReferenceLoader { this.repos.set(AuthorizableReferenceType.Task, { repo: this.taskRepo }); this.repos.set(AuthorizableReferenceType.Course, { repo: this.courseRepo }); this.repos.set(AuthorizableReferenceType.CourseGroup, { repo: this.courseGroupRepo }); - this.repos.set(AuthorizableReferenceType.User, { repo: this.userRepo, populate: true }); + this.repos.set(AuthorizableReferenceType.User, { repo: this.userRepo }); this.repos.set(AuthorizableReferenceType.School, { repo: this.schoolRepo }); this.repos.set(AuthorizableReferenceType.Lesson, { repo: this.lessonRepo }); this.repos.set(AuthorizableReferenceType.Team, { repo: this.teamsRepo, populate: true }); @@ -90,10 +89,4 @@ export class ReferenceLoader { return object; } - - async getUserWithPermissions(userId: EntityId): Promise { - const user = await this.userRepo.findById(userId, true); - - return user; - } } diff --git a/apps/server/src/modules/authorization/rule-manager.spec.ts b/apps/server/src/modules/authorization/domain/service/rule-manager.spec.ts similarity index 97% rename from apps/server/src/modules/authorization/rule-manager.spec.ts rename to apps/server/src/modules/authorization/domain/service/rule-manager.spec.ts index 0a2b90c7639..78ef313ade1 100644 --- a/apps/server/src/modules/authorization/rule-manager.spec.ts +++ b/apps/server/src/modules/authorization/domain/service/rule-manager.spec.ts @@ -1,6 +1,8 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { InternalServerErrorException, NotImplementedException } from '@nestjs/common'; import { Test } from '@nestjs/testing'; +import { courseFactory, setupEntities, userFactory } from '@shared/testing'; +import { AuthorizationContextBuilder } from '../mapper'; import { BoardDoRule, ContextExternalToolRule, @@ -13,10 +15,8 @@ import { TaskRule, TeamRule, UserRule, -} from '@shared/domain/rules'; -import { UserLoginMigrationRule } from '@shared/domain/rules/user-login-migration.rule'; -import { courseFactory, setupEntities, userFactory } from '@shared/testing'; -import { AuthorizationContextBuilder } from './authorization-context.builder'; + UserLoginMigrationRule, +} from '../rules'; import { RuleManager } from './rule-manager'; describe('RuleManager', () => { diff --git a/apps/server/src/modules/authorization/rule-manager.ts b/apps/server/src/modules/authorization/domain/service/rule-manager.ts similarity index 88% rename from apps/server/src/modules/authorization/rule-manager.ts rename to apps/server/src/modules/authorization/domain/service/rule-manager.ts index 3aece68402a..77d09f284c2 100644 --- a/apps/server/src/modules/authorization/rule-manager.ts +++ b/apps/server/src/modules/authorization/domain/service/rule-manager.ts @@ -1,21 +1,21 @@ import { Injectable, InternalServerErrorException, NotImplementedException } from '@nestjs/common'; import { BaseDO, User } from '@shared/domain'; import { AuthorizableObject } from '@shared/domain/domain-object'; // fix import when it is avaible +import type { AuthorizationContext, Rule } from '../type'; import { BoardDoRule, + ContextExternalToolRule, CourseGroupRule, CourseRule, + LegacySchoolRule, LessonRule, SchoolExternalToolRule, - LegacySchoolRule, SubmissionRule, TaskRule, TeamRule, + UserLoginMigrationRule, UserRule, -} from '@shared/domain/rules'; -import { ContextExternalToolRule } from '@shared/domain/rules/context-external-tool.rule'; -import { UserLoginMigrationRule } from '@shared/domain/rules/user-login-migration.rule'; -import { AuthorizationContext, Rule } from './types'; +} from '../rules'; @Injectable() export class RuleManager { diff --git a/apps/server/src/modules/authorization/types/action.enum.ts b/apps/server/src/modules/authorization/domain/type/action.enum.ts similarity index 100% rename from apps/server/src/modules/authorization/types/action.enum.ts rename to apps/server/src/modules/authorization/domain/type/action.enum.ts diff --git a/apps/server/src/modules/authorization/types/allowed-authorization-object-type.enum.ts b/apps/server/src/modules/authorization/domain/type/allowed-authorization-object-type.enum.ts similarity index 100% rename from apps/server/src/modules/authorization/types/allowed-authorization-object-type.enum.ts rename to apps/server/src/modules/authorization/domain/type/allowed-authorization-object-type.enum.ts diff --git a/apps/server/src/modules/authorization/types/authorization-context.interface.ts b/apps/server/src/modules/authorization/domain/type/authorization-context.interface.ts similarity index 100% rename from apps/server/src/modules/authorization/types/authorization-context.interface.ts rename to apps/server/src/modules/authorization/domain/type/authorization-context.interface.ts diff --git a/apps/server/src/modules/authorization/types/authorization-loader-service.ts b/apps/server/src/modules/authorization/domain/type/authorization-loader-service.ts similarity index 100% rename from apps/server/src/modules/authorization/types/authorization-loader-service.ts rename to apps/server/src/modules/authorization/domain/type/authorization-loader-service.ts diff --git a/apps/server/src/modules/authorization/types/index.ts b/apps/server/src/modules/authorization/domain/type/index.ts similarity index 100% rename from apps/server/src/modules/authorization/types/index.ts rename to apps/server/src/modules/authorization/domain/type/index.ts index 92e7b0c8bf5..b1942491098 100644 --- a/apps/server/src/modules/authorization/types/index.ts +++ b/apps/server/src/modules/authorization/domain/type/index.ts @@ -1,5 +1,5 @@ export * from './action.enum'; export * from './authorization-context.interface'; export * from './rule.interface'; -export * from './allowed-authorization-object-type.enum'; export * from './authorization-loader-service'; +export * from './allowed-authorization-object-type.enum'; diff --git a/apps/server/src/modules/authorization/types/rule.interface.ts b/apps/server/src/modules/authorization/domain/type/rule.interface.ts similarity index 100% rename from apps/server/src/modules/authorization/types/rule.interface.ts rename to apps/server/src/modules/authorization/domain/type/rule.interface.ts diff --git a/apps/server/src/modules/authorization/index.ts b/apps/server/src/modules/authorization/index.ts index bee8b7d4bb1..e129df2cd11 100644 --- a/apps/server/src/modules/authorization/index.ts +++ b/apps/server/src/modules/authorization/index.ts @@ -1,5 +1,15 @@ -export * from './authorization.module'; -export * from './authorization.service'; -export * from './authorization-context.builder'; -export * from './types'; -export * from './feathers'; +export { AuthorizationModule } from './authorization.module'; +export { + AuthorizationService, + AuthorizationHelper, + AuthorizationContextBuilder, + ForbiddenLoggableException, + Rule, + AuthorizationContext, + // Action should not be exported, but hard to solve for now. The AuthorizationContextBuilder is the prefared way + Action, + AuthorizationLoaderService, + AuthorizationLoaderServiceGeneric, +} from './domain'; +// Should not used anymore +export { FeathersAuthorizationService } from './feathers'; diff --git a/apps/server/src/modules/board/uc/board.uc.ts b/apps/server/src/modules/board/uc/board.uc.ts index 7c3194916ac..3e39fd32de3 100644 --- a/apps/server/src/modules/board/uc/board.uc.ts +++ b/apps/server/src/modules/board/uc/board.uc.ts @@ -9,8 +9,8 @@ import { EntityId, } from '@shared/domain'; import { LegacyLogger } from '@src/core/logger'; -import { AuthorizationService } from '@src/modules/authorization/authorization.service'; -import { Action } from '@src/modules/authorization/types/action.enum'; +import { AuthorizationService } from '@src/modules/authorization/domain'; +import { Action } from '@src/modules/authorization'; import { CardService, ColumnBoardService, ColumnService } from '../service'; import { BoardDoAuthorizableService } from '../service/board-do-authorizable.service'; diff --git a/apps/server/src/modules/board/uc/card.uc.ts b/apps/server/src/modules/board/uc/card.uc.ts index 577f3a8b963..170469f0cc4 100644 --- a/apps/server/src/modules/board/uc/card.uc.ts +++ b/apps/server/src/modules/board/uc/card.uc.ts @@ -1,8 +1,7 @@ import { Injectable } from '@nestjs/common'; import { AnyBoardDo, AnyContentElementDo, Card, ContentElementType, EntityId } from '@shared/domain'; import { LegacyLogger } from '@src/core/logger'; -import { AuthorizationService } from '@src/modules/authorization/authorization.service'; -import { Action } from '@src/modules/authorization/types/action.enum'; +import { AuthorizationService, Action } from '@src/modules/authorization'; import { BoardDoAuthorizableService, CardService, ContentElementService } from '../service'; @Injectable() diff --git a/apps/server/src/modules/board/uc/element.uc.ts b/apps/server/src/modules/board/uc/element.uc.ts index e5dc039168c..08357b01798 100644 --- a/apps/server/src/modules/board/uc/element.uc.ts +++ b/apps/server/src/modules/board/uc/element.uc.ts @@ -8,8 +8,7 @@ import { UserRoleEnum, } from '@shared/domain'; import { Logger } from '@src/core/logger'; -import { AuthorizationService } from '@src/modules/authorization'; -import { Action } from '@src/modules/authorization/types/action.enum'; +import { AuthorizationService, Action } from '@src/modules/authorization'; import { AnyElementContentBody } from '../controller/dto'; import { BoardDoAuthorizableService, ContentElementService } from '../service'; import { SubmissionItemService } from '../service/submission-item.service'; diff --git a/apps/server/src/modules/board/uc/submission-item.uc.spec.ts b/apps/server/src/modules/board/uc/submission-item.uc.spec.ts index 33bc8468fc9..5d06172acee 100644 --- a/apps/server/src/modules/board/uc/submission-item.uc.spec.ts +++ b/apps/server/src/modules/board/uc/submission-item.uc.spec.ts @@ -9,8 +9,7 @@ import { userFactory, } from '@shared/testing'; import { Logger } from '@src/core/logger'; -import { AuthorizationService } from '@src/modules/authorization'; -import { Action } from '@src/modules/authorization/types/action.enum'; +import { AuthorizationService, Action } from '@src/modules/authorization'; import { BoardDoAuthorizableService, ContentElementService, SubmissionItemService } from '../service'; import { SubmissionItemUc } from './submission-item.uc'; diff --git a/apps/server/src/modules/board/uc/submission-item.uc.ts b/apps/server/src/modules/board/uc/submission-item.uc.ts index e59afa4b49b..67e7951673f 100644 --- a/apps/server/src/modules/board/uc/submission-item.uc.ts +++ b/apps/server/src/modules/board/uc/submission-item.uc.ts @@ -9,8 +9,7 @@ import { UserRoleEnum, } from '@shared/domain'; import { Logger } from '@src/core/logger'; -import { AuthorizationService } from '@src/modules/authorization'; -import { Action } from '@src/modules/authorization/types/action.enum'; +import { AuthorizationService, Action } from '@src/modules/authorization'; import { BoardDoAuthorizableService, ContentElementService, SubmissionItemService } from '../service'; @Injectable() diff --git a/apps/server/src/modules/files-storage/README.md b/apps/server/src/modules/files-storage/README.md index 4bb6fb4ccb8..f44be536309 100644 --- a/apps/server/src/modules/files-storage/README.md +++ b/apps/server/src/modules/files-storage/README.md @@ -88,7 +88,7 @@ folder structure in S3 > schoolId/fileRecordId > .trash/schoolId/fileRecordId (see: ## Goals and Ideas > ### Deleting Files) -### Authorisation Module +### Authorization Module The authorisation is solved by parents. Therefore it is required that the parent types must be known to the authentication service. diff --git a/apps/server/src/modules/files-storage/files-storage-api.module.ts b/apps/server/src/modules/files-storage/files-storage-api.module.ts index 9d5283b47b7..aab383a158f 100644 --- a/apps/server/src/modules/files-storage/files-storage-api.module.ts +++ b/apps/server/src/modules/files-storage/files-storage-api.module.ts @@ -2,13 +2,13 @@ import { HttpModule } from '@nestjs/axios'; import { Module } from '@nestjs/common'; import { CoreModule } from '@src/core'; import { AuthenticationModule } from '@src/modules/authentication/authentication.module'; -import { AuthorizationModule } from '@src/modules/authorization'; +import { AuthorizationReferenceModule } from '@src/modules/authorization/authorization-reference.module'; import { FileSecurityController, FilesStorageController } from './controller'; import { FilesStorageModule } from './files-storage.module'; import { FilesStorageUC } from './uc'; @Module({ - imports: [AuthorizationModule, FilesStorageModule, AuthenticationModule, CoreModule, HttpModule], + imports: [AuthorizationReferenceModule, FilesStorageModule, AuthenticationModule, CoreModule, HttpModule], controllers: [FilesStorageController, FileSecurityController], providers: [FilesStorageUC], }) diff --git a/apps/server/src/modules/files-storage/mapper/files-storage.mapper.spec.ts b/apps/server/src/modules/files-storage/mapper/files-storage.mapper.spec.ts index 3165ec49021..a26103ae983 100644 --- a/apps/server/src/modules/files-storage/mapper/files-storage.mapper.spec.ts +++ b/apps/server/src/modules/files-storage/mapper/files-storage.mapper.spec.ts @@ -1,6 +1,6 @@ import { NotImplementedException } from '@nestjs/common'; import { fileRecordFactory, setupEntities } from '@shared/testing'; -import { AuthorizableReferenceType } from '@src/modules/authorization'; +import { AuthorizableReferenceType } from '@src/modules/authorization/domain'; import { DownloadFileParams, FileRecordListResponse, diff --git a/apps/server/src/modules/files-storage/mapper/files-storage.mapper.ts b/apps/server/src/modules/files-storage/mapper/files-storage.mapper.ts index 3d298cd3b2e..9b30acd4ada 100644 --- a/apps/server/src/modules/files-storage/mapper/files-storage.mapper.ts +++ b/apps/server/src/modules/files-storage/mapper/files-storage.mapper.ts @@ -1,5 +1,5 @@ import { NotImplementedException, StreamableFile } from '@nestjs/common'; -import { AuthorizableReferenceType } from '@src/modules/authorization'; +import { AuthorizableReferenceType } from '@src/modules/authorization/domain'; import { plainToClass } from 'class-transformer'; import { DownloadFileParams, diff --git a/apps/server/src/modules/files-storage/uc/files-storage-copy.uc.spec.ts b/apps/server/src/modules/files-storage/uc/files-storage-copy.uc.spec.ts index 5d4ab900549..612558e80c1 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage-copy.uc.spec.ts +++ b/apps/server/src/modules/files-storage/uc/files-storage-copy.uc.spec.ts @@ -8,7 +8,8 @@ import { AntivirusService } from '@shared/infra/antivirus'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { fileRecordFactory, setupEntities } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { Action, AuthorizationService } from '@src/modules/authorization'; +import { Action } from '@src/modules/authorization'; +import { AuthorizationReferenceService } from '@src/modules/authorization/domain'; import { FileRecordParams } from '../controller/dto'; import { FileRecord, FileRecordParentType } from '../entity'; import { CopyFileResponseBuilder } from '../mapper'; @@ -68,7 +69,7 @@ describe('FilesStorageUC', () => { let module: TestingModule; let filesStorageUC: FilesStorageUC; let filesStorageService: DeepMocked; - let authorizationService: DeepMocked; + let authorizationReferenceService: DeepMocked; beforeEach(() => { jest.resetAllMocks(); @@ -97,8 +98,8 @@ describe('FilesStorageUC', () => { useValue: createMock(), }, { - provide: AuthorizationService, - useValue: createMock(), + provide: AuthorizationReferenceService, + useValue: createMock(), }, { provide: HttpService, @@ -112,7 +113,7 @@ describe('FilesStorageUC', () => { }).compile(); filesStorageUC = module.get(FilesStorageUC); - authorizationService = module.get(AuthorizationService); + authorizationReferenceService = module.get(AuthorizationReferenceService); filesStorageService = module.get(FilesStorageService); }); @@ -134,7 +135,7 @@ describe('FilesStorageUC', () => { const fileResponse = CopyFileResponseBuilder.build(targetFile.id, sourceFile.id, targetFile.name); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce().mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce().mockResolvedValueOnce(); filesStorageService.copyFilesOfParent.mockResolvedValueOnce([[fileResponse], 1]); return { sourceParams, targetParams, userId, fileResponse }; @@ -145,7 +146,7 @@ describe('FilesStorageUC', () => { await filesStorageUC.copyFilesOfParent(userId, sourceParams, targetParams); - expect(authorizationService.checkPermissionByReferences).toHaveBeenNthCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenNthCalledWith( 1, userId, sourceParams.parentType, @@ -159,7 +160,7 @@ describe('FilesStorageUC', () => { await filesStorageUC.copyFilesOfParent(userId, sourceParams, targetParams); - expect(authorizationService.checkPermissionByReferences).toHaveBeenNthCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenNthCalledWith( 2, userId, targetParams.target.parentType, @@ -191,7 +192,7 @@ describe('FilesStorageUC', () => { const targetParams = createTargetParams(); const error = new ForbiddenException(); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(error).mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockRejectedValueOnce(error).mockResolvedValueOnce(); return { sourceParams, targetParams, userId, error }; }; @@ -210,7 +211,7 @@ describe('FilesStorageUC', () => { const targetParams = createTargetParams(); const error = new ForbiddenException(); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce().mockRejectedValueOnce(error); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce().mockRejectedValueOnce(error); return { sourceParams, targetParams, userId, error }; }; @@ -229,7 +230,9 @@ describe('FilesStorageUC', () => { const targetParams = createTargetParams(); const error = new ForbiddenException(); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(error).mockRejectedValueOnce(error); + authorizationReferenceService.checkPermissionByReferences + .mockRejectedValueOnce(error) + .mockRejectedValueOnce(error); return { sourceParams, targetParams, userId, error }; }; @@ -249,7 +252,7 @@ describe('FilesStorageUC', () => { const error = new Error('test'); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce().mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce().mockResolvedValueOnce(); filesStorageService.copyFilesOfParent.mockRejectedValueOnce(error); return { sourceParams, targetParams, userId, error }; @@ -289,7 +292,7 @@ describe('FilesStorageUC', () => { ); filesStorageService.getFileRecord.mockResolvedValue(fileRecord); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce().mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce().mockResolvedValueOnce(); filesStorageService.copy.mockResolvedValueOnce([fileResponse]); return { singleFileParams, copyFileParams, userId, fileResponse, fileRecord }; @@ -308,7 +311,7 @@ describe('FilesStorageUC', () => { await filesStorageUC.copyOneFile(userId, singleFileParams, copyFileParams); - expect(authorizationService.checkPermissionByReferences).toHaveBeenNthCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenNthCalledWith( 1, userId, fileRecord.parentType, @@ -322,7 +325,7 @@ describe('FilesStorageUC', () => { await filesStorageUC.copyOneFile(userId, singleFileParams, copyFileParams); - expect(authorizationService.checkPermissionByReferences).toHaveBeenNthCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenNthCalledWith( 2, userId, copyFileParams.target.parentType, @@ -355,7 +358,7 @@ describe('FilesStorageUC', () => { const error = new ForbiddenException(); filesStorageService.getFileRecord.mockResolvedValueOnce(fileRecord); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(error).mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockRejectedValueOnce(error).mockResolvedValueOnce(); return { singleFileParams, copyFileParams, userId, fileRecord, error }; }; @@ -375,7 +378,7 @@ describe('FilesStorageUC', () => { const error = new ForbiddenException(); filesStorageService.getFileRecord.mockResolvedValue(fileRecord); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce().mockRejectedValueOnce(error); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce().mockRejectedValueOnce(error); return { singleFileParams, copyFileParams, userId, fileRecord, error }; }; @@ -395,7 +398,9 @@ describe('FilesStorageUC', () => { const error = new ForbiddenException(); filesStorageService.getFileRecord.mockResolvedValue(fileRecord); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(error).mockRejectedValueOnce(error); + authorizationReferenceService.checkPermissionByReferences + .mockRejectedValueOnce(error) + .mockRejectedValueOnce(error); return { singleFileParams, copyFileParams, userId, fileRecord, error }; }; @@ -434,7 +439,7 @@ describe('FilesStorageUC', () => { const error = new Error('test'); filesStorageService.getFileRecord.mockResolvedValue(fileRecord); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce().mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce().mockResolvedValueOnce(); filesStorageService.copy.mockRejectedValueOnce(error); return { singleFileParams, copyFileParams, userId, fileRecord, error }; diff --git a/apps/server/src/modules/files-storage/uc/files-storage-delete.uc.spec.ts b/apps/server/src/modules/files-storage/uc/files-storage-delete.uc.spec.ts index eb13f830be6..fc461a50106 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage-delete.uc.spec.ts +++ b/apps/server/src/modules/files-storage/uc/files-storage-delete.uc.spec.ts @@ -8,7 +8,7 @@ import { AntivirusService } from '@shared/infra/antivirus'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { fileRecordFactory, setupEntities } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationReferenceService } from '@src/modules/authorization/domain'; import { FileRecordParams } from '../controller/dto'; import { FileRecord, FileRecordParentType } from '../entity'; import { FileStorageAuthorizationContext } from '../files-storage.const'; @@ -57,7 +57,7 @@ describe('FilesStorageUC delete methods', () => { let filesStorageUC: FilesStorageUC; let filesStorageService: DeepMocked; let previewService: DeepMocked; - let authorizationService: DeepMocked; + let authorizationReferenceService: DeepMocked; beforeAll(async () => { await setupEntities([FileRecord]); @@ -82,8 +82,8 @@ describe('FilesStorageUC delete methods', () => { useValue: createMock(), }, { - provide: AuthorizationService, - useValue: createMock(), + provide: AuthorizationReferenceService, + useValue: createMock(), }, { provide: HttpService, @@ -97,7 +97,7 @@ describe('FilesStorageUC delete methods', () => { }).compile(); filesStorageUC = module.get(FilesStorageUC); - authorizationService = module.get(AuthorizationService); + authorizationReferenceService = module.get(AuthorizationReferenceService); filesStorageService = module.get(FilesStorageService); previewService = module.get(PreviewService); }); @@ -122,7 +122,7 @@ describe('FilesStorageUC delete methods', () => { const fileRecord = fileRecords[0]; const mockedResult = [[fileRecord], 0] as Counted; - authorizationService.checkPermissionByReferences.mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); filesStorageService.deleteFilesOfParent.mockResolvedValueOnce(mockedResult); return { params, userId, mockedResult, requestParams, fileRecord }; @@ -134,7 +134,7 @@ describe('FilesStorageUC delete methods', () => { await filesStorageUC.deleteFilesOfParent(userId, requestParams); - expect(authorizationService.checkPermissionByReferences).toBeCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toBeCalledWith( userId, allowedType, requestParams.parentId, @@ -171,7 +171,7 @@ describe('FilesStorageUC delete methods', () => { const setup = () => { const { requestParams, userId } = createParams(); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(new ForbiddenException()); + authorizationReferenceService.checkPermissionByReferences.mockRejectedValueOnce(new ForbiddenException()); return { requestParams, userId }; }; @@ -192,7 +192,7 @@ describe('FilesStorageUC delete methods', () => { const { requestParams, userId } = createParams(); const error = new Error('test'); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); filesStorageService.deleteFilesOfParent.mockRejectedValueOnce(error); return { requestParams, userId, error }; @@ -214,7 +214,7 @@ describe('FilesStorageUC delete methods', () => { const requestParams = { fileRecordId: fileRecord.id, parentType: fileRecord.parentType }; filesStorageService.getFileRecord.mockResolvedValueOnce(fileRecord); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); filesStorageService.delete.mockResolvedValueOnce(); return { requestParams, userId, fileRecord }; @@ -227,7 +227,7 @@ describe('FilesStorageUC delete methods', () => { const allowedType = FilesStorageMapper.mapToAllowedAuthorizationEntityType(requestParams.parentType); - expect(authorizationService.checkPermissionByReferences).toBeCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toBeCalledWith( userId, allowedType, fileRecord.parentId, @@ -301,7 +301,7 @@ describe('FilesStorageUC delete methods', () => { const requestParams = { fileRecordId: fileRecord.id, parentType: fileRecord.parentType }; filesStorageService.getFileRecord.mockResolvedValueOnce(fileRecord); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(new ForbiddenException()); + authorizationReferenceService.checkPermissionByReferences.mockRejectedValueOnce(new ForbiddenException()); return { requestParams, userId }; }; @@ -322,7 +322,7 @@ describe('FilesStorageUC delete methods', () => { const error = new Error('test'); filesStorageService.getFileRecord.mockResolvedValueOnce(fileRecord); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); filesStorageService.delete.mockRejectedValueOnce(error); return { requestParams, userId, error }; diff --git a/apps/server/src/modules/files-storage/uc/files-storage-download-preview.uc.spec.ts b/apps/server/src/modules/files-storage/uc/files-storage-download-preview.uc.spec.ts index 3a2f6f1ac21..795939e5cb2 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage-download-preview.uc.spec.ts +++ b/apps/server/src/modules/files-storage/uc/files-storage-download-preview.uc.spec.ts @@ -7,7 +7,7 @@ import { AntivirusService } from '@shared/infra/antivirus'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { fileRecordFactory, setupEntities } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationReferenceService } from '@src/modules/authorization/domain'; import { SingleFileParams } from '../controller/dto'; import { FileRecord } from '../entity'; import { FileStorageAuthorizationContext } from '../files-storage.const'; @@ -43,7 +43,7 @@ describe('FilesStorageUC', () => { let filesStorageUC: FilesStorageUC; let filesStorageService: DeepMocked; let previewService: DeepMocked; - let authorizationService: DeepMocked; + let authorizationReferenceService: DeepMocked; beforeAll(async () => { await setupEntities([FileRecord]); @@ -72,8 +72,8 @@ describe('FilesStorageUC', () => { useValue: createMock(), }, { - provide: AuthorizationService, - useValue: createMock(), + provide: AuthorizationReferenceService, + useValue: createMock(), }, { provide: HttpService, @@ -83,7 +83,7 @@ describe('FilesStorageUC', () => { }).compile(); filesStorageUC = module.get(FilesStorageUC); - authorizationService = module.get(AuthorizationService); + authorizationReferenceService = module.get(AuthorizationReferenceService); filesStorageService = module.get(FilesStorageService); previewService = module.get(PreviewService); }); @@ -143,7 +143,7 @@ describe('FilesStorageUC', () => { await filesStorageUC.downloadPreview(userId, fileDownloadParams, previewParams); const allowedType = FilesStorageMapper.mapToAllowedAuthorizationEntityType(fileRecord.parentType); - expect(authorizationService.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( userId, allowedType, fileRecord.parentId, @@ -190,7 +190,7 @@ describe('FilesStorageUC', () => { filesStorageService.getFileRecord.mockResolvedValueOnce(fileRecord); const error = new ForbiddenException(); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(error); + authorizationReferenceService.checkPermissionByReferences.mockRejectedValueOnce(error); return { fileDownloadParams, userId, fileRecord, previewParams, error }; }; diff --git a/apps/server/src/modules/files-storage/uc/files-storage-download.uc.spec.ts b/apps/server/src/modules/files-storage/uc/files-storage-download.uc.spec.ts index b1aa6d4b437..3e7fa61fd7f 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage-download.uc.spec.ts +++ b/apps/server/src/modules/files-storage/uc/files-storage-download.uc.spec.ts @@ -7,7 +7,7 @@ import { AntivirusService } from '@shared/infra/antivirus'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { fileRecordFactory, setupEntities } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationReferenceService } from '@src/modules/authorization/domain'; import { SingleFileParams } from '../controller/dto'; import { FileRecord } from '../entity'; import { FileStorageAuthorizationContext } from '../files-storage.const'; @@ -34,7 +34,7 @@ describe('FilesStorageUC', () => { let module: TestingModule; let filesStorageUC: FilesStorageUC; let filesStorageService: DeepMocked; - let authorizationService: DeepMocked; + let authorizationReferenceService: DeepMocked; beforeAll(async () => { await setupEntities([FileRecord]); @@ -59,8 +59,8 @@ describe('FilesStorageUC', () => { useValue: createMock(), }, { - provide: AuthorizationService, - useValue: createMock(), + provide: AuthorizationReferenceService, + useValue: createMock(), }, { provide: HttpService, @@ -74,7 +74,7 @@ describe('FilesStorageUC', () => { }).compile(); filesStorageUC = module.get(FilesStorageUC); - authorizationService = module.get(AuthorizationService); + authorizationReferenceService = module.get(AuthorizationReferenceService); filesStorageService = module.get(FilesStorageService); }); @@ -99,7 +99,7 @@ describe('FilesStorageUC', () => { const fileResponse = createMock(); filesStorageService.getFileRecord.mockResolvedValueOnce(fileRecord); - authorizationService.checkPermissionByReferences.mockResolvedValue(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValue(); filesStorageService.download.mockResolvedValueOnce(fileResponse); return { fileDownloadParams, userId, fileRecord, fileResponse }; @@ -121,7 +121,7 @@ describe('FilesStorageUC', () => { await filesStorageUC.download(userId, fileDownloadParams); const allowedType = FilesStorageMapper.mapToAllowedAuthorizationEntityType(fileRecord.parentType); - expect(authorizationService.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( userId, allowedType, fileRecord.parentId, @@ -171,7 +171,7 @@ describe('FilesStorageUC', () => { const error = new ForbiddenException(); filesStorageService.getFileRecord.mockResolvedValueOnce(fileRecord); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(error); + authorizationReferenceService.checkPermissionByReferences.mockRejectedValueOnce(error); return { fileDownloadParams, userId, fileRecord }; }; @@ -190,7 +190,7 @@ describe('FilesStorageUC', () => { const error = new Error('test'); filesStorageService.getFileRecord.mockResolvedValueOnce(fileRecord); - authorizationService.checkPermissionByReferences.mockResolvedValue(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValue(); filesStorageService.download.mockRejectedValueOnce(error); return { fileDownloadParams, userId, error }; diff --git a/apps/server/src/modules/files-storage/uc/files-storage-get.uc.spec.ts b/apps/server/src/modules/files-storage/uc/files-storage-get.uc.spec.ts index 02cdb82ded6..7f372a1fe80 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage-get.uc.spec.ts +++ b/apps/server/src/modules/files-storage/uc/files-storage-get.uc.spec.ts @@ -6,7 +6,7 @@ import { AntivirusService } from '@shared/infra/antivirus'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { fileRecordFactory, setupEntities } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationReferenceService } from '@src/modules/authorization/domain'; import { FileRecordParams } from '../controller/dto'; import { FileRecord, FileRecordParentType } from '../entity'; import { FileStorageAuthorizationContext } from '../files-storage.const'; @@ -37,7 +37,7 @@ describe('FilesStorageUC', () => { let module: TestingModule; let filesStorageUC: FilesStorageUC; let filesStorageService: DeepMocked; - let authorizationService: DeepMocked; + let authorizationReferenceService: DeepMocked; beforeAll(async () => { await setupEntities([FileRecord]); @@ -62,8 +62,8 @@ describe('FilesStorageUC', () => { useValue: createMock(), }, { - provide: AuthorizationService, - useValue: createMock(), + provide: AuthorizationReferenceService, + useValue: createMock(), }, { provide: HttpService, @@ -77,7 +77,7 @@ describe('FilesStorageUC', () => { }).compile(); filesStorageUC = module.get(FilesStorageUC); - authorizationService = module.get(AuthorizationService); + authorizationReferenceService = module.get(AuthorizationReferenceService); filesStorageService = module.get(FilesStorageService); }); @@ -100,7 +100,7 @@ describe('FilesStorageUC', () => { const { fileRecords, params } = buildFileRecordsWithParams(); filesStorageService.getFileRecordsOfParent.mockResolvedValueOnce([fileRecords, fileRecords.length]); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); return { userId, params, fileRecords }; }; @@ -110,7 +110,7 @@ describe('FilesStorageUC', () => { await filesStorageUC.getFileRecordsOfParent(userId, params); - expect(authorizationService.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( userId, params.parentType, params.parentId, @@ -141,7 +141,7 @@ describe('FilesStorageUC', () => { const { fileRecords, params } = buildFileRecordsWithParams(); filesStorageService.getFileRecordsOfParent.mockResolvedValueOnce([fileRecords, fileRecords.length]); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(new Error('Bla')); + authorizationReferenceService.checkPermissionByReferences.mockRejectedValueOnce(new Error('Bla')); return { userId, params, fileRecords }; }; @@ -160,7 +160,7 @@ describe('FilesStorageUC', () => { const fileRecords = []; filesStorageService.getFileRecordsOfParent.mockResolvedValueOnce([fileRecords, fileRecords.length]); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); return { userId, params, fileRecords }; }; diff --git a/apps/server/src/modules/files-storage/uc/files-storage-restore.uc.spec.ts b/apps/server/src/modules/files-storage/uc/files-storage-restore.uc.spec.ts index be8a6d32561..e01e3116b79 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage-restore.uc.spec.ts +++ b/apps/server/src/modules/files-storage/uc/files-storage-restore.uc.spec.ts @@ -7,7 +7,7 @@ import { AntivirusService } from '@shared/infra/antivirus'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { fileRecordFactory, setupEntities } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationReferenceService } from '@src/modules/authorization/domain'; import { FileRecordParams, SingleFileParams } from '../controller/dto'; import { FileRecord, FileRecordParentType } from '../entity'; import { FileStorageAuthorizationContext } from '../files-storage.const'; @@ -52,7 +52,7 @@ describe('FilesStorageUC', () => { let module: TestingModule; let filesStorageUC: FilesStorageUC; let filesStorageService: DeepMocked; - let authorizationService: DeepMocked; + let authorizationReferenceService: DeepMocked; beforeAll(async () => { await setupEntities([FileRecord]); @@ -77,8 +77,8 @@ describe('FilesStorageUC', () => { useValue: createMock(), }, { - provide: AuthorizationService, - useValue: createMock(), + provide: AuthorizationReferenceService, + useValue: createMock(), }, { provide: HttpService, @@ -92,7 +92,7 @@ describe('FilesStorageUC', () => { }).compile(); filesStorageUC = module.get(FilesStorageUC); - authorizationService = module.get(AuthorizationService); + authorizationReferenceService = module.get(AuthorizationReferenceService); filesStorageService = module.get(FilesStorageService); }); @@ -113,7 +113,7 @@ describe('FilesStorageUC', () => { const setup = () => { const { params, userId, fileRecords } = buildFileRecordsWithParams(); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); filesStorageService.restoreFilesOfParent.mockResolvedValueOnce([fileRecords, fileRecords.length]); return { params, userId, fileRecords }; @@ -125,7 +125,7 @@ describe('FilesStorageUC', () => { await filesStorageUC.restoreFilesOfParent(userId, params); - expect(authorizationService.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( userId, allowedType, params.parentId, @@ -153,7 +153,7 @@ describe('FilesStorageUC', () => { describe('WHEN user is not authorised ', () => { const setup = () => { const { params, userId } = buildFileRecordsWithParams(); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(new ForbiddenException()); + authorizationReferenceService.checkPermissionByReferences.mockRejectedValueOnce(new ForbiddenException()); return { params, userId }; }; @@ -189,7 +189,7 @@ describe('FilesStorageUC', () => { const { params, userId, fileRecord } = buildFileRecordWithParams(); filesStorageService.getFileRecordMarkedForDelete.mockResolvedValueOnce(fileRecord); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); filesStorageService.restore.mockResolvedValueOnce(); return { params, userId, fileRecord }; @@ -209,7 +209,7 @@ describe('FilesStorageUC', () => { await filesStorageUC.restoreOneFile(userId, params); - expect(authorizationService.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( userId, allowedType, fileRecord.parentId, @@ -239,7 +239,7 @@ describe('FilesStorageUC', () => { const { params, userId, fileRecord } = buildFileRecordWithParams(); filesStorageService.getFileRecordMarkedForDelete.mockResolvedValueOnce(fileRecord); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(new ForbiddenException()); + authorizationReferenceService.checkPermissionByReferences.mockRejectedValueOnce(new ForbiddenException()); return { params, userId }; }; @@ -276,7 +276,7 @@ describe('FilesStorageUC', () => { const error = new Error('test'); filesStorageService.getFileRecordMarkedForDelete.mockResolvedValueOnce(fileRecord); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); filesStorageService.restore.mockRejectedValueOnce(error); return { params, userId, error }; diff --git a/apps/server/src/modules/files-storage/uc/files-storage-update.uc.spec.ts b/apps/server/src/modules/files-storage/uc/files-storage-update.uc.spec.ts index 57ec96cff61..19d9984eea8 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage-update.uc.spec.ts +++ b/apps/server/src/modules/files-storage/uc/files-storage-update.uc.spec.ts @@ -6,7 +6,7 @@ import { AntivirusService } from '@shared/infra/antivirus'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { fileRecordFactory, setupEntities } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationReferenceService } from '@src/modules/authorization/domain'; import { RenameFileParams, ScanResultParams, SingleFileParams } from '../controller/dto'; import { FileRecord } from '../entity'; import { FileStorageAuthorizationContext } from '../files-storage.const'; @@ -31,7 +31,7 @@ describe('FilesStorageUC', () => { let module: TestingModule; let filesStorageUC: FilesStorageUC; let filesStorageService: DeepMocked; - let authorizationService: DeepMocked; + let authorizationReferenceService: DeepMocked; beforeAll(async () => { await setupEntities([FileRecord]); @@ -56,8 +56,8 @@ describe('FilesStorageUC', () => { useValue: createMock(), }, { - provide: AuthorizationService, - useValue: createMock(), + provide: AuthorizationReferenceService, + useValue: createMock(), }, { provide: HttpService, @@ -71,7 +71,7 @@ describe('FilesStorageUC', () => { }).compile(); filesStorageUC = module.get(FilesStorageUC); - authorizationService = module.get(AuthorizationService); + authorizationReferenceService = module.get(AuthorizationReferenceService); filesStorageService = module.get(FilesStorageService); }); @@ -137,7 +137,7 @@ describe('FilesStorageUC', () => { const data: RenameFileParams = { fileName: 'test_new_name.txt' }; filesStorageService.getFileRecord.mockResolvedValueOnce(fileRecord); - authorizationService.checkPermissionByReferences.mockResolvedValueOnce(); + authorizationReferenceService.checkPermissionByReferences.mockResolvedValueOnce(); filesStorageService.patchFilename.mockResolvedValueOnce(fileRecord); return { userId, params, fileRecord, data }; @@ -155,7 +155,7 @@ describe('FilesStorageUC', () => { await filesStorageUC.patchFilename(userId, params, data); - expect(authorizationService.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( userId, fileRecord.parentType, fileRecord.parentId, diff --git a/apps/server/src/modules/files-storage/uc/files-storage-upload.uc.spec.ts b/apps/server/src/modules/files-storage/uc/files-storage-upload.uc.spec.ts index 903a2f2a6a6..ed7defb54fb 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage-upload.uc.spec.ts +++ b/apps/server/src/modules/files-storage/uc/files-storage-upload.uc.spec.ts @@ -8,7 +8,8 @@ import { AntivirusService } from '@shared/infra/antivirus'; import { S3ClientAdapter } from '@shared/infra/s3-client'; import { AxiosHeadersKeyValue, axiosResponseFactory, fileRecordFactory, setupEntities } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { Action, AuthorizationService } from '@src/modules/authorization'; +import { Action } from '@src/modules/authorization'; +import { AuthorizationReferenceService } from '@src/modules/authorization/domain'; import { AxiosRequestConfig, AxiosResponse } from 'axios'; import { Request } from 'express'; import { of } from 'rxjs'; @@ -72,7 +73,7 @@ describe('FilesStorageUC upload methods', () => { let module: TestingModule; let filesStorageUC: FilesStorageUC; let filesStorageService: DeepMocked; - let authorizationService: DeepMocked; + let authorizationReferenceService: DeepMocked; let httpService: DeepMocked; beforeAll(async () => { @@ -98,8 +99,8 @@ describe('FilesStorageUC upload methods', () => { useValue: createMock(), }, { - provide: AuthorizationService, - useValue: createMock(), + provide: AuthorizationReferenceService, + useValue: createMock(), }, { provide: HttpService, @@ -113,7 +114,7 @@ describe('FilesStorageUC upload methods', () => { }).compile(); filesStorageUC = module.get(FilesStorageUC); - authorizationService = module.get(AuthorizationService); + authorizationReferenceService = module.get(AuthorizationReferenceService); httpService = module.get(HttpService); filesStorageService = module.get(FilesStorageService); }); @@ -171,7 +172,7 @@ describe('FilesStorageUC upload methods', () => { await filesStorageUC.uploadFromUrl(userId, uploadFromUrlParams); - expect(authorizationService.checkPermissionByReferences).toBeCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toBeCalledWith( userId, uploadFromUrlParams.parentType, uploadFromUrlParams.parentId, @@ -218,7 +219,7 @@ describe('FilesStorageUC upload methods', () => { const setup = () => { const { userId, uploadFromUrlParams } = createUploadFromUrlParams(); const error = new Error('test'); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(error); + authorizationReferenceService.checkPermissionByReferences.mockRejectedValueOnce(error); return { uploadFromUrlParams, userId, error }; }; @@ -300,7 +301,7 @@ describe('FilesStorageUC upload methods', () => { await filesStorageUC.upload(userId, params, request); const allowedType = FilesStorageMapper.mapToAllowedAuthorizationEntityType(params.parentType); - expect(authorizationService.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( userId, allowedType, params.parentId, @@ -365,7 +366,7 @@ describe('FilesStorageUC upload methods', () => { const request = createRequest(); const error = new ForbiddenException(); - authorizationService.checkPermissionByReferences.mockRejectedValueOnce(error); + authorizationReferenceService.checkPermissionByReferences.mockRejectedValueOnce(error); return { params, userId, request, error }; }; diff --git a/apps/server/src/modules/files-storage/uc/files-storage.uc.ts b/apps/server/src/modules/files-storage/uc/files-storage.uc.ts index fa6a27202de..f5e6d372a6b 100644 --- a/apps/server/src/modules/files-storage/uc/files-storage.uc.ts +++ b/apps/server/src/modules/files-storage/uc/files-storage.uc.ts @@ -2,7 +2,8 @@ import { HttpService } from '@nestjs/axios'; import { Injectable, NotFoundException } from '@nestjs/common'; import { Counted, EntityId } from '@shared/domain'; import { LegacyLogger } from '@src/core/logger'; -import { AuthorizationContext, AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationContext } from '@src/modules/authorization'; +import { AuthorizationReferenceService } from '@src/modules/authorization/domain'; import { AxiosRequestConfig, AxiosResponse } from 'axios'; import busboy from 'busboy'; import { Request } from 'express'; @@ -32,7 +33,7 @@ import { PreviewService } from '../service/preview.service'; export class FilesStorageUC { constructor( private logger: LegacyLogger, - private readonly authorizationService: AuthorizationService, + private readonly authorizationReferenceService: AuthorizationReferenceService, private readonly httpService: HttpService, private readonly filesStorageService: FilesStorageService, private readonly previewService: PreviewService @@ -47,7 +48,7 @@ export class FilesStorageUC { context: AuthorizationContext ) { const allowedType = FilesStorageMapper.mapToAllowedAuthorizationEntityType(parentType); - await this.authorizationService.checkPermissionByReferences(userId, allowedType, parentId, context); + await this.authorizationReferenceService.checkPermissionByReferences(userId, allowedType, parentId, context); } // upload diff --git a/apps/server/src/modules/learnroom/controller/api-test/rooms-copy-timeout.api.spec.ts b/apps/server/src/modules/learnroom/controller/api-test/rooms-copy-timeout.api.spec.ts index ed62a2b6ade..3b9f04e07db 100644 --- a/apps/server/src/modules/learnroom/controller/api-test/rooms-copy-timeout.api.spec.ts +++ b/apps/server/src/modules/learnroom/controller/api-test/rooms-copy-timeout.api.spec.ts @@ -1,5 +1,4 @@ import { Configuration } from '@hpi-schul-cloud/commons/lib'; -import { IConfig } from '@hpi-schul-cloud/commons/lib/interfaces/IConfig'; import { EntityManager } from '@mikro-orm/mongodb'; import { ExecutionContext, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; @@ -16,11 +15,15 @@ import { import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; import { Request } from 'express'; import request from 'supertest'; +import { FilesStorageClientAdapterService } from '@src/modules/files-storage-client'; +import { createMock } from '@golevelup/ts-jest'; +// config must be set outside before the server module is importat, otherwise the configuration is already set +const configBefore = Configuration.toObject({ plainSecrets: true }); Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); Configuration.set('INCOMING_REQUEST_TIMEOUT_COPY_API', 1); // eslint-disable-next-line import/first -import { ServerTestModule } from '@src/modules/server/server.module'; +import { ServerTestModule } from '@src/modules/server'; // This needs to be in a separate test file because of the above configuration. // When we find a way to mock the config, it should be moved alongside the other API tests. @@ -28,10 +31,8 @@ describe('Rooms copy (API)', () => { let app: INestApplication; let em: EntityManager; let currentUser: ICurrentUser; - let configBefore: IConfig; beforeAll(async () => { - configBefore = Configuration.toObject({ plainSecrets: true }); const moduleFixture: TestingModule = await Test.createTestingModule({ imports: [ServerTestModule], }) @@ -43,6 +44,8 @@ describe('Rooms copy (API)', () => { return true; }, }) + .overrideProvider(FilesStorageClientAdapterService) + .useValue(createMock()) .compile(); app = moduleFixture.createNestApplication(); diff --git a/apps/server/src/modules/learnroom/index.ts b/apps/server/src/modules/learnroom/index.ts index f2dc136ce5e..e4d907784d5 100644 --- a/apps/server/src/modules/learnroom/index.ts +++ b/apps/server/src/modules/learnroom/index.ts @@ -1,2 +1,3 @@ export * from './learnroom.module'; export * from './service/course-copy.service'; +export { CourseService } from './service'; diff --git a/apps/server/src/modules/learnroom/learnroom-api.module.ts b/apps/server/src/modules/learnroom/learnroom-api.module.ts index b72db2d7f59..81a514a0a7b 100644 --- a/apps/server/src/modules/learnroom/learnroom-api.module.ts +++ b/apps/server/src/modules/learnroom/learnroom-api.module.ts @@ -1,6 +1,7 @@ import { Module } from '@nestjs/common'; import { BoardRepo, CourseRepo, DashboardModelMapper, DashboardRepo, LessonRepo, UserRepo } from '@shared/repo'; import { AuthorizationModule } from '@src/modules/authorization'; +import { AuthorizationReferenceModule } from '@src/modules/authorization/authorization-reference.module'; import { CopyHelperModule } from '@src/modules/copy-helper'; import { LessonModule } from '@src/modules/lesson'; import { CourseController } from './controller/course.controller'; @@ -20,7 +21,7 @@ import { } from './uc'; @Module({ - imports: [AuthorizationModule, LessonModule, CopyHelperModule, LearnroomModule], + imports: [AuthorizationModule, LessonModule, CopyHelperModule, LearnroomModule, AuthorizationReferenceModule], controllers: [DashboardController, CourseController, RoomsController], providers: [ DashboardUc, diff --git a/apps/server/src/modules/learnroom/uc/course-copy.uc.spec.ts b/apps/server/src/modules/learnroom/uc/course-copy.uc.spec.ts index 2e7e9f739ad..33beee8c4db 100644 --- a/apps/server/src/modules/learnroom/uc/course-copy.uc.spec.ts +++ b/apps/server/src/modules/learnroom/uc/course-copy.uc.spec.ts @@ -3,9 +3,9 @@ import { Configuration } from '@hpi-schul-cloud/commons'; import { ForbiddenException, InternalServerErrorException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { Permission } from '@shared/domain'; -import { boardFactory, courseFactory, setupEntities, userFactory } from '@shared/testing'; -import { Action, AuthorizableReferenceType } from '@src/modules/authorization'; -import { AuthorizationService } from '@src/modules/authorization/authorization.service'; +import { courseFactory, setupEntities, userFactory } from '@shared/testing'; +import { AuthorizationContextBuilder } from '@src/modules/authorization'; +import { AuthorizationReferenceService, AuthorizableReferenceType } from '@src/modules/authorization/domain'; import { CopyElementType, CopyStatusEnum } from '@src/modules/copy-helper'; import { CourseCopyService } from '../service'; import { CourseCopyUC } from './course-copy.uc'; @@ -13,7 +13,7 @@ import { CourseCopyUC } from './course-copy.uc'; describe('course copy uc', () => { let module: TestingModule; let uc: CourseCopyUC; - let authorization: DeepMocked; + let authorization: DeepMocked; let courseCopyService: DeepMocked; beforeAll(async () => { @@ -22,8 +22,8 @@ describe('course copy uc', () => { providers: [ CourseCopyUC, { - provide: AuthorizationService, - useValue: createMock(), + provide: AuthorizationReferenceService, + useValue: createMock(), }, { provide: CourseCopyService, @@ -33,7 +33,7 @@ describe('course copy uc', () => { }).compile(); uc = module.get(CourseCopyUC); - authorization = module.get(AuthorizationService); + authorization = module.get(AuthorizationReferenceService); courseCopyService = module.get(CourseCopyService); }); @@ -41,91 +41,99 @@ describe('course copy uc', () => { await module.close(); }); - beforeEach(() => { - Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); - }); + // Please be careful the Configuration.set is effects all tests !!! describe('copy course', () => { - const setup = () => { - const user = userFactory.buildWithId(); - const allCourses = courseFactory.buildList(3, { teachers: [user] }); - const course = allCourses[0]; - const originalBoard = boardFactory.build({ course }); - const courseCopy = courseFactory.buildWithId({ teachers: [user] }); - const boardCopy = boardFactory.build({ course: courseCopy }); - - authorization.getUserWithPermissions.mockResolvedValue(user); - const status = { - title: 'courseCopy', - type: CopyElementType.COURSE, - status: CopyStatusEnum.SUCCESS, - copyEntity: courseCopy, + describe('when authorization to course resolve with void and feature is deactivated', () => { + const setup = () => { + Configuration.set('FEATURE_COPY_SERVICE_ENABLED', false); + const user = userFactory.buildWithId(); + const course = courseFactory.buildWithId({ teachers: [user] }); + + return { + userId: user.id, + courseId: course.id, + }; }; - courseCopyService.copyCourse.mockResolvedValue(status); + it('should throw if copy feature is deactivated', async () => { + const { courseId, userId } = setup(); + + await expect(uc.copyCourse(userId, courseId)).rejects.toThrowError( + new InternalServerErrorException('Copy Feature not enabled') + ); + }); + }); - return { - user, - course, - originalBoard, - courseCopy, - boardCopy, - allCourses, - status, + describe('when authorization to course resolve with void and feature is activated', () => { + const setup = () => { + Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); + const user = userFactory.buildWithId(); + const course = courseFactory.buildWithId({ teachers: [user] }); + const courseCopy = courseFactory.buildWithId({ teachers: [user] }); + + const status = { + title: 'courseCopy', + type: CopyElementType.COURSE, + status: CopyStatusEnum.SUCCESS, + copyEntity: courseCopy, + }; + + authorization.checkPermissionByReferences.mockResolvedValueOnce(); + courseCopyService.copyCourse.mockResolvedValueOnce(status); + + return { + userId: user.id, + courseId: course.id, + status, + }; }; - }; - it('should throw if copy feature is deactivated', async () => { - Configuration.set('FEATURE_COPY_SERVICE_ENABLED', false); - const { course, user } = setup(); - await expect(uc.copyCourse(user.id, course.id)).rejects.toThrowError(InternalServerErrorException); - }); + it('should check permission to create a course', async () => { + const { courseId, userId } = setup(); - it('should check permission to create a course', async () => { - const { course, user } = setup(); - await uc.copyCourse(user.id, course.id); - expect(authorization.checkPermissionByReferences).toBeCalledWith( - user.id, - AuthorizableReferenceType.Course, - course.id, - { - action: Action.write, - requiredPermissions: [Permission.COURSE_CREATE], - } - ); - }); + await uc.copyCourse(userId, courseId); - it('should call course copy service', async () => { - const { course, user } = setup(); - await uc.copyCourse(user.id, course.id); - expect(courseCopyService.copyCourse).toBeCalledWith({ userId: user.id, courseId: course.id }); - }); + const context = AuthorizationContextBuilder.write([Permission.COURSE_CREATE]); + expect(authorization.checkPermissionByReferences).toBeCalledWith( + userId, + AuthorizableReferenceType.Course, + courseId, + context + ); + }); + + it('should call course copy service', async () => { + const { courseId, userId } = setup(); + + await uc.copyCourse(userId, courseId); + + expect(courseCopyService.copyCourse).toBeCalledWith({ userId, courseId }); + }); + + it('should return status', async () => { + const { courseId, userId, status } = setup(); + + const result = await uc.copyCourse(userId, courseId); - it('should return status', async () => { - const { course, user, status } = setup(); - const result = await uc.copyCourse(user.id, course.id); - expect(result).toEqual(status); + expect(result).toEqual(status); + }); }); - describe('when access to course is forbidden', () => { + describe('when authorization to course throw a forbidden exception', () => { const setupWithCourseForbidden = () => { + Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); const user = userFactory.buildWithId(); const course = courseFactory.buildWithId(); - authorization.checkPermissionByReferences.mockImplementation(() => { - throw new ForbiddenException(); - }); + authorization.checkPermissionByReferences.mockRejectedValueOnce(new ForbiddenException()); + return { user, course }; }; it('should throw ForbiddenException', async () => { const { course, user } = setupWithCourseForbidden(); - try { - await uc.copyCourse(user.id, course.id); - throw new Error('should have failed'); - } catch (err) { - expect(err).toBeInstanceOf(ForbiddenException); - } + await expect(uc.copyCourse(user.id, course.id)).rejects.toThrowError(new ForbiddenException()); }); }); }); diff --git a/apps/server/src/modules/learnroom/uc/course-copy.uc.ts b/apps/server/src/modules/learnroom/uc/course-copy.uc.ts index 0d806c36263..0f700d57f17 100644 --- a/apps/server/src/modules/learnroom/uc/course-copy.uc.ts +++ b/apps/server/src/modules/learnroom/uc/course-copy.uc.ts @@ -1,24 +1,23 @@ import { Configuration } from '@hpi-schul-cloud/commons'; import { Injectable, InternalServerErrorException } from '@nestjs/common'; import { EntityId, Permission } from '@shared/domain'; -import { Action, AuthorizableReferenceType, AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationContextBuilder } from '@src/modules/authorization'; +import { AuthorizationReferenceService, AuthorizableReferenceType } from '@src/modules/authorization/domain'; import { CopyStatus } from '@src/modules/copy-helper'; import { CourseCopyService } from '../service'; @Injectable() export class CourseCopyUC { constructor( - private readonly authorization: AuthorizationService, + private readonly authorization: AuthorizationReferenceService, private readonly courseCopyService: CourseCopyService ) {} async copyCourse(userId: EntityId, courseId: EntityId): Promise { this.checkFeatureEnabled(); - await this.authorization.checkPermissionByReferences(userId, AuthorizableReferenceType.Course, courseId, { - action: Action.write, - requiredPermissions: [Permission.COURSE_CREATE], - }); + const context = AuthorizationContextBuilder.write([Permission.COURSE_CREATE]); + await this.authorization.checkPermissionByReferences(userId, AuthorizableReferenceType.Course, courseId, context); const result = await this.courseCopyService.copyCourse({ userId, courseId }); @@ -26,6 +25,7 @@ export class CourseCopyUC { } private checkFeatureEnabled() { + // @hpi-schul-cloud/commons is deprecated way to get envirements const enabled = Configuration.get('FEATURE_COPY_SERVICE_ENABLED') as boolean; if (!enabled) { throw new InternalServerErrorException('Copy Feature not enabled'); diff --git a/apps/server/src/modules/learnroom/uc/course-export.uc.spec.ts b/apps/server/src/modules/learnroom/uc/course-export.uc.spec.ts index 3d93827f06d..04e3d0de480 100644 --- a/apps/server/src/modules/learnroom/uc/course-export.uc.spec.ts +++ b/apps/server/src/modules/learnroom/uc/course-export.uc.spec.ts @@ -1,7 +1,9 @@ import { Test, TestingModule } from '@nestjs/testing'; import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { CommonCartridgeExportService } from '@src/modules/learnroom/service/common-cartridge-export.service'; -import { AuthorizationService } from '@src/modules'; +import { AuthorizationReferenceService } from '@src/modules/authorization/domain'; +import { ObjectId } from 'bson'; +import { ForbiddenException } from '@nestjs/common'; import { CourseExportUc } from './course-export.uc'; import { CommonCartridgeVersion } from '../common-cartridge'; @@ -9,7 +11,7 @@ describe('CourseExportUc', () => { let module: TestingModule; let courseExportUc: CourseExportUc; let courseExportServiceMock: DeepMocked; - let authorizationServiceMock: DeepMocked; + let authorizationServiceMock: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -20,33 +22,86 @@ describe('CourseExportUc', () => { useValue: createMock(), }, { - provide: AuthorizationService, - useValue: createMock(), + provide: AuthorizationReferenceService, + useValue: createMock(), }, ], }).compile(); courseExportUc = module.get(CourseExportUc); courseExportServiceMock = module.get(CommonCartridgeExportService); - authorizationServiceMock = module.get(AuthorizationService); + authorizationServiceMock = module.get(AuthorizationReferenceService); }); afterAll(async () => { await module.close(); }); + afterEach(() => { + // is needed to solve buffer test isolation + jest.resetAllMocks(); + }); + describe('exportCourse', () => { - const version: CommonCartridgeVersion = CommonCartridgeVersion.V_1_1_0; - it('should check for permissions', async () => { - authorizationServiceMock.checkPermissionByReferences.mockResolvedValueOnce(); - await expect(courseExportUc.exportCourse('', '', version)).resolves.not.toThrow(); - expect(authorizationServiceMock.checkPermissionByReferences).toBeCalledTimes(1); + const setupParams = () => { + const courseId = new ObjectId().toHexString(); + const userId = new ObjectId().toHexString(); + const version: CommonCartridgeVersion = CommonCartridgeVersion.V_1_1_0; + + return { version, userId, courseId }; + }; + + describe('when authorization throw a error', () => { + const setup = () => { + authorizationServiceMock.checkPermissionByReferences.mockRejectedValueOnce(new ForbiddenException()); + courseExportServiceMock.exportCourse.mockResolvedValueOnce(Buffer.from('')); + + return setupParams(); + }; + + it('should pass this error', async () => { + const { courseId, userId, version } = setup(); + + await expect(courseExportUc.exportCourse(courseId, userId, version)).rejects.toThrowError( + new ForbiddenException() + ); + }); + }); + + describe('when course export service throw a error', () => { + const setup = () => { + authorizationServiceMock.checkPermissionByReferences.mockResolvedValueOnce(); + courseExportServiceMock.exportCourse.mockRejectedValueOnce(new Error()); + + return setupParams(); + }; + + it('should pass this error', async () => { + const { courseId, userId, version } = setup(); + + await expect(courseExportUc.exportCourse(courseId, userId, version)).rejects.toThrowError(new Error()); + }); }); - it('should return a binary file as buffer', async () => { - courseExportServiceMock.exportCourse.mockResolvedValueOnce(Buffer.from('')); - authorizationServiceMock.checkPermissionByReferences.mockResolvedValueOnce(); + describe('when authorization resolve', () => { + const setup = () => { + authorizationServiceMock.checkPermissionByReferences.mockResolvedValueOnce(); + courseExportServiceMock.exportCourse.mockResolvedValueOnce(Buffer.from('')); + + return setupParams(); + }; + + it('should check for permissions', async () => { + const { courseId, userId, version } = setup(); + + await expect(courseExportUc.exportCourse(courseId, userId, version)).resolves.not.toThrow(); + expect(authorizationServiceMock.checkPermissionByReferences).toBeCalledTimes(1); + }); + + it('should return a binary file as buffer', async () => { + const { courseId, userId, version } = setup(); - await expect(courseExportUc.exportCourse('', '', version)).resolves.toBeInstanceOf(Buffer); + await expect(courseExportUc.exportCourse(courseId, userId, version)).resolves.toBeInstanceOf(Buffer); + }); }); }); }); diff --git a/apps/server/src/modules/learnroom/uc/course-export.uc.ts b/apps/server/src/modules/learnroom/uc/course-export.uc.ts index 418812e0cd8..07e427c8fa8 100644 --- a/apps/server/src/modules/learnroom/uc/course-export.uc.ts +++ b/apps/server/src/modules/learnroom/uc/course-export.uc.ts @@ -1,6 +1,7 @@ import { Injectable } from '@nestjs/common'; import { EntityId, Permission } from '@shared/domain'; -import { Action, AuthorizableReferenceType, AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationContextBuilder } from '@src/modules/authorization'; +import { AuthorizationReferenceService, AuthorizableReferenceType } from '@src/modules/authorization/domain'; import { CommonCartridgeVersion } from '../common-cartridge'; import { CommonCartridgeExportService } from '../service/common-cartridge-export.service'; @@ -8,14 +9,18 @@ import { CommonCartridgeExportService } from '../service/common-cartridge-export export class CourseExportUc { constructor( private readonly courseExportService: CommonCartridgeExportService, - private readonly authorizationService: AuthorizationService + private readonly authorizationService: AuthorizationReferenceService ) {} async exportCourse(courseId: EntityId, userId: EntityId, version: CommonCartridgeVersion): Promise { - await this.authorizationService.checkPermissionByReferences(userId, AuthorizableReferenceType.Course, courseId, { - action: Action.read, - requiredPermissions: [Permission.COURSE_EDIT], - }); + const context = AuthorizationContextBuilder.read([Permission.COURSE_EDIT]); + await this.authorizationService.checkPermissionByReferences( + userId, + AuthorizableReferenceType.Course, + courseId, + context + ); + return this.courseExportService.exportCourse(courseId, userId, version); } } diff --git a/apps/server/src/modules/learnroom/uc/lesson-copy.uc.spec.ts b/apps/server/src/modules/learnroom/uc/lesson-copy.uc.spec.ts index a00e0be6c26..34d73449b4c 100644 --- a/apps/server/src/modules/learnroom/uc/lesson-copy.uc.spec.ts +++ b/apps/server/src/modules/learnroom/uc/lesson-copy.uc.spec.ts @@ -3,13 +3,12 @@ import { Configuration } from '@hpi-schul-cloud/commons'; import { ObjectId } from '@mikro-orm/mongodb'; import { ForbiddenException, InternalServerErrorException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; -import { BaseDO, Permission, User } from '@shared/domain'; +import { Permission } from '@shared/domain'; import { CourseRepo, LessonRepo, UserRepo } from '@shared/repo'; import { courseFactory, lessonFactory, setupEntities, userFactory } from '@shared/testing'; -import { Action, AuthorizableReferenceType, AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationContextBuilder, AuthorizationService } from '@src/modules/authorization'; import { CopyElementType, CopyHelperService, CopyStatusEnum } from '@src/modules/copy-helper'; import { EtherpadService, LessonCopyService } from '@src/modules/lesson/service'; -import { AuthorizableObject } from '@shared/domain/domain-object'; import { LessonCopyUC } from './lesson-copy.uc'; describe('lesson copy uc', () => { @@ -71,193 +70,286 @@ describe('lesson copy uc', () => { copyHelperService = module.get(CopyHelperService); }); - beforeEach(() => { - Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); + afterEach(() => { jest.resetAllMocks(); }); + // Please be careful the Configuration.set is effects all tests !!! + describe('copy lesson', () => { - const setup = () => { - const user = userFactory.buildWithId(); - const course = courseFactory.buildWithId({ teachers: [user] }); - const allLessons = lessonFactory.buildList(3, { course }); - const lesson = allLessons[0]; - - authorisation.getUserWithPermissions.mockResolvedValue(user); - lessonRepo.findById.mockResolvedValue(lesson); - lessonRepo.findAllByCourseIds.mockResolvedValue([allLessons, allLessons.length]); - lessonRepo.save.mockResolvedValue(undefined); - - courseRepo.findById.mockResolvedValue(course); - authorisation.hasPermission.mockReturnValue(true); - const copy = lessonFactory.buildWithId({ course }); - const status = { - title: 'lessonCopy', - type: CopyElementType.LESSON, - status: CopyStatusEnum.SUCCESS, - copyEntity: copy, - }; - lessonCopyService.copyLesson.mockResolvedValue(status); - lessonCopyService.updateCopiedEmbeddedTasks.mockReturnValue(status); - const lessonCopyName = 'Copy'; - copyHelperService.deriveCopyName.mockReturnValue(lessonCopyName); - - return { - user, - course, - lesson, - copy, - status, - lessonCopyName, - allLessons, - userId: user.id, + // missing tests + // when course repo is throw a error + // when lesson repo is throw a error + describe('when feature flag is disabled', () => { + const setup = () => { + Configuration.set('FEATURE_COPY_SERVICE_ENABLED', false); + + const user = userFactory.buildWithId(); + const course = courseFactory.buildWithId({ teachers: [user] }); + const lesson = lessonFactory.build({ course }); + + const parentParams = { courseId: course.id, userId: user.id }; + + return { + userId: user.id, + lessonId: lesson.id, + parentParams, + }; }; - }; - - it('should throw if copy feature is deactivated', async () => { - Configuration.set('FEATURE_COPY_SERVICE_ENABLED', false); - const { course, user, lesson, userId } = setup(); - await expect(uc.copyLesson(user.id, lesson.id, { courseId: course.id, userId })).rejects.toThrowError( - InternalServerErrorException - ); - }); - it('should fetch correct user', async () => { - const { course, user, lesson, userId } = setup(); - await uc.copyLesson(user.id, lesson.id, { courseId: course.id, userId }); - expect(authorisation.getUserWithPermissions).toBeCalledWith(user.id); - }); + it('should throw if copy feature is deactivated', async () => { + const { userId, lessonId, parentParams } = setup(); - it('should fetch correct lesson', async () => { - const { course, user, lesson, userId } = setup(); - await uc.copyLesson(user.id, lesson.id, { courseId: course.id, userId }); - expect(lessonRepo.findById).toBeCalledWith(lesson.id); + await expect(uc.copyLesson(userId, lessonId, parentParams)).rejects.toThrowError( + new InternalServerErrorException('Copy Feature not enabled') + ); + }); }); - it('should fetch destination course', async () => { - const { course, user, lesson, userId } = setup(); - await uc.copyLesson(user.id, lesson.id, { courseId: course.id, userId }); - expect(courseRepo.findById).toBeCalledWith(course.id); - }); + describe('when authorization resolve and no destination course is passed', () => { + const setup = () => { + Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); - it('should pass without destination course', async () => { - const { user, lesson, userId } = setup(); - await uc.copyLesson(user.id, lesson.id, { userId }); - expect(courseRepo.findById).not.toHaveBeenCalled(); - }); + const user = userFactory.buildWithId(); + const course = courseFactory.buildWithId({ teachers: [user] }); + const allLessons = lessonFactory.buildList(3, { course }); + const copy = lessonFactory.buildWithId({ course }); + + const lesson = allLessons[0]; + const status = { + title: 'lessonCopy', + type: CopyElementType.LESSON, + status: CopyStatusEnum.SUCCESS, + copyEntity: copy, + }; + const lessonCopyName = 'Copy'; + const parentParams = { userId: user.id }; + + authorisation.getUserWithPermissions.mockResolvedValueOnce(user); + authorisation.hasPermission.mockReturnValue(true); + + lessonRepo.findById.mockResolvedValueOnce(lesson); + lessonRepo.findAllByCourseIds.mockResolvedValueOnce([allLessons, allLessons.length]); + courseRepo.findById.mockResolvedValueOnce(course); + + lessonCopyService.copyLesson.mockResolvedValueOnce(status); + copyHelperService.deriveCopyName.mockReturnValueOnce(lessonCopyName); + + return { + user, + userId: user.id, + course, + courseId: course.id, + lessonId: lesson.id, + parentParams, + }; + }; + + it('should pass without destination course', async () => { + const { lessonId, userId, parentParams } = setup(); - it('should check authorisation for lesson', async () => { - const { course, user, lesson, userId } = setup(); - await uc.copyLesson(user.id, lesson.id, { courseId: course.id, userId }); - expect(authorisation.hasPermission).toBeCalledWith(user, lesson, { - action: Action.read, - requiredPermissions: [Permission.TOPIC_CREATE], + await uc.copyLesson(userId, lessonId, parentParams); + + expect(courseRepo.findById).not.toHaveBeenCalled(); }); - }); - it('should check authorisation for destination course', async () => { - const { course, user, lesson, userId } = setup(); - await uc.copyLesson(user.id, lesson.id, { courseId: course.id, userId }); - expect(authorisation.checkPermissionByReferences).toBeCalledWith( - user.id, - AuthorizableReferenceType.Course, - course.id, - { - action: Action.write, - requiredPermissions: [], - } - ); - }); + it('should pass authorisation check without destination course', async () => { + const { course, user, lessonId, userId, parentParams } = setup(); + + await uc.copyLesson(userId, lessonId, parentParams); - it('should pass authorisation check without destination course', async () => { - const { course, user, lesson, userId } = setup(); - await uc.copyLesson(user.id, lesson.id, { userId }); - expect(authorisation.hasPermission).not.toBeCalledWith(user, course, { - action: Action.write, - requiredPermissions: [], + const context = AuthorizationContextBuilder.write([]); + expect(authorisation.hasPermission).not.toBeCalledWith(user, course, context); }); }); - it('should call copy service', async () => { - const { course, user, lesson, lessonCopyName, userId } = setup(); - await uc.copyLesson(user.id, lesson.id, { courseId: course.id, userId }); - expect(lessonCopyService.copyLesson).toBeCalledWith({ - originalLessonId: lesson.id, - destinationCourse: course, - user, - copyName: lessonCopyName, + describe('when authorization resolve', () => { + const setup = () => { + Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); + + const user = userFactory.buildWithId(); + const course = courseFactory.buildWithId({ teachers: [user] }); + const allLessons = lessonFactory.buildList(3, { course }); + const copy = lessonFactory.buildWithId({ course }); + + const lesson = allLessons[0]; + const status = { + title: 'lessonCopy', + type: CopyElementType.LESSON, + status: CopyStatusEnum.SUCCESS, + copyEntity: copy, + }; + const lessonCopyName = 'Copy'; + const parentParams = { courseId: course.id, userId: user.id }; + + authorisation.getUserWithPermissions.mockResolvedValueOnce(user); + authorisation.hasPermission.mockReturnValue(true); + + lessonRepo.findById.mockResolvedValueOnce(lesson); + lessonRepo.findAllByCourseIds.mockResolvedValueOnce([allLessons, allLessons.length]); + courseRepo.findById.mockResolvedValueOnce(course); + + lessonCopyService.copyLesson.mockResolvedValueOnce(status); + // lessonCopyService.updateCopiedEmbeddedTasks.mockReturnValue(status); + copyHelperService.deriveCopyName.mockReturnValueOnce(lessonCopyName); + + return { + user, + userId: user.id, + course, + courseId: course.id, + lesson, + lessonId: lesson.id, + parentParams, + copy, + status, + lessonCopyName, + allLessons, + }; + }; + + it('should fetch correct user', async () => { + const { lessonId, userId, parentParams } = setup(); + + await uc.copyLesson(userId, lessonId, parentParams); + + expect(authorisation.getUserWithPermissions).toBeCalledWith(userId); }); - }); - it('should return status', async () => { - const { course, user, lesson, status, userId } = setup(); - const result = await uc.copyLesson(user.id, lesson.id, { courseId: course.id, userId }); - expect(result).toEqual(status); - }); + it('should fetch correct lesson', async () => { + const { lessonId, userId, parentParams } = setup(); - it('should use copyHelperService', async () => { - const { course, user, lesson, allLessons, userId } = setup(); - await uc.copyLesson(user.id, lesson.id, { courseId: course.id, userId }); - const existingNames = allLessons.map((l) => l.name); - expect(copyHelperService.deriveCopyName).toHaveBeenCalledWith(lesson.name, existingNames); - }); + await uc.copyLesson(userId, lessonId, parentParams); + + expect(lessonRepo.findById).toBeCalledWith(lessonId); + }); + + it('should fetch destination course', async () => { + const { course, lessonId, userId, parentParams } = setup(); + + await uc.copyLesson(userId, lessonId, parentParams); + + expect(courseRepo.findById).toBeCalledWith(course.id); + }); + + it('should check authorisation for lesson', async () => { + const { lessonId, userId, user, lesson, parentParams } = setup(); + + await uc.copyLesson(userId, lessonId, parentParams); + + const context = AuthorizationContextBuilder.read([Permission.TOPIC_CREATE]); + expect(authorisation.hasPermission).toBeCalledWith(user, lesson, context); + }); + + it('should check authorisation for destination course', async () => { + const { course, user, lessonId, userId, parentParams } = setup(); + + await uc.copyLesson(userId, lessonId, parentParams); + + const context = AuthorizationContextBuilder.write([]); + expect(authorisation.checkPermission).toBeCalledWith(user, course, context); + }); + + it('should call copy service', async () => { + const { course, user, lessonId, lessonCopyName, userId, parentParams } = setup(); + + await uc.copyLesson(userId, lessonId, parentParams); + + expect(lessonCopyService.copyLesson).toBeCalledWith({ + originalLessonId: lessonId, + destinationCourse: course, + user, + copyName: lessonCopyName, + }); + }); - it('should use findAllByCourseIds to determine existing lesson names', async () => { - const { course, user, lesson, userId } = setup(); - await uc.copyLesson(user.id, lesson.id, { courseId: course.id, userId }); - expect(lessonRepo.findAllByCourseIds).toHaveBeenCalledWith([course.id]); + it('should return status', async () => { + const { lessonId, status, userId, parentParams } = setup(); + + const result = await uc.copyLesson(userId, lessonId, parentParams); + + expect(result).toEqual(status); + }); + + it('should use copyHelperService', async () => { + const { lessonId, allLessons, userId, lesson, parentParams } = setup(); + + await uc.copyLesson(userId, lessonId, parentParams); + + const existingNames = allLessons.map((l) => l.name); + expect(copyHelperService.deriveCopyName).toHaveBeenCalledWith(lesson.name, existingNames); + }); + + it('should use findAllByCourseIds to determine existing lesson names', async () => { + const { courseId, userId, lessonId, parentParams } = setup(); + + await uc.copyLesson(userId, lessonId, parentParams); + + expect(lessonRepo.findAllByCourseIds).toHaveBeenCalledWith([courseId]); + }); }); - describe('when access to lesson is forbidden', () => { - const setupWithLessonForbidden = () => { + describe('when authorization of lesson throw forbidden exception', () => { + const setup = () => { + Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); + const user = userFactory.buildWithId(); const course = courseFactory.buildWithId(); const lesson = lessonFactory.buildWithId(); - userRepo.findById.mockResolvedValue(user); - lessonRepo.findById.mockResolvedValue(lesson); - // authorisation should not be mocked - authorisation.hasPermission.mockImplementation((u: User, e: AuthorizableObject | BaseDO) => e !== lesson); - return { user, course, lesson }; + const parentParams = { courseId: course.id, userId: new ObjectId().toHexString() }; + + userRepo.findById.mockResolvedValueOnce(user); + lessonRepo.findById.mockResolvedValueOnce(lesson); + courseRepo.findById.mockResolvedValueOnce(course); + authorisation.hasPermission.mockReturnValueOnce(false); + + return { + userId: user.id, + lessonId: lesson.id, + parentParams, + }; }; - it('should throw NotFoundException', async () => { - const { course, user, lesson } = setupWithLessonForbidden(); + it('should throw ForbiddenException', async () => { + const { parentParams, userId, lessonId } = setup(); - try { - await uc.copyLesson(user.id, lesson.id, { courseId: course.id, userId: new ObjectId().toHexString() }); - throw new Error('should have failed'); - } catch (err) { - expect(err).toBeInstanceOf(ForbiddenException); - } + await expect(uc.copyLesson(userId, lessonId, parentParams)).rejects.toThrowError( + new ForbiddenException('could not find lesson to copy') + ); }); }); - describe('when access to course is forbidden', () => { - const setupWithCourseForbidden = () => { + describe('when authorization of course throw with forbidden exception', () => { + const setup = () => { + Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); + const user = userFactory.buildWithId(); const course = courseFactory.buildWithId(); const lesson = lessonFactory.buildWithId(); - userRepo.findById.mockResolvedValue(user); - lessonRepo.findById.mockResolvedValue(lesson); - courseRepo.findById.mockResolvedValue(course); - authorisation.hasPermission.mockReturnValue(true); - authorisation.checkPermissionByReferences.mockImplementation(() => { + + const parentParams = { courseId: course.id, userId: new ObjectId().toHexString() }; + + userRepo.findById.mockResolvedValueOnce(user); + lessonRepo.findById.mockResolvedValueOnce(lesson); + courseRepo.findById.mockResolvedValueOnce(course); + authorisation.checkPermission.mockImplementationOnce(() => { throw new ForbiddenException(); }); - return { user, course, lesson }; + authorisation.hasPermission.mockReturnValueOnce(true); + + return { + userId: user.id, + lessonId: lesson.id, + parentParams, + }; }; - it('should throw Forbidden Exception', async () => { - const { course, user, lesson } = setupWithCourseForbidden(); + it('should pass the forbidden exception', async () => { + const { parentParams, userId, lessonId } = setup(); - try { - await uc.copyLesson(user.id, lesson.id, { courseId: course.id, userId: new ObjectId().toHexString() }); - throw new Error('should have failed'); - } catch (err) { - expect(err).toBeInstanceOf(ForbiddenException); - } + await expect(uc.copyLesson(userId, lessonId, parentParams)).rejects.toThrowError(new ForbiddenException()); }); }); }); diff --git a/apps/server/src/modules/learnroom/uc/lesson-copy.uc.ts b/apps/server/src/modules/learnroom/uc/lesson-copy.uc.ts index 753200a5718..7ec51f5ef1c 100644 --- a/apps/server/src/modules/learnroom/uc/lesson-copy.uc.ts +++ b/apps/server/src/modules/learnroom/uc/lesson-copy.uc.ts @@ -1,14 +1,9 @@ import { Configuration } from '@hpi-schul-cloud/commons'; import { ForbiddenException, Injectable, InternalServerErrorException } from '@nestjs/common'; -import { EntityId } from '@shared/domain'; +import { Course, EntityId, LessonEntity, User } from '@shared/domain'; import { Permission } from '@shared/domain/interface/permission.enum'; import { CourseRepo, LessonRepo } from '@shared/repo'; -import { - Action, - AuthorizableReferenceType, - AuthorizationContextBuilder, - AuthorizationService, -} from '@src/modules/authorization'; +import { AuthorizationContextBuilder, AuthorizationService } from '@src/modules/authorization'; import { CopyHelperService, CopyStatus } from '@src/modules/copy-helper'; import { LessonCopyParentParams } from '@src/modules/lesson'; import { LessonCopyService } from '@src/modules/lesson/service'; @@ -24,27 +19,24 @@ export class LessonCopyUC { ) {} async copyLesson(userId: EntityId, lessonId: EntityId, parentParams: LessonCopyParentParams): Promise { - this.featureEnabled(); - const user = await this.authorisation.getUserWithPermissions(userId); - const originalLesson = await this.lessonRepo.findById(lessonId); - const context = AuthorizationContextBuilder.read([Permission.TOPIC_CREATE]); - if (!this.authorisation.hasPermission(user, originalLesson, context)) { - throw new ForbiddenException('could not find lesson to copy'); - } + this.checkFeatureEnabled(); + + const [user, originalLesson]: [User, LessonEntity] = await Promise.all([ + this.authorisation.getUserWithPermissions(userId), + this.lessonRepo.findById(lessonId), + ]); + this.checkOriginalLessonAuthorization(user, originalLesson); + + // should be a seperate private method const destinationCourse = parentParams.courseId ? await this.courseRepo.findById(parentParams.courseId) : originalLesson.course; - await this.authorisation.checkPermissionByReferences( - userId, - AuthorizableReferenceType.Course, - destinationCourse.id, - { - action: Action.write, - requiredPermissions: [], - } - ); + // --- + + this.checkDestinationCourseAuthorization(user, destinationCourse); + // should be a seperate private method const [existingLessons] = await this.lessonRepo.findAllByCourseIds([originalLesson.course.id]); const existingNames = existingLessons.map((l) => l.name); const copyName = this.copyHelperService.deriveCopyName(originalLesson.name, existingNames); @@ -55,11 +47,25 @@ export class LessonCopyUC { user, copyName, }); + // --- return copyStatus; } - private featureEnabled() { + private checkOriginalLessonAuthorization(user: User, originalLesson: LessonEntity): void { + const contextReadWithTopicCreate = AuthorizationContextBuilder.read([Permission.TOPIC_CREATE]); + if (!this.authorisation.hasPermission(user, originalLesson, contextReadWithTopicCreate)) { + // error message is not correct, switch to authorisation.checkPermission() makse sense for me + throw new ForbiddenException('could not find lesson to copy'); + } + } + + private checkDestinationCourseAuthorization(user: User, destinationCourse: Course): void { + const contextCanWrite = AuthorizationContextBuilder.write([]); + this.authorisation.checkPermission(user, destinationCourse, contextCanWrite); + } + + private checkFeatureEnabled() { const enabled = Configuration.get('FEATURE_COPY_SERVICE_ENABLED') as boolean; if (!enabled) { throw new InternalServerErrorException('Copy Feature not enabled'); diff --git a/apps/server/src/modules/learnroom/uc/room-board-dto.factory.ts b/apps/server/src/modules/learnroom/uc/room-board-dto.factory.ts index 98a0957f3d3..cafa02e4d20 100644 --- a/apps/server/src/modules/learnroom/uc/room-board-dto.factory.ts +++ b/apps/server/src/modules/learnroom/uc/room-board-dto.factory.ts @@ -14,8 +14,7 @@ import { TaskWithStatusVo, User, } from '@shared/domain'; -import { AuthorizationService } from '@src/modules/authorization/authorization.service'; -import { Action } from '@src/modules/authorization/types/action.enum'; +import { AuthorizationService, Action } from '@src/modules/authorization'; import { ColumnBoardMetaData, LessonMetaData, diff --git a/apps/server/src/modules/legacy-school/uc/legacy-school.uc.spec.ts b/apps/server/src/modules/legacy-school/uc/legacy-school.uc.spec.ts index 17dc2de5fd0..8747a07ada6 100644 --- a/apps/server/src/modules/legacy-school/uc/legacy-school.uc.spec.ts +++ b/apps/server/src/modules/legacy-school/uc/legacy-school.uc.spec.ts @@ -66,6 +66,9 @@ describe('LegacySchoolUc', () => { jest.resetAllMocks(); }); + // Tests with case of authService.checkPermission.mockImplementation(() => throw new ForbiddenException()); + // are missed for both methodes + describe('setMigration is called', () => { describe('when first starting the migration', () => { const setup = () => { @@ -77,7 +80,7 @@ describe('LegacySchoolUc', () => { }); userLoginMigrationService.findMigrationBySchool.mockResolvedValue(null); - authService.checkPermissionByReferences.mockImplementation(() => Promise.resolve()); + authService.checkPermission.mockReturnValueOnce(); schoolService.getSchoolById.mockResolvedValue(school); userLoginMigrationService.setMigration.mockResolvedValue(userLoginMigration); }; @@ -107,7 +110,7 @@ describe('LegacySchoolUc', () => { }); userLoginMigrationService.findMigrationBySchool.mockResolvedValue(userLoginMigration); - authService.checkPermissionByReferences.mockImplementation(() => Promise.resolve()); + authService.checkPermission.mockReturnValueOnce(); schoolService.getSchoolById.mockResolvedValue(school); userLoginMigrationService.setMigration.mockResolvedValue(updatedUserLoginMigration); schoolMigrationService.hasSchoolMigratedUser.mockResolvedValue(true); @@ -138,7 +141,7 @@ describe('LegacySchoolUc', () => { }); userLoginMigrationService.findMigrationBySchool.mockResolvedValue(userLoginMigration); - authService.checkPermissionByReferences.mockImplementation(() => Promise.resolve()); + authService.checkPermission.mockReturnValueOnce(); schoolService.getSchoolById.mockResolvedValue(school); userLoginMigrationService.setMigration.mockResolvedValue(updatedUserLoginMigration); schoolMigrationService.hasSchoolMigratedUser.mockResolvedValue(false); @@ -177,7 +180,7 @@ describe('LegacySchoolUc', () => { }); userLoginMigrationService.findMigrationBySchool.mockResolvedValue(userLoginMigration); - authService.checkPermissionByReferences.mockImplementation(() => Promise.resolve()); + authService.checkPermission.mockReturnValueOnce(); schoolService.getSchoolById.mockResolvedValue(school); userLoginMigrationService.setMigration.mockResolvedValue(updatedUserLoginMigration); }; @@ -208,7 +211,7 @@ describe('LegacySchoolUc', () => { }); userLoginMigrationService.findMigrationBySchool.mockResolvedValue(userLoginMigration); - authService.checkPermissionByReferences.mockImplementation(() => Promise.resolve()); + authService.checkPermission.mockReturnValueOnce(); schoolService.getSchoolById.mockResolvedValue(school); userLoginMigrationService.setMigration.mockResolvedValue(updatedUserLoginMigration); schoolMigrationService.validateGracePeriod.mockImplementation(() => { @@ -241,7 +244,7 @@ describe('LegacySchoolUc', () => { userLoginMigrationService.findMigrationBySchool.mockResolvedValue(userLoginMigration); schoolService.getSchoolById.mockResolvedValue(school); - authService.checkPermissionByReferences.mockImplementation(() => Promise.resolve()); + authService.checkPermission.mockReturnValueOnce(); }; it('should return a migration', async () => { @@ -265,7 +268,7 @@ describe('LegacySchoolUc', () => { userLoginMigrationService.findMigrationBySchool.mockResolvedValue(null); schoolService.getSchoolById.mockResolvedValue(school); - authService.checkPermissionByReferences.mockImplementation(() => Promise.resolve()); + authService.checkPermission.mockReturnValueOnce(); }; it('should return no migration information', async () => { diff --git a/apps/server/src/modules/legacy-school/uc/legacy-school.uc.ts b/apps/server/src/modules/legacy-school/uc/legacy-school.uc.ts index 2ccc9dc5698..d1d13ffb037 100644 --- a/apps/server/src/modules/legacy-school/uc/legacy-school.uc.ts +++ b/apps/server/src/modules/legacy-school/uc/legacy-school.uc.ts @@ -1,6 +1,6 @@ import { Injectable } from '@nestjs/common'; -import { Permission, LegacySchoolDo, UserLoginMigrationDO } from '@shared/domain'; -import { Action, AuthorizableReferenceType, AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationContextBuilder, AuthorizationService } from '@src/modules/authorization'; +import { Permission, LegacySchoolDo, UserLoginMigrationDO, User } from '@shared/domain'; import { SchoolMigrationService, UserLoginMigrationRevertService, @@ -30,10 +30,12 @@ export class LegacySchoolUc { oauthMigrationFinished: boolean, userId: string ): Promise { - await this.authService.checkPermissionByReferences(userId, AuthorizableReferenceType.School, schoolId, { - action: Action.read, - requiredPermissions: [Permission.SCHOOL_EDIT], - }); + const [authorizableUser, school]: [User, LegacySchoolDo] = await Promise.all([ + this.authService.getUserWithPermissions(userId), + this.schoolService.getSchoolById(schoolId), + ]); + + this.checkSchoolAuthorization(authorizableUser, school); const existingUserLoginMigration: UserLoginMigrationDO | null = await this.userLoginMigrationService.findMigrationBySchool(schoolId); @@ -61,8 +63,6 @@ export class LegacySchoolUc { await this.schoolMigrationService.unmarkOutdatedUsers(schoolId); } - const school: LegacySchoolDo = await this.schoolService.getSchoolById(schoolId); - const migrationDto: OauthMigrationDto = new OauthMigrationDto({ oauthMigrationPossible: !updatedUserLoginMigration.closedAt ? updatedUserLoginMigration.startedAt : undefined, oauthMigrationMandatory: updatedUserLoginMigration.mandatorySince, @@ -75,17 +75,17 @@ export class LegacySchoolUc { } async getMigration(schoolId: string, userId: string): Promise { - await this.authService.checkPermissionByReferences(userId, AuthorizableReferenceType.School, schoolId, { - action: Action.read, - requiredPermissions: [Permission.SCHOOL_EDIT], - }); + const [authorizableUser, school]: [User, LegacySchoolDo] = await Promise.all([ + this.authService.getUserWithPermissions(userId), + this.schoolService.getSchoolById(schoolId), + ]); + + this.checkSchoolAuthorization(authorizableUser, school); const userLoginMigration: UserLoginMigrationDO | null = await this.userLoginMigrationService.findMigrationBySchool( schoolId ); - const school: LegacySchoolDo = await this.schoolService.getSchoolById(schoolId); - const migrationDto: OauthMigrationDto = new OauthMigrationDto({ oauthMigrationPossible: userLoginMigration && !userLoginMigration.closedAt ? userLoginMigration.startedAt : undefined, @@ -97,4 +97,9 @@ export class LegacySchoolUc { return migrationDto; } + + private checkSchoolAuthorization(authorizableUser: User, school: LegacySchoolDo): void { + const context = AuthorizationContextBuilder.read([Permission.SCHOOL_EDIT]); + this.authService.checkPermission(authorizableUser, school, context); + } } diff --git a/apps/server/src/modules/oauth-provider/uc/oauth-provider.client-crud.uc.ts b/apps/server/src/modules/oauth-provider/uc/oauth-provider.client-crud.uc.ts index b344f1b6cdc..d0480398ec8 100644 --- a/apps/server/src/modules/oauth-provider/uc/oauth-provider.client-crud.uc.ts +++ b/apps/server/src/modules/oauth-provider/uc/oauth-provider.client-crud.uc.ts @@ -1,7 +1,7 @@ import { Injectable } from '@nestjs/common'; import { OauthProviderService } from '@shared/infra/oauth-provider/index'; import { Permission, User } from '@shared/domain/index'; -import { AuthorizationService } from '@src/modules/authorization/authorization.service'; +import { AuthorizationService } from '@src/modules/authorization'; import { ProviderOauthClient } from '@shared/infra/oauth-provider/dto'; import { ICurrentUser } from '@src/modules/authentication'; diff --git a/apps/server/src/modules/sharing/controller/api-test/sharing-create-token.api.spec.ts b/apps/server/src/modules/sharing/controller/api-test/sharing-create-token.api.spec.ts index 68c3a141f9b..2abd2019fbf 100644 --- a/apps/server/src/modules/sharing/controller/api-test/sharing-create-token.api.spec.ts +++ b/apps/server/src/modules/sharing/controller/api-test/sharing-create-token.api.spec.ts @@ -1,8 +1,8 @@ import { Request } from 'express'; import request from 'supertest'; import { Configuration } from '@hpi-schul-cloud/commons/lib'; -import { EntityManager } from '@mikro-orm/mongodb'; -import { ExecutionContext, INestApplication } from '@nestjs/common'; +import { EntityManager, ObjectId } from '@mikro-orm/mongodb'; +import { ExecutionContext, HttpStatus, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { ApiValidationError } from '@shared/common'; import { Permission } from '@shared/domain'; @@ -101,27 +101,27 @@ describe(`share token creation (api)`, () => { const response = await api.post({ parentId: course.id, parentType: ShareTokenParentType.Course }); - expect(response.status).toEqual(500); + expect(response.status).toEqual(HttpStatus.INTERNAL_SERVER_ERROR); }); }); - describe('with ivalid request data', () => { + describe('with invalid request data', () => { it('should return status 400 on empty parent id', async () => { const response = await api.post({ parentId: '', parentType: ShareTokenParentType.Course, }); - expect(response.status).toEqual(400); + expect(response.status).toEqual(HttpStatus.BAD_REQUEST); }); - it('should return status 403 when parent id is not found', async () => { + it('should return status 404 when parent id is not found', async () => { const response = await api.post({ - parentId: '000011112222333344445555', + parentId: new ObjectId().toHexString(), parentType: ShareTokenParentType.Course, }); - expect(response.status).toEqual(403); + expect(response.status).toEqual(HttpStatus.NOT_FOUND); }); it('should return status 400 on invalid parent id', async () => { @@ -130,7 +130,7 @@ describe(`share token creation (api)`, () => { parentType: ShareTokenParentType.Course, }); - expect(response.status).toEqual(400); + expect(response.status).toEqual(HttpStatus.BAD_REQUEST); }); it('should return status 400 on invalid parent type', async () => { @@ -142,7 +142,7 @@ describe(`share token creation (api)`, () => { parentType: 'invalid', }); - expect(response.status).toEqual(400); + expect(response.status).toEqual(HttpStatus.BAD_REQUEST); }); it('should return status 400 when expiresInDays is invalid integer', async () => { @@ -155,7 +155,7 @@ describe(`share token creation (api)`, () => { expiresInDays: 'foo', }); - expect(response.status).toEqual(400); + expect(response.status).toEqual(HttpStatus.BAD_REQUEST); }); it('should return status 400 when expiresInDays is negative', async () => { @@ -167,7 +167,7 @@ describe(`share token creation (api)`, () => { expiresInDays: -10, }); - expect(response.status).toEqual(400); + expect(response.status).toEqual(HttpStatus.BAD_REQUEST); }); it('should return status 400 when expiresInDays is not an integer', async () => { @@ -179,7 +179,7 @@ describe(`share token creation (api)`, () => { expiresInDays: 2.5, }); - expect(response.status).toEqual(400); + expect(response.status).toEqual(HttpStatus.BAD_REQUEST); }); }); @@ -189,7 +189,7 @@ describe(`share token creation (api)`, () => { const response = await api.post({ parentId: course.id, parentType: ShareTokenParentType.Course }); - expect(response.status).toEqual(201); + expect(response.status).toEqual(HttpStatus.CREATED); }); it('should return a valid result', async () => { @@ -216,7 +216,7 @@ describe(`share token creation (api)`, () => { schoolExclusive: true, }); - expect(response.status).toEqual(201); + expect(response.status).toEqual(HttpStatus.CREATED); }); it('should return a valid result', async () => { @@ -248,7 +248,7 @@ describe(`share token creation (api)`, () => { expiresInDays: 5, }); - expect(response.status).toEqual(201); + expect(response.status).toEqual(HttpStatus.CREATED); }); it('should return a valid result containg the expiration timestamp', async () => { diff --git a/apps/server/src/modules/sharing/controller/api-test/sharing-import-token.api.spec.ts b/apps/server/src/modules/sharing/controller/api-test/sharing-import-token.api.spec.ts index e58940addda..737a24b022e 100644 --- a/apps/server/src/modules/sharing/controller/api-test/sharing-import-token.api.spec.ts +++ b/apps/server/src/modules/sharing/controller/api-test/sharing-import-token.api.spec.ts @@ -1,6 +1,6 @@ import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { EntityManager } from '@mikro-orm/mongodb'; -import { ExecutionContext, INestApplication } from '@nestjs/common'; +import { ExecutionContext, HttpStatus, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; import { ApiValidationError } from '@shared/common'; import { Permission } from '@shared/domain'; @@ -118,16 +118,17 @@ describe(`share token import (api)`, () => { const response = await api.post({ token }, { newName: 'NewName' }); - expect(response.status).toEqual(500); + expect(response.status).toEqual(HttpStatus.INTERNAL_SERVER_ERROR); }); }); describe('with a valid token', () => { it('should return status 201', async () => { const { token } = await setup(); + const response = await api.post({ token }, { newName: 'NewName' }); - expect(response.status).toEqual(201); + expect(response.status).toEqual(HttpStatus.CREATED); }); it('should return a valid result', async () => { @@ -149,23 +150,53 @@ describe(`share token import (api)`, () => { describe('with invalid token', () => { it('should return status 404', async () => { await setup(); + const response = await api.post({ token: 'invalid_token' }, { newName: 'NewName' }); - expect(response.status).toEqual(404); + + expect(response.status).toEqual(HttpStatus.NOT_FOUND); }); }); describe('with invalid context', () => { - it('should return status 403', async () => { + const setup2 = async () => { + const school = schoolFactory.build(); const otherSchool = schoolFactory.build(); - await em.persistAndFlush(otherSchool); - em.clear(); + const roles = roleFactory.buildList(1, { + permissions: [Permission.COURSE_CREATE], + }); - const { token } = await setup({ + const user = userFactory.build({ school, roles }); + const course = courseFactory.build({ teachers: [user] }); + await em.persistAndFlush([user, course, otherSchool]); + + const context = { contextType: ShareTokenContextType.School, contextId: otherSchool.id, - }); - const response = await api.post({ token }, { newName: 'NewName' }); - expect(response.status).toEqual(403); + }; + + const shareToken = await shareTokenService.createToken( + { + parentType: ShareTokenParentType.Course, + parentId: course.id, + }, + { context } + ); + + em.clear(); + + currentUser = mapUserToCurrentUser(user); + + return { + shareTokenFromDifferentCourse: shareToken.token, + }; + }; + + it('should return status 403', async () => { + const { shareTokenFromDifferentCourse } = await setup2(); + + const response = await api.post({ token: shareTokenFromDifferentCourse }, { newName: 'NewName' }); + + expect(response.status).toEqual(HttpStatus.FORBIDDEN); }); }); @@ -175,7 +206,7 @@ describe(`share token import (api)`, () => { // @ts-expect-error invalid new name const response = await api.post({ token }, { newName: 42 }); - expect(response.status).toEqual(501); + expect(response.status).toEqual(HttpStatus.NOT_IMPLEMENTED); }); }); }); diff --git a/apps/server/src/modules/sharing/controller/api-test/sharing-lookup-token.api.spec.ts b/apps/server/src/modules/sharing/controller/api-test/sharing-lookup-token.api.spec.ts index a5c1304a730..7065c06a026 100644 --- a/apps/server/src/modules/sharing/controller/api-test/sharing-lookup-token.api.spec.ts +++ b/apps/server/src/modules/sharing/controller/api-test/sharing-lookup-token.api.spec.ts @@ -1,167 +1,210 @@ -import { Request } from 'express'; -import request from 'supertest'; -import { Configuration } from '@hpi-schul-cloud/commons/lib'; +import { Configuration } from '@hpi-schul-cloud/commons'; import { EntityManager } from '@mikro-orm/mongodb'; -import { ExecutionContext, INestApplication } from '@nestjs/common'; +import { HttpStatus, INestApplication } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; -import { ApiValidationError } from '@shared/common'; import { Permission } from '@shared/domain'; -import { ICurrentUser } from '@src/modules/authentication'; -import { - cleanupCollections, - courseFactory, - mapUserToCurrentUser, - roleFactory, - schoolFactory, - userFactory, -} from '@shared/testing'; -import { JwtAuthGuard } from '@src/modules/authentication/guard/jwt-auth.guard'; +import { TestApiClient, UserAndAccountTestFactory, courseFactory, schoolFactory } from '@shared/testing'; import { ServerTestModule } from '@src/modules/server'; import { ShareTokenService } from '../../service'; -import { ShareTokenInfoResponse, ShareTokenResponse, ShareTokenUrlParams } from '../dto'; -import { ShareTokenContext, ShareTokenContextType, ShareTokenParentType } from '../../domainobject/share-token.do'; - -const baseRouteName = '/sharetoken'; - -class API { - app: INestApplication; - - constructor(app: INestApplication) { - this.app = app; - } - - async get(urlParams: ShareTokenUrlParams) { - const response = await request(this.app.getHttpServer()) - .get(`${baseRouteName}/${urlParams.token}`) - .set('Accept', 'application/json'); - - return { - result: response.body as ShareTokenResponse, - error: response.body as ApiValidationError, - status: response.status, - }; - } -} +import { ShareTokenInfoResponse } from '../dto'; +import { ShareTokenContextType, ShareTokenParentType } from '../../domainobject/share-token.do'; describe(`share token lookup (api)`, () => { let app: INestApplication; let em: EntityManager; - let currentUser: ICurrentUser; let shareTokenService: ShareTokenService; - let api: API; + let testApiClient: TestApiClient; beforeAll(async () => { const module: TestingModule = await Test.createTestingModule({ imports: [ServerTestModule], - }) - .overrideGuard(JwtAuthGuard) - .useValue({ - canActivate(context: ExecutionContext) { - const req: Request = context.switchToHttp().getRequest(); - req.user = currentUser; - return true; - }, - }) - .compile(); + }).compile(); app = module.createNestApplication(); await app.init(); em = module.get(EntityManager); shareTokenService = module.get(ShareTokenService); - api = new API(app); + testApiClient = new TestApiClient(app, 'sharetoken'); }); afterAll(async () => { await app.close(); }); - beforeEach(() => { - Configuration.set('FEATURE_COURSE_SHARE_NEW', true); - }); + describe('with the feature disabled', () => { + const setup = async () => { + Configuration.set('FEATURE_COURSE_SHARE_NEW', false); - const setup = async (context?: ShareTokenContext) => { - await cleanupCollections(em); - const school = schoolFactory.build(); - const roles = roleFactory.buildList(1, { - permissions: [Permission.COURSE_CREATE], - }); - const user = userFactory.build({ school, roles }); - const course = courseFactory.build({ teachers: [user] }); - await em.persistAndFlush([user, course]); - - const shareToken = await shareTokenService.createToken( - { - parentType: ShareTokenParentType.Course, - parentId: course.id, - }, - { context } - ); - - em.clear(); - - currentUser = mapUserToCurrentUser(user); - - return { - parentType: ShareTokenParentType.Course, - parentName: course.getMetadata().title, - token: shareToken.token, + const parentType = ShareTokenParentType.Course; + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({}, [Permission.COURSE_CREATE]); + const course = courseFactory.build({ teachers: [teacherUser] }); + + await em.persistAndFlush([course, teacherAccount, teacherUser]); + em.clear(); + + const shareToken = await shareTokenService.createToken( + { + parentType, + parentId: course.id, + }, + undefined + ); + + const loggedInClient = await testApiClient.login(teacherAccount); + + return { + token: shareToken.token, + loggedInClient, + }; }; - }; - describe('with the feature disabled', () => { it('should return status 500', async () => { - Configuration.set('FEATURE_COURSE_SHARE_NEW', false); - const { token } = await setup(); + const { token, loggedInClient } = await setup(); - const response = await api.get({ token }); + const response = await loggedInClient.get(token); - expect(response.status).toEqual(500); + expect(response.status).toEqual(HttpStatus.INTERNAL_SERVER_ERROR); + expect(response.body).toEqual({ + code: 500, + message: 'Import Course Feature not enabled', + title: 'Internal Server Error', + type: 'INTERNAL_SERVER_ERROR', + }); }); }); + // test and setup for other feature flags are missed + describe('with a valid token', () => { - it('should return status 200', async () => { - const { token } = await setup(); - const response = await api.get({ token }); + const setup = async () => { + Configuration.set('FEATURE_COURSE_SHARE_NEW', true); - expect(response.status).toEqual(200); - }); + const parentType = ShareTokenParentType.Course; + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({}, [Permission.COURSE_CREATE]); + const course = courseFactory.build({ teachers: [teacherUser] }); - it('should return a valid result', async () => { - const { parentType, parentName, token } = await setup(); - const response = await api.get({ token }); + await em.persistAndFlush([course, teacherAccount, teacherUser]); + em.clear(); + + const shareToken = await shareTokenService.createToken( + { + parentType, + parentId: course.id, + }, + undefined + ); + + const loggedInClient = await testApiClient.login(teacherAccount); const expectedResult: ShareTokenInfoResponse = { - token, + token: shareToken.token, parentType, - parentName, + parentName: course.getMetadata().title, }; - expect(response.result).toEqual(expectedResult); + return { + expectedResult, + token: shareToken.token, + loggedInClient, + }; + }; + + it('should return status 200 with correct formated body', async () => { + const { token, loggedInClient, expectedResult } = await setup(); + + const response = await loggedInClient.get(token); + + expect(response.status).toEqual(HttpStatus.OK); + expect(response.body).toEqual(expectedResult); }); }); describe('with invalid token', () => { + const setup = async () => { + Configuration.set('FEATURE_COURSE_SHARE_NEW', true); + + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({}, [Permission.COURSE_CREATE]); + const course = courseFactory.build({ teachers: [teacherUser] }); + + await em.persistAndFlush([course, teacherAccount, teacherUser]); + em.clear(); + + const loggedInClient = await testApiClient.login(teacherAccount); + + return { + invalidToken: 'invalid_token', + loggedInClient, + }; + }; + it('should return status 404', async () => { - await setup(); - const response = await api.get({ token: 'invalid_token' }); - expect(response.status).toEqual(404); + const { invalidToken, loggedInClient } = await setup(); + + const response = await loggedInClient.get(invalidToken); + + expect(response.status).toEqual(HttpStatus.NOT_FOUND); + expect(response.body).toEqual({ + code: 404, + message: 'The requested ShareToken: [object Object] has not been found.', + title: 'Not Found', + type: 'NOT_FOUND', + }); }); }); describe('with invalid context', () => { - it('should return status 403', async () => { + const setup = async () => { + Configuration.set('FEATURE_COURSE_SHARE_NEW', true); + + const parentType = ShareTokenParentType.Course; + const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({}, [Permission.COURSE_CREATE]); const otherSchool = schoolFactory.build(); - await em.persistAndFlush(otherSchool); + const course = courseFactory.build({ teachers: [teacherUser] }); + + await em.persistAndFlush([course, teacherAccount, teacherUser, otherSchool]); em.clear(); - const { token } = await setup({ + const context = { contextType: ShareTokenContextType.School, contextId: otherSchool.id, + }; + + const shareToken = await shareTokenService.createToken( + { + parentType, + parentId: course.id, + }, + { context } + ); + + const loggedInClient = await testApiClient.login(teacherAccount); + + const expectedResult: ShareTokenInfoResponse = { + token: shareToken.token, + parentType, + parentName: course.getMetadata().title, + }; + + return { + expectedResult, + token: shareToken.token, + loggedInClient, + }; + }; + + it('should return status 403', async () => { + const { token, loggedInClient } = await setup(); + + const response = await loggedInClient.get(token); + + expect(response.status).toEqual(HttpStatus.FORBIDDEN); + expect(response.body).toEqual({ + code: 403, + message: 'Forbidden', + title: 'Forbidden', + type: 'FORBIDDEN', }); - const response = await api.get({ token }); - expect(response.status).toEqual(403); }); }); }); diff --git a/apps/server/src/modules/sharing/mapper/context-type.mapper.spec.ts b/apps/server/src/modules/sharing/mapper/context-type.mapper.spec.ts index 9684a1dbb58..68c0ccbd972 100644 --- a/apps/server/src/modules/sharing/mapper/context-type.mapper.spec.ts +++ b/apps/server/src/modules/sharing/mapper/context-type.mapper.spec.ts @@ -1,5 +1,5 @@ import { NotImplementedException } from '@nestjs/common'; -import { AuthorizableReferenceType } from '@src/modules/authorization'; +import { AuthorizableReferenceType } from '@src/modules/authorization/domain'; import { ShareTokenContextType } from '../domainobject/share-token.do'; import { ShareTokenContextTypeMapper } from './context-type.mapper'; diff --git a/apps/server/src/modules/sharing/mapper/context-type.mapper.ts b/apps/server/src/modules/sharing/mapper/context-type.mapper.ts index 7c9b4c8bb1d..05ed42843c7 100644 --- a/apps/server/src/modules/sharing/mapper/context-type.mapper.ts +++ b/apps/server/src/modules/sharing/mapper/context-type.mapper.ts @@ -1,5 +1,5 @@ import { NotImplementedException } from '@nestjs/common'; -import { AuthorizableReferenceType } from '@src/modules/authorization'; +import { AuthorizableReferenceType } from '@src/modules/authorization/domain'; import { ShareTokenContextType } from '../domainobject/share-token.do'; export class ShareTokenContextTypeMapper { @@ -12,6 +12,7 @@ export class ShareTokenContextTypeMapper { if (!res) { throw new NotImplementedException(); } + return res; } } diff --git a/apps/server/src/modules/sharing/mapper/parent-type.mapper.spec.ts b/apps/server/src/modules/sharing/mapper/parent-type.mapper.spec.ts index c6d8669bc70..4f8750245b4 100644 --- a/apps/server/src/modules/sharing/mapper/parent-type.mapper.spec.ts +++ b/apps/server/src/modules/sharing/mapper/parent-type.mapper.spec.ts @@ -1,5 +1,5 @@ import { NotImplementedException } from '@nestjs/common'; -import { AuthorizableReferenceType } from '@src/modules/authorization'; +import { AuthorizableReferenceType } from '@src/modules/authorization/domain'; import { ShareTokenParentType } from '../domainobject/share-token.do'; import { ShareTokenParentTypeMapper } from './parent-type.mapper'; diff --git a/apps/server/src/modules/sharing/mapper/parent-type.mapper.ts b/apps/server/src/modules/sharing/mapper/parent-type.mapper.ts index 54d8ceb0470..2ea01ea39f9 100644 --- a/apps/server/src/modules/sharing/mapper/parent-type.mapper.ts +++ b/apps/server/src/modules/sharing/mapper/parent-type.mapper.ts @@ -1,5 +1,5 @@ import { NotImplementedException } from '@nestjs/common'; -import { AuthorizableReferenceType } from '@src/modules/authorization'; +import { AuthorizableReferenceType } from '@src/modules/authorization/domain'; import { ShareTokenParentType } from '../domainobject/share-token.do'; export class ShareTokenParentTypeMapper { diff --git a/apps/server/src/modules/sharing/sharing.module.ts b/apps/server/src/modules/sharing/sharing.module.ts index f09214e9cf8..519033065b5 100644 --- a/apps/server/src/modules/sharing/sharing.module.ts +++ b/apps/server/src/modules/sharing/sharing.module.ts @@ -1,6 +1,7 @@ import { Module } from '@nestjs/common'; import { LoggerModule } from '@src/core/logger'; import { AuthorizationModule } from '@src/modules/authorization'; +import { AuthorizationReferenceModule } from '@src/modules/authorization/authorization-reference.module'; import { ShareTokenController } from './controller/share-token.controller'; import { ShareTokenUC } from './uc'; import { ShareTokenService, TokenGenerator } from './service'; @@ -10,7 +11,7 @@ import { LearnroomModule } from '../learnroom'; import { TaskModule } from '../task'; @Module({ - imports: [AuthorizationModule, LoggerModule, LearnroomModule, LessonModule, TaskModule], + imports: [AuthorizationModule, AuthorizationReferenceModule, LoggerModule, LearnroomModule, LessonModule, TaskModule], controllers: [], providers: [ShareTokenService, TokenGenerator, ShareTokenRepo], exports: [ShareTokenService], @@ -18,7 +19,15 @@ import { TaskModule } from '../task'; export class SharingModule {} @Module({ - imports: [SharingModule, AuthorizationModule, LearnroomModule, LessonModule, TaskModule, LoggerModule], + imports: [ + SharingModule, + AuthorizationModule, + AuthorizationReferenceModule, + LearnroomModule, + LessonModule, + TaskModule, + LoggerModule, + ], controllers: [ShareTokenController], providers: [ShareTokenUC], }) diff --git a/apps/server/src/modules/sharing/uc/share-token.uc.spec.ts b/apps/server/src/modules/sharing/uc/share-token.uc.spec.ts index 73234960794..8f33076cff2 100644 --- a/apps/server/src/modules/sharing/uc/share-token.uc.spec.ts +++ b/apps/server/src/modules/sharing/uc/share-token.uc.spec.ts @@ -15,7 +15,8 @@ import { userFactory, } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { Action, AuthorizableReferenceType, AuthorizationService } from '@src/modules/authorization'; +import { Action, AuthorizationService } from '@src/modules/authorization'; +import { AuthorizableReferenceType, AuthorizationReferenceService } from '@src/modules/authorization/domain'; import { CopyElementType, CopyStatus, CopyStatusEnum } from '@src/modules/copy-helper'; import { CourseCopyService } from '@src/modules/learnroom'; import { CourseService } from '@src/modules/learnroom/service'; @@ -33,6 +34,7 @@ describe('ShareTokenUC', () => { let lessonCopyService: DeepMocked; let taskCopyService: DeepMocked; let authorization: DeepMocked; + let authorizationReferenceService: DeepMocked; let courseService: DeepMocked; let lessonRepo: DeepMocked; @@ -48,6 +50,10 @@ describe('ShareTokenUC', () => { provide: AuthorizationService, useValue: createMock(), }, + { + provide: AuthorizationReferenceService, + useValue: createMock(), + }, { provide: CourseCopyService, useValue: createMock(), @@ -81,8 +87,10 @@ describe('ShareTokenUC', () => { lessonCopyService = module.get(LessonCopyService); taskCopyService = module.get(TaskCopyService); authorization = module.get(AuthorizationService); + authorizationReferenceService = module.get(AuthorizationReferenceService); courseService = module.get(CourseService); lessonRepo = module.get(LessonRepo); + await setupEntities(); }); @@ -93,6 +101,7 @@ describe('ShareTokenUC', () => { beforeEach(() => { jest.resetAllMocks(); jest.clearAllMocks(); + // configuration sets must be part of the setup functions and part of the describe when ...and feature x is activated Configuration.set('FEATURE_COURSE_SHARE_NEW', true); Configuration.set('FEATURE_LESSON_SHARE', true); Configuration.set('FEATURE_TASK_SHARE', true); @@ -129,7 +138,7 @@ describe('ShareTokenUC', () => { parentType: ShareTokenParentType.Course, }); - expect(authorization.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( user.id, AuthorizableReferenceType.Course, course.id, @@ -148,7 +157,7 @@ describe('ShareTokenUC', () => { parentType: ShareTokenParentType.Course, }); - expect(authorization.checkPermissionByReferences).toHaveBeenCalledTimes(1); + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledTimes(1); }); it('should call the service', async () => { @@ -190,7 +199,7 @@ describe('ShareTokenUC', () => { parentType: ShareTokenParentType.Lesson, }); - expect(authorization.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( user.id, AuthorizableReferenceType.Lesson, lesson.id, @@ -209,7 +218,7 @@ describe('ShareTokenUC', () => { parentType: ShareTokenParentType.Lesson, }); - expect(authorization.checkPermissionByReferences).toHaveBeenCalledTimes(1); + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledTimes(1); }); it('should call the service', async () => { @@ -251,7 +260,7 @@ describe('ShareTokenUC', () => { parentType: ShareTokenParentType.Task, }); - expect(authorization.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( user.id, AuthorizableReferenceType.Task, task.id, @@ -270,7 +279,7 @@ describe('ShareTokenUC', () => { parentType: ShareTokenParentType.Task, }); - expect(authorization.checkPermissionByReferences).toHaveBeenCalledTimes(1); + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledTimes(1); }); it('should call the service', async () => { @@ -309,7 +318,7 @@ describe('ShareTokenUC', () => { } ); - expect(authorization.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( user.id, AuthorizableReferenceType.Course, course.id, @@ -337,7 +346,7 @@ describe('ShareTokenUC', () => { } ); - expect(authorization.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( user.id, AuthorizableReferenceType.School, school.id, @@ -574,7 +583,7 @@ describe('ShareTokenUC', () => { await uc.lookupShareToken(user.id, shareToken.token); - expect(authorization.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( user.id, AuthorizableReferenceType.School, school.id, @@ -601,7 +610,7 @@ describe('ShareTokenUC', () => { await uc.lookupShareToken(user.id, shareToken.token); - expect(authorization.checkPermissionByReferences).not.toHaveBeenCalled(); + expect(authorizationReferenceService.checkPermissionByReferences).not.toHaveBeenCalled(); }); }); }); @@ -686,7 +695,7 @@ describe('ShareTokenUC', () => { await uc.importShareToken(user.id, shareToken.token, 'NewName'); - expect(authorization.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( user.id, AuthorizableReferenceType.School, school.id, @@ -706,7 +715,7 @@ describe('ShareTokenUC', () => { await uc.importShareToken(user.id, shareToken.token, 'NewName'); - expect(authorization.checkPermissionByReferences).not.toHaveBeenCalled(); + expect(authorizationReferenceService.checkPermissionByReferences).not.toHaveBeenCalled(); }); }); }); @@ -803,7 +812,7 @@ describe('ShareTokenUC', () => { await uc.importShareToken(user.id, shareToken.token, 'NewName'); - expect(authorization.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( user.id, AuthorizableReferenceType.School, school.id, @@ -823,7 +832,7 @@ describe('ShareTokenUC', () => { await uc.importShareToken(user.id, shareToken.token, 'NewName'); - expect(authorization.checkPermissionByReferences).not.toHaveBeenCalled(); + expect(authorizationReferenceService.checkPermissionByReferences).not.toHaveBeenCalled(); }); }); }); @@ -919,7 +928,7 @@ describe('ShareTokenUC', () => { await uc.importShareToken(user.id, shareToken.token, 'NewName'); - expect(authorization.checkPermissionByReferences).toHaveBeenCalledWith( + expect(authorizationReferenceService.checkPermissionByReferences).toHaveBeenCalledWith( user.id, AuthorizableReferenceType.School, school.id, @@ -939,7 +948,7 @@ describe('ShareTokenUC', () => { await uc.importShareToken(user.id, shareToken.token, 'NewName'); - expect(authorization.checkPermissionByReferences).not.toHaveBeenCalled(); + expect(authorizationReferenceService.checkPermissionByReferences).not.toHaveBeenCalled(); }); }); }); @@ -962,6 +971,7 @@ describe('ShareTokenUC', () => { service.lookupToken.mockResolvedValue(shareToken); jest.spyOn(ShareTokenUC.prototype as any, 'checkFeatureEnabled').mockReturnValue(undefined); jest.spyOn(ShareTokenUC.prototype as any, 'checkCreatePermission').mockReturnValue(undefined); + await expect(uc.importShareToken('userId', shareToken.token, 'NewName')).rejects.toThrowError( NotImplementedException ); diff --git a/apps/server/src/modules/sharing/uc/share-token.uc.ts b/apps/server/src/modules/sharing/uc/share-token.uc.ts index 0b72be7ce31..b2bbc635403 100644 --- a/apps/server/src/modules/sharing/uc/share-token.uc.ts +++ b/apps/server/src/modules/sharing/uc/share-token.uc.ts @@ -2,7 +2,8 @@ import { Configuration } from '@hpi-schul-cloud/commons/lib'; import { BadRequestException, Injectable, InternalServerErrorException, NotImplementedException } from '@nestjs/common'; import { EntityId, Permission } from '@shared/domain'; import { LegacyLogger } from '@src/core/logger'; -import { Action, AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationContextBuilder, AuthorizationService } from '@src/modules/authorization'; +import { AuthorizationReferenceService } from '@src/modules/authorization/domain'; import { CopyStatus } from '@src/modules/copy-helper'; import { CourseCopyService } from '@src/modules/learnroom'; import { CourseService } from '@src/modules/learnroom/service'; @@ -24,6 +25,7 @@ export class ShareTokenUC { constructor( private readonly shareTokenService: ShareTokenService, private readonly authorizationService: AuthorizationService, + private readonly authorizationReferenceService: AuthorizationReferenceService, private readonly courseCopyService: CourseCopyService, private readonly lessonCopyService: LessonCopyService, private readonly courseService: CourseService, @@ -177,18 +179,26 @@ export class ShareTokenUC { requiredPermissions = [Permission.HOMEWORK_CREATE]; } - await this.authorizationService.checkPermissionByReferences(userId, allowedParentType, payload.parentId, { - action: Action.write, - requiredPermissions, - }); + const authorizationContext = AuthorizationContextBuilder.write(requiredPermissions); + + await this.authorizationReferenceService.checkPermissionByReferences( + userId, + allowedParentType, + payload.parentId, + authorizationContext + ); } private async checkContextReadPermission(userId: EntityId, context: ShareTokenContext) { const allowedContextType = ShareTokenContextTypeMapper.mapToAllowedAuthorizationEntityType(context.contextType); - await this.authorizationService.checkPermissionByReferences(userId, allowedContextType, context.contextId, { - action: Action.read, - requiredPermissions: [], - }); + const authorizationContext = AuthorizationContextBuilder.read([]); + + await this.authorizationReferenceService.checkPermissionByReferences( + userId, + allowedContextType, + context.contextId, + authorizationContext + ); } private async checkCreatePermission(userId: EntityId, parentType: ShareTokenParentType) { @@ -221,16 +231,19 @@ export class ShareTokenUC { private checkFeatureEnabled(parentType: ShareTokenParentType) { switch (parentType) { case ShareTokenParentType.Course: + // Configuration.get is the deprecated way to read envirment variables if (!(Configuration.get('FEATURE_COURSE_SHARE_NEW') as boolean)) { throw new InternalServerErrorException('Import Course Feature not enabled'); } break; case ShareTokenParentType.Lesson: + // Configuration.get is the deprecated way to read envirment variables if (!(Configuration.get('FEATURE_LESSON_SHARE') as boolean)) { throw new InternalServerErrorException('Import Lesson Feature not enabled'); } break; case ShareTokenParentType.Task: + // Configuration.get is the deprecated way to read envirment variables if (!(Configuration.get('FEATURE_TASK_SHARE') as boolean)) { throw new InternalServerErrorException('Import Task Feature not enabled'); } diff --git a/apps/server/src/modules/task/uc/task-copy.uc.spec.ts b/apps/server/src/modules/task/uc/task-copy.uc.spec.ts index a7666d479a9..2ad21c68dc2 100644 --- a/apps/server/src/modules/task/uc/task-copy.uc.spec.ts +++ b/apps/server/src/modules/task/uc/task-copy.uc.spec.ts @@ -3,15 +3,14 @@ import { Configuration } from '@hpi-schul-cloud/commons'; import { ObjectId } from '@mikro-orm/mongodb'; import { ForbiddenException, InternalServerErrorException, NotFoundException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; -import { BaseDO, User } from '@shared/domain'; import { CourseRepo, LessonRepo, TaskRepo, UserRepo } from '@shared/repo'; import { courseFactory, lessonFactory, setupEntities, taskFactory, userFactory } from '@shared/testing'; -import { Action, AuthorizableReferenceType, AuthorizationService } from '@src/modules/authorization'; +import { Action, AuthorizationContextBuilder, AuthorizationService } from '@src/modules/authorization'; import { CopyElementType, CopyHelperService, CopyStatusEnum } from '@src/modules/copy-helper'; import { FilesStorageClientAdapterService } from '@src/modules/files-storage-client'; -import { AuthorizableObject } from '@shared/domain/domain-object'; import { TaskCopyService } from '../service'; import { TaskCopyUC } from './task-copy.uc'; +import { TaskCopyParentParams } from '../types'; describe('task copy uc', () => { let uc: TaskCopyUC; @@ -92,14 +91,8 @@ describe('task copy uc', () => { const lesson = lessonFactory.buildWithId({ course }); const allTasks = taskFactory.buildList(3, { course }); const task = allTasks[0]; - authorisation.getUserWithPermissions.mockResolvedValue(user); - taskRepo.findById.mockResolvedValue(task); - lessonRepo.findById.mockResolvedValue(lesson); - taskRepo.findBySingleParent.mockResolvedValue([allTasks, allTasks.length]); - courseRepo.findById.mockResolvedValue(course); - authorisation.hasPermission.mockReturnValue(true); const copyName = 'name of the copy'; - copyHelperService.deriveCopyName.mockReturnValue(copyName); + const copy = taskFactory.buildWithId({ creator: user, course }); const status = { title: 'taskCopy', @@ -108,9 +101,16 @@ describe('task copy uc', () => { copyEntity: copy, originalEntity: task, }; - taskCopyService.copyTask.mockResolvedValue(status); - taskRepo.save.mockResolvedValue(undefined); - const userId = user.id; + + authorisation.getUserWithPermissions.mockResolvedValueOnce(user); + taskRepo.findById.mockResolvedValueOnce(task); + lessonRepo.findById.mockResolvedValueOnce(lesson); + taskRepo.findBySingleParent.mockResolvedValueOnce([allTasks, allTasks.length]); + courseRepo.findById.mockResolvedValueOnce(course); + authorisation.hasPermission.mockReturnValueOnce(true).mockReturnValueOnce(true); + copyHelperService.deriveCopyName.mockReturnValueOnce(copyName); + taskCopyService.copyTask.mockResolvedValueOnce(status); + taskRepo.save.mockResolvedValueOnce(); return { user, @@ -121,15 +121,16 @@ describe('task copy uc', () => { copy, allTasks, status, - userId, + userId: user.id, }; }; describe('feature is deactivated', () => { it('should throw InternalServerErrorException', async () => { + const { course, user, task, userId } = setup(); Configuration.set('FEATURE_COPY_SERVICE_ENABLED', false); - await expect(uc.copyTask('user.id', 'task.id', { courseId: 'course.id', userId: 'test' })).rejects.toThrowError( + await expect(uc.copyTask(user.id, task.id, { courseId: course.id, userId })).rejects.toThrowError( InternalServerErrorException ); }); @@ -214,15 +215,9 @@ describe('task copy uc', () => { const { course, user, task, userId } = setup(); await uc.copyTask(user.id, task.id, { courseId: course.id, userId }); - expect(authorisation.checkPermissionByReferences).toBeCalledWith( - user.id, - AuthorizableReferenceType.Course, - course.id, - { - action: Action.write, - requiredPermissions: [], - } - ); + + const context = AuthorizationContextBuilder.write([]); + expect(authorisation.checkPermission).toBeCalledWith(user, course, context); }); it('should pass authorisation check without destination course', async () => { @@ -230,10 +225,8 @@ describe('task copy uc', () => { await uc.copyTask(user.id, task.id, { userId }); - expect(authorisation.hasPermission).not.toBeCalledWith(user, course, { - action: Action.write, - requiredPermissions: [], - }); + const context = AuthorizationContextBuilder.write([]); + expect(authorisation.hasPermission).not.toBeCalledWith(user, course, context); }); it('should check authorisation for destination lesson', async () => { @@ -260,57 +253,64 @@ describe('task copy uc', () => { describe('when access to task is forbidden', () => { const setupWithTaskForbidden = () => { + Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); + const user = userFactory.buildWithId(); const course = courseFactory.buildWithId(); const lesson = lessonFactory.buildWithId({ course }); const task = taskFactory.buildWithId(); - userRepo.findById.mockResolvedValue(user); - taskRepo.findById.mockResolvedValue(task); - // authorisation should not be mocked - authorisation.hasPermission.mockImplementation((u: User, e: AuthorizableObject | BaseDO) => e !== task); - return { user, course, lesson, task }; + + userRepo.findById.mockResolvedValueOnce(user); + taskRepo.findById.mockResolvedValueOnce(task); + authorisation.hasPermission.mockReturnValueOnce(false); + + const parentParams: TaskCopyParentParams = { + courseId: course.id, + lessonId: lesson.id, + userId: new ObjectId().toHexString(), + }; + + return { user, course, lesson, task, parentParams }; }; it('should throw NotFoundException', async () => { - const { course, lesson, user, task } = setupWithTaskForbidden(); - - try { - await uc.copyTask(user.id, task.id, { - courseId: course.id, - lessonId: lesson.id, - userId: new ObjectId().toHexString(), - }); - throw new Error('should have failed'); - } catch (err) { - expect(err).toBeInstanceOf(NotFoundException); - } + const { user, task, parentParams } = setupWithTaskForbidden(); + + await expect(uc.copyTask(user.id, task.id, parentParams)).rejects.toThrowError( + new NotFoundException('could not find task to copy') + ); }); }); describe('when access to course is forbidden', () => { const setupWithCourseForbidden = () => { + Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); + const user = userFactory.buildWithId(); const course = courseFactory.buildWithId(); const task = taskFactory.buildWithId(); - userRepo.findById.mockResolvedValue(user); - taskRepo.findById.mockResolvedValue(task); - // authorisation should not be mocked - authorisation.hasPermission.mockImplementation((u: User, e: AuthorizableObject | BaseDO) => e !== course); - authorisation.checkPermissionByReferences.mockImplementation(() => { + + userRepo.findById.mockResolvedValueOnce(user); + taskRepo.findById.mockResolvedValueOnce(task); + courseRepo.findById.mockResolvedValueOnce(course); + authorisation.hasPermission.mockReturnValueOnce(true).mockReturnValueOnce(true); + authorisation.checkPermission.mockImplementationOnce(() => { throw new ForbiddenException(); }); - return { user, course, task }; + + const parentParams: TaskCopyParentParams = { courseId: course.id, userId: new ObjectId().toHexString() }; + + return { + userId: user.id, + taskId: task.id, + parentParams, + }; }; it('should throw Forbidden Exception', async () => { - const { course, user, task } = setupWithCourseForbidden(); - - try { - await uc.copyTask(user.id, task.id, { courseId: course.id, userId: new ObjectId().toHexString() }); - throw new Error('should have failed'); - } catch (err) { - expect(err).toBeInstanceOf(ForbiddenException); - } + const { userId, taskId, parentParams } = setupWithCourseForbidden(); + + await expect(uc.copyTask(userId, taskId, parentParams)).rejects.toThrowError(new ForbiddenException()); }); }); }); @@ -355,32 +355,35 @@ describe('task copy uc', () => { describe('when access to lesson is forbidden', () => { const setupWithLessonForbidden = () => { + Configuration.set('FEATURE_COPY_SERVICE_ENABLED', true); + const user = userFactory.buildWithId(); const course = courseFactory.buildWithId(); const lesson = lessonFactory.buildWithId({ course }); const task = taskFactory.buildWithId(); - userRepo.findById.mockResolvedValue(user); - taskRepo.findById.mockResolvedValue(task); - courseRepo.findById.mockResolvedValue(course); - lessonRepo.findById.mockResolvedValue(lesson); - // Authorisation should not be mocked - authorisation.hasPermission.mockImplementation((u: User, e: AuthorizableObject | BaseDO) => { - if (e === lesson) return false; - return true; - }); - return { user, lesson, task }; + userRepo.findById.mockResolvedValueOnce(user); + taskRepo.findById.mockResolvedValueOnce(task); + courseRepo.findById.mockResolvedValueOnce(course); + lessonRepo.findById.mockResolvedValueOnce(lesson); + // first canReadTask > second canWriteLesson + authorisation.hasPermission.mockReturnValueOnce(true).mockReturnValueOnce(false); + + const parentParams: TaskCopyParentParams = { lessonId: lesson.id, userId: new ObjectId().toHexString() }; + + return { + userId: user.id, + taskId: task.id, + parentParams, + }; }; it('should throw Forbidden Exception', async () => { - const { lesson, user, task } = setupWithLessonForbidden(); - - try { - await uc.copyTask(user.id, task.id, { lessonId: lesson.id, userId: new ObjectId().toHexString() }); - throw new Error('should have failed'); - } catch (err) { - expect(err).toBeInstanceOf(ForbiddenException); - } + const { userId, taskId, parentParams } = setupWithLessonForbidden(); + + await expect(uc.copyTask(userId, taskId, parentParams)).rejects.toThrowError( + new ForbiddenException('you dont have permission to add to this lesson') + ); }); }); }); diff --git a/apps/server/src/modules/task/uc/task-copy.uc.ts b/apps/server/src/modules/task/uc/task-copy.uc.ts index b1cdd212919..94b0a5a3acb 100644 --- a/apps/server/src/modules/task/uc/task-copy.uc.ts +++ b/apps/server/src/modules/task/uc/task-copy.uc.ts @@ -1,13 +1,8 @@ import { Configuration } from '@hpi-schul-cloud/commons'; import { ForbiddenException, Injectable, InternalServerErrorException, NotFoundException } from '@nestjs/common'; -import { Course, EntityId, LessonEntity, User } from '@shared/domain'; +import { Course, EntityId, Task, LessonEntity, User } from '@shared/domain'; import { CourseRepo, LessonRepo, TaskRepo } from '@shared/repo'; -import { - Action, - AuthorizableReferenceType, - AuthorizationContextBuilder, - AuthorizationService, -} from '@src/modules/authorization'; +import { AuthorizationContextBuilder, AuthorizationService } from '@src/modules/authorization'; import { CopyHelperService, CopyStatus } from '@src/modules/copy-helper'; import { TaskCopyService } from '../service'; import { TaskCopyParentParams } from '../types'; @@ -24,46 +19,73 @@ export class TaskCopyUC { ) {} async copyTask(userId: EntityId, taskId: EntityId, parentParams: TaskCopyParentParams): Promise { - this.featureEnabled(); - const user = await this.authorisation.getUserWithPermissions(userId); - const originalTask = await this.taskRepo.findById(taskId); - if (!this.authorisation.hasPermission(user, originalTask, AuthorizationContextBuilder.read([]))) { - throw new NotFoundException('could not find task to copy'); - } + this.checkFeatureEnabled(); + + // i put it to promise all, it do not look like any more information can be expose over errors if it is called between the authorizations + // TODO: Add try catch around it with throw BadRequest invalid data + const [authorizableUser, originalTask, destinationCourse]: [User, Task, Course | undefined] = await Promise.all([ + this.authorisation.getUserWithPermissions(userId), + this.taskRepo.findById(taskId), + this.getDestinationCourse(parentParams.courseId), + ]); - const destinationCourse = await this.getDestinationCourse(parentParams.courseId); - if (parentParams.courseId) { - await this.authorisation.checkPermissionByReferences( - userId, - AuthorizableReferenceType.Course, - parentParams.courseId, - { - action: Action.write, - requiredPermissions: [], - } - ); + this.checkOriginalTaskAuthorization(authorizableUser, originalTask); + + if (destinationCourse) { + this.checkDestinationCourseAuthorisation(authorizableUser, destinationCourse); } - const destinationLesson = await this.getDestinationLesson(parentParams.lessonId, user); - const copyName = await this.getCopyName(originalTask.name, parentParams.courseId); + // i think getDestinationLesson can also to a promise.all on top + // then getCopyName can be put into if (destinationCourse) { + // but then the test need to cleanup + const [destinationLesson, copyName]: [LessonEntity | undefined, string | undefined] = await Promise.all([ + this.getDestinationLesson(parentParams.lessonId), + this.getCopyName(originalTask.name, parentParams.courseId), + ]); + + if (destinationLesson) { + this.checkDestinationLessonAuthorization(authorizableUser, destinationLesson); + } const status = await this.taskCopyService.copyTask({ originalTaskId: originalTask.id, destinationCourse, destinationLesson, - user, + user: authorizableUser, copyName, }); return status; } + private checkOriginalTaskAuthorization(authorizableUser: User, originalTask: Task): void { + const context = AuthorizationContextBuilder.read([]); + if (!this.authorisation.hasPermission(authorizableUser, originalTask, context)) { + // error message and erorr type are not correct + throw new NotFoundException('could not find task to copy'); + } + } + + private checkDestinationCourseAuthorisation(authorizableUser: User, destinationCourse: Course): void { + const context = AuthorizationContextBuilder.write([]); + this.authorisation.checkPermission(authorizableUser, destinationCourse, context); + } + + private checkDestinationLessonAuthorization(authorizableUser: User, destinationLesson: LessonEntity): void { + const context = AuthorizationContextBuilder.write([]); + if (!this.authorisation.hasPermission(authorizableUser, destinationLesson, context)) { + throw new ForbiddenException('you dont have permission to add to this lesson'); + } + } + private async getCopyName(originalTaskName: string, parentCourseId: EntityId | undefined) { let existingNames: string[] = []; if (parentCourseId) { + // It should really get an task where the creatorId === '' ? const [existingTasks] = await this.taskRepo.findBySingleParent('', parentCourseId); existingNames = existingTasks.map((t) => t.name); } + return this.copyHelperService.deriveCopyName(originalTaskName, existingNames); } @@ -77,19 +99,18 @@ export class TaskCopyUC { return destinationCourse; } - private async getDestinationLesson(lessonId: string | undefined, user: User): Promise { + private async getDestinationLesson(lessonId: string | undefined): Promise { if (lessonId === undefined) { return undefined; } const destinationLesson = await this.lessonRepo.findById(lessonId); - if (!this.authorisation.hasPermission(user, destinationLesson, AuthorizationContextBuilder.write([]))) { - throw new ForbiddenException('you dont have permission to add to this lesson'); - } + return destinationLesson; } - private featureEnabled() { + private checkFeatureEnabled() { + // This is the deprecated way to read envirement variables const enabled = Configuration.get('FEATURE_COPY_SERVICE_ENABLED') as boolean; if (!enabled) { throw new InternalServerErrorException('Copy Feature not enabled'); diff --git a/apps/server/src/modules/tool/common/common-tool.module.ts b/apps/server/src/modules/tool/common/common-tool.module.ts index cc7f5f86f00..0bd9ba5385b 100644 --- a/apps/server/src/modules/tool/common/common-tool.module.ts +++ b/apps/server/src/modules/tool/common/common-tool.module.ts @@ -3,11 +3,12 @@ import { ContextExternalToolRepo, SchoolExternalToolRepo } from '@shared/repo'; import { LoggerModule } from '@src/core/logger'; import { AuthorizationModule } from '@src/modules/authorization'; import { LegacySchoolModule } from '@src/modules/legacy-school'; +import { LearnroomModule } from '@src/modules/learnroom'; import { CommonToolService, CommonToolValidationService } from './service'; import { ToolPermissionHelper } from './uc/tool-permission-helper'; @Module({ - imports: [LoggerModule, forwardRef(() => AuthorizationModule), LegacySchoolModule], + imports: [LoggerModule, forwardRef(() => AuthorizationModule), LegacySchoolModule, LearnroomModule], // TODO: make deletion of entities cascading, adjust ExternalToolService.deleteExternalTool and remove the repos from here providers: [ CommonToolService, diff --git a/apps/server/src/modules/tool/common/mapper/context-type.mapper.spec.ts b/apps/server/src/modules/tool/common/mapper/context-type.mapper.spec.ts new file mode 100644 index 00000000000..78d79f45a1b --- /dev/null +++ b/apps/server/src/modules/tool/common/mapper/context-type.mapper.spec.ts @@ -0,0 +1,11 @@ +import { AuthorizableReferenceType } from '@src/modules/authorization/domain'; +import { ToolContextType } from '../enum'; +import { ContextTypeMapper } from './context-type.mapper'; + +describe('context-type.mapper', () => { + it('should map ToolContextType.COURSE to AuthorizableReferenceType.Course', () => { + const mappedCourse = ContextTypeMapper.mapContextTypeToAllowedAuthorizationEntityType(ToolContextType.COURSE); + + expect(mappedCourse).toEqual(AuthorizableReferenceType.Course); + }); +}); diff --git a/apps/server/src/modules/tool/common/mapper/context-type.mapper.ts b/apps/server/src/modules/tool/common/mapper/context-type.mapper.ts index bf8fb537924..883400f4258 100644 --- a/apps/server/src/modules/tool/common/mapper/context-type.mapper.ts +++ b/apps/server/src/modules/tool/common/mapper/context-type.mapper.ts @@ -1,4 +1,4 @@ -import { AuthorizableReferenceType } from '@src/modules/authorization/types'; +import { AuthorizableReferenceType } from '@src/modules/authorization/domain/'; import { ToolContextType } from '../enum'; const typeMapping: Record = { diff --git a/apps/server/src/modules/tool/common/uc/tool-permission-helper.ts b/apps/server/src/modules/tool/common/uc/tool-permission-helper.ts index dc4f339b4ab..7f0cb68ade8 100644 --- a/apps/server/src/modules/tool/common/uc/tool-permission-helper.ts +++ b/apps/server/src/modules/tool/common/uc/tool-permission-helper.ts @@ -1,16 +1,21 @@ import { forwardRef, Inject, Injectable } from '@nestjs/common'; -import { EntityId, LegacySchoolDo, User } from '@shared/domain'; -import { AuthorizableReferenceType, AuthorizationContext, AuthorizationService } from '@src/modules/authorization'; +import { Course, EntityId, LegacySchoolDo, User } from '@shared/domain'; +import { AuthorizationContext, AuthorizationService } from '@src/modules/authorization'; import { LegacySchoolService } from '@src/modules/legacy-school'; +import { CourseService } from '@src/modules/learnroom'; import { ContextExternalTool } from '../../context-external-tool/domain'; import { SchoolExternalTool } from '../../school-external-tool/domain'; -import { ContextTypeMapper } from '../mapper'; +// import { ContextTypeMapper } from '../mapper'; @Injectable() export class ToolPermissionHelper { constructor( - @Inject(forwardRef(() => AuthorizationService)) private authorizationService: AuthorizationService, - private readonly schoolService: LegacySchoolService + @Inject(forwardRef(() => AuthorizationService)) private readonly authorizationService: AuthorizationService, + private readonly schoolService: LegacySchoolService, + // invalid dependency on this place it is in UC layer in a other module + // loading of ressources should be part of service layer + // if it must resolve different loadings based on the request it can be added in own service and use in UC + private readonly courseService: CourseService ) {} // TODO build interface to get contextDO by contextType @@ -19,21 +24,19 @@ export class ToolPermissionHelper { contextExternalTool: ContextExternalTool, context: AuthorizationContext ): Promise { + // loading of ressources should be part of the UC -> unnessasary awaits + const [authorizableUser, course]: [User, Course] = await Promise.all([ + this.authorizationService.getUserWithPermissions(userId), + this.courseService.findById(contextExternalTool.contextRef.id), + ]); + if (contextExternalTool.id) { - await this.authorizationService.checkPermissionByReferences( - userId, - AuthorizableReferenceType.ContextExternalToolEntity, - contextExternalTool.id, - context - ); + this.authorizationService.checkPermission(authorizableUser, contextExternalTool, context); } - await this.authorizationService.checkPermissionByReferences( - userId, - ContextTypeMapper.mapContextTypeToAllowedAuthorizationEntityType(contextExternalTool.contextRef.type), - contextExternalTool.contextRef.id, - context - ); + // const type = ContextTypeMapper.mapContextTypeToAllowedAuthorizationEntityType(contextExternalTool.contextRef.type); + // no different types possible until it is fixed. + this.authorizationService.checkPermission(authorizableUser, course, context); } public async ensureSchoolPermissions( @@ -41,8 +44,12 @@ export class ToolPermissionHelper { schoolExternalTool: SchoolExternalTool, context: AuthorizationContext ): Promise { - const user: User = await this.authorizationService.getUserWithPermissions(userId); - const school: LegacySchoolDo = await this.schoolService.getSchoolById(schoolExternalTool.schoolId); + // loading of ressources should be part of the UC -> unnessasary awaits + const [user, school]: [User, LegacySchoolDo] = await Promise.all([ + this.authorizationService.getUserWithPermissions(userId), + this.schoolService.getSchoolById(schoolExternalTool.schoolId), + ]); + this.authorizationService.checkPermission(user, school, context); } } diff --git a/apps/server/src/modules/tool/common/uc/tool-permissions-helper.spec.ts b/apps/server/src/modules/tool/common/uc/tool-permissions-helper.spec.ts index b1567693130..0f2a1192d57 100644 --- a/apps/server/src/modules/tool/common/uc/tool-permissions-helper.spec.ts +++ b/apps/server/src/modules/tool/common/uc/tool-permissions-helper.spec.ts @@ -2,13 +2,17 @@ import { Test, TestingModule } from '@nestjs/testing'; import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { contextExternalToolFactory, + courseFactory, legacySchoolDoFactory, schoolExternalToolFactory, setupEntities, + userFactory, } from '@shared/testing'; import { Permission, LegacySchoolDo } from '@shared/domain'; import { AuthorizationContext, AuthorizationContextBuilder, AuthorizationService } from '@src/modules/authorization'; import { LegacySchoolService } from '@src/modules/legacy-school'; +import { ForbiddenException } from '@nestjs/common'; +import { CourseService } from '@src/modules/learnroom'; import { ContextExternalTool } from '../../context-external-tool/domain'; import { ToolPermissionHelper } from './tool-permission-helper'; import { SchoolExternalTool } from '../../school-external-tool/domain'; @@ -18,6 +22,7 @@ describe('ToolPermissionHelper', () => { let helper: ToolPermissionHelper; let authorizationService: DeepMocked; + let courseService: DeepMocked; let schoolService: DeepMocked; beforeAll(async () => { @@ -29,6 +34,10 @@ describe('ToolPermissionHelper', () => { provide: AuthorizationService, useValue: createMock(), }, + { + provide: CourseService, + useValue: createMock(), + }, { provide: LegacySchoolService, useValue: createMock(), @@ -38,6 +47,7 @@ describe('ToolPermissionHelper', () => { helper = module.get(ToolPermissionHelper); authorizationService = module.get(AuthorizationService); + courseService = module.get(CourseService); schoolService = module.get(LegacySchoolService); }); @@ -50,29 +60,98 @@ describe('ToolPermissionHelper', () => { }); describe('ensureContextPermissions', () => { - describe('when context external tool is given', () => { + describe('when context external tool with id is given', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const course = courseFactory.buildWithId(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId(); + const context: AuthorizationContext = AuthorizationContextBuilder.read([Permission.CONTEXT_TOOL_USER]); + + courseService.findById.mockResolvedValueOnce(course); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + authorizationService.checkPermission.mockReturnValueOnce().mockReturnValueOnce(); + + return { + user, + course, + contextExternalTool, + context, + }; + }; + + it('should check permission for context external tool', async () => { + const { user, course, contextExternalTool, context } = setup(); + + await helper.ensureContextPermissions(user.id, contextExternalTool, context); + + expect(authorizationService.checkPermission).toHaveBeenCalledTimes(2); + expect(authorizationService.checkPermission).toHaveBeenNthCalledWith(1, user, contextExternalTool, context); + expect(authorizationService.checkPermission).toHaveBeenNthCalledWith(2, user, course, context); + }); + + it('should return undefined', async () => { + const { user, contextExternalTool, context } = setup(); + + const result = await helper.ensureContextPermissions(user.id, contextExternalTool, context); + + expect(result).toBeUndefined(); + }); + }); + + describe('when context external tool without id is given', () => { const setup = () => { - const userId = 'userId'; + const user = userFactory.buildWithId(); + const course = courseFactory.buildWithId(); const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build(); const context: AuthorizationContext = AuthorizationContextBuilder.read([Permission.CONTEXT_TOOL_USER]); + courseService.findById.mockResolvedValueOnce(course); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + return { - userId, + user, + course, contextExternalTool, context, }; }; it('should check permission for context external tool', async () => { - const { userId, contextExternalTool, context } = setup(); + const { user, course, contextExternalTool, context } = setup(); + + await helper.ensureContextPermissions(user.id, contextExternalTool, context); - await helper.ensureContextPermissions(userId, contextExternalTool, context); + expect(authorizationService.checkPermission).toHaveBeenCalledTimes(1); + expect(authorizationService.checkPermission).toHaveBeenCalledWith(user, course, context); + }); + }); - expect(authorizationService.checkPermissionByReferences).toHaveBeenCalledWith( - userId, - 'courses', - contextExternalTool.contextRef.id, - context + describe('when user is unauthorized', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const course = courseFactory.buildWithId(); + const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId(); + const context: AuthorizationContext = AuthorizationContextBuilder.read([Permission.CONTEXT_TOOL_USER]); + + courseService.findById.mockResolvedValueOnce(course); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + authorizationService.checkPermission.mockImplementationOnce(() => { + throw new ForbiddenException(); + }); + + return { + user, + course, + contextExternalTool, + context, + }; + }; + + it('should check permission for context external tool', async () => { + const { user, contextExternalTool, context } = setup(); + + await expect(helper.ensureContextPermissions(user.id, contextExternalTool, context)).rejects.toThrowError( + new ForbiddenException() ); }); }); @@ -81,15 +160,16 @@ describe('ToolPermissionHelper', () => { describe('ensureSchoolPermissions', () => { describe('when school external tool is given', () => { const setup = () => { - const userId = 'userId'; + const user = userFactory.buildWithId(); const schoolExternalTool: SchoolExternalTool = schoolExternalToolFactory.buildWithId(); const context: AuthorizationContext = AuthorizationContextBuilder.read([Permission.SCHOOL_TOOL_ADMIN]); const school: LegacySchoolDo = legacySchoolDoFactory.build({ id: schoolExternalTool.schoolId }); schoolService.getSchoolById.mockResolvedValue(school); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); return { - userId, + user, schoolExternalTool, school, context, @@ -97,11 +177,20 @@ describe('ToolPermissionHelper', () => { }; it('should check permission for school external tool', async () => { - const { userId, schoolExternalTool, context, school } = setup(); + const { user, schoolExternalTool, context, school } = setup(); + + await helper.ensureSchoolPermissions(user.id, schoolExternalTool, context); + + expect(authorizationService.checkPermission).toHaveBeenCalledTimes(1); + expect(authorizationService.checkPermission).toHaveBeenCalledWith(user, school, context); + }); + + it('should return undefined', async () => { + const { user, schoolExternalTool, context } = setup(); - await helper.ensureSchoolPermissions(userId, schoolExternalTool, context); + const result = await helper.ensureSchoolPermissions(user.id, schoolExternalTool, context); - expect(authorizationService.checkPermission).toHaveBeenCalledWith(userId, school, context); + expect(result).toBeUndefined(); }); }); }); diff --git a/apps/server/src/modules/tool/context-external-tool/controller/api-test/tool-context.api.spec.ts b/apps/server/src/modules/tool/context-external-tool/controller/api-test/tool-context.api.spec.ts index 6e8ca18253f..b5584426763 100644 --- a/apps/server/src/modules/tool/context-external-tool/controller/api-test/tool-context.api.spec.ts +++ b/apps/server/src/modules/tool/context-external-tool/controller/api-test/tool-context.api.spec.ts @@ -114,29 +114,29 @@ describe('ToolContextController (API)', () => { }); }); - describe('when creation of contextExternalTool failed', () => { + describe('when user is not authorized for the requested context', () => { const setup = async () => { const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); - - const course: Course = courseFactory.buildWithId({ teachers: [teacherUser] }); - - const schoolExternalToolEntity: SchoolExternalToolEntity = schoolExternalToolEntityFactory.buildWithId({ + const school = schoolFactory.build(); + const course = courseFactory.build({ teachers: [teacherUser] }); + const otherCourse = courseFactory.build(); + const schoolExternalToolEntity: SchoolExternalToolEntity = schoolExternalToolEntityFactory.build({ schoolParameters: [], toolVersion: 1, + school, }); - const randomTestId = new ObjectId().toString(); + await em.persistAndFlush([course, otherCourse, school, teacherUser, teacherAccount, schoolExternalToolEntity]); + em.clear(); + const postParams: ContextExternalToolPostParams = { - schoolToolId: randomTestId, - contextId: randomTestId, + schoolToolId: school.id, + contextId: otherCourse.id, contextType: ToolContextType.COURSE, parameters: [], toolVersion: 1, }; - await em.persistAndFlush([course, teacherUser, teacherAccount, schoolExternalToolEntity]); - em.clear(); - const loggedInClient: TestApiClient = await testApiClient.login(teacherAccount); return { @@ -145,12 +145,13 @@ describe('ToolContextController (API)', () => { }; }; - it('when user is not authorized, it should return forbidden', async () => { + it('it should return forbidden', async () => { const { postParams, loggedInClient } = await setup(); const response = await loggedInClient.post().send(postParams); expect(response.statusCode).toEqual(HttpStatus.FORBIDDEN); + // expected body is missed }); }); }); @@ -204,23 +205,26 @@ describe('ToolContextController (API)', () => { describe('when deletion of contextExternalTool failed', () => { const setup = async () => { const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher(); - - const course: Course = courseFactory.buildWithId({ teachers: [teacherUser] }); - - const schoolExternalToolEntity: SchoolExternalToolEntity = schoolExternalToolEntityFactory.buildWithId({ + const course = courseFactory.buildWithId({ teachers: [teacherUser] }); + const schoolExternalToolEntity = schoolExternalToolEntityFactory.buildWithId({ toolVersion: 1, }); - - const contextExternalToolEntity: ContextExternalToolEntity = contextExternalToolEntityFactory.buildWithId({ + const contextExternalToolEntity = contextExternalToolEntityFactory.buildWithId({ schoolTool: schoolExternalToolEntity, + contextId: course.id, toolVersion: 1, }); - em.persist([course, teacherUser, teacherAccount, schoolExternalToolEntity, contextExternalToolEntity]); - await em.flush(); + await em.persistAndFlush([ + course, + teacherUser, + teacherAccount, + schoolExternalToolEntity, + contextExternalToolEntity, + ]); em.clear(); - const loggedInClient: TestApiClient = await testApiClient.login(teacherAccount); + const loggedInClient = await testApiClient.login(teacherAccount); return { contextExternalToolEntity, @@ -234,6 +238,7 @@ describe('ToolContextController (API)', () => { const result = await loggedInClient.delete(`${contextExternalToolEntity.id}`); expect(result.statusCode).toEqual(HttpStatus.FORBIDDEN); + // result.body is missed }); }); }); @@ -543,37 +548,29 @@ describe('ToolContextController (API)', () => { describe('when user has not the required permission', () => { const setup = async () => { - const school: SchoolEntity = schoolFactory.buildWithId(); - + const school = schoolFactory.build(); const { studentUser, studentAccount } = UserAndAccountTestFactory.buildStudent({ school }); - - const course: Course = courseFactory.buildWithId({ + const course = courseFactory.build({ teachers: [studentUser], school, }); - - const externalTool: ExternalToolEntity = externalToolEntityFactory.buildWithId(); - const schoolExternalTool: SchoolExternalToolEntity = schoolExternalToolEntityFactory.buildWithId({ + const externalTool: ExternalToolEntity = externalToolEntityFactory.build(); + const schoolExternalTool: SchoolExternalToolEntity = schoolExternalToolEntityFactory.build({ school, tool: externalTool, toolVersion: 1, }); - const contextExternalTool: ContextExternalToolEntity = contextExternalToolEntityFactory.buildWithId({ + + await em.persistAndFlush([school, course, externalTool, schoolExternalTool, studentAccount, studentUser]); + + const contextExternalTool: ContextExternalToolEntity = contextExternalToolEntityFactory.build({ contextId: course.id, schoolTool: schoolExternalTool, toolVersion: 1, contextType: ContextExternalToolType.COURSE, }); - await em.persistAndFlush([ - school, - course, - externalTool, - schoolExternalTool, - contextExternalTool, - studentAccount, - studentUser, - ]); + await em.persistAndFlush([contextExternalTool]); em.clear(); const loggedInClient: TestApiClient = await testApiClient.login(studentAccount); @@ -591,6 +588,7 @@ describe('ToolContextController (API)', () => { const response = await loggedInClient.get(`${contextExternalTool.id}`); expect(response.status).toEqual(HttpStatus.FORBIDDEN); + // body check }); }); }); @@ -688,34 +686,37 @@ describe('ToolContextController (API)', () => { describe('when the user is not authorized', () => { const setup = async () => { - const roleWithoutPermission = roleFactory.buildWithId(); - const { teacherUser, teacherAccount } = UserAndAccountTestFactory.buildTeacher(); - + const roleWithoutPermission = roleFactory.build(); teacherUser.roles.set([roleWithoutPermission]); - const school: SchoolEntity = schoolFactory.buildWithId(); - - const course: Course = courseFactory.buildWithId({ teachers: [teacherUser], school }); - + const school = schoolFactory.build(); + const course = courseFactory.build({ teachers: [teacherUser], school }); const contextParameter = customParameterEntityFactory.build({ scope: CustomParameterScope.CONTEXT, regex: 'testValue123', }); - - const externalToolEntity: ExternalToolEntity = externalToolEntityFactory.buildWithId({ + const externalToolEntity: ExternalToolEntity = externalToolEntityFactory.build({ parameters: [contextParameter], version: 2, }); - - const schoolExternalToolEntity: SchoolExternalToolEntity = schoolExternalToolEntityFactory.buildWithId({ + const schoolExternalToolEntity: SchoolExternalToolEntity = schoolExternalToolEntityFactory.build({ tool: externalToolEntity, school, schoolParameters: [], toolVersion: 2, }); - const contextExternalToolEntity: ContextExternalToolEntity = contextExternalToolEntityFactory.buildWithId({ + await em.persistAndFlush([ + course, + school, + teacherUser, + teacherAccount, + externalToolEntity, + schoolExternalToolEntity, + ]); + + const contextExternalToolEntity: ContextExternalToolEntity = contextExternalToolEntityFactory.build({ schoolTool: schoolExternalToolEntity, contextId: course.id, contextType: ContextExternalToolType.COURSE, @@ -724,6 +725,10 @@ describe('ToolContextController (API)', () => { toolVersion: 1, }); + await em.persistAndFlush([contextExternalToolEntity]); + + em.clear(); + const postParams: ContextExternalToolPostParams = { schoolToolId: schoolExternalToolEntity.id, contextId: course.id, @@ -738,17 +743,6 @@ describe('ToolContextController (API)', () => { toolVersion: 2, }; - await em.persistAndFlush([ - course, - school, - teacherUser, - teacherAccount, - externalToolEntity, - schoolExternalToolEntity, - contextExternalToolEntity, - ]); - em.clear(); - const loggedInClient: TestApiClient = await testApiClient.login(teacherAccount); return { loggedInClient, postParams, contextExternalToolEntity }; @@ -760,6 +754,7 @@ describe('ToolContextController (API)', () => { const response = await loggedInClient.put(`${contextExternalToolEntity.id}`).send(postParams); expect(response.status).toEqual(HttpStatus.FORBIDDEN); + // body missed }); }); diff --git a/apps/server/src/modules/tool/context-external-tool/controller/tool-context.controller.ts b/apps/server/src/modules/tool/context-external-tool/controller/tool-context.controller.ts index 491e7d9f2d6..3e7d8199fa2 100644 --- a/apps/server/src/modules/tool/context-external-tool/controller/tool-context.controller.ts +++ b/apps/server/src/modules/tool/context-external-tool/controller/tool-context.controller.ts @@ -58,6 +58,7 @@ export class ToolContextController { ContextExternalToolResponseMapper.mapContextExternalToolResponse(createdTool); this.logger.debug(`ContextExternalTool with id ${response.id} was created by user with id ${currentUser.userId}`); + return response; } diff --git a/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.spec.ts b/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.spec.ts index 801765f80e5..8be134b80f2 100644 --- a/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.spec.ts +++ b/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.spec.ts @@ -5,8 +5,12 @@ import { Test, TestingModule } from '@nestjs/testing'; import { EntityId, Permission, User } from '@shared/domain'; import { contextExternalToolFactory, setupEntities, userFactory } from '@shared/testing'; import { LegacyLogger } from '@src/core/logger'; -import { Action, AuthorizationContextBuilder, AuthorizationService } from '@src/modules/authorization'; -import { ForbiddenLoggableException } from '@src/modules/authorization/errors/forbidden.loggable-exception'; +import { + Action, + AuthorizationContextBuilder, + AuthorizationService, + ForbiddenLoggableException, +} from '@src/modules/authorization'; import { ToolContextType } from '../../common/enum'; import { ToolPermissionHelper } from '../../common/uc/tool-permission-helper'; import { ContextExternalTool } from '../domain'; diff --git a/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.ts b/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.ts index 04002cf9fc6..9b6e3e3fe66 100644 --- a/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.ts +++ b/apps/server/src/modules/tool/context-external-tool/uc/context-external-tool.uc.ts @@ -61,18 +61,20 @@ export class ContextExternalToolUc { return saved; } - async deleteContextExternalTool(userId: EntityId, contextExternalToolId: EntityId): Promise { + public async deleteContextExternalTool(userId: EntityId, contextExternalToolId: EntityId): Promise { const tool: ContextExternalTool = await this.contextExternalToolService.findById(contextExternalToolId); - const context: AuthorizationContext = AuthorizationContextBuilder.write([Permission.CONTEXT_TOOL_ADMIN]); + const context = AuthorizationContextBuilder.write([Permission.CONTEXT_TOOL_ADMIN]); await this.toolPermissionHelper.ensureContextPermissions(userId, tool, context); - const promise: Promise = this.contextExternalToolService.deleteContextExternalTool(tool); - - return promise; + await this.contextExternalToolService.deleteContextExternalTool(tool); } - async getContextExternalToolsForContext(userId: EntityId, contextType: ToolContextType, contextId: string) { + public async getContextExternalToolsForContext( + userId: EntityId, + contextType: ToolContextType, + contextId: string + ): Promise { const tools: ContextExternalTool[] = await this.contextExternalToolService.findAllByContext( new ContextRef({ id: contextId, type: contextType }) ); diff --git a/apps/server/src/modules/tool/external-tool/controller/api-test/tool-configuration.api.spec.ts b/apps/server/src/modules/tool/external-tool/controller/api-test/tool-configuration.api.spec.ts index 70ec7d4be67..d0030bf7b46 100644 --- a/apps/server/src/modules/tool/external-tool/controller/api-test/tool-configuration.api.spec.ts +++ b/apps/server/src/modules/tool/external-tool/controller/api-test/tool-configuration.api.spec.ts @@ -33,7 +33,6 @@ describe('ToolConfigurationController (API)', () => { let app: INestApplication; let em: EntityManager; let orm: MikroORM; - let testApiClient: TestApiClient; beforeAll(async () => { @@ -346,22 +345,19 @@ describe('ToolConfigurationController (API)', () => { describe('GET tools/school-external-tools/:schoolExternalToolId/configuration-template', () => { describe('when the user is not authorized', () => { const setup = async () => { - const school: SchoolEntity = schoolFactory.buildWithId(); - - const user: User = userFactory.buildWithId({ school, roles: [] }); - const account: Account = accountFactory.buildWithId({ userId: user.id }); - - const externalTool: ExternalToolEntity = externalToolEntityFactory.buildWithId(); - - const schoolExternalTool: SchoolExternalToolEntity = schoolExternalToolEntityFactory.buildWithId({ + const school = schoolFactory.build(); + // not on same school like the tool + const { adminAccount, adminUser } = UserAndAccountTestFactory.buildAdmin({}, []); + const externalTool: ExternalToolEntity = externalToolEntityFactory.build(); + const schoolExternalTool: SchoolExternalToolEntity = schoolExternalToolEntityFactory.build({ school, tool: externalTool, }); - await em.persistAndFlush([user, account, school, externalTool, schoolExternalTool]); + await em.persistAndFlush([adminAccount, adminUser, school, externalTool, schoolExternalTool]); em.clear(); - const loggedInClient: TestApiClient = await testApiClient.login(account); + const loggedInClient: TestApiClient = await testApiClient.login(adminAccount); return { loggedInClient, @@ -477,51 +473,43 @@ describe('ToolConfigurationController (API)', () => { describe('GET tools/context-external-tools/:contextExternalToolId/configuration-template', () => { describe('when the user is not authorized', () => { const setup = async () => { - const school: SchoolEntity = schoolFactory.buildWithId(); - - const course: Course = courseFactory.buildWithId(); - - const user: User = userFactory.buildWithId({ school, roles: [] }); - const account: Account = accountFactory.buildWithId({ userId: user.id }); - - const externalTool: ExternalToolEntity = externalToolEntityFactory.buildWithId(); - - const schoolExternalTool: SchoolExternalToolEntity = schoolExternalToolEntityFactory.buildWithId({ + const school = schoolFactory.build(); + const { adminUser, adminAccount } = UserAndAccountTestFactory.buildAdmin({}, [Permission.SCHOOL_TOOL_ADMIN]); + // user is not part of the course + const course = courseFactory.build(); + const externalTool: ExternalToolEntity = externalToolEntityFactory.build(); + const schoolExternalTool: SchoolExternalToolEntity = schoolExternalToolEntityFactory.build({ school, tool: externalTool, }); - const contextExternalTool: ContextExternalToolEntity = contextExternalToolEntityFactory.buildWithId({ + await em.persistAndFlush([course, adminUser, adminAccount, school, externalTool, schoolExternalTool]); + + const contextExternalTool: ContextExternalToolEntity = contextExternalToolEntityFactory.build({ schoolTool: schoolExternalTool, + contextId: course.id, }); - await em.persistAndFlush([ - user, - account, - school, - externalTool, - schoolExternalTool, - contextExternalTool, - course, - ]); + await em.persistAndFlush([contextExternalTool]); em.clear(); - const loggedInClient: TestApiClient = await testApiClient.login(account); + const loggedInClient: TestApiClient = await testApiClient.login(adminAccount); return { loggedInClient, - contextExternalTool, + contextExternalToolId: contextExternalTool.id, }; }; it('should return a forbidden status', async () => { - const { loggedInClient, contextExternalTool } = await setup(); + const { loggedInClient, contextExternalToolId } = await setup(); const response: Response = await loggedInClient.get( - `context-external-tools/${contextExternalTool.id}/configuration-template` + `context-external-tools/${contextExternalToolId}/configuration-template` ); expect(response.status).toEqual(HttpStatus.FORBIDDEN); + // body }); }); @@ -607,36 +595,26 @@ describe('ToolConfigurationController (API)', () => { describe('when tool is hidden', () => { const setup = async () => { - const school: SchoolEntity = schoolFactory.buildWithId(); - + const school = schoolFactory.build(); const { teacherAccount, teacherUser } = UserAndAccountTestFactory.buildTeacher({ school }, [ Permission.CONTEXT_TOOL_ADMIN, ]); - - const course: Course = courseFactory.buildWithId({ school, teachers: [teacherUser] }); - - const externalTool: ExternalToolEntity = externalToolEntityFactory.buildWithId({ isHidden: true }); - - const schoolExternalTool: SchoolExternalToolEntity = schoolExternalToolEntityFactory.buildWithId({ + const course = courseFactory.build({ school, teachers: [teacherUser] }); + const externalTool: ExternalToolEntity = externalToolEntityFactory.build({ isHidden: true }); + const schoolExternalTool: SchoolExternalToolEntity = schoolExternalToolEntityFactory.build({ school, tool: externalTool, }); - const contextExternalTool: ContextExternalToolEntity = contextExternalToolEntityFactory.buildWithId({ + await em.persistAndFlush([teacherUser, school, teacherAccount, externalTool, schoolExternalTool, course]); + + const contextExternalTool: ContextExternalToolEntity = contextExternalToolEntityFactory.build({ schoolTool: schoolExternalTool, contextType: ContextExternalToolType.COURSE, contextId: course.id, }); - await em.persistAndFlush([ - teacherUser, - school, - teacherAccount, - externalTool, - schoolExternalTool, - contextExternalTool, - course, - ]); + await em.persistAndFlush([contextExternalTool]); em.clear(); const loggedInClient: TestApiClient = await testApiClient.login(teacherAccount); diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool-configuration.uc.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool-configuration.uc.ts index 16dd9626d0c..1e91214ccda 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool-configuration.uc.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool-configuration.uc.ts @@ -140,8 +140,8 @@ export class ExternalToolConfigurationUc { const contextExternalTool: ContextExternalTool = await this.contextExternalToolService.findById( contextExternalToolId ); - const context: AuthorizationContext = AuthorizationContextBuilder.read([Permission.CONTEXT_TOOL_ADMIN]); + const context: AuthorizationContext = AuthorizationContextBuilder.read([Permission.CONTEXT_TOOL_ADMIN]); await this.toolPermissionHelper.ensureContextPermissions(userId, contextExternalTool, context); const schoolExternalTool: SchoolExternalTool = await this.schoolExternalToolService.findById( diff --git a/apps/server/src/modules/tool/tool-launch/tool-launch.module.ts b/apps/server/src/modules/tool/tool-launch/tool-launch.module.ts index 6799a50bca2..95d8a42ce1f 100644 --- a/apps/server/src/modules/tool/tool-launch/tool-launch.module.ts +++ b/apps/server/src/modules/tool/tool-launch/tool-launch.module.ts @@ -1,4 +1,4 @@ -import { Module } from '@nestjs/common'; +import { Module, forwardRef } from '@nestjs/common'; import { LearnroomModule } from '@src/modules/learnroom'; import { LegacySchoolModule } from '@src/modules/legacy-school'; import { PseudonymModule } from '@src/modules/pseudonym'; @@ -18,7 +18,7 @@ import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrat ContextExternalToolModule, LegacySchoolModule, UserModule, - PseudonymModule, + forwardRef(() => PseudonymModule), // i do not like this solution, the root problem is on other place but not detectable for me LearnroomModule, ], providers: [ diff --git a/apps/server/src/modules/video-conference/mapper/video-conference.mapper.ts b/apps/server/src/modules/video-conference/mapper/video-conference.mapper.ts index 727cfdfce7d..1f51a8cb3ff 100644 --- a/apps/server/src/modules/video-conference/mapper/video-conference.mapper.ts +++ b/apps/server/src/modules/video-conference/mapper/video-conference.mapper.ts @@ -1,5 +1,4 @@ -import { Permission, VideoConferenceScope } from '@shared/domain'; -import { AuthorizableReferenceType } from '@src/modules/authorization'; +import { Permission } from '@shared/domain'; import { BBBRole } from '../bbb'; import { VideoConferenceCreateParams, @@ -16,11 +15,6 @@ export const PermissionMapping = { [BBBRole.VIEWER]: Permission.JOIN_MEETING, }; -export const PermissionScopeMapping = { - [VideoConferenceScope.COURSE]: AuthorizableReferenceType.Course, - [VideoConferenceScope.EVENT]: AuthorizableReferenceType.Team, -}; - const stateMapping = { [VideoConferenceState.NOT_STARTED]: VideoConferenceStateResponse.NOT_STARTED, [VideoConferenceState.RUNNING]: VideoConferenceStateResponse.RUNNING, diff --git a/apps/server/src/modules/video-conference/service/video-conference.service.spec.ts b/apps/server/src/modules/video-conference/service/video-conference.service.spec.ts index b0c28b6a3c9..280a11976d7 100644 --- a/apps/server/src/modules/video-conference/service/video-conference.service.spec.ts +++ b/apps/server/src/modules/video-conference/service/video-conference.service.spec.ts @@ -14,14 +14,10 @@ import { } from '@shared/domain'; import { CalendarEventDto, CalendarService } from '@shared/infra/calendar'; import { TeamsRepo, VideoConferenceRepo } from '@shared/repo'; -import { - AuthorizableReferenceType, - AuthorizationContextBuilder, - AuthorizationService, -} from '@src/modules/authorization'; +import { AuthorizationContextBuilder, AuthorizationService } from '@src/modules/authorization'; import { LegacySchoolService } from '@src/modules/legacy-school'; import { UserService } from '@src/modules/user'; -import { courseFactory, roleFactory, setupEntities, userDoFactory } from '@shared/testing'; +import { courseFactory, roleFactory, setupEntities, userDoFactory, userFactory } from '@shared/testing'; import { videoConferenceDOFactory } from '@shared/testing/factory/video-conference.do.factory'; import { ObjectId } from 'bson'; import { teamFactory } from '@shared/testing/factory/team.factory'; @@ -332,64 +328,160 @@ describe('VideoConferenceService', () => { }); describe('checkPermission', () => { - const setup = () => { - const userId = 'user-id'; - const conferenceScope = VideoConferenceScope.COURSE; - const entityId = 'entity-id'; + describe('when user has START_MEETING permission and is in course scope', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const entity = courseFactory.buildWithId(); + const conferenceScope = VideoConferenceScope.COURSE; - return { - userId, - conferenceScope, - entityId, + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + authorizationService.hasPermission.mockReturnValueOnce(true).mockReturnValueOnce(false); + courseService.findById.mockResolvedValueOnce(entity); + + return { + user, + userId: user.id, + entity, + entityId: entity.id, + conferenceScope, + }; }; - }; - describe('when user has START_MEETING permission', () => { + it('should call the correct authorization order', async () => { + const { user, entity, userId, conferenceScope, entityId } = setup(); + + await service.determineBbbRole(userId, entityId, conferenceScope); + + expect(authorizationService.hasPermission).toHaveBeenCalledWith( + user, + entity, + AuthorizationContextBuilder.read([Permission.START_MEETING]) + ); + }); + it('should return BBBRole.MODERATOR', async () => { const { userId, conferenceScope, entityId } = setup(); - authorizationService.hasPermissionByReferences.mockResolvedValueOnce(true); - authorizationService.hasPermissionByReferences.mockResolvedValueOnce(false); - - const result: BBBRole = await service.determineBbbRole(userId, entityId, conferenceScope); + const result = await service.determineBbbRole(userId, entityId, conferenceScope); expect(result).toBe(BBBRole.MODERATOR); - expect(authorizationService.hasPermissionByReferences).toHaveBeenCalledWith( - userId, - AuthorizableReferenceType.Course, - entityId, + }); + }); + + // can be removed when team / course / user is passed from UC + // missing when course / team loading throw an error, but also not nessasary if it is passed to UC. + describe('when user has START_MEETING permission and is in team(event) scope', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const entity = teamFactory.buildWithId(); + const conferenceScope = VideoConferenceScope.EVENT; + + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + authorizationService.hasPermission.mockReturnValueOnce(true).mockReturnValueOnce(false); + teamsRepo.findById.mockResolvedValueOnce(entity); + + return { + user, + userId: user.id, + entity, + entityId: entity.id, + conferenceScope, + }; + }; + + it('should call the correct authorization order', async () => { + const { user, entity, userId, conferenceScope, entityId } = setup(); + + await service.determineBbbRole(userId, entityId, conferenceScope); + + expect(authorizationService.hasPermission).toHaveBeenCalledWith( + user, + entity, AuthorizationContextBuilder.read([Permission.START_MEETING]) ); }); + + it('should return BBBRole.MODERATOR', async () => { + const { userId, conferenceScope, entityId } = setup(); + + const result = await service.determineBbbRole(userId, entityId, conferenceScope); + + expect(result).toBe(BBBRole.MODERATOR); + }); }); describe('when user has JOIN_MEETING permission', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const entity = courseFactory.buildWithId(); + const conferenceScope = VideoConferenceScope.COURSE; + + authorizationService.hasPermission.mockReturnValueOnce(false).mockReturnValueOnce(true); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + courseService.findById.mockResolvedValueOnce(entity); + + return { + user, + userId: user.id, + entity, + entityId: entity.id, + conferenceScope, + }; + }; + + it('should call the correct authorization order', async () => { + const { user, entity, userId, conferenceScope, entityId } = setup(); + + await service.determineBbbRole(userId, entityId, conferenceScope); + + expect(authorizationService.hasPermission).toHaveBeenNthCalledWith( + 1, + user, + entity, + AuthorizationContextBuilder.read([Permission.START_MEETING]) + ); + expect(authorizationService.hasPermission).toHaveBeenNthCalledWith( + 2, + user, + entity, + AuthorizationContextBuilder.read([Permission.JOIN_MEETING]) + ); + }); + it('should return BBBRole.VIEWER', async () => { const { userId, conferenceScope, entityId } = setup(); - authorizationService.hasPermissionByReferences.mockResolvedValueOnce(false); - authorizationService.hasPermissionByReferences.mockResolvedValueOnce(true); - const result: BBBRole = await service.determineBbbRole(userId, entityId, conferenceScope); + const result = await service.determineBbbRole(userId, entityId, conferenceScope); expect(result).toBe(BBBRole.VIEWER); - expect(authorizationService.hasPermissionByReferences).toHaveBeenCalledWith( - userId, - AuthorizableReferenceType.Course, - entityId, - AuthorizationContextBuilder.read([Permission.JOIN_MEETING]) - ); }); }); describe('when user has neither START_MEETING nor JOIN_MEETING permission', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const entity = courseFactory.buildWithId(); + const conferenceScope = VideoConferenceScope.COURSE; + + authorizationService.hasPermission.mockReturnValueOnce(false).mockReturnValueOnce(false); + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + courseService.findById.mockResolvedValueOnce(entity); + + return { + user, + userId: user.id, + entity, + entityId: entity.id, + conferenceScope, + }; + }; + it('should throw a ForbiddenException', async () => { const { userId, conferenceScope, entityId } = setup(); - authorizationService.hasPermissionByReferences.mockResolvedValueOnce(false); - authorizationService.hasPermissionByReferences.mockResolvedValueOnce(false); - const func = () => service.determineBbbRole(userId, entityId, conferenceScope); + const callDetermineBbbRole = () => service.determineBbbRole(userId, entityId, conferenceScope); - await expect(func).rejects.toThrow(new ForbiddenException(ErrorStatus.INSUFFICIENT_PERMISSION)); + await expect(callDetermineBbbRole).rejects.toThrow(new ForbiddenException(ErrorStatus.INSUFFICIENT_PERMISSION)); }); }); }); diff --git a/apps/server/src/modules/video-conference/service/video-conference.service.ts b/apps/server/src/modules/video-conference/service/video-conference.service.ts index 0201b150f33..69a1a7fd74f 100644 --- a/apps/server/src/modules/video-conference/service/video-conference.service.ts +++ b/apps/server/src/modules/video-conference/service/video-conference.service.ts @@ -8,6 +8,7 @@ import { SchoolFeatures, TeamEntity, TeamUserEntity, + User, UserDO, VideoConferenceDO, VideoConferenceOptionsDO, @@ -15,19 +16,13 @@ import { } from '@shared/domain'; import { CalendarEventDto, CalendarService } from '@shared/infra/calendar'; import { TeamsRepo, VideoConferenceRepo } from '@shared/repo'; -import { - Action, - AuthorizableReferenceType, - AuthorizationContextBuilder, - AuthorizationService, -} from '@src/modules/authorization'; -import { CourseService } from '@src/modules/learnroom/service'; +import { AuthorizationContextBuilder, AuthorizationService } from '@src/modules/authorization'; +import { CourseService } from '@src/modules/learnroom'; import { LegacySchoolService } from '@src/modules/legacy-school'; import { UserService } from '@src/modules/user'; import { BBBRole } from '../bbb'; import { ErrorStatus } from '../error'; import { IVideoConferenceSettings, VideoConferenceOptions, VideoConferenceSettings } from '../interface'; -import { PermissionScopeMapping } from '../mapper/video-conference.mapper'; import { IScopeInfo, VideoConferenceState } from '../uc/dto'; @Injectable() @@ -97,39 +92,59 @@ export class VideoConferenceService { return isExpert; } - async determineBbbRole(userId: EntityId, scopeId: EntityId, scope: VideoConferenceScope): Promise { - const permissionMap: Map> = this.hasPermissions( - userId, - PermissionScopeMapping[scope], - scopeId, - [Permission.START_MEETING, Permission.JOIN_MEETING], - Action.read - ); - - if (await permissionMap.get(Permission.START_MEETING)) { - return BBBRole.MODERATOR; - } - if (await permissionMap.get(Permission.JOIN_MEETING)) { - return BBBRole.VIEWER; + // should be public to expose ressources to UC for passing it to authrisation and improve performance + private async loadScopeRessources( + scopeId: EntityId, + scope: VideoConferenceScope + ): Promise { + let scopeRessource: Course | TeamEntity | null = null; + + if (scope === VideoConferenceScope.COURSE) { + scopeRessource = await this.courseService.findById(scopeId); + } else if (scope === VideoConferenceScope.EVENT) { + scopeRessource = await this.teamsRepo.findById(scopeId); + } else { + // Need to be solve the null with throw by it self. } - throw new ForbiddenException(ErrorStatus.INSUFFICIENT_PERMISSION); + + return scopeRessource; + } + + private isNullOrUndefined(value: unknown): value is null { + return !value; } - private hasPermissions( - userId: EntityId, - entityName: AuthorizableReferenceType, - entityId: EntityId, - permissions: Permission[], - action: Action - ): Map> { - const returnMap: Map> = new Map(); - permissions.forEach((perm) => { - const context = - action === Action.read ? AuthorizationContextBuilder.read([perm]) : AuthorizationContextBuilder.write([perm]); - const ret = this.authorizationService.hasPermissionByReferences(userId, entityName, entityId, context); - returnMap.set(perm, ret); - }); - return returnMap; + private hasStartMeetingAndCanRead(authorizableUser: User, entity: Course | TeamEntity): boolean { + const context = AuthorizationContextBuilder.read([Permission.START_MEETING]); + const hasPermission = this.authorizationService.hasPermission(authorizableUser, entity, context); + + return hasPermission; + } + + private hasJoinMeetingAndCanRead(authorizableUser: User, entity: Course | TeamEntity): boolean { + const context = AuthorizationContextBuilder.read([Permission.JOIN_MEETING]); + const hasPermission = this.authorizationService.hasPermission(authorizableUser, entity, context); + + return hasPermission; + } + + async determineBbbRole(userId: EntityId, scopeId: EntityId, scope: VideoConferenceScope): Promise { + // ressource loading need to be move to uc + const [authorizableUser, scopeRessource]: [User, TeamEntity | Course | null] = await Promise.all([ + this.authorizationService.getUserWithPermissions(userId), + this.loadScopeRessources(scopeId, scope), + ]); + + if (!this.isNullOrUndefined(scopeRessource)) { + if (this.hasStartMeetingAndCanRead(authorizableUser, scopeRessource)) { + return BBBRole.MODERATOR; + } + if (this.hasJoinMeetingAndCanRead(authorizableUser, scopeRessource)) { + return BBBRole.VIEWER; + } + } + + throw new ForbiddenException(ErrorStatus.INSUFFICIENT_PERMISSION); } async throwOnFeaturesDisabled(schoolId: EntityId): Promise { diff --git a/apps/server/src/modules/video-conference/uc/video-conference-create.uc.ts b/apps/server/src/modules/video-conference/uc/video-conference-create.uc.ts index e7a28c9a7ca..853bf0f13bc 100644 --- a/apps/server/src/modules/video-conference/uc/video-conference-create.uc.ts +++ b/apps/server/src/modules/video-conference/uc/video-conference-create.uc.ts @@ -38,6 +38,12 @@ export class VideoConferenceCreateUc { } private async create(currentUserId: EntityId, scope: ScopeRef, options: VideoConferenceOptions): Promise { + /* need to be replace with + const [authorizableUser, scopeRessource]: [User, TeamEntity | Course] = await Promise.all([ + this.authorizationService.getUserWithPermissions(userId), + this.videoConferenceService.loadScopeRessources(scopeId, scope), + ]); + */ const user: UserDO = await this.userService.findById(currentUserId); await this.verifyFeaturesEnabled(user.schoolId); diff --git a/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.spec.ts b/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.spec.ts index 3680c4519da..4ff494a4cc0 100644 --- a/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.spec.ts +++ b/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.spec.ts @@ -19,9 +19,10 @@ import { CalendarEventDto } from '@shared/infra/calendar/dto/calendar-event.dto' import { TeamsRepo, VideoConferenceRepo } from '@shared/repo'; import { roleFactory, setupEntities, userDoFactory } from '@shared/testing'; import { teamFactory } from '@shared/testing/factory/team.factory'; -import { AuthorizationService, LegacySchoolService, UserService } from '@src/modules'; +import { LegacySchoolService, UserService } from '@src/modules'; +import { AuthorizationReferenceService } from '@src/modules/authorization/domain'; import { ICurrentUser } from '@src/modules/authentication'; -import { CourseService } from '@src/modules/learnroom/service'; +import { CourseService } from '@src/modules/learnroom'; import { IScopeInfo, VideoConference, VideoConferenceJoin, VideoConferenceState } from './dto'; import { VideoConferenceDeprecatedUc } from './video-conference-deprecated.uc'; import { @@ -63,7 +64,7 @@ describe('VideoConferenceUc', () => { let useCase: VideoConferenceDeprecatedUcSpec; let bbbService: DeepMocked; - let authorizationService: DeepMocked; + let authorizationService: DeepMocked; let videoConferenceRepo: DeepMocked; let teamsRepo: DeepMocked; let courseService: DeepMocked; @@ -118,8 +119,8 @@ describe('VideoConferenceUc', () => { useValue: createMock(), }, { - provide: AuthorizationService, - useValue: createMock(), + provide: AuthorizationReferenceService, + useValue: createMock(), }, { provide: VideoConferenceRepo, @@ -149,7 +150,7 @@ describe('VideoConferenceUc', () => { }).compile(); useCase = module.get(VideoConferenceDeprecatedUcSpec); schoolService = module.get(LegacySchoolService); - authorizationService = module.get(AuthorizationService); + authorizationService = module.get(AuthorizationReferenceService); courseService = module.get(CourseService); calendarService = module.get(CalendarService); videoConferenceRepo = module.get(VideoConferenceRepo); diff --git a/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.ts b/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.ts index 7bce5b2f5a4..e6b1f11ed50 100644 --- a/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.ts +++ b/apps/server/src/modules/video-conference/uc/video-conference-deprecated.uc.ts @@ -18,14 +18,10 @@ import { CalendarEventDto } from '@shared/infra/calendar/dto/calendar-event.dto' import { TeamsRepo } from '@shared/repo'; import { VideoConferenceRepo } from '@shared/repo/videoconference/video-conference.repo'; import { ICurrentUser } from '@src/modules/authentication'; -import { - Action, - AuthorizableReferenceType, - AuthorizationContextBuilder, - AuthorizationService, -} from '@src/modules/authorization'; -import { CourseService } from '@src/modules/learnroom/service'; +import { Action, AuthorizationContextBuilder } from '@src/modules/authorization'; +import { AuthorizationReferenceService, AuthorizableReferenceType } from '@src/modules/authorization/domain'; import { LegacySchoolService } from '@src/modules/legacy-school'; +import { CourseService } from '@src/modules/learnroom'; import { UserService } from '@src/modules/user'; import { BBBBaseMeetingConfig, @@ -62,7 +58,7 @@ export class VideoConferenceDeprecatedUc { constructor( private readonly bbbService: BBBService, - private readonly authorizationService: AuthorizationService, + private readonly authorizationReferenceService: AuthorizationReferenceService, private readonly videoConferenceRepo: VideoConferenceRepo, private readonly teamsRepo: TeamsRepo, private readonly courseService: CourseService, @@ -413,7 +409,7 @@ export class VideoConferenceDeprecatedUc { permissions.forEach((perm) => { const context = action === Action.read ? AuthorizationContextBuilder.read([perm]) : AuthorizationContextBuilder.write([perm]); - const ret = this.authorizationService.hasPermissionByReferences(userId, entityName, entityId, context); + const ret = this.authorizationReferenceService.hasPermissionByReferences(userId, entityName, entityId, context); returnMap.set(perm, ret); }); return returnMap; diff --git a/apps/server/src/modules/video-conference/uc/video-conference-end.uc.ts b/apps/server/src/modules/video-conference/uc/video-conference-end.uc.ts index a9799f67a89..50318c001c0 100644 --- a/apps/server/src/modules/video-conference/uc/video-conference-end.uc.ts +++ b/apps/server/src/modules/video-conference/uc/video-conference-end.uc.ts @@ -16,6 +16,12 @@ export class VideoConferenceEndUc { ) {} async end(currentUserId: EntityId, scope: ScopeRef): Promise> { + /* need to be replace with + const [authorizableUser, scopeRessource]: [User, TeamEntity | Course] = await Promise.all([ + this.authorizationService.getUserWithPermissions(userId), + this.videoConferenceService.loadScopeRessources(scopeId, scope), + ]); + */ const user: UserDO = await this.userService.findById(currentUserId); const userId: string = user.id as string; diff --git a/apps/server/src/modules/video-conference/uc/video-conference-info.uc.ts b/apps/server/src/modules/video-conference/uc/video-conference-info.uc.ts index 91ebb23ea2b..79a1f95b8d1 100644 --- a/apps/server/src/modules/video-conference/uc/video-conference-info.uc.ts +++ b/apps/server/src/modules/video-conference/uc/video-conference-info.uc.ts @@ -17,6 +17,12 @@ export class VideoConferenceInfoUc { ) {} async getMeetingInfo(currentUserId: EntityId, scope: ScopeRef): Promise { + /* need to be replace with + const [authorizableUser, scopeRessource]: [User, TeamEntity | Course] = await Promise.all([ + this.authorizationService.getUserWithPermissions(userId), + this.videoConferenceService.loadScopeRessources(scopeId, scope), + ]); + */ const user: UserDO = await this.userService.findById(currentUserId); await this.videoConferenceService.throwOnFeaturesDisabled(user.schoolId); diff --git a/apps/server/src/modules/video-conference/video-conference.module.ts b/apps/server/src/modules/video-conference/video-conference.module.ts index 70a999437c0..6277f0dde0a 100644 --- a/apps/server/src/modules/video-conference/video-conference.module.ts +++ b/apps/server/src/modules/video-conference/video-conference.module.ts @@ -3,6 +3,7 @@ import { HttpModule } from '@nestjs/axios'; import { CalendarModule } from '@shared/infra/calendar'; import { VideoConferenceRepo } from '@shared/repo/videoconference/video-conference.repo'; import { AuthorizationModule } from '@src/modules/authorization'; +import { AuthorizationReferenceModule } from '@src/modules/authorization/authorization-reference.module'; import { TeamsRepo } from '@shared/repo'; import { LegacySchoolModule } from '@src/modules/legacy-school'; import { LoggerModule } from '@src/core/logger'; @@ -19,6 +20,7 @@ import { LearnroomModule } from '../learnroom'; @Module({ imports: [ AuthorizationModule, + AuthorizationReferenceModule, // can be removed wenn video-conference-deprecated is removed CalendarModule, HttpModule, LegacySchoolModule, diff --git a/apps/server/src/shared/domain/rules/index.ts b/apps/server/src/shared/domain/rules/index.ts deleted file mode 100644 index 888b2ee8501..00000000000 --- a/apps/server/src/shared/domain/rules/index.ts +++ /dev/null @@ -1,39 +0,0 @@ -import { BoardDoRule } from './board-do.rule'; -import { ContextExternalToolRule } from './context-external-tool.rule'; -import { CourseGroupRule } from './course-group.rule'; -import { CourseRule } from './course.rule'; -import { LessonRule } from './lesson.rule'; -import { SchoolExternalToolRule } from './school-external-tool.rule'; -import { LegacySchoolRule } from './legacy-school.rule'; -import { SubmissionRule } from './submission.rule'; -import { TaskRule } from './task.rule'; -import { TeamRule } from './team.rule'; -import { UserLoginMigrationRule } from './user-login-migration.rule'; -import { UserRule } from './user.rule'; - -export * from './board-do.rule'; -export * from './course-group.rule'; -export * from './course.rule'; -export * from './lesson.rule'; -export * from './school-external-tool.rule'; -export * from './legacy-school.rule'; -export * from './submission.rule'; -export * from './task.rule'; -export * from './team.rule'; -export * from './user.rule'; -export * from './context-external-tool.rule'; - -export const ALL_RULES = [ - LessonRule, - CourseRule, - CourseGroupRule, - LegacySchoolRule, - SubmissionRule, - TaskRule, - TeamRule, - UserRule, - SchoolExternalToolRule, - BoardDoRule, - ContextExternalToolRule, - UserLoginMigrationRule, -]; diff --git a/apps/server/src/shared/infra/antivirus/index.ts b/apps/server/src/shared/infra/antivirus/index.ts index 833c46d81a7..2816f95ee57 100644 --- a/apps/server/src/shared/infra/antivirus/index.ts +++ b/apps/server/src/shared/infra/antivirus/index.ts @@ -1,3 +1,3 @@ -export * from './interfaces'; -export * from './antivirus.module'; -export * from './antivirus.service'; +export * from './interfaces'; +export * from './antivirus.module'; +export * from './antivirus.service'; diff --git a/apps/server/src/shared/testing/test-api-client.ts b/apps/server/src/shared/testing/test-api-client.ts index 733dcdd5cfc..75be7b7dc5f 100644 --- a/apps/server/src/shared/testing/test-api-client.ts +++ b/apps/server/src/shared/testing/test-api-client.ts @@ -140,6 +140,10 @@ export class TestApiClient { } private getJwtFromResponse(response: Response): string { + if (response.error) { + const error = JSON.stringify(response.error); + throw new Error(error); + } if (!this.isAuthenticationResponse(response.body)) { const body = JSON.stringify(response.body); throw new Error(`${testReqestConst.errorMessage} ${body}`);