Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): handle dpop and client certificate for token exchange #6199

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
27 changes: 5 additions & 22 deletions packages/core/src/oidc/grants/client-credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ import { buildOrganizationUrn } from '@logto/core-kit';
import { cond } from '@silverhand/essentials';
import type Provider from 'oidc-provider';
import { errors } from 'oidc-provider';
import epochTime from 'oidc-provider/lib/helpers/epoch_time.js';
import dpopValidate from 'oidc-provider/lib/helpers/validate_dpop.js';
import instance from 'oidc-provider/lib/helpers/weak_cache.js';
import checkResource from 'oidc-provider/lib/shared/check_resource.js';

Expand All @@ -34,6 +32,8 @@ import assertThat from '#src/utils/assert-that.js';

import { getSharedResourceServerData, reversedResourceAccessTokenTtl } from '../resource.js';

import { handleClientCertificate, handleDPoP } from './utils.js';

const { AccessDenied, InvalidClient, InvalidGrant, InvalidScope, InvalidTarget } = errors;

/**
Expand All @@ -51,7 +51,7 @@ export const buildHandler: (
// eslint-disable-next-line complexity
) => Parameters<Provider['registerGrantType']>[1] = (envSet, queries) => async (ctx, next) => {
const { client, params } = ctx.oidc;
const { ClientCredentials, ReplayDetection } = ctx.oidc.provider;
const { ClientCredentials } = ctx.oidc.provider;

assertThat(client, new InvalidClient('client must be available'));

Expand All @@ -62,8 +62,6 @@ export const buildHandler: (
scopes: statics,
} = instance(ctx.oidc.provider).configuration();

const dPoP = await dpopValidate(ctx);

/* === RFC 0006 === */
// The value type is `unknown`, which will swallow other type inferences. So we have to cast it
// to `Boolean` first.
Expand Down Expand Up @@ -166,23 +164,8 @@ export const buildHandler: (
token.setThumbprint('x5t', cert);
}

if (dPoP) {
// @ts-expect-error -- code from oidc-provider
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
const unique: unknown = await ReplayDetection.unique(
client.clientId,
dPoP.jti,
epochTime() + 300
);

assertThat(unique, new InvalidGrant('DPoP proof JWT Replay detected'));

// @ts-expect-error -- code from oidc-provider
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
token.setThumbprint('jkt', dPoP.thumbprint);
} else if (ctx.oidc.client?.dpopBoundAccessTokens) {
throw new InvalidGrant('DPoP proof JWT not provided');
}
await handleDPoP(ctx, token);
await handleClientCertificate(ctx, token);

ctx.oidc.entity('ClientCredentials', token);
const value = await token.save();
Expand Down
57 changes: 5 additions & 52 deletions packages/core/src/oidc/grants/refresh-token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,14 @@
* The commit hash of the original file is `cf2069cbb31a6a855876e95157372d25dde2511c`.
*/

import { type X509Certificate } from 'node:crypto';

import { UserScope, buildOrganizationUrn } from '@logto/core-kit';
import { type Optional, isKeyInObject, cond } from '@silverhand/essentials';
import { isKeyInObject, cond } from '@silverhand/essentials';
import type Provider from 'oidc-provider';
import { errors } from 'oidc-provider';
import difference from 'oidc-provider/lib/helpers/_/difference.js';
import certificateThumbprint from 'oidc-provider/lib/helpers/certificate_thumbprint.js';
import epochTime from 'oidc-provider/lib/helpers/epoch_time.js';
import filterClaims from 'oidc-provider/lib/helpers/filter_claims.js';
import resolveResource from 'oidc-provider/lib/helpers/resolve_resource.js';
import revoke from 'oidc-provider/lib/helpers/revoke.js';
import dpopValidate from 'oidc-provider/lib/helpers/validate_dpop.js';
import validatePresence from 'oidc-provider/lib/helpers/validate_presence.js';
import instance from 'oidc-provider/lib/helpers/weak_cache.js';

Expand All @@ -46,6 +41,8 @@ import {
isOrganizationConsentedToApplication,
} from '../resource.js';

import { handleClientCertificate, handleDPoP } from './utils.js';

const { InvalidClient, InvalidGrant, InvalidScope, InsufficientScope, AccessDenied } = errors;

/** The grant type name. `gty` follows the name in oidc-provider. */
Expand Down Expand Up @@ -93,8 +90,6 @@ export const buildHandler: (
},
} = providerInstance.configuration();

const dPoP = await dpopValidate(ctx);
wangsijie marked this conversation as resolved.
Show resolved Hide resolved

// @gao: I believe the presence of the param is validated by required parameters of this grant.
// Add `String` to make TS happy.
let refreshTokenValue = String(params.refresh_token);
Expand All @@ -112,23 +107,6 @@ export const buildHandler: (
throw new InvalidGrant('refresh token is expired');
}

let cert: Optional<string | X509Certificate>;
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- the original code uses `||`
if (client.tlsClientCertificateBoundAccessTokens || refreshToken['x5t#S256']) {
cert = getCertificate(ctx);
if (!cert) {
throw new InvalidGrant('mutual TLS client certificate not provided');
}
}

if (!dPoP && client.dpopBoundAccessTokens) {
throw new InvalidGrant('DPoP proof JWT not provided');
}

if (refreshToken['x5t#S256'] && refreshToken['x5t#S256'] !== certificateThumbprint(cert!)) {
throw new InvalidGrant('failed x5t#S256 verification');
}

/* === RFC 0001 === */
// The value type is `unknown`, which will swallow other type inferences. So we have to cast it
// to `Boolean` first.
Expand Down Expand Up @@ -177,22 +155,6 @@ export const buildHandler: (
}
}

