better implementation of validateRedirectUri

Author: jorenvandeweyer

docs/model/spec.rst

@@ -989,12 +989,12 @@ Returns ``true`` if the access token passes, ``false`` otherwise.
 
 .. _Model#validateRedirectUri:
 
-``validateRedirectUri(redirect_uri, redirect_uris, [callback])``
+``validateRedirectUri(redirectUri, client, [callback])``
 ================================================================
 
-Invoked to check if the provided ``redirect_uri`` is valid for a particular ``client``.
+Invoked to check if the provided ``redirectUri`` is valid for a particular ``client``.
 
-This model function is **optional**. If not implemented, the redirect_uri should be included in the provided redirect_uris of the client.
+This model function is **optional**. If not implemented, the ``redirectUri`` should be included in the provided ``redirectUris`` of the client.
 
 **Invoked during:**
 
@@ -1007,18 +1007,18 @@ This model function is **optional**. If not implemented, the redirect_uri should
 +=================+==========+=====================================================================+
 | redirect_uri    | String   | The redirect URI to validate.                                       |
 +-----------------+----------+---------------------------------------------------------------------+
-| redirect_uris   | Array    | The list of redirect URIs configured for the client.               |
+| client          | Object   | The associated client.                                              |
 +-----------------+----------+---------------------------------------------------------------------+
 
 **Return value:**
 
-Returns ``true`` if the ``redirect_uri`` is valid, ``false`` otherwise.
+Returns ``true`` if the ``redirectUri`` is valid, ``false`` otherwise.
 
 **Remarks:**
 
 ::
 
-  function validateRedirectUri(redirect_uri, redirect_uris) {
-    return redirect_uris.includes(redirect_uri);
+  function validateRedirectUri(redirectUri, client) {
+    return client.redirectUris.includes(redirectUri);
   }
 

lib/handlers/authorize-handler.js

@@ -194,14 +194,17 @@ AuthorizeHandler.prototype.getClient = function(request) {
         throw new InvalidClientError('Invalid client: missing client `redirectUri`');
       }
 
-      if (redirectUri && typeof self.model.validateRedirectUri === 'function') {
-        if (!self.model.validateRedirectUri(redirectUri, client.redirectUris)) {
-          throw new InvalidClientError('Invalid client: `redirect_uri` does not match client value');
-        }
-      } else if (redirectUri && !client.redirectUris.includes(redirectUri)) {
-        throw new InvalidClientError('Invalid client: `redirect_uri` does not match client value');
+      if (redirectUri) {
+        return self.validateRedirectUri(redirectUri, client)
+          .then(function(valid) {
+            if (!valid) {
+              throw new InvalidClientError('Invalid client: `redirect_uri` does not match client value');
+            }
+            return client;
+          });
+      } else {
+        return client;
       }
-      return client;
     });
 };
 
@@ -294,6 +297,14 @@ AuthorizeHandler.prototype.saveAuthorizationCode = function(authorizationCode, e
   return promisify(this.model.saveAuthorizationCode, 3).call(this.model, code, client, user);
 };
 
+
+AuthorizeHandler.prototype.validateRedirectUri = function(redirectUri, client) {
+  if (this.model.validateRedirectUri) {
+    return promisify(this.model.validateRedirectUri, 2).call(this.model, redirectUri, client);
+  }
+
+  return Promise.resolve(client.redirectUris.includes(redirectUri));
+};
 /**
  * Get response type.
  */

test/integration/handlers/authorize-handler_test.js

@@ -634,6 +634,65 @@ describe('AuthorizeHandler integration', function() {
     });
   });
 
+  describe('validateRedirectUri()', function() {
+    it('should support empty model', function() {
+      const model = {
+        getAccessToken: function() {},
+        getClient: function() {},
+        saveAuthorizationCode: function() {}
+      };
+
+      const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model });
+
+      handler.validateRedirectUri('http://example.com/a', { redirectUris: ['http://example.com/a'] }).should.be.an.instanceOf(Promise);
+    });
+
+    it('should support promises', function() {
+      const model = {
+        getAccessToken: function() {},
+        getClient: function() {},
+        saveAuthorizationCode: function() {},
+        validateRedirectUri: function() {
+          return Promise.resolve(true);
+        }
+      };
+
+      const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model });
+
+      handler.validateRedirectUri('http://example.com/a', { }).should.be.an.instanceOf(Promise);
+    });
+
+    it('should support non-promises', function() {
+      const model = {
+        getAccessToken: function() {},
+        getClient: function() {},
+        saveAuthorizationCode: function() {},
+        validateRedirectUri: function() {
+          return true;
+        }
+      };
+
+      const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model });
+
+      handler.validateRedirectUri('http://example.com/a', { }).should.be.an.instanceOf(Promise);
+    });
+
+    it('should support callbacks', function() {
+      const model = {
+        getAccessToken: function() {},
+        getClient: function() {},
+        saveAuthorizationCode: function() {},
+        validateRedirectUri: function(redirectUri, client, callback) {
+          callback(null, false);
+        }
+      };
+
+      const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model });
+
+      handler.validateRedirectUri('http://example.com/a', { }).should.be.an.instanceOf(Promise);
+    });
+  });
+
   describe('getClient()', function() {
     it('should throw an error if `client_id` is missing', function() {
       const model = {

test/unit/handlers/authorize-handler_test.js

@@ -123,7 +123,7 @@ describe('AuthorizeHandler', function() {
           model.validateRedirectUri.callCount.should.equal(1);
           model.validateRedirectUri.firstCall.args.should.have.length(2);
           model.validateRedirectUri.firstCall.args[0].should.equal(redirect_uri);
-          model.validateRedirectUri.firstCall.args[1].should.equal(client.redirectUris);
+          model.validateRedirectUri.firstCall.args[1].should.equal(client);
           model.validateRedirectUri.firstCall.thisValue.should.equal(model);
         })
         .catch(should.fail);