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-2191 lti encryption #5274

Merged
merged 8 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,11 @@ data:

# ========== Start of the CTL seed data configuration section.
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)

mongosh $DATABASE__URL --quiet --eval 'db.getCollection("external-tools").updateOne(
{
"name": "Product Test Onlinediagnose Grundschule - Mathematik",
Expand Down
58 changes: 58 additions & 0 deletions apps/server/src/migrations/mikro-orm/Migration20240926205656.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { Migration } from '@mikro-orm/migrations-mongodb';
import CryptoJs from 'crypto-js';

// eslint-disable-next-line no-process-env

export class Migration20240926205656 extends Migration {
async up(): Promise<void> {
// eslint-disable-next-line no-process-env
const { AES_KEY } = process.env;

if (AES_KEY) {
const tools = await this.driver.aggregate('external-tools', [{ $match: { config_type: { $eq: 'lti11' } } }]);

for await (const tool of tools) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
if (tool.config_secret) {
// eslint-disable-next-line no-await-in-loop
await this.driver.nativeUpdate(
'external-tools',
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access
{ _id: tool._id },
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-argument,@typescript-eslint/no-unsafe-member-access
{ $set: { config_secret: CryptoJs.AES.encrypt(tool.config_secret, AES_KEY).toString() } }
);
}
}
console.info(`Encrypt field config_secret with AES_KEY of the svs.`);
} else {
console.info(`FAILED: Encrypt field config_secret with AES_KEY of the svs. REASON: AES KEY is not provided.`);
}
}

async down(): Promise<void> {
// eslint-disable-next-line no-process-env
const { AES_KEY } = process.env;

if (AES_KEY) {
const tools = await this.driver.aggregate('external-tools', [{ $match: { config_type: { $eq: 'lti11' } } }]);

for await (const tool of tools) {
if (tool) {
// eslint-disable-next-line no-await-in-loop
await this.driver.nativeUpdate(
'external-tools',
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access
{ _id: tool._id },
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-argument,@typescript-eslint/no-unsafe-member-access
{ $set: { config_secret: CryptoJs.AES.decrypt(tool.config_secret, AES_KEY).toString(CryptoJs.enc.Utf8) } }
);
}
}

console.info(`Rollback: Encrypt field config_secret with AES_KEY of the svs.`);
} else {
console.info(`FAILED: Encrypt field config_secret with AES_KEY of the svs. REASON: AES KEY is not provided.`);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,27 @@ 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' });

return {
changedTool,
};
};

it('should call externalToolServiceMapper', async () => {
const { changedTool } = setup();

await service.updateExternalTool(changedTool);

expect(externalToolRepo.save).toHaveBeenLastCalledWith(changedTool);
});
});

