From e75a1eb0d7c5da739754dec2619130474590674c Mon Sep 17 00:00:00 2001 From: Gereon Dusella Date: Tue, 17 Dec 2019 12:06:21 +0100 Subject: [PATCH 1/3] added fetching groups of user if the scope includes 'api', extended and refactored tests This commit includes changes that extend the profile-object returned by this application to include the groups the user has access to. This is done by means of a nested callback which is executed if the scope includes 'api'. The callback extends the profile object by the `groups`-property, which value is an array containing the group names. To test the behaviour of the newly added code, the strategy.profile.spec.js was extended with more tests. It was necessary to wrap the _oauth.get-method inside a closure to generate different behaviour, as this class mocks the behaviour of that method. The oAuthGetReplacementClosure now accepts a config object to e.g. return a malformed response to the groups request, but a correct one to the profile-request. The existing tests were adapted to use the newly introduced method. Signed-off-by: Gereon Dusella --- lib/strategy.js | 24 ++- test/strategy.profile.spec.js | 322 ++++++++++++++++++++++++++-------- test/strategy.spec.js | 4 + 3 files changed, 274 insertions(+), 76 deletions(-) diff --git a/lib/strategy.js b/lib/strategy.js index e5e8fbf..7009338 100644 --- a/lib/strategy.js +++ b/lib/strategy.js @@ -49,6 +49,7 @@ function Strategy(options, verify) { OAuth2Strategy.call(this, options, verify); this.name = 'gitlab'; this._profileURL = options.profileURL || url.resolve(this._baseURL, 'api/v4/user'); + this._groupsURL = options.groupsURL || url.resolve(this._baseURL, 'api/v4/groups?min_access_level=10'); this._oauth2.useAuthorizationHeaderforGET(true); } @@ -65,6 +66,7 @@ util.inherits(Strategy, OAuth2Strategy); * - `username` the user's GitLab username * - `displayName` the user's full name * - `emails` the proxied or contact email address granted by the user + * - `groups` the user's groups * - `avatarUrl` the URL of the GitLab profile picture * - `profileUrl` the URL to the GitLab profile of the user * @@ -105,7 +107,27 @@ Strategy.prototype.userProfile = function(accessToken, done) { profile._raw = body; profile._json = json; - done(null, profile); + if (self._scope.includes('api')) { + self._oauth2.get(self._groupsURL, accessToken, function(err, body) { + let jsonGroups; + + if (err) { + return done(new InternalOAuthError('Failed to fetch groups of user', err)); + } + + try { + jsonGroups = JSON.parse(body); + } catch (ex) { + return done(new Error('Failed to parse groups of user')); + } + + profile.groups = jsonGroups.map((g) => g.name); + + done(null, profile); + }); + } else { + done(null, profile); + } }); }; diff --git a/test/strategy.profile.spec.js b/test/strategy.profile.spec.js index 58749cd..c489e00 100644 --- a/test/strategy.profile.spec.js +++ b/test/strategy.profile.spec.js @@ -1,59 +1,131 @@ const expect = require('chai').expect; const GitLabStrategy = require('../lib/strategy'); -describe('Profile', function() { - describe('fetched from default endpoint', function() { - let profile; +/** + * This function wraps the mock for the default strategy._oauth.get function. + * It provides the option to configure the moc function to behave in specific ways. + * + * @param{object} config The config object + * @return {function} The adapted function + */ +function oAuthGetReplacementClosure(config) { + return function(url, accessToken, callback) { + if (!['https://gitlab.com/api/v4/user', 'https://gitlab.com/api/v4/groups?min_access_level=10'].includes(url)) { + return callback(new Error('incorrect url argument')); + } + if (accessToken !== 'token') { + return callback(new Error('incorrect token argument')); + } - const strategy = new GitLabStrategy({ - clientID: 'ABC123', - clientSecret: 'secret' - }, function() {}); + if (url === 'https://gitlab.com/api/v4/user') { + let body; + if (config.userError === 'none') { + body = JSON.stringify({ + // jshint camelcase: false + // jscs:disable requireCamelCaseOrUpperCaseIdentifiers + id: 1, + name: 'John Smith', + username: 'john_smith', + state: 'active', + avatar_url: 'https://gitlab.com/uploads/user/avatar/1/index.jpg', + web_url: 'https://gitlab.com/u/john_smith', + created_at: '2012-05-23T08:00:58Z', + is_admin: false, + bio: '', + location: '', + skype: '', + linkedin: '', + twitter: '', + website_url: '', + last_sign_in_at: '2012-09-23T08:00:58Z', + confirmed_at: '2012-06-23T08:00:58Z', + email: 'john@example.com', + theme_id: 4, + color_scheme_id: 5, + projects_limit: 100000, + current_sign_in_at: '2012-010-23T08:00:58Z', + identities: [], + can_create_group: true, + can_create_project: true, + two_factor_enabled: true, + external: false, + private_token: 'dd34asd13as' + // jshint camelcase: true + // jscs:enable requireCamelCaseOrUpperCaseIdentifiers + }); + } else if (config.userError === 'invalidToken') { + body = JSON.stringify({ + error: { + message: 'Invalid OAuth access token.', + type: 'OAuthException', + code: 190, + fbtraceid: 'XxXXXxXxX0x' + } + }); - strategy._oauth2.get = function(url, accessToken, callback) { - if (url !== 'https://gitlab.com/api/v4/user') { - return callback(new Error('incorrect url argument')); + return callback({statusCode: 400, data: body}); + } else if (config.userError === 'malformedResponse') { + body = 'Hello World.'; + } else if (config.userError === 'internalError') { + return callback(new Error('something went wrong')); } - if (accessToken !== 'token') { - return callback(new Error('incorrect token argument')); + + callback(null, body, undefined); + } else if (url === 'https://gitlab.com/api/v4/groups?min_access_level=10') { + let body; + if (config.groupsError === 'none') { + body = JSON.stringify([ + { + id: 0, + name: 'groupA', + path: 'groupA', + description: '', + visibility: 'private', + lfs_enabled: true, + avatar_url: null, + web_url: 'https://gitlab.com/groups/groupA', + request_access_enabled: false, + full_name: 'groupA', + full_path: 'groupA' + }, { + id: 1, + name: 'groupB', + path: 'groupB', + description: '', + visibility: 'private', + lfs_enabled: true, + avatar_url: null, + web_url: 'https://gitlab.com/groups/groupB', + request_access_enabled: false, + full_name: 'groupB', + full_path: 'groupB' + } + ]); + } else if (config.groupsError === 'malformedResponse') { + body = 'Hello World.'; + } else if (config.groupsError === 'internalError') { + return callback(new Error('something went wrong')); } + callback(null, body, undefined); + } + }; +} - const body = JSON.stringify({ - // jshint camelcase: false - // jscs:disable requireCamelCaseOrUpperCaseIdentifiers - id: 1, - name: 'John Smith', - username: 'john_smith', - state: 'active', - avatar_url: 'https://gitlab.com/uploads/user/avatar/1/index.jpg', - web_url: 'https://gitlab.com/u/john_smith', - created_at: '2012-05-23T08:00:58Z', - is_admin: false, - bio: '', - location: '', - skype: '', - linkedin: '', - twitter: '', - website_url: '', - last_sign_in_at: '2012-09-23T08:00:58Z', - confirmed_at: '2012-06-23T08:00:58Z', - email: 'john@example.com', - theme_id: 4, - color_scheme_id: 5, - projects_limit: 100000, - current_sign_in_at: '2012-010-23T08:00:58Z', - identities: [], - can_create_group: true, - can_create_project: true, - two_factor_enabled: true, - external: false, - private_token: 'dd34asd13as' - // jshint camelcase: true - // jscs:enable requireCamelCaseOrUpperCaseIdentifiers - }); +describe('Profile', function() { + describe('fetched from default endpoint using read_user scope', function() { + let profile; - callback(null, body, undefined); - }; + const strategy = new GitLabStrategy({ + clientID: 'ABC123', + clientSecret: 'secret', + scope: 'read_user' + }, function() { + }); + + strategy._oauth2.get = oAuthGetReplacementClosure({ + userError: 'none', + groupsError: 'none' + }); before(function(done) { strategy.userProfile('token', function(err, p) { @@ -82,28 +154,59 @@ describe('Profile', function() { it('should set json property', function() { expect(profile._json).to.be.an('object'); }); + + it('should not have property groups', function() { + expect(profile).to.not.have.property('groups'); + }); }); - describe('error caused by invalid token', function() { - let err; + describe('fetched from default endpoint using `read_user` and `api` scope', function() { + let profile; const strategy = new GitLabStrategy({ clientID: 'ABC123', - clientSecret: 'secret' - }, function() {}); - - strategy._oauth2.get = function(url, accessToken, callback) { - const body = JSON.stringify({ - error: { - message: 'Invalid OAuth access token.', - type: 'OAuthException', - code: 190, - fbtraceid: 'XxXXXxXxX0x' + clientSecret: 'secret', + scope: 'read_user api' + }, function() { + }); + + strategy._oauth2.get = oAuthGetReplacementClosure({ + userError: 'none', + groupsError: 'none' + }); + + before(function(done) { + strategy.userProfile('token', function(err, p) { + if (err) { + return done(err); } + profile = p; + done(); }); + }); - callback({statusCode: 400, data: body}); - }; + it('should have property groups', function() { + expect(profile).to.have.property('groups'); + }); + + it('should parse groups', function() { + expect(profile.groups).to.eql(['groupA', 'groupB']); + }); + }); + + describe('error caused by invalid token', function() { + let err; + + const strategy = new GitLabStrategy({ + clientID: 'ABC123', + clientSecret: 'secret', + scope: 'read_user' + }, function() { + }); + + strategy._oauth2.get = oAuthGetReplacementClosure({ + userError: 'invalidToken' + }); before(function(done) { strategy.userProfile('token', function(e, p) { @@ -120,18 +223,19 @@ describe('Profile', function() { }); }); - describe('error caused by malformed response', function() { + describe('error caused by malformed response at read profile', function() { let err; const strategy = new GitLabStrategy({ clientID: 'ABC123', - clientSecret: 'secret' - }, function() {}); + clientSecret: 'secret', + scope: 'read_user' + }, function() { + }); - strategy._oauth2.get = function(url, accessToken, callback) { - const body = 'Hello, world.'; - callback(null, body, undefined); - }; + strategy._oauth2.get = oAuthGetReplacementClosure({ + userError: 'malformedResponse' + }); before(function(done) { strategy.userProfile('token', function(e, p) { @@ -147,21 +251,52 @@ describe('Profile', function() { }); }); - describe('internal error', function() { + describe('error caused by malformed response at read groups', function() { + let err; + + const strategy = new GitLabStrategy({ + clientID: 'ABC123', + clientSecret: 'secret', + scope: 'read_user api' + }, function() { + }); + + strategy._oauth2.get = oAuthGetReplacementClosure({ + userError: 'none', + groupsError: 'malformedResponse' + }); + + before(function(done) { + strategy.userProfile('token', function(e, p) { + err = e; + profile = p; + done(); + }); + }); + + it('should error', function() { + expect(err).to.be.an.instanceOf(Error); + expect(err.message).to.equal('Failed to parse groups of user'); + }); + }); + + describe('internal error at read profile', function() { let err; let profile; const strategy = new GitLabStrategy({ clientID: 'ABC123', - clientSecret: 'secret' - }, function() {}); + clientSecret: 'secret', + scope: 'read_user' + }, function() { + }); - strategy._oauth2.get = function(url, accessToken, callback) { - return callback(new Error('something went wrong')); - }; + strategy._oauth2.get = oAuthGetReplacementClosure({ + userError: 'internalError' + }); before(function(done) { - strategy.userProfile('wrong-token', function(e, p) { + strategy.userProfile('token', function(e, p) { err = e; profile = p; done(); @@ -180,4 +315,41 @@ describe('Profile', function() { expect(profile).to.be.an('undefined'); }); }); + + describe('internal error at read groups', function() { + let err; + let profile; + + const strategy = new GitLabStrategy({ + clientID: 'ABC123', + clientSecret: 'secret', + scope: 'read_user api' + }, function() { + }); + + strategy._oauth2.get = oAuthGetReplacementClosure({ + userError: 'none', + groupsError: 'internalError' + }); + + before(function(done) { + strategy.userProfile('token', function(e, p) { + err = e; + profile = p; + done(); + }); + }); + + it('should error', function() { + expect(err).to.be.an.instanceOf(Error); + expect(err.constructor.name).to.equal('InternalOAuthError'); + expect(err.message).to.equal('Failed to fetch groups of user'); + expect(err.oauthError).to.be.an.instanceOf(Error); + expect(err.oauthError.message).to.equal('something went wrong'); + }); + + it('should not load profile', function() { + expect(profile).to.be.an('undefined'); + }); + }); }); diff --git a/test/strategy.spec.js b/test/strategy.spec.js index 92f1808..92b3129 100644 --- a/test/strategy.spec.js +++ b/test/strategy.spec.js @@ -23,6 +23,10 @@ describe('Strategy', function() { it('should have correct profile URL', function() { expect(strategy._profileURL).to.equal('https://example.com/gl/api/v4/user'); }); + + it('should have correct groups URL', function() { + expect(strategy._groupsURL).to.equal('https://example.com/gl/api/v4/groups'); + }); }); describe('constructed with undefined options', function() { From ea69cd1079a307108afbe536133344e776d45f10 Mon Sep 17 00:00:00 2001 From: Gereon Dusella Date: Tue, 17 Dec 2019 12:24:28 +0100 Subject: [PATCH 2/3] extended the README.md to inform about the newly added feature Signed-off-by: Gereon Dusella --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 531b7fb..019401c 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,12 @@ app.get('/auth/gitlab', })); ``` +##### How can I fetch the groups of the logged-in user? + +If the OAuth2-scope is set to `read_user api`, the user's groups are also +requested and included in the `groups` property of the returned object. +Groups are saved as an array of the group names represented as strings. + More information can be found in the [official GitLab documentation](https://docs.gitlab.com/ce/integration/oauth_provider.html#authorized-applications). ## Contributing From 7d4457a89f8fdf5fd7f238f6e2c1373f5c31e6e1 Mon Sep 17 00:00:00 2001 From: Gereon Dusella Date: Tue, 17 Dec 2019 12:32:55 +0100 Subject: [PATCH 3/3] the strategy.spec.js missed a query parameter the strategy.spec.js missed the query parameter which limits the administrator from being included in all groups. Signed-off-by: Gereon Dusella --- test/strategy.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/strategy.spec.js b/test/strategy.spec.js index 92b3129..3adf849 100644 --- a/test/strategy.spec.js +++ b/test/strategy.spec.js @@ -25,7 +25,7 @@ describe('Strategy', function() { }); it('should have correct groups URL', function() { - expect(strategy._groupsURL).to.equal('https://example.com/gl/api/v4/groups'); + expect(strategy._groupsURL).to.equal('https://example.com/gl/api/v4/groups?min_access_level=10'); }); });