diff --git a/services/web/app/src/Features/User/UserGetter.js b/services/web/app/src/Features/User/UserGetter.js index 4325bb4ea5..4bf19ec58b 100644 --- a/services/web/app/src/Features/User/UserGetter.js +++ b/services/web/app/src/Features/User/UserGetter.js @@ -2,7 +2,6 @@ const { callbackify } = require('util') const { db } = require('../../infrastructure/mongodb') const moment = require('moment') const settings = require('@overleaf/settings') -const { promisifyAll } = require('@overleaf/promise-utils') const { promises: InstitutionsAPIPromises, } = require('../Institutions/InstitutionsAPI') @@ -163,141 +162,113 @@ async function getWritefullData(userId) { } } +async function getUser(query, projection = {}) { + query = normalizeQuery(query) + return await db.users.findOne(query, { projection }) +} + +async function getUserEmail(userId) { + const user = await UserGetter.promises.getUser(userId, { email: 1 }) + return user && user.email +} + +async function getUserByMainEmail(email, projection = {}) { + email = email.trim() + return await db.users.findOne({ email }, { projection }) +} + +async function getUserByAnyEmail(email, projection = {}) { + email = email.trim() + + // $exists: true MUST be set to use the partial index + const query = { emails: { $exists: true }, 'emails.email': email } + const user = await db.users.findOne(query, { projection }) + if (user) return user + + // While multiple emails are being rolled out, check for the main email as + // well + return await getUserByMainEmail(email, projection) +} + +async function getUsersByAnyConfirmedEmail(emails, projection = {}) { + const query = { + 'emails.email': { $in: emails }, // use the index on emails.email + emails: { + $exists: true, + $elemMatch: { + email: { $in: emails }, + confirmedAt: { $exists: true }, + }, + }, + } + + return await db.users.find(query, { projection }).toArray() +} + +async function getUsersByV1Ids(v1Ids, projection = {}) { + const query = { 'overleaf.id': { $in: v1Ids } } + return await db.users.find(query, { projection }).toArray() +} + +async function getUsersByHostname(hostname, projection) { + const reversedHostname = hostname.trim().split('').reverse().join('') + const query = { + emails: { $exists: true }, + 'emails.reversedHostname': reversedHostname, + } + return await db.users.find(query, { projection }).toArray() +} + +async function getInstitutionUsersByHostname(hostname) { + const projection = { + _id: 1, + email: 1, + emails: 1, + samlIdentifiers: 1, + } + + const users = await UserGetter.getUsersByHostname(hostname, projection) + users.forEach(user => { + user.emails = decorateFullEmails( + user.email, + user.emails, + [], + user.samlIdentifiers || [] + ) + }) + return users +} + +async function getUsers(query, projection) { + query = normalizeMultiQuery(query) + if (query?._id?.$in?.length === 0) return [] // shortcut for getUsers([]) + return await db.users.find(query, { projection }).toArray() +} + +// check for duplicate email address. This is also enforced at the DB level +async function ensureUniqueEmailAddress(newEmail) { + const user = await UserGetter.promises.getUserByAnyEmail(newEmail) + if (user) { + throw new Errors.EmailExistsError() + } +} + const UserGetter = { getSsoUsersAtInstitution: callbackify(getSsoUsersAtInstitution), - - getUser(query, projection, callback) { - if (arguments.length === 2) { - callback = projection - projection = {} - } - try { - query = normalizeQuery(query) - db.users.findOne(query, { projection }, callback) - } catch (err) { - callback(err) - } - }, - + getUser: callbackify(getUser), getUserFeatures: callbackify(getUserFeatures), - - getUserEmail(userId, callback) { - this.getUser(userId, { email: 1 }, (error, user) => - callback(error, user && user.email) - ) - }, - + getUserEmail: callbackify(getUserEmail), getUserFullEmails: callbackify(getUserFullEmails), - getUserConfirmedEmails: callbackify(getUserConfirmedEmails), - - getUserByMainEmail(email, projection, callback) { - email = email.trim() - if (arguments.length === 2) { - callback = projection - projection = {} - } - db.users.findOne({ email }, { projection }, callback) - }, - - getUserByAnyEmail(email, projection, callback) { - email = email.trim() - if (arguments.length === 2) { - callback = projection - projection = {} - } - // $exists: true MUST be set to use the partial index - const query = { emails: { $exists: true }, 'emails.email': email } - db.users.findOne(query, { projection }, (error, user) => { - if (error || user) { - return callback(error, user) - } - - // While multiple emails are being rolled out, check for the main email as - // well - this.getUserByMainEmail(email, projection, callback) - }) - }, - - getUsersByAnyConfirmedEmail(emails, projection, callback) { - if (arguments.length === 2) { - callback = projection - projection = {} - } - - const query = { - 'emails.email': { $in: emails }, // use the index on emails.email - emails: { - $exists: true, - $elemMatch: { - email: { $in: emails }, - confirmedAt: { $exists: true }, - }, - }, - } - - db.users.find(query, { projection }).toArray(callback) - }, - - getUsersByV1Ids(v1Ids, projection, callback) { - if (arguments.length === 2) { - callback = projection - projection = {} - } - const query = { 'overleaf.id': { $in: v1Ids } } - db.users.find(query, { projection }).toArray(callback) - }, - - getUsersByHostname(hostname, projection, callback) { - const reversedHostname = hostname.trim().split('').reverse().join('') - const query = { - emails: { $exists: true }, - 'emails.reversedHostname': reversedHostname, - } - db.users.find(query, { projection }).toArray(callback) - }, - - getInstitutionUsersByHostname(hostname, callback) { - const projection = { - _id: 1, - email: 1, - emails: 1, - samlIdentifiers: 1, - } - UserGetter.getUsersByHostname(hostname, projection, (err, users) => { - if (err) return callback(err) - - users.forEach(user => { - user.emails = decorateFullEmails( - user.email, - user.emails, - [], - user.samlIdentifiers || [] - ) - }) - callback(null, users) - }) - }, - - getUsers(query, projection, callback) { - try { - query = normalizeMultiQuery(query) - if (query?._id?.$in?.length === 0) return callback(null, []) // shortcut for getUsers([]) - db.users.find(query, { projection }).toArray(callback) - } catch (err) { - callback(err) - } - }, - + getUserByMainEmail: callbackify(getUserByMainEmail), + getUserByAnyEmail: callbackify(getUserByAnyEmail), + getUsersByAnyConfirmedEmail: callbackify(getUsersByAnyConfirmedEmail), + getUsersByV1Ids: callbackify(getUsersByV1Ids), + getUsersByHostname: callbackify(getUsersByHostname), + getInstitutionUsersByHostname: callbackify(getInstitutionUsersByHostname), + getUsers: callbackify(getUsers), // check for duplicate email address. This is also enforced at the DB level - ensureUniqueEmailAddress(newEmail, callback) { - this.getUserByAnyEmail(newEmail, function (error, user) { - if (user) { - return callback(new Errors.EmailExistsError()) - } - callback(error) - }) - }, + ensureUniqueEmailAddress: callbackify(ensureUniqueEmailAddress), getWritefullData: callbackify(getWritefullData), } @@ -378,17 +349,20 @@ const decorateFullEmails = ( } UserGetter.promises = { - ...promisifyAll(UserGetter, { - without: [ - 'getSsoUsersAtInstitution', - 'getUserFullEmails', - 'getUserFeatures', - 'getWritefullData', - ], - }), - getUserFullEmails, getSsoUsersAtInstitution, + getUser, getUserFeatures, + getUserEmail, + getUserFullEmails, + getUserConfirmedEmails, + getUserByMainEmail, + getUserByAnyEmail, + getUsersByAnyConfirmedEmail, + getUsersByV1Ids, + getUsersByHostname, + getInstitutionUsersByHostname, + getUsers, + ensureUniqueEmailAddress, getWritefullData, } diff --git a/services/web/test/unit/src/User/UserGetterTests.js b/services/web/test/unit/src/User/UserGetterTests.js index 4fc2e46621..5119d1d807 100644 --- a/services/web/test/unit/src/User/UserGetterTests.js +++ b/services/web/test/unit/src/User/UserGetterTests.js @@ -31,8 +31,8 @@ describe('UserGetter', function () { { email: 'email2@foo.bar', reversedHostname: 'rab.oof' }, ], } - this.findOne = sinon.stub().callsArgWith(2, null, this.fakeUser) - this.findToArrayStub = sinon.stub().yields(null, [this.fakeUser]) + this.findOne = sinon.stub().resolves(this.fakeUser) + this.findToArrayStub = sinon.stub().resolves([this.fakeUser]) this.find = sinon.stub().returns({ toArray: this.findToArrayStub }) this.Mongo = { db: { @@ -79,118 +79,94 @@ describe('UserGetter', function () { }) describe('getSsoUsersAtInstitution', function () { - it('should throw an error when no projection is passed', function (done) { - this.UserGetter.getSsoUsersAtInstitution(1, undefined, error => { - expect(error).to.exist - expect(error.message).to.equal('missing projection') - done() - }) + it('should throw an error when no projection is passed', async function () { + await expect( + this.UserGetter.promises.getSsoUsersAtInstitution(1, undefined) + ).to.be.rejectedWith('missing projection') }) }) describe('getUser', function () { - it('should get user', function (done) { + it('should get user', async function () { const query = { _id: '000000000000000000000000' } const projection = { email: 1 } - this.UserGetter.getUser(query, projection, (error, user) => { - expect(error).to.not.exist - this.findOne.called.should.equal(true) - this.findOne.calledWith(query, { projection }).should.equal(true) - user.should.deep.equal(this.fakeUser) - done() - }) + const user = await this.UserGetter.promises.getUser(query, projection) + this.findOne.called.should.equal(true) + this.findOne.calledWith(query, { projection }).should.equal(true) + expect(user).to.deep.equal(this.fakeUser) }) - it('should not allow null query', function (done) { - this.UserGetter.getUser(null, {}, error => { - error.should.exist - error.message.should.equal('no query provided') - done() - }) + it('should not allow null query', async function () { + await expect( + this.UserGetter.promises.getUser(null, {}) + ).to.be.rejectedWith('no query provided') }) }) describe('getUsers', function () { - it('should get users with array of userIds', function (done) { + it('should get users with array of userIds', async function () { const query = [new ObjectId()] const projection = { email: 1 } - this.UserGetter.getUsers(query, projection, (error, users) => { - expect(error).to.not.exist - this.find.should.have.been.calledWithMatch( - { _id: { $in: query } }, - { projection } - ) - users.should.deep.equal([this.fakeUser]) - done() - }) + const users = await this.UserGetter.promises.getUsers(query, projection) + this.find.should.have.been.calledWithMatch( + { _id: { $in: query } }, + { projection } + ) + users.should.deep.equal([this.fakeUser]) }) - it('should not call mongo with empty list', function (done) { + it('should not call mongo with empty list', async function () { const query = [] const projection = { email: 1 } - this.UserGetter.getUsers(query, projection, (error, users) => { - expect(error).to.not.exist - expect(users).to.deep.equal([]) - expect(this.find).to.not.have.been.called - done() - }) + const users = await this.UserGetter.promises.getUsers(query, projection) + expect(users).to.deep.equal([]) + expect(this.find).to.not.have.been.called }) - it('should not allow null query', function (done) { - this.UserGetter.getUser(null, {}, error => { - error.should.exist - error.message.should.equal('no query provided') - done() - }) + it('should not allow null query', async function () { + await expect( + this.UserGetter.promises.getUsers(null, {}) + ).to.be.rejectedWith('no query provided') }) }) describe('getUserFullEmails', function () { - it('should get user', function (done) { + it('should get user', async function () { this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser) const projection = { email: 1, emails: 1, samlIdentifiers: 1 } - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - this.UserGetter.promises.getUser.called.should.equal(true) - this.UserGetter.promises.getUser - .calledWith(this.fakeUser._id, projection) - .should.equal(true) - done() - } - ) + await this.UserGetter.promises.getUserFullEmails(this.fakeUser._id) + this.UserGetter.promises.getUser.called.should.equal(true) + this.UserGetter.promises.getUser + .calledWith(this.fakeUser._id, projection) + .should.equal(true) }) - it('should fetch emails data', function (done) { + it('should fetch emails data', async function () { this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - assert.deepEqual(fullEmails, [ - { - email: 'email1@foo.bar', - reversedHostname: 'rab.oof', - confirmedAt: this.fakeUser.emails[0].confirmedAt, - lastConfirmedAt: this.fakeUser.emails[0].lastConfirmedAt, - emailHasInstitutionLicence: false, - default: false, - }, - { - email: 'email2@foo.bar', - reversedHostname: 'rab.oof', - emailHasInstitutionLicence: false, - default: true, - lastConfirmedAt: null, - }, - ]) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + + assert.deepEqual(fullEmails, [ + { + email: 'email1@foo.bar', + reversedHostname: 'rab.oof', + confirmedAt: this.fakeUser.emails[0].confirmedAt, + lastConfirmedAt: this.fakeUser.emails[0].lastConfirmedAt, + emailHasInstitutionLicence: false, + default: false, + }, + { + email: 'email2@foo.bar', + reversedHostname: 'rab.oof', + emailHasInstitutionLicence: false, + default: true, + lastConfirmedAt: null, + }, + ]) }) - it('should merge affiliation data', function (done) { + it('should merge affiliation data', async function () { this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser) const affiliationsData = [ { @@ -213,86 +189,80 @@ describe('UserGetter', function () { }, ] this.getUserAffiliations.resolves(affiliationsData) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - assert.deepEqual(fullEmails, [ - { - email: 'email1@foo.bar', - reversedHostname: 'rab.oof', - confirmedAt: this.fakeUser.emails[0].confirmedAt, - lastConfirmedAt: this.fakeUser.emails[0].lastConfirmedAt, - default: false, - emailHasInstitutionLicence: true, - affiliation: { - institution: affiliationsData[0].institution, - inferred: affiliationsData[0].inferred, - department: affiliationsData[0].department, - role: affiliationsData[0].role, - lastDayToReconfirm: undefined, - licence: affiliationsData[0].licence, - inReconfirmNotificationPeriod: false, - cachedConfirmedAt: '2019-07-11T18:25:01.639Z', - cachedReconfirmedAt: '2021-07-11T18:25:01.639Z', - cachedEntitlement: false, - cachedLastDayToReconfirm: undefined, - cachedPastReconfirmDate: false, - pastReconfirmDate: false, - portal: undefined, - }, - }, - { - email: 'email2@foo.bar', - reversedHostname: 'rab.oof', - emailHasInstitutionLicence: false, - default: true, - lastConfirmedAt: null, - }, - ]) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + + assert.deepEqual(fullEmails, [ + { + email: 'email1@foo.bar', + reversedHostname: 'rab.oof', + confirmedAt: this.fakeUser.emails[0].confirmedAt, + lastConfirmedAt: this.fakeUser.emails[0].lastConfirmedAt, + default: false, + emailHasInstitutionLicence: true, + affiliation: { + institution: affiliationsData[0].institution, + inferred: affiliationsData[0].inferred, + department: affiliationsData[0].department, + role: affiliationsData[0].role, + lastDayToReconfirm: undefined, + licence: affiliationsData[0].licence, + inReconfirmNotificationPeriod: false, + cachedConfirmedAt: '2019-07-11T18:25:01.639Z', + cachedReconfirmedAt: '2021-07-11T18:25:01.639Z', + cachedEntitlement: false, + cachedLastDayToReconfirm: undefined, + cachedPastReconfirmDate: false, + pastReconfirmDate: false, + portal: undefined, + }, + }, + { + email: 'email2@foo.bar', + reversedHostname: 'rab.oof', + emailHasInstitutionLicence: false, + default: true, + lastConfirmedAt: null, + }, + ]) }) - it('should merge SAML identifier', function (done) { + it('should merge SAML identifier', async function () { const fakeSamlIdentifiers = [ - { providerId: 'saml_id', exteranlUserId: 'whatever' }, + { providerId: 'saml_id', externalUserId: 'whatever' }, ] const fakeUserWithSaml = this.fakeUser fakeUserWithSaml.emails[0].samlProviderId = 'saml_id' fakeUserWithSaml.samlIdentifiers = fakeSamlIdentifiers this.UserGetter.promises.getUser = sinon.stub().resolves(this.fakeUser) this.getUserAffiliations.resolves([]) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - assert.deepEqual(fullEmails, [ - { - email: 'email1@foo.bar', - reversedHostname: 'rab.oof', - confirmedAt: this.fakeUser.emails[0].confirmedAt, - lastConfirmedAt: this.fakeUser.emails[0].lastConfirmedAt, - default: false, - emailHasInstitutionLicence: false, - samlProviderId: 'saml_id', - samlIdentifier: fakeSamlIdentifiers[0], - }, - { - email: 'email2@foo.bar', - reversedHostname: 'rab.oof', - emailHasInstitutionLicence: false, - default: true, - lastConfirmedAt: null, - }, - ]) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + + assert.deepEqual(fullEmails, [ + { + email: 'email1@foo.bar', + reversedHostname: 'rab.oof', + confirmedAt: this.fakeUser.emails[0].confirmedAt, + lastConfirmedAt: this.fakeUser.emails[0].lastConfirmedAt, + default: false, + emailHasInstitutionLicence: false, + samlProviderId: 'saml_id', + samlIdentifier: fakeSamlIdentifiers[0], + }, + { + email: 'email2@foo.bar', + reversedHostname: 'rab.oof', + emailHasInstitutionLicence: false, + default: true, + lastConfirmedAt: null, + }, + ]) }) - it('should get user when it has no emails field', function (done) { + it('should get user when it has no emails field', async function () { this.fakeUserNoEmails = { _id: '12390i', email: 'email2@foo.bar', @@ -301,18 +271,14 @@ describe('UserGetter', function () { .stub() .resolves(this.fakeUserNoEmails) const projection = { email: 1, emails: 1, samlIdentifiers: 1 } - this.UserGetter.getUserFullEmails( - this.fakeUserNoEmails._id, - (error, fullEmails) => { - expect(error).to.not.exist - this.UserGetter.promises.getUser.called.should.equal(true) - this.UserGetter.promises.getUser - .calledWith(this.fakeUserNoEmails._id, projection) - .should.equal(true) - assert.deepEqual(fullEmails, []) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUserNoEmails._id ) + this.UserGetter.promises.getUser.called.should.equal(true) + this.UserGetter.promises.getUser + .calledWith(this.fakeUserNoEmails._id, projection) + .should.equal(true) + assert.deepEqual(fullEmails, []) }) describe('affiliation reconfirmation', function () { @@ -356,7 +322,7 @@ describe('UserGetter', function () { institution: institutionNonSSO, }, ] - it('should flag inReconfirmNotificationPeriod for all affiliations in period', function (done) { + it('should flag inReconfirmNotificationPeriod for all affiliations in period', async function () { const { maxConfirmationMonths } = institutionNonSSO const confirmed1 = moment() .subtract(maxConfirmationMonths + 2, 'months') @@ -392,21 +358,18 @@ describe('UserGetter', function () { affiliations[1].last_day_to_reconfirm = lastDayToReconfirm2 this.getUserAffiliations.resolves(affiliations) this.UserGetter.promises.getUser = sinon.stub().resolves(user) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - expect( - fullEmails[0].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - expect( - fullEmails[1].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + expect( + fullEmails[0].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) + expect( + fullEmails[1].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) }) - it('should not flag affiliations outside of notification period', function (done) { + + it('should not flag affiliations outside of notification period', async function () { const { maxConfirmationMonths } = institutionNonSSO const confirmed1 = new Date() const lastDayToReconfirm1 = moment(confirmed1) @@ -441,24 +404,20 @@ describe('UserGetter', function () { affiliations[1].last_day_to_reconfirm = lastDayToReconfirm2 this.getUserAffiliations.resolves(affiliations) this.UserGetter.promises.getUser = sinon.stub().resolves(user) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - expect( - fullEmails[0].affiliation.inReconfirmNotificationPeriod - ).to.equal(false) - expect( - fullEmails[1].affiliation.inReconfirmNotificationPeriod - ).to.equal(false) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + expect( + fullEmails[0].affiliation.inReconfirmNotificationPeriod + ).to.equal(false) + expect( + fullEmails[1].affiliation.inReconfirmNotificationPeriod + ).to.equal(false) }) }) describe('SSO institutions', function () { - it('should flag only linked email, if in notification period', function (done) { + it('should flag only linked email, if in notification period', async function () { const { maxConfirmationMonths } = institutionSSO const email1 = 'email1@sso.bar' const email2 = 'email2@sso.bar' @@ -530,27 +489,23 @@ describe('UserGetter', function () { ] this.getUserAffiliations.resolves(affiliations) this.UserGetter.promises.getUser = sinon.stub().resolves(user) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - expect( - fullEmails[0].affiliation.inReconfirmNotificationPeriod - ).to.equal(false) - expect( - fullEmails[1].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - expect( - fullEmails[2].affiliation.inReconfirmNotificationPeriod - ).to.equal(false) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + expect( + fullEmails[0].affiliation.inReconfirmNotificationPeriod + ).to.equal(false) + expect( + fullEmails[1].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) + expect( + fullEmails[2].affiliation.inReconfirmNotificationPeriod + ).to.equal(false) }) }) describe('multiple institution affiliations', function () { - it('should flag each institution', function (done) { + it('should flag each institution', async function () { const { maxConfirmationMonths } = institutionSSO const email1 = 'email1@sso.bar' const email2 = 'email2@sso.bar' @@ -640,30 +595,26 @@ describe('UserGetter', function () { this.getUserAffiliations.resolves(affiliationsData) this.UserGetter.promises.getUser = sinon.stub().resolves(user) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - expect( - fullEmails[0].affiliation.inReconfirmNotificationPeriod - ).to.to.equal(false) - expect( - fullEmails[1].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - expect( - fullEmails[2].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - expect( - fullEmails[3].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + expect( + fullEmails[0].affiliation.inReconfirmNotificationPeriod + ).to.to.equal(false) + expect( + fullEmails[1].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) + expect( + fullEmails[2].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) + expect( + fullEmails[3].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) }) }) describe('reconfirmedAt', function () { - it('only use confirmedAt when no reconfirmedAt', function (done) { + it('only use confirmedAt when no reconfirmedAt', async function () { const { maxConfirmationMonths } = institutionSSO const email1 = 'email1@foo.bar' const reconfirmed1 = moment().subtract( @@ -755,28 +706,24 @@ describe('UserGetter', function () { } this.getUserAffiliations.resolves(affiliationsData) this.UserGetter.promises.getUser = sinon.stub().resolves(user) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - expect( - fullEmails[0].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - expect( - fullEmails[1].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - expect( - fullEmails[2].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + expect( + fullEmails[0].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) + expect( + fullEmails[1].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) + expect( + fullEmails[2].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) }) }) describe('before reconfirmation period expires and within reconfirmation notification period', function () { const email = 'leonard@example-affiliation.com' - it('should flag the email', function (done) { + it('should flag the email', async function () { const { maxConfirmationMonths } = institutionNonSSO const confirmedAt = moment() .subtract(maxConfirmationMonths, 'months') @@ -808,21 +755,17 @@ describe('UserGetter', function () { } this.getUserAffiliations.resolves(affiliationsData) this.UserGetter.promises.getUser = sinon.stub().resolves(user) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - expect( - fullEmails[0].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + expect( + fullEmails[0].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) }) }) describe('when no Settings.reconfirmNotificationDays', function () { - it('should always return inReconfirmNotificationPeriod:false', function (done) { + it('should always return inReconfirmNotificationPeriod:false', async function () { const email1 = 'email1@sso.bar' const email2 = 'email2@foo.bar' const email3 = 'email3@foo.bar' @@ -874,26 +817,22 @@ describe('UserGetter', function () { this.settings.reconfirmNotificationDays = undefined this.getUserAffiliations.resolves(affiliationsData) this.UserGetter.promises.getUser = sinon.stub().resolves(user) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - expect( - fullEmails[0].affiliation.inReconfirmNotificationPeriod - ).to.to.equal(false) - expect( - fullEmails[1].affiliation.inReconfirmNotificationPeriod - ).to.equal(false) - expect( - fullEmails[2].affiliation.inReconfirmNotificationPeriod - ).to.equal(false) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + expect( + fullEmails[0].affiliation.inReconfirmNotificationPeriod + ).to.equal(false) + expect( + fullEmails[1].affiliation.inReconfirmNotificationPeriod + ).to.equal(false) + expect( + fullEmails[2].affiliation.inReconfirmNotificationPeriod + ).to.equal(false) }) }) - it('should flag to show notification if v1 shows as past reconfirmation but v2 does not', function (done) { + it('should flag to show notification if v1 shows as past reconfirmation but v2 does not', async function () { const email = 'abc123@test.com' const confirmedAt = new Date() const affiliationsData = [ @@ -918,19 +857,15 @@ describe('UserGetter', function () { } this.getUserAffiliations.resolves(affiliationsData) this.UserGetter.promises.getUser = sinon.stub().resolves(user) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - expect( - fullEmails[0].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + expect( + fullEmails[0].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) }) - it('should flag to show notification if v1 shows as reconfirmation upcoming but v2 does not', function (done) { + it('should flag to show notification if v1 shows as reconfirmation upcoming but v2 does not', async function () { const email = 'abc123@test.com' const { maxConfirmationMonths } = institutionNonSSO const affiliationsData = [ @@ -957,19 +892,15 @@ describe('UserGetter', function () { } this.getUserAffiliations.resolves(affiliationsData) this.UserGetter.promises.getUser = sinon.stub().resolves(user) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - expect( - fullEmails[0].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + expect( + fullEmails[0].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) }) - it('should flag to show notification if v2 shows as reconfirmation upcoming but v1 does not', function (done) { + it('should flag to show notification if v2 shows as reconfirmation upcoming but v1 does not', async function () { const email = 'abc123@test.com' const { maxConfirmationMonths } = institutionNonSSO @@ -1001,16 +932,12 @@ describe('UserGetter', function () { } this.getUserAffiliations.resolves(affiliationsData) this.UserGetter.promises.getUser = sinon.stub().resolves(user) - this.UserGetter.getUserFullEmails( - this.fakeUser._id, - (error, fullEmails) => { - expect(error).to.not.exist - expect( - fullEmails[0].affiliation.inReconfirmNotificationPeriod - ).to.equal(true) - done() - } + const fullEmails = await this.UserGetter.promises.getUserFullEmails( + this.fakeUser._id ) + expect( + fullEmails[0].affiliation.inReconfirmNotificationPeriod + ).to.equal(true) }) describe('cachedLastDayToReconfirm', function () { @@ -1212,171 +1139,130 @@ describe('UserGetter', function () { }) describe('getUserbyMainEmail', function () { - it('query user by main email', function (done) { + it('query user by main email', async function () { const email = 'hello@world.com' const projection = { emails: 1 } - this.UserGetter.getUserByMainEmail(email, projection, (error, user) => { - expect(error).to.not.exist - this.findOne.called.should.equal(true) - this.findOne.calledWith({ email }, { projection }).should.equal(true) - done() - }) + await this.UserGetter.promises.getUserByMainEmail(email, projection) + this.findOne.called.should.equal(true) + this.findOne.calledWith({ email }, { projection }).should.equal(true) }) - it('return user if found', function (done) { + it('return user if found', async function () { const email = 'hello@world.com' - this.UserGetter.getUserByMainEmail(email, (error, user) => { - expect(error).to.not.exist - user.should.deep.equal(this.fakeUser) - done() - }) + const user = await this.UserGetter.promises.getUserByMainEmail(email) + user.should.deep.equal(this.fakeUser) }) - it('trim email', function (done) { + it('trim email', async function () { const email = 'hello@world.com' - this.UserGetter.getUserByMainEmail(` ${email} `, (error, user) => { - expect(error).to.not.exist - this.findOne.called.should.equal(true) - this.findOne.calledWith({ email }).should.equal(true) - done() - }) + await this.UserGetter.promises.getUserByMainEmail(` ${email} `) + this.findOne.called.should.equal(true) + this.findOne.calledWith({ email }).should.equal(true) }) }) describe('getUserByAnyEmail', function () { - it('query user for any email', function (done) { + it('query user for any email', async function () { const email = 'hello@world.com' const expectedQuery = { emails: { $exists: true }, 'emails.email': email, } const projection = { emails: 1 } - this.UserGetter.getUserByAnyEmail( + const user = await this.UserGetter.promises.getUserByAnyEmail( ` ${email} `, - projection, - (error, user) => { - expect(error).to.not.exist - this.findOne - .calledWith(expectedQuery, { projection }) - .should.equal(true) - user.should.deep.equal(this.fakeUser) - done() - } + projection ) + this.findOne.calledWith(expectedQuery, { projection }).should.equal(true) + user.should.deep.equal(this.fakeUser) }) - it('query contains $exists:true so partial index is used', function (done) { + it('query contains $exists:true so partial index is used', async function () { const expectedQuery = { emails: { $exists: true }, 'emails.email': '', } - this.UserGetter.getUserByAnyEmail('', {}, (error, user) => { - expect(error).to.not.exist - this.findOne - .calledWith(expectedQuery, { projection: {} }) - .should.equal(true) - done() - }) + await this.UserGetter.promises.getUserByAnyEmail('', {}) + this.findOne + .calledWith(expectedQuery, { projection: {} }) + .should.equal(true) }) - it('checks main email as well', function (done) { - this.findOne.callsArgWith(2, null, null) + it('checks main email as well', async function () { + this.findOne.resolves(null) const email = 'hello@world.com' const projection = { emails: 1 } - this.UserGetter.getUserByAnyEmail( - ` ${email} `, - projection, - (error, user) => { - expect(error).to.not.exist - this.findOne.calledTwice.should.equal(true) - this.findOne.calledWith({ email }, { projection }).should.equal(true) - done() - } - ) + await this.UserGetter.promises.getUserByAnyEmail(` ${email} `, projection) + this.findOne.calledTwice.should.equal(true) + this.findOne.calledWith({ email }, { projection }).should.equal(true) }) }) describe('getUsersByHostname', function () { - it('should find user by hostname', function (done) { + it('should find user by hostname', async function () { const hostname = 'bar.foo' const expectedQuery = { emails: { $exists: true }, 'emails.reversedHostname': hostname.split('').reverse().join(''), } const projection = { emails: 1 } - this.UserGetter.getUsersByHostname( - hostname, - projection, - (error, users) => { - expect(error).to.not.exist - this.find.calledOnce.should.equal(true) - this.find.calledWith(expectedQuery, { projection }).should.equal(true) - done() - } - ) + await this.UserGetter.promises.getUsersByHostname(hostname, projection) + this.find.calledOnce.should.equal(true) + this.find.calledWith(expectedQuery, { projection }).should.equal(true) }) }) describe('getUsersByAnyConfirmedEmail', function () { - it('should find users by confirmed email', function (done) { + it('should find users by confirmed email', async function () { const emails = ['confirmed@example.com'] - this.UserGetter.getUsersByAnyConfirmedEmail(emails, (error, users) => { - expect(error).to.not.exist - expect(this.find).to.be.calledOnceWith( - { - 'emails.email': { $in: emails }, // use the index on emails.email - emails: { - $exists: true, - $elemMatch: { - email: { $in: emails }, - confirmedAt: { $exists: true }, - }, + await this.UserGetter.promises.getUsersByAnyConfirmedEmail(emails) + expect(this.find).to.be.calledOnceWith( + { + 'emails.email': { $in: emails }, // use the index on emails.email + emails: { + $exists: true, + $elemMatch: { + email: { $in: emails }, + confirmedAt: { $exists: true }, }, }, - { projection: {} } - ) - done() - }) + }, + { projection: {} } + ) }) }) describe('getUsersByV1Id', function () { - it('should find users by list of v1 ids', function (done) { + it('should find users by list of v1 ids', async function () { const v1Ids = [501] const expectedQuery = { 'overleaf.id': { $in: v1Ids }, } const projection = { emails: 1 } - this.UserGetter.getUsersByV1Ids(v1Ids, projection, (error, users) => { - expect(error).to.not.exist - this.find.calledOnce.should.equal(true) - this.find.calledWith(expectedQuery, { projection }).should.equal(true) - done() - }) + await this.UserGetter.promises.getUsersByV1Ids(v1Ids, projection) + this.find.calledOnce.should.equal(true) + this.find.calledWith(expectedQuery, { projection }).should.equal(true) }) }) describe('ensureUniqueEmailAddress', function () { beforeEach(function () { - this.UserGetter.getUserByAnyEmail = sinon.stub() + this.UserGetter.promises.getUserByAnyEmail = sinon.stub() }) - it('should return error if existing user is found', function (done) { - this.UserGetter.getUserByAnyEmail.callsArgWith(1, null, this.fakeUser) - this.UserGetter.ensureUniqueEmailAddress(this.newEmail, err => { - expect(err).to.exist - expect(err).to.be.an.instanceof(Errors.EmailExistsError) - done() - }) + it('should return error if existing user is found', async function () { + this.UserGetter.promises.getUserByAnyEmail.resolves(this.fakeUser) + await expect( + this.UserGetter.promises.ensureUniqueEmailAddress(this.newEmail) + ).to.be.rejectedWith(Errors.EmailExistsError) }) - it('should return null if no user is found', function (done) { - this.UserGetter.getUserByAnyEmail.callsArgWith(1) - this.UserGetter.ensureUniqueEmailAddress(this.newEmail, err => { - expect(err).not.to.exist - done() - }) + it('should return null if no user is found', async function () { + this.UserGetter.promises.getUserByAnyEmail.resolves(null) + await expect( + this.UserGetter.promises.ensureUniqueEmailAddress(this.newEmail) + ).to.be.fulfilled }) }) @@ -1386,13 +1272,12 @@ describe('UserGetter', function () { this.fakeUser.features = {} }) - it('should return user features', function (done) { + it('should return user features', async function () { this.fakeUser.features = { feature1: true, feature2: false } - this.UserGetter.getUserFeatures(new ObjectId(), (error, features) => { - expect(error).to.not.exist - expect(features).to.deep.equal(this.fakeUser.features) - done() - }) + const features = await this.UserGetter.promises.getUserFeatures( + new ObjectId() + ) + expect(features).to.deep.equal(this.fakeUser.features) }) it('should return user features when using promises', async function () {