Merge pull request #3094 from overleaf/sk-restrict-admin-flag

Check domain of emails on admin users

GitOrigin-RevId: 75de9cff30e3c628249fcd0ea3446a33d51d39b4

services/web/app/src/Features/Authentication/AuthenticationController.js

@@ -17,6 +17,7 @@ const UrlHelper = require('../Helpers/UrlHelper')
 const AsyncFormHelper = require('../Helpers/AsyncFormHelper')
 const SudoModeHandler = require('../SudoMode/SudoModeHandler')
 const _ = require('lodash')
+const OError = require('@overleaf/o-error')
 const {
   acceptsJson
 } = require('../../infrastructure/RequestContentTypeDetection')
@@ -319,6 +320,37 @@ const AuthenticationController = {
     }
   },
 
+  validateAdmin(req, res, next) {
+    const adminDomains = Settings.adminDomains
+    if (
+      !adminDomains ||
+      !(Array.isArray(adminDomains) && adminDomains.length)
+    ) {
+      return next()
+    }
+    const user = AuthenticationController.getSessionUser(req)
+    if (!(user && user.isAdmin)) {
+      return next()
+    }
+    const email = user.email
+    if (email == null) {
+      return next(
+        new OError('[ValidateAdmin] Admin user without email address', {
+          userId: user._id
+        })
+      )
+    }
+    if (!adminDomains.find(domain => email.endsWith(`@${domain}`))) {
+      return next(
+        new OError('[ValidateAdmin] Admin user with invalid email domain', {
+          email: email,
+          userId: user._id
+        })
+      )
+    }
+    return next()
+  },
+
   httpAuth: basicAuth(function(user, pass) {
     let expectedPassword = Settings.httpAuthUsers[user]
     const isValid =

services/web/app/src/infrastructure/Server.js

@@ -199,6 +199,8 @@ webRouter.use(function(req, res, next) {
   }
 })
 
+webRouter.use(AuthenticationController.validateAdmin)
+
 // add security headers using Helmet
 const noCacheMiddleware = require('nocache')()
 webRouter.use(function(req, res, next) {

services/web/config/settings.defaults.coffee

@@ -499,6 +499,7 @@ module.exports = settings =
 	appName: process.env['APP_NAME'] or "ShareLaTeX (Community Edition)"
 
 	adminEmail: process.env['ADMIN_EMAIL'] or "placeholder@example.com"
+	adminDomains: JSON.parse(process.env['ADMIN_DOMAINS'] or 'null')
 
 	salesEmail: process.env['SALES_EMAIL'] or "placeholder@example.com"
 

services/web/test/acceptance/config/settings.test.coffee

@@ -13,6 +13,7 @@ module.exports =
 	enableSubscriptions: true
 
 	httpAuthUsers: httpAuthUsers
+	adminDomains: ['example.com']
 
 	apis:
 		web:

services/web/test/acceptance/src/AdminEmailTests.js

@@ -0,0 +1,57 @@
+const { expect } = require('chai')
+const async = require('async')
+const User = require('./helpers/User')
+
+describe('AdminEmails', function() {
+  beforeEach(function(done) {
+    this.timeout(5000)
+    done()
+  })
+
+  describe('an admin with an invalid email address', function() {
+    before(function(done) {
+      this.badUser = new User({ email: 'alice@evil.com' })
+      async.series(
+        [
+          cb => this.badUser.ensureUserExists(cb),
+          cb => this.badUser.ensureAdmin(cb)
+        ],
+        done
+      )
+    })
+
+    it('should block the user', function(done) {
+      this.badUser.login(err => {
+        expect(err).to.not.exist
+        this.badUser.getProjectListPage((err, statusCode) => {
+          expect(err).to.exist
+          done()
+        })
+      })
+    })
+  })
+
+  describe('an admin with a valid email address', function() {
+    before(function(done) {
+      this.goodUser = new User({ email: 'alice@example.com' })
+      async.series(
+        [
+          cb => this.goodUser.ensureUserExists(cb),
+          cb => this.goodUser.ensureAdmin(cb)
+        ],
+        done
+      )
+    })
+
+    it('should not block the user', function(done) {
+      this.goodUser.login(err => {
+        expect(err).to.not.exist
+        this.goodUser.getProjectListPage((err, statusCode) => {
+          expect(err).to.not.exist
+          expect(statusCode).to.equal(200)
+          done()
+        })
+      })
+    })
+  })
+})

services/web/test/unit/src/Authentication/AuthenticationControllerTests.js

@@ -52,10 +52,10 @@ describe('AuthenticationController', function() {
           error: sinon.stub(),
           err: sinon.stub()
         }),
-        'settings-sharelatex': {
+        'settings-sharelatex': (this.Settings = {
           siteUrl: 'http://www.foo.bar',
           httpAuthUsers: this.httpAuthUsers
-        },
+        }),
         passport: (this.passport = {
           authenticate: sinon.stub().returns(sinon.stub())
         }),
@@ -109,6 +109,80 @@ describe('AuthenticationController', function() {
     tk.reset()
   })
 
+  describe('validateAdmin', function() {
+    beforeEach(function() {
+      this.Settings.adminDomains = ['good.example.com']
+      this.goodAdmin = {
+        email: 'alice@good.example.com',
+        isAdmin: true
+      }
+      this.badAdmin = {
+        email: 'beatrice@bad.example.com',
+        isAdmin: true
+      }
+      this.normalUser = {
+        email: 'claire@whatever.example.com',
+        isAdmin: false
+      }
+    })
+
+    it('should skip when adminDomains are not configured', function(done) {
+      this.Settings.adminDomains = []
+      this.AuthenticationController.getSessionUser = sinon
+        .stub()
+        .returns(this.normalUser)
+      this.AuthenticationController.validateAdmin(this.req, this.res, err => {
+        this.AuthenticationController.getSessionUser.called.should.equal(false)
+        expect(err).to.not.exist
+        done()
+      })
+    })
+
+    it('should skip non-admin user', function(done) {
+      this.AuthenticationController.getSessionUser = sinon
+        .stub()
+        .returns(this.normalUser)
+      this.AuthenticationController.validateAdmin(this.req, this.res, err => {
+        this.AuthenticationController.getSessionUser.called.should.equal(true)
+        expect(err).to.not.exist
+        done()
+      })
+    })
+
+    it('should permit an admin with the right doman', function(done) {
+      this.AuthenticationController.getSessionUser = sinon
+        .stub()
+        .returns(this.goodAdmin)
+      this.AuthenticationController.validateAdmin(this.req, this.res, err => {
+        this.AuthenticationController.getSessionUser.called.should.equal(true)
+        expect(err).to.not.exist
+        done()
+      })
+    })
+
+    it('should block an admin with a missing email', function(done) {
+      this.AuthenticationController.getSessionUser = sinon
+        .stub()
+        .returns({ isAdmin: true })
+      this.AuthenticationController.validateAdmin(this.req, this.res, err => {
+        this.AuthenticationController.getSessionUser.called.should.equal(true)
+        expect(err).to.exist
+        done()
+      })
+    })
+
+    it('should block an admin with a bad domain', function(done) {
+      this.AuthenticationController.getSessionUser = sinon
+        .stub()
+        .returns(this.badAdmin)
+      this.AuthenticationController.validateAdmin(this.req, this.res, err => {
+        this.AuthenticationController.getSessionUser.called.should.equal(true)
+        expect(err).to.exist
+        done()
+      })
+    })
+  })
+
   describe('isUserLoggedIn', function() {
     beforeEach(function() {
       this.stub = sinon.stub(this.AuthenticationController, 'getLoggedInUserId')