From d7426fac5be99644a5ad8136a86294253d90535e Mon Sep 17 00:00:00 2001 From: Mrika Llabani Date: Mon, 7 Oct 2024 14:14:36 +0200 Subject: [PATCH 01/10] N21-2191 fix configmap --- .../templates/configmap_file_init.yml.j2 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2 b/ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2 index f191ce69ca8..238b44226e4 100644 --- a/ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2 +++ b/ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2 @@ -519,8 +519,8 @@ data: echo "Inserting ctl seed data secrets to external-tools..." # Encrypt secrets of external tools that contain an lti11 config. - $CTL_SEED_SECRET_ONLINE_DIA_MATHE=$(node scripts/secret.js -s $AES_KEY -e $CTL_SEED_SECRET_ONLINE_DIA_MATHE) - $CTL_SEED_SECRET_ONLINE_DIA_DEUTSCH=$(node scripts/secret.js -s $AES_KEY -e $CTL_SEED_SECRET_ONLINE_DIA_DEUTSCH) + CTL_SEED_SECRET_ONLINE_DIA_MATHE=$(node scripts/secret.js -s $AES_KEY -e $CTL_SEED_SECRET_ONLINE_DIA_MATHE) + CTL_SEED_SECRET_ONLINE_DIA_DEUTSCH=$(node scripts/secret.js -s $AES_KEY -e $CTL_SEED_SECRET_ONLINE_DIA_DEUTSCH) mongosh $DATABASE__URL --quiet --eval 'db.getCollection("external-tools").updateOne( { From 271c09a0cc9e7deef506a56acc45da2da2a59526 Mon Sep 17 00:00:00 2001 From: Mrika Llabani Date: Mon, 7 Oct 2024 17:31:57 +0200 Subject: [PATCH 02/10] N21-2191 refactor encryption --- .../service/external-tool.service.spec.ts | 31 +---- .../service/external-tool.service.ts | 12 +- .../external-tool/uc/external-tool.uc.spec.ts | 114 ++++++++++++++++++ .../tool/external-tool/uc/external-tool.uc.ts | 10 +- 4 files changed, 128 insertions(+), 39 deletions(-) diff --git a/apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts b/apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts index 5e08193da2c..38a14509f9d 100644 --- a/apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts +++ b/apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts @@ -1,5 +1,4 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { DefaultEncryptionService, EncryptionService } from '@infra/encryption'; import { ProviderOauthClient } from '@modules/oauth-provider/domain'; import { UnprocessableEntityException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; @@ -24,7 +23,6 @@ describe(ExternalToolService.name, () => { let oauthProviderService: DeepMocked; let commonToolDeleteService: DeepMocked; let mapper: DeepMocked; - let encryptionService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -42,10 +40,6 @@ describe(ExternalToolService.name, () => { provide: ExternalToolServiceMapper, useValue: createMock(), }, - { - provide: DefaultEncryptionService, - useValue: createMock(), - }, { provide: LegacyLogger, useValue: createMock(), @@ -62,7 +56,6 @@ describe(ExternalToolService.name, () => { oauthProviderService = module.get(OauthProviderService); mapper = module.get(ExternalToolServiceMapper); commonToolDeleteService = module.get(CommonToolDeleteService); - encryptionService = module.get(DefaultEncryptionService); }); afterAll(async () => { @@ -160,29 +153,14 @@ describe(ExternalToolService.name, () => { describe('when lti11 config is set', () => { const setup = () => { - const encryptedSecret = 'encryptedSecret'; const { externalTool, lti11ToolConfig } = createTools(); externalTool.config = lti11ToolConfig; - const lti11ToolConfigDOEncrypted: Lti11ToolConfig = { ...lti11ToolConfig, secret: encryptedSecret }; - const externalToolDOEncrypted: ExternalTool = externalToolFactory.build({ - ...externalTool, - config: lti11ToolConfigDOEncrypted, - }); - encryptionService.encrypt.mockReturnValue(encryptedSecret); - externalToolRepo.save.mockResolvedValue(externalToolDOEncrypted); + externalToolRepo.save.mockResolvedValue(externalTool); - return { externalTool, lti11ToolConfig, encryptedSecret, externalToolDOEncrypted }; + return { externalTool, lti11ToolConfig }; }; - it('should encrypt the secret', async () => { - const { externalTool } = setup(); - - await service.createExternalTool(externalTool); - - expect(encryptionService.encrypt).toHaveBeenCalledWith('secret'); - }); - it('should call the repo to save a tool', async () => { const { externalTool } = setup(); @@ -192,11 +170,11 @@ describe(ExternalToolService.name, () => { }); it('should save DO', async () => { - const { externalTool, externalToolDOEncrypted } = setup(); + const { externalTool } = setup(); const result: ExternalTool = await service.createExternalTool(externalTool); - expect(result).toEqual(externalToolDOEncrypted); + expect(result).toEqual(externalTool); }); }); }); @@ -512,7 +490,6 @@ describe(ExternalToolService.name, () => { describe('updateExternalTool', () => { describe('when external tool with lti11 config is given', () => { const setup = () => { - encryptionService.encrypt.mockReturnValue('newEncryptedSecret'); const changedTool: ExternalTool = externalToolFactory .withLti11Config({ secret: 'newEncryptedSecret' }) .build({ name: 'newName' }); diff --git a/apps/server/src/modules/tool/external-tool/service/external-tool.service.ts b/apps/server/src/modules/tool/external-tool/service/external-tool.service.ts index 784e64c5643..1d320179cd6 100644 --- a/apps/server/src/modules/tool/external-tool/service/external-tool.service.ts +++ b/apps/server/src/modules/tool/external-tool/service/external-tool.service.ts @@ -1,7 +1,6 @@ -import { DefaultEncryptionService, EncryptionService } from '@infra/encryption'; import { ProviderOauthClient } from '@modules/oauth-provider/domain'; import { OauthProviderService } from '@modules/oauth-provider/domain/service/oauth-provider.service'; -import { Inject, Injectable, UnprocessableEntityException } from '@nestjs/common'; +import { Injectable, UnprocessableEntityException } from '@nestjs/common'; import { Page } from '@shared/domain/domainobject'; import { IFindOptions } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; @@ -19,15 +18,12 @@ export class ExternalToolService { private readonly externalToolRepo: ExternalToolRepo, private readonly oauthProviderService: OauthProviderService, private readonly mapper: ExternalToolServiceMapper, - @Inject(DefaultEncryptionService) private readonly encryptionService: EncryptionService, private readonly legacyLogger: LegacyLogger, private readonly commonToolDeleteService: CommonToolDeleteService ) {} public async createExternalTool(externalTool: ExternalTool): Promise { - if (ExternalTool.isLti11Config(externalTool.config) && externalTool.config.secret) { - externalTool.config.secret = this.encryptionService.encrypt(externalTool.config.secret); - } else if (ExternalTool.isOauth2Config(externalTool.config)) { + if (ExternalTool.isOauth2Config(externalTool.config)) { const oauthClient: Partial = this.mapper.mapDoToProviderOauthClient( externalTool.name, externalTool.config @@ -42,10 +38,6 @@ export class ExternalToolService { } public async updateExternalTool(toUpdate: ExternalTool): Promise { - if (ExternalTool.isLti11Config(toUpdate.config) && toUpdate.config.secret) { - toUpdate.config.secret = this.encryptionService.encrypt(toUpdate.config.secret); - } - await this.updateOauth2ToolConfig(toUpdate); const externalTool: ExternalTool = await this.externalToolRepo.save(toUpdate); diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts index b3af5f0e83e..bf7cee3866d 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts @@ -1,5 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Configuration } from '@hpi-schul-cloud/commons/lib'; +import { DefaultEncryptionService, EncryptionService } from '@infra/encryption'; import { ObjectId } from '@mikro-orm/mongodb'; import { Action, AuthorizationService } from '@modules/authorization'; import { School, SchoolService } from '@modules/school'; @@ -22,6 +23,7 @@ import { ExternalToolMetadata, ExternalToolParameterDatasheetTemplateProperty, ExternalToolProps, + Lti11ToolConfig, Oauth2ToolConfig, } from '../domain'; import { @@ -36,6 +38,7 @@ import { externalToolDatasheetTemplateDataFactory, externalToolFactory, fileRecordRefFactory, + lti11ToolConfigFactory, oauth2ToolConfigFactory, } from '../testing'; import { ExternalToolCreate, ExternalToolImportResult, ExternalToolUpdate } from './dto'; @@ -54,6 +57,7 @@ describe(ExternalToolUc.name, () => { let commonToolMetadataService: DeepMocked; let pdfService: DeepMocked; let externalToolImageService: DeepMocked; + let encryptionService: DeepMocked; beforeAll(async () => { await setupEntities(); @@ -99,6 +103,10 @@ describe(ExternalToolUc.name, () => { provide: ExternalToolImageService, useValue: createMock(), }, + { + provide: DefaultEncryptionService, + useValue: createMock(), + }, ], }).compile(); @@ -112,6 +120,7 @@ describe(ExternalToolUc.name, () => { commonToolMetadataService = module.get(CommonToolMetadataService); pdfService = module.get(DatasheetPdfService); externalToolImageService = module.get(ExternalToolImageService); + encryptionService = module.get(DefaultEncryptionService); }); afterAll(async () => { @@ -140,6 +149,7 @@ describe(ExternalToolUc.name, () => { const externalTool: ExternalTool = externalToolFactory.withCustomParameters(1).buildWithId(); const oauth2ConfigWithoutExternalData: Oauth2ToolConfig = oauth2ToolConfigFactory.build(); + const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); const query: ExternalToolSearchQuery = { name: externalTool.name, @@ -171,6 +181,7 @@ describe(ExternalToolUc.name, () => { query, toolId, mockLogoBase64, + lti11ToolConfig, }; }; @@ -334,6 +345,19 @@ describe(ExternalToolUc.name, () => { ); }); }); + + describe('when external tool with lti11 config is given', () => { + it('should call the encryption service', async () => { + const { currentUser } = setupAuthorization(); + const { externalTool, lti11ToolConfig } = setupDefault(); + externalTool.config = lti11ToolConfig; + encryptionService.encrypt.mockReturnValue('encrypted'); + + await uc.createExternalTool(currentUser.userId, externalTool.getProps(), 'jwt'); + + expect(encryptionService.encrypt).toHaveBeenCalledWith('secret'); + }); + }); }); describe('importExternalTools', () => { @@ -643,6 +667,8 @@ describe(ExternalToolUc.name, () => { url: undefined, }); + const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); + externalToolService.updateExternalTool.mockResolvedValue(updatedExternalTool); externalToolService.findById.mockResolvedValue(new ExternalTool(externalToolToUpdate)); @@ -652,6 +678,7 @@ describe(ExternalToolUc.name, () => { externalToolDOtoUpdate: externalToolToUpdate, toolId, mockLogoBase64, + lti11ToolConfig, }; }; @@ -868,6 +895,93 @@ describe(ExternalToolUc.name, () => { ); }); }); + + describe('when lti11 config is given and secret is not encrypted', () => { + const setupLTI = () => { + const { externalTool, toolId, mockLogoBase64 } = setupDefault(); + + const externalToolToUpdate: ExternalToolUpdate = { + ...externalTool.getProps(), + name: 'newName', + url: undefined, + }; + const updatedExternalTool: ExternalTool = externalToolFactory.build({ + ...externalTool.getProps(), + name: 'newName', + url: undefined, + }); + + const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); + externalToolToUpdate.config = lti11ToolConfig; + + const currentTestTool = new ExternalTool({ ...externalToolToUpdate }); + currentTestTool.config = lti11ToolConfigFactory.buildWithId({ secret: 'encryptedSecret' }); + + externalToolService.findById.mockResolvedValue(currentTestTool); + encryptionService.encrypt.mockReturnValue(lti11ToolConfig.secret); + externalToolService.updateExternalTool.mockResolvedValue(updatedExternalTool); + + return { + externalTool, + updatedExternalToolDO: updatedExternalTool, + externalToolDOtoUpdate: externalToolToUpdate, + toolId, + mockLogoBase64, + lti11ToolConfig, + }; + }; + + it('should call encryption service', async () => { + const { currentUser } = setupAuthorization(); + const { toolId, externalToolDOtoUpdate } = setupLTI(); + + await uc.updateExternalTool(currentUser.userId, toolId, externalToolDOtoUpdate, 'jwt'); + + expect(encryptionService.encrypt).toHaveBeenCalled(); + }); + }); + + describe('when lti11 config is given and secret is encrypted', () => { + const setupLTI = () => { + const { externalTool, toolId, mockLogoBase64 } = setupDefault(); + + const externalToolToUpdate: ExternalToolUpdate = { + ...externalTool.getProps(), + name: 'newName', + url: undefined, + }; + const updatedExternalTool: ExternalTool = externalToolFactory.build({ + ...externalTool.getProps(), + name: 'newName', + url: undefined, + }); + + const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); + + externalToolToUpdate.config = lti11ToolConfig; + encryptionService.encrypt.mockReturnValue(lti11ToolConfig.secret); + externalToolService.findById.mockResolvedValue(new ExternalTool(externalToolToUpdate)); + externalToolService.updateExternalTool.mockResolvedValue(updatedExternalTool); + + return { + externalTool, + updatedExternalToolDO: updatedExternalTool, + externalToolDOtoUpdate: externalToolToUpdate, + toolId, + mockLogoBase64, + lti11ToolConfig, + }; + }; + + it('should not call encryption service', async () => { + const { currentUser } = setupAuthorization(); + const { toolId, externalToolDOtoUpdate } = setupLTI(); + + await uc.updateExternalTool(currentUser.userId, toolId, externalToolDOtoUpdate, 'jwt'); + + expect(encryptionService.encrypt).not.toHaveBeenCalled(); + }); + }); }); describe('deleteExternalTool', () => { diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts index bc79c03f090..1f64ebd79b9 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts @@ -1,9 +1,10 @@ +import { DefaultEncryptionService, EncryptionService } from '@infra/encryption'; import { ObjectId } from '@mikro-orm/mongodb'; import { AuthorizationService } from '@modules/authorization'; import { School, SchoolService } from '@modules/school'; import { SchoolExternalTool } from '@modules/tool/school-external-tool/domain'; import { SchoolExternalToolService } from '@modules/tool/school-external-tool/service'; -import { Injectable } from '@nestjs/common'; +import { Inject, Injectable } from '@nestjs/common'; import { Page } from '@shared/domain/domainobject'; import { User } from '@shared/domain/entity'; import { IFindOptions, Permission } from '@shared/domain/interface'; @@ -32,7 +33,8 @@ export class ExternalToolUc { private readonly externalToolLogoService: ExternalToolLogoService, private readonly commonToolMetadataService: CommonToolMetadataService, private readonly datasheetPdfService: DatasheetPdfService, - private readonly externalToolImageService: ExternalToolImageService + private readonly externalToolImageService: ExternalToolImageService, + @Inject(DefaultEncryptionService) private readonly encryptionService: EncryptionService ) {} public async createExternalTool( @@ -42,6 +44,10 @@ export class ExternalToolUc { ): Promise { await this.ensurePermission(userId, Permission.TOOL_ADMIN); + if (ExternalTool.isLti11Config(externalToolCreate.config) && externalToolCreate.config.secret) { + externalToolCreate.config.secret = this.encryptionService.encrypt(externalToolCreate.config.secret); + } + const tool: ExternalTool = await this.validateAndSaveExternalTool(externalToolCreate, jwt); return tool; From 543cd990afb9a4894c7be5da3f0148840df13871 Mon Sep 17 00:00:00 2001 From: Mrika Llabani Date: Mon, 7 Oct 2024 17:31:57 +0200 Subject: [PATCH 03/10] N21-2191 refactor encryption --- .../service/external-tool.service.spec.ts | 31 +---- .../service/external-tool.service.ts | 12 +- .../external-tool/uc/external-tool.uc.spec.ts | 114 ++++++++++++++++++ .../tool/external-tool/uc/external-tool.uc.ts | 18 ++- 4 files changed, 136 insertions(+), 39 deletions(-) diff --git a/apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts b/apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts index 5e08193da2c..38a14509f9d 100644 --- a/apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts +++ b/apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts @@ -1,5 +1,4 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; -import { DefaultEncryptionService, EncryptionService } from '@infra/encryption'; import { ProviderOauthClient } from '@modules/oauth-provider/domain'; import { UnprocessableEntityException } from '@nestjs/common'; import { Test, TestingModule } from '@nestjs/testing'; @@ -24,7 +23,6 @@ describe(ExternalToolService.name, () => { let oauthProviderService: DeepMocked; let commonToolDeleteService: DeepMocked; let mapper: DeepMocked; - let encryptionService: DeepMocked; beforeAll(async () => { module = await Test.createTestingModule({ @@ -42,10 +40,6 @@ describe(ExternalToolService.name, () => { provide: ExternalToolServiceMapper, useValue: createMock(), }, - { - provide: DefaultEncryptionService, - useValue: createMock(), - }, { provide: LegacyLogger, useValue: createMock(), @@ -62,7 +56,6 @@ describe(ExternalToolService.name, () => { oauthProviderService = module.get(OauthProviderService); mapper = module.get(ExternalToolServiceMapper); commonToolDeleteService = module.get(CommonToolDeleteService); - encryptionService = module.get(DefaultEncryptionService); }); afterAll(async () => { @@ -160,29 +153,14 @@ describe(ExternalToolService.name, () => { describe('when lti11 config is set', () => { const setup = () => { - const encryptedSecret = 'encryptedSecret'; const { externalTool, lti11ToolConfig } = createTools(); externalTool.config = lti11ToolConfig; - const lti11ToolConfigDOEncrypted: Lti11ToolConfig = { ...lti11ToolConfig, secret: encryptedSecret }; - const externalToolDOEncrypted: ExternalTool = externalToolFactory.build({ - ...externalTool, - config: lti11ToolConfigDOEncrypted, - }); - encryptionService.encrypt.mockReturnValue(encryptedSecret); - externalToolRepo.save.mockResolvedValue(externalToolDOEncrypted); + externalToolRepo.save.mockResolvedValue(externalTool); - return { externalTool, lti11ToolConfig, encryptedSecret, externalToolDOEncrypted }; + return { externalTool, lti11ToolConfig }; }; - it('should encrypt the secret', async () => { - const { externalTool } = setup(); - - await service.createExternalTool(externalTool); - - expect(encryptionService.encrypt).toHaveBeenCalledWith('secret'); - }); - it('should call the repo to save a tool', async () => { const { externalTool } = setup(); @@ -192,11 +170,11 @@ describe(ExternalToolService.name, () => { }); it('should save DO', async () => { - const { externalTool, externalToolDOEncrypted } = setup(); + const { externalTool } = setup(); const result: ExternalTool = await service.createExternalTool(externalTool); - expect(result).toEqual(externalToolDOEncrypted); + expect(result).toEqual(externalTool); }); }); }); @@ -512,7 +490,6 @@ describe(ExternalToolService.name, () => { describe('updateExternalTool', () => { describe('when external tool with lti11 config is given', () => { const setup = () => { - encryptionService.encrypt.mockReturnValue('newEncryptedSecret'); const changedTool: ExternalTool = externalToolFactory .withLti11Config({ secret: 'newEncryptedSecret' }) .build({ name: 'newName' }); diff --git a/apps/server/src/modules/tool/external-tool/service/external-tool.service.ts b/apps/server/src/modules/tool/external-tool/service/external-tool.service.ts index 784e64c5643..1d320179cd6 100644 --- a/apps/server/src/modules/tool/external-tool/service/external-tool.service.ts +++ b/apps/server/src/modules/tool/external-tool/service/external-tool.service.ts @@ -1,7 +1,6 @@ -import { DefaultEncryptionService, EncryptionService } from '@infra/encryption'; import { ProviderOauthClient } from '@modules/oauth-provider/domain'; import { OauthProviderService } from '@modules/oauth-provider/domain/service/oauth-provider.service'; -import { Inject, Injectable, UnprocessableEntityException } from '@nestjs/common'; +import { Injectable, UnprocessableEntityException } from '@nestjs/common'; import { Page } from '@shared/domain/domainobject'; import { IFindOptions } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; @@ -19,15 +18,12 @@ export class ExternalToolService { private readonly externalToolRepo: ExternalToolRepo, private readonly oauthProviderService: OauthProviderService, private readonly mapper: ExternalToolServiceMapper, - @Inject(DefaultEncryptionService) private readonly encryptionService: EncryptionService, private readonly legacyLogger: LegacyLogger, private readonly commonToolDeleteService: CommonToolDeleteService ) {} public async createExternalTool(externalTool: ExternalTool): Promise { - if (ExternalTool.isLti11Config(externalTool.config) && externalTool.config.secret) { - externalTool.config.secret = this.encryptionService.encrypt(externalTool.config.secret); - } else if (ExternalTool.isOauth2Config(externalTool.config)) { + if (ExternalTool.isOauth2Config(externalTool.config)) { const oauthClient: Partial = this.mapper.mapDoToProviderOauthClient( externalTool.name, externalTool.config @@ -42,10 +38,6 @@ export class ExternalToolService { } public async updateExternalTool(toUpdate: ExternalTool): Promise { - if (ExternalTool.isLti11Config(toUpdate.config) && toUpdate.config.secret) { - toUpdate.config.secret = this.encryptionService.encrypt(toUpdate.config.secret); - } - await this.updateOauth2ToolConfig(toUpdate); const externalTool: ExternalTool = await this.externalToolRepo.save(toUpdate); diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts index b3af5f0e83e..bf7cee3866d 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts @@ -1,5 +1,6 @@ import { createMock, DeepMocked } from '@golevelup/ts-jest'; import { Configuration } from '@hpi-schul-cloud/commons/lib'; +import { DefaultEncryptionService, EncryptionService } from '@infra/encryption'; import { ObjectId } from '@mikro-orm/mongodb'; import { Action, AuthorizationService } from '@modules/authorization'; import { School, SchoolService } from '@modules/school'; @@ -22,6 +23,7 @@ import { ExternalToolMetadata, ExternalToolParameterDatasheetTemplateProperty, ExternalToolProps, + Lti11ToolConfig, Oauth2ToolConfig, } from '../domain'; import { @@ -36,6 +38,7 @@ import { externalToolDatasheetTemplateDataFactory, externalToolFactory, fileRecordRefFactory, + lti11ToolConfigFactory, oauth2ToolConfigFactory, } from '../testing'; import { ExternalToolCreate, ExternalToolImportResult, ExternalToolUpdate } from './dto'; @@ -54,6 +57,7 @@ describe(ExternalToolUc.name, () => { let commonToolMetadataService: DeepMocked; let pdfService: DeepMocked; let externalToolImageService: DeepMocked; + let encryptionService: DeepMocked; beforeAll(async () => { await setupEntities(); @@ -99,6 +103,10 @@ describe(ExternalToolUc.name, () => { provide: ExternalToolImageService, useValue: createMock(), }, + { + provide: DefaultEncryptionService, + useValue: createMock(), + }, ], }).compile(); @@ -112,6 +120,7 @@ describe(ExternalToolUc.name, () => { commonToolMetadataService = module.get(CommonToolMetadataService); pdfService = module.get(DatasheetPdfService); externalToolImageService = module.get(ExternalToolImageService); + encryptionService = module.get(DefaultEncryptionService); }); afterAll(async () => { @@ -140,6 +149,7 @@ describe(ExternalToolUc.name, () => { const externalTool: ExternalTool = externalToolFactory.withCustomParameters(1).buildWithId(); const oauth2ConfigWithoutExternalData: Oauth2ToolConfig = oauth2ToolConfigFactory.build(); + const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); const query: ExternalToolSearchQuery = { name: externalTool.name, @@ -171,6 +181,7 @@ describe(ExternalToolUc.name, () => { query, toolId, mockLogoBase64, + lti11ToolConfig, }; }; @@ -334,6 +345,19 @@ describe(ExternalToolUc.name, () => { ); }); }); + + describe('when external tool with lti11 config is given', () => { + it('should call the encryption service', async () => { + const { currentUser } = setupAuthorization(); + const { externalTool, lti11ToolConfig } = setupDefault(); + externalTool.config = lti11ToolConfig; + encryptionService.encrypt.mockReturnValue('encrypted'); + + await uc.createExternalTool(currentUser.userId, externalTool.getProps(), 'jwt'); + + expect(encryptionService.encrypt).toHaveBeenCalledWith('secret'); + }); + }); }); describe('importExternalTools', () => { @@ -643,6 +667,8 @@ describe(ExternalToolUc.name, () => { url: undefined, }); + const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); + externalToolService.updateExternalTool.mockResolvedValue(updatedExternalTool); externalToolService.findById.mockResolvedValue(new ExternalTool(externalToolToUpdate)); @@ -652,6 +678,7 @@ describe(ExternalToolUc.name, () => { externalToolDOtoUpdate: externalToolToUpdate, toolId, mockLogoBase64, + lti11ToolConfig, }; }; @@ -868,6 +895,93 @@ describe(ExternalToolUc.name, () => { ); }); }); + + describe('when lti11 config is given and secret is not encrypted', () => { + const setupLTI = () => { + const { externalTool, toolId, mockLogoBase64 } = setupDefault(); + + const externalToolToUpdate: ExternalToolUpdate = { + ...externalTool.getProps(), + name: 'newName', + url: undefined, + }; + const updatedExternalTool: ExternalTool = externalToolFactory.build({ + ...externalTool.getProps(), + name: 'newName', + url: undefined, + }); + + const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); + externalToolToUpdate.config = lti11ToolConfig; + + const currentTestTool = new ExternalTool({ ...externalToolToUpdate }); + currentTestTool.config = lti11ToolConfigFactory.buildWithId({ secret: 'encryptedSecret' }); + + externalToolService.findById.mockResolvedValue(currentTestTool); + encryptionService.encrypt.mockReturnValue(lti11ToolConfig.secret); + externalToolService.updateExternalTool.mockResolvedValue(updatedExternalTool); + + return { + externalTool, + updatedExternalToolDO: updatedExternalTool, + externalToolDOtoUpdate: externalToolToUpdate, + toolId, + mockLogoBase64, + lti11ToolConfig, + }; + }; + + it('should call encryption service', async () => { + const { currentUser } = setupAuthorization(); + const { toolId, externalToolDOtoUpdate } = setupLTI(); + + await uc.updateExternalTool(currentUser.userId, toolId, externalToolDOtoUpdate, 'jwt'); + + expect(encryptionService.encrypt).toHaveBeenCalled(); + }); + }); + + describe('when lti11 config is given and secret is encrypted', () => { + const setupLTI = () => { + const { externalTool, toolId, mockLogoBase64 } = setupDefault(); + + const externalToolToUpdate: ExternalToolUpdate = { + ...externalTool.getProps(), + name: 'newName', + url: undefined, + }; + const updatedExternalTool: ExternalTool = externalToolFactory.build({ + ...externalTool.getProps(), + name: 'newName', + url: undefined, + }); + + const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); + + externalToolToUpdate.config = lti11ToolConfig; + encryptionService.encrypt.mockReturnValue(lti11ToolConfig.secret); + externalToolService.findById.mockResolvedValue(new ExternalTool(externalToolToUpdate)); + externalToolService.updateExternalTool.mockResolvedValue(updatedExternalTool); + + return { + externalTool, + updatedExternalToolDO: updatedExternalTool, + externalToolDOtoUpdate: externalToolToUpdate, + toolId, + mockLogoBase64, + lti11ToolConfig, + }; + }; + + it('should not call encryption service', async () => { + const { currentUser } = setupAuthorization(); + const { toolId, externalToolDOtoUpdate } = setupLTI(); + + await uc.updateExternalTool(currentUser.userId, toolId, externalToolDOtoUpdate, 'jwt'); + + expect(encryptionService.encrypt).not.toHaveBeenCalled(); + }); + }); }); describe('deleteExternalTool', () => { diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts index bc79c03f090..c5c86ba2def 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts @@ -1,9 +1,10 @@ +import { DefaultEncryptionService, EncryptionService } from '@infra/encryption'; import { ObjectId } from '@mikro-orm/mongodb'; import { AuthorizationService } from '@modules/authorization'; import { School, SchoolService } from '@modules/school'; import { SchoolExternalTool } from '@modules/tool/school-external-tool/domain'; import { SchoolExternalToolService } from '@modules/tool/school-external-tool/service'; -import { Injectable } from '@nestjs/common'; +import { Inject, Injectable } from '@nestjs/common'; import { Page } from '@shared/domain/domainobject'; import { User } from '@shared/domain/entity'; import { IFindOptions, Permission } from '@shared/domain/interface'; @@ -32,7 +33,8 @@ export class ExternalToolUc { private readonly externalToolLogoService: ExternalToolLogoService, private readonly commonToolMetadataService: CommonToolMetadataService, private readonly datasheetPdfService: DatasheetPdfService, - private readonly externalToolImageService: ExternalToolImageService + private readonly externalToolImageService: ExternalToolImageService, + @Inject(DefaultEncryptionService) private readonly encryptionService: EncryptionService ) {} public async createExternalTool( @@ -42,6 +44,10 @@ export class ExternalToolUc { ): Promise { await this.ensurePermission(userId, Permission.TOOL_ADMIN); + if (ExternalTool.isLti11Config(externalToolCreate.config) && externalToolCreate.config.secret) { + externalToolCreate.config.secret = this.encryptionService.encrypt(externalToolCreate.config.secret); + } + const tool: ExternalTool = await this.validateAndSaveExternalTool(externalToolCreate, jwt); return tool; @@ -125,6 +131,14 @@ export class ExternalToolUc { // Use secrets from existing config const updatedConfigProps: ExternalToolConfig = { ...currentExternalTool.config, ...externalToolUpdateProps.config }; + if ( + ExternalTool.isLti11Config(updatedConfigProps) && + ExternalTool.isLti11Config(currentExternalTool.config) && + updatedConfigProps.secret !== currentExternalTool.config.secret + ) { + updatedConfigProps.secret = this.encryptionService.encrypt(updatedConfigProps.secret); + } + const pendingExternalTool: ExternalTool = new ExternalTool({ ...currentExternalTool.getProps(), ...externalToolUpdateProps, From d136631147979cd0b2e997694f7bd8a2a5b867fb Mon Sep 17 00:00:00 2001 From: Mrika Llabani Date: Tue, 8 Oct 2024 13:23:28 +0200 Subject: [PATCH 04/10] N21-2191 fix module import --- apps/server/src/modules/tool/tool-api.module.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/server/src/modules/tool/tool-api.module.ts b/apps/server/src/modules/tool/tool-api.module.ts index 12d33bd3d90..51eec1332ff 100644 --- a/apps/server/src/modules/tool/tool-api.module.ts +++ b/apps/server/src/modules/tool/tool-api.module.ts @@ -1,3 +1,4 @@ +import { EncryptionModule } from '@infra/encryption'; import { AuthorizationModule } from '@modules/authorization'; import { BoardModule } from '@modules/board'; import { LegacySchoolModule } from '@modules/legacy-school'; @@ -35,6 +36,7 @@ import { ToolModule } from './tool.module'; BoardModule, SchoolModule, UserLicenseModule, + EncryptionModule, ], controllers: [ ToolLaunchController, From 68ab05c28119c35cbeff71f3aae857786fc4a10a Mon Sep 17 00:00:00 2001 From: Mrika Llabani Date: Tue, 8 Oct 2024 15:52:57 +0200 Subject: [PATCH 05/10] N21-2191 encrypt secrets of importTools --- .../external-tool/uc/external-tool.uc.spec.ts | 64 ++++++++++++++++++- .../tool/external-tool/uc/external-tool.uc.ts | 4 ++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts index bf7cee3866d..c24c00a2de7 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts @@ -347,12 +347,20 @@ describe(ExternalToolUc.name, () => { }); describe('when external tool with lti11 config is given', () => { - it('should call the encryption service', async () => { + const setupLTI = () => { const { currentUser } = setupAuthorization(); const { externalTool, lti11ToolConfig } = setupDefault(); externalTool.config = lti11ToolConfig; encryptionService.encrypt.mockReturnValue('encrypted'); + return { + currentUser, + externalTool, + }; + }; + it('should call the encryption service', async () => { + const { currentUser, externalTool } = setupLTI(); + await uc.createExternalTool(currentUser.userId, externalTool.getProps(), 'jwt'); expect(encryptionService.encrypt).toHaveBeenCalledWith('secret'); @@ -406,6 +414,14 @@ describe(ExternalToolUc.name, () => { }; }; + it('should call encryption service', async () => { + const { user, externalTool1, externalTool2 } = setup(); + + await uc.importExternalTools(user.id, [externalTool1.getProps(), externalTool2.getProps()], 'jwt'); + + expect(encryptionService.encrypt).not.toHaveBeenCalled(); + }); + it('should check the users permission', async () => { const { user, externalTool1, externalTool2 } = setup(); @@ -501,6 +517,52 @@ describe(ExternalToolUc.name, () => { }); }); + describe('when importing lti tool', () => { + const setup = () => { + const user = userFactory.buildWithId(); + const externalTool1 = externalToolFactory.build({ + name: 'tool1', + medium: { + mediumId: 'medium1', + mediaSourceId: 'mediumSource1', + }, + }); + + const ltiConfig = lti11ToolConfigFactory.build(); + externalTool1.config = ltiConfig; + + const externalToolCreate1: ExternalToolCreate = { + ...externalTool1.getProps(), + thumbnailUrl: 'https://thumbnail.url1', + }; + + const jwt = 'jwt'; + + authorizationService.getUserWithPermissions.mockResolvedValueOnce(user); + logoService.fetchLogo.mockResolvedValueOnce(undefined); + const thumbnailFileRecordRef = fileRecordRefFactory.build(); + externalToolImageService.uploadImageFileFromUrl.mockResolvedValueOnce(thumbnailFileRecordRef); + externalToolService.createExternalTool.mockResolvedValueOnce(externalTool1); + encryptionService.encrypt.mockReturnValue('encrypted'); + + return { + user, + externalTool1, + externalToolCreate1, + thumbnailFileRecordRef, + jwt, + }; + }; + + it('should call encryption service', async () => { + const { user, externalTool1 } = setup(); + + await uc.importExternalTools(user.id, [externalTool1.getProps()], 'jwt'); + + expect(encryptionService.encrypt).toHaveBeenCalled(); + }); + }); + describe('when an external tools fails the validation', () => { const setup = () => { const user = userFactory.buildWithId(); diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts index c5c86ba2def..fa9846d0f67 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts @@ -70,6 +70,10 @@ export class ExternalToolUc { }); try { + if (ExternalTool.isLti11Config(externalTool.config) && externalTool.config.secret) { + externalTool.config.secret = this.encryptionService.encrypt(externalTool.config.secret); + } + // eslint-disable-next-line no-await-in-loop const savedTool: ExternalTool = await this.validateAndSaveExternalTool(externalTool, jwt); From 4e3f00b8530afb4063dd5669db9be96964069d2e Mon Sep 17 00:00:00 2001 From: Mrika Llabani Date: Wed, 9 Oct 2024 11:04:33 +0200 Subject: [PATCH 06/10] N21-2191 fix --- .../modules/tool/external-tool/uc/external-tool.uc.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts index fa9846d0f67..74c1bb39eb9 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts @@ -135,10 +135,13 @@ export class ExternalToolUc { // Use secrets from existing config const updatedConfigProps: ExternalToolConfig = { ...currentExternalTool.config, ...externalToolUpdateProps.config }; + const { isLti11Config } = ExternalTool; + if ( - ExternalTool.isLti11Config(updatedConfigProps) && - ExternalTool.isLti11Config(currentExternalTool.config) && - updatedConfigProps.secret !== currentExternalTool.config.secret + (isLti11Config(updatedConfigProps) && + isLti11Config(currentExternalTool.config) && + updatedConfigProps.secret !== currentExternalTool.config.secret) || + (!isLti11Config(currentExternalTool.config) && isLti11Config(updatedConfigProps)) ) { updatedConfigProps.secret = this.encryptionService.encrypt(updatedConfigProps.secret); } From 2dc84f11d98319db46534b795b0b4a7be2753bf9 Mon Sep 17 00:00:00 2001 From: Mrika Llabani Date: Wed, 9 Oct 2024 13:20:43 +0200 Subject: [PATCH 07/10] N21-2236 refactor --- .../tool/external-tool/uc/external-tool.uc.ts | 48 ++++++++++++------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts index 74c1bb39eb9..5997ce2a30b 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts @@ -11,7 +11,15 @@ import { IFindOptions, Permission } from '@shared/domain/interface'; import { EntityId } from '@shared/domain/types'; import { ExternalToolSearchQuery } from '../../common/interface'; import { CommonToolMetadataService } from '../../common/service/common-tool-metadata.service'; -import { ExternalTool, ExternalToolConfig, ExternalToolDatasheetTemplateData, ExternalToolMetadata } from '../domain'; +import { + BasicToolConfig, + ExternalTool, + ExternalToolConfig, + ExternalToolDatasheetTemplateData, + ExternalToolMetadata, + Lti11ToolConfig, + Oauth2ToolConfig, +} from '../domain'; import { ExternalToolDatasheetMapper } from '../mapper/external-tool-datasheet.mapper'; import { DatasheetPdfService, @@ -44,9 +52,8 @@ export class ExternalToolUc { ): Promise { await this.ensurePermission(userId, Permission.TOOL_ADMIN); - if (ExternalTool.isLti11Config(externalToolCreate.config) && externalToolCreate.config.secret) { - externalToolCreate.config.secret = this.encryptionService.encrypt(externalToolCreate.config.secret); - } + const updatedConfig = this.encryptLtiSecret(externalToolCreate); + externalToolCreate.config = updatedConfig; const tool: ExternalTool = await this.validateAndSaveExternalTool(externalToolCreate, jwt); @@ -70,9 +77,8 @@ export class ExternalToolUc { }); try { - if (ExternalTool.isLti11Config(externalTool.config) && externalTool.config.secret) { - externalTool.config.secret = this.encryptionService.encrypt(externalTool.config.secret); - } + const updatedConfig = this.encryptLtiSecret(externalTool); + externalTool.config = updatedConfig; // eslint-disable-next-line no-await-in-loop const savedTool: ExternalTool = await this.validateAndSaveExternalTool(externalTool, jwt); @@ -132,20 +138,12 @@ export class ExternalToolUc { const currentExternalTool: ExternalTool = await this.externalToolService.findById(toolId); + const updatedConfig = this.encryptLtiSecret(externalToolUpdate); + externalToolUpdate.config = updatedConfig; + // Use secrets from existing config const updatedConfigProps: ExternalToolConfig = { ...currentExternalTool.config, ...externalToolUpdateProps.config }; - const { isLti11Config } = ExternalTool; - - if ( - (isLti11Config(updatedConfigProps) && - isLti11Config(currentExternalTool.config) && - updatedConfigProps.secret !== currentExternalTool.config.secret) || - (!isLti11Config(currentExternalTool.config) && isLti11Config(updatedConfigProps)) - ) { - updatedConfigProps.secret = this.encryptionService.encrypt(updatedConfigProps.secret); - } - const pendingExternalTool: ExternalTool = new ExternalTool({ ...currentExternalTool.getProps(), ...externalToolUpdateProps, @@ -269,4 +267,18 @@ export class ExternalToolUc { return fileName; } + + private encryptLtiSecret( + externalToolUpdate: ExternalToolCreate | ExternalToolUpdate + ): BasicToolConfig | Lti11ToolConfig | Oauth2ToolConfig { + if (ExternalTool.isLti11Config(externalToolUpdate.config) && externalToolUpdate.config.secret) { + const encrypted = this.encryptionService.encrypt(externalToolUpdate.config.secret); + + const updatedConfig = new Lti11ToolConfig({ ...externalToolUpdate.config, secret: encrypted }); + + return updatedConfig; + } + + return externalToolUpdate.config; + } } From 908bf8dc90da95c974bad842aab86a83021c6694 Mon Sep 17 00:00:00 2001 From: Mrika Llabani Date: Wed, 9 Oct 2024 16:48:34 +0200 Subject: [PATCH 08/10] N21-2236 review changes --- .../external-tool/uc/external-tool.uc.spec.ts | 99 ++++++++++++++----- .../tool/external-tool/uc/external-tool.uc.ts | 21 ++-- 2 files changed, 81 insertions(+), 39 deletions(-) diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts index c24c00a2de7..fc24aac7fff 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts @@ -14,6 +14,7 @@ import { Role, User } from '@shared/domain/entity'; import { IFindOptions, Permission, SortOrder } from '@shared/domain/interface'; import { currentUserFactory, roleFactory, setupEntities, userFactory } from '@shared/testing'; import { CustomParameter } from '../../common/domain'; +import { LtiMessageType, LtiPrivacyPermission, ToolConfigType } from '../../common/enum'; import { ExternalToolSearchQuery } from '../../common/interface'; import { CommonToolMetadataService } from '../../common/service/common-tool-metadata.service'; import { schoolExternalToolFactory } from '../../school-external-tool/testing'; @@ -41,7 +42,7 @@ import { lti11ToolConfigFactory, oauth2ToolConfigFactory, } from '../testing'; -import { ExternalToolCreate, ExternalToolImportResult, ExternalToolUpdate } from './dto'; +import { ExternalToolCreate, ExternalToolImportResult, ExternalToolUpdate, Lti11ToolConfigUpdate } from './dto'; import { ExternalToolUc } from './external-tool.uc'; describe(ExternalToolUc.name, () => { @@ -260,15 +261,6 @@ describe(ExternalToolUc.name, () => { ); }); - it('should return saved a tool', async () => { - const { currentUser } = setupAuthorization(); - const { externalTool } = setupDefault(); - - const result: ExternalTool = await uc.createExternalTool(currentUser.userId, externalTool.getProps(), 'jwt'); - - expect(result).toEqual(externalTool); - }); - describe('when fetching logo', () => { const setup = () => { const user: User = userFactory.buildWithId(); @@ -351,6 +343,7 @@ describe(ExternalToolUc.name, () => { const { currentUser } = setupAuthorization(); const { externalTool, lti11ToolConfig } = setupDefault(); externalTool.config = lti11ToolConfig; + encryptionService.encrypt.mockReturnValue('encrypted'); return { @@ -365,6 +358,22 @@ describe(ExternalToolUc.name, () => { expect(encryptionService.encrypt).toHaveBeenCalledWith('secret'); }); + + it('should call the service to save a tool', async () => { + const { currentUser, externalTool } = setupLTI(); + + await uc.createExternalTool(currentUser.userId, externalTool.getProps(), 'jwt'); + + expect(externalToolService.createExternalTool).toHaveBeenNthCalledWith( + 1, + new ExternalTool({ + ...externalTool.getProps(), + logo: 'base64LogoString', + config: { ...externalTool.config, secret: 'encrypted' }, + id: expect.any(String), + }) + ); + }); }); }); @@ -414,7 +423,7 @@ describe(ExternalToolUc.name, () => { }; }; - it('should call encryption service', async () => { + it('should not call encryption service', async () => { const { user, externalTool1, externalTool2 } = setup(); await uc.importExternalTools(user.id, [externalTool1.getProps(), externalTool2.getProps()], 'jwt'); @@ -962,25 +971,29 @@ describe(ExternalToolUc.name, () => { const setupLTI = () => { const { externalTool, toolId, mockLogoBase64 } = setupDefault(); + const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); const externalToolToUpdate: ExternalToolUpdate = { ...externalTool.getProps(), name: 'newName', + config: lti11ToolConfig, url: undefined, + logo: mockLogoBase64, }; + + const currentTestTool = new ExternalTool({ ...externalToolToUpdate }); + const expectedLtiConfig = lti11ToolConfigFactory.buildWithId({ secret: 'encrypted' }); + currentTestTool.config = expectedLtiConfig; + const updatedExternalTool: ExternalTool = externalToolFactory.build({ ...externalTool.getProps(), name: 'newName', + config: expectedLtiConfig, url: undefined, + logo: mockLogoBase64, }); - const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); - externalToolToUpdate.config = lti11ToolConfig; - - const currentTestTool = new ExternalTool({ ...externalToolToUpdate }); - currentTestTool.config = lti11ToolConfigFactory.buildWithId({ secret: 'encryptedSecret' }); - externalToolService.findById.mockResolvedValue(currentTestTool); - encryptionService.encrypt.mockReturnValue(lti11ToolConfig.secret); + encryptionService.encrypt.mockReturnValue(expectedLtiConfig.secret); externalToolService.updateExternalTool.mockResolvedValue(updatedExternalTool); return { @@ -995,34 +1008,57 @@ describe(ExternalToolUc.name, () => { it('should call encryption service', async () => { const { currentUser } = setupAuthorization(); - const { toolId, externalToolDOtoUpdate } = setupLTI(); + const { toolId, externalToolDOtoUpdate, lti11ToolConfig } = setupLTI(); await uc.updateExternalTool(currentUser.userId, toolId, externalToolDOtoUpdate, 'jwt'); - expect(encryptionService.encrypt).toHaveBeenCalled(); + expect(encryptionService.encrypt).toHaveBeenNthCalledWith(1, lti11ToolConfig.secret); + }); + + it('should call the service to update the tool', async () => { + const { currentUser } = setupAuthorization(); + const { toolId, externalToolDOtoUpdate, updatedExternalToolDO } = setupLTI(); + + await uc.updateExternalTool(currentUser.userId, toolId, externalToolDOtoUpdate, 'jwt'); + + expect(externalToolService.updateExternalTool).toHaveBeenCalledWith(updatedExternalToolDO); }); }); - describe('when lti11 config is given and secret is encrypted', () => { + describe('when lti11 config is given and secret is already encrypted', () => { const setupLTI = () => { const { externalTool, toolId, mockLogoBase64 } = setupDefault(); + const lti11ToolConfig: Lti11ToolConfigUpdate = { + type: ToolConfigType.LTI11, + baseUrl: 'https://www.basic-baseUrl.com/', + key: 'key', + privacy_permission: LtiPrivacyPermission.PSEUDONYMOUS, + lti_message_type: LtiMessageType.BASIC_LTI_LAUNCH_REQUEST, + launch_presentation_locale: 'de-DE', + }; + const externalToolToUpdate: ExternalToolUpdate = { ...externalTool.getProps(), + config: lti11ToolConfig, name: 'newName', url: undefined, }; + const updatedExternalTool: ExternalTool = externalToolFactory.build({ ...externalTool.getProps(), + config: lti11ToolConfigFactory.build({ ...externalTool.config, secret: 'encrypted' }), name: 'newName', url: undefined, + logo: mockLogoBase64, }); - const lti11ToolConfig: Lti11ToolConfig = lti11ToolConfigFactory.build(); - - externalToolToUpdate.config = lti11ToolConfig; - encryptionService.encrypt.mockReturnValue(lti11ToolConfig.secret); - externalToolService.findById.mockResolvedValue(new ExternalTool(externalToolToUpdate)); + externalToolService.findById.mockResolvedValue( + new ExternalTool({ + ...externalToolToUpdate, + config: lti11ToolConfigFactory.build({ ...externalToolToUpdate.config, secret: 'encrypted' }), + }) + ); externalToolService.updateExternalTool.mockResolvedValue(updatedExternalTool); return { @@ -1041,7 +1077,16 @@ describe(ExternalToolUc.name, () => { await uc.updateExternalTool(currentUser.userId, toolId, externalToolDOtoUpdate, 'jwt'); - expect(encryptionService.encrypt).not.toHaveBeenCalled(); + expect(encryptionService.encrypt).not.toHaveBeenCalledWith(); + }); + + it('should call the service to update the tool', async () => { + const { currentUser } = setupAuthorization(); + const { toolId, externalToolDOtoUpdate, updatedExternalToolDO } = setupLTI(); + + await uc.updateExternalTool(currentUser.userId, toolId, externalToolDOtoUpdate, 'jwt'); + + expect(externalToolService.updateExternalTool).toHaveBeenCalledWith(updatedExternalToolDO); }); }); }); diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts index 5997ce2a30b..f9396b3e299 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.ts @@ -52,8 +52,7 @@ export class ExternalToolUc { ): Promise { await this.ensurePermission(userId, Permission.TOOL_ADMIN); - const updatedConfig = this.encryptLtiSecret(externalToolCreate); - externalToolCreate.config = updatedConfig; + externalToolCreate.config = this.encryptLtiSecret(externalToolCreate); const tool: ExternalTool = await this.validateAndSaveExternalTool(externalToolCreate, jwt); @@ -77,8 +76,7 @@ export class ExternalToolUc { }); try { - const updatedConfig = this.encryptLtiSecret(externalTool); - externalTool.config = updatedConfig; + externalTool.config = this.encryptLtiSecret(externalTool); // eslint-disable-next-line no-await-in-loop const savedTool: ExternalTool = await this.validateAndSaveExternalTool(externalTool, jwt); @@ -134,13 +132,12 @@ export class ExternalToolUc { ): Promise { await this.ensurePermission(userId, Permission.TOOL_ADMIN); + externalToolUpdate.config = this.encryptLtiSecret(externalToolUpdate); + const { thumbnailUrl, ...externalToolUpdateProps } = externalToolUpdate; const currentExternalTool: ExternalTool = await this.externalToolService.findById(toolId); - const updatedConfig = this.encryptLtiSecret(externalToolUpdate); - externalToolUpdate.config = updatedConfig; - // Use secrets from existing config const updatedConfigProps: ExternalToolConfig = { ...currentExternalTool.config, ...externalToolUpdateProps.config }; @@ -269,16 +266,16 @@ export class ExternalToolUc { } private encryptLtiSecret( - externalToolUpdate: ExternalToolCreate | ExternalToolUpdate + externalTool: ExternalToolCreate | ExternalToolUpdate ): BasicToolConfig | Lti11ToolConfig | Oauth2ToolConfig { - if (ExternalTool.isLti11Config(externalToolUpdate.config) && externalToolUpdate.config.secret) { - const encrypted = this.encryptionService.encrypt(externalToolUpdate.config.secret); + if (ExternalTool.isLti11Config(externalTool.config) && externalTool.config.secret) { + const encrypted = this.encryptionService.encrypt(externalTool.config.secret); - const updatedConfig = new Lti11ToolConfig({ ...externalToolUpdate.config, secret: encrypted }); + const updatedConfig = new Lti11ToolConfig({ ...externalTool.config, secret: encrypted }); return updatedConfig; } - return externalToolUpdate.config; + return externalTool.config; } } From 976cc3f48b489a9bd6e47182d9e0fed572458985 Mon Sep 17 00:00:00 2001 From: Mrika Llabani Date: Wed, 9 Oct 2024 17:14:21 +0200 Subject: [PATCH 09/10] N21-2236 test --- .../external-tool/uc/external-tool.uc.spec.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts index fc24aac7fff..b483c9e1594 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts @@ -570,6 +570,21 @@ describe(ExternalToolUc.name, () => { expect(encryptionService.encrypt).toHaveBeenCalled(); }); + + it('should save tool', async () => { + const { user, externalTool1 } = setup(); + + await uc.importExternalTools(user.id, [externalTool1.getProps()], 'jwt'); + + expect(externalToolService.createExternalTool).toHaveBeenNthCalledWith( + 1, + new ExternalTool({ + ...externalTool1.getProps(), + config: lti11ToolConfigFactory.build({ ...externalTool1.config, secret: 'encrypted' }), + id: expect.any(String), + }) + ); + }); }); describe('when an external tools fails the validation', () => { From b599da6abd1125b0e76e51bc1c816d5858803d24 Mon Sep 17 00:00:00 2001 From: Mrika Llabani Date: Thu, 10 Oct 2024 09:34:37 +0200 Subject: [PATCH 10/10] N21-2236 test --- .../tool/external-tool/uc/external-tool.uc.spec.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts index b483c9e1594..625d60eee53 100644 --- a/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts +++ b/apps/server/src/modules/tool/external-tool/uc/external-tool.uc.spec.ts @@ -261,6 +261,15 @@ describe(ExternalToolUc.name, () => { ); }); + it('should return saved a tool', async () => { + const { currentUser } = setupAuthorization(); + const { externalTool } = setupDefault(); + + const result: ExternalTool = await uc.createExternalTool(currentUser.userId, externalTool.getProps(), 'jwt'); + + expect(result).toEqual(externalTool); + }); + describe('when fetching logo', () => { const setup = () => { const user: User = userFactory.buildWithId();