Merge pull request #4112 from overleaf/tm-private-api-basic-auth

Add requireBasicAuth middleware and refactor httpAuth to use it

GitOrigin-RevId: 7f68c0dc4a40102bfe4a97711def517e465ec7fd

services/web/app/src/Features/Analytics/AnalyticsRouter.js

@@ -16,7 +16,7 @@ module.exports = {
 
     publicApiRouter.use(
       '/analytics/uniExternalCollaboration',
-      AuthenticationController.httpAuth,
+      AuthenticationController.requirePrivateApiAuth(),
       AnalyticsProxy.call('/uniExternalCollaboration')
     )
   },

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

@@ -316,7 +316,7 @@ const AuthenticationController = {
     }
 
     if (req.headers.authorization != null) {
-      AuthenticationController.httpAuth(req, res, next)
+      AuthenticationController.requirePrivateApiAuth()(req, res, next)
     } else if (AuthenticationController.isUserLoggedIn(req)) {
       next()
     } else {
@@ -361,8 +361,9 @@ const AuthenticationController = {
     return next()
   },
 
-  httpAuth: basicAuth(function (user, pass) {
-    const expectedPassword = Settings.httpAuthUsers[user]
+  requireBasicAuth: function (userDetails) {
+    return basicAuth(function (user, pass) {
+      const expectedPassword = userDetails[user]
       const isValid =
         expectedPassword &&
         expectedPassword.length === pass.length &&
@@ -371,7 +372,12 @@ const AuthenticationController = {
         logger.err({ user, pass }, 'invalid login details')
       }
       return isValid
-  }),
+    })
+  },
+
+  requirePrivateApiAuth() {
+    return AuthenticationController.requireBasicAuth(Settings.httpAuthUsers)
+  },
 
   setRedirectInSession(req, value) {
     if (value == null) {

services/web/app/src/Features/Editor/EditorRouter.js

@@ -57,7 +57,7 @@ module.exports = {
     )
     apiRouter.post(
       '/project/:Project_id/doc/:entity_id/convert-to-file',
-      AuthenticationController.httpAuth,
+      AuthenticationController.requirePrivateApiAuth(),
       validate({
         body: Joi.object({
           userId: Joi.objectId().required(),
@@ -71,7 +71,7 @@ module.exports = {
     // whenever a user joins a project, like updating the deleted status.
     apiRouter.post(
       '/project/:Project_id/join',
-      AuthenticationController.httpAuth,
+      AuthenticationController.requirePrivateApiAuth(),
       RateLimiterMiddleware.rateLimit({
         endpointName: 'join-project',
         params: ['Project_id'],

services/web/app/src/Features/Subscription/SubscriptionRouter.js

@@ -136,7 +136,7 @@ module.exports = {
     // Currently used in acceptance tests only, as a way to trigger the syncing logic
     return publicApiRouter.post(
       '/user/:user_id/features/sync',
-      AuthenticationController.httpAuth,
+      AuthenticationController.requirePrivateApiAuth(),
       SubscriptionController.refreshUserFeatures
     )
   },

services/web/app/src/router.js

@@ -231,7 +231,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
   )
   privateApiRouter.get(
     '/user/:user_id/personal_info',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     UserInfoController.getPersonalInfo
   )
 
@@ -564,7 +564,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
   )
   privateApiRouter.post(
     '/project/:Project_id/history/resync',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     HistoryController.resyncProjectHistory
   )
 
@@ -639,28 +639,28 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
   )
   privateApiRouter.post(
     '/internal/expire-deleted-projects-after-duration',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     ProjectController.expireDeletedProjectsAfterDuration
   )
   privateApiRouter.post(
     '/internal/expire-deleted-users-after-duration',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     UserController.expireDeletedUsersAfterDuration
   )
   privateApiRouter.post(
     '/internal/project/:projectId/expire-deleted-project',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     ProjectController.expireDeletedProject
   )
   privateApiRouter.post(
     '/internal/users/:userId/expire',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     UserController.expireDeletedUser
   )
 
   privateApiRouter.get(
     '/user/:userId/tag',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     TagsController.apiGetAllTags
   )
   webRouter.get(
@@ -733,35 +733,35 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
   // Deprecated in favour of /internal/project/:project_id but still used by versioning
   privateApiRouter.get(
     '/project/:project_id/details',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     ProjectApiController.getProjectDetails
   )
 
   // New 'stable' /internal API end points
   privateApiRouter.get(
     '/internal/project/:project_id',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     ProjectApiController.getProjectDetails
   )
   privateApiRouter.get(
     '/internal/project/:Project_id/zip',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     ProjectDownloadsController.downloadProject
   )
   privateApiRouter.get(
     '/internal/project/:project_id/compile/pdf',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     CompileController.compileAndDownloadPdf
   )
 
   privateApiRouter.post(
     '/internal/deactivateOldProjects',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     InactiveProjectController.deactivateOldProjects
   )
   privateApiRouter.post(
     '/internal/project/:project_id/deactivate',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     InactiveProjectController.deactivateProject
   )
 
@@ -775,40 +775,40 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
       req.params = params
       next()
     },
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     CompileController.getFileFromClsi
   )
 
   privateApiRouter.get(
     '/project/:Project_id/doc/:doc_id',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     DocumentController.getDocument
   )
   privateApiRouter.post(
     '/project/:Project_id/doc/:doc_id',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     DocumentController.setDocument
   )
 
   privateApiRouter.post(
     '/user/:user_id/update/*',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     TpdsController.mergeUpdate
   )
   privateApiRouter.delete(
     '/user/:user_id/update/*',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     TpdsController.deleteUpdate
   )
 
   privateApiRouter.post(
     '/project/:project_id/contents/*',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     TpdsController.updateProjectContents
   )
   privateApiRouter.delete(
     '/project/:project_id/contents/*',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     TpdsController.deleteProjectContents
   )
 
@@ -884,7 +884,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
   // overleaf-integration module), but may expand beyond that role.
   publicApiRouter.post(
     '/api/clsi/compile/:submission_id',
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     CompileController.compileSubmission
   )
   publicApiRouter.get(
@@ -898,7 +898,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
       req.params = params
       next()
     },
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     CompileController.getFileFromClsiWithoutUser
   )
   publicApiRouter.post(
@@ -908,7 +908,7 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) {
       maxRequests: 1,
       timeInterval: 60,
     }),
-    AuthenticationController.httpAuth,
+    AuthenticationController.requirePrivateApiAuth(),
     InstitutionsController.confirmDomain
   )
 

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

@@ -630,7 +630,10 @@ describe('AuthenticationController', function () {
   describe('requireGlobalLogin', function () {
     beforeEach(function () {
       this.req.headers = {}
-      this.AuthenticationController.httpAuth = sinon.stub()
+      this.middleware = sinon.stub()
+      this.AuthenticationController.requirePrivateApiAuth = sinon
+        .stub()
+        .returns(this.middleware)
       this.setRedirect = sinon.spy(
         this.AuthenticationController,
         'setRedirectInSession'
@@ -684,8 +687,8 @@ describe('AuthenticationController', function () {
         )
       })
 
-      it('should pass the request onto httpAuth', function () {
-        this.AuthenticationController.httpAuth
+      it('should pass the request onto requirePrivateApiAuth middleware', function () {
+        this.middleware
           .calledWith(this.req, this.res, this.next)
           .should.equal(true)
       })
@@ -726,7 +729,16 @@ describe('AuthenticationController', function () {
     })
   })
 
-  describe('httpAuth', function () {
+  describe('requireBasicAuth', function () {
+    beforeEach(function () {
+      this.basicAuthUsers = {
+        'basic-auth-user': 'basic-auth-password',
+      }
+      this.middleware = this.AuthenticationController.requireBasicAuth(
+        this.basicAuthUsers
+      )
+    })
+
     describe('with http auth', function () {
       it('should error with incorrect user', function (done) {
         this.req.headers = {
@@ -736,12 +748,12 @@ describe('AuthenticationController', function () {
           expect(status).to.equal('Unauthorized')
           done()
         }
-        this.AuthenticationController.httpAuth(this.req, this.req)
+        this.middleware(this.req, this.req)
       })
 
       it('should error with incorrect password', function (done) {
         this.req.headers = {
-          authorization: `Basic ${Buffer.from('valid-test-user:nope').toString(
+          authorization: `Basic ${Buffer.from('basic-auth-user:nope').toString(
             'base64'
           )}`,
         }
@@ -749,12 +761,12 @@ describe('AuthenticationController', function () {
           expect(status).to.equal('Unauthorized')
           done()
         }
-        this.AuthenticationController.httpAuth(this.req, this.req)
+        this.middleware(this.req, this.req)
       })
 
       it('should fail with empty pass', function (done) {
         this.req.headers = {
-          authorization: `Basic ${Buffer.from(`invalid-test-user:`).toString(
+          authorization: `Basic ${Buffer.from(`basic-auth-user:`).toString(
             'base64'
           )}`,
         }
@@ -762,20 +774,33 @@ describe('AuthenticationController', function () {
           expect(status).to.equal('Unauthorized')
           done()
         }
-        this.AuthenticationController.httpAuth(this.req, this.req)
+        this.middleware(this.req, this.req)
       })
 
       it('should succeed with correct user/pass', function (done) {
         this.req.headers = {
           authorization: `Basic ${Buffer.from(
-            `valid-test-user:${this.httpAuthUsers['valid-test-user']}`
+            `basic-auth-user:${this.basicAuthUsers['basic-auth-user']}`
           ).toString('base64')}`,
         }
-        this.AuthenticationController.httpAuth(this.req, this.res, done)
+        this.middleware(this.req, this.res, done)
       })
     })
   })
 
+  describe('requirePrivateApiAuth', function () {
+    beforeEach(function () {
+      this.AuthenticationController.requireBasicAuth = sinon.stub()
+    })
+
+    it('should call requireBasicAuth with the private API user details', function () {
+      this.AuthenticationController.requirePrivateApiAuth()
+      this.AuthenticationController.requireBasicAuth
+        .calledWith(this.httpAuthUsers)
+        .should.equal(true)
+    })
+  })
+
   describe('_redirectToLoginOrRegisterPage', function () {
     beforeEach(function () {
       this.middleware = this.AuthenticationController.requireLogin(