From 37b05f9d49683f00bcb5d1933f679ff17d05d588 Mon Sep 17 00:00:00 2001 From: wangsijie Date: Tue, 1 Oct 2024 09:43:17 +0800 Subject: [PATCH] feat(core): trigger webhook event in profile api (#6641) --- packages/core/src/routes/init.ts | 4 + packages/core/src/routes/profile/index.ts | 15 +-- packages/core/src/routes/types.ts | 2 +- .../core/src/routes/verification/index.ts | 3 - packages/integration-tests/src/api/api.ts | 4 +- .../integration-tests/src/helpers/profile.ts | 65 ++++++++++++- .../src/tests/api/profile/index.test.ts | 92 ++++++++++++++----- 7 files changed, 145 insertions(+), 40 deletions(-) diff --git a/packages/core/src/routes/init.ts b/packages/core/src/routes/init.ts index a697cfbeb..b07d883da 100644 --- a/packages/core/src/routes/init.ts +++ b/packages/core/src/routes/init.ts @@ -11,6 +11,7 @@ import koaTenantGuard from '#src/middleware/koa-tenant-guard.js'; import type TenantContext from '#src/tenants/TenantContext.js'; import koaAuth from '../middleware/koa-auth/index.js'; +import koaOidcAuth from '../middleware/koa-auth/koa-oidc-auth.js'; import adminUserRoutes from './admin-user/index.js'; import applicationOrganizationRoutes from './applications/application-organization.js'; @@ -99,6 +100,9 @@ const createRouters = (tenant: TenantContext) => { const anonymousRouter: AnonymousRouter = new Router(); const userRouter: UserRouter = new Router(); + userRouter.use(koaOidcAuth(tenant.provider)); + // TODO(LOG-10147): Rename to koaApiHooks, this middleware is used for both management API and user API + userRouter.use(koaManagementApiHooks(tenant.libraries.hooks)); profileRoutes(userRouter, tenant); verificationRoutes(userRouter, tenant); diff --git a/packages/core/src/routes/profile/index.ts b/packages/core/src/routes/profile/index.ts index 3e79334af..f7d346b08 100644 --- a/packages/core/src/routes/profile/index.ts +++ b/packages/core/src/routes/profile/index.ts @@ -7,12 +7,11 @@ import koaGuard from '#src/middleware/koa-guard.js'; import { EnvSet } from '../../env-set/index.js'; import { encryptUserPassword } from '../../libraries/user.utils.js'; import { buildUserVerificationRecordById } from '../../libraries/verification.js'; -import koaOidcAuth from '../../middleware/koa-auth/koa-oidc-auth.js'; import assertThat from '../../utils/assert-that.js'; import type { UserRouter, RouterInitArgs } from '../types.js'; export default function profileRoutes( - ...[router, { provider, queries, libraries }]: RouterInitArgs + ...[router, { queries, libraries }]: RouterInitArgs ) { const { users: { updateUserById }, @@ -22,8 +21,6 @@ export default function profileRoutes( users: { checkIdentifierCollision }, } = libraries; - router.use(koaOidcAuth(provider)); - if (!EnvSet.values.isDevFeaturesEnabled) { return; } @@ -56,7 +53,7 @@ export default function profileRoutes( const updatedUser = await updateUserById(userId, { name, avatar, username }); - // TODO(LOG-10005): trigger user updated webhook + ctx.appendDataHookContext('User.Data.Updated', { user: updatedUser }); // Only return the fields that were actually updated ctx.body = { @@ -80,7 +77,6 @@ export default function profileRoutes( const { password, verificationRecordId } = ctx.guard.body; // TODO(LOG-9947): apply password policy - // TODO(LOG-10005): trigger user updated webhook const verificationRecord = await buildUserVerificationRecordById( userId, @@ -91,7 +87,12 @@ export default function profileRoutes( assertThat(verificationRecord.isVerified, 'verification_record.not_found'); const { passwordEncrypted, passwordEncryptionMethod } = await encryptUserPassword(password); - await updateUserById(userId, { passwordEncrypted, passwordEncryptionMethod }); + const updatedUser = await updateUserById(userId, { + passwordEncrypted, + passwordEncryptionMethod, + }); + + ctx.appendDataHookContext('User.Data.Updated', { user: updatedUser }); ctx.status = 204; diff --git a/packages/core/src/routes/types.ts b/packages/core/src/routes/types.ts index c6e09905c..e86584717 100644 --- a/packages/core/src/routes/types.ts +++ b/packages/core/src/routes/types.ts @@ -17,7 +17,7 @@ export type ManagementApiRouterContext = WithAuthContext & export type ManagementApiRouter = Router; -export type UserRouter = Router; +export type UserRouter = Router; type RouterInit = (router: T, tenant: TenantContext) => void; export type RouterInitArgs = Parameters>; diff --git a/packages/core/src/routes/verification/index.ts b/packages/core/src/routes/verification/index.ts index 680cf788d..6a55fd96e 100644 --- a/packages/core/src/routes/verification/index.ts +++ b/packages/core/src/routes/verification/index.ts @@ -5,7 +5,6 @@ import koaGuard from '#src/middleware/koa-guard.js'; import { EnvSet } from '../../env-set/index.js'; import { saveVerificationRecord } from '../../libraries/verification.js'; -import koaOidcAuth from '../../middleware/koa-auth/koa-oidc-auth.js'; import { withSentinel } from '../experience/classes/libraries/sentinel-guard.js'; import { PasswordVerification } from '../experience/classes/verifications/password-verification.js'; import type { UserRouter, RouterInitArgs } from '../types.js'; @@ -13,8 +12,6 @@ import type { UserRouter, RouterInitArgs } from '../types.js'; export default function verificationRoutes( ...[router, { provider, queries, libraries, sentinel }]: RouterInitArgs ) { - router.use(koaOidcAuth(provider)); - if (!EnvSet.values.isDevFeaturesEnabled) { return; } diff --git a/packages/integration-tests/src/api/api.ts b/packages/integration-tests/src/api/api.ts index 06b025259..8e2bd9252 100644 --- a/packages/integration-tests/src/api/api.ts +++ b/packages/integration-tests/src/api/api.ts @@ -10,8 +10,8 @@ const api = ky.extend({ export default api; -export const baseAdminTenantApi = ky.extend({ - prefixUrl: new URL(logtoConsoleUrl), +export const baseApi = ky.extend({ + prefixUrl: new URL(logtoUrl), }); // TODO: @gao rename diff --git a/packages/integration-tests/src/helpers/profile.ts b/packages/integration-tests/src/helpers/profile.ts index c92264131..69f76cfce 100644 --- a/packages/integration-tests/src/helpers/profile.ts +++ b/packages/integration-tests/src/helpers/profile.ts @@ -1,18 +1,75 @@ import { type LogtoConfig } from '@logto/node'; +import { demoAppApplicationId, InteractionEvent, type User } from '@logto/schemas'; -import { baseAdminTenantApi } from '../api/api.js'; +import { type InteractionPayload } from '#src/api/interaction.js'; +import { demoAppRedirectUri, logtoUrl } from '#src/constants.js'; +import { generatePassword, generateUsername } from '#src/utils.js'; -import { initClientAndSignIn } from './admin-tenant.js'; +import api, { baseApi, authedAdminApi } from '../api/api.js'; + +import { initClient } from './client.js'; + +export const createDefaultTenantUserWithPassword = async () => { + const username = generateUsername(); + const password = generatePassword(); + const user = await authedAdminApi + .post('users', { + json: { username, password }, + }) + .json(); + + return { user, username, password }; +}; + +export const deleteDefaultTenantUser = async (id: string) => { + await authedAdminApi.delete(`users/${id}`); +}; + +export const putInteraction = async (cookie: string, payload: InteractionPayload) => + api + .put('interaction', { + headers: { cookie }, + json: payload, + redirect: 'manual', + throwHttpErrors: false, + }) + .json(); + +export const initClientAndSignInForDefaultTenant = async ( + username: string, + password: string, + config?: Partial +) => { + const client = await initClient( + { + endpoint: logtoUrl, + appId: demoAppApplicationId, + ...config, + }, + demoAppRedirectUri + ); + await client.successSend(putInteraction, { + event: InteractionEvent.SignIn, + identifier: { + username, + password, + }, + }); + const { redirectTo } = await client.submitInteraction(); + await client.processSession(redirectTo); + + return client; +}; export const signInAndGetUserApi = async ( username: string, password: string, config?: Partial ) => { - const client = await initClientAndSignIn(username, password, config); + const client = await initClientAndSignInForDefaultTenant(username, password, config); const accessToken = await client.getAccessToken(); - return baseAdminTenantApi.extend({ + return baseApi.extend({ headers: { Authorization: `Bearer ${accessToken}`, }, diff --git a/packages/integration-tests/src/tests/api/profile/index.test.ts b/packages/integration-tests/src/tests/api/profile/index.test.ts index 64700e565..c44d1d06d 100644 --- a/packages/integration-tests/src/tests/api/profile/index.test.ts +++ b/packages/integration-tests/src/tests/api/profile/index.test.ts @@ -1,25 +1,52 @@ +import { hookEvents } from '@logto/schemas'; + import { getUserInfo, updatePassword, updateUser } from '#src/api/profile.js'; import { createVerificationRecordByPassword } from '#src/api/verification-record.js'; -import { - createUserWithPassword, - deleteUser, - initClientAndSignIn, -} from '#src/helpers/admin-tenant.js'; +import { WebHookApiTest } from '#src/helpers/hook.js'; import { expectRejects } from '#src/helpers/index.js'; -import { signInAndGetUserApi } from '#src/helpers/profile.js'; +import { + createDefaultTenantUserWithPassword, + deleteDefaultTenantUser, + initClientAndSignInForDefaultTenant, + signInAndGetUserApi, +} from '#src/helpers/profile.js'; import { enableAllPasswordSignInMethods } from '#src/helpers/sign-in-experience.js'; import { devFeatureTest, generatePassword, generateUsername } from '#src/utils.js'; +import WebhookMockServer from '../hook/WebhookMockServer.js'; +import { assertHookLogResult } from '../hook/utils.js'; + const { describe, it } = devFeatureTest; describe('profile', () => { + const webHookMockServer = new WebhookMockServer(9999); + const webHookApi = new WebHookApiTest(); + const hookName = 'profileApiHookEventListener'; + beforeAll(async () => { + await webHookMockServer.listen(); await enableAllPasswordSignInMethods(); }); + afterAll(async () => { + await webHookMockServer.close(); + }); + + beforeEach(async () => { + await webHookApi.create({ + name: hookName, + events: [...hookEvents], + config: { url: webHookMockServer.endpoint }, + }); + }); + + afterEach(async () => { + await webHookApi.cleanUp(); + }); + describe('PATCH /profile', () => { it('should be able to update name', async () => { - const { user, username, password } = await createUserWithPassword(); + const { user, username, password } = await createDefaultTenantUserWithPassword(); const api = await signInAndGetUserApi(username, password); const newName = generateUsername(); @@ -29,11 +56,22 @@ describe('profile', () => { const userInfo = await getUserInfo(api); expect(userInfo).toHaveProperty('name', newName); - await deleteUser(user.id); + // Check if the hook is triggered + const hook = webHookApi.hooks.get(hookName)!; + await assertHookLogResult(hook, 'User.Data.Updated', { + hookPayload: { + event: 'User.Data.Updated', + data: expect.objectContaining({ + name: newName, + }), + }, + }); + + await deleteDefaultTenantUser(user.id); }); it('should be able to update picture', async () => { - const { user, username, password } = await createUserWithPassword(); + const { user, username, password } = await createDefaultTenantUserWithPassword(); const api = await signInAndGetUserApi(username, password); const newAvatar = 'https://example.com/avatar.png'; @@ -44,11 +82,11 @@ describe('profile', () => { // In OIDC, the avatar is mapped to the `picture` field expect(userInfo).toHaveProperty('picture', newAvatar); - await deleteUser(user.id); + await deleteDefaultTenantUser(user.id); }); it('should be able to update username', async () => { - const { user, username, password } = await createUserWithPassword(); + const { user, username, password } = await createDefaultTenantUserWithPassword(); const api = await signInAndGetUserApi(username, password); const newUsername = generateUsername(); @@ -56,14 +94,14 @@ describe('profile', () => { expect(response).toMatchObject({ username: newUsername }); // Sign in with new username - await initClientAndSignIn(newUsername, password); + await initClientAndSignInForDefaultTenant(newUsername, password); - await deleteUser(user.id); + await deleteDefaultTenantUser(user.id); }); it('should fail if username is already in use', async () => { - const { user, username, password } = await createUserWithPassword(); - const { user: user2, username: username2 } = await createUserWithPassword(); + const { user, username, password } = await createDefaultTenantUserWithPassword(); + const { user: user2, username: username2 } = await createDefaultTenantUserWithPassword(); const api = await signInAndGetUserApi(username, password); await expectRejects(updateUser(api, { username: username2 }), { @@ -71,14 +109,14 @@ describe('profile', () => { status: 422, }); - await deleteUser(user.id); - await deleteUser(user2.id); + await deleteDefaultTenantUser(user.id); + await deleteDefaultTenantUser(user2.id); }); }); describe('POST /profile/password', () => { it('should fail if verification record is invalid', async () => { - const { user, username, password } = await createUserWithPassword(); + const { user, username, password } = await createDefaultTenantUserWithPassword(); const api = await signInAndGetUserApi(username, password); const newPassword = generatePassword(); @@ -87,21 +125,29 @@ describe('profile', () => { status: 400, }); - await deleteUser(user.id); + await deleteDefaultTenantUser(user.id); }); it('should be able to update password', async () => { - const { user, username, password } = await createUserWithPassword(); + const { user, username, password } = await createDefaultTenantUserWithPassword(); const api = await signInAndGetUserApi(username, password); const verificationRecordId = await createVerificationRecordByPassword(api, password); const newPassword = generatePassword(); await updatePassword(api, verificationRecordId, newPassword); - // Sign in with new password - await initClientAndSignIn(username, newPassword); + // Check if the hook is triggered + const hook = webHookApi.hooks.get(hookName)!; + await assertHookLogResult(hook, 'User.Data.Updated', { + hookPayload: { + event: 'User.Data.Updated', + }, + }); - await deleteUser(user.id); + // Sign in with new password + await initClientAndSignInForDefaultTenant(username, newPassword); + + await deleteDefaultTenantUser(user.id); }); }); });