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

refactor(core,schemas): add user detail payload to User.Deleted webhook event #5986

Merged
merged 2 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/fuzzy-eyes-add.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@logto/core": patch
---

add user detail data payload to the `User.Deleted` webhook event
2 changes: 0 additions & 2 deletions packages/core/src/libraries/hook/context-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import {
hasRegisteredDataHookEvent,
} from './utils.js';

type ManagementApiHooksRegistrationKey = keyof typeof managementApiHooksRegistration;

type DataHookMetadata = {
userAgent?: string;
ip: string;
Expand Down
7 changes: 6 additions & 1 deletion packages/core/src/routes/admin-user/basics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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();
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/routes/admin-user/basics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -373,11 +374,20 @@ export default function adminUserBasicsRoutes<T extends ManagementApiRouter>(
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();
}
);
Expand Down
10 changes: 1 addition & 9 deletions packages/core/src/routes/well-known.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { pickDefault, createMockUtils } from '@logto/shared/esm';
import { createMockUtils, pickDefault } from '@logto/shared/esm';

import {
mockAliyunDmConnector,
Expand Down Expand Up @@ -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 () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/utils/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,15 @@ type RouteLauncher<T extends ManagementApiRouter | AnonymousRouter> = (
tenant: TenantContext
) => void;

export function createRequester({
export function createRequester<StateT, ContextT extends IRouterParamContext, ResponseT>({
anonymousRoutes,
authedRoutes,
middlewares,
tenantContext,
}: {
anonymousRoutes?: RouteLauncher<AnonymousRouter> | Array<RouteLauncher<AnonymousRouter>>;
authedRoutes?: RouteLauncher<ManagementApiRouter> | Array<RouteLauncher<ManagementApiRouter>>;
middlewares?: Middleware[];
middlewares?: Array<Middleware<StateT, ContextT, ResponseT>>;
tenantContext?: TenantContext;
}) {
const app = new Koa();
Expand Down
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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),
},
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
7 changes: 0 additions & 7 deletions packages/integration-tests/src/tests/api/hook/test-cases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [
Expand Down
2 changes: 1 addition & 1 deletion packages/schemas/src/foundations/jsonb-types/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Loading