From 2496b57fef6e5f7427774ebd85702d81706f7080 Mon Sep 17 00:00:00 2001 From: Gao Sun Date: Sun, 18 Dec 2022 12:11:00 +0800 Subject: [PATCH] refactor: minor improvements --- .../core/src/event-listeners/grant.test.ts | 2 +- .../src/event-listeners/interaction.test.ts | 2 +- .../core/src/middleware/koa-audit-log.test.ts | 46 ++++++++++++++++++- .../actions/submit-interaction.test.ts | 2 +- .../core/src/routes/interaction/index.test.ts | 2 +- .../utils/passcode-validation.test.ts | 2 +- .../interaction/utils/passcode-validation.ts | 4 +- .../utils/social-verification.test.ts | 2 +- .../identifier-payload-verification.test.ts | 2 +- ...ofile-verification-forgot-password.test.ts | 2 +- ...profile-verification-profile-exist.test.ts | 2 +- ...le-verification-profile-registered.test.ts | 2 +- ...-verification-protected-identifier.test.ts | 2 +- .../user-identity-verification.test.ts | 2 +- .../{koa-log.ts => koa-audit-log.ts} | 0 packages/integration-tests/package.json | 3 +- .../integration-tests/src/client/index.ts | 17 +++++++ .../src/tests/api/audit-logs/index.test.ts | 46 ++++++++----------- packages/schemas/src/types/log/interaction.ts | 7 +-- 19 files changed, 97 insertions(+), 50 deletions(-) rename packages/core/src/test-utils/{koa-log.ts => koa-audit-log.ts} (100%) diff --git a/packages/core/src/event-listeners/grant.test.ts b/packages/core/src/event-listeners/grant.test.ts index 740d2506c..70eb00725 100644 --- a/packages/core/src/event-listeners/grant.test.ts +++ b/packages/core/src/event-listeners/grant.test.ts @@ -1,7 +1,7 @@ import type { LogKey } from '@logto/schemas'; import { LogResult, token } from '@logto/schemas'; -import { createMockLogContext } from '#src/test-utils/koa-log.js'; +import { createMockLogContext } from '#src/test-utils/koa-audit-log.js'; import { stringifyError } from '#src/utils/format.js'; import { createContextWithRouteParameters } from '#src/utils/test-utils.js'; diff --git a/packages/core/src/event-listeners/interaction.test.ts b/packages/core/src/event-listeners/interaction.test.ts index c94690494..2277eb037 100644 --- a/packages/core/src/event-listeners/interaction.test.ts +++ b/packages/core/src/event-listeners/interaction.test.ts @@ -1,7 +1,7 @@ import type { LogKey } from '@logto/schemas'; import type { PromptDetail } from 'oidc-provider'; -import { createMockLogContext } from '#src/test-utils/koa-log.js'; +import { createMockLogContext } from '#src/test-utils/koa-audit-log.js'; import { createContextWithRouteParameters } from '#src/utils/test-utils.js'; import { interactionEndedListener, interactionStartedListener } from './interaction.js'; diff --git a/packages/core/src/middleware/koa-audit-log.test.ts b/packages/core/src/middleware/koa-audit-log.test.ts index eb78164fe..7e76a6cb9 100644 --- a/packages/core/src/middleware/koa-audit-log.test.ts +++ b/packages/core/src/middleware/koa-audit-log.test.ts @@ -22,7 +22,6 @@ mockEsm('nanoid', () => ({ const koaLog = await pickDefault(import('./koa-audit-log.js')); -// TODO: test with multiple logs describe('koaAuditLog middleware', () => { const logKey: LogKey = 'Interaction.SignIn.Identifier.VerificationCode.Submit'; const mockPayload: LogPayload = { @@ -67,6 +66,46 @@ describe('koaAuditLog middleware', () => { }); }); + it('should insert multiple success logs when needed', async () => { + // @ts-expect-error for testing + const ctx: WithLogContext> = { + ...createContextWithRouteParameters({ headers: { 'user-agent': userAgent } }), + }; + ctx.request.ip = ip; + const additionalMockPayload: LogPayload = { foo: 'bar' }; + + const next = async () => { + const log = ctx.createLog(logKey); + log.append(mockPayload); + log.append(additionalMockPayload); + const log2 = ctx.createLog(logKey); + log2.append(mockPayload); + }; + await koaLog()(ctx, next); + + const basePayload = { + ...mockPayload, + key: logKey, + result: LogResult.Success, + ip, + userAgent, + }; + + expect(insertLog).toHaveBeenCalledWith({ + id: nanoIdMock, + type: logKey, + payload: basePayload, + }); + expect(insertLog).toHaveBeenCalledWith({ + id: nanoIdMock, + type: logKey, + payload: { + ...basePayload, + ...additionalMockPayload, + }, + }); + }); + it('should not log when there is no log type', async () => { // @ts-expect-error for testing const ctx: WithLogContext> = { @@ -112,7 +151,7 @@ describe('koaAuditLog middleware', () => { }); }); - it('should insert an error log with the error body when next() throws a RequestError', async () => { + it('should update all logs with error result when next() throws a RequestError', async () => { // @ts-expect-error for testing const ctx: WithLogContext> = { ...createContextWithRouteParameters({ headers: { 'user-agent': userAgent } }), @@ -128,10 +167,13 @@ describe('koaAuditLog middleware', () => { const next = async () => { const log = ctx.createLog(logKey); log.append(mockPayload); + const log2 = ctx.createLog(logKey); + log2.append(mockPayload); throw error; }; await expect(koaLog()(ctx, next)).rejects.toMatchError(error); + expect(insertLog).toHaveBeenCalledTimes(2); expect(insertLog).toBeCalledWith({ id: nanoIdMock, type: logKey, diff --git a/packages/core/src/routes/interaction/actions/submit-interaction.test.ts b/packages/core/src/routes/interaction/actions/submit-interaction.test.ts index e3994823e..44f60ce16 100644 --- a/packages/core/src/routes/interaction/actions/submit-interaction.test.ts +++ b/packages/core/src/routes/interaction/actions/submit-interaction.test.ts @@ -1,7 +1,7 @@ import { Event } from '@logto/schemas'; import { mockEsm, pickDefault } from '@logto/shared/esm'; -import { createMockLogContext } from '#src/test-utils/koa-log.js'; +import { createMockLogContext } from '#src/test-utils/koa-audit-log.js'; import { createMockProvider } from '#src/test-utils/oidc-provider.js'; import { createContextWithRouteParameters } from '#src/utils/test-utils.js'; diff --git a/packages/core/src/routes/interaction/index.test.ts b/packages/core/src/routes/interaction/index.test.ts index 8df4aaa0e..cb073317f 100644 --- a/packages/core/src/routes/interaction/index.test.ts +++ b/packages/core/src/routes/interaction/index.test.ts @@ -6,7 +6,7 @@ import { mockEsm, mockEsmDefault, mockEsmWithActual, pickDefault } from '@logto/ import { mockSignInExperience } from '#src/__mocks__/sign-in-experience.js'; import RequestError from '#src/errors/RequestError/index.js'; import type koaAuditLog from '#src/middleware/koa-audit-log.js'; -import { createMockLogContext } from '#src/test-utils/koa-log.js'; +import { createMockLogContext } from '#src/test-utils/koa-audit-log.js'; import { createMockProvider } from '#src/test-utils/oidc-provider.js'; import { createRequester } from '#src/utils/test-utils.js'; diff --git a/packages/core/src/routes/interaction/utils/passcode-validation.test.ts b/packages/core/src/routes/interaction/utils/passcode-validation.test.ts index 40499ae32..a6d7d65db 100644 --- a/packages/core/src/routes/interaction/utils/passcode-validation.test.ts +++ b/packages/core/src/routes/interaction/utils/passcode-validation.test.ts @@ -1,7 +1,7 @@ import { PasscodeType, Event } from '@logto/schemas'; import { mockEsmWithActual } from '@logto/shared/esm'; -import { createMockLogContext } from '#src/test-utils/koa-log.js'; +import { createMockLogContext } from '#src/test-utils/koa-audit-log.js'; import type { SendPasscodePayload } from '../types/index.js'; diff --git a/packages/core/src/routes/interaction/utils/passcode-validation.ts b/packages/core/src/routes/interaction/utils/passcode-validation.ts index ffa09307b..d9a31fc9f 100644 --- a/packages/core/src/routes/interaction/utils/passcode-validation.ts +++ b/packages/core/src/routes/interaction/utils/passcode-validation.ts @@ -43,7 +43,9 @@ export const verifyIdentifierByPasscode = async ( const { event, passcode, ...identifier } = payload; const passcodeType = getPasscodeTypeByEvent(event); - // TODO: @simeng append more log content? + // TODO: @Simeng maybe we should just log all interaction payload in every request? const log = createLog(`Interaction.${event}.Identifier.VerificationCode.Submit`); + log.append(identifier); + await verifyPasscode(jti, passcodeType, passcode, identifier); }; diff --git a/packages/core/src/routes/interaction/utils/social-verification.test.ts b/packages/core/src/routes/interaction/utils/social-verification.test.ts index 0f74667f3..910f17849 100644 --- a/packages/core/src/routes/interaction/utils/social-verification.test.ts +++ b/packages/core/src/routes/interaction/utils/social-verification.test.ts @@ -1,7 +1,7 @@ import { ConnectorType } from '@logto/connector-kit'; import { mockEsm } from '@logto/shared/esm'; -import { createMockLogContext } from '#src/test-utils/koa-log.js'; +import { createMockLogContext } from '#src/test-utils/koa-audit-log.js'; const { jest } = import.meta; diff --git a/packages/core/src/routes/interaction/verifications/identifier-payload-verification.test.ts b/packages/core/src/routes/interaction/verifications/identifier-payload-verification.test.ts index 97ef5094e..8c373077f 100644 --- a/packages/core/src/routes/interaction/verifications/identifier-payload-verification.test.ts +++ b/packages/core/src/routes/interaction/verifications/identifier-payload-verification.test.ts @@ -2,7 +2,7 @@ import { Event } from '@logto/schemas'; import { mockEsm, mockEsmDefault, mockEsmWithActual, pickDefault } from '@logto/shared/esm'; import RequestError from '#src/errors/RequestError/index.js'; -import { createMockLogContext } from '#src/test-utils/koa-log.js'; +import { createMockLogContext } from '#src/test-utils/koa-audit-log.js'; import { createMockProvider } from '#src/test-utils/oidc-provider.js'; import { createContextWithRouteParameters } from '#src/utils/test-utils.js'; diff --git a/packages/core/src/routes/interaction/verifications/profile-verification-forgot-password.test.ts b/packages/core/src/routes/interaction/verifications/profile-verification-forgot-password.test.ts index f23fd90a7..867c39881 100644 --- a/packages/core/src/routes/interaction/verifications/profile-verification-forgot-password.test.ts +++ b/packages/core/src/routes/interaction/verifications/profile-verification-forgot-password.test.ts @@ -2,7 +2,7 @@ import { Event } from '@logto/schemas'; import { mockEsm, mockEsmWithActual, pickDefault } from '@logto/shared/esm'; import RequestError from '#src/errors/RequestError/index.js'; -import { createMockLogContext } from '#src/test-utils/koa-log.js'; +import { createMockLogContext } from '#src/test-utils/koa-audit-log.js'; import { createMockProvider } from '#src/test-utils/oidc-provider.js'; import { createContextWithRouteParameters } from '#src/utils/test-utils.js'; diff --git a/packages/core/src/routes/interaction/verifications/profile-verification-profile-exist.test.ts b/packages/core/src/routes/interaction/verifications/profile-verification-profile-exist.test.ts index afd947782..2782ca4b5 100644 --- a/packages/core/src/routes/interaction/verifications/profile-verification-profile-exist.test.ts +++ b/packages/core/src/routes/interaction/verifications/profile-verification-profile-exist.test.ts @@ -2,7 +2,7 @@ import { Event } from '@logto/schemas'; import { mockEsm, mockEsmWithActual, pickDefault } from '@logto/shared/esm'; import RequestError from '#src/errors/RequestError/index.js'; -import { createMockLogContext } from '#src/test-utils/koa-log.js'; +import { createMockLogContext } from '#src/test-utils/koa-audit-log.js'; import { createMockProvider } from '#src/test-utils/oidc-provider.js'; import { createContextWithRouteParameters } from '#src/utils/test-utils.js'; diff --git a/packages/core/src/routes/interaction/verifications/profile-verification-profile-registered.test.ts b/packages/core/src/routes/interaction/verifications/profile-verification-profile-registered.test.ts index cc7b5aebd..e01d98a97 100644 --- a/packages/core/src/routes/interaction/verifications/profile-verification-profile-registered.test.ts +++ b/packages/core/src/routes/interaction/verifications/profile-verification-profile-registered.test.ts @@ -2,7 +2,7 @@ import { Event } from '@logto/schemas'; import { mockEsm, mockEsmWithActual, pickDefault } from '@logto/shared/esm'; import RequestError from '#src/errors/RequestError/index.js'; -import { createMockLogContext } from '#src/test-utils/koa-log.js'; +import { createMockLogContext } from '#src/test-utils/koa-audit-log.js'; import { createMockProvider } from '#src/test-utils/oidc-provider.js'; import { createContextWithRouteParameters } from '#src/utils/test-utils.js'; diff --git a/packages/core/src/routes/interaction/verifications/profile-verification-protected-identifier.test.ts b/packages/core/src/routes/interaction/verifications/profile-verification-protected-identifier.test.ts index 790456d1f..b93b6e6e3 100644 --- a/packages/core/src/routes/interaction/verifications/profile-verification-protected-identifier.test.ts +++ b/packages/core/src/routes/interaction/verifications/profile-verification-protected-identifier.test.ts @@ -2,7 +2,7 @@ import { Event } from '@logto/schemas'; import { mockEsm, mockEsmWithActual, pickDefault } from '@logto/shared/esm'; import RequestError from '#src/errors/RequestError/index.js'; -import { createMockLogContext } from '#src/test-utils/koa-log.js'; +import { createMockLogContext } from '#src/test-utils/koa-audit-log.js'; import { createMockProvider } from '#src/test-utils/oidc-provider.js'; import { createContextWithRouteParameters } from '#src/utils/test-utils.js'; diff --git a/packages/core/src/routes/interaction/verifications/user-identity-verification.test.ts b/packages/core/src/routes/interaction/verifications/user-identity-verification.test.ts index dc6efd648..b0cd5c756 100644 --- a/packages/core/src/routes/interaction/verifications/user-identity-verification.test.ts +++ b/packages/core/src/routes/interaction/verifications/user-identity-verification.test.ts @@ -2,7 +2,7 @@ import { Event } from '@logto/schemas'; import { mockEsm, mockEsmDefault, mockEsmWithActual, pickDefault } from '@logto/shared/esm'; import RequestError from '#src/errors/RequestError/index.js'; -import { createMockLogContext } from '#src/test-utils/koa-log.js'; +import { createMockLogContext } from '#src/test-utils/koa-audit-log.js'; import { createMockProvider } from '#src/test-utils/oidc-provider.js'; import { createContextWithRouteParameters } from '#src/utils/test-utils.js'; diff --git a/packages/core/src/test-utils/koa-log.ts b/packages/core/src/test-utils/koa-audit-log.ts similarity index 100% rename from packages/core/src/test-utils/koa-log.ts rename to packages/core/src/test-utils/koa-audit-log.ts diff --git a/packages/integration-tests/package.json b/packages/integration-tests/package.json index 464924d89..a537a10a8 100644 --- a/packages/integration-tests/package.json +++ b/packages/integration-tests/package.json @@ -12,10 +12,9 @@ "scripts": { "build": "rm -rf lib/ && tsc -p tsconfig.test.json --sourcemap", "test:only": "NODE_OPTIONS=--experimental-vm-modules jest", - "test": "pnpm build && pnpm test:api && pnpm test:ui && pnpm test:interaction", + "test": "pnpm build && pnpm test:api && pnpm test:ui", "test:api": "pnpm test:only -i ./lib/tests/api", "test:ui": "pnpm test:only -i --config=jest.config.ui.js ./lib/tests/ui", - "test:interaction": "pnpm test:only -i ./lib/tests/interaction", "lint": "eslint --ext .ts src", "lint:report": "pnpm lint --format json --output-file report.json", "start": "pnpm test" diff --git a/packages/integration-tests/src/client/index.ts b/packages/integration-tests/src/client/index.ts index aad377617..2af732f2c 100644 --- a/packages/integration-tests/src/client/index.ts +++ b/packages/integration-tests/src/client/index.ts @@ -1,6 +1,7 @@ import type { LogtoConfig } from '@logto/node'; import LogtoClient from '@logto/node'; import { demoAppApplicationId } from '@logto/schemas/lib/seeds/index.js'; +import type { Optional } from '@silverhand/essentials'; import { assert } from '@silverhand/essentials'; import { got } from 'got'; @@ -41,6 +42,22 @@ export default class MockClient { return this.rawCookies.join('; '); } + public get parsedCookies(): Map> { + const map = new Map>(); + + for (const cookie of this.rawCookies) { + for (const element of cookie.split(';')) { + const [key, value] = element.trim().split('='); + + if (key) { + map.set(key, value); + } + } + } + + return map; + } + public async initSession(callbackUri = demoAppRedirectUri) { await this.logto.signIn(callbackUri); diff --git a/packages/integration-tests/src/tests/api/audit-logs/index.test.ts b/packages/integration-tests/src/tests/api/audit-logs/index.test.ts index 24fb50237..3f4425f61 100644 --- a/packages/integration-tests/src/tests/api/audit-logs/index.test.ts +++ b/packages/integration-tests/src/tests/api/audit-logs/index.test.ts @@ -1,36 +1,27 @@ -import { interaction } from '@logto/schemas'; -import type { Optional } from '@silverhand/essentials'; +import { Event, interaction, SignInIdentifier } from '@logto/schemas'; import { assert } from '@silverhand/essentials'; import { deleteUser } from '#src/api/admin-user.js'; +import { putInteraction } from '#src/api/interaction.js'; import { getLogs } from '#src/api/logs.js'; -import { registerUserWithUsernameAndPassword } from '#src/api/session.js'; import MockClient from '#src/client/index.js'; -import { generatePassword, generateUsername } from '#src/utils.js'; -const parseCookies = (cookies: string[]): Map> => { - const map = new Map>(); +import { enableAllPasswordSignInMethods } from '../interaction/utils/sign-in-experience.js'; +import { generateNewUserProfile } from '../interaction/utils/user.js'; - for (const cookie of cookies) { - for (const element of cookie.split(';')) { - const [key, value] = element.trim().split('='); - - if (key) { - map.set(key, value); - } - } - } - - return map; -}; - -// TODO: @Gao Use new interaction APIs describe('audit logs for interaction', () => { + beforeAll(async () => { + await enableAllPasswordSignInMethods({ + identifiers: [SignInIdentifier.Username], + password: true, + verify: false, + }); + }); + it('should insert log after interaction started and ended', async () => { const client = new MockClient(); await client.initSession(); - const cookies = parseCookies(client.rawCookies); - const interactionId = cookies.get('_interaction'); + const interactionId = client.parsedCookies.get('_interaction'); assert(interactionId, new Error('No interaction found in cookie')); console.debug('Testing interaction', interactionId); @@ -42,11 +33,12 @@ describe('audit logs for interaction', () => { expect(createLogs.some((value) => value.payload.interactionId === interactionId)).toBeTruthy(); // Process interaction with minimum effort - const username = generateUsername(); - const password = generatePassword(); - const response = await registerUserWithUsernameAndPassword( - username, - password, + const { username, password } = generateNewUserProfile({ username: true, password: true }); + const response = await putInteraction( + { + event: Event.Register, + profile: { username, password }, + }, client.interactionCookie ); await client.processSession(response.redirectTo); diff --git a/packages/schemas/src/types/log/interaction.ts b/packages/schemas/src/types/log/interaction.ts index bfcecb20e..b40baab2c 100644 --- a/packages/schemas/src/types/log/interaction.ts +++ b/packages/schemas/src/types/log/interaction.ts @@ -31,12 +31,7 @@ export enum Action { /** * The union type of all available log keys for interaction. - * - * The key MUST describe an {@link Action}: - * - * - {@link Action.Create} (Create a new entity); - * - {@link Action.Update} (Update an existing entity); - * - {@link Action.Submit} (Submit updated info to an entity, or submit to the system). + * The key MUST describe an {@link Action}. * * ### Keys breakdown *