diff --git a/services/web/app/src/Features/Institutions/InstitutionsAPI.js b/services/web/app/src/Features/Institutions/InstitutionsAPI.js index f6cea4fb96..8266d9e552 100644 --- a/services/web/app/src/Features/Institutions/InstitutionsAPI.js +++ b/services/web/app/src/Features/Institutions/InstitutionsAPI.js @@ -17,6 +17,24 @@ const settings = require('settings-sharelatex') const request = require('request') const { promisifyAll } = require('../../util/promises') const NotificationsBuilder = require('../Notifications/NotificationsBuilder') +const V1Api = require('../V1/V1Api') + +function getInstitutionViaDomain(domain) { + return new Promise(function(resolve, reject) { + V1Api.request( + { + timeout: 20 * 1000, + uri: `api/v1/sharelatex/university_saml?hostname=${domain}` + }, + function(error, response, body) { + if (error) { + reject(error) + } + resolve(body) + } + ) + }) +} const InstitutionsAPI = { getInstitutionAffiliations(institutionId, callback) { @@ -33,6 +51,8 @@ const InstitutionsAPI = { ) }, + getInstitutionViaDomain, + getInstitutionLicences(institutionId, startDate, endDate, lag, callback) { if (callback == null) { callback = function(error, body) {} diff --git a/services/web/app/src/Features/User/UserEmailsController.js b/services/web/app/src/Features/User/UserEmailsController.js index b6dea95519..7ede21051c 100644 --- a/services/web/app/src/Features/User/UserEmailsController.js +++ b/services/web/app/src/Features/User/UserEmailsController.js @@ -1,5 +1,7 @@ let UserEmailsController const AuthenticationController = require('../Authentication/AuthenticationController') +const Features = require('../../infrastructure/Features') +const InstitutionsAPI = require('../Institutions/InstitutionsAPI') const UserGetter = require('./UserGetter') const UserUpdater = require('./UserUpdater') const EmailHelper = require('../Helpers/EmailHelper') @@ -38,6 +40,52 @@ function add(req, res, next) { }) } +async function resendConfirmation(req, res, next) { + const userId = AuthenticationController.getLoggedInUserId(req) + const email = EmailHelper.parseEmail(req.body.email) + if (!email) { + return res.sendStatus(422) + } + UserGetter.getUserByAnyEmail(email, { _id: 1 }, async function(error, user) { + if (error) { + return next(error) + } + if (!user || user._id.toString() !== userId) { + logger.log( + { userId, email, foundUserId: user && user._id }, + "email doesn't match logged in user" + ) + return res.sendStatus(422) + } + if (Features.hasFeature('saml') || req.session.samlBeta) { + // institution SSO emails cannot be confirmed by email, + // confirmation happens by linking to the institution + let institution + try { + institution = await InstitutionsAPI.getInstitutionViaDomain( + email.split('@').pop() + ) + } catch (error) { + if (!(error instanceof Errors.NotFoundError)) { + throw error + } + } + if (institution && institution.sso_enabled) { + return res.sendStatus(422) + } + } + logger.log({ userId, email }, 'resending email confirmation token') + UserEmailsConfirmationHandler.sendConfirmationEmail(userId, email, function( + error + ) { + if (error) { + return next(error) + } + res.sendStatus(200) + }) + }) +} + module.exports = UserEmailsController = { list(req, res, next) { const userId = AuthenticationController.getLoggedInUserId(req) @@ -102,36 +150,7 @@ module.exports = UserEmailsController = { ) }, - resendConfirmation(req, res, next) { - const userId = AuthenticationController.getLoggedInUserId(req) - const email = EmailHelper.parseEmail(req.body.email) - if (!email) { - return res.sendStatus(422) - } - UserGetter.getUserByAnyEmail(email, { _id: 1 }, function(error, user) { - if (error) { - return next(error) - } - if (!user || user._id.toString() !== userId) { - logger.log( - { userId, email, foundUserId: user && user._id }, - "email doesn't match logged in user" - ) - return res.sendStatus(422) - } - logger.log({ userId, email }, 'resending email confirmation token') - UserEmailsConfirmationHandler.sendConfirmationEmail( - userId, - email, - function(error) { - if (error) { - return next(error) - } - res.sendStatus(200) - } - ) - }) - }, + resendConfirmation, showConfirm(req, res, next) { res.render('user/confirm_email', { diff --git a/services/web/app/views/project/list/notifications.pug b/services/web/app/views/project/list/notifications.pug index 6e77204cf7..2fd7db6920 100644 --- a/services/web/app/views/project/list/notifications.pug +++ b/services/web/app/views/project/list/notifications.pug @@ -55,7 +55,7 @@ span(ng-controller="NotificationsController").userNotifications ) li.notification_entry( ng-repeat="userEmail in userEmails", - ng-if="!userEmail.confirmedAt && !userEmail.hide" + ng-if="showConfirmEmail(userEmail)" ) .row .col-xs-12 diff --git a/services/web/public/src/main/project-list/notifications-controller.js b/services/web/public/src/main/project-list/notifications-controller.js index 9a13482b9f..0115de610c 100644 --- a/services/web/public/src/main/project-list/notifications-controller.js +++ b/services/web/public/src/main/project-list/notifications-controller.js @@ -99,6 +99,19 @@ define(['base'], function(App) { UserAffiliationsDataService ) { $scope.userEmails = [] + $scope.showConfirmEmail = email => { + if (!email.confirmedAt && !email.hide) { + if ( + email.affiliation && + email.affiliation.institution && + email.affiliation.institution.ssoEnabled && + (ExposedSettings.hasSamlBeta || ExposedSettings.hasSamlFeature) + ) { + return false + } + return true + } + } for (let userEmail of Array.from($scope.userEmails)) { userEmail.hide = false } diff --git a/services/web/test/unit/src/Institutions/InstitutionsAPITests.js b/services/web/test/unit/src/Institutions/InstitutionsAPITests.js index 3df2ad15c5..6c64995b81 100644 --- a/services/web/test/unit/src/Institutions/InstitutionsAPITests.js +++ b/services/web/test/unit/src/Institutions/InstitutionsAPITests.js @@ -43,6 +43,9 @@ describe('InstitutionsAPI', function() { request: this.request, '../Notifications/NotificationsBuilder': { ipMatcherAffiliation: sinon.stub().returns(this.ipMatcherNotification) + }, + '../../../../../app/src/Features/V1/V1Api': { + request: sinon.stub() } } }) diff --git a/services/web/test/unit/src/User/UserEmailsControllerTests.js b/services/web/test/unit/src/User/UserEmailsControllerTests.js index df3739d51b..4c205ae5f7 100644 --- a/services/web/test/unit/src/User/UserEmailsControllerTests.js +++ b/services/web/test/unit/src/User/UserEmailsControllerTests.js @@ -32,6 +32,9 @@ describe('UserEmailsController', function() { getLoggedInUserId: sinon.stub().returns(this.user._id), setInSessionUser: sinon.stub() } + this.Features = { + hasFeature: sinon.stub() + } this.UserUpdater = { addEmailAddress: sinon.stub(), removeEmailAddress: sinon.stub(), @@ -40,6 +43,15 @@ describe('UserEmailsController', function() { } this.EmailHelper = { parseEmail: sinon.stub() } this.endorseAffiliation = sinon.stub().yields() + this.InstitutionsAPI = { + endorseAffiliation: this.endorseAffiliation, + getInstitutionViaDomain: sinon + .stub() + .withArgs('overleaf.com') + .resolves({ sso_enabled: true }) + .withArgs('example.com') + .resolves({ sso_enabled: false }) + } return (this.UserEmailsController = SandboxedModule.require(modulePath, { globals: { console: console @@ -47,13 +59,12 @@ describe('UserEmailsController', function() { requires: { '../Authentication/AuthenticationController': this .AuthenticationController, + '../../infrastructure/Features': this.Features, './UserGetter': this.UserGetter, './UserUpdater': this.UserUpdater, '../Helpers/EmailHelper': this.EmailHelper, './UserEmailsConfirmationHandler': (this.UserEmailsConfirmationHandler = {}), - '../Institutions/InstitutionsAPI': { - endorseAffiliation: this.endorseAffiliation - }, + '../Institutions/InstitutionsAPI': this.InstitutionsAPI, '../Errors/Errors': Errors, 'logger-sharelatex': { log() { @@ -315,4 +326,88 @@ describe('UserEmailsController', function() { }) }) }) + describe('resendConfirmation', function() { + beforeEach(function() { + this.req = { + body: {} + } + this.res = { + sendStatus: sinon.stub() + } + this.next = sinon.stub() + this.UserEmailsConfirmationHandler.sendConfirmationEmail = sinon + .stub() + .yields() + }) + describe('when institution SSO is released', function() { + beforeEach(function() { + this.Features.hasFeature.withArgs('saml').returns(true) + }) + describe('for an institution SSO email', function() { + beforeEach(function() { + this.req.body.email = 'with-sso@overleaf.com' + }) + it('should not send the email', function() { + this.UserEmailsController.resendConfirmation( + this.req, + this.res, + () => { + this.UserEmailsConfirmationHandler.sendConfirmationEmail.should + .not.have.been.called.once + } + ) + }) + }) + describe('for a non-institution SSO email', function() { + beforeEach(function() { + this.req.body.email = 'without-sso@example.com' + }) + it('should send the email', function() { + this.UserEmailsController.resendConfirmation( + this.req, + this.res, + () => { + this.UserEmailsConfirmationHandler.sendConfirmationEmail.should + .have.been.called.once + } + ) + }) + }) + }) + describe('when institution SSO is not released', function() { + beforeEach(function() { + this.Features.hasFeature.withArgs('saml').returns(false) + }) + describe('for an institution SSO email', function() { + beforeEach(function() { + this.req.body.email = 'with-sso@overleaf.com' + }) + it('should send the email', function() { + this.UserEmailsController.resendConfirmation( + this.req, + this.res, + () => { + this.UserEmailsConfirmationHandler.sendConfirmationEmail.should + .have.been.called.once + } + ) + }) + }) + describe('for a non-institution SSO email', function() { + beforeEach(function() { + this.req.body.email = 'without-sso@example.com' + }) + it('should send the email', function() { + this.UserEmailsController.resendConfirmation( + this.req, + this.res, + () => { + this.UserEmailsConfirmationHandler.sendConfirmationEmail.should + .have.been.called.once + } + ) + }) + }) + }) + }) })