fix(pkce): Don't require PKCE in the direct grant flow. (#566) r=@vladikoff

Fixes #559.

config/test.json

@@ -17,7 +17,8 @@
       "imageUri": "https://example2.domain/logo",
       "redirectUri": "https://example2.domain/redirect",
       "trusted": true,
-      "canGrant": true
+      "canGrant": true,
+      "publicClient": true
     },
     {
       "id": "98e6508e88680e1b",

lib/routes/authorization.js

@@ -119,6 +119,31 @@ function generateGrant(claims, client, scope, req) {
   });
 }
 
+// Check that PKCE is provided if and only if appropriate.
+function checkPKCEParams(req, client) {
+  if (req.payload.response_type === TOKEN) {
+    // Direct token grant can't use PKCE.
+    if (req.payload.code_challenge_method) {
+      throw new AppError.invalidRequestParameter('code_challenge_method');
+    }
+    if (req.payload.code_challenge) {
+      throw new AppError.invalidRequestParameter('code_challenge');
+    }
+  } else if (client.publicClient) {
+    // Public clients *must* use PKCE.
+    if (! req.payload.code_challenge_method || ! req.payload.code_challenge) {
+      logger.info('client.missingPkceParameters');
+      throw AppError.missingPkceParameters();
+    }
+  } else {
+    // non-Public Clients can't use PKCE.
+    if (req.payload.code_challenge_method || req.payload.code_challenge) {
+      logger.info('client.notPublicClient');
+      throw AppError.notPublicClient({ id: req.payload.client_id });
+    }
+  }
+}
+
 module.exports = {
   validate: {
     payload: {
@@ -233,19 +258,6 @@ module.exports = {
           }
         }
 
-        // PKCE client enforcement
-        if (client.publicClient &&
-           (! req.payload.code_challenge_method || ! req.payload.code_challenge)) {
-          // only Public Clients support code_challenge
-          logger.info('client.missingPkceParameters');
-          throw AppError.missingPkceParameters();
-        } else if (! client.publicClient &&
-            (req.payload.code_challenge_method || req.payload.code_challenge)) {
-          // non-Public Clients do not allow code challenge
-          logger.info('client.notPublicClient');
-          throw AppError.notPublicClient({ id: req.payload.client_id });
-        }
-
         var uri = req.payload.redirect_uri || client.redirectUri;
 
         if (uri !== client.redirectUri) {
@@ -271,6 +283,8 @@ module.exports = {
 
         req.payload.redirect_uri = uri;
 
+        checkPKCEParams(req, client);
+
         return client;
       }),
       scope.values(),

lib/routes/token.js

@@ -206,7 +206,7 @@ module.exports = {
         return getClientById(clientId).then(function(client) {
           var confirmClientPromise;
 
-          if (client.publicClient && validPublicClient(client)) {
+          if (client.publicClient) {
             if (params.client_secret) {
               throw new AppError.invalidRequestParameter('client_secret');
             }

test/api.js

@@ -421,6 +421,22 @@ describe('/v1', function() {
         });
       });
 
+      it('should allow Public Clients to direct grant without PKCE', function() {
+        var client = clientByName('Admin');
+        mockAssertion().reply(200, VERIFY_GOOD);
+        return Server.api.post({
+          url: '/authorization',
+          payload: authParams({
+            client_id: client.id,
+            response_type: 'token',
+            scope: 'profile profile:write profile:uid',
+          })
+        }).then(function(res) {
+          assert.equal(res.statusCode, 200);
+          assertSecurityHeaders(res);
+        });
+      });
+
       it('only works with Public Clients', function() {
         var client = clientByName('Mocha');
         mockAssertion().reply(200, VERIFY_GOOD);