From ede3b6a24816caf6f4830ba668ef974fb9b679d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Alby?= Date: Tue, 15 Sep 2020 14:49:42 +0200 Subject: [PATCH] Merge pull request #3152 from overleaf/ta-jpa-confirm-entitlement Send Entitlement for Affiliations During Domains Confirmation GitOrigin-RevId: 9d6b41022adfdb5e1a797b9471830014b1ef43e3 --- .../Features/Institutions/InstitutionsAPI.js | 10 +++- .../Institutions/InstitutionsController.js | 25 +++++--- .../src/Institutions/InstitutionsAPITests.js | 6 +- .../InstitutionsControllerTests.js | 58 ++++++++++++++++--- 4 files changed, 79 insertions(+), 20 deletions(-) diff --git a/services/web/app/src/Features/Institutions/InstitutionsAPI.js b/services/web/app/src/Features/Institutions/InstitutionsAPI.js index bb503dade9..1c566a331a 100644 --- a/services/web/app/src/Features/Institutions/InstitutionsAPI.js +++ b/services/web/app/src/Features/Institutions/InstitutionsAPI.js @@ -60,12 +60,18 @@ const InstitutionsAPI = { affiliationOptions = {} } - const { university, department, role, confirmedAt } = affiliationOptions + const { + university, + department, + role, + confirmedAt, + entitlement + } = affiliationOptions makeAffiliationRequest( { method: 'POST', path: `/api/v2/users/${userId.toString()}/affiliations`, - body: { email, university, department, role, confirmedAt }, + body: { email, university, department, role, confirmedAt, entitlement }, defaultErrorMessage: "Couldn't create affiliation" }, function(error, body) { diff --git a/services/web/app/src/Features/Institutions/InstitutionsController.js b/services/web/app/src/Features/Institutions/InstitutionsController.js index 815432ab16..dfe8e3a4b5 100644 --- a/services/web/app/src/Features/Institutions/InstitutionsController.js +++ b/services/web/app/src/Features/Institutions/InstitutionsController.js @@ -23,10 +23,7 @@ var affiliateUsers = function(hostname, callback) { .split('') .reverse() .join('') - UserGetter.getUsersByHostname(hostname, { _id: 1, emails: 1 }, function( - error, - users - ) { + UserGetter.getUsersByHostname(hostname, { _id: 1 }, function(error, users) { if (error) { OError.tag(error, 'problem fetching users by hostname') return callback(error) @@ -35,8 +32,13 @@ var affiliateUsers = function(hostname, callback) { async.mapLimit( users, ASYNC_AFFILIATIONS_LIMIT, - (user, innerCallback) => - affiliateUserByReversedHostname(user, reversedHostname, innerCallback), + (user, innerCallback) => { + UserGetter.getUserFullEmails(user._id, (error, emails) => { + if (error) return innerCallback(error) + user.emails = emails + affiliateUserByReversedHostname(user, reversedHostname, innerCallback) + }) + }, callback ) }) @@ -52,11 +54,15 @@ var affiliateUserByReversedHostname = function( ) async.mapSeries( matchingEmails, - (email, innerCallback) => + (email, innerCallback) => { addAffiliation( user._id, email.email, - { confirmedAt: email.confirmedAt }, + { + confirmedAt: email.confirmedAt, + entitlement: + email.samlIdentifier && email.samlIdentifier.hasEntitlement + }, error => { if (error) { OError.tag( @@ -67,7 +73,8 @@ var affiliateUserByReversedHostname = function( } FeaturesUpdater.refreshFeatures(user._id, innerCallback) } - ), + ) + }, callback ) } diff --git a/services/web/test/unit/src/Institutions/InstitutionsAPITests.js b/services/web/test/unit/src/Institutions/InstitutionsAPITests.js index d90c794eb1..0a98498857 100644 --- a/services/web/test/unit/src/Institutions/InstitutionsAPITests.js +++ b/services/web/test/unit/src/Institutions/InstitutionsAPITests.js @@ -190,7 +190,8 @@ describe('InstitutionsAPI', function() { university: { id: 1 }, role: 'Prof', department: 'Math', - confirmedAt: new Date() + confirmedAt: new Date(), + entitlement: true } return this.InstitutionsAPI.addAffiliation( this.stubbedUser._id, @@ -207,12 +208,13 @@ describe('InstitutionsAPI', function() { requestOptions.method.should.equal('POST') const { body } = requestOptions - Object.keys(body).length.should.equal(5) + Object.keys(body).length.should.equal(6) body.email.should.equal(this.newEmail) body.university.should.equal(affiliationOptions.university) body.department.should.equal(affiliationOptions.department) body.role.should.equal(affiliationOptions.role) body.confirmedAt.should.equal(affiliationOptions.confirmedAt) + body.entitlement.should.equal(affiliationOptions.entitlement) this.markAsReadIpMatcher.calledOnce.should.equal(true) return done() } diff --git a/services/web/test/unit/src/Institutions/InstitutionsControllerTests.js b/services/web/test/unit/src/Institutions/InstitutionsControllerTests.js index a0cdbd04d6..42e29a276f 100644 --- a/services/web/test/unit/src/Institutions/InstitutionsControllerTests.js +++ b/services/web/test/unit/src/Institutions/InstitutionsControllerTests.js @@ -38,18 +38,48 @@ describe('InstitutionsController', function() { { email: 'another@mit.edu', reversedHostname: this.host } ] } + this.stubbedUser1DecoratedEmails = [ + { + email: 'stubb1@mit.edu', + reversedHostname: this.host, + samlIdentifier: { hasEntitlement: false } + }, + { email: 'test@test.com', reversedHostname: 'test.com' }, + { + email: 'another@mit.edu', + reversedHostname: this.host, + samlIdentifier: { hasEntitlement: true } + } + ] this.stubbedUser2 = { _id: '3131232', name: 'test', email: 'hello2@world.com', emails: [{ email: 'subb2@mit.edu', reversedHostname: this.host }] } + this.stubbedUser2DecoratedEmails = [ + { + email: 'subb2@mit.edu', + reversedHostname: this.host + } + ] - this.getUsersByHostname = sinon - .stub() - .callsArgWith(2, null, [this.stubbedUser1, this.stubbedUser2]) + this.getUsersByHostname = sinon.stub().callsArgWith( + 2, + null, + [this.stubbedUser1, this.stubbedUser2].map(user => { + return { _id: user._id } + }) + ) this.addAffiliation = sinon.stub().callsArgWith(3, null) this.refreshFeatures = sinon.stub().yields(null) + this.getUserFullEmails = sinon.stub() + this.getUserFullEmails + .withArgs(this.stubbedUser1._id) + .yields(null, this.stubbedUser1DecoratedEmails) + this.getUserFullEmails + .withArgs(this.stubbedUser2._id) + .yields(null, this.stubbedUser2DecoratedEmails) this.InstitutionsController = SandboxedModule.require(modulePath, { globals: { console: console @@ -57,7 +87,8 @@ describe('InstitutionsController', function() { requires: { 'logger-sharelatex': this.logger, '../User/UserGetter': { - getUsersByHostname: this.getUsersByHostname + getUsersByHostname: this.getUsersByHostname, + getUserFullEmails: this.getUserFullEmails }, '../Institutions/InstitutionsAPI': { addAffiliation: this.addAffiliation @@ -84,13 +115,25 @@ describe('InstitutionsController', function() { this.getUsersByHostname.calledOnce.should.equal(true) this.addAffiliation.calledThrice.should.equal(true) this.addAffiliation - .calledWith(this.stubbedUser1._id, this.stubbedUser1.emails[0].email) + .calledWithMatch( + this.stubbedUser1._id, + this.stubbedUser1.emails[0].email, + { entitlement: false } + ) .should.equal(true) this.addAffiliation - .calledWith(this.stubbedUser1._id, this.stubbedUser1.emails[2].email) + .calledWithMatch( + this.stubbedUser1._id, + this.stubbedUser1.emails[2].email, + { entitlement: true } + ) .should.equal(true) this.addAffiliation - .calledWith(this.stubbedUser2._id, this.stubbedUser2.emails[0].email) + .calledWithMatch( + this.stubbedUser2._id, + this.stubbedUser2.emails[0].email, + { entitlement: undefined } + ) .should.equal(true) this.refreshFeatures .calledWith(this.stubbedUser1._id) @@ -100,6 +143,7 @@ describe('InstitutionsController', function() { .should.equal(true) return done() } + this.next.callsFake(done) return this.InstitutionsController.confirmDomain( this.req, this.res,