Merge pull request #112 from FStefanni/89_11_565

Fixed order of checks in authorize-handler
node-oauth · Aug 25, 2022 · 848b0bb · 848b0bb
2 parents c2d108d + 7a61930
commit 848b0bb
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 34 deletions.
diff --git a/lib/handlers/authorize-handler.js b/lib/handlers/authorize-handler.js
@@ -77,10 +77,6 @@ AuthorizeHandler.prototype.handle = function(request, response) {
     throw new InvalidArgumentError('Invalid argument: `response` must be an instance of Response');
   }
 
-  if (request.query.allowed === 'false' || request.body.allowed === 'false') {
-    return Promise.reject(new AccessDeniedError('Access denied: user denied access to application'));
-  }
-
   const fns = [
     this.getAuthorizationCodeLifetime(),
     this.getClient(request),
@@ -98,7 +94,7 @@ AuthorizeHandler.prototype.handle = function(request, response) {
       return Promise.bind(this)
         .then(function() {
           state = this.getState(request);
-          if(request.query.allowed === 'false') {
+          if (request.query.allowed === 'false' || request.body.allowed === 'false') {
             throw new AccessDeniedError('Access denied: user denied access to application');
           }
         })

diff --git a/test/integration/handlers/authorize-handler_test.js b/test/integration/handlers/authorize-handler_test.js
@@ -159,7 +159,7 @@ describe('AuthorizeHandler integration', function() {
       }
     });
 
-    it('should throw an error if `allowed` is `false`', function() {
+    it('should redirect to an error response if user denied access', function() {
       const model = {
         getAccessToken: function() {
           return {
@@ -170,49 +170,29 @@ describe('AuthorizeHandler integration', function() {
         getClient: function() {
           return { grants: ['authorization_code'], redirectUris: ['http://example.com/cb'] };
         },
-        saveAuthorizationCode: function() {
-          throw new Error('Unhandled exception');
-        }
+        saveAuthorizationCode: function() {}
       };
       const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model });
       const request = new Request({
         body: {
-          client_id: 'test'
+          client_id: 12345,
+          response_type: 'code'
         },
+        method: {},
         headers: {
           'Authorization': 'Bearer foo'
         },
-        method: {},
         query: {
-          allowed: 'false',
-          state: 'foobar'
+          state: 'foobar',
+          allowed: 'false'
         }
       });
       const response = new Response({ body: {}, headers: {} });
 
       return handler.handle(request, response)
         .then(should.fail)
-        .catch(function(e) {
-          e.should.be.an.instanceOf(AccessDeniedError);
-          e.message.should.equal('Access denied: user denied access to application');
-        });
-    });
-
-    it('should throw an error if `allowed` is `false` body', function() {
-      const model = {
-        getAccessToken: function() {},
-        getClient: function() {},
-        saveAuthorizationCode: function() {}
-      };
-      const handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model });
-      const request = new Request({ body: { allowed: 'false' }, headers: {}, method: {}, query: {} });
-      const response = new Response({ body: {}, headers: {} });
-
-      return handler.handle(request, response)
-        .then(should.fail)
-        .catch(function(e) {
-          e.should.be.an.instanceOf(AccessDeniedError);
-          e.message.should.equal('Access denied: user denied access to application');
+        .catch(function() {
+          response.get('location').should.equal('http://example.com/cb?error=access_denied&error_description=Access%20denied%3A%20user%20denied%20access%20to%20application&state=foobar');
         });
     });