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.

Sign up for GitHub

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-2236 fix lti encryption #5281

Merged
merged 14 commits into from
Oct 10, 2024
Merged

N21-2236 fix lti encryption #5281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
d7426fa
N21-2191 fix configmap
mrikallab Oct 7, 2024
271c09a
N21-2191 refactor encryption
mrikallab Oct 7, 2024
543cd99
N21-2191 refactor encryption
mrikallab Oct 7, 2024
f53b39c
Merge remote-tracking branch 'origin/N21-2191-fix-lti-encryption' int…
mrikallab Oct 7, 2024
d136631
N21-2191 fix module import
mrikallab Oct 8, 2024
68ab05c
N21-2191 encrypt secrets of importTools
mrikallab Oct 8, 2024
0d5ce48
Merge branch 'refs/heads/main' into N21-2191-fix-lti-encryption
mrikallab Oct 8, 2024
4e3f00b
N21-2191 fix
mrikallab Oct 9, 2024
7be01c6
Merge branch 'refs/heads/main' into N21-2191-fix-lti-encryption
mrikallab Oct 9, 2024
2dc84f1
N21-2236 refactor
mrikallab Oct 9, 2024
908bf8d
N21-2236 review changes
mrikallab Oct 9, 2024
976cc3f
N21-2236 test
mrikallab Oct 9, 2024
b599da6
N21-2236 test
mrikallab Oct 10, 2024
3b39cbd
Merge branch 'refs/heads/main' into N21-2236-fix-lti-encryption
mrikallab Oct 10, 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
4 changes: 2 additions & 2 deletions ansible/roles/schulcloud-server-init/templates/configmap_file_init.yml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
{
Expand Down
31 changes: 4 additions & 27 deletions apps/server/src/modules/tool/external-tool/service/external-tool.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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({
Expand All @@ -42,10 +40,6 @@ describe(ExternalToolService.name, () => {
provide: ExternalToolServiceMapper,
useValue: createMock<ExternalToolServiceMapper>(),
},
{
provide: DefaultEncryptionService,
useValue: createMock<EncryptionService>(),
},
{
provide: LegacyLogger,
useValue: createMock<LegacyLogger>(),
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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();

Expand All @@ -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);
});
});
});
Expand Down Expand Up @@ -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' });
Expand Down
12 changes: 2 additions & 10 deletions apps/server/src/modules/tool/external-tool/service/external-tool.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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
Expand All @@ -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);
Expand Down
Loading
Loading