feat(core): enforce application access during OIDC flows

packages/core/src/libraries/application-access-control.test.ts

@@ -1,4 +1,5 @@
 import {
+  accountCenterApplicationId,
   createDefaultApplicationAccessControl,
   type Application,
   type ApplicationAccessControl,
@@ -86,6 +87,15 @@ describe('assertUserHasApplicationAccess()', () => {
     expect(findApplicationAccessControl).not.toHaveBeenCalled();
   });
 
+  it('allows built-in applications without querying the application', async () => {
+    await expect(
+      createLibrary().assertUserHasApplicationAccess(accountCenterApplicationId, userId)
+    ).resolves.not.toThrow();
+
+    expect(findApplicationById).not.toHaveBeenCalled();
+    expect(findApplicationAccessControl).not.toHaveBeenCalled();
+  });
+
   it('allows without querying rule tables when app-level access control is disabled', async () => {
     findApplicationById.mockResolvedValueOnce(disabledApplication);
 

packages/core/src/libraries/application-access-control.ts

@@ -1,4 +1,4 @@
-import type { ApplicationAccessControl } from '@logto/schemas';
+import { isBuiltInApplicationId, type ApplicationAccessControl } from '@logto/schemas';
 
 import { EnvSet } from '#src/env-set/index.js';
 import RequestError from '#src/errors/RequestError/index.js';
@@ -25,7 +25,7 @@ export const createApplicationAccessControlLibrary = (queries: Queries) => {
   } = queries;
 
   const assertUserHasApplicationAccess = async (applicationId: string, userId: string) => {
-    if (!EnvSet.values.isDevFeaturesEnabled) {
+    if (!EnvSet.values.isDevFeaturesEnabled || isBuiltInApplicationId(applicationId)) {
       return;
     }
 

packages/core/src/libraries/session/consent.test.ts

@@ -59,7 +59,12 @@ describe('consent', () => {
 
   it('should update with new grantId if not exist', async () => {
     const provider = createMockProvider(jest.fn().mockResolvedValue(baseInteractionDetails), Grant);
-    await consent({ ctx: context, provider, queries, interactionDetails: baseInteractionDetails });
+    await consent({
+      ctx: context,
+      provider,
+      queries,
+      interactionDetails: baseInteractionDetails,
+    });
 
     expect(grantSave).toHaveBeenCalled();
 
@@ -85,7 +90,12 @@ describe('consent', () => {
 
     const provider = createMockProvider(jest.fn().mockResolvedValue(interactionDetails), Grant);
 
-    await consent({ ctx: context, provider, queries, interactionDetails });
+    await consent({
+      ctx: context,
+      provider,
+      queries,
+      interactionDetails,
+    });
 
     expect(grantSave).toHaveBeenCalled();
 
@@ -109,7 +119,12 @@ describe('consent', () => {
     }));
 
     const provider = createMockProvider(jest.fn().mockResolvedValue(baseInteractionDetails), Grant);
-    await consent({ ctx: context, provider, queries, interactionDetails: baseInteractionDetails });
+    await consent({
+      ctx: context,
+      provider,
+      queries,
+      interactionDetails: baseInteractionDetails,
+    });
 
     expect(userQueries.updateUserById).toHaveBeenCalledWith(mockUser.id, {
       applicationId: baseInteractionDetails.params.client_id,

packages/core/src/libraries/session/consent.ts

@@ -2,6 +2,7 @@ import { jsonObjectGuard } from '@logto/schemas';
 import { conditional } from '@silverhand/essentials';
 import type { Context } from 'koa';
 import type { PromptDetail, Provider } from 'oidc-provider';
+import { errors } from 'oidc-provider';
 import { z } from 'zod';
 
 import type Queries from '#src/tenants/Queries.js';
@@ -103,19 +104,23 @@ export const consent = async ({
   const {
     session,
     grantId,
-    params: { client_id },
+    params: { client_id: clientId },
   } = interactionDetails;
 
   assertThat(session, 'session.not_found');
+  assertThat(
+    clientId && typeof clientId === 'string',
+    new errors.InvalidClient('client must be available')
+  );
 
   const { accountId } = session;
 
   const grant =
     conditional(grantId && (await provider.Grant.find(grantId))) ??
-    new provider.Grant({ accountId, clientId: String(client_id) });
+    new provider.Grant({ accountId, clientId });
 
   await Promise.all([
-    saveUserFirstConsentedAppId(queries, accountId, String(client_id)),
+    saveUserFirstConsentedAppId(queries, accountId, clientId),
     saveInteractionLastSubmissionToSession(queries, interactionDetails),
   ]);
 

packages/core/src/libraries/session/session-context.test.ts

@@ -51,7 +51,12 @@ describe('saveInteractionLastSubmissionToSession', () => {
 
     const provider = createMockProvider(jest.fn().mockResolvedValue(interactionDetails), Grant);
 
-    await consent({ ctx: context, provider, queries, interactionDetails });
+    await consent({
+      ctx: context,
+      provider,
+      queries,
+      interactionDetails,
+    });
 
     expect(oidcSessionExtensionsInsert).toHaveBeenCalledWith({
       sessionUid: 'sessionUid',

packages/core/src/middleware/koa-app-access-control.test.ts

@@ -0,0 +1,144 @@
+import type { Provider } from 'oidc-provider';
+
+import RequestError from '#src/errors/RequestError/index.js';
+import { markAppLevelAccessControlChecked } from '#src/oidc/application-access-control.js';
+import { MockTenant } from '#src/test-utils/tenant.js';
+import { createContextWithRouteParameters } from '#src/utils/test-utils.js';
+
+import koaAppAccessControl from './koa-app-access-control.js';
+
+const { jest } = import.meta;
+type InteractionDetails = Awaited<ReturnType<Provider['interactionDetails']>>;
+
+describe('koaAppAccessControl middleware', () => {
+  const assertUserHasApplicationAccess = jest.fn(async () => {
+    await Promise.resolve();
+  });
+  const mockTenant = new MockTenant(undefined, undefined, undefined, {
+    applicationAccessControl: { assertUserHasApplicationAccess },
+  });
+  const next = jest.fn();
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  const createContext = (interactionDetails: Partial<InteractionDetails>) => {
+    // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- minimal interaction details stub for middleware testing
+    const details = {
+      persist: jest.fn(),
+      ...interactionDetails,
+    } as InteractionDetails;
+
+    return {
+      ...createContextWithRouteParameters({
+        url: `/consent`,
+      }),
+      interactionDetails: details,
+    };
+  };
+
+  it('should assert application access before next middleware', async () => {
+    const ctx = createContext({
+      params: { client_id: 'app-id' },
+      // @ts-expect-error
+      session: { accountId: 'user-id' },
+    });
+    const guard = koaAppAccessControl(mockTenant.libraries);
+
+    await guard(ctx, next);
+
+    expect(assertUserHasApplicationAccess).toHaveBeenCalledWith('app-id', 'user-id');
+    expect(ctx.interactionDetails.persist).not.toHaveBeenCalled();
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('should mark application access checked when configured', async () => {
+    const ctx = createContext({
+      params: { client_id: 'app-id' },
+      // @ts-expect-error
+      session: { accountId: 'user-id' },
+    });
+    const guard = koaAppAccessControl(mockTenant.libraries, { markInteractionResult: true });
+
+    await guard(ctx, next);
+
+    expect(assertUserHasApplicationAccess).toHaveBeenCalledWith('app-id', 'user-id');
+    expect(ctx.interactionDetails.persist).toHaveBeenCalled();
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('should mark application access checked when the mark predicate returns true', async () => {
+    const ctx = createContext({
+      params: { client_id: 'app-id' },
+      // @ts-expect-error
+      session: { accountId: 'user-id' },
+    });
+    const guard = koaAppAccessControl(mockTenant.libraries, {
+      markInteractionResult: async () => true,
+    });
+
+    await guard(ctx, next);
+
+    expect(assertUserHasApplicationAccess).toHaveBeenCalledWith('app-id', 'user-id');
+    expect(ctx.interactionDetails.persist).toHaveBeenCalled();
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('should not mark application access checked when the mark predicate returns false', async () => {
+    const ctx = createContext({
+      params: { client_id: 'app-id' },
+      // @ts-expect-error
+      session: { accountId: 'user-id' },
+    });
+    const guard = koaAppAccessControl(mockTenant.libraries, {
+      markInteractionResult: async () => false,
+    });
+
+    await guard(ctx, next);
+
+    expect(assertUserHasApplicationAccess).toHaveBeenCalledWith('app-id', 'user-id');
+    expect(ctx.interactionDetails.persist).not.toHaveBeenCalled();
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('should skip duplicated application access check in the same interaction', async () => {
+    const ctx = createContext({
+      params: { client_id: 'app-id' },
+      // @ts-expect-error
+      session: { accountId: 'user-id' },
+      result: markAppLevelAccessControlChecked(undefined, 'app-id', 'user-id'),
+    });
+    const guard = koaAppAccessControl(mockTenant.libraries);
+
+    await guard(ctx, next);
+
+    expect(assertUserHasApplicationAccess).not.toHaveBeenCalled();
+    expect(ctx.interactionDetails.persist).not.toHaveBeenCalled();
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('should throw when access is denied', async () => {
+    const ctx = createContext({
+      params: { client_id: 'app-id' },
+      // @ts-expect-error
+      session: { accountId: 'user-id' },
+    });
+    assertUserHasApplicationAccess.mockRejectedValueOnce(new RequestError('oidc.access_denied'));
+    const guard = koaAppAccessControl(mockTenant.libraries);
+
+    await expect(guard(ctx, next)).rejects.toMatchObject({ code: 'oidc.access_denied' });
+    expect(ctx.interactionDetails.persist).not.toHaveBeenCalled();
+    expect(next).not.toHaveBeenCalled();
+  });
+
+  it('should throw an error if session is not found', async () => {
+    const ctx = createContext({
+      params: { client_id: 'app-id' },
+      session: undefined,
+    });
+    const guard = koaAppAccessControl(mockTenant.libraries);
+
+    await expect(guard(ctx, next)).rejects.toThrow(new RequestError({ code: 'session.not_found' }));
+  });
+});

packages/core/src/middleware/koa-app-access-control.ts

@@ -0,0 +1,75 @@
+import { type MiddlewareType } from 'koa';
+import { errors } from 'oidc-provider';
+
+import RequestError from '#src/errors/RequestError/index.js';
+import type { WithInteractionDetailsContext } from '#src/middleware/koa-interaction-details.js';
+import {
+  hasAppLevelAccessControlChecked,
+  markAppLevelAccessControlChecked,
+} from '#src/oidc/application-access-control.js';
+import type Libraries from '#src/tenants/Libraries.js';
+import assertThat from '#src/utils/assert-that.js';
+
+type KoaAppAccessControlOptions<ContextT> = {
+  readonly markInteractionResult?: boolean | ((ctx: ContextT) => boolean | Promise<boolean>);
+};
+
+const shouldMarkInteractionResult = async <ContextT>(
+  ctx: ContextT,
+  option: KoaAppAccessControlOptions<ContextT>['markInteractionResult']
+) => {
+  if (typeof option === 'function') {
+    return option(ctx);
+  }
+
+  return option === true;
+};
+
+export default function koaAppAccessControl<
+  StateT,
+  ContextT extends WithInteractionDetailsContext,
+  ResponseBodyT,
+>(
+  libraries: Libraries,
+  options: KoaAppAccessControlOptions<ContextT> = {}
+): MiddlewareType<StateT, ContextT, ResponseBodyT> {
+  return async (ctx, next) => {
+    const {
+      params: { client_id: clientId },
+      session,
+    } = ctx.interactionDetails;
+
+    assertThat(session, new RequestError({ code: 'session.not_found' }));
+    assertThat(
+      clientId && typeof clientId === 'string',
+      new errors.InvalidClient('client must be available')
+    );
+
+    if (
+      hasAppLevelAccessControlChecked(ctx.interactionDetails.result, clientId, session.accountId) ||
+      hasAppLevelAccessControlChecked(
+        ctx.interactionDetails.lastSubmission,
+        clientId,
+        session.accountId
+      )
+    ) {
+      return next();
+    }
+
+    await libraries.applicationAccessControl.assertUserHasApplicationAccess(
+      clientId,
+      session.accountId
+    );
+
+    if (await shouldMarkInteractionResult(ctx, options.markInteractionResult)) {
+      ctx.interactionDetails.result = markAppLevelAccessControlChecked(
+        ctx.interactionDetails.result ?? ctx.interactionDetails.lastSubmission,
+        clientId,
+        session.accountId
+      );
+      await ctx.interactionDetails.persist();
+    }
+
+    return next();
+  };
+}