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

N21-1395 Fix authorization not working for CTL board elements after authorization refactoring #4491

Merged
merged 4 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
25 changes: 5 additions & 20 deletions apps/server/src/modules/tool/common/common-tool.module.ts
Original file line number Diff line number Diff line change
@@ -1,28 +1,13 @@
import { forwardRef, Module } from '@nestjs/common';
import { LegacySchoolModule } from '@modules/legacy-school';
import { Module } from '@nestjs/common';
import { ContextExternalToolRepo, SchoolExternalToolRepo } from '@shared/repo';
import { LoggerModule } from '@src/core/logger';
import { AuthorizationModule } from '@modules/authorization';
import { LegacySchoolModule } from '@modules/legacy-school';
import { LearnroomModule } from '@modules/learnroom';
import { CommonToolService, CommonToolValidationService } from './service';
import { ToolPermissionHelper } from './uc/tool-permission-helper';

@Module({
imports: [LoggerModule, forwardRef(() => AuthorizationModule), LegacySchoolModule, LearnroomModule],
imports: [LoggerModule, LegacySchoolModule],
// TODO: make deletion of entities cascading, adjust ExternalToolService.deleteExternalTool and remove the repos from here
providers: [
CommonToolService,
CommonToolValidationService,
ToolPermissionHelper,
SchoolExternalToolRepo,
ContextExternalToolRepo,
],
exports: [
CommonToolService,
CommonToolValidationService,
ToolPermissionHelper,
SchoolExternalToolRepo,
ContextExternalToolRepo,
],
providers: [CommonToolService, CommonToolValidationService, SchoolExternalToolRepo, ContextExternalToolRepo],
exports: [CommonToolService, CommonToolValidationService, SchoolExternalToolRepo, ContextExternalToolRepo],
})
export class CommonToolModule {}

This file was deleted.

13 changes: 0 additions & 13 deletions apps/server/src/modules/tool/common/mapper/context-type.mapper.ts

This file was deleted.

2 changes: 1 addition & 1 deletion apps/server/src/modules/tool/common/mapper/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export * from './context-type.mapper';
export * from './tool-status-response.mapper';
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ToolConfigurationStatusResponse } from '../../context-external-tool/controller/dto/tool-configuration-status.response';
import { ToolConfigurationStatusResponse } from '../../context-external-tool/controller/dto';
import { ToolConfigurationStatus } from '../enum';

export const statusMapping: Record<ToolConfigurationStatus, ToolConfigurationStatusResponse> = {
Expand Down
43 changes: 26 additions & 17 deletions apps/server/src/modules/tool/common/uc/tool-permission-helper.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { forwardRef, Inject, Injectable } from '@nestjs/common';
import { Course, EntityId, LegacySchoolDo, User } from '@shared/domain';
import { AuthorizationContext, AuthorizationService } from '@modules/authorization';
import { LegacySchoolService } from '@modules/legacy-school';
import { AuthorizationContext, AuthorizationService, ForbiddenLoggableException } from '@modules/authorization';
import { AuthorizableReferenceType } from '@modules/authorization/domain';
import { BoardDoAuthorizableService, ContentElementService } from '@modules/board';
import { CourseService } from '@modules/learnroom';
import { LegacySchoolService } from '@modules/legacy-school';
import { forwardRef, Inject, Injectable } from '@nestjs/common';
import { BoardDoAuthorizable, Course, EntityId, LegacySchoolDo, User } from '@shared/domain';
import { ContextExternalTool } from '../../context-external-tool/domain';
import { SchoolExternalTool } from '../../school-external-tool/domain';
// import { ContextTypeMapper } from '../mapper';
import { ToolContextType } from '../enum';

@Injectable()
export class ToolPermissionHelper {
Expand All @@ -15,7 +17,9 @@ export class ToolPermissionHelper {
// 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
private readonly courseService: CourseService,
private readonly boardElementService: ContentElementService,
private readonly boardService: BoardDoAuthorizableService
) {}

// TODO build interface to get contextDO by contextType
Expand All @@ -24,19 +28,24 @@ export class ToolPermissionHelper {
contextExternalTool: ContextExternalTool,
context: AuthorizationContext
): Promise<void> {
// 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),
]);
const authorizableUser = await this.authorizationService.getUserWithPermissions(userId);

if (contextExternalTool.id) {
this.authorizationService.checkPermission(authorizableUser, contextExternalTool, context);
}
this.authorizationService.checkPermission(authorizableUser, contextExternalTool, context);

if (contextExternalTool.contextRef.type === ToolContextType.COURSE) {
// loading of ressources should be part of the UC -> unnessasary awaits
const course: Course = await this.courseService.findById(contextExternalTool.contextRef.id);

this.authorizationService.checkPermission(authorizableUser, course, context);
} else if (contextExternalTool.contextRef.type === ToolContextType.BOARD_ELEMENT) {
const boardElement = await this.boardElementService.findById(contextExternalTool.contextRef.id);

// const type = ContextTypeMapper.mapContextTypeToAllowedAuthorizationEntityType(contextExternalTool.contextRef.type);
// no different types possible until it is fixed.
this.authorizationService.checkPermission(authorizableUser, course, context);
const board: BoardDoAuthorizable = await this.boardService.getBoardAuthorizable(boardElement);

this.authorizationService.checkPermission(authorizableUser, board, context);
} else {
throw new ForbiddenLoggableException(userId, AuthorizableReferenceType.ContextExternalToolEntity, context);
}
}

public async ensureSchoolPermissions(
Expand Down
129 changes: 91 additions & 38 deletions apps/server/src/modules/tool/common/uc/tool-permissions-helper.spec.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,41 @@
import { Test, TestingModule } from '@nestjs/testing';
import { createMock, DeepMocked } from '@golevelup/ts-jest';
import {
AuthorizationContext,
AuthorizationContextBuilder,
AuthorizationService,
ForbiddenLoggableException,
} from '@modules/authorization';
import { AuthorizableReferenceType } from '@modules/authorization/domain';
import { BoardDoAuthorizableService, ContentElementService } from '@modules/board';
import { CourseService } from '@modules/learnroom';
import { LegacySchoolService } from '@modules/legacy-school';
import { ForbiddenException } from '@nestjs/common';
import { Test, TestingModule } from '@nestjs/testing';
import { BoardDoAuthorizable, ExternalToolElement, LegacySchoolDo, Permission } from '@shared/domain';
import {
contextExternalToolFactory,
courseFactory,
externalToolElementFactory,
legacySchoolDoFactory,
schoolExternalToolFactory,
setupEntities,
userFactory,
} from '@shared/testing';
import { Permission, LegacySchoolDo } from '@shared/domain';
import { AuthorizationContext, AuthorizationContextBuilder, AuthorizationService } from '@modules/authorization';
import { LegacySchoolService } from '@modules/legacy-school';
import { ForbiddenException } from '@nestjs/common';
import { CourseService } from '@modules/learnroom';
import { ContextExternalTool } from '../../context-external-tool/domain';
import { ToolPermissionHelper } from './tool-permission-helper';
import { boardDoAuthorizableFactory } from '@shared/testing/factory/domainobject/board/board-do-authorizable.factory';
import { ContextExternalTool, ContextRef } from '../../context-external-tool/domain';
import { SchoolExternalTool } from '../../school-external-tool/domain';
import { ToolContextType } from '../enum';
import { ToolPermissionHelper } from './tool-permission-helper';

describe('ToolPermissionHelper', () => {
let module: TestingModule;
let helper: ToolPermissionHelper;

let authorizationService: DeepMocked<AuthorizationService>;
let courseService: DeepMocked<CourseService>;
let schoolService: DeepMocked<LegacySchoolService>;
let courseService: DeepMocked<CourseService>;
let contentElementService: DeepMocked<ContentElementService>;
let boardDoAuthorizableService: DeepMocked<BoardDoAuthorizableService>;

beforeAll(async () => {
await setupEntities();
Expand All @@ -34,21 +46,31 @@ describe('ToolPermissionHelper', () => {
provide: AuthorizationService,
useValue: createMock<AuthorizationService>(),
},
{
provide: LegacySchoolService,
useValue: createMock<LegacySchoolService>(),
},
{
provide: CourseService,
useValue: createMock<CourseService>(),
},
{
provide: LegacySchoolService,
useValue: createMock<LegacySchoolService>(),
provide: ContentElementService,
useValue: createMock<ContentElementService>(),
},
{
provide: BoardDoAuthorizableService,
useValue: createMock<BoardDoAuthorizableService>(),
},
],
}).compile();

helper = module.get(ToolPermissionHelper);
authorizationService = module.get(AuthorizationService);
courseService = module.get(CourseService);
schoolService = module.get(LegacySchoolService);
courseService = module.get(CourseService);
contentElementService = module.get(ContentElementService);
boardDoAuthorizableService = module.get(BoardDoAuthorizableService);
});

afterAll(async () => {
Expand All @@ -60,16 +82,20 @@ describe('ToolPermissionHelper', () => {
});

describe('ensureContextPermissions', () => {
describe('when context external tool with id is given', () => {
describe('when a context external tool for context "course" is given', () => {
const setup = () => {
const user = userFactory.buildWithId();
const course = courseFactory.buildWithId();
const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId();
const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId({
contextRef: new ContextRef({
id: course.id,
type: ToolContextType.COURSE,
}),
});
const context: AuthorizationContext = AuthorizationContextBuilder.read([Permission.CONTEXT_TOOL_USER]);

courseService.findById.mockResolvedValueOnce(course);
authorizationService.getUserWithPermissions.mockResolvedValueOnce(user);
authorizationService.checkPermission.mockReturnValueOnce().mockReturnValueOnce();
courseService.findById.mockResolvedValueOnce(course);

return {
user,
Expand All @@ -88,70 +114,97 @@ describe('ToolPermissionHelper', () => {
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();
describe('when a context external tool for context "board element" is given', () => {
const setup = () => {
const user = userFactory.buildWithId();
const externalToolElement: ExternalToolElement = externalToolElementFactory.build();
const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId({
contextRef: new ContextRef({
id: externalToolElement.id,
type: ToolContextType.BOARD_ELEMENT,
}),
});
const board: BoardDoAuthorizable = boardDoAuthorizableFactory.build();
const context: AuthorizationContext = AuthorizationContextBuilder.read([Permission.CONTEXT_TOOL_USER]);

const result = await helper.ensureContextPermissions(user.id, contextExternalTool, context);
authorizationService.getUserWithPermissions.mockResolvedValueOnce(user);
contentElementService.findById.mockResolvedValueOnce(externalToolElement);
boardDoAuthorizableService.getBoardAuthorizable.mockResolvedValueOnce(board);

expect(result).toBeUndefined();
return {
user,
board,
contextExternalTool,
context,
};
};

it('should check permission for context external tool', async () => {
const { user, board, 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, board, context);
});
});

describe('when context external tool without id is given', () => {
describe('when the context external tool has an unkown context', () => {
const setup = () => {
const user = userFactory.buildWithId();
const course = courseFactory.buildWithId();
const contextExternalTool: ContextExternalTool = contextExternalToolFactory.build();
const contextExternalTool: ContextExternalTool = contextExternalToolFactory.buildWithId({
contextRef: {
type: 'unknown type' as unknown as ToolContextType,
},
});
const context: AuthorizationContext = AuthorizationContextBuilder.read([Permission.CONTEXT_TOOL_USER]);

courseService.findById.mockResolvedValueOnce(course);
authorizationService.getUserWithPermissions.mockResolvedValueOnce(user);

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);
it('should throw a forbidden loggable exception', async () => {
const { user, contextExternalTool, context } = setup();

expect(authorizationService.checkPermission).toHaveBeenCalledTimes(1);
expect(authorizationService.checkPermission).toHaveBeenCalledWith(user, course, context);
await expect(helper.ensureContextPermissions(user.id, contextExternalTool, context)).rejects.toThrowError(
new ForbiddenLoggableException(user.id, AuthorizableReferenceType.ContextExternalToolEntity, 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]);
const error = new ForbiddenException();

courseService.findById.mockResolvedValueOnce(course);
authorizationService.getUserWithPermissions.mockResolvedValueOnce(user);
authorizationService.checkPermission.mockImplementationOnce(() => {
throw new ForbiddenException();
throw error;
});

return {
user,
course,
contextExternalTool,
context,
error,
};
};

it('should check permission for context external tool', async () => {
const { user, contextExternalTool, context } = setup();
it('should check permission for context external tool and fail', async () => {
const { user, contextExternalTool, context, error } = setup();

await expect(helper.ensureContextPermissions(user.id, contextExternalTool, context)).rejects.toThrowError(
new ForbiddenException()
error
);
});
});
Expand Down
12 changes: 9 additions & 3 deletions apps/server/src/modules/tool/tool-api.module.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { Module } from '@nestjs/common';
import { LtiToolRepo } from '@shared/repo';
import { LoggerModule } from '@src/core/logger';
import { AuthorizationModule } from '@modules/authorization';
import { LegacySchoolModule } from '@modules/legacy-school';
import { UserModule } from '@modules/user';
import { Module } from '@nestjs/common';
import { LtiToolRepo } from '@shared/repo';
import { LoggerModule } from '@src/core/logger';
import { BoardModule } from '../board';
import { LearnroomModule } from '../learnroom';
import { CommonToolModule } from './common';
import { ToolPermissionHelper } from './common/uc/tool-permission-helper';
import { ToolContextController } from './context-external-tool/controller';
import { ToolReferenceController } from './context-external-tool/controller/tool-reference.controller';
import { ContextExternalToolUc, ToolReferenceUc } from './context-external-tool/uc';
Expand All @@ -29,6 +32,8 @@ import { ToolModule } from './tool.module';
LoggerModule,
LegacySchoolModule,
ToolConfigModule,
LearnroomModule,
BoardModule,
],
controllers: [
ToolLaunchController,
Expand All @@ -51,6 +56,7 @@ import { ToolModule } from './tool.module';
ContextExternalToolUc,
ToolLaunchUc,
ToolReferenceUc,
ToolPermissionHelper,
],
})
export class ToolApiModule {}
Loading
Loading