feat: require use of PKCE

BREAKING CHANGE: Default PKCE use policy now enforces the use of PKCE `code_challenge` for all requests where PKCE applies. Use the `pkce.required` helper to revert to the old policy if you have a reason to exempt some clients from this policy.
panva · Feb 2, 2021 · aa2bd51 · aa2bd51
1 parent 801d28f
commit aa2bd51
Show file tree

Hide file tree

Showing 6 changed files with 11 additions and 29 deletions.
diff --git a/docs/README.md b/docs/README.md
@@ -2876,7 +2876,7 @@ Configures if and when the OP requires clients to use PKCE. This helper is calle
 _**default value**_:
 ```js
 function pkceRequired(ctx, client) {
-  return client.applicationType === 'native';
+  return true;
 }
 ```
 

diff --git a/lib/actions/authorization/check_pkce.js b/lib/actions/authorization/check_pkce.js
@@ -32,7 +32,7 @@ module.exports = function checkPKCE(ctx, next) {
   // checking for response_type presence disables the need for PKCE for device_code grant
   if (typeof params.response_type === 'string' && params.response_type.includes('code')) {
     if (!params.code_challenge && pkce.required(ctx, ctx.oidc.client)) {
-      throw new InvalidRequest('PKCE must be used for this request');
+      throw new InvalidRequest('Authorization Server policy requires PKCE to be used for this request');
     }
   }
 

diff --git a/lib/helpers/defaults.js b/lib/helpers/defaults.js
@@ -259,8 +259,8 @@ async function issueRefreshToken(ctx, client, code) {
   return client.grantTypeAllowed('refresh_token') && code.scopes.has('offline_access');
 }
 
-function pkceRequired(ctx, client) {
-  return client.applicationType === 'native';
+function pkceRequired(ctx, client) { // eslint-disable-line no-unused-vars
+  return true;
 }
 
 async function pairwiseIdentifier(ctx, accountId, client) {

diff --git a/test/default.config.js b/test/default.config.js
@@ -54,4 +54,7 @@ module.exports = merge({
   features: {},
   enabledJWA: cloneDeep(JWA),
   allowOmittingSingleRegisteredRedirectUri: true,
+  pkce: {
+    required: () => false,
+  },
 }, global.TEST_CONFIGURATION_DEFAULTS);
diff --git a/test/pkce/pkce.config.js b/test/pkce/pkce.config.js
@@ -12,27 +12,9 @@ config.pkce = { methods: ['plain', 'S256'] };
 module.exports = {
   config,
   clients: [{
-    client_id: 'clientPost',
-    token_endpoint_auth_method: 'client_secret_post',
-    client_secret: 'secret',
-    grant_types: ['authorization_code', 'implicit', 'refresh_token'],
-    response_types: ['code', 'id_token', 'code id_token'],
-    redirect_uris: ['https://rp.example.com/cb'],
-  }, {
     client_id: 'client',
     client_secret: 'secret',
-    grant_types: ['authorization_code', 'refresh_token'],
-    redirect_uris: ['https://rp.example.com/cb'],
-  }, {
-    token_endpoint_auth_method: 'none',
-    client_id: 'clientNone',
-    grant_types: ['authorization_code', 'refresh_token'],
-    redirect_uris: ['https://rp.example.com/cb'],
-  }, {
     response_types: ['code', 'code id_token', 'id_token'],
-    application_type: 'native',
-    token_endpoint_auth_method: 'none',
-    client_id: 'native',
     grant_types: ['authorization_code', 'refresh_token', 'implicit'],
     redirect_uris: ['https://rp.example.com/cb'],
   }],

diff --git a/test/pkce/pkce.test.js b/test/pkce/pkce.test.js
@@ -122,41 +122,38 @@ describe('PKCE RFC7636', () => {
         .expect(auth.validateErrorDescription('not supported value of code_challenge_method'));
     });
 
-    it('forces native clients using code flow to use pkce', function () {
+    it('forces clients using code flow to use pkce', function () {
       const auth = new this.AuthorizationRequest({
         response_type: 'code',
         scope: 'openid',
-        client_id: 'native',
       });
 
       return this.agent.get('/auth')
         .query(auth)
         .expect(auth.validatePresence(['error', 'error_description', 'state']))
         .expect(auth.validateError('invalid_request'))
-        .expect(auth.validateErrorDescription('PKCE must be used for this request'));
+        .expect(auth.validateErrorDescription('Authorization Server policy requires PKCE to be used for this request'));
     });
 
-    it('forces native clients using hybrid flow to use pkce', function () {
+    it('forces clients using hybrid flow to use pkce', function () {
       const auth = new this.AuthorizationRequest({
         response_type: 'code id_token',
         scope: 'openid',
-        client_id: 'native',
       });
 
       return this.agent.get('/auth')
         .query(auth)
         .expect(auth.validateFragment)
         .expect(auth.validatePresence(['error', 'error_description', 'state']))
         .expect(auth.validateError('invalid_request'))
-        .expect(auth.validateErrorDescription('PKCE must be used for this request'));
+        .expect(auth.validateErrorDescription('Authorization Server policy requires PKCE to be used for this request'));
     });
 
     bootstrap.passInteractionChecks('native_client_prompt', () => {
       it('is not in effect for implicit flows', function () {
         const auth = new this.AuthorizationRequest({
           response_type: 'id_token',
           scope: 'openid',
-          client_id: 'native',
         });
 
         return this.agent.get('/auth')