Skip to content

Commit

Permalink
N21-2191 lti encryption (#5274)
Browse files Browse the repository at this point in the history
  • Loading branch information
mrikallab authored Oct 7, 2024
1 parent a82af02 commit 5be1238
Show file tree
Hide file tree
Showing 10 changed files with 454 additions and 468 deletions.
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

0 comments on commit 5be1238

Please sign in to comment.