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

feat(core): sign hook payload data #3854

Merged
merged 10 commits into from
May 23, 2023
14 changes: 12 additions & 2 deletions packages/core/src/libraries/hook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { got } from 'got';
import type { Interaction } from './hook.js';

const { jest } = import.meta;
const { mockEsmWithActual } = createMockUtils(jest);
const { mockEsmWithActual, mockEsm } = createMockUtils(jest);

const nanoIdMock = 'mockId';
await mockEsmWithActual('@logto/shared', () => ({
Expand All @@ -15,6 +15,12 @@ await mockEsmWithActual('@logto/shared', () => ({
generateStandardId: () => nanoIdMock,
}));

const mockSignature = 'mockSignature';
mockEsm('#src/utils/sign.js', () => ({
sign: () => mockSignature,
signAsync: jest.fn().mockResolvedValue(mockSignature),
}));

const { MockQueries } = await import('#src/test-utils/tenant.js');

const url = 'https://logto.gg';
Expand Down Expand Up @@ -78,7 +84,11 @@ describe('triggerInteractionHooksIfNeeded()', () => {

expect(findAllHooks).toHaveBeenCalled();
expect(post).toHaveBeenCalledWith(url, {
headers: { 'user-agent': 'Logto (https://logto.io)', bar: 'baz' },
headers: {
'user-agent': 'Logto (https://logto.io/)',
bar: 'baz',
'logto-signature-256': mockSignature,
},
json: {
hookId: 'foo',
event: 'PostSignIn',
Expand Down
13 changes: 10 additions & 3 deletions packages/core/src/libraries/hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ import {
} from '@logto/schemas';
import { generateStandardId } from '@logto/shared';
import { conditional, pick, trySafe } from '@silverhand/essentials';
import type { Response } from 'got';
import { got, HTTPError } from 'got';
import { type Response } from 'got';
import type Provider from 'oidc-provider';

import { LogEntry } from '#src/middleware/koa-audit-log.js';
import type Queries from '#src/tenants/Queries.js';
import { consoleLog } from '#src/utils/console.js';
import { signAsync } from '#src/utils/sign.js';

const parseResponse = ({ statusCode, body }: Response) => ({
statusCode,
Expand Down Expand Up @@ -81,7 +82,7 @@ export const createHookLibrary = (queries: Queries) => {
} satisfies Omit<HookEventPayload, 'hookId'>;

await Promise.all(
rows.map(async ({ config: { url, headers, retries }, id }) => {
rows.map(async ({ config: { url, headers, retries }, id, signingKey }) => {
consoleLog.info(`\tTriggering hook ${id} due to ${hookEvent} event`);
const json: HookEventPayload = { hookId: id, ...payload };
const logEntry = new LogEntry(`TriggerHook.${hookEvent}`);
Expand All @@ -91,7 +92,13 @@ export const createHookLibrary = (queries: Queries) => {
// Trigger web hook and log response
await got
.post(url, {
headers: { 'user-agent': 'Logto (https://logto.io)', ...headers },
headers: {
'user-agent': 'Logto (https://logto.io/)',
xiaoyijun marked this conversation as resolved.
Show resolved Hide resolved
...headers,
...conditional(
signingKey && { 'logto-signature-256': await signAsync(signingKey, json) }
),
},
json,
retry: { limit: retries ?? 3 },
timeout: { request: 10_000 },
Expand Down
31 changes: 31 additions & 0 deletions packages/core/src/utils/sign.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { sign, signAsync } from './sign.js';

describe('sign', () => {
it('should generate correct signature with both async and sync version', async () => {
const signingKey = 'foo';
const payload = {
bar: 'bar',
foo: 'foo',
};

const signature = sign(signingKey, payload);
const signatureByAsync = await signAsync(signingKey, payload);
const expectedResult =
'sha256=436958f1dbfefab37712fb3927760490fbf7757da8c0b2306ee7b485f0360eee';

expect(signature).toBe(expectedResult);
expect(signatureByAsync).toBe(expectedResult);
});

it('should generate correct signature if payload is empty with both async and sync version', async () => {
const signingKey = 'foo';
const payload = {};
const signature = sign(signingKey, payload);
const signatureByAsync = await signAsync(signingKey, payload);
const expectedResult =
'sha256=c76356efa19d219d1d7e08ccb20b1d26db53b143156f406c99dcb8e0876d6c55';

expect(signature).toBe(expectedResult);
expect(signatureByAsync).toBe(expectedResult);
});
});
14 changes: 14 additions & 0 deletions packages/core/src/utils/sign.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { createHmac } from 'node:crypto';
import { promisify } from 'node:util';

export const sign = (signingKey: string, payload: Record<string, unknown>): string => {
const hmac = createHmac('sha256', signingKey);
const payloadString = JSON.stringify(payload);
hmac.update(payloadString);
return `sha256=${hmac.digest('hex')}`;
};

export const signAsync = async (signingKey: string, payload: Record<string, unknown>) =>
promisify<string>((callback) => {
callback(null, sign(signingKey, payload));
})();
xiaoyijun marked this conversation as resolved.
Show resolved Hide resolved
16 changes: 9 additions & 7 deletions packages/integration-tests/src/helpers/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import fs from 'node:fs/promises';
import { createServer } from 'node:http';
import { createServer, type RequestListener } from 'node:http';
import path from 'node:path';

import { mockSmsVerificationCodeFileName } from '@logto/connector-kit';
Expand Down Expand Up @@ -87,12 +87,14 @@ export const expectRequestError = (error: unknown, code: string, messageIncludes
}
};

export const createMockServer = (port: number) => {
const server = createServer((request, response) => {
// eslint-disable-next-line @silverhand/fp/no-mutation
response.statusCode = 204;
response.end();
});
const defaultRequestListener: RequestListener = (request, response) => {
// eslint-disable-next-line @silverhand/fp/no-mutation
response.statusCode = 204;
response.end();
};

export const createMockServer = (port: number, requestListener?: RequestListener) => {
const server = createServer(requestListener ?? defaultRequestListener);

return {
listen: async () =>
Expand Down
79 changes: 78 additions & 1 deletion packages/integration-tests/src/tests/api/hooks.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { createHmac } from 'node:crypto';
import { type RequestListener } from 'node:http';

import type { Hook, HookConfig, Log, LogKey } from '@logto/schemas';
import { HookEvent, SignInIdentifier, LogResult, InteractionEvent } from '@logto/schemas';

Expand All @@ -22,8 +25,35 @@ const createPayload = (event: HookEvent, url = 'not_work_url'): CreateHookPayloa
},
});

type HookSecureData = {
signature: string;
payload: string;
};

// Note: return hook payload and signature for webhook security testing
const hookServerRequestListener: RequestListener = (request, response) => {
// eslint-disable-next-line @silverhand/fp/no-mutation
response.statusCode = 204;

const data: Buffer[] = [];
request.on('data', (chunk: Buffer) => {
// eslint-disable-next-line @silverhand/fp/no-mutating-methods
data.push(chunk);
});

request.on('end', () => {
response.writeHead(200, { 'Content-Type': 'application/json' });
const payload = Buffer.concat(data).toString();
response.end(
JSON.stringify({
signature: request.headers['logto-signature-256'] as string,
payload,
} satisfies HookSecureData)
);
});
};
describe('hooks', () => {
const { listen, close } = createMockServer(9999);
const { listen, close } = createMockServer(9999, hookServerRequestListener);

beforeAll(async () => {
await enableAllPasswordSignInMethods({
Expand Down Expand Up @@ -259,4 +289,51 @@ describe('hooks', () => {

await deleteUser(id);
});

it('should secure webhook payload data successfully', async () => {
const createdHook = await authedAdminApi
.post('hooks', { json: createPayload(HookEvent.PostRegister, 'http://localhost:9999') })
.json<Hook>();

// Init session and submit
const { username, password } = generateNewUserProfile({ username: true, password: true });
const client = await initClient();
await client.send(putInteraction, {
event: InteractionEvent.Register,
profile: {
username,
password,
},
});
const { redirectTo } = await client.submitInteraction();
const id = await processSession(client, redirectTo);
await waitFor(500); // Wait for hooks execution

const logs = await authedAdminApi
.get(`hooks/${createdHook.id}/recent-logs?page_size=100`)
.json<Log[]>();

const log = logs.find(({ payload: { hookId } }) => hookId === createdHook.id);
expect(log).toBeTruthy();

const response = log?.payload.response;
expect(response).toBeTruthy();

const {
body: { signature, payload },
} = response as { body: HookSecureData };

expect(signature).toBeTruthy();
expect(payload).toBeTruthy();

const calculateSignature = `sha256=${createHmac('sha256', createdHook.signingKey)
.update(payload)
.digest('hex')}`;

expect(calculateSignature).toEqual(signature);

await authedAdminApi.delete(`hooks/${createdHook.id}`);

await deleteUser(id);
});
});