Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions packages/core/src/libraries/application-access-control.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
accountCenterApplicationId,
createDefaultApplicationAccessControl,
type Application,
type ApplicationAccessControl,
Expand Down Expand Up @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/libraries/application-access-control.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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)) {
Comment thread
simeng-li marked this conversation as resolved.
Comment thread
charIeszhao marked this conversation as resolved.
return;
}

Expand Down
21 changes: 18 additions & 3 deletions packages/core/src/libraries/session/consent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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();

Expand All @@ -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,
Expand Down
11 changes: 8 additions & 3 deletions packages/core/src/libraries/session/consent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
Comment thread
charIeszhao marked this conversation as resolved.
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),
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
95 changes: 95 additions & 0 deletions packages/core/src/middleware/koa-app-access-control.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
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).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' }));
});
});
55 changes: 55 additions & 0 deletions packages/core/src/middleware/koa-app-access-control.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
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';

export default function koaAppAccessControl<
StateT,
ContextT extends WithInteractionDetailsContext,
ResponseBodyT,
>(libraries: Libraries): 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,
Comment thread
charIeszhao marked this conversation as resolved.
Outdated
clientId,
session.accountId
)
) {
return next();
}

await libraries.applicationAccessControl.assertUserHasApplicationAccess(
Comment thread
charIeszhao marked this conversation as resolved.
clientId,
session.accountId
);

ctx.interactionDetails.result = markAppLevelAccessControlChecked(
ctx.interactionDetails.result ?? ctx.interactionDetails.lastSubmission,
clientId,
session.accountId
);
await ctx.interactionDetails.persist();

return next();
Comment thread
charIeszhao marked this conversation as resolved.
};
}
13 changes: 7 additions & 6 deletions packages/core/src/middleware/koa-auto-consent.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,24 @@
import { buildBuiltInApplicationDataForTenant, isBuiltInApplicationId } from '@logto/schemas';
import { type MiddlewareType } from 'koa';
import { type IRouterParamContext } from 'koa-router';
import type { Provider } from 'oidc-provider';
import { errors } from 'oidc-provider';

import { consent, getMissingScopes } from '#src/libraries/session/index.js';
import type { WithInteractionDetailsContext } from '#src/middleware/koa-interaction-details.js';
import type Queries from '#src/tenants/Queries.js';
import assertThat from '#src/utils/assert-that.js';

/**
* Automatically consent for the first party apps.
*/

export default function koaAutoConsent<StateT, ContextT extends IRouterParamContext, ResponseBodyT>(
provider: Provider,
query: Queries
): MiddlewareType<StateT, ContextT, ResponseBodyT> {
export default function koaAutoConsent<
StateT,
ContextT extends WithInteractionDetailsContext,
ResponseBodyT,
>(provider: Provider, query: Queries): MiddlewareType<StateT, ContextT, ResponseBodyT> {
return async (ctx, next) => {
const interactionDetails = await provider.interactionDetails(ctx.req, ctx.res);
const { interactionDetails } = ctx;
const {
params: { client_id: clientId },
prompt,
Expand Down
Loading
Loading