From 7a279be1fcd46693270b62606a896f6f013445a4 Mon Sep 17 00:00:00 2001 From: simeng-li Date: Tue, 4 Jun 2024 19:09:27 +0800 Subject: [PATCH] refactor(core,schemas): add user detail payload to User.Deleted webhook event (#5986) * refactor(core,schemas): add user detail payload to User.Deleted DataHook event add user detail data payload to the User.Deleted DataHook event * fix(core): fix unit test fix unit test --- .changeset/fuzzy-eyes-add.md | 5 +++++ .../core/src/libraries/hook/context-manager.ts | 2 -- .../core/src/routes/admin-user/basics.test.ts | 7 ++++++- packages/core/src/routes/admin-user/basics.ts | 10 ++++++++++ packages/core/src/routes/well-known.test.ts | 10 +--------- packages/core/src/utils/test-utils.ts | 4 ++-- .../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 +- 10 files changed, 47 insertions(+), 23 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 000000000..0a5a5e82f --- /dev/null +++ b/.changeset/fuzzy-eyes-add.md @@ -0,0 +1,5 @@ +--- +"@logto/core": patch +--- + +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 a8d5a0848..83a144977 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 9ca8a668d..99c272e35 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 eb5548394..3ffaea835 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 since we need to send the user data in the payload + ctx.appendDataHookContext({ + event: 'User.Deleted', + ...buildManagementApiContext(ctx), + data: pick(user, ...userInfoSelectFields), + }); + return next(); } ); diff --git a/packages/core/src/routes/well-known.test.ts b/packages/core/src/routes/well-known.test.ts index ddb83e956..be0fa554f 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 6a0a1be40..3143dcff4 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(); 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 70ee3b6f4..3b3040440 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 5cf6b6bc2..3b8ebf2c3 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 4d142eb3b..7f072d93d 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 fc47556d8..0b4204c1d 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',