describe('when external tool with oauthConfig is given', () => {
const setup = () => {
const existingTool: ExternalTool = externalToolFactory.withOauth2Config().buildWithId();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ export class ExternalToolService {
}

public async updateExternalTool(toUpdate: ExternalTool): Promise<ExternalTool> {
// TODO N21-2097 use encryption for secret
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
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createMock, DeepMocked } from '@golevelup/ts-jest';
import { DefaultEncryptionService, EncryptionService } from '@infra/encryption';
import { ObjectId } from '@mikro-orm/mongodb';
import { PseudonymService } from '@modules/pseudonym/service';
import { UserService } from '@modules/user';
Expand Down Expand Up @@ -36,6 +37,7 @@ describe('Lti11ToolLaunchStrategy', () => {
let userService: DeepMocked<UserService>;
let pseudonymService: DeepMocked<PseudonymService>;
let lti11EncryptionService: DeepMocked<Lti11EncryptionService>;
let encryptionService: DeepMocked<EncryptionService>;

beforeAll(async () => {
module = await Test.createTestingModule({
Expand Down Expand Up @@ -77,6 +79,10 @@ describe('Lti11ToolLaunchStrategy', () => {
provide: AutoGroupExternalUuidStrategy,
useValue: createMock<AutoGroupExternalUuidStrategy>(),
},
{
provide: DefaultEncryptionService,
useValue: createMock<EncryptionService>(),
},
],
}).compile();

Expand All @@ -85,6 +91,7 @@ describe('Lti11ToolLaunchStrategy', () => {
userService = module.get(UserService);
pseudonymService = module.get(PseudonymService);
lti11EncryptionService = module.get(Lti11EncryptionService);
encryptionService = module.get(DefaultEncryptionService);
});

afterAll(async () => {
Expand Down Expand Up @@ -134,10 +141,13 @@ describe('Lti11ToolLaunchStrategy', () => {
],
});

const decrypted = 'decryptedSecret';
encryptionService.decrypt.mockReturnValue(decrypted);
userService.findById.mockResolvedValue(user);

return {
data,
decrypted,
user,
mockKey,
mockSecret,
Expand All @@ -148,14 +158,14 @@ describe('Lti11ToolLaunchStrategy', () => {
};

it('should contain lti key and secret without location', async () => {
const { data, mockKey, mockSecret } = setup();
const { data, mockKey, decrypted } = setup();

const result: PropertyData[] = await strategy.buildToolLaunchDataFromConcreteConfig('userId', data);

expect(result).toEqual(
expect.arrayContaining([
new PropertyData({ name: 'key', value: mockKey }),
new PropertyData({ name: 'secret', value: mockSecret }),
new PropertyData({ name: 'secret', value: decrypted }),
])
);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { DefaultEncryptionService, EncryptionService } from '@infra/encryption';
import { ObjectId } from '@mikro-orm/mongodb';
import { PseudonymService } from '@modules/pseudonym/service';
import { UserService } from '@modules/user';
import { Injectable, InternalServerErrorException, UnprocessableEntityException } from '@nestjs/common';
import { Inject, Injectable, InternalServerErrorException, UnprocessableEntityException } from '@nestjs/common';
import { Pseudonym, RoleReference, UserDO } from '@shared/domain/domainobject';
import { RoleName } from '@shared/domain/interface';
import { EntityId } from '@shared/domain/types';
Expand All @@ -28,6 +29,7 @@ export class Lti11ToolLaunchStrategy extends AbstractLaunchStrategy {
private readonly userService: UserService,
private readonly pseudonymService: PseudonymService,
private readonly lti11EncryptionService: Lti11EncryptionService,
@Inject(DefaultEncryptionService) private readonly encryptionService: EncryptionService,
autoSchoolIdStrategy: AutoSchoolIdStrategy,
autoSchoolNumberStrategy: AutoSchoolNumberStrategy,
autoContextIdStrategy: AutoContextIdStrategy,
Expand Down Expand Up @@ -63,10 +65,11 @@ export class Lti11ToolLaunchStrategy extends AbstractLaunchStrategy {
const roleNames: RoleName[] = user.roles.map((roleRef: RoleReference): RoleName => roleRef.name);
const ltiRoles: LtiRole[] = LtiRoleMapper.mapRolesToLtiRoles(roleNames);

const decrypted = this.encryptionService.decrypt(config.secret);

const additionalProperties: PropertyData[] = [
new PropertyData({ name: 'key', value: config.key }),
// TODO N21-2097 use decryption for secret
new PropertyData({ name: 'secret', value: config.secret }),
new PropertyData({ name: 'secret', value: decrypted }),

new PropertyData({ name: 'lti_message_type', value: config.lti_message_type, location: PropertyLocation.BODY }),
new PropertyData({ name: 'lti_version', value: 'LTI-1p0', location: PropertyLocation.BODY }),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { EncryptionModule } from '@infra/encryption';
import { BoardModule } from '@modules/board';
import { LearnroomModule } from '@modules/learnroom';
import { LegacySchoolModule } from '@modules/legacy-school';
Expand Down Expand Up @@ -32,6 +33,7 @@ import { BasicToolLaunchStrategy, Lti11ToolLaunchStrategy, OAuth2ToolLaunchStrat
LearnroomModule,
BoardModule,
GroupModule,
EncryptionModule,
],
providers: [
ToolLaunchService,
Expand Down
Loading
Loading