From d527546b3213075a16a0698d7a1e09c1e0de7d52 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Tue, 4 Jun 2024 11:01:23 +0800 Subject: [PATCH 1/2] refactor(core,schemas): add user detail payload to User.Deleted DataHook event add user detail data payload to the User.Deleted DataHook event --- .changeset/fuzzy-eyes-add.md | 5 +++++ packages/core/src/routes/admin-user/basics.ts | 10 ++++++++++ .../api/hook/hook.trigger.custom.data.test.ts | 18 +++++++++++++++++- .../tests/api/hook/hook.trigger.data.test.ts | 5 +++++ .../src/tests/api/hook/test-cases.ts | 7 ------- .../src/foundations/jsonb-types/hooks.ts | 2 +- 6 files changed, 38 insertions(+), 9 deletions(-) create mode 100644 .changeset/fuzzy-eyes-add.md diff --git a/.changeset/fuzzy-eyes-add.md b/.changeset/fuzzy-eyes-add.md new file mode 100644 index 00000000000..00fb30cb3f9 --- /dev/null +++ b/.changeset/fuzzy-eyes-add.md @@ -0,0 +1,5 @@ +--- +"@logto/core": patch +--- + +add user detail data payload to the User.Deleted DataHook event diff --git a/packages/core/src/routes/admin-user/basics.ts b/packages/core/src/routes/admin-user/basics.ts index eb5548394ad..71e9eae77e0 100644 --- a/packages/core/src/routes/admin-user/basics.ts +++ b/packages/core/src/routes/admin-user/basics.ts @@ -11,6 +11,7 @@ import { conditional, pick, yes } from '@silverhand/essentials'; import { boolean, literal, nativeEnum, object, string } from 'zod'; import RequestError from '#src/errors/RequestError/index.js'; +import { buildManagementApiContext } from '#src/libraries/hook/utils.js'; import { encryptUserPassword } from '#src/libraries/user.js'; import koaGuard from '#src/middleware/koa-guard.js'; import assertThat from '#src/utils/assert-that.js'; @@ -373,11 +374,20 @@ export default function adminUserBasicsRoutes( throw new RequestError('user.cannot_delete_self'); } + const user = await findUserById(userId); + await signOutUser(userId); await deleteUserById(userId); ctx.status = 204; + // Manually trigger the User.Deleted hook with the user data payload + ctx.appendDataHookContext({ + event: 'User.Deleted', + ...buildManagementApiContext(ctx), + data: pick(user, ...userInfoSelectFields), + }); + return next(); } ); diff --git a/packages/integration-tests/src/tests/api/hook/hook.trigger.custom.data.test.ts b/packages/integration-tests/src/tests/api/hook/hook.trigger.custom.data.test.ts index 70ee3b6f493..3b304044028 100644 --- a/packages/integration-tests/src/tests/api/hook/hook.trigger.custom.data.test.ts +++ b/packages/integration-tests/src/tests/api/hook/hook.trigger.custom.data.test.ts @@ -1,5 +1,7 @@ -import { hookEvents } from '@logto/schemas'; +import { hookEvents, userInfoSelectFields } from '@logto/schemas'; +import { pick } from '@silverhand/essentials'; +import { createUser, deleteUser } from '#src/api/admin-user.js'; import { OrganizationRoleApi } from '#src/api/organization-role.js'; import { OrganizationScopeApi } from '#src/api/organization-scope.js'; import { createResource, deleteResource } from '#src/api/resource.js'; @@ -94,4 +96,18 @@ describe('trigger custom data hook events', () => { await roleApi.delete(organizationRole.id); await organizationScopeApi.delete(scope.id); }); + + it('delete user should trigger User.Deleted event with selected user info', async () => { + const user = await createUser(); + const hook = webHookApi.hooks.get(hookName)!; + + await deleteUser(user.id); + + await assertHookLogResult(hook, 'User.Deleted', { + hookPayload: { + event: 'User.Deleted', + data: pick(user, ...userInfoSelectFields), + }, + }); + }); }); diff --git a/packages/integration-tests/src/tests/api/hook/hook.trigger.data.test.ts b/packages/integration-tests/src/tests/api/hook/hook.trigger.data.test.ts index 5cf6b6bc210..3b8ebf2c372 100644 --- a/packages/integration-tests/src/tests/api/hook/hook.trigger.data.test.ts +++ b/packages/integration-tests/src/tests/api/hook/hook.trigger.data.test.ts @@ -120,6 +120,11 @@ describe('user data hook events', () => { expect(hook?.payload.event).toBe(event); } ); + + // Clean up + afterAll(async () => { + await authedAdminApi.delete(`users/${userId}`); + }); }); describe('role data hook events', () => { diff --git a/packages/integration-tests/src/tests/api/hook/test-cases.ts b/packages/integration-tests/src/tests/api/hook/test-cases.ts index 4d142eb3b56..7f072d93d50 100644 --- a/packages/integration-tests/src/tests/api/hook/test-cases.ts +++ b/packages/integration-tests/src/tests/api/hook/test-cases.ts @@ -44,13 +44,6 @@ export const userDataHookTestCases: TestCase[] = [ endpoint: `users/{userId}/is-suspended`, payload: { isSuspended: true }, }, - { - route: 'DELETE /users/:userId', - event: 'User.Deleted', - method: 'delete', - endpoint: `users/{userId}`, - payload: {}, - }, ]; export const roleDataHookTestCases: TestCase[] = [ diff --git a/packages/schemas/src/foundations/jsonb-types/hooks.ts b/packages/schemas/src/foundations/jsonb-types/hooks.ts index fc47556d8d4..0b4204c1d25 100644 --- a/packages/schemas/src/foundations/jsonb-types/hooks.ts +++ b/packages/schemas/src/foundations/jsonb-types/hooks.ts @@ -117,7 +117,7 @@ type ApiMethod = 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH'; */ export const managementApiHooksRegistration = Object.freeze({ 'POST /users': 'User.Created', - 'DELETE /users/:userId': 'User.Deleted', + // `User.Deleted` event is triggered manually in the `DELETE /users/:userId` route for better payload control 'PATCH /users/:userId': 'User.Data.Updated', 'PATCH /users/:userId/custom-data': 'User.Data.Updated', 'PATCH /users/:userId/profile': 'User.Data.Updated', From 1c6cadf4e0b27aac9f0329484ef22d96718b4b32 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Tue, 4 Jun 2024 14:53:30 +0800 Subject: [PATCH 2/2] fix(core): fix unit test fix unit test --- .changeset/fuzzy-eyes-add.md | 2 +- packages/core/src/libraries/hook/context-manager.ts | 2 -- packages/core/src/routes/admin-user/basics.test.ts | 7 ++++++- packages/core/src/routes/admin-user/basics.ts | 2 +- packages/core/src/routes/well-known.test.ts | 10 +--------- packages/core/src/utils/test-utils.ts | 4 ++-- 6 files changed, 11 insertions(+), 16 deletions(-) diff --git a/.changeset/fuzzy-eyes-add.md b/.changeset/fuzzy-eyes-add.md index 00fb30cb3f9..0a5a5e82f9a 100644 --- a/.changeset/fuzzy-eyes-add.md +++ b/.changeset/fuzzy-eyes-add.md @@ -2,4 +2,4 @@ "@logto/core": patch --- -add user detail data payload to the User.Deleted DataHook event +add user detail data payload to the `User.Deleted` webhook event diff --git a/packages/core/src/libraries/hook/context-manager.ts b/packages/core/src/libraries/hook/context-manager.ts index a8d5a084848..83a1449777d 100644 --- a/packages/core/src/libraries/hook/context-manager.ts +++ b/packages/core/src/libraries/hook/context-manager.ts @@ -16,8 +16,6 @@ import { hasRegisteredDataHookEvent, } from './utils.js'; -type ManagementApiHooksRegistrationKey = keyof typeof managementApiHooksRegistration; - type DataHookMetadata = { userAgent?: string; ip: string; diff --git a/packages/core/src/routes/admin-user/basics.test.ts b/packages/core/src/routes/admin-user/basics.test.ts index 9ca8a668d0c..99c272e3561 100644 --- a/packages/core/src/routes/admin-user/basics.test.ts +++ b/packages/core/src/routes/admin-user/basics.test.ts @@ -5,6 +5,7 @@ import { removeUndefinedKeys } from '@silverhand/essentials'; import { mockUser, mockUserResponse } from '#src/__mocks__/index.js'; import RequestError from '#src/errors/RequestError/index.js'; +import { koaManagementApiHooks } from '#src/middleware/koa-management-api-hooks.js'; import type Libraries from '#src/tenants/Libraries.js'; import type Queries from '#src/tenants/Queries.js'; import { MockTenant, type Partial2 } from '#src/test-utils/tenant.js'; @@ -95,7 +96,11 @@ describe('adminUserRoutes', () => { const tenantContext = new MockTenant(undefined, mockedQueries, undefined, { users: usersLibraries, }); - const userRequest = createRequester({ authedRoutes: adminUserRoutes, tenantContext }); + const userRequest = createRequester({ + middlewares: [koaManagementApiHooks(tenantContext.libraries.hooks)], + authedRoutes: adminUserRoutes, + tenantContext, + }); afterEach(() => { jest.clearAllMocks(); diff --git a/packages/core/src/routes/admin-user/basics.ts b/packages/core/src/routes/admin-user/basics.ts index 71e9eae77e0..3ffaea83561 100644 --- a/packages/core/src/routes/admin-user/basics.ts +++ b/packages/core/src/routes/admin-user/basics.ts @@ -381,7 +381,7 @@ export default function adminUserBasicsRoutes( ctx.status = 204; - // Manually trigger the User.Deleted hook with the user data payload + // Manually trigger the `User.Deleted` hook since we need to send the user data in the payload ctx.appendDataHookContext({ event: 'User.Deleted', ...buildManagementApiContext(ctx), diff --git a/packages/core/src/routes/well-known.test.ts b/packages/core/src/routes/well-known.test.ts index ddb83e956b8..be0fa554fab 100644 --- a/packages/core/src/routes/well-known.test.ts +++ b/packages/core/src/routes/well-known.test.ts @@ -1,4 +1,4 @@ -import { pickDefault, createMockUtils } from '@logto/shared/esm'; +import { createMockUtils, pickDefault } from '@logto/shared/esm'; import { mockAliyunDmConnector, @@ -63,14 +63,6 @@ describe('GET /.well-known/sign-in-exp', () => { const sessionRequest = createRequester({ anonymousRoutes: wellKnownRoutes, tenantContext, - middlewares: [ - async (ctx, next) => { - ctx.addLogContext = jest.fn(); - ctx.log = jest.fn(); - - return next(); - }, - ], }); it('should return github and facebook connector instances', async () => { diff --git a/packages/core/src/utils/test-utils.ts b/packages/core/src/utils/test-utils.ts index 6a0a1be408d..3143dcff4bb 100644 --- a/packages/core/src/utils/test-utils.ts +++ b/packages/core/src/utils/test-utils.ts @@ -112,7 +112,7 @@ type RouteLauncher = ( tenant: TenantContext ) => void; -export function createRequester({ +export function createRequester({ anonymousRoutes, authedRoutes, middlewares, @@ -120,7 +120,7 @@ export function createRequester({ }: { anonymousRoutes?: RouteLauncher | Array>; authedRoutes?: RouteLauncher | Array>; - middlewares?: Middleware[]; + middlewares?: Array>; tenantContext?: TenantContext; }) { const app = new Koa();