Skip to content

Commit

Permalink
refactor(FAPI): deprecate FAPI profile hardcoded PKCE checks
Browse files Browse the repository at this point in the history
  • Loading branch information
panva committed May 20, 2023
1 parent 76f9af0 commit 56641ec
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 10 deletions.
1 change: 1 addition & 0 deletions certification/fapi/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ const fapi = new Provider(ISSUER, {
properties: ['profile'],
},
pkce: {
// TODO: remove in v9.x
required: () => false,
},
});
Expand Down
20 changes: 19 additions & 1 deletion docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2724,7 +2724,25 @@ Configures if and when the OP requires clients to use `PKCE`. This helper is cal
_**default value**_:
```js
function pkceRequired(ctx, client) {
return true;
const fapiProfile = ctx.oidc.isFapi('2.0', '1.0 Final');
switch (true) {
// FAPI 2.0 as per
// https://openid.net/specs/fapi-2_0-security-profile-ID2.html#section-5.3.1.2-2.5.1
case fapiProfile === '2.0':
return true;
// FAPI 1.0 Advanced as per
// https://openid.net/specs/openid-financial-api-part-2-1_0.html#authorization-server
case fapiProfile === '1.0 Final' && ctx.oidc.route === 'pushed_authorization_request':
return true;
// All Public clients as per
// https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics-22#section-2.1.1-2.1.1
case client.clientAuthMethod === 'none':
return true;
// All other cases RECOMMENDED as per
// https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics-22#section-2.1.1-2.2.1
default:
return true;
}
}
```
Expand Down
24 changes: 16 additions & 8 deletions lib/actions/authorization/check_pkce.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { InvalidRequest } from '../../helpers/errors.js';
import instance from '../../helpers/weak_cache.js';
import checkFormat from '../../helpers/pkce_format.js';
import * as attention from '../../helpers/attention.js';
import { defaults } from '../../helpers/defaults.js';

/*
* - assign default code_challenge_method if a code_challenge is provided
Expand Down Expand Up @@ -30,14 +32,20 @@ export default function checkPKCE(ctx, next) {
}

if (params.response_type.includes('code')) {
if (
!params.code_challenge
&& (
pkce.required(ctx, ctx.oidc.client)
|| ctx.oidc.isFapi('2.0')
|| (ctx.oidc.isFapi('1.0 Final') && route === 'pushed_authorization_request')
)) {
throw new InvalidRequest('Authorization Server policy requires PKCE to be used for this request');
if (!params.code_challenge) {
if (pkce.required(ctx, ctx.oidc.client)) {
throw new InvalidRequest('Authorization Server policy requires PKCE to be used for this request');
}
// TODO: remove in v9.x
if (
defaults.pkce.required !== pkce.required && (
ctx.oidc.isFapi('2.0')
|| (ctx.oidc.isFapi('1.0 Final') && route === 'pushed_authorization_request')
)
) {
attention.warn('FAPI profile related PKCE policy should be moved to the `pkce.required` configuration helper function');
throw new InvalidRequest('Authorization Server policy requires PKCE to be used for this request');
}
}
}

Expand Down
23 changes: 22 additions & 1 deletion lib/helpers/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,28 @@ async function issueRefreshToken(ctx, client, code) {
}

function pkceRequired(ctx, client) {
return true;
const fapiProfile = ctx.oidc.isFapi('2.0', '1.0 Final');
switch (true) {
// FAPI 2.0 as per
// https://openid.net/specs/fapi-2_0-security-profile-ID2.html#section-5.3.1.2-2.5.1
case fapiProfile === '2.0':
return true;

// FAPI 1.0 Advanced as per
// https://openid.net/specs/openid-financial-api-part-2-1_0.html#authorization-server
case fapiProfile === '1.0 Final' && ctx.oidc.route === 'pushed_authorization_request':
return true;

// All Public clients as per
// https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics-22#section-2.1.1-2.1.1
case client.clientAuthMethod === 'none':
return true;

// All other cases RECOMMENDED as per
// https://datatracker.ietf.org/doc/html/draft-ietf-oauth-security-topics-22#section-2.1.1-2.2.1
default:
return true;
}
}

async function pairwiseIdentifier(ctx, accountId, client) {
Expand Down

0 comments on commit 56641ec

Please sign in to comment.