feat(core,schemas): make it type-safer to log (#656)

packages/core/package.json

@@ -25,6 +25,7 @@
     "@silverhand/essentials": "^1.1.0",
     "dayjs": "^1.10.5",
     "decamelize": "^5.0.0",
+    "deepmerge": "^4.2.2",
     "dotenv": "^16.0.0",
     "got": "^11.8.2",
     "i18next": "^21.0.0",

packages/core/src/middleware/koa-log.test.ts

@@ -1,12 +1,14 @@
-import { LogType, LogResult } from '@logto/schemas';
+import { LogPayload, LogResult } from '@logto/schemas';
 
 import { insertLog } from '@/queries/log';
 import { createContextWithRouteParameters } from '@/utils/test-utils';
 
-import koaLog, { WithLogContext, LogContext } from './koa-log';
+import koaLog, { WithLogContext } from './koa-log';
 
 const nanoIdMock = 'mockId';
 
+const log = jest.fn();
+
 jest.mock('@/queries/log', () => ({
   insertLog: jest.fn(async () => Promise.resolve()),
 }));
@@ -19,11 +21,10 @@ describe('koaLog middleware', () => {
   const insertLogMock = insertLog as jest.Mock;
   const next = jest.fn();
 
-  const logMock: Partial<LogContext> = {
-    type: LogType.SignInUsernamePassword,
-    applicationId: 'foo',
+  const type = 'SignInUsernamePassword';
+  const payload: LogPayload = {
     userId: 'foo',
-    username: 'Foo Bar',
+    username: 'Bar',
   };
 
   afterEach(() => {
@@ -33,21 +34,20 @@ describe('koaLog middleware', () => {
   it('insert log with success response', async () => {
     const ctx: WithLogContext<ReturnType<typeof createContextWithRouteParameters>> = {
       ...createContextWithRouteParameters(),
-      log: {}, // Bypass middleware context type assert
+      log, // Bypass middleware context type assert
     };
 
     next.mockImplementationOnce(async () => {
-      ctx.log = logMock;
+      ctx.log(type, payload);
     });
 
     await koaLog()(ctx, next);
 
-    const { type, ...rest } = logMock;
     expect(insertLogMock).toBeCalledWith({
       id: nanoIdMock,
       type,
       payload: {
-        ...rest,
+        ...payload,
         result: LogResult.Success,
       },
     });
@@ -56,7 +56,7 @@ describe('koaLog middleware', () => {
   it('should not block request if insertLog throws error', async () => {
     const ctx: WithLogContext<ReturnType<typeof createContextWithRouteParameters>> = {
       ...createContextWithRouteParameters(),
-      log: {}, // Bypass middleware context type assert
+      log, // Bypass middleware context type assert
     };
 
     const error = new Error('Failed to insert log');
@@ -65,17 +65,16 @@ describe('koaLog middleware', () => {
     });
 
     next.mockImplementationOnce(async () => {
-      ctx.log = logMock;
+      ctx.log(type, payload);
     });
 
     await koaLog()(ctx, next);
 
-    const { type, ...rest } = logMock;
     expect(insertLogMock).toBeCalledWith({
       id: nanoIdMock,
       type,
       payload: {
-        ...rest,
+        ...payload,
         result: LogResult.Success,
       },
     });
@@ -84,24 +83,23 @@ describe('koaLog middleware', () => {
   it('should insert log with failed result if next throws error', async () => {
     const ctx: WithLogContext<ReturnType<typeof createContextWithRouteParameters>> = {
       ...createContextWithRouteParameters(),
-      log: {}, // Bypass middleware context type assert
+      log, // Bypass middleware context type assert
     };
 
     const error = new Error('next error');
 
     next.mockImplementationOnce(async () => {
-      ctx.log = logMock;
+      ctx.log(type, payload);
       throw error;
     });
 
     await expect(koaLog()(ctx, next)).rejects.toMatchError(error);
 
-    const { type, ...rest } = logMock;
     expect(insertLogMock).toBeCalledWith({
       id: nanoIdMock,
       type,
       payload: {
-        ...rest,
+        ...payload,
         result: LogResult.Error,
         error: String(error),
       },

packages/core/src/middleware/koa-log.ts

@@ -1,33 +1,21 @@
-import { LogResult, LogType } from '@logto/schemas';
-import { Context, MiddlewareType } from 'koa';
+import { LogPayload, LogPayloads, LogResult, LogType } from '@logto/schemas';
+import { Optional } from '@silverhand/essentials';
+import deepmerge from 'deepmerge';
+import { MiddlewareType } from 'koa';
 import { nanoid } from 'nanoid';
 
 import { insertLog } from '@/queries/log';
 
 export type WithLogContext<ContextT> = ContextT & {
-  log: LogContext;
+  log: <T extends LogType>(type: T, payload: LogPayloads[T]) => void;
 };
 
-export interface LogContext {
-  [key: string]: unknown;
-  type?: LogType;
-}
-
-const log = async (ctx: WithLogContext<Context>, result: LogResult) => {
-  const { type, ...rest } = ctx.log;
-
-  if (!type) {
-    return;
-  }
-
+const saveLog = async (type: LogType, payload: LogPayload) => {
   try {
     await insertLog({
       id: nanoid(),
       type,
-      payload: {
-        ...rest,
-        result,
-      },
+      payload,
     });
   } catch (error: unknown) {
     console.error('An error occurred while inserting log');
@@ -41,14 +29,33 @@ export default function koaLog<StateT, ContextT, ResponseBodyT>(): MiddlewareTyp
   ResponseBodyT
 > {
   return async (ctx, next) => {
-    ctx.log = {};
+    // eslint-disable-next-line @silverhand/fp/no-let
+    let logType: Optional<LogType>;
+    // eslint-disable-next-line @silverhand/fp/no-let
+    let logPayload: LogPayload = {};
+
+    ctx.log = (type, payload) => {
+      if (logType !== type) {
+        // eslint-disable-next-line @silverhand/fp/no-mutation
+        logPayload = {}; // Reset payload when type changes
+      }
+
+      // eslint-disable-next-line @silverhand/fp/no-mutation
+      logType = type; // Use first initialized log type
+      // eslint-disable-next-line @silverhand/fp/no-mutation
+      logPayload = deepmerge(logPayload, payload);
+    };
 
     try {
       await next();
-      await log(ctx, LogResult.Success);
+
+      if (logType) {
+        await saveLog(logType, { ...logPayload, result: LogResult.Success });
+      }
     } catch (error: unknown) {
-      ctx.log.error = String(error);
-      await log(ctx, LogResult.Error);
+      if (logType) {
+        await saveLog(logType, { ...logPayload, result: LogResult.Error, error: String(error) });
+      }
       throw error;
     }
   };

packages/core/src/routes/session.test.ts

@@ -153,7 +153,7 @@ describe('sessionRoutes', () => {
     provider: new Provider(''),
     middlewares: [
       async (ctx, next) => {
-        ctx.log = {};
+        ctx.log = jest.fn();
 
         return next();
       },

packages/core/src/routes/session.ts

@@ -2,7 +2,7 @@
 import path from 'path';
 
 import { LogtoErrorCode } from '@logto/phrases';
-import { LogType, PasscodeType, userInfoSelectFields } from '@logto/schemas';
+import { PasscodeType, userInfoSelectFields } from '@logto/schemas';
 import { conditional } from '@silverhand/essentials';
 import pick from 'lodash.pick';
 import { Provider } from 'oidc-provider';
@@ -67,13 +67,14 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
       }),
     }),
     async (ctx, next) => {
+      const { jti } = await provider.interactionDetails(ctx.req, ctx.res);
       const { username, password } = ctx.guard.body;
-      ctx.log.type = LogType.SignInUsernamePassword;
-      ctx.log.username = username;
+      const type = 'SignInUsernamePassword';
+      ctx.log(type, { sessionId: jti, username });
       assertThat(password, 'session.insufficient_info');
 
       const { id } = await findUserByUsernameAndPassword(username, password);
-      ctx.log.userId = id;
+      ctx.log(type, { userId: id });
       await assignInteractionResults(ctx, provider, { login: { accountId: id } });
 
       return next();
@@ -84,12 +85,10 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
     '/session/sign-in/passwordless/sms/send-passcode',
     koaGuard({ body: object({ phone: string().regex(phoneRegEx) }) }),
     async (ctx, next) => {
-      const { phone } = ctx.guard.body;
-      ctx.log.type = LogType.SignInSmsSendPasscode;
-      ctx.log.phone = phone;
-
       const { jti } = await provider.interactionDetails(ctx.req, ctx.res);
-      ctx.log.sessionId = jti;
+      const { phone } = ctx.guard.body;
+      const type = 'SignInSmsSendPasscode';
+      ctx.log(type, { sessionId: jti, phone });
 
       assertThat(
         await hasUserWithPhone(phone),
@@ -97,7 +96,7 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
       );
 
       const passcode = await createPasscode(jti, PasscodeType.SignIn, { phone });
-      ctx.log.passcode = passcode;
+      ctx.log(type, { passcode });
 
       await sendPasscode(passcode);
       ctx.status = 204;
@@ -110,13 +109,10 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
     '/session/sign-in/passwordless/sms/verify-passcode',
     koaGuard({ body: object({ phone: string().regex(phoneRegEx), code: string() }) }),
     async (ctx, next) => {
-      const { phone, code } = ctx.guard.body;
-      ctx.log.type = LogType.SignInSms;
-      ctx.log.phone = phone;
-      ctx.log.passcode = code;
-
       const { jti } = await provider.interactionDetails(ctx.req, ctx.res);
-      ctx.log.sessionId = jti;
+      const { phone, code } = ctx.guard.body;
+      const type = 'SignInSms';
+      ctx.log(type, { sessionId: jti, phone, code });
 
       assertThat(
         await hasUserWithPhone(phone),
@@ -125,7 +121,7 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
 
       await verifyPasscode(jti, PasscodeType.SignIn, code, { phone });
       const { id } = await findUserByPhone(phone);
-      ctx.log.userId = id;
+      ctx.log(type, { userId: id });
 
       await assignInteractionResults(ctx, provider, { login: { accountId: id } });
 
@@ -137,12 +133,10 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
     '/session/sign-in/passwordless/email/send-passcode',
     koaGuard({ body: object({ email: string().regex(emailRegEx) }) }),
     async (ctx, next) => {
-      const { email } = ctx.guard.body;
-      ctx.log.type = LogType.SignInEmailSendPasscode;
-      ctx.log.email = email;
-
       const { jti } = await provider.interactionDetails(ctx.req, ctx.res);
-      ctx.log.sessionId = jti;
+      const { email } = ctx.guard.body;
+      const type = 'SignInEmailSendPasscode';
+      ctx.log(type, { sessionId: jti, email });
 
       assertThat(
         await hasUserWithEmail(email),
@@ -150,7 +144,7 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
       );
 
       const passcode = await createPasscode(jti, PasscodeType.SignIn, { email });
-      ctx.log.passcode = passcode;
+      ctx.log(type, { passcode });
 
       await sendPasscode(passcode);
       ctx.status = 204;
@@ -163,13 +157,10 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
     '/session/sign-in/passwordless/email/verify-passcode',
     koaGuard({ body: object({ email: string().regex(emailRegEx), code: string() }) }),
     async (ctx, next) => {
-      const { email, code } = ctx.guard.body;
-      ctx.log.type = LogType.SignInEmail;
-      ctx.log.email = email;
-      ctx.log.passcode = code;
-
       const { jti } = await provider.interactionDetails(ctx.req, ctx.res);
-      ctx.log.sessionId = jti;
+      const { email, code } = ctx.guard.body;
+      const type = 'SignInEmail';
+      ctx.log(type, { sessionId: jti, email, code });
 
       assertThat(
         await hasUserWithEmail(email),
@@ -178,7 +169,7 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
 
       await verifyPasscode(jti, PasscodeType.SignIn, code, { email });
       const { id } = await findUserByEmail(email);
-      ctx.log.userId = id;
+      ctx.log(type, { userId: id });
 
       await assignInteractionResults(ctx, provider, { login: { accountId: id } });
 
@@ -198,11 +189,8 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
     }),
     async (ctx, next) => {
       const { connectorId, code, state, redirectUri } = ctx.guard.body;
-      ctx.log.type = LogType.SignInSocial;
-      ctx.log.connectorId = connectorId;
-      ctx.log.code = code;
-      ctx.log.state = state;
-      ctx.log.redirectUri = redirectUri;
+      const type = 'SignInSocial';
+      ctx.log(type, { connectorId, code, state, redirectUri });
 
       if (!code) {
         assertThat(state && redirectUri, 'session.insufficient_info');
@@ -210,13 +198,13 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
         assertThat(connector.connector.enabled, 'connector.not_enabled');
         const redirectTo = await connector.getAuthorizationUri(redirectUri, state);
         ctx.body = { redirectTo };
-        ctx.log.redirectTo = redirectTo;
+        ctx.log(type, { redirectTo });
 
         return next();
       }
 
       const userInfo = await getUserInfoByAuthCode(connectorId, code, redirectUri);
-      ctx.log.userInfo = userInfo;
+      ctx.log(type, { userInfo });
 
       if (!(await hasUserWithIdentity(connectorId, userInfo.id))) {
         await assignInteractionResults(ctx, provider, { connectorId, userInfo }, true);
@@ -231,7 +219,7 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
       }
 
       const { id, identities } = await findUserByIdentity(connectorId, userInfo.id);
-      ctx.log.userId = id;
+      ctx.log(type, { userId: id });
 
       // Update social connector's user info
       await updateUserById(id, {
@@ -249,21 +237,21 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
       body: object({ connectorId: string() }),
     }),
     async (ctx, next) => {
-      const { connectorId } = ctx.guard.body;
-      ctx.log.type = LogType.SignInSocialBind;
-      ctx.log.connectorId = connectorId;
-
-      const { result } = await provider.interactionDetails(ctx.req, ctx.res);
+      const { jti, result } = await provider.interactionDetails(ctx.req, ctx.res);
       assertThat(result, 'session.connector_session_not_found');
 
+      const { connectorId } = ctx.guard.body;
+      const type = 'SignInSocialBind';
+      ctx.log(type, { sessionId: jti, connectorId });
+
       const userInfo = await getUserInfoFromInteractionResult(connectorId, result);
-      ctx.log.userInfo = userInfo;
+      ctx.log(type, { userInfo });
 
       const relatedInfo = await findSocialRelatedUser(userInfo);
       assertThat(relatedInfo, 'session.connector_session_not_found');
 
       const { id, identities } = relatedInfo[1];
-      ctx.log.userId = id;
+      ctx.log(type, { userId: id });
 
       await updateUserById(id, {
         identities: { ...identities, [connectorId]: { userId: userInfo.id, details: userInfo } },

packages/schemas/src/foundations/jsonb-types.ts

@@ -5,7 +5,9 @@ import { z } from 'zod';
  * Commonly Used
  */
 
-export const arbitraryObjectGuard = z.object({}).catchall(z.unknown());
+// Cannot declare `z.object({}).catchall(z.unknown().optional())` to guard `{ [key: string]?: unknown }` (invalid type),
+// so do it another way to guard `{ [x: string]: unknown; } | {}`.
+export const arbitraryObjectGuard = z.union([z.object({}).catchall(z.unknown()), z.object({})]);
 
 export type ArbitraryObject = z.infer<typeof arbitraryObjectGuard>;
 

packages/schemas/src/types/log.ts

@@ -1,14 +1,66 @@
-export enum LogType {
-  SignInUsernamePassword = 'SignInUsernamePassword',
-  SignInEmail = 'SignInEmail',
-  SignInEmailSendPasscode = 'SignInEmailSendPasscode',
-  SignInSms = 'SignInSms',
-  SignInSmsSendPasscode = 'SignInSmsSendPasscode',
-  SignInSocial = 'SignInSocial',
-  SignInSocialBind = 'SignInSocialBind',
-}
+import { Passcode } from '../db-entries';
 
 export enum LogResult {
   Success = 'Success',
   Error = 'Error',
 }
+
+interface BaseLogPayload {
+  sessionId?: string;
+  result?: LogResult;
+  error?: string;
+}
+
+interface SignInUsernamePasswordLogPayload extends BaseLogPayload {
+  userId?: string;
+  username?: string;
+}
+
+interface SignInEmailSendPasscodeLogPayload extends BaseLogPayload {
+  email?: string;
+  passcode?: Passcode;
+}
+
+interface SignInEmailLogPayload extends BaseLogPayload {
+  email?: string;
+  code?: string;
+  userId?: string;
+}
+
+interface SignInSmsSendPasscodeLogPayload extends BaseLogPayload {
+  phone?: string;
+  passcode?: Passcode;
+}
+
+interface SignInSmsLogPayload extends BaseLogPayload {
+  phone?: string;
+  code?: string;
+  userId?: string;
+}
+
+interface SignInSocialBindLogPayload extends BaseLogPayload {
+  connectorId?: string;
+  userInfo?: object;
+  userId?: string;
+}
+
+interface SignInSocialLogPayload extends SignInSocialBindLogPayload {
+  code?: string;
+  state?: string;
+  redirectUri?: string;
+  redirectTo?: string;
+}
+
+export type LogPayloads = {
+  SignInUsernamePassword: SignInUsernamePasswordLogPayload;
+  SignInEmailSendPasscode: SignInEmailSendPasscodeLogPayload;
+  SignInEmail: SignInEmailLogPayload;
+  SignInSmsSendPasscode: SignInSmsSendPasscodeLogPayload;
+  SignInSms: SignInSmsLogPayload;
+  SignInSocialBind: SignInSocialBindLogPayload;
+  SignInSocial: SignInSocialLogPayload;
+};
+
+export type LogType = keyof LogPayloads;
+
+export type LogPayload = LogPayloads[LogType];

pnpm-lock.yaml

@@ -273,6 +273,7 @@ importers:
       copyfiles: ^2.4.1
       dayjs: ^1.10.5
       decamelize: ^5.0.0
+      deepmerge: ^4.2.2
       dotenv: ^16.0.0
       eslint: ^8.10.0
       got: ^11.8.2
@@ -314,6 +315,7 @@ importers:
       '@silverhand/essentials': 1.1.2
       dayjs: 1.10.7
       decamelize: 5.0.1
+      deepmerge: 4.2.2
       dotenv: 16.0.0
       got: 11.8.3
       i18next: 21.6.12