if (dPoP) {
// @ts-expect-error -- code from oidc-provider
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
const unique: unknown = await ReplayDetection.unique(
client.clientId,
dPoP.jti,
epochTime() + 300
);

assertThat(unique, new errors.InvalidGrant('DPoP proof JWT Replay detected'));
}

if (refreshToken.jkt && (!dPoP || refreshToken.jkt !== dPoP.thumbprint)) {
throw new InvalidGrant('failed jkt verification');
}

ctx.oidc.entity('RefreshToken', refreshToken);
ctx.oidc.entity('Grant', grant);

Expand Down Expand Up @@ -304,17 +266,8 @@ export const buildHandler: (
scope: undefined!,
});

if (client.tlsClientCertificateBoundAccessTokens) {
// @ts-expect-error -- code from oidc-provider
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
at.setThumbprint('x5t', cert);
}

if (dPoP) {
// @ts-expect-error -- code from oidc-provider
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
at.setThumbprint('jkt', dPoP.thumbprint);
}
await handleDPoP(ctx, at, refreshToken);
await handleClientCertificate(ctx, at, refreshToken);

if (at.gty && !at.gty.endsWith(gty)) {
at.gty = `${at.gty} ${gty}`;
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/oidc/grants/token-exchange/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
getSharedResourceServerData,
reversedResourceAccessTokenTtl,
} from '../../resource.js';
import { handleClientCertificate, handleDPoP } from '../utils.js';

import { handleActorToken } from './actor-token.js';
import { TokenExchangeTokenType, type TokenExchangeAct } from './types.js';
Expand Down Expand Up @@ -93,7 +94,6 @@ export const buildHandler: (
throw new InvalidGrant('refresh token invalid (referenced account not found)');
}

// TODO: (LOG-9501) Implement general security checks like dPop
ctx.oidc.entity('Account', account);

/* === RFC 0001 === */
Expand Down Expand Up @@ -137,6 +137,9 @@ export const buildHandler: (
scope: undefined!,
});

await handleDPoP(ctx, accessToken);
await handleClientCertificate(ctx, accessToken);

/** The scopes requested by the client. If not provided, use the scopes from the refresh token. */
const scope = requestParamScopes;
const resource = await resolveResource(
Expand Down
81 changes: 81 additions & 0 deletions packages/core/src/oidc/grants/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import type Provider from 'oidc-provider';
import { errors, type KoaContextWithOIDC } from 'oidc-provider';
import certificateThumbprint from 'oidc-provider/lib/helpers/certificate_thumbprint.js';
import epochTime from 'oidc-provider/lib/helpers/epoch_time.js';
import dpopValidate from 'oidc-provider/lib/helpers/validate_dpop.js';
import instance from 'oidc-provider/lib/helpers/weak_cache.js';

import assertThat from '#src/utils/assert-that.js';

const { InvalidGrant, InvalidClient } = errors;

/**
* Handle DPoP bound access tokens.
*/
export const handleDPoP = async (
ctx: KoaContextWithOIDC,
token: InstanceType<Provider['AccessToken']> | InstanceType<Provider['ClientCredentials']>,
originalToken?: InstanceType<Provider['RefreshToken']>
) => {
const { client } = ctx.oidc;
assertThat(client, new InvalidClient('client must be available'));

const dPoP = await dpopValidate(ctx);

if (dPoP) {
// @ts-expect-error -- code from oidc-provider
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
const unique: unknown = await ReplayDetection.unique(
client.clientId,
dPoP.jti,
epochTime() + 300
);

assertThat(unique, new InvalidGrant('DPoP proof JWT Replay detected'));

// @ts-expect-error -- code from oidc-provider
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
token.setThumbprint('jkt', dPoP.thumbprint);

Check warning on line 38 in packages/core/src/oidc/grants/utils.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/oidc/grants/utils.ts#L26-L38

Added lines #L26 - L38 were not covered by tests
} else if (client.dpopBoundAccessTokens) {
throw new InvalidGrant('DPoP proof JWT not provided');
}

Check warning on line 41 in packages/core/src/oidc/grants/utils.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/oidc/grants/utils.ts#L40-L41

Added lines #L40 - L41 were not covered by tests

if (originalToken?.jkt && (!dPoP || originalToken.jkt !== dPoP.thumbprint)) {
throw new InvalidGrant('failed jkt verification');
}

Check warning on line 45 in packages/core/src/oidc/grants/utils.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/oidc/grants/utils.ts#L44-L45

Added lines #L44 - L45 were not covered by tests
};

/**
* Handle client certificate bound access tokens.
*/
export const handleClientCertificate = async (
ctx: KoaContextWithOIDC,
token: InstanceType<Provider['AccessToken']> | InstanceType<Provider['ClientCredentials']>,
originalToken?: InstanceType<Provider['RefreshToken']>
) => {
const { client, provider } = ctx.oidc;
assertThat(client, new InvalidClient('client must be available'));

const providerInstance = instance(provider);
const {
features: {
mTLS: { getCertificate },
},
} = providerInstance.configuration();

// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
if (client.tlsClientCertificateBoundAccessTokens || originalToken?.['x5t#S256']) {
const cert = getCertificate(ctx);

if (!cert) {
throw new InvalidGrant('mutual TLS client certificate not provided');
}
// @ts-expect-error -- code from oidc-provider
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
token.setThumbprint('x5t', cert);

if (originalToken?.['x5t#S256'] && originalToken['x5t#S256'] !== certificateThumbprint(cert)) {
throw new InvalidGrant('failed x5t#S256 verification');
}
}

Check warning on line 80 in packages/core/src/oidc/grants/utils.ts

View check run for this annotation

Codecov / codecov/patch

packages/core/src/oidc/grants/utils.ts#L68-L80

Added lines #L68 - L80 were not covered by tests
};
Loading