refactor(core, schemas): update interaction webhook middleware using contextManager (#5834)

* feat(core): update interaction webhook middleware using contextManager
updaet interaction webhook middleware using contextManager

* fix(test): fix ut

fix ut

* refactor(core, schemas): refactor DataHook context structure

refactor DataHook context structure

* fix(core): fix demo-app application not found error

fix demo-app application not found error

* chore(core): update comments

update comments

packages/core/src/libraries/hook/context-manager.ts

@@ -1,24 +1,65 @@
-import { type DataHookEvent } from '@logto/schemas';
+import { InteractionEvent, InteractionHookEvent, type DataHookEvent } from '@logto/schemas';
+import { type Optional } from '@silverhand/essentials';
 
-type DataHookContext = {
-  event: DataHookEvent;
-  data?: Record<string, unknown>;
-};
+import type { InteractionApiMetadata, ManagementApiContext } from './type.js';
 
 type DataHookMetadata = {
   userAgent?: string;
   ip: string;
-};
+} & Partial<InteractionApiMetadata>;
+
+type DataHookContext = {
+  event: DataHookEvent;
+  /** Data details */
+  data?: unknown;
+} & Partial<ManagementApiContext>;
 
 export class DataHookContextManager {
   contextArray: DataHookContext[] = [];
 
   constructor(public metadata: DataHookMetadata) {}
 
-  appendContext({ event, data }: DataHookContext) {
+  appendContext(context: DataHookContext) {
     // eslint-disable-next-line @silverhand/fp/no-mutating-methods
-    this.contextArray.push({ event, data });
+    this.contextArray.push(context);
   }
 }
 
-// TODO: @simeng-li migrate the current interaction hook context using hook context manager
+type InteractionHookMetadata = {
+  userAgent?: string;
+  userIp?: string;
+} & InteractionApiMetadata;
+
+/**
+ * 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 interactionEventToHookEvent: Record<InteractionEvent, InteractionHookEvent> = {
+  [InteractionEvent.Register]: InteractionHookEvent.PostRegister,
+  [InteractionEvent.SignIn]: InteractionHookEvent.PostSignIn,
+  [InteractionEvent.ForgotPassword]: InteractionHookEvent.PostResetPassword,
+};
+
+export class InteractionHookContextManager {
+  public interactionHookResult: Optional<InteractionHookResult>;
+
+  constructor(public metadata: InteractionHookMetadata) {}
+
+  get hookEvent() {
+    return interactionEventToHookEvent[this.metadata.interactionEvent];
+  }
+
+  /**
+   * 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.
+   */
+  assignInteractionHookResult(result: InteractionHookResult) {
+    this.interactionHookResult = result;
+  }
+}

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

@@ -6,7 +6,6 @@ import { createMockUtils } from '@logto/shared/esm';
 import RequestError from '#src/errors/RequestError/index.js';
 import { mockId, mockIdGenerators } from '#src/test-utils/nanoid.js';
 
-import { DataHookContextManager } from './context-manager.js';
 import { generateHookTestPayload, parseResponse } from './utils.js';
 
 const { jest } = import.meta;
@@ -60,6 +59,7 @@ const mockHookState = { requestCount: 100, successCount: 10 };
 const getHookExecutionStatsByHookId = jest.fn().mockResolvedValue(mockHookState);
 const findAllHooks = jest.fn().mockResolvedValue([hook, dataHook]);
 const findHookById = jest.fn().mockResolvedValue(hook);
+const findApplicationById = jest.fn().mockResolvedValue({ id: 'app_id', extraField: 'not_ok' });
 
 const { createHookLibrary } = await import('./index.js');
 const { triggerInteractionHooks, triggerTestHook, triggerDataHooks } = createHookLibrary(
@@ -72,13 +72,17 @@ const { triggerInteractionHooks, triggerTestHook, triggerDataHooks } = createHoo
       }),
     },
     applications: {
-      findApplicationById: jest.fn().mockResolvedValue({ id: 'app_id', extraField: 'not_ok' }),
+      findApplicationById,
     },
     logs: { insertLog, getHookExecutionStatsByHookId },
     hooks: { findAllHooks, findHookById },
   })
 );
 
