From d5d0f3d82017d31fecfb943b47aa4c4b83af2c53 Mon Sep 17 00:00:00 2001 From: BoHong Li Date: Thu, 27 Feb 2020 23:49:05 +0800 Subject: [PATCH 1/2] fix: extractProfileAttribute not working correctly Signed-off-by: BoHong Li --- lib/auth/oauth2/index.js | 78 +------------------------------ lib/auth/oauth2/strategy.js | 74 +++++++++++++++++++++++++++++ lib/config/default.js | 7 ++- test/auth/oauth2/strategy.test.js | 56 ++++++++++++++++++++++ 4 files changed, 138 insertions(+), 77 deletions(-) create mode 100644 lib/auth/oauth2/strategy.js create mode 100644 test/auth/oauth2/strategy.test.js diff --git a/lib/auth/oauth2/index.js b/lib/auth/oauth2/index.js index cee4584f..fdb035bb 100644 --- a/lib/auth/oauth2/index.js +++ b/lib/auth/oauth2/index.js @@ -2,87 +2,13 @@ const Router = require('express').Router const passport = require('passport') -const { Strategy, InternalOAuthError } = require('passport-oauth2') + const config = require('../../config') const { setReturnToFromReferer, passportGeneralCallback } = require('../utils') +const { OAuth2CustomStrategy } = require('./strategy') const oauth2Auth = module.exports = Router() -class OAuth2CustomStrategy extends Strategy { - constructor (options, verify) { - options.customHeaders = options.customHeaders || {} - super(options, verify) - this.name = 'oauth2' - this._userProfileURL = options.userProfileURL - this._oauth2.useAuthorizationHeaderforGET(true) - } - - userProfile (accessToken, done) { - this._oauth2.get(this._userProfileURL, accessToken, function (err, body, res) { - var json - - if (err) { - return done(new InternalOAuthError('Failed to fetch user profile', err)) - } - - try { - json = JSON.parse(body) - } catch (ex) { - return done(new Error('Failed to parse user profile')) - } - - const profile = parseProfile(json) - profile.provider = 'oauth2' - - done(null, profile) - }) - } -} - -function extractProfileAttribute (data, path) { - // can handle stuff like `attrs[0].name` - path = path.split('.') - for (const segment of path) { - const m = segment.match(/([\d\w]+)\[(.*)\]/) - data = m ? data[m[1]][m[2]] : data[segment] - } - return data -} - -function parseProfile (data) { - const username = extractProfileAttribute(data, config.oauth2.userProfileUsernameAttr) - const displayName = extractProfileAttribute(data, config.oauth2.userProfileDisplayNameAttr) - const email = extractProfileAttribute(data, config.oauth2.userProfileEmailAttr) - - return { - id: username, - username: username, - displayName: displayName, - email: email - } -} - -OAuth2CustomStrategy.prototype.userProfile = function (accessToken, done) { - this._oauth2.get(this._userProfileURL, accessToken, function (err, body, res) { - var json - - if (err) { - return done(new InternalOAuthError('Failed to fetch user profile', err)) - } - - try { - json = JSON.parse(body) - } catch (ex) { - return done(new Error('Failed to parse user profile')) - } - - const profile = parseProfile(json) - profile.provider = 'oauth2' - - done(null, profile) - }) -} - passport.use(new OAuth2CustomStrategy({ authorizationURL: config.oauth2.authorizationURL, tokenURL: config.oauth2.tokenURL, diff --git a/lib/auth/oauth2/strategy.js b/lib/auth/oauth2/strategy.js new file mode 100644 index 00000000..7f4abb63 --- /dev/null +++ b/lib/auth/oauth2/strategy.js @@ -0,0 +1,74 @@ +'use strict' + +const { Strategy, InternalOAuthError } = require('passport-oauth2') +const config = require('../../config') + +function parseProfile (data) { + const username = extractProfileAttribute(data, config.oauth2.userProfileUsernameAttr) + const displayName = extractProfileAttribute(data, config.oauth2.userProfileDisplayNameAttr) + const email = extractProfileAttribute(data, config.oauth2.userProfileEmailAttr) + + if (!username) { + throw new Error('cannot fetch username: please set correct CMD_OAUTH2_USER_PROFILE_USERNAME_ATTR') + } + + return { + id: username, + username: username, + displayName: displayName, + email: email + } +} + +function extractProfileAttribute (data, path) { + if (!data) return undefined + if (typeof path !== 'string') return undefined + // can handle stuff like `attrs[0].name` + path = path.split('.') + for (const segment of path) { + const m = segment.match(/([\d\w]+)\[(.*)\]/) + if (!m) { + data = data[segment] + } else { + if (m.length < 3) return undefined + if (!data[m[1]]) return undefined + data = data[m[1]][m[2]] + } + if (!data) return undefined + } + return data +} + +class OAuth2CustomStrategy extends Strategy { + constructor (options, verify) { + options.customHeaders = options.customHeaders || {} + super(options, verify) + this.name = 'oauth2' + this._userProfileURL = options.userProfileURL + this._oauth2.useAuthorizationHeaderforGET(true) + } + + userProfile (accessToken, done) { + this._oauth2.get(this._userProfileURL, accessToken, function (err, body, res) { + if (err) { + return done(new InternalOAuthError('Failed to fetch user profile', err)) + } + + let profile, json + try { + json = JSON.parse(body) + profile = parseProfile(json) + } catch (ex) { + return done(new InternalOAuthError('Failed to parse user profile' + ex.toString())) + } + + profile.provider = 'oauth2' + + done(null, profile) + }) + } +} + +exports.OAuth2CustomStrategy = OAuth2CustomStrategy +exports.parseProfile = parseProfile +exports.extractProfileAttribute = extractProfileAttribute diff --git a/lib/config/default.js b/lib/config/default.js index 2e76b143..65f29b36 100644 --- a/lib/config/default.js +++ b/lib/config/default.js @@ -93,7 +93,12 @@ module.exports = { authorizationURL: undefined, tokenURL: undefined, clientID: undefined, - clientSecret: undefined + clientSecret: undefined, + baseURL: undefined, + userProfileURL: undefined, + userProfileUsernameAttr: 'username', + userProfileDisplayNameAttr: 'displayName', + userProfileEmailAttr: 'email' }, facebook: { clientID: undefined, diff --git a/test/auth/oauth2/strategy.test.js b/test/auth/oauth2/strategy.test.js new file mode 100644 index 00000000..4902b040 --- /dev/null +++ b/test/auth/oauth2/strategy.test.js @@ -0,0 +1,56 @@ +/* eslint-env node, mocha */ +'use strict' + +const assert = require('assert') +const chance = require('chance')() + +const { extractProfileAttribute } = require('../../../lib/auth/oauth2/strategy') + +describe('OAuth2CustomStrategy', function () { + describe('#extractProfileAttribute', function () { + const data = { + user: { + email: chance.email() + }, + arrayData: [ + { + email: chance.email() + }, + { + email: chance.email() + } + ] + } + + it('should parse normal attribute correctly', function () { + assert(extractProfileAttribute(data, 'user.email') === data.user.email) + }) + + it('should return undefined when nested object key not exists', function () { + assert(extractProfileAttribute(data, 'user.profile') === undefined) + }) + + it('should return undefined when whole object key not exists', function () { + assert(extractProfileAttribute(data, 'profile.email') === undefined) + }) + + it('should return attribute in array correct', function () { + assert(extractProfileAttribute(data, 'arrayData[0].email') === data.arrayData[0].email) + assert(extractProfileAttribute(data, 'arrayData[1].email') === data.arrayData[1].email) + }) + + it('should return undefined when array index out of bound', function () { + assert(extractProfileAttribute(data, 'arrayData[3].email') === undefined) + }) + + it('should return undefined when array key not exists', function () { + assert(extractProfileAttribute(data, 'notExistsArray[5].email') === undefined) + }) + + it('should return undefined when data is undefined', function () { + assert(extractProfileAttribute(undefined, 'email') === undefined) + assert(extractProfileAttribute(null, 'email') === undefined) + assert(extractProfileAttribute({}, 'email') === undefined) + }) + }) +}) From 72c5b0d14e856c692fb4f015916e666e42ba1f9d Mon Sep 17 00:00:00 2001 From: BoHong Li Date: Fri, 28 Feb 2020 02:13:58 +0800 Subject: [PATCH 2/2] feat: support customize scope in OAuth2 provider Signed-off-by: BoHong Li --- lib/auth/oauth2/index.js | 3 ++- lib/config/default.js | 3 ++- lib/config/environment.js | 11 ++++++----- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/auth/oauth2/index.js b/lib/auth/oauth2/index.js index fdb035bb..f223a159 100644 --- a/lib/auth/oauth2/index.js +++ b/lib/auth/oauth2/index.js @@ -15,7 +15,8 @@ passport.use(new OAuth2CustomStrategy({ clientID: config.oauth2.clientID, clientSecret: config.oauth2.clientSecret, callbackURL: config.serverURL + '/auth/oauth2/callback', - userProfileURL: config.oauth2.userProfileURL + userProfileURL: config.oauth2.userProfileURL, + scope: config.oauth2.scope }, passportGeneralCallback)) oauth2Auth.get('/auth/oauth2', function (req, res, next) { diff --git a/lib/config/default.js b/lib/config/default.js index 65f29b36..12324680 100644 --- a/lib/config/default.js +++ b/lib/config/default.js @@ -98,7 +98,8 @@ module.exports = { userProfileURL: undefined, userProfileUsernameAttr: 'username', userProfileDisplayNameAttr: 'displayName', - userProfileEmailAttr: 'email' + userProfileEmailAttr: 'email', + scope: 'email' }, facebook: { clientID: undefined, diff --git a/lib/config/environment.js b/lib/config/environment.js index 8426d82f..e46c58f1 100644 --- a/lib/config/environment.js +++ b/lib/config/environment.js @@ -88,14 +88,15 @@ module.exports = { oauth2: { providerName: process.env.CMD_OAUTH2_PROVIDERNAME, baseURL: process.env.CMD_OAUTH2_BASEURL, + clientID: process.env.CMD_OAUTH2_CLIENT_ID, + clientSecret: process.env.CMD_OAUTH2_CLIENT_SECRET, + authorizationURL: process.env.CMD_OAUTH2_AUTHORIZATION_URL, + tokenURL: process.env.CMD_OAUTH2_TOKEN_URL, userProfileURL: process.env.CMD_OAUTH2_USER_PROFILE_URL, + scope: process.env.CMD_OAUTH2_SCOPE, userProfileUsernameAttr: process.env.CMD_OAUTH2_USER_PROFILE_USERNAME_ATTR, userProfileDisplayNameAttr: process.env.CMD_OAUTH2_USER_PROFILE_DISPLAY_NAME_ATTR, - userProfileEmailAttr: process.env.CMD_OAUTH2_USER_PROFILE_EMAIL_ATTR, - tokenURL: process.env.CMD_OAUTH2_TOKEN_URL, - authorizationURL: process.env.CMD_OAUTH2_AUTHORIZATION_URL, - clientID: process.env.CMD_OAUTH2_CLIENT_ID, - clientSecret: process.env.CMD_OAUTH2_CLIENT_SECRET + userProfileEmailAttr: process.env.CMD_OAUTH2_USER_PROFILE_EMAIL_ATTR }, dropbox: { clientID: process.env.CMD_DROPBOX_CLIENTID,