refactor(core)!: update koaAuth() to inject detailed auth info (#1977)

* refactor(core)!: update `koaAuth()` to inject detailed auth info

* test(core): add auth context to unit test requester

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

@@ -18,10 +18,12 @@ describe('koaAuth middleware', () => {
 
   const ctx: WithAuthContext<Context & IRouterParamContext> = {
     ...baseCtx,
-    auth: '',
+    auth: {
+      type: 'user',
+      id: '',
+    },
   };
 
-  const unauthorizedError = new RequestError({ code: 'auth.unauthorized', status: 401 });
   const jwtSubMissingError = new RequestError({ code: 'auth.jwt_sub_missing', status: 401 });
   const authHeaderMissingError = new RequestError({
     code: 'auth.authorization_header_missing',
@@ -39,7 +41,10 @@ describe('koaAuth middleware', () => {
   const next = jest.fn();
 
   beforeEach(() => {
-    ctx.auth = '';
+    ctx.auth = {
+      type: 'user',
+      id: '',
+    };
     ctx.request = baseCtx.request;
     jest.resetModules();
   });
@@ -50,7 +55,7 @@ describe('koaAuth middleware', () => {
       .mockReturnValue({ ...envSet.values, developmentUserId: 'foo' });
 
     await koaAuth()(ctx, next);
-    expect(ctx.auth).toEqual('foo');
+    expect(ctx.auth).toEqual({ type: 'user', id: 'foo' });
 
     spy.mockRestore();
   });
@@ -65,7 +70,7 @@ describe('koaAuth middleware', () => {
     };
 
     await koaAuth()(mockCtx, next);
-    expect(mockCtx.auth).toEqual('foo');
+    expect(mockCtx.auth).toEqual({ type: 'user', id: 'foo' });
   });
 
   it('should read DEVELOPMENT_USER_ID from env variable first if is in production and integration test', async () => {
@@ -77,7 +82,7 @@ describe('koaAuth middleware', () => {
     });
 
     await koaAuth()(ctx, next);
-    expect(ctx.auth).toEqual('foo');
+    expect(ctx.auth).toEqual({ type: 'user', id: 'foo' });
 
     spy.mockRestore();
   });
@@ -98,7 +103,7 @@ describe('koaAuth middleware', () => {
     };
 
     await koaAuth()(mockCtx, next);
-    expect(mockCtx.auth).toEqual('foo');
+    expect(mockCtx.auth).toEqual({ type: 'user', id: 'foo' });
 
     spy.mockRestore();
   });
@@ -111,7 +116,7 @@ describe('koaAuth middleware', () => {
       },
     };
     await koaAuth()(ctx, next);
-    expect(ctx.auth).toEqual('fooUser');
+    expect(ctx.auth).toEqual({ type: 'user', id: 'fooUser' });
   });
 
   it('expect to throw if authorization header is missing', async () => {
@@ -143,6 +148,21 @@ describe('koaAuth middleware', () => {
     await expect(koaAuth()(ctx, next)).rejects.toMatchError(jwtSubMissingError);
   });
 
+  it('expect to have `client` type per jwt verify result', async () => {
+    const mockJwtVerify = jwtVerify as jest.Mock;
+    mockJwtVerify.mockImplementationOnce(() => ({ payload: { sub: 'bar', client_id: 'bar' } }));
+
+    ctx.request = {
+      ...ctx.request,
+      headers: {
+        authorization: 'Bearer access_token',
+      },
+    };
+
+    await koaAuth()(ctx, next);
+    expect(ctx.auth).toEqual({ type: 'app', id: 'bar' });
+  });
+
   it('expect to throw if jwt role_names is missing', async () => {
     const mockJwtVerify = jwtVerify as jest.Mock;
     mockJwtVerify.mockImplementationOnce(() => ({ payload: { sub: 'fooUser' } }));

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

@@ -11,9 +11,14 @@ import envSet from '@/env-set';
 import RequestError from '@/errors/RequestError';
 import assertThat from '@/utils/assert-that';
 
+export type Auth = {
+  type: 'user' | 'app';
+  id: string;
+};
+
 export type WithAuthContext<ContextT extends IRouterParamContext = IRouterParamContext> =
   ContextT & {
-    auth: string;
+    auth: Auth;
   };
 
 const bearerTokenIdentifier = 'Bearer';
@@ -36,6 +41,7 @@ const extractBearerTokenFromHeaders = ({ authorization }: IncomingHttpHeaders) =
 
 type TokenInfo = {
   sub: string;
+  clientId: unknown;
   roleNames?: string[];
 };
 
@@ -47,13 +53,13 @@ export const verifyBearerTokenFromRequest = async (
   const userId = request.headers['development-user-id']?.toString() ?? developmentUserId;
 
   if ((!isProduction || isIntegrationTest) && userId) {
-    return { sub: userId, roleNames: [UserRole.Admin] };
+    return { sub: userId, clientId: undefined, roleNames: [UserRole.Admin] };
   }
 
   try {
     const { localJWKSet, issuer } = oidc;
     const {
-      payload: { sub, role_names: roleNames },
+      payload: { sub, client_id: clientId, role_names: roleNames },
     } = await jwtVerify(extractBearerTokenFromHeaders(request.headers), localJWKSet, {
       issuer,
       audience: resourceIndicator,
@@ -61,7 +67,7 @@ export const verifyBearerTokenFromRequest = async (
 
     assertThat(sub, new RequestError({ code: 'auth.jwt_sub_missing', status: 401 }));
 
-    return { sub, roleNames: conditional(Array.isArray(roleNames) && roleNames) };
+    return { sub, clientId, roleNames: conditional(Array.isArray(roleNames) && roleNames) };
   } catch (error: unknown) {
     if (error instanceof RequestError) {
       throw error;
@@ -75,7 +81,7 @@ export default function koaAuth<StateT, ContextT extends IRouterParamContext, Re
   forRole?: UserRole
 ): MiddlewareType<StateT, WithAuthContext<ContextT>, ResponseBodyT> {
   return async (ctx, next) => {
-    const { sub, roleNames } = await verifyBearerTokenFromRequest(ctx.request);
+    const { sub, clientId, roleNames } = await verifyBearerTokenFromRequest(ctx.request);
 
     if (forRole) {
       assertThat(
@@ -84,7 +90,10 @@ export default function koaAuth<StateT, ContextT extends IRouterParamContext, Re
       );
     }
 
-    ctx.auth = sub;
+    ctx.auth = {
+      type: sub === clientId ? 'app' : 'user',
+      id: sub,
+    };
 
     return next();
   };

packages/core/src/middleware/koa-user-info.test.ts

@@ -1,38 +0,0 @@
-import { mockUser, mockUserResponse } from '@/__mocks__';
-import RequestError from '@/errors/RequestError';
-import * as userQueries from '@/queries/user';
-import { createContextWithRouteParameters } from '@/utils/test-utils';
-
-import koaUserInfo from './koa-user-info';
-
-const findUserByIdSpy = jest.spyOn(userQueries, 'findUserById');
-
-describe('koaUserInfo middleware', () => {
-  const next = jest.fn();
-
-  it('should set userInfo to the context', async () => {
-    findUserByIdSpy.mockImplementationOnce(async () => mockUser);
-
-    const ctx = {
-      ...createContextWithRouteParameters(),
-      auth: 'foo',
-      userInfo: { id: '' }, // Bypass the middleware Context type
-    };
-
-    await koaUserInfo()(ctx, next);
-
-    expect(ctx.userInfo).toEqual(mockUserResponse);
-  });
-
-  it('should throw if is not authenticated', async () => {
-    const ctx = {
-      ...createContextWithRouteParameters(),
-      auth: 'foo',
-      userInfo: { id: '' }, // Bypass the middleware Context type
-    };
-
-    await expect(koaUserInfo()(ctx, next)).rejects.toMatchError(
-      new RequestError({ code: 'auth.unauthorized', status: 401 })
-    );
-  });
-});

packages/core/src/middleware/koa-user-info.ts

@@ -1,29 +0,0 @@
-import { UserInfo, userInfoSelectFields } from '@logto/schemas';
-import { MiddlewareType } from 'koa';
-import pick from 'lodash.pick';
-
-import RequestError from '@/errors/RequestError';
-import { WithAuthContext } from '@/middleware/koa-auth';
-import { findUserById } from '@/queries/user';
-
-export type WithUserInfoContext<ContextT extends WithAuthContext = WithAuthContext> = ContextT & {
-  userInfo: UserInfo;
-};
-
-export default function koaUserInfo<
-  StateT,
-  ContextT extends WithAuthContext,
-  ResponseBodyT
->(): MiddlewareType<StateT, WithUserInfoContext<ContextT>, ResponseBodyT> {
-  return async (ctx, next) => {
-    try {
-      const { auth: userId } = ctx;
-      const userInfo = await findUserById(userId);
-      ctx.userInfo = pick(userInfo, ...userInfoSelectFields);
-    } catch {
-      throw new RequestError({ code: 'auth.unauthorized', status: 401 });
-    }
-
-    return next();
-  };
-}

packages/core/src/routes/admin-user.test.ts

@@ -280,11 +280,18 @@ describe('adminUserRoutes', () => {
   });
 
   it('DELETE /users/:userId', async () => {
-    const userId = 'foo';
+    const userId = 'fooUser';
     const response = await userRequest.delete(`/users/${userId}`);
     expect(response.status).toEqual(204);
   });
 
+  it('DELETE /users/:userId should throw if user is deleting self', async () => {
+    const userId = 'foo';
+    const response = await userRequest.delete(`/users/${userId}`);
+    expect(response.status).toEqual(400);
+    expect(deleteUserIdentity).not.toHaveBeenCalled();
+  });
+
   it('DELETE /users/:userId should throw if user not found', async () => {
     const notExistedUserId = 'notExisitedUserId';
     const mockedFindUserById = findUserById as jest.Mock;

packages/core/src/routes/admin-user.ts

@@ -22,8 +22,6 @@ import assertThat from '@/utils/assert-that';
 
 import { AuthedRouter } from './types';
 
-const getComputedUserId = (userId: string, auth: string) => (userId === 'me' ? auth : userId);
-
 export default function adminUserRoutes<T extends AuthedRouter>(router: T) {
   router.get(
     '/users',
@@ -60,7 +58,7 @@ export default function adminUserRoutes<T extends AuthedRouter>(router: T) {
         params: { userId },
       } = ctx.guard;
 
-      const user = await findUserById(getComputedUserId(userId, ctx.auth));
+      const user = await findUserById(userId);
 
       ctx.body = pick(user, ...userInfoSelectFields);
 
@@ -79,7 +77,7 @@ export default function adminUserRoutes<T extends AuthedRouter>(router: T) {
         params: { userId },
       } = ctx.guard;
 
-      const { customData } = await findUserById(getComputedUserId(userId, ctx.auth));
+      const { customData } = await findUserById(userId);
       ctx.body = customData;
 
       return next();
@@ -95,10 +93,9 @@ export default function adminUserRoutes<T extends AuthedRouter>(router: T) {
     }),
     async (ctx, next) => {
       const {
-        params: { userId: userIdParameter },
+        params: { userId },
         body: { customData },
       } = ctx.guard;
-      const userId = getComputedUserId(userIdParameter, ctx.auth);
 
       await findUserById(userId);
 
@@ -162,10 +159,9 @@ export default function adminUserRoutes<T extends AuthedRouter>(router: T) {
     }),
     async (ctx, next) => {
       const {
-        params: { userId: userIdParameter },
+        params: { userId },
         body,
       } = ctx.guard;
-      const userId = getComputedUserId(userIdParameter, ctx.auth);
 
       await findUserById(userId);
 
@@ -210,10 +206,9 @@ export default function adminUserRoutes<T extends AuthedRouter>(router: T) {
     }),
     async (ctx, next) => {
       const {
-        params: { userId: userIdParameter },
+        params: { userId },
         body: { password },
       } = ctx.guard;
-      const userId = getComputedUserId(userIdParameter, ctx.auth);
 
       await findUserById(userId);
 
@@ -240,7 +235,7 @@ export default function adminUserRoutes<T extends AuthedRouter>(router: T) {
         params: { userId },
       } = ctx.guard;
 
-      if (userId === ctx.auth) {
+      if (userId === ctx.auth.id) {
         throw new RequestError('user.cannot_delete_self');
       }
 
@@ -259,9 +254,8 @@ export default function adminUserRoutes<T extends AuthedRouter>(router: T) {
     koaGuard({ params: object({ userId: string(), target: string() }) }),
     async (ctx, next) => {
       const {
-        params: { userId: userIdParameter, target },
+        params: { userId, target },
       } = ctx.guard;
-      const userId = getComputedUserId(userIdParameter, ctx.auth);
 
       const { identities } = await findUserById(userId);
 

packages/core/src/routes/types.ts

@@ -3,11 +3,7 @@ import Router from 'koa-router';
 import { WithAuthContext } from '@/middleware/koa-auth';
 import { WithI18nContext } from '@/middleware/koa-i18next';
 import { WithLogContext } from '@/middleware/koa-log';
-import { WithUserInfoContext } from '@/middleware/koa-user-info';
 
 export type AnonymousRouter = Router<unknown, WithLogContext & WithI18nContext>;
 
-export type AuthedRouter = Router<
-  unknown,
-  WithUserInfoContext & WithAuthContext & WithLogContext & WithI18nContext
->;
+export type AuthedRouter = Router<unknown, WithAuthContext & WithLogContext & WithI18nContext>;

packages/core/src/utils/test-utils.ts

@@ -140,6 +140,12 @@ export function createRequester({
   if (authedRoutes) {
     const authRouter: AuthedRouter = new Router();
 
+    authRouter.use(async (ctx, next) => {
+      ctx.auth = { type: 'user', id: 'foo' };
+
+      return next();
+    });
+
     for (const route of Array.isArray(authedRoutes) ? authedRoutes : [authedRoutes]) {
       route(authRouter);
     }