+const { DataHookContextManager, InteractionHookContextManager } = await import(
+  './context-manager.js'
+);
+
 describe('triggerInteractionHooks()', () => {
   afterEach(() => {
     jest.clearAllMocks();
@@ -87,20 +91,27 @@ describe('triggerInteractionHooks()', () => {
   it('should set correct payload when hook triggered', async () => {
     jest.useFakeTimers().setSystemTime(100_000);
 
-    await triggerInteractionHooks(
-      new ConsoleLog(),
-      { event: InteractionEvent.SignIn, sessionId: 'some_jti', applicationId: 'some_client' },
-      { userId: '123' }
-    );
+    const interactionHookContext = new InteractionHookContextManager({
+      interactionEvent: InteractionEvent.SignIn,
+      applicationId: 'some_client',
+      sessionId: 'some_jti',
+    });
+
+    interactionHookContext.assignInteractionHookResult({
+      userId: '123',
+    });
+
+    await triggerInteractionHooks(new ConsoleLog(), interactionHookContext);
 
     expect(findAllHooks).toHaveBeenCalled();
+    expect(findApplicationById).toHaveBeenCalledWith('some_client');
     expect(sendWebhookRequest).toHaveBeenCalledWith({
       hookConfig: hook.config,
       payload: {
         hookId: 'foo',
         event: 'PostSignIn',
         interactionEvent: 'SignIn',
-        sessionId: 'some_jti',
+        sessionId: interactionHookContext.metadata.sessionId,
         userId: '123',
         user: { id: 'user_id', username: 'user' },
         application: { id: 'app_id' },
@@ -180,22 +191,22 @@ describe('triggerDataHooks()', () => {
     jest.clearAllMocks();
   });
 
-  it('should set correct payload when hook triggered', async () => {
+  it('should set correct payload when hook triggered by management API', async () => {
     jest.useFakeTimers().setSystemTime(100_000);
 
     const metadata = { userAgent: 'ua', ip: 'ip' };
-    const hookData = { path: '/test', method: 'POST', body: { success: true } };
+    const hookData = { path: '/test', method: 'POST', data: { success: true } };
 
     const hooksManager = new DataHookContextManager(metadata);
     hooksManager.appendContext({
       event: 'Role.Created',
-      data: hookData,
+      ...hookData,
     });
 
     await triggerDataHooks(new ConsoleLog(), hooksManager);
 
     expect(findAllHooks).toHaveBeenCalled();
-
+    expect(findApplicationById).not.toHaveBeenCalled();
     expect(sendWebhookRequest).toHaveBeenCalledWith({
       hookConfig: dataHook.config,
       payload: {
@@ -232,4 +243,40 @@ describe('triggerDataHooks()', () => {
 
     jest.useRealTimers();
   });
+
+  it('should set correct payload when hook triggered by interaction API', async () => {
+    jest.useFakeTimers().setSystemTime(100_000);
+
+    const metadata = {
+      userAgent: 'ua',
+      ip: 'ip',
+      interactionEvent: InteractionEvent.Register,
+      applicationId: 'some_client',
+      sessionId: 'some_jti',
+    };
+
+    const hooksManager = new DataHookContextManager(metadata);
+
+    hooksManager.appendContext({
+      event: 'Role.Created',
+      data: { id: 'user_id', username: 'user' },
+    });
+
+    await triggerDataHooks(new ConsoleLog(), hooksManager);
+
+    expect(findAllHooks).toHaveBeenCalled();
+    expect(findApplicationById).toHaveBeenCalledWith('some_client');
+    expect(sendWebhookRequest).toHaveBeenCalledWith({
+      hookConfig: dataHook.config,
+      payload: {
+        hookId: 'foo',
+        event: 'Role.Created',
+        createdAt: new Date(100_000).toISOString(),
+        data: { id: 'user_id', username: 'user' },
+        ...metadata,
+        application: { id: 'app_id' },
+      },
+      signingKey: dataHook.signingKey,
+    });
+  });
 });

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

@@ -1,13 +1,10 @@
 import {
   LogResult,
   userInfoSelectFields,
-  type DataHookEventPayload,
   type Hook,
   type HookConfig,
   type HookEvent,
-  type HookEventPayload,
   type HookTestErrorResponseData,
-  type InteractionHookEventPayload,
 } from '@logto/schemas';
 import { generateStandardId, normalizeError, type ConsoleLog } from '@logto/shared';
 import { conditional, pick, trySafe } from '@silverhand/essentials';
@@ -18,12 +15,15 @@ import RequestError from '#src/errors/RequestError/index.js';
 import { LogEntry } from '#src/middleware/koa-audit-log.js';
 import type Queries from '#src/tenants/Queries.js';
 
-import { type DataHookContextManager } from './context-manager.js';
 import {
-  interactionEventToHookEvent,
-  type InteractionHookContext,
-  type InteractionHookResult,
-} from './types.js';
+  type DataHookContextManager,
+  type InteractionHookContextManager,
+} from './context-manager.js';
+import type {
+  DataHookEventPayload,
+  HookEventPayload,
+  InteractionHookEventPayload,
+} from './type.js';
 import { generateHookTestPayload, parseResponse, sendWebhookRequest } from './utils.js';
 
 type BetterOmit<T, Ignore> = {
@@ -103,15 +103,19 @@ export const createHookLibrary = (queries: Queries) => {
    */
   const triggerInteractionHooks = async (
     consoleLog: ConsoleLog,
-    interactionContext: InteractionHookContext,
-    interactionResult: InteractionHookResult,
-    userAgent?: string
+    contextManager: InteractionHookContextManager
   ) => {
-    const { userId } = interactionResult;
-    const { event, sessionId, applicationId, userIp } = interactionContext;
+    if (!contextManager.interactionHookResult) {
+      return;
+    }
+
+    const { interactionEvent, sessionId, applicationId, userIp, userAgent } =
+      contextManager.metadata;
+    const { userId } = contextManager.interactionHookResult;
+    const { hookEvent } = contextManager;
 
-    const hookEvent = interactionEventToHookEvent[event];
     const found = await findAllHooks();
+
     const hooks = found.filter(
       ({ event, events, enabled }) =>
         enabled && (events.length > 0 ? events.includes(hookEvent) : event === hookEvent) // For backward compatibility
@@ -128,7 +132,7 @@ export const createHookLibrary = (queries: Queries) => {
 
     const payload = {
       event: hookEvent,
-      interactionEvent: event,
+      interactionEvent,
       createdAt: new Date().toISOString(),
       sessionId,
       userAgent,
@@ -157,8 +161,14 @@ export const createHookLibrary = (queries: Queries) => {
 
     const found = await findAllHooks();
 
+    // Fetch application detail if available
+    const { applicationId } = contextManager.metadata;
+    const application = applicationId
+      ? await trySafe(async () => findApplicationById(applicationId))
+      : undefined;
+
     // Filter hooks that match each events
-    const webhooks = contextManager.contextArray.flatMap(({ event, data }) => {
+    const webhooks = contextManager.contextArray.flatMap(({ event, ...rest }) => {
       const hooks = found.filter(
         ({ event: hookEvent, events, enabled }) =>
           enabled && (events.length > 0 ? events.includes(event) : event === hookEvent)
@@ -168,7 +178,10 @@ export const createHookLibrary = (queries: Queries) => {
         event,
         createdAt: new Date().toISOString(),
         ...contextManager.metadata,
-        ...data,
+        ...conditional(
+          application && { application: pick(application, 'id', 'type', 'name', 'description') }
+        ),
+        ...rest,
       } satisfies BetterOmit<DataHookEventPayload, 'hookId'>;
 
       return hooks.map((hook) => ({ hook, payload }));

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

@@ -0,0 +1,74 @@
+import {
+  type Application,
+  type DataHookEvent,
+  type InteractionEvent,
+  type InteractionHookEvent,
+  type User,
+  type userInfoSelectFields,
+} from '@logto/schemas';
+
+/**
+ * The interaction API context for triggering InteractionHook and DataHook events.
+ * 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 InteractionApiMetadata = {
+  /** The application ID if the hook is triggered by interaction API. */
+  applicationId?: string;
+  /** The session ID if the hook is triggered by interaction API. */
+  sessionId?: string;
+  /** The InteractionEvent if the hook is triggered by interaction API. */
+  interactionEvent: InteractionEvent;
+};
+
+type InteractionApiContextPayload = {
+  /** Fetch application detail by application ID before sending the hook event */
+  application?: Pick<Application, 'id' | 'type' | 'name' | 'description'>;
+  sessionId?: string;
+  interactionEvent?: InteractionEvent;
+};
+
+export type InteractionHookEventPayload = {
+  event: InteractionHookEvent;
+  createdAt: string;
+  hookId: string;
+  userAgent?: string;
+  userIp?: string;
+  /** InteractionHook result */
+  userId?: string;
+  /** Fetch user detail by user ID before sending the hook event */
+  user?: Pick<User, (typeof userInfoSelectFields)[number]>;
+} & InteractionApiContextPayload &
+  Record<string, unknown>;
+
+/**
+ * The API context for management API triggered data hooks.
+ * In the `koaManagementApiHooks` middleware,
+ * we will store the context of management API requests that triggers the DataHook events.
+ * Can't put it in the DataHookMetadata because the matched API context is only available after the request is processed.
+ */
+export type ManagementApiContext = {
+  /** Request route params. */
+  params?: Record<string, string>;
+  /** Request route path. */
+  path: string;
+  /** Matched route used as the identifier to trigger the hook. */
+  matchedRoute?: string;
+  /** Request method. */
+  method: string;
+  /** Response status code. */
+  status: number;
+};
+
+export type DataHookEventPayload = {
+  event: DataHookEvent;
+  createdAt: string;
+  hookId: string;
+  ip?: string;
+  userAgent?: string;
+  data?: unknown;
+} & Partial<InteractionApiContextPayload> &
+  Partial<ManagementApiContext> &
+  Record<string, unknown>;
+
+export type HookEventPayload = InteractionHookEventPayload | DataHookEventPayload;

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

@@ -1,28 +0,0 @@
-import { InteractionEvent, InteractionHookEvent } from '@logto/schemas';
-
-/**
- * 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;
-  userIp?: 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;
-};
-
-export const interactionEventToHookEvent: Record<InteractionEvent, InteractionHookEvent> = {
-  [InteractionEvent.Register]: InteractionHookEvent.PostRegister,
-  [InteractionEvent.SignIn]: InteractionHookEvent.PostSignIn,
-  [InteractionEvent.ForgotPassword]: InteractionHookEvent.PostResetPassword,
-};

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

@@ -3,7 +3,6 @@ import {
   managementApiHooksRegistration,
   type HookConfig,
   type HookEvent,
-  type HookEventPayload,
 } from '@logto/schemas';
 import { conditional, trySafe } from '@silverhand/essentials';
 import { type IRouterParamContext } from 'koa-router';
@@ -11,6 +10,8 @@ import ky, { type KyResponse } from 'ky';
 
 import { sign } from '#src/utils/sign.js';
 
+import { type HookEventPayload } from './type.js';
+
 export const parseResponse = async (response: KyResponse) => {
   const body = await response.text();
   return {

packages/core/src/middleware/koa-management-api-hooks.test.ts

@@ -46,6 +46,7 @@ describe('koaManagementApiHooks', () => {
     expect(triggerDataHooks).toBeCalledWith(
       expect.any(ConsoleLog),
       expect.objectContaining({
+        metadata: { userAgent: ctx.header['user-agent'], ip: ctx.ip },
         contextArray: [
           {
             event: 'Role.Created',
@@ -89,16 +90,12 @@ describe('koaManagementApiHooks', () => {
           contextArray: [
             {
               event,
-              data: {
-                path: route,
-                method,
-                response: {
-                  body: { key },
-                },
-                params: ctxParams.params,
-                matchedRoute: route,
-                status: 200,
-              },
+              data: { key },
+              path: route,
+              method,
+              params: ctxParams.params,
+              matchedRoute: route,
+              status: 200,
             },
           ],
         })

packages/core/src/middleware/koa-management-api-hooks.ts

@@ -38,17 +38,16 @@ export const koaManagementApiHooks = <StateT, ContextT extends IRouterParamConte
       ip,
     } = ctx;
 
-    const dataHooks = new DataHookContextManager({ userAgent, ip });
+    const dataHooksContextManager = new DataHookContextManager({ userAgent, ip });
 
     /**
      * Append a hook context to trigger management hooks. If multiple contexts are appended, all of
      * them will be triggered.
      */
-    ctx.appendDataHookContext = dataHooks.appendContext.bind(dataHooks);
+    ctx.appendDataHookContext = dataHooksContextManager.appendContext.bind(dataHooksContextManager);
 
     await next();
 
-    // Auto append pre-registered management API hooks if any
     const {
       path,
       method,
@@ -58,20 +57,30 @@ export const koaManagementApiHooks = <StateT, ContextT extends IRouterParamConte
       response: { body },
     } = ctx;
 
-    const hookRegistrationKey = buildManagementApiDataHookRegistrationKey(method, matchedRoute);
+    const hookRegistrationMatchedRouteKey = buildManagementApiDataHookRegistrationKey(
+      method,
+      matchedRoute
+    );
 
-    if (hasRegisteredDataHookEvent(hookRegistrationKey)) {
-      const event = managementApiHooksRegistration[hookRegistrationKey];
+    // Auto append pre-registered management API hooks if any
+    if (hasRegisteredDataHookEvent(hookRegistrationMatchedRouteKey)) {
+      const event = managementApiHooksRegistration[hookRegistrationMatchedRouteKey];
 
-      dataHooks.appendContext({
+      dataHooksContextManager.appendContext({
         event,
-        data: { path, method, response: { body }, status, params, matchedRoute },
+        path,
+        method,
+        status,
+        params,
+        matchedRoute: matchedRoute && String(matchedRoute),
+        data: body,
       });
     }
 
-    if (dataHooks.contextArray.length > 0) {
+    // Trigger data hooks
+    if (dataHooksContextManager.contextArray.length > 0) {
       // Hooks should not crash the app
-      void trySafe(hooks.triggerDataHooks(getConsoleLogFromContext(ctx), dataHooks));
+      void trySafe(hooks.triggerDataHooks(getConsoleLogFromContext(ctx), dataHooksContextManager));
     }
   };
 };

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

@@ -1,11 +1,11 @@
-import { conditionalString, trySafe, type Optional } from '@silverhand/essentials';
+import { conditionalString, trySafe } from '@silverhand/essentials';
 import type { MiddlewareType } from 'koa';
 import type { IRouterParamContext } from 'koa-router';
 
 import {
-  type InteractionHookContext,
+  InteractionHookContextManager,
   type InteractionHookResult,
-} from '#src/libraries/hook/types.js';
+} from '#src/libraries/hook/context-manager.js';
 import type Libraries from '#src/tenants/Libraries.js';
 import { getConsoleLogFromContext } from '#src/utils/console.js';
 
@@ -32,7 +32,7 @@ export default function koaInteractionHooks<
   hooks: { triggerInteractionHooks },
 }: Libraries): MiddlewareType<StateT, WithInteractionHooksContext<ContextT>, ResponseT> {
   return async (ctx, next) => {
-    const { event } = getInteractionStorage(ctx.interactionDetails.result);
+    const { event: interactionEvent } = getInteractionStorage(ctx.interactionDetails.result);
 
     const {
       interactionDetails,
@@ -40,41 +40,24 @@ export default function koaInteractionHooks<
       ip,
     } = ctx;
 
-    // Predefined interaction hook context
-    const interactionHookContext: InteractionHookContext = {
-      event,
-      sessionId: interactionDetails.jti,
-      applicationId: conditionalString(interactionDetails.params.client_id),
+    const interactionHookContext = new InteractionHookContextManager({
+      interactionEvent,
+      userAgent,
       userIp: ip,
-    };
+      applicationId: conditionalString(interactionDetails.params.client_id),
+      sessionId: interactionDetails.jti,
+    });
 
-    // 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;
-    };
+    ctx.assignInteractionHookResult =
+      interactionHookContext.assignInteractionHookResult.bind(interactionHookContext);
 
     // TODO: @simeng-li Add DataHookContext to the interaction hook middleware as well
 
     await next();
 
-    if (interactionHookResult) {
+    if (interactionHookContext.interactionHookResult) {
       // Hooks should not crash the app
-      void trySafe(
-        triggerInteractionHooks(
-          getConsoleLogFromContext(ctx),
-          interactionHookContext,
-          interactionHookResult,
-          userAgent
-        )
-      );
+      void trySafe(triggerInteractionHooks(getConsoleLogFromContext(ctx), interactionHookContext));
     }
   };
 }

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

@@ -36,7 +36,7 @@ const mockHookResponseGuard = z.object({
     event: hookEventGuard,
     createdAt: z.string(),
     hookId: z.string(),
-    body: jsonGuard.optional(),
+    data: jsonGuard.optional(),
     method: z
       .string()
       .optional()

packages/schemas/src/types/hook.ts

@@ -1,45 +1,6 @@
 import { z } from 'zod';
 
-import { Hooks, type Application, type User } from '../db-entries/index.js';
-import { type DataHookEvent, type InteractionHookEvent } from '../foundations/index.js';
-
-import type { userInfoSelectFields } from './user.js';
-
-export type InteractionHookEventPayload = {
-  event: InteractionHookEvent;
-  createdAt: string;
-  hookId: string;
-  sessionId?: string;
-  userAgent?: string;
-  userId?: string;
-  userIp?: string;
-  user?: Pick<User, (typeof userInfoSelectFields)[number]>;
-  application?: Pick<Application, 'id' | 'type' | 'name' | 'description'>;
-} & Record<string, unknown>;
-
-export type DataHookEventPayload = {
-  event: DataHookEvent;
-  createdAt: string;
-  hookId: string;
-  ip?: string;
-  userAgent?: string;
-  /** An object that contains response data. */
-  response?: {
-    body?: Record<string, unknown>;
-  };
-  /** Request route params. */
-  params?: Record<string, string>;
-  /** Request route path. */
-  path?: string;
-  /** Matched route used as the identifier to trigger the hook. */
-  matchedRoute?: string;
-  /** Response status code. */
-  status?: number;
-  /** Request method. */
-  method?: string;
-} & Record<string, unknown>;
-
-export type HookEventPayload = InteractionHookEventPayload | DataHookEventPayload;
+import { Hooks } from '../db-entries/index.js';
 
 const hookExecutionStatsGuard = z.object({
   successCount: z.number(),