N21-2236 fix lti encryption (#5281)

hpi-schul-cloud · Oct 10, 2024 · 943220f · 943220f
1 parent 691acab
commit 943220f
Show file tree

Hide file tree

Showing 6 changed files with 289 additions and 43 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
@@ -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(
            {

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
@@ -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<OauthProviderService>;
 	let commonToolDeleteService: DeepMocked<CommonToolDeleteService>;
 	let mapper: DeepMocked<ExternalToolServiceMapper>;
-	let encryptionService: DeepMocked<EncryptionService>;
 
 	beforeAll(async () => {
 		module = await Test.createTestingModule({
@@ -42,10 +40,6 @@ describe(ExternalToolService.name, () => {
 					provide: ExternalToolServiceMapper,
 					useValue: createMock<ExternalToolServiceMapper>(),
 				},
-				{
-					provide: DefaultEncryptionService,
-					useValue: createMock<EncryptionService>(),
-				},
 				{
 					provide: LegacyLogger,
 					useValue: createMock<LegacyLogger>(),
@@ -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
@@ -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<ExternalTool> {
-		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<ProviderOauthClient> = this.mapper.mapDoToProviderOauthClient(
 				externalTool.name,
 				externalTool.config
@@ -42,10 +38,6 @@ export class ExternalToolService {
 	}
 
 	public async updateExternalTool(toUpdate: ExternalTool): Promise<ExternalTool> {
-		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);