From 1b747b51194990fb03d8839461257b2eb2ced23f Mon Sep 17 00:00:00 2001 From: Jonathon Storer Date: Thu, 11 May 2023 10:33:12 -0400 Subject: [PATCH 1/3] Add handler for subclasses to process the response from OAuth2 token request before validation --- lib/strategy.js | 28 +++-- test/oauth2.subclass.test.js | 228 ++++++++++++++++++++++++++++------- 2 files changed, 208 insertions(+), 48 deletions(-) diff --git a/lib/strategy.js b/lib/strategy.js index 8575b72..054f5f7 100644 --- a/lib/strategy.js +++ b/lib/strategy.js @@ -172,9 +172,12 @@ OAuth2Strategy.prototype.authenticate = function(req, options) { params.code_verifier = ok; } - self._oauth2.getOAuthAccessToken(code, params, - function(err, accessToken, refreshToken, params) { - if (err) { return self.error(self._createOAuthError('Failed to obtain access token', err)); } + self._oauth2.getOAuthAccessToken(code, params, function(err, _accessToken, _refreshToken, _params) { + if (err) { return self.error(self._createOAuthError('Failed to obtain access token', err)); } + + self.handleOAuthAccessTokenReponse(_accessToken, _refreshToken, _params, function (err, accessToken, refreshToken, params) { + if (err) { return self.error(self._createOAuthError('Failed to handle oauth access token response', err)); } + if (!accessToken) { return self.error(new Error('Failed to obtain access token')); } self._loadUserProfile(accessToken, function(err, profile) { @@ -206,11 +209,14 @@ OAuth2Strategy.prototype.authenticate = function(req, options) { } } } catch (ex) { + console.log(ex); return self.error(ex); } }); - } - ); + + }); + + }); } var state = (req.query && req.query.state) || (req.body && req.body.state); @@ -247,7 +253,7 @@ OAuth2Strategy.prototype.authenticate = function(req, options) { default: return this.error(new Error('Unsupported code verifier transformation method: ' + this._pkceMethod)); } - + params.code_challenge = challenge; params.code_challenge_method = this._pkceMethod; } @@ -265,7 +271,7 @@ OAuth2Strategy.prototype.authenticate = function(req, options) { // the state will be automatically managed and persisted by the // state store. params.state = state; - + var parsed = url.parse(this._oauth2._authorizeUrl, true); utils.merge(parsed.query, params); parsed.query['client_id'] = this._oauth2._clientId; @@ -303,6 +309,14 @@ OAuth2Strategy.prototype.authenticate = function(req, options) { } }; +/** + * Write a blurb + */ + +OAuth2Strategy.prototype.handleOAuthAccessTokenReponse = function(accessToken, refreshToken, params, done) { + done(null, accessToken, refreshToken, params); +}; + /** * Retrieve user profile from service provider. * diff --git a/test/oauth2.subclass.test.js b/test/oauth2.subclass.test.js index f49129e..b2caefb 100644 --- a/test/oauth2.subclass.test.js +++ b/test/oauth2.subclass.test.js @@ -4,9 +4,8 @@ var OAuth2Strategy = require('../lib/strategy') , chai = require('chai') , util = require('util'); - describe('OAuth2Strategy subclass', function() { - + describe('that overrides authorizationParams', function() { function FooOAuth2Strategy(options, verify) { OAuth2Strategy.call(this, options, verify); @@ -16,8 +15,8 @@ describe('OAuth2Strategy subclass', function() { FooOAuth2Strategy.prototype.authorizationParams = function(options) { return { prompt: options.prompt }; } - - + + describe('issuing authorization request that redirects to service provider', function() { var strategy = new FooOAuth2Strategy({ authorizationURL: 'https://www.example.com/oauth2/authorize', @@ -29,14 +28,14 @@ describe('OAuth2Strategy subclass', function() { function(accessToken, refreshToken, profile, done) { if (accessToken !== '2YotnFZFEjr1zCsicMWpAA') { return done(new Error('incorrect accessToken argument')); } if (refreshToken !== 'tGzv3JOkF0XG5Qx2TlKWIA') { return done(new Error('incorrect refreshToken argument')); } - + return done(null, { id: '1234' }, { message: 'Hello' }); }); - - + + describe('with prompt', function() { var url; - + before(function(done) { chai.passport.use(strategy) .redirect(function(u) { @@ -47,15 +46,15 @@ describe('OAuth2Strategy subclass', function() { }) .authenticate({ prompt: 'mobile' }); }); - + it('should be redirected', function() { expect(url).to.equal('https://www.example.com/oauth2/authorize?prompt=mobile&response_type=code&redirect_uri=https%3A%2F%2Fwww.example.net%2Fauth%2Fexample%2Fcallback&client_id=ABC123'); }); }); // with prompt - + describe('with scope and prompt', function() { var url; - + before(function(done) { chai.passport.use(strategy) .redirect(function(u) { @@ -66,17 +65,17 @@ describe('OAuth2Strategy subclass', function() { }) .authenticate({ scope: 'email', prompt: 'mobile' }); }); - + it('should be redirected', function() { expect(url).to.equal('https://www.example.com/oauth2/authorize?prompt=mobile&response_type=code&redirect_uri=https%3A%2F%2Fwww.example.net%2Fauth%2Fexample%2Fcallback&scope=email&client_id=ABC123'); }); }); // with scope and prompt - + }); // issuing authorization request that redirects to service provider - + }); // that overrides authorizationParams - - + + describe('that overrides tokenParams', function() { function FooOAuth2Strategy(options, verify) { OAuth2Strategy.call(this, options, verify); @@ -86,8 +85,8 @@ describe('OAuth2Strategy subclass', function() { FooOAuth2Strategy.prototype.tokenParams = function(options) { return { type: options.type }; } - - + + describe('processing response to authorization request that was approved', function() { var strategy = new FooOAuth2Strategy({ authorizationURL: 'https://www.example.com/oauth2/authorize', @@ -99,23 +98,23 @@ describe('OAuth2Strategy subclass', function() { function(accessToken, refreshToken, profile, done) { if (accessToken !== '2YotnFZFEjr1zCsicMWpAA') { return done(new Error('incorrect accessToken argument')); } if (refreshToken !== 'tGzv3JOkF0XG5Qx2TlKWIA') { return done(new Error('incorrect refreshToken argument')); } - + return done(null, { id: '1234' }, { message: 'Hello' }); }); - + strategy._oauth2.getOAuthAccessToken = function(code, options, callback) { if (code !== 'SplxlOBeZQQYbYS6WxSbIA') { return callback(new Error('incorrect code argument')); } if (options.grant_type !== 'authorization_code') { return callback(new Error('incorrect options.grant_type argument')); } if (options.redirect_uri !== 'https://www.example.net/auth/example/callback') { return callback(new Error('incorrect options.redirect_uri argument')); } if (options.type !== 'web_server') { return callback(new Error('incorrect options.type argument')); } - + callback(null, '2YotnFZFEjr1zCsicMWpAA', 'tGzv3JOkF0XG5Qx2TlKWIA', { token_type: 'example' }); } - - + + var user , info; - + before(function(done) { chai.passport.use(strategy) .success(function(u, i) { @@ -140,10 +139,10 @@ describe('OAuth2Strategy subclass', function() { expect(info.message).to.equal('Hello'); }); }); // processing response to authorization request that was approved - + }); // that overrides tokenParams - - + + describe('that overrides parseErrorResponse', function() { function FooOAuth2Strategy(options, verify) { OAuth2Strategy.call(this, options, verify); @@ -152,14 +151,14 @@ describe('OAuth2Strategy subclass', function() { FooOAuth2Strategy.prototype.parseErrorResponse = function(body, status) { if (status === 500) { throw new Error('something went horribly wrong'); } - + var e = new Error('Custom OAuth error'); e.body = body; e.status = status; return e; } - - + + describe('and supplies error', function() { var strategy = new FooOAuth2Strategy({ authorizationURL: 'https://www.example.com/oauth2/authorize', @@ -169,17 +168,17 @@ describe('OAuth2Strategy subclass', function() { callbackURL: 'https://www.example.net/auth/example/callback', }, function(accessToken, refreshToken, profile, done) { - if (accessToken == '2YotnFZFEjr1zCsicMWpAA' && refreshToken == 'tGzv3JOkF0XG5Qx2TlKWIA') { + if (accessToken == '2YotnFZFEjr1zCsicMWpAA' && refreshToken == 'tGzv3JOkF0XG5Qx2TlKWIA') { return done(null, { id: '1234' }, { message: 'Hello' }); } return done(null, false); }); - + strategy._oauth2.getOAuthAccessToken = function(code, options, callback) { return callback({ statusCode: 400, data: 'Invalid code' }); } - - + + var err; before(function(done) { @@ -202,7 +201,7 @@ describe('OAuth2Strategy subclass', function() { expect(err.status).to.equal(400); }); }); // and supplies error - + describe('and throws exception', function() { var strategy = new FooOAuth2Strategy({ authorizationURL: 'https://www.example.com/oauth2/authorize', @@ -212,17 +211,17 @@ describe('OAuth2Strategy subclass', function() { callbackURL: 'https://www.example.net/auth/example/callback', }, function(accessToken, refreshToken, profile, done) { - if (accessToken == '2YotnFZFEjr1zCsicMWpAA' && refreshToken == 'tGzv3JOkF0XG5Qx2TlKWIA') { + if (accessToken == '2YotnFZFEjr1zCsicMWpAA' && refreshToken == 'tGzv3JOkF0XG5Qx2TlKWIA') { return done(null, { id: '1234' }, { message: 'Hello' }); } return done(null, false); }); - + strategy._oauth2.getOAuthAccessToken = function(code, options, callback) { return callback({ statusCode: 500, data: 'Invalid code' }); } - - + + var err; before(function(done) { @@ -245,7 +244,154 @@ describe('OAuth2Strategy subclass', function() { expect(err.oauthError.data).to.equal('Invalid code'); }); }); // and throws exception - + }); // that overrides parseErrorResponse - + + describe('that overrides handleOAuthAccessTokenReponse', function () { + var options = { + authorizationURL: 'https://www.example.com/oauth2/authorize', + tokenURL: 'https://www.example.com/oauth2/token', + clientID: 'clientId', + clientSecret: 'clientSecret', + callbackURL: 'https://www.example.net/auth/example/callback', + }; + + function BarOAuth2Strategy(options, verify) { + OAuth2Strategy.call(this, options, verify); + } + util.inherits(BarOAuth2Strategy, OAuth2Strategy); + + BarOAuth2Strategy.prototype.handleOAuthAccessTokenReponse = function(accessToken, refreshToken, params, done) { + done(new Error('this needs to be defined in each test')); + } + + var strategy = new BarOAuth2Strategy(options, function(accessToken, refreshToken, profile, done) { + return done(null, profile); + }); + + strategy._oauth2.getOAuthAccessToken = function(code, options, callback) { + return callback(null, 'access-token', 'refresh-token', { token_type: 'example' }); + } + + beforeEach(function () { + strategy.__verify = strategy._verify; + }); + + afterEach(function () { + strategy._verify = strategy.__verify; + delete strategy.__verify; + }); + + describe('failure', function () { + var err; + + describe('handleOAuthAccessTokenReponse ecounters an exception', function () { + beforeEach(function(done) { + strategy._verify = function (at, rt, params, done) { + return done(new Error('verify callback should not be called')); + } + + strategy.__proto__.handleOAuthAccessTokenReponse = function(at, rt, p, done) { done(new Error()); } + + chai.passport.use(strategy) + .error(function(e) { + err = e; + done(); + }) + .req(function(req) { + req.query = {}; + req.query.code = 'authorization-code'; + }) + .authenticate(); + }); + + it('should error', function() { + expect(err).to.be.an.instanceof(Error) + expect(err.message).to.equal('Failed to handle oauth access token response'); + }); + }); // exception encountered in handleOAuthAccessTokenReponse + + describe('handleOAuthAccessTokenReponse does not pass an accessToken back', function () { + beforeEach(function(done) { + strategy.__proto__.handleOAuthAccessTokenReponse = function(at, rt, p, done) { + done(); + } + + chai.passport.use(strategy) + .error(function(e) { + err = e; + done(); + }) + .req(function(req) { + req.query = {}; + req.query.code = 'authorization-code'; + }) + .authenticate(); + }); + + it('should error', function() { + expect(err).to.be.an.instanceof(Error) + expect(err.message).to.equal('Failed to obtain access token'); + }); + }); // failed to pass back accessToken handleOAuthAccessTokenReponse + }); + + describe('success', function () { + var user + , info + , accessToken; + + beforeEach(function() { + strategy._verify = function (accessToken, refreshToken, params, profile, next) { + profile.user = true; + profile.accessToken = accessToken; + profile.refreshToken = refreshToken; + + next(null, profile, params) + } + + strategy.__proto__.handleOAuthAccessTokenReponse = function(at, rt, p, done) { + expect(at).to.eql('at'); + expect(rt).to.eql('rt'); + expect(p).to.eql({ token_type: 'test' }); + + process.nextTick(function () { + at = 'newAt'; + rt = 'newRt'; + p.token_type = 'new token type'; + + done(null, at, rt, p); + }); + } + + strategy._oauth2.getOAuthAccessToken = function(code, options, callback) { + return callback(null, 'at', 'rt', { token_type: 'test' }); + } + }); + + it('alters the accessToken, refreshToken, and params', function(done) { + chai.passport.use(strategy) + .success(function(profile, info) { + try { + expect(profile).to.be.an.object; + expect(profile.user).to.be.true; + expect(profile.accessToken).to.eql('newAt'); + expect(profile.refreshToken).to.eql('newRt'); + + expect(info).to.be.an.object; + expect(info.token_type).to.eql('new token type'); + } catch (e) { + return done(e); + } + return done(); + }) + .req(function(req) { + req.query = { code: 'code' }; + }) + .authenticate(); + }); // failed to pass back accessToken handleOAuthAccessTokenReponse + }); + + }); // that overrides handleOAuthAccessTokenReponse + }); From 78927867aa54901ecfb1cfda6902e8332a33473c Mon Sep 17 00:00:00 2001 From: Jonathon Storer Date: Thu, 11 May 2023 15:55:41 -0400 Subject: [PATCH 2/3] wip --- lib/strategy.js | 18 +++++++++++++--- test/oauth2.subclass.test.js | 42 +++++++++++++++++------------------- 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/lib/strategy.js b/lib/strategy.js index 054f5f7..7fca04c 100644 --- a/lib/strategy.js +++ b/lib/strategy.js @@ -175,7 +175,7 @@ OAuth2Strategy.prototype.authenticate = function(req, options) { self._oauth2.getOAuthAccessToken(code, params, function(err, _accessToken, _refreshToken, _params) { if (err) { return self.error(self._createOAuthError('Failed to obtain access token', err)); } - self.handleOAuthAccessTokenReponse(_accessToken, _refreshToken, _params, function (err, accessToken, refreshToken, params) { + self.handleOAuthAccessTokenResponse(_accessToken, _refreshToken, _params, function (err, accessToken, refreshToken, params) { if (err) { return self.error(self._createOAuthError('Failed to handle oauth access token response', err)); } if (!accessToken) { return self.error(new Error('Failed to obtain access token')); } @@ -310,10 +310,22 @@ OAuth2Strategy.prototype.authenticate = function(req, options) { }; /** - * Write a blurb + * Handle OAuthAccessTokenResponse. + * + * OAuth 2.0-based authentication strategies can overrride this function in + * order to format the OAuth Access Token Response + * + * Some auth providers overload the OAuthAccessToken response + * which may require reformatting before continuing with the standard flow + * See https://github.com/nmaves/passport-slack-oauth2/issues/9#issuecomment-1544231819 + * + * @param {String:null} accessToken + * @param {String:null} refreshToken + * @param {Object} params + * @param {Function} done */ -OAuth2Strategy.prototype.handleOAuthAccessTokenReponse = function(accessToken, refreshToken, params, done) { +OAuth2Strategy.prototype.handleOAuthAccessTokenResponse = function(accessToken, refreshToken, params, done) { done(null, accessToken, refreshToken, params); }; diff --git a/test/oauth2.subclass.test.js b/test/oauth2.subclass.test.js index b2caefb..b2e3b6c 100644 --- a/test/oauth2.subclass.test.js +++ b/test/oauth2.subclass.test.js @@ -247,7 +247,7 @@ describe('OAuth2Strategy subclass', function() { }); // that overrides parseErrorResponse - describe('that overrides handleOAuthAccessTokenReponse', function () { + describe('that overrides handleOAuthAccessTokenResponse', function () { var options = { authorizationURL: 'https://www.example.com/oauth2/authorize', tokenURL: 'https://www.example.com/oauth2/token', @@ -261,7 +261,7 @@ describe('OAuth2Strategy subclass', function() { } util.inherits(BarOAuth2Strategy, OAuth2Strategy); - BarOAuth2Strategy.prototype.handleOAuthAccessTokenReponse = function(accessToken, refreshToken, params, done) { + BarOAuth2Strategy.prototype.handleOAuthAccessTokenResponse = function(accessToken, refreshToken, params, done) { done(new Error('this needs to be defined in each test')); } @@ -285,13 +285,13 @@ describe('OAuth2Strategy subclass', function() { describe('failure', function () { var err; - describe('handleOAuthAccessTokenReponse ecounters an exception', function () { + describe('handleOAuthAccessTokenResponse encounters an exception', function () { beforeEach(function(done) { strategy._verify = function (at, rt, params, done) { return done(new Error('verify callback should not be called')); } - strategy.__proto__.handleOAuthAccessTokenReponse = function(at, rt, p, done) { done(new Error()); } + strategy.__proto__.handleOAuthAccessTokenResponse = function(at, rt, p, done) { done(new Error()); } chai.passport.use(strategy) .error(function(e) { @@ -309,11 +309,11 @@ describe('OAuth2Strategy subclass', function() { expect(err).to.be.an.instanceof(Error) expect(err.message).to.equal('Failed to handle oauth access token response'); }); - }); // exception encountered in handleOAuthAccessTokenReponse + }); // exception encountered in handleOAuthAccessTokenResponse - describe('handleOAuthAccessTokenReponse does not pass an accessToken back', function () { + describe('handleOAuthAccessTokenResponse does not pass an accessToken back', function () { beforeEach(function(done) { - strategy.__proto__.handleOAuthAccessTokenReponse = function(at, rt, p, done) { + strategy.__proto__.handleOAuthAccessTokenResponse = function(at, rt, p, done) { done(); } @@ -333,8 +333,8 @@ describe('OAuth2Strategy subclass', function() { expect(err).to.be.an.instanceof(Error) expect(err.message).to.equal('Failed to obtain access token'); }); - }); // failed to pass back accessToken handleOAuthAccessTokenReponse - }); + }); // failed to pass back accessToken handleOAuthAccessTokenResponse + }); // failed describe('success', function () { var user @@ -350,17 +350,17 @@ describe('OAuth2Strategy subclass', function() { next(null, profile, params) } - strategy.__proto__.handleOAuthAccessTokenReponse = function(at, rt, p, done) { - expect(at).to.eql('at'); - expect(rt).to.eql('rt'); - expect(p).to.eql({ token_type: 'test' }); + strategy.__proto__.handleOAuthAccessTokenResponse = function(accesToken, refreshToken, params, done) { + expect(accesToken).to.eql('at'); + expect(refreshToken).to.eql('rt'); + expect(params).to.eql({ token_type: 'test' }); process.nextTick(function () { - at = 'newAt'; - rt = 'newRt'; - p.token_type = 'new token type'; + accesToken = 'newAt'; + refreshToken = 'newRt'; + params.token_type = 'new token type'; - done(null, at, rt, p); + done(null, accesToken, refreshToken, params); }); } @@ -389,9 +389,7 @@ describe('OAuth2Strategy subclass', function() { req.query = { code: 'code' }; }) .authenticate(); - }); // failed to pass back accessToken handleOAuthAccessTokenReponse - }); - - }); // that overrides handleOAuthAccessTokenReponse - + }); // failed to pass back accessToken handleOAuthAccessTokenResponse + }); // success + }); // that overrides handleOAuthAccessTokenResponse }); From 1bd13632c5d19e6b6cf27c14de617c252f41eb12 Mon Sep 17 00:00:00 2001 From: Jonathon Storer Date: Fri, 12 May 2023 09:51:01 -0400 Subject: [PATCH 3/3] remove console.log --- lib/strategy.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/strategy.js b/lib/strategy.js index 7fca04c..2ac51af 100644 --- a/lib/strategy.js +++ b/lib/strategy.js @@ -209,7 +209,6 @@ OAuth2Strategy.prototype.authenticate = function(req, options) { } } } catch (ex) { - console.log(ex); return self.error(ex); } });