fix(core): trigger reset password hook (#3916)

.changeset/silly-impalas-pump.md

@@ -0,0 +1,5 @@
+---
+"@logto/core": patch
+---
+
+Bug fix: reset password webhook should be triggered when the user resets password

packages/core/src/libraries/hook/index.test.ts

@@ -5,7 +5,6 @@ import { createMockUtils } from '@logto/shared/esm';
 import { mockHook } from '#src/__mocks__/hook.js';
 import RequestError from '#src/errors/RequestError/index.js';
 
-import type { Interaction } from './index.js';
 import { generateHookTestPayload, parseResponse } from './utils.js';
 
 const { jest } = import.meta;
@@ -50,7 +49,7 @@ const findAllHooks = jest.fn().mockResolvedValue([hook]);
 const findHookById = jest.fn().mockResolvedValue(hook);
 
 const { createHookLibrary } = await import('./index.js');
-const { triggerInteractionHooksIfNeeded, attachExecutionStatsToHook, testHook } = createHookLibrary(
+const { triggerInteractionHooks, attachExecutionStatsToHook, testHook } = createHookLibrary(
   new MockQueries({
     users: {
       findUserById: jest.fn().mockReturnValue({
@@ -67,28 +66,17 @@ const { triggerInteractionHooksIfNeeded, attachExecutionStatsToHook, testHook }
   })
 );
 
-describe('triggerInteractionHooksIfNeeded()', () => {
+describe('triggerInteractionHooks()', () => {
   afterEach(() => {
     jest.clearAllMocks();
   });
 
-  it('should return if no user ID found', async () => {
-    await triggerInteractionHooksIfNeeded(InteractionEvent.SignIn);
-
-    expect(findAllHooks).not.toBeCalled();
-  });
-
   it('should set correct payload when hook triggered', async () => {
     jest.useFakeTimers().setSystemTime(100_000);
 
-    await triggerInteractionHooksIfNeeded(
-      InteractionEvent.SignIn,
-      // @ts-expect-error
-      {
-        jti: 'some_jti',
-        result: { login: { accountId: '123' } },
-        params: { client_id: 'some_client' },
-      } as Interaction
+    await triggerInteractionHooks(
+      { event: InteractionEvent.SignIn, sessionId: 'some_jti', applicationId: 'some_client' },
+      { userId: '123' }
     );
 
     expect(findAllHooks).toHaveBeenCalled();

packages/core/src/libraries/hook/index.ts

@@ -11,7 +11,6 @@ import {
 import { generateStandardId } from '@logto/shared';
 import { conditional, pick, trySafe } from '@silverhand/essentials';
 import { HTTPError } from 'got';
-import type Provider from 'oidc-provider';
 
 import RequestError from '#src/errors/RequestError/index.js';
 import { LogEntry } from '#src/middleware/koa-audit-log.js';
@@ -20,14 +19,32 @@ import { consoleLog } from '#src/utils/console.js';
 
 import { generateHookTestPayload, parseResponse, sendWebhookRequest } from './utils.js';
 
+/**
+ * The context for triggering interaction hooks by `triggerInteractionHooks`.
+ * In the `koaInteractionHooks` middleware,
+ * we will store the context before processing the interaction and consume it after the interaction is processed if needed.
+ */
+export type InteractionHookContext = {
+  event: InteractionEvent;
+  sessionId?: string;
+  applicationId?: string;
+};
+
+/**
+ * The interaction hook result for triggering interaction hooks by `triggerInteractionHooks`.
+ * In the `koaInteractionHooks` middleware,
+ * if we get an interaction hook result after the interaction is processed, related hooks will be triggered.
+ */
+export type InteractionHookResult = {
+  userId: string;
+};
+
 const eventToHook: Record<InteractionEvent, HookEvent> = {
   [InteractionEvent.Register]: HookEvent.PostRegister,
   [InteractionEvent.SignIn]: HookEvent.PostSignIn,
   [InteractionEvent.ForgotPassword]: HookEvent.PostResetPassword,
 };
 
-export type Interaction = Awaited<ReturnType<Provider['interactionDetails']>>;
-
 export const createHookLibrary = (queries: Queries) => {
   const {
     applications: { findApplicationById },
@@ -37,18 +54,13 @@ export const createHookLibrary = (queries: Queries) => {
     hooks: { findAllHooks, findHookById },
   } = queries;
 
-  const triggerInteractionHooksIfNeeded = async (
-    event: InteractionEvent,
-    details?: Interaction,
+  const triggerInteractionHooks = async (
+    interactionContext: InteractionHookContext,
+    interactionResult: InteractionHookResult,
     userAgent?: string
   ) => {
-    const userId = details?.result?.login?.accountId;
-    const sessionId = details?.jti;
-    const applicationId = details?.params.client_id;
-
-    if (!userId) {
-      return;
-    }
+    const { userId } = interactionResult;
+    const { event, sessionId, applicationId } = interactionContext;
 
     const hookEvent = eventToHook[event];
     const found = await findAllHooks();
@@ -63,9 +75,7 @@ export const createHookLibrary = (queries: Queries) => {
 
     const [user, application] = await Promise.all([
       trySafe(findUserById(userId)),
-      trySafe(async () =>
-        conditional(typeof applicationId === 'string' && (await findApplicationById(applicationId)))
-      ),
+      trySafe(async () => conditional(applicationId && (await findApplicationById(applicationId)))),
     ]);
 
     const payload = {
@@ -155,7 +165,7 @@ export const createHookLibrary = (queries: Queries) => {
   });
 
   return {
-    triggerInteractionHooksIfNeeded,
+    triggerInteractionHooks,
     attachExecutionStatsToHook,
     testHook,
   };

packages/core/src/routes/interaction/actions/submit-interaction.test.ts

@@ -64,6 +64,7 @@ describe('submit action', () => {
     ...createMockLogContext(),
     // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
     interactionDetails: { params: {} } as Awaited<ReturnType<Provider['interactionDetails']>>,
+    assignInteractionHookResult: jest.fn(),
   };
   const profile = {
     username: 'username',

packages/core/src/routes/interaction/actions/submit-interaction.ts

@@ -17,11 +17,12 @@ import { EnvSet } from '#src/env-set/index.js';
 import type { ConnectorLibrary } from '#src/libraries/connector.js';
 import { assignInteractionResults } from '#src/libraries/session.js';
 import { encryptUserPassword } from '#src/libraries/user.js';
-import type { LogEntry } from '#src/middleware/koa-audit-log.js';
+import type { LogEntry, WithLogContext } from '#src/middleware/koa-audit-log.js';
 import type TenantContext from '#src/tenants/TenantContext.js';
 import { getTenantId } from '#src/utils/tenant.js';
 
 import type { WithInteractionDetailsContext } from '../middleware/koa-interaction-details.js';
+import { type WithInteractionHooksContext } from '../middleware/koa-interaction-hooks.js';
 import type {
   Identifier,
   VerifiedInteractionResult,
@@ -165,7 +166,7 @@ const parseUserProfile = async (
 
 export default async function submitInteraction(
   interaction: VerifiedInteractionResult,
-  ctx: WithInteractionDetailsContext,
+  ctx: WithLogContext & WithInteractionDetailsContext & WithInteractionHooksContext,
   { provider, libraries, connectors, queries }: TenantContext,
   log?: LogEntry
 ) {
@@ -211,6 +212,7 @@ export default async function submitInteraction(
     }
 
     await assignInteractionResults(ctx, provider, { login: { accountId: id } });
+    ctx.assignInteractionHookResult({ userId: id });
 
     log?.append({ userId: id });
     appInsights.client?.trackEvent({ name: getEventName(Component.Core, CoreEvent.Register) });
@@ -227,6 +229,7 @@ export default async function submitInteraction(
 
     await updateUserById(accountId, updateUserProfile);
     await assignInteractionResults(ctx, provider, { login: { accountId } });
+    ctx.assignInteractionHookResult({ userId: accountId });
 
     appInsights.client?.trackEvent({ name: getEventName(Component.Core, CoreEvent.SignIn) });
 
@@ -239,6 +242,7 @@ export default async function submitInteraction(
   );
 
   await updateUserById(accountId, { passwordEncrypted, passwordEncryptionMethod });
+  ctx.assignInteractionHookResult({ userId: accountId });
   await clearInteractionStorage(ctx, provider);
   ctx.status = 204;
 }

packages/core/src/routes/interaction/consent.ts

@@ -1,5 +1,6 @@
 import { conditional } from '@silverhand/essentials';
 import type Router from 'koa-router';
+import { type IRouterParamContext } from 'koa-router';
 import { z } from 'zod';
 
 import { assignInteractionResults, saveUserFirstConsentedAppId } from '#src/libraries/session.js';
@@ -9,7 +10,7 @@ import assertThat from '#src/utils/assert-that.js';
 import { interactionPrefix } from './const.js';
 import type { WithInteractionDetailsContext } from './middleware/koa-interaction-details.js';
 
-export default function consentRoutes<T>(
+export default function consentRoutes<T extends IRouterParamContext>(
   router: Router<unknown, WithInteractionDetailsContext<T>>,
   { provider, libraries, queries }: TenantContext
 ) {

packages/core/src/routes/interaction/index.ts

@@ -285,7 +285,7 @@ export default function interactionRoutes<T extends AnonymousRouter>(
   router.post(
     `${interactionPrefix}/submit`,
     koaInteractionSie(queries),
-    koaInteractionHooks(tenant),
+    koaInteractionHooks(libraries),
     async (ctx, next) => {
       const { interactionDetails, createLog } = ctx;
       const interactionStorage = getInteractionStorage(interactionDetails.result);

packages/core/src/routes/interaction/middleware/koa-interaction-details.ts

@@ -1,15 +1,18 @@
 import type { MiddlewareType } from 'koa';
+import { type IRouterParamContext } from 'koa-router';
 import type Provider from 'oidc-provider';
 
-import type { WithLogContext } from '#src/middleware/koa-audit-log.js';
-
-export type WithInteractionDetailsContext<ContextT = WithLogContext> = ContextT & {
+export type WithInteractionDetailsContext<
+  ContextT extends IRouterParamContext = IRouterParamContext
+> = ContextT & {
   interactionDetails: Awaited<ReturnType<Provider['interactionDetails']>>;
 };
 
-export default function koaInteractionDetails<StateT, ContextT>(
-  provider: Provider
-): MiddlewareType<StateT, WithInteractionDetailsContext<ContextT>> {
+export default function koaInteractionDetails<
+  StateT,
+  ContextT extends IRouterParamContext,
+  ResponseT
+>(provider: Provider): MiddlewareType<StateT, WithInteractionDetailsContext<ContextT>, ResponseT> {
   return async (ctx, next) => {
     ctx.interactionDetails = await provider.interactionDetails(ctx.req, ctx.res);
 

packages/core/src/routes/interaction/middleware/koa-interaction-hooks.ts

@@ -1,31 +1,69 @@
-import { trySafe } from '@silverhand/essentials';
+import { type Optional, trySafe, conditionalString } from '@silverhand/essentials';
 import type { MiddlewareType } from 'koa';
 import type { IRouterParamContext } from 'koa-router';
 
-import type TenantContext from '#src/tenants/TenantContext.js';
+import {
+  type InteractionHookContext,
+  type InteractionHookResult,
+} from '#src/libraries/hook/index.js';
+import type Libraries from '#src/tenants/Libraries.js';
 
 import { getInteractionStorage } from '../utils/interaction.js';
 
 import type { WithInteractionDetailsContext } from './koa-interaction-details.js';
 
+type AssignInteractionHookResult = (result: InteractionHookResult) => void;
+
+export type WithInteractionHooksContext<
+  ContextT extends IRouterParamContext = IRouterParamContext
+> = ContextT & { assignInteractionHookResult: AssignInteractionHookResult };
+
+/**
+ * The factory to create a new interaction hook middleware function.
+ * Interaction related event hooks will be triggered once we got the interaction hook result.
+ * Use `assignInteractionHookResult` to assign the interaction hook result.
+ */
 export default function koaInteractionHooks<
   StateT,
-  ContextT extends WithInteractionDetailsContext<IRouterParamContext>,
+  ContextT extends WithInteractionDetailsContext,
   ResponseT
 >({
-  provider,
-  libraries: {
-    hooks: { triggerInteractionHooksIfNeeded },
-  },
-}: TenantContext): MiddlewareType<StateT, ContextT, ResponseT> {
+  hooks: { triggerInteractionHooks },
+}: Libraries): MiddlewareType<StateT, WithInteractionHooksContext<ContextT>, ResponseT> {
   return async (ctx, next) => {
     const { event } = getInteractionStorage(ctx.interactionDetails.result);
+    const {
+      interactionDetails,
+      header: { 'user-agent': userAgent },
+    } = ctx;
+
+    // Predefined interaction hook context
+    const interactionHookContext: InteractionHookContext = {
+      event,
+      sessionId: interactionDetails.jti,
+      applicationId: conditionalString(interactionDetails.params.client_id),
+    };
+
+    // eslint-disable-next-line @silverhand/fp/no-let
+    let interactionHookResult: Optional<InteractionHookResult>;
+
+    /**
+     * Assign an interaction hook result to trigger webhook.
+     * Calling it multiple times will overwrite the original result, but only one webhook will be triggered.
+     * @param result The result to assign.
+     */
+    ctx.assignInteractionHookResult = (result) => {
+      // eslint-disable-next-line @silverhand/fp/no-mutation
+      interactionHookResult = result;
+    };
 
     await next();
 
-    // Get up-to-date interaction details
-    const details = await trySafe(provider.interactionDetails(ctx.req, ctx.res));
-    // Hooks should not crash the app
-    void trySafe(triggerInteractionHooksIfNeeded(event, details, ctx.header['user-agent']));
+    if (interactionHookResult) {
+      // Hooks should not crash the app
+      void trySafe(
+        triggerInteractionHooks(interactionHookContext, interactionHookResult, userAgent)
+      );
+    }
   };
 }

packages/core/src/routes/interaction/middleware/koa-interaction-sie.ts

@@ -1,15 +1,13 @@
 import type { SignInExperience } from '@logto/schemas';
 import type { MiddlewareType } from 'koa';
+import { type IRouterParamContext } from 'koa-router';
 
 import type Queries from '#src/tenants/Queries.js';
 
-import type { WithInteractionDetailsContext } from './koa-interaction-details.js';
+export type WithInteractionSieContext<ContextT extends IRouterParamContext = IRouterParamContext> =
+  ContextT & { signInExperience: SignInExperience };
 
-export type WithInteractionSieContext<ContextT> = WithInteractionDetailsContext<ContextT> & {
-  signInExperience: SignInExperience;
-};
-
-export default function koaInteractionSie<StateT, ContextT, ResponseT>({
+export default function koaInteractionSie<StateT, ContextT extends IRouterParamContext, ResponseT>({
   signInExperiences: { findDefaultSignInExperience },
 }: Queries): MiddlewareType<StateT, WithInteractionSieContext<ContextT>, ResponseT> {
   return async (ctx, next) => {

packages/core/src/routes/interaction/verifications/mandatory-user-profile-validation.ts

@@ -2,12 +2,12 @@ import type { Profile, SignInExperience, User } from '@logto/schemas';
 import { InteractionEvent, MissingProfile, SignInIdentifier } from '@logto/schemas';
 import type { Nullable } from '@silverhand/essentials';
 import { conditional } from '@silverhand/essentials';
-import type { Context } from 'koa';
 
 import RequestError from '#src/errors/RequestError/index.js';
 import type Queries from '#src/tenants/Queries.js';
 import assertThat from '#src/utils/assert-that.js';
 
+import { type WithInteractionDetailsContext } from '../middleware/koa-interaction-details.js';
 import type { WithInteractionSieContext } from '../middleware/koa-interaction-sie.js';
 import type {
   SocialIdentifier,
@@ -213,7 +213,7 @@ const fillMissingProfileWithSocialIdentity = async (
 
 export default async function validateMandatoryUserProfile(
   userQueries: Queries['users'],
-  ctx: WithInteractionSieContext<Context>,
+  ctx: WithInteractionSieContext & WithInteractionDetailsContext,
   interaction: MandatoryProfileValidationInteraction
 ) {
   const { signUp } = ctx.signInExperience;

packages/integration-tests/src/helpers/interactions.ts

@@ -10,11 +10,12 @@ import {
   createSocialAuthorizationUri,
   patchInteractionIdentifiers,
   putInteractionProfile,
+  sendVerificationCode,
 } from '#src/api/index.js';
 import { generateUserId } from '#src/utils.js';
 
 import { initClient, processSession, logoutClient } from './client.js';
-import { expectRejects } from './index.js';
+import { expectRejects, readVerificationCode } from './index.js';
 import { enableAllPasswordSignInMethods } from './sign-in-experience.js';
 import { generateNewUser } from './user.js';
 
@@ -85,3 +86,23 @@ export const createNewSocialUserWithUsernameAndPassword = async (connectorId: st
 
   return processSession(client, redirectTo);
 };
+
+export const resetPassword = async (
+  profile: { email: string } | { phone: string },
+  newPassword: string
+) => {
+  const client = await initClient();
+
+  await client.successSend(putInteraction, { event: InteractionEvent.ForgotPassword });
+  await client.successSend(sendVerificationCode, {
+    ...profile,
+  });
+
+  const { code: verificationCode } = await readVerificationCode();
+  await client.successSend(patchInteractionIdentifiers, {
+    ...profile,
+    verificationCode,
+  });
+  await client.successSend(putInteractionProfile, { password: newPassword });
+  await client.submitInteraction();
+};

packages/integration-tests/src/tests/api/hook/hook.trigger.test.ts

@@ -8,16 +8,26 @@ import {
   LogResult,
   SignInIdentifier,
   type Log,
+  ConnectorType,
 } from '@logto/schemas';
 
 import { deleteUser } from '#src/api/admin-user.js';
 import { authedAdminApi } from '#src/api/api.js';
 import { getLogs } from '#src/api/logs.js';
+import {
+  clearConnectorsByTypes,
+  setEmailConnector,
+  setSmsConnector,
+} from '#src/helpers/connector.js';
 import { getHookCreationPayload } from '#src/helpers/hook.js';
 import { createMockServer } from '#src/helpers/index.js';
-import { registerNewUser, signInWithPassword } from '#src/helpers/interactions.js';
-import { enableAllPasswordSignInMethods } from '#src/helpers/sign-in-experience.js';
+import { registerNewUser, resetPassword, signInWithPassword } from '#src/helpers/interactions.js';
+import {
+  enableAllPasswordSignInMethods,
+  enableAllVerificationCodeSignInMethods,
+} from '#src/helpers/sign-in-experience.js';
 import { generateNewUser, generateNewUserProfile } from '#src/helpers/user.js';
+import { generatePassword, waitFor } from '#src/utils.js';
 
 type HookSecureData = {
   signature: string;
@@ -182,4 +192,46 @@ describe('trigger hooks', () => {
 
     await deleteUser(userId);
   });
+
+  it('should trigger reset password hook and record properly when interaction finished', async () => {
+    await clearConnectorsByTypes([ConnectorType.Email, ConnectorType.Sms]);
+    await setEmailConnector();
+    await setSmsConnector();
+    await enableAllVerificationCodeSignInMethods({
+      identifiers: [SignInIdentifier.Email, SignInIdentifier.Phone],
+      password: true,
+      verify: true,
+    });
+    // Create a reset password hook
+    const resetPasswordHook = await authedAdminApi
+      .post('hooks', {
+        json: getHookCreationPayload(HookEvent.PostResetPassword, 'http://localhost:9999'),
+      })
+      .json<Hook>();
+    const logKey: LogKey = 'TriggerHook.PostResetPassword';
+
+    const { user, userProfile } = await generateNewUser({
+      primaryPhone: true,
+      primaryEmail: true,
+      password: true,
+    });
+    // Reset Password by Email
+    await resetPassword({ email: userProfile.primaryEmail }, generatePassword());
+    // Reset Password by Phone
+    await resetPassword({ phone: userProfile.primaryPhone }, generatePassword());
+    // Wait for the hook to be trigged
+    await waitFor(1000);
+
+    const logs = await getLogs(new URLSearchParams({ logKey, page_size: '100' }));
+    const relatedLogs = logs.filter(
+      ({ payload: { hookId, result } }) =>
+        hookId === resetPasswordHook.id && result === LogResult.Success
+    );
+
+    expect(relatedLogs).toHaveLength(2);
+
+    await authedAdminApi.delete(`hooks/${resetPasswordHook.id}`);
+    await deleteUser(user.id);
+    await clearConnectorsByTypes([ConnectorType.Email, ConnectorType.Sms]);
+  });
 });