Skip to content

Commit

Permalink
requested changes WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
IgorCapCoder committed Nov 28, 2023
1 parent fbcd329 commit f746e86
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Injectable } from '@nestjs/common';
import { ExternalTool } from '../../external-tool/domain';
import { SchoolExternalTool } from '../../school-external-tool/domain';
import { ContextExternalTool } from '../../context-external-tool/domain';
import { ToolConfigurationStatus } from '../enum';
import { ToolConfigurationStatus, ToolContextType } from '../enum';
import { ToolVersion } from '../interface';

// TODO N21-1337 remove class when tool versioning is removed
Expand All @@ -11,7 +11,7 @@ export class CommonToolService {
/**
* @deprecated use ToolVersionService
*/
determineToolConfigurationStatus(
public determineToolConfigurationStatus(
externalTool: ExternalTool,
schoolExternalTool: SchoolExternalTool,
contextExternalTool: ContextExternalTool
Expand All @@ -30,4 +30,11 @@ export class CommonToolService {
private isLatest(tool1: ToolVersion, tool2: ToolVersion): boolean {
return tool1.getVersion() >= tool2.getVersion();
}

public isContextRestricted(externalTool: ExternalTool, context: ToolContextType): boolean {
if (externalTool.restrictToContexts?.length && !externalTool.restrictToContexts.includes(context)) {
return true;
}
return false;
}
}
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
import { Injectable } from '@nestjs/common';
import { EntityId } from '@shared/domain';
import { ContextExternalToolRepo } from '@shared/repo';
import { AuthorizationContext } from '@modules/authorization';
import { ForbiddenLoggableException } from '@modules/authorization/domain/error';
import { ContextExternalTool, ContextRef } from '../domain';
import { ContextExternalToolQuery } from '../uc/dto/context-external-tool.types';
import { SchoolExternalTool } from '../../school-external-tool/domain';
import { ExternalTool } from '../../external-tool/domain';
import { ExternalToolService } from '../../external-tool/service';
import { SchoolExternalToolService } from '../../school-external-tool/service';
import { RestrictedContextMismatchLoggable } from './restricted-context-mismatch-loggabble';
import { CommonToolService } from '../../common/service';

@Injectable()
export class ContextExternalToolService {
constructor(
private readonly contextExternalToolRepo: ContextExternalToolRepo,
private readonly externalToolService: ExternalToolService,
private readonly schoolExternalToolService: SchoolExternalToolService
private readonly schoolExternalToolService: SchoolExternalToolService,
private readonly commonToolService: CommonToolService
) {}

public async findContextExternalTools(query: ContextExternalToolQuery): Promise<ContextExternalTool[]> {
Expand Down Expand Up @@ -58,21 +59,15 @@ export class ContextExternalToolService {
return contextExternalTools;
}

public async checkContextRestrictions(
contextExternalTool: ContextExternalTool,
userId: EntityId,
context: AuthorizationContext
): Promise<void> {
public async checkContextRestrictions(contextExternalTool: ContextExternalTool): Promise<void> {
const schoolExternalTool: SchoolExternalTool = await this.schoolExternalToolService.findById(
contextExternalTool.schoolToolRef.schoolToolId
);

const externalTool: ExternalTool = await this.externalToolService.findById(schoolExternalTool.toolId);

if (externalTool.restrictToContexts && externalTool.restrictToContexts[0]) {
if (!externalTool.restrictToContexts.includes(contextExternalTool.contextRef.type)) {
throw new ForbiddenLoggableException(userId, 'ContextExternalTool', context);
}
if (this.commonToolService.isContextRestricted(externalTool, contextExternalTool.contextRef.type)) {
throw new RestrictedContextMismatchLoggable(externalTool.name, contextExternalTool.contextRef.type);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { ToolContextType } from '../../common/enum';
import { RestrictedContextMismatchLoggable } from './restricted-context-mismatch-loggabble';

describe('RestrictedContextMismatchLoggable', () => {
describe('constructor', () => {
const setup = () => {
const externalToolName = 'name';
const context: ToolContextType = ToolContextType.COURSE;

return { externalToolName, context };
};

it('should create an instance of RestrictedContextMismatchLoggable', () => {
const { externalToolName, context } = setup();

const loggable = new RestrictedContextMismatchLoggable(externalToolName, context);

expect(loggable).toBeInstanceOf(RestrictedContextMismatchLoggable);
});
});

describe('getLogMessage', () => {
const setup = () => {
const externalToolName = 'name';
const context: ToolContextType = ToolContextType.COURSE;
const loggable = new RestrictedContextMismatchLoggable(externalToolName, context);

return { loggable, externalToolName, context };
};

it('should return a loggable message', () => {
const { loggable, externalToolName, context } = setup();

const message = loggable.getLogMessage();

expect(message).toEqual({
type: 'UNPROCESSABLE_ENTITY_EXCEPTION',
message: `Could not create an instance of ${externalToolName} in context: ${context} because of the context restrictions of the tool.`,
stack: loggable.stack,
data: {
externalToolName,
context,
},
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { UnprocessableEntityException } from '@nestjs/common';
import { Loggable } from '@src/core/logger/interfaces';
import { ErrorLogMessage, LogMessage, ValidationErrorLogMessage } from '@src/core/logger';
import { ToolContextType } from '../../common/enum';

export class RestrictedContextMismatchLoggable extends UnprocessableEntityException implements Loggable {
constructor(private readonly externalToolName: string, private readonly context: ToolContextType) {
super();
}

getLogMessage(): LogMessage | ErrorLogMessage | ValidationErrorLogMessage {
const message: LogMessage | ErrorLogMessage | ValidationErrorLogMessage = {
type: 'UNPROCESSABLE_ENTITY_EXCEPTION',
message: `Could not create an instance of ${this.externalToolName} in context: ${this.context} because of the context restrictions of the tool.`,
stack: this.stack,
data: {
externalToolName: this.externalToolName,
context: this.context,
},
};

return message;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class ContextExternalToolUc {

await this.toolPermissionHelper.ensureContextPermissions(userId, contextExternalTool, context);

await this.contextExternalToolService.checkContextRestrictions(contextExternalTool, userId, context);
await this.contextExternalToolService.checkContextRestrictions(contextExternalTool);

await this.contextExternalToolValidationService.validate(contextExternalTool);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
schoolExternalToolFactory,
setupEntities,
} from '@shared/testing';
import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { CustomParameter } from '../../common/domain';
import { CustomParameterScope, ToolContextType } from '../../common/enum';
import { ContextExternalTool } from '../../context-external-tool/domain';
Expand All @@ -16,10 +17,12 @@ import { ExternalTool } from '../domain';
import { ContextExternalToolTemplateInfo } from '../uc';
import { ExternalToolConfigurationService } from './external-tool-configuration.service';
import { ToolContextTypesList } from '../controller/dto/response/tool-context-types-list';
import { CommonToolService } from '../../common/service';

describe('ExternalToolConfigurationService', () => {
let module: TestingModule;
let service: ExternalToolConfigurationService;
let commonToolservice: DeepMocked<CommonToolService>;

let toolFeatures: IToolFeatures;

Expand All @@ -35,11 +38,16 @@ describe('ExternalToolConfigurationService', () => {
contextConfigurationEnabled: false,
},
},
{
provide: CommonToolService,
useValue: createMock<CommonToolService>(),
},
],
}).compile();

service = module.get(ExternalToolConfigurationService);
toolFeatures = module.get(ToolFeatures);
commonToolservice = module.get(CommonToolService);
});

afterEach(() => {
Expand Down Expand Up @@ -197,6 +205,8 @@ describe('ExternalToolConfigurationService', () => {

const availableTools: ContextExternalToolTemplateInfo[] = [{ externalTool, schoolExternalTool }];

commonToolservice.isContextRestricted.mockReturnValueOnce(false);

return {
contextType,
availableTools,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@ import { IToolFeatures, ToolFeatures } from '../../tool-config';
import { ExternalTool } from '../domain';
import { ContextExternalToolTemplateInfo } from '../uc/dto';
import { ToolContextTypesList } from '../controller/dto/response/tool-context-types-list';
import { CommonToolService } from '../../common/service';

@Injectable()
export class ExternalToolConfigurationService {
constructor(@Inject(ToolFeatures) private readonly toolFeatures: IToolFeatures) {}
constructor(
@Inject(ToolFeatures) private readonly toolFeatures: IToolFeatures,
private readonly commonTooLService: CommonToolService
) {}

public filterForAvailableTools(externalTools: Page<ExternalTool>, toolIdsInUse: EntityId[]): ExternalTool[] {
const visibleTools: ExternalTool[] = externalTools.data.filter((tool: ExternalTool): boolean => !tool.isHidden);
Expand Down Expand Up @@ -79,12 +83,9 @@ export class ExternalToolConfigurationService {
availableTools: ContextExternalToolTemplateInfo[],
contextType: ToolContextType
): ContextExternalToolTemplateInfo[] {
const availableToolsForContext: ContextExternalToolTemplateInfo[] = availableTools.filter((availableTool) => {
if (availableTool.externalTool.restrictToContexts && availableTool.externalTool.restrictToContexts[0]) {
return availableTool.externalTool.restrictToContexts.includes(contextType);
}
return true;
});
const availableToolsForContext: ContextExternalToolTemplateInfo[] = availableTools.filter(
(availableTool) => !this.commonTooLService.isContextRestricted(availableTool.externalTool, contextType)
);
return availableToolsForContext;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,15 @@ import {
schoolFactory,
TestApiClient,
UserAndAccountTestFactory,
customParameterFactory,
} from '@shared/testing';
import { ServerTestModule } from '@modules/server';
import { Response } from 'supertest';
import { SchoolExternalToolEntity } from '../../../school-external-tool/entity';
import { LaunchRequestMethod } from '../../types';
import { ToolLaunchRequestResponse, ToolLaunchParams } from '../dto';
import { ContextExternalToolEntity, ContextExternalToolType } from '../../../context-external-tool/entity';
import {
CustomParameterLocation,
CustomParameterScope,
CustomParameterType,
ExternalToolEntity,
} from '../../../external-tool/entity';
import { CustomParameterLocation, CustomParameterScope, ExternalToolEntity } from '../../../external-tool/entity';
import { ToolConfigType } from '../../../common/enum';

describe('ToolLaunchController (API)', () => {
Expand Down Expand Up @@ -70,22 +66,16 @@ describe('ToolLaunchController (API)', () => {
config: basicToolConfigFactory.build({ baseUrl: 'https://mockurl.de', type: ToolConfigType.BASIC }),
version: 0,
parameters: [
{
customParameterFactory.build({
name: 'schoolMockParameter',
displayName: 'MockParameter',
scope: CustomParameterScope.SCHOOL,
type: CustomParameterType.STRING,
location: CustomParameterLocation.PATH,
isOptional: false,
},
{
}),
customParameterFactory.build({
name: 'contextMockParameter',
displayName: 'MockParameter',
scope: CustomParameterScope.CONTEXT,
type: CustomParameterType.STRING,
location: CustomParameterLocation.PATH,
isOptional: false,
},
}),
],
});
const schoolExternalToolEntity: SchoolExternalToolEntity = schoolExternalToolEntityFactory.buildWithId({
Expand Down

0 comments on commit f746e86

Please sign in to comment.