refactor(core): refactor admin user auth check logic (#1970)

* refactor(core): refactor admin user auth check logic

refactor admin user auth check logic

* test(core): add ut

add ut

* test(core): add integration test

add integration test

* fix(test): fix integration test

fix integration test

packages/core/src/lib/session.ts

@@ -9,7 +9,7 @@ export const assignInteractionResults = async (
   result: InteractionResults,
   merge = false
 ) => {
-  // The "mergeWithLastSubmission" will only merge current request's interfaction results,
+  // The "mergeWithLastSubmission" will only merge current request's interaction results,
   // which is stored in ctx.oidc, we need to merge interaction results in two requests,
   // have to do it manually
   // refer to: https://github.com/panva/node-oidc-provider/blob/c243bf6b6663c41ff3e75c09b95fb978eba87381/lib/actions/authorization/interactions.js#L106

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

@@ -1,4 +1,5 @@
 import { User } from '@logto/schemas';
+import { adminConsoleApplicationId } from '@logto/schemas/lib/seeds';
 import { Provider } from 'oidc-provider';
 
 import { mockUser } from '@/__mocks__';
@@ -128,6 +129,7 @@ describe('sessionRoutes', () => {
           expect.anything()
         );
       });
+
       it('should save application id when the user first consented', async () => {
         interactionDetails.mockResolvedValueOnce({
           session: { accountId: mockUser.id },
@@ -139,11 +141,15 @@ describe('sessionRoutes', () => {
           },
           grantId: 'grantId',
         });
+
         findUserById.mockImplementationOnce(async () => ({ ...mockUser, applicationId: null }));
+
         const response = await sessionRequest.post('/session/consent');
+
         expect(updateUserById).toHaveBeenCalledWith(mockUser.id, { applicationId: 'clientId' });
         expect(response.statusCode).toEqual(200);
       });
+
       it('missingOIDCScope and missingResourceScopes', async () => {
         interactionDetails.mockResolvedValueOnce({
           session: { accountId: 'accountId' },
@@ -173,6 +179,30 @@ describe('sessionRoutes', () => {
         );
       });
     });
+
+    it('should throw is non-admin user request for AC consent', async () => {
+      interactionDetails.mockResolvedValueOnce({
+        session: { accountId: mockUser.id },
+        params: { client_id: adminConsoleApplicationId },
+        prompt: {
+          name: 'consent',
+          details: {},
+          reasons: ['consent_prompt', 'native_client_prompt'],
+        },
+        grantId: 'grantId',
+      });
+
+      findUserById.mockImplementationOnce(async () => ({
+        ...mockUser,
+        roleNames: [],
+        applicationId: null,
+      }));
+
+      const response = await sessionRequest.post('/session/consent');
+
+      expect(response.statusCode).toEqual(401);
+    });
+
     it('throws if session is missing', async () => {
       interactionDetails.mockResolvedValueOnce({ params: { client_id: 'clientId' } });
       await expect(sessionRequest.post('/session/consent')).resolves.toHaveProperty(

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

@@ -1,12 +1,15 @@
 import path from 'path';
 
 import { LogtoErrorCode } from '@logto/phrases';
+import { UserRole } from '@logto/schemas';
+import { adminConsoleApplicationId } from '@logto/schemas/lib/seeds';
 import { conditional } from '@silverhand/essentials';
 import { Provider } from 'oidc-provider';
 import { object, string } from 'zod';
 
 import RequestError from '@/errors/RequestError';
 import { assignInteractionResults, saveUserFirstConsentedAppId } from '@/lib/session';
+import { findUserById } from '@/queries/user';
 import assertThat from '@/utils/assert-that';
 
 import { AnonymousRouter } from '../types';
@@ -46,6 +49,18 @@ export default function sessionRoutes<T extends AnonymousRouter>(router: T, prov
     assertThat(session, 'session.not_found');
 
     const { accountId } = session;
+
+    // Temp solution before migrating to RBAC.  Block non-admin user from consent to admin console
+
+    if (String(client_id) === adminConsoleApplicationId) {
+      const { roleNames } = await findUserById(accountId);
+
+      assertThat(
+        roleNames.includes(UserRole.Admin),
+        new RequestError({ code: 'auth.forbidden', status: 401 })
+      );
+    }
+
     const grant =
       conditional(grantId && (await provider.Grant.find(grantId))) ??
       new provider.Grant({ accountId, clientId: String(client_id) });

packages/core/src/routes/session/username-password.test.ts

@@ -133,31 +133,6 @@ describe('sessionRoutes', () => {
       });
       expect(response.statusCode).toEqual(400);
     });
-
-    it('throw if non-admin user sign in to AC', async () => {
-      interactionDetails.mockResolvedValueOnce({
-        params: { client_id: adminConsoleApplicationId },
-      });
-      const response = await sessionRequest.post(signInRoute).send({
-        username: 'username',
-        password: 'password',
-      });
-
-      expect(response.statusCode).toEqual(403);
-      console.log(response);
-    });
-
-    it('should not throw if admin user sign in to AC', async () => {
-      interactionDetails.mockResolvedValueOnce({
-        params: { client_id: adminConsoleApplicationId },
-      });
-      const response = await sessionRequest.post(signInRoute).send({
-        username: 'admin',
-        password: 'password',
-      });
-
-      expect(response.statusCode).toEqual(200);
-    });
   });
 
   describe('POST /session/register/username-password', () => {

packages/core/src/routes/session/username-password.ts

@@ -43,15 +43,7 @@ export default function usernamePasswordRoutes<T extends AnonymousRouter>(
       const type = 'SignInUsernamePassword';
       ctx.log(type, { username });
 
-      const { id, roleNames } = await findUserByUsernameAndPassword(username, password);
-
-      // Temp solution before migrating to RBAC. As AC sign-in exp currently hardcoded to username password only.
-      if (String(client_id) === adminConsoleApplicationId) {
-        assertThat(
-          roleNames.includes(UserRole.Admin),
-          new RequestError({ code: 'auth.forbidden', status: 403 })
-        );
-      }
+      const { id } = await findUserByUsernameAndPassword(username, password);
 
       ctx.log(type, { userId: id });
       await updateLastSignInAt(id);

packages/integration-tests/src/client/index.ts

@@ -99,6 +99,10 @@ export default class MockClient {
     return this.logto.getIdTokenClaims();
   }
 
+  public assignCookie(cookie: string) {
+    this.interactionCookie = cookie;
+  }
+
   private readonly consent = async () => {
     // Note: If sign in action completed successfully, we will get `_session.sig` in the cookie.
     assert(this.interactionCookie, new Error('Session not found'));

packages/integration-tests/tests/api/session.test.ts

@@ -1,3 +1,4 @@
+import { adminConsoleApplicationId } from '@logto/schemas/lib/seeds';
 import { assert } from '@silverhand/essentials';
 
 import {
@@ -216,3 +217,36 @@ describe('sign-in and sign-out', () => {
     await expect(client.isAuthenticated()).resolves.toBe(false);
   });
 });
+
+describe('sign-in to demo app and revisit Admin Console', () => {
+  const username = generateUsername();
+  const password = generatePassword();
+
+  beforeAll(async () => {
+    await createUserByAdmin(username, password);
+  });
+
+  it('should throw in Admin Console consent step if a logged in user does not have admin role', async () => {
+    const client = new MockClient();
+    await client.initSession();
+
+    assert(client.interactionCookie, new Error('Session not found'));
+
+    const { redirectTo } = await signInWithUsernameAndPassword(
+      username,
+      password,
+      client.interactionCookie
+    );
+
+    await client.processSession(redirectTo);
+
+    await expect(client.isAuthenticated()).resolves.toBe(true);
+
+    const { interactionCookie } = client;
+    const acClient = new MockClient({ appId: adminConsoleApplicationId });
+
+    acClient.assignCookie(interactionCookie);
+
+    await expect(acClient.initSession()).rejects.toThrow();
+  });
+});