Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BC-8137 - move auth rules into modules #5271

Merged
merged 29 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c7dee3d
move context external tool authorisation code
Metauriel Sep 27, 2024
f2e8acd
move school external tool authorisation code
Metauriel Sep 27, 2024
128d522
add context external tool tests
Metauriel Sep 27, 2024
1a53e92
move external tool authorisation code
Metauriel Sep 27, 2024
8408a74
begin injection of course rule and reference loader
Metauriel Sep 27, 2024
7acb4c4
Revert "begin injection of course rule and reference loader"
Metauriel Oct 2, 2024
0240542
fix build
Metauriel Oct 2, 2024
54d4648
make rule manager independent of all rules
Metauriel Oct 2, 2024
bac69bc
inject lesson reference loader
Metauriel Oct 4, 2024
2d5f05c
inject teams reference loader
Metauriel Oct 4, 2024
d5e36ce
update reference loader tests
Metauriel Oct 4, 2024
dd1909a
make coursegroup rule independent of other rules
Metauriel Oct 4, 2024
70b0993
Merge branch 'main' into BC-8137-move-auth-rules-into-modules
Metauriel Oct 4, 2024
d91c29c
Revert "make coursegroup rule independent of other rules"
Metauriel Oct 4, 2024
8754686
move rules into seperate module
Metauriel Oct 4, 2024
1ad6954
fix building for feathers tests
Metauriel Oct 4, 2024
27825ba
update rule manager tests
Metauriel Oct 4, 2024
91a771d
linter
Metauriel Oct 7, 2024
455dfe4
move authorization-reference code into seperate module
Metauriel Oct 7, 2024
b12a61e
fix imports in tests
Metauriel Oct 7, 2024
a6bec4b
N21-2191 lti encryption (#5274)
mrikallab Oct 7, 2024
f441161
N21 2202 fix shd school data cannot be updated (#5261)
sdinkov Oct 7, 2024
564f260
Merge branch 'main' into BC-8137-move-auth-rules-into-modules
Metauriel Oct 7, 2024
c62727b
Merge branch 'main' into BC-8137-move-auth-rules-into-modules
CeEv Oct 10, 2024
79e919f
Merge branch 'main' into BC-8137-move-auth-rules-into-modules
CeEv Oct 15, 2024
8f22064
Merge branch 'main' into BC-8137-move-auth-rules-into-modules
Metauriel Oct 18, 2024
728262e
review comments
Metauriel Oct 18, 2024
f2892a3
Merge branch 'main' into BC-8137-move-auth-rules-into-modules
Metauriel Oct 21, 2024
bcef9f1
more review fixes
Metauriel Oct 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions apps/server/src/apps/server.app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@
/* eslint-disable no-console */
import { MikroORM } from '@mikro-orm/core';
import { AccountService } from '@modules/account';
import { SystemRule } from '@modules/authorization/domain/rules';

import { SystemRule } from '@modules/authorization-rules';
import { ColumnBoardService } from '@modules/board';
import { CollaborativeStorageUc } from '@modules/collaborative-storage/uc/collaborative-storage.uc';

Check warning on line 9 in apps/server/src/apps/server.app.ts

View workflow job for this annotation

GitHub Actions / nest_lint

'@modules/collaborative-storage/uc/collaborative-storage.uc' import is restricted from being used by a pattern. Do not deep import from a module
import { GroupService } from '@modules/group';
import { InternalServerModule } from '@modules/internal-server';
import { RocketChatService } from '@modules/rocketchat';
import { FeathersRosterService } from '@modules/roster';
import { ServerModule } from '@modules/server';
import { TeamService } from '@modules/teams/service/team.service';

Check warning on line 15 in apps/server/src/apps/server.app.ts

View workflow job for this annotation

GitHub Actions / nest_lint

'@modules/teams/service/team.service' import is restricted from being used by a pattern. Do not deep import from a module
import { NestFactory } from '@nestjs/core';
import { ExpressAdapter } from '@nestjs/platform-express';
import { enableOpenApiDocs } from '@shared/controller/swagger';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { EntityId } from '@shared/domain/types';
import { Injectable } from '@nestjs/common';
import { AuthorizableReferenceType, AuthorizationContext, AuthorizationReferenceService } from '../domain';
import { AuthorizationContext, AuthorizableReferenceType } from '@modules/authorization';
import { AuthorizedReponse } from './dto';
import { AuthorizationReponseMapper } from './mapper';
import { AuthorizationReferenceService } from '../domain';

@Injectable()
export class AuthorizationReferenceUc {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { Permission } from '@shared/domain/interface';
import { Action, AuthorizationContext, AuthorizableReferenceType } from '@modules/authorization';
import { ApiProperty } from '@nestjs/swagger';
import { Permission } from '@shared/domain/interface';
import { Type } from 'class-transformer';
import { IsArray, IsEnum, IsMongoId, ValidateNested } from 'class-validator';
import { Action, AuthorizableReferenceType, AuthorizationContext } from '../../domain';

class AuthorizationContextParams implements AuthorizationContext {
@IsEnum(Action)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import { HttpStatus, INestApplication } from '@nestjs/common';
import { Test } from '@nestjs/testing';
import { TestApiClient, UserAndAccountTestFactory } from '@shared/testing';
import { Permission } from '@shared/domain/interface';
import { Action, AuthorizableReferenceType, AuthorizationContext, AuthorizationContextBuilder } from '../../domain';
import {
Action,
AuthorizationContext,
AuthorizationContextBuilder,
AuthorizableReferenceType,
} from '@modules/authorization';
import { AuthorizationReponseMapper } from '../mapper';
import { AuthorizationBodyParams } from '../dto';

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { LoggerConfig } from '@src/core/logger';

export interface AuthorizationReferenceConfig extends LoggerConfig {}
Original file line number Diff line number Diff line change
@@ -1,39 +1,25 @@
import { InstanceModule } from '@modules/instance';
import { LessonModule } from '@modules/lesson';
import { ToolModule } from '@modules/tool';
import { forwardRef, Module } from '@nestjs/common';
import {
CourseGroupRepo,
CourseRepo,
LegacySchoolRepo,
SchoolExternalToolRepo,
SubmissionRepo,
TaskRepo,
UserRepo,
} from '@shared/repo';
import { Module } from '@nestjs/common';
import { CourseGroupRepo, CourseRepo, LegacySchoolRepo, SubmissionRepo, TaskRepo, UserRepo } from '@shared/repo';
import { LoggerModule } from '@src/core/logger';
import { AuthorizationModule } from './authorization.module';
import { AuthorizationHelper, AuthorizationReferenceService, ReferenceLoader } from './domain';
import { TeamsModule } from '../teams';
import { AuthorizationModule } from '@modules/authorization';
import { AuthorizationReferenceService, ReferenceLoader } from './domain';

/**
* 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
imports: [AuthorizationModule, LessonModule, TeamsModule, forwardRef(() => ToolModule), LoggerModule, InstanceModule],
imports: [AuthorizationModule, LoggerModule, InstanceModule],
providers: [
AuthorizationHelper,
ReferenceLoader,
UserRepo,
CourseRepo,
CourseGroupRepo,
TaskRepo,
LegacySchoolRepo,
SubmissionRepo,
SchoolExternalToolRepo,
AuthorizationReferenceService,
],
exports: [AuthorizationReferenceService],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ import { NotFoundException } from '@nestjs/common';
import { Test, TestingModule } from '@nestjs/testing';
import { courseFactory, setupEntities, userFactory } from '@shared/testing';
import { ObjectId } from '@mikro-orm/mongodb';
import { AuthorizationService } from '@modules/authorization';
import { AuthorizableReferenceType } from '../type';
import {
AuthorizableReferenceType,
AuthorizationContextBuilder,
AuthorizationService,
ForbiddenLoggableException,
} from '@modules/authorization';
import { ReferenceLoader } from './reference.loader';
import { AuthorizationContextBuilder } from '../mapper';
import { ForbiddenLoggableException } from '../error';
import { AuthorizationReferenceService } from './authorization-reference.service';

describe('AuthorizationReferenceService', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { Injectable } from '@nestjs/common';
import { EntityId } from '@shared/domain/types';
import { ForbiddenLoggableException } from '../error';
import { AuthorizableReferenceType, AuthorizationContext } from '../type';
import { AuthorizationService } from './authorization.service';
import {
AuthorizationContext,
AuthorizationService,
ForbiddenLoggableException,
AuthorizableReferenceType,
} from '@src/modules/authorization';
hoeppner-dataport marked this conversation as resolved.
Show resolved Hide resolved
import { ReferenceLoader } from './reference.loader';

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './authorization-reference.service';
export * from './reference.loader';
Original file line number Diff line number Diff line change
@@ -1,25 +1,13 @@
import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { ObjectId } from '@mikro-orm/mongodb';
import { InstanceService } from '@modules/instance';
import { LessonService } from '@modules/lesson';
import { ContextExternalToolAuthorizableService, ExternalToolAuthorizableService } from '@modules/tool';
import { NotImplementedException } from '@nestjs/common';
import { Test, TestingModule } from '@nestjs/testing';
import { EntityId } from '@shared/domain/types';
import {
CourseGroupRepo,
CourseRepo,
LegacySchoolRepo,
SchoolExternalToolRepo,
SubmissionRepo,
TaskRepo,
UserRepo,
} from '@shared/repo';
import { CourseGroupRepo, CourseRepo, LegacySchoolRepo, SubmissionRepo, TaskRepo, UserRepo } from '@shared/repo';
import { setupEntities, userFactory } from '@shared/testing';
import { TeamAuthorisableService } from '@src/modules/teams';
import { AuthorizableReferenceType } from '../type';
import { AuthorizationInjectionService, AuthorizableReferenceType } from '@src/modules/authorization';
hoeppner-dataport marked this conversation as resolved.
Show resolved Hide resolved
import { ReferenceLoader } from './reference.loader';
import { AuthorizationInjectionService } from './authorization-injection.service';

describe('reference.loader', () => {
let service: ReferenceLoader;
Expand All @@ -29,12 +17,7 @@ describe('reference.loader', () => {
let courseGroupRepo: DeepMocked<CourseGroupRepo>;
let taskRepo: DeepMocked<TaskRepo>;
let schoolRepo: DeepMocked<LegacySchoolRepo>;
let lessonService: DeepMocked<LessonService>;
let teamsAuthorisableService: DeepMocked<TeamAuthorisableService>;
let submissionRepo: DeepMocked<SubmissionRepo>;
let schoolExternalToolRepo: DeepMocked<SchoolExternalToolRepo>;
let contextExternalToolAuthorizableService: DeepMocked<ContextExternalToolAuthorizableService>;
let externalToolAuthorizableService: DeepMocked<ExternalToolAuthorizableService>;
let instanceService: DeepMocked<InstanceService>;
const entityId: EntityId = new ObjectId().toHexString();

Expand Down Expand Up @@ -68,30 +51,10 @@ describe('reference.loader', () => {
provide: LegacySchoolRepo,
useValue: createMock<LegacySchoolRepo>(),
},
{
provide: LessonService,
useValue: createMock<LessonService>(),
},
{
provide: TeamAuthorisableService,
useValue: createMock<TeamAuthorisableService>(),
},
{
provide: SubmissionRepo,
useValue: createMock<SubmissionRepo>(),
},
{
provide: SchoolExternalToolRepo,
useValue: createMock<SchoolExternalToolRepo>(),
},
{
provide: ContextExternalToolAuthorizableService,
useValue: createMock<ContextExternalToolAuthorizableService>(),
},
{
provide: ExternalToolAuthorizableService,
useValue: createMock<ExternalToolAuthorizableService>(),
},
{
provide: InstanceService,
useValue: createMock<InstanceService>(),
Expand All @@ -106,12 +69,7 @@ describe('reference.loader', () => {
courseGroupRepo = await module.get(CourseGroupRepo);
taskRepo = await module.get(TaskRepo);
schoolRepo = await module.get(LegacySchoolRepo);
lessonService = await module.get(LessonService);
teamsAuthorisableService = await module.get(TeamAuthorisableService);
submissionRepo = await module.get(SubmissionRepo);
schoolExternalToolRepo = await module.get(SchoolExternalToolRepo);
contextExternalToolAuthorizableService = await module.get(ContextExternalToolAuthorizableService);
externalToolAuthorizableService = await module.get(ExternalToolAuthorizableService);
instanceService = await module.get(InstanceService);
});

Expand Down Expand Up @@ -184,45 +142,13 @@ describe('reference.loader', () => {
expect(injectionService.injectReferenceLoader).toBeCalledWith(AuthorizableReferenceType.School, schoolRepo);
});

it('should inject lesson service', () => {
expect(injectionService.injectReferenceLoader).toBeCalledWith(AuthorizableReferenceType.Lesson, lessonService);
});

it('should inject teams repo', () => {
expect(injectionService.injectReferenceLoader).toBeCalledWith(
AuthorizableReferenceType.Team,
teamsAuthorisableService
);
});

it('should inject submission repo', () => {
expect(injectionService.injectReferenceLoader).toBeCalledWith(
AuthorizableReferenceType.Submission,
submissionRepo
);
});

it('should inject school external tool repo', () => {
expect(injectionService.injectReferenceLoader).toBeCalledWith(
AuthorizableReferenceType.SchoolExternalToolEntity,
schoolExternalToolRepo
);
});

it('should inject context external tool authorizable service', () => {
expect(injectionService.injectReferenceLoader).toBeCalledWith(
AuthorizableReferenceType.ContextExternalToolEntity,
contextExternalToolAuthorizableService
);
});

it('should inject external tool authorizable service', () => {
expect(injectionService.injectReferenceLoader).toBeCalledWith(
AuthorizableReferenceType.ExternalTool,
externalToolAuthorizableService
);
});

it('should inject instance service', () => {
expect(injectionService.injectReferenceLoader).toBeCalledWith(
AuthorizableReferenceType.Instance,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,26 +1,15 @@
// TODO fix modules circular dependency
// eslint-disable-next-line @typescript-eslint/no-restricted-imports
import { ContextExternalToolAuthorizableService } from '@modules/tool/context-external-tool/service';
// eslint-disable-next-line @typescript-eslint/no-restricted-imports
import { TeamAuthorisableService } from '@src/modules/teams/service/team-authorisable.service';
import { ExternalToolAuthorizableService } from '@modules/tool/external-tool/service';
import { LessonService } from '@modules/lesson';
import { Injectable, NotImplementedException } from '@nestjs/common';
import { AuthorizableObject } from '@shared/domain/domain-object';
import { BaseDO } from '@shared/domain/domainobject';
import { EntityId } from '@shared/domain/types';
import { CourseGroupRepo, CourseRepo, LegacySchoolRepo, SubmissionRepo, TaskRepo, UserRepo } from '@shared/repo';
import {
CourseGroupRepo,
CourseRepo,
LegacySchoolRepo,
SchoolExternalToolRepo,
SubmissionRepo,
TaskRepo,
UserRepo,
} from '@shared/repo';
import { InstanceService } from '../../../instance';
import { AuthorizableReferenceType, AuthorizationLoaderService } from '../type';
import { AuthorizationInjectionService } from './authorization-injection.service';
AuthorizationInjectionService,
AuthorizationLoaderService,
AuthorizableReferenceType,
} from '@src/modules/authorization/domain';
hoeppner-dataport marked this conversation as resolved.
Show resolved Hide resolved
import { InstanceService } from '../../instance';
hoeppner-dataport marked this conversation as resolved.
Show resolved Hide resolved

@Injectable()
export class ReferenceLoader {
CeEv marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -30,12 +19,7 @@ export class ReferenceLoader {
private readonly courseGroupRepo: CourseGroupRepo,
private readonly taskRepo: TaskRepo,
private readonly schoolRepo: LegacySchoolRepo,
private readonly lessonService: LessonService,
private readonly teamAuthorisableService: TeamAuthorisableService,
private readonly submissionRepo: SubmissionRepo,
private readonly schoolExternalToolRepo: SchoolExternalToolRepo,
private readonly contextExternalToolAuthorizableService: ContextExternalToolAuthorizableService,
private readonly externalToolAuthorizableService: ExternalToolAuthorizableService,
private readonly instanceService: InstanceService,
private readonly authorizationInjectionService: AuthorizationInjectionService
) {
Expand All @@ -45,15 +29,7 @@ export class ReferenceLoader {
service.injectReferenceLoader(AuthorizableReferenceType.CourseGroup, this.courseGroupRepo);
service.injectReferenceLoader(AuthorizableReferenceType.User, this.userRepo);
service.injectReferenceLoader(AuthorizableReferenceType.School, this.schoolRepo);
service.injectReferenceLoader(AuthorizableReferenceType.Lesson, this.lessonService);
service.injectReferenceLoader(AuthorizableReferenceType.Team, this.teamAuthorisableService);
service.injectReferenceLoader(AuthorizableReferenceType.Submission, this.submissionRepo);
service.injectReferenceLoader(AuthorizableReferenceType.SchoolExternalToolEntity, this.schoolExternalToolRepo);
service.injectReferenceLoader(
AuthorizableReferenceType.ContextExternalToolEntity,
this.contextExternalToolAuthorizableService
);
service.injectReferenceLoader(AuthorizableReferenceType.ExternalTool, this.externalToolAuthorizableService);
service.injectReferenceLoader(AuthorizableReferenceType.Instance, this.instanceService);
}

Expand Down
3 changes: 3 additions & 0 deletions apps/server/src/modules/authorization-reference/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export { AuthorizationReferenceModule } from './authorization-reference.module';
export { AuthorizationReferenceApiModule } from './authorization-reference.api.module';
export { AuthorizationReferenceService } from './domain/authorization-reference.service';
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { LoggerConfig } from '@src/core/logger';

export interface AuthorizationRulesConfig extends LoggerConfig {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { Module } from '@nestjs/common';
import { AuthorizationModule } from '@modules/authorization';
import {
CourseGroupRule,
CourseRule,
GroupRule,
InstanceRule,
LegacySchoolRule,
LessonRule,
SchoolRule,
SchoolSystemOptionsRule,
SubmissionRule,
TaskRule,
TeamRule,
UserLoginMigrationRule,
UserRule,
SystemRule,
} from './rules';

@Module({
imports: [AuthorizationModule /* FeathersModule */ /* LoggerModule */],
hoeppner-dataport marked this conversation as resolved.
Show resolved Hide resolved
providers: [
// rules
CourseGroupRule,
CourseRule,
GroupRule,
LessonRule,
SchoolRule,
SubmissionRule,
TaskRule,
TeamRule,
UserRule,
UserLoginMigrationRule,
LegacySchoolRule,
SystemRule,
SchoolSystemOptionsRule,
InstanceRule,
],
exports: [
SystemRule, // Why export? This is a no go!
CeEv marked this conversation as resolved.
Show resolved Hide resolved
],
})
export class AuthorizationRulesModule {}
2 changes: 2 additions & 0 deletions apps/server/src/modules/authorization-rules/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export { AuthorizationRulesModule } from './authorization-rules.module';
export { SystemRule } from './rules/system.rule';
Loading
Loading