Merge pull request #31 from DawoonC/master

- abstracted redirect uri checking in authorization.js to client model
t1msh · Mar 6, 2016 · 7d9a3b2 · 7d9a3b2
2 parents 4ffe594 + b4cca97
commit 7d9a3b2
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 14 deletions.
diff --git a/lib/controller/authorization.js b/lib/controller/authorization.js
@@ -90,8 +90,7 @@ module.exports = function(req, res, next) {
         function(cb) {
             if (!req.oauth2.model.client.getRedirectUri(client))
                 cb(new error.unsupportedResponseType('RedirectUri is not set for the client'));
-            else if (redirectUri.indexOf(req.oauth2.model.client.getRedirectUri(client)) !== 0 ||
-                     redirectUri.replace(req.oauth2.model.client.getRedirectUri(client), '').indexOf('#') === 0)
+            else if (!req.oauth2.model.client.checkRedirectUri(client, redirectUri))
                 cb(new error.invalidRequest('Wrong RedirectUri provided'));
             else {
                 req.oauth2.logger.debug('RedirectUri check passed: ', redirectUri);

diff --git a/lib/model/client.js b/lib/model/client.js
@@ -42,6 +42,25 @@ module.exports.getRedirectUri = function(client) {
     throw new error.serverError('Client model method "getRedirectUri" is not implemented');
 };
 
+/**
+ * Checks redirect uri for the client
+ *
+ * @param client {Object} Client object
+ * @param client {String} Redirect URI to be checked
+ */
+module.exports.checkRedirectUri = function(client, redirectUri) {
+    /**
+     * For example:
+     * // for single redirect uri per client
+     * return (redirectUri.indexOf(getRedirectUri(client)) === 0 &&
+     *         redirectUri.replace(getRedirectUri(client), '').indexOf('#') === -1);
+     *
+     * // for multiple redirect uris per client
+     * return (getRedirectUri(client).indexOf(redirectUri) !== -1);
+     */
+    throw new error.serverError('Client model method "checkRedirectUri" is not implemented');
+};
+
 /**
  * Fetches client object by primary key
  * Should be implemented with server logic

diff --git a/test/server/model/memory/oauth2/client.js b/test/server/model/memory/oauth2/client.js
@@ -4,9 +4,9 @@ module.exports.getId = function(client) {
     return client.id;
 };
 
-module.exports.getRedirectUri = function(client) {
-    return client.redirectUri;
-};
+module.exports.getRedirectUri = getRedirectUri;
+
+module.exports.checkRedirectUri = checkRedirectUri;
 
 module.exports.fetchById = function(clientId, cb) {
     for (var i in clients) {
@@ -17,4 +17,13 @@ module.exports.fetchById = function(clientId, cb) {
 
 module.exports.checkSecret = function(client, secret, cb) {
     return cb(null, client.secret == secret);
+};
+
+function getRedirectUri(client) {
+    return client.redirectUri;
+};
+
+function checkRedirectUri(client, redirectUri) {
+    return (redirectUri.indexOf(getRedirectUri(client)) === 0 &&
+            redirectUri.replace(getRedirectUri(client), '').indexOf('#') === -1);
 };
diff --git a/test/server/model/redis/oauth2/accessToken.js b/test/server/model/redis/oauth2/accessToken.js
@@ -47,7 +47,7 @@ module.exports.checkTTL = function(accessToken) {
     return true;
 };
 
-module.exports.getTTL = function(accessToken, cb) {
+module.exports.getTTL = function(token, cb) {
     redis.ttl(util.format(KEY.ACCESS_TOKEN, token), cb);
 };
 

diff --git a/test/server/model/redis/oauth2/client.js b/test/server/model/redis/oauth2/client.js
@@ -12,9 +12,9 @@ module.exports.getId = function(client) {
     return client.id;
 };
 
-module.exports.getRedirectUri = function(client) {
-    return client.redirectUri;
-};
+module.exports.getRedirectUri = getRedirectUri;
+
+module.exports.checkRedirectUri = checkRedirectUri;
 
 module.exports.fetchById = function(clientId, cb) {
     redis.get(util.format(KEY.CLIENT, clientId), function(err, stringified) {
@@ -34,4 +34,13 @@ module.exports.fetchById = function(clientId, cb) {
 // Add some hashing algorithm for security
 module.exports.checkSecret = function(client, secret, cb) {
     return cb(null, client.secret == secret);
-};
+};
+
+function getRedirectUri(client) {
+    return client.redirectUri;
+}
+
+function checkRedirectUri(client, redirectUri) {
+    return (redirectUri.indexOf(getRedirectUri(client)) === 0 &&
+            redirectUri.replace(getRedirectUri(client), '').indexOf('#') === -1);
+}
diff --git a/test/server/model/rethinkdb/oauth2/client.js b/test/server/model/rethinkdb/oauth2/client.js
@@ -7,9 +7,9 @@ module.exports.getId = function(client) {
     return client.id;
 };
 
-module.exports.getRedirectUri = function(client) {
-    return client.redirectUri;
-};
+module.exports.getRedirectUri = getRedirectUri;
+
+module.exports.checkRedirectUri = checkRedirectUri;
 
 module.exports.fetchById = function(clientId, cb) {
     connection.acquire(function(err, conn) {
@@ -20,4 +20,13 @@ module.exports.fetchById = function(clientId, cb) {
 
 module.exports.checkSecret = function(client, secret, cb) {
     return cb(null, client.secret == secret);
-};
+};
+
+function getRedirectUri(client) {
+    return client.redirectUri;
+}
+
+function checkRedirectUri(client, redirectUri) {
+    return (redirectUri.indexOf(getRedirectUri(client)) === 0 &&
+            redirectUri.replace(getRedirectUri(client), '').indexOf('#') === -1);
+}
diff --git a/test/server/oauth20.js b/test/server/oauth20.js
@@ -13,6 +13,7 @@ module.exports = function(type) {
     // Set client methods
     obj.model.client.getId = model.client.getId;
     obj.model.client.getRedirectUri = model.client.getRedirectUri;
+    obj.model.client.checkRedirectUri = model.client.checkRedirectUri;
     obj.model.client.fetchById = model.client.fetchById;
     obj.model.client.checkSecret = model.client.checkSecret;