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-2248 Create LTI 1.1 deep links with external tools #5349

Merged
merged 27 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
00bc3b1
interfaces
MarvinOehlerkingCap Nov 14, 2024
40edeeb
deep link impl
MarvinOehlerkingCap Nov 14, 2024
dbc1177
check oauth signature
MarvinOehlerkingCap Nov 15, 2024
1ddb008
tests pt 1
MarvinOehlerkingCap Nov 18, 2024
92ba0bf
launch strat test
MarvinOehlerkingCap Nov 20, 2024
02618c9
provide repo
MarvinOehlerkingCap Nov 20, 2024
52e320b
add api test
MarvinOehlerkingCap Nov 21, 2024
1e21117
fix tests
MarvinOehlerkingCap Nov 21, 2024
bf6400c
fix tests
MarvinOehlerkingCap Nov 21, 2024
7122ec2
fix tests
MarvinOehlerkingCap Nov 21, 2024
610f558
Merge branch 'main' into N21-2248-merlin-link
MarvinOehlerkingCap Nov 22, 2024
e18944b
add db check to api test
MarvinOehlerkingCap Nov 22, 2024
5e12520
improve coverage
MarvinOehlerkingCap Nov 22, 2024
844f7fc
remove todo
MarvinOehlerkingCap Nov 22, 2024
02e67aa
fix oauth signature validation
MarvinOehlerkingCap Nov 22, 2024
c49eb51
fix oauth signature validation
MarvinOehlerkingCap Nov 22, 2024
4c9f1c5
decrypt secret
MarvinOehlerkingCap Nov 22, 2024
1384ea9
add missing attribute
MarvinOehlerkingCap Nov 22, 2024
442f7c8
set display name
MarvinOehlerkingCap Nov 25, 2024
ec903b0
add tool launch type
MarvinOehlerkingCap Nov 25, 2024
6d19926
fix test
MarvinOehlerkingCap Nov 25, 2024
7262fd4
fix test
MarvinOehlerkingCap Nov 25, 2024
939b91a
remove name validation
MarvinOehlerkingCap Nov 26, 2024
cca5fac
adjust seed data
MBergCap Nov 26, 2024
9420adc
Merge branch 'main' into N21-2248-merlin-link
MBergCap Nov 28, 2024
45b8be2
Merge branch 'main' into N21-2248-merlin-link
MarvinOehlerkingCap Dec 2, 2024
d81689c
Merge branch 'main' into N21-2248-merlin-link
MBergCap Dec 2, 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
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { MediaUserLicenseEntity } from '@modules/user-license/entity';
import { mediaSourceEntityFactory, mediaUserLicenseEntityFactory } from '@modules/user-license/testing';
import { HttpStatus, INestApplication } from '@nestjs/common';
import { Test, TestingModule } from '@nestjs/testing';
import { type DatesToStrings, fileRecordFactory, TestApiClient, UserAndAccountTestFactory } from '@shared/testing';
import { DateToString, fileRecordFactory, TestApiClient, UserAndAccountTestFactory } from '@shared/testing';
import { BoardExternalReferenceType, BoardLayout, MediaBoardColors } from '../../../domain';
import { BoardNodeEntity } from '../../../repo';
import {
Expand Down Expand Up @@ -85,7 +85,7 @@ describe('Media Board (API)', () => {

const response = await studentClient.get('me');

expect(response.body).toEqual<DatesToStrings<MediaBoardResponse>>({
expect(response.body).toEqual<DateToString<MediaBoardResponse>>({
id: mediaBoard.id,
timestamps: {
createdAt: mediaBoard.createdAt.toISOString(),
Expand Down Expand Up @@ -205,7 +205,7 @@ describe('Media Board (API)', () => {

const response = await studentClient.post(`${mediaBoard.id}/media-lines`);

expect(response.body).toEqual<DatesToStrings<MediaLineResponse>>({
expect(response.body).toEqual<DateToString<MediaLineResponse>>({
id: expect.any(String),
timestamps: {
createdAt: expect.any(String),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ describe(BoardNodeCopyService.name, () => {
FILES_STORAGE__SERVICE_BASE_URL: '',
CTL_TOOLS__PREFERRED_TOOLS_LIMIT: 10,
FEATURE_PREFERRED_CTL_TOOLS_ENABLED: false,
PUBLIC_BACKEND_URL: '',
};
let contextExternalToolService: DeepMocked<ContextExternalToolService>;
let copyHelperService: DeepMocked<CopyHelperService>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ describe(OauthProviderController.name, () => {
const setup = async () => {
const { studentAccount, studentUser } = UserAndAccountTestFactory.buildStudent();
const consentListResponse: ProviderConsentSessionResponse = providerConsentSessionResponseFactory.build();
const externalTool = externalToolEntityFactory.withOauth2Config('clientId').buildWithId();
const externalTool = externalToolEntityFactory.withOauth2Config().buildWithId();
const pseudonym = externalToolPseudonymEntityFactory.buildWithId({
toolId: externalTool.id,
userId: studentUser.id,
Expand Down
1 change: 1 addition & 0 deletions apps/server/src/modules/server/admin-api-server.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ const config: AdminApiServerConfig = {
TEACHER_VISIBILITY_FOR_EXTERNAL_TEAM_INVITATION: Configuration.get(
'TEACHER_VISIBILITY_FOR_EXTERNAL_TEAM_INVITATION'
) as string,
PUBLIC_BACKEND_URL: Configuration.get('PUBLIC_BACKEND_URL') as string,
};

export const adminApiServerConfig = () => config;
1 change: 1 addition & 0 deletions apps/server/src/modules/server/server.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ const config: ServerConfig = {
AES_KEY: Configuration.get('AES_KEY') as string,
FEATURE_OAUTH_LOGIN: Configuration.get('FEATURE_OAUTH_LOGIN') as boolean,
FEATURE_EXTERNAL_SYSTEM_LOGOUT_ENABLED: Configuration.get('FEATURE_EXTERNAL_SYSTEM_LOGOUT_ENABLED') as boolean,
PUBLIC_BACKEND_URL: Configuration.get('PUBLIC_BACKEND_URL') as string,
};

export const serverConfig = () => config;
Expand Down
9 changes: 8 additions & 1 deletion apps/server/src/modules/tool/common/common-tool.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import { CqrsModule } from '@nestjs/cqrs';
import { ContextExternalToolRepo, ExternalToolRepo, SchoolExternalToolRepo } from '@shared/repo';
import { LoggerModule } from '@src/core/logger';
import { SchoolModule } from '@src/modules/school';
import { CommonToolDeleteService, CommonToolService, CommonToolValidationService } from './service';
import {
CommonToolDeleteService,
CommonToolService,
CommonToolValidationService,
Lti11EncryptionService,
} from './service';
import { CommonToolMetadataService } from './service/common-tool-metadata.service';

@Module({
Expand All @@ -18,6 +23,7 @@ import { CommonToolMetadataService } from './service/common-tool-metadata.servic
ContextExternalToolRepo,
CommonToolMetadataService,
CommonToolDeleteService,
Lti11EncryptionService,
],
exports: [
CommonToolService,
Expand All @@ -27,6 +33,7 @@ import { CommonToolMetadataService } from './service/common-tool-metadata.servic
ContextExternalToolRepo,
CommonToolMetadataService,
CommonToolDeleteService,
Lti11EncryptionService,
],
})
export class CommonToolModule {}
1 change: 1 addition & 0 deletions apps/server/src/modules/tool/common/service/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from './common-tool.service';
export { CommonToolValidationService, ToolParameterTypeValidationUtil } from './validation';
export { CommonToolDeleteService } from './common-tool-delete.service';
export { Lti11EncryptionService } from './lti11-encryption.service';
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { Test, TestingModule } from '@nestjs/testing';
import { Authorization } from 'oauth-1.0a';
import { Lti11EncryptionService } from './lti11-encryption.service';

describe(Lti11EncryptionService.name, () => {
let module: TestingModule;
let service: Lti11EncryptionService;

beforeAll(async () => {
module = await Test.createTestingModule({
providers: [Lti11EncryptionService],
}).compile();

service = module.get(Lti11EncryptionService);
});

afterAll(async () => {
await module.close();
});

describe('sign', () => {
describe('when signing with OAuth1', () => {
const setup = () => {
const mockKey = 'mockKey';
const mockSecret = 'mockSecret';
const mockUrl = 'https://mockurl.com/';
const testPayload: Record<string, string> = {
param1: 'test1',
};

return {
mockKey,
mockSecret,
mockUrl,
testPayload,
};
};

it('should sign the payload with OAuth1', () => {
const { mockKey, mockSecret, mockUrl, testPayload } = setup();

const result: Authorization = service.sign(mockKey, mockSecret, mockUrl, testPayload);

expect(result).toEqual<Authorization>({
oauth_consumer_key: mockKey,
oauth_nonce: expect.any(String),
oauth_signature: expect.any(String),
oauth_signature_method: 'HMAC-SHA1',
oauth_timestamp: expect.any(Number),
oauth_version: '1.0',
...testPayload,
});
});
});
});

describe('verify', () => {
describe('when the OAuth1 signature is valid', () => {
const setup = () => {
const mockKey = 'mockKey';
const mockSecret = 'mockSecret';
const mockUrl = 'https://mockurl.com/';
const testPayload: Record<string, string> = {
param1: 'test1',
};

const signedPayload: Authorization = service.sign(mockKey, mockSecret, mockUrl, testPayload);

return {
mockKey,
mockSecret,
mockUrl,
testPayload,
signedPayload,
};
};

it('should return true', () => {
const { mockKey, mockSecret, mockUrl, signedPayload } = setup();

const result = service.verify(mockKey, mockSecret, mockUrl, signedPayload);

expect(result).toEqual(true);
});
});

describe('when the OAuth1 signature is invalid', () => {
const setup = () => {
const mockKey = 'mockKey';
const mockSecret = 'mockSecret';
const mockUrl = 'https://mockurl.com/';
const testPayload: Record<string, string> = {
param1: 'test1',
};

const signedPayload: Authorization = service.sign(mockKey, mockSecret, mockUrl, testPayload);
const tamperedPayload = { ...signedPayload, param1: 'test2' };

return {
mockKey,
mockSecret,
mockUrl,
testPayload,
tamperedPayload,
};
};

it('should return false', () => {
const { mockKey, mockSecret, mockUrl, tamperedPayload } = setup();

const result = service.verify(mockKey, mockSecret, mockUrl, tamperedPayload);

expect(result).toEqual(false);
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import OAuth, { Authorization, RequestOptions } from 'oauth-1.0a';

@Injectable()
export class Lti11EncryptionService {
public sign(key: string, secret: string, url: string, payload: Record<string, string>): Authorization {
public sign(key: string, secret: string, url: string, payload: unknown): Authorization {
const requestData: RequestOptions = {
url,
method: 'POST',
Expand All @@ -25,4 +25,15 @@ export class Lti11EncryptionService {

return authorization;
}

public verify(key: string, secret: string, url: string, payload: Authorization): boolean {
// eslint-disable-next-line @typescript-eslint/naming-convention
const { oauth_signature, ...validationPayload } = payload;

const authorization: Authorization = this.sign(key, secret, url, validationPayload);

const isValid = oauth_signature === authorization.oauth_signature;

return isValid;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,16 @@ import { CommonToolModule } from '../common';
import { ExternalToolModule } from '../external-tool';
import { SchoolExternalToolModule } from '../school-external-tool';
import { ContextExternalToolRule } from './authorisation/context-external-tool.rule';
import { ContextExternalToolAuthorizableService, ContextExternalToolService, ToolReferenceService } from './service';
import { LTI_DEEP_LINK_TOKEN_REPO, LtiDeepLinkTokenMikroOrmRepo } from './repo';
import {
ContextExternalToolAuthorizableService,
ContextExternalToolService,
LtiDeepLinkingService,
LtiDeepLinkTokenService,
ToolConfigurationStatusService,
ToolReferenceService,
} from './service';
import { ContextExternalToolValidationService } from './service/context-external-tool-validation.service';
import { ToolConfigurationStatusService } from './service/tool-configuration-status.service';

@Module({
imports: [
Expand All @@ -26,12 +33,20 @@ import { ToolConfigurationStatusService } from './service/tool-configuration-sta
ToolReferenceService,
ToolConfigurationStatusService,
ContextExternalToolRule,
LtiDeepLinkTokenService,
LtiDeepLinkingService,
{
provide: LTI_DEEP_LINK_TOKEN_REPO,
useClass: LtiDeepLinkTokenMikroOrmRepo,
},
],
exports: [
ContextExternalToolService,
ContextExternalToolValidationService,
ToolReferenceService,
ToolConfigurationStatusService,
LtiDeepLinkTokenService,
LtiDeepLinkingService,
],
})
export class ContextExternalToolModule {}
Loading
Loading