From 9037614b037becf13526c9b465a6def177c8a769 Mon Sep 17 00:00:00 2001 From: Igor Czechowski Date: Tue, 23 Jan 2018 17:59:56 +0100 Subject: [PATCH 1/2] Compute the correct redirect_uri in case of resource over denies access According to https://tools.ietf.org/html/rfc6749#section-4.1.2.1 once the redirect_uri & client_id is correct authorization server should inform the clinet, that user denied access. The change is to move validation of resource owner approval after the redirect_uri & client identifier validation so the correct redirect url is computed --- lib/handlers/authorize-handler.js | 21 +++++++----- .../handlers/authorize-handler_test.js | 33 +++++++++++++++---- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/lib/handlers/authorize-handler.js b/lib/handlers/authorize-handler.js index 984136a8d..682e07a0d 100644 --- a/lib/handlers/authorize-handler.js +++ b/lib/handlers/authorize-handler.js @@ -78,9 +78,9 @@ AuthorizeHandler.prototype.handle = function(request, response) { throw new InvalidArgumentError('Invalid argument: `response` must be an instance of Response'); } - if ('false' === request.query.allowed) { - return Promise.reject(new AccessDeniedError('Access denied: user denied access to application')); - } + // if ('false' === request.query.allowed) { + // return Promise.reject(new AccessDeniedError('Access denied: user denied access to application')); + // } var fns = [ this.getAuthorizationCodeLifetime(), @@ -97,14 +97,19 @@ AuthorizeHandler.prototype.handle = function(request, response) { var ResponseType; return Promise.bind(this) - .then(function() { + .then(function() { scope = this.getScope(request); - - return this.generateAuthorizationCode(client, user, scope); - }) - .then(function(authorizationCode) { state = this.getState(request); ResponseType = this.getResponseType(request); + }) + .then(function() { + if ('false' === request.query.allowed) { + return Promise.reject(new AccessDeniedError('Access denied: user denied access to application')); + } + + return this.generateAuthorizationCode(client, user, scope); + }) + .then(function(authorizationCode) { return this.saveAuthorizationCode(authorizationCode, expiresAt, scope, client, uri, user); }) diff --git a/test/integration/handlers/authorize-handler_test.js b/test/integration/handlers/authorize-handler_test.js index 0d1aa333b..44dfc5fd7 100644 --- a/test/integration/handlers/authorize-handler_test.js +++ b/test/integration/handlers/authorize-handler_test.js @@ -159,21 +159,40 @@ 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() { var model = { - getAccessToken: function() {}, - getClient: function() {}, + getAccessToken: function() { + return { + user: {}, + accessTokenExpiresAt: new Date(new Date().getTime() + 10000) + }; + }, + getClient: function() { + return { grants: ['authorization_code'], redirectUris: ['http://example.com/cb'] }; + }, saveAuthorizationCode: function() {} }; var handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model }); - var request = new Request({ body: {}, headers: {}, method: {}, query: { allowed: 'false' } }); + var request = new Request({ + body: { + client_id: 12345, + response_type: 'code' + }, + method: {}, + headers: { + 'Authorization': 'Bearer foo' + }, + query: { + state: 'foobar', + allowed: 'false' + } + }); var 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'); }); }); From 38f9b15e1a7b5ede464f33730103edc8d03242fb Mon Sep 17 00:00:00 2001 From: Szymon Kiebzak Date: Tue, 23 Apr 2019 14:55:03 +0200 Subject: [PATCH 2/2] Remove commented code --- lib/handlers/authorize-handler.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/handlers/authorize-handler.js b/lib/handlers/authorize-handler.js index 682e07a0d..54bda5f6e 100644 --- a/lib/handlers/authorize-handler.js +++ b/lib/handlers/authorize-handler.js @@ -78,10 +78,6 @@ AuthorizeHandler.prototype.handle = function(request, response) { throw new InvalidArgumentError('Invalid argument: `response` must be an instance of Response'); } - // if ('false' === request.query.allowed) { - // return Promise.reject(new AccessDeniedError('Access denied: user denied access to application')); - // } - var fns = [ this.getAuthorizationCodeLifetime(), this.getClient(request),