PKCE refactor, add PKCEEnabled option, require client.isPublic property with PKCE

Author: visvk

lib/grant-types/authorization-code-grant-type.js

@@ -124,22 +124,27 @@ AuthorizationCodeGrantType.prototype.getAuthorizationCode = function(request, cl
 
       if (this.PKCEEnabled && client.isPublic) {
         if (!code.codeChallenge) {
-          throw new ServerError('Server error: `getAuthorizationCode()` did not return a `codeChallenge` property');
+          throw new InvalidGrantError('Invalid grant: code challenge is invalid');
         }
 
+        /**
+         * The "code_challenge_method" is bound to the Authorization Code when
+         * the Authorization Code is issued.  That is the method that the token
+         * endpoint MUST use to verify the "code_verifier".
+        */
         if (!code.codeChallengeMethod) {
           throw new ServerError('Server error: `getAuthorizationCode()` did not return a `codeChallengeMethod` property');
         }
 
         if (code.codeChallengeMethod === 'plain' && code.codeChallenge !== request.body.code_verifier) {
-          throw new InvalidGrantError('Invalid grant: `code_verifier` is invalid');
+          throw new InvalidGrantError('Invalid grant: code verifier is invalid');
         }
 
         if (code.codeChallengeMethod === 'S256') {
           var hash = stringUtil.base64URLEncode(crypto.createHash('sha256').update(request.body.code_verifier).digest());
 
           if (code.codeChallenge !== hash) {
-            throw new InvalidGrantError('Invalid grant: `code_verifier` is invalid');
+            throw new InvalidGrantError('Invalid grant: code verifier is invalid');
           }
         }
       }

lib/handlers/authorize-handler.js

@@ -151,6 +151,14 @@ AuthorizeHandler.prototype.generateAuthorizationCode = function(client, user, sc
 AuthorizeHandler.prototype.getCodeChallenge = function(request, client) {
   var codeChallenge = request.body.code_challenge || request.query.code_challenge;
 
+  /** 
+   * If the server requires Proof Key for Code Exchange (PKCE) by OAuth
+   * public clients and the client does not send the "code_challenge" in
+   * the request, the authorization endpoint MUST return the authorization
+   * error response with the "error" value set to "invalid_request".  The
+   * "error_description" or the response of "error_uri" SHOULD explain the
+   * nature of error, e.g., code challenge required.
+  */
   if (this.PKCEEnabled && client.isPublic && _.isEmpty(codeChallenge)) {
     throw new InvalidRequestError('Missing parameter: `code_challenge`. Public clients must include a code_challenge');
   }
@@ -162,7 +170,7 @@ AuthorizeHandler.prototype.getCodeChallengeMethod = function(request) {
   var codeChallengeMethod = request.body.code_challenge_method || request.query.code_challenge_method || 'plain';
 
   if (!_.includes(['S256', 'plain'], codeChallengeMethod)) {
-    throw new InvalidRequestError('Invalid parameter: `code_challenge_method`');
+    throw new InvalidRequestError('Invalid parameter: `code_challenge_method`, use S256 instead');
   }
 
   return codeChallengeMethod;

test/integration/grant-types/authorization-code-grant-type_test.js

@@ -386,7 +386,7 @@ describe('AuthorizationCodeGrantType integration', function() {
         .then(should.fail)
         .catch(function(e) {
           e.should.be.an.instanceOf(InvalidGrantError);
-          e.message.should.equal('Invalid grant: `code_verifier` is invalid');
+          e.message.should.equal('Invalid grant: code verifier is invalid');
         });
     });
 
@@ -412,7 +412,7 @@ describe('AuthorizationCodeGrantType integration', function() {
         .then(should.fail)
         .catch(function(e) {
           e.should.be.an.instanceOf(InvalidGrantError);
-          e.message.should.equal('Invalid grant: `code_verifier` is invalid');
+          e.message.should.equal('Invalid grant: code verifier is invalid');
         });
     });
 

test/integration/handlers/authorize-handler_test.js

@@ -908,7 +908,7 @@ describe('AuthorizeHandler integration', function() {
         should.fail();
       } catch (e) {
         e.should.be.an.instanceOf(InvalidRequestError);
-        e.message.should.equal('Invalid parameter: `code_challenge_method`');
+        e.message.should.equal('Invalid parameter: `code_challenge_method`, use S256 instead');
       }
     });
     describe('with `code_challenge_method` in the request body', function() {