Merge pull request #6141 from overleaf/bg-update-basic-auth

[web] remove deprecated basic-auth-connect module

GitOrigin-RevId: b18435c98696858da70f3a715258c3c7a86c3b54

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

@@ -7,8 +7,8 @@ const Metrics = require('@overleaf/metrics')
 const logger = require('@overleaf/logger')
 const querystring = require('querystring')
 const Settings = require('@overleaf/settings')
-const basicAuth = require('basic-auth-connect')
-const crypto = require('crypto')
+const basicAuth = require('basic-auth')
+const tsscmp = require('tsscmp')
 const UserHandler = require('../User/UserHandler')
 const UserSessionsManager = require('../User/UserSessionsManager')
 const SessionStoreManager = require('../../infrastructure/SessionStoreManager')
@@ -29,6 +29,20 @@ function send401WithChallenge(res) {
   res.sendStatus(401)
 }
 
+function checkCredentials(userDetailsMap, user, password) {
+  const expectedPassword = userDetailsMap.get(user)
+  const userExists = userDetailsMap.has(user) && expectedPassword // user exists with a non-null password
+  const isValid = userExists && tsscmp(expectedPassword, password)
+  if (!isValid) {
+    logger.err({ user }, 'invalid login details')
+  }
+  Metrics.inc('security.http-auth.check-credentials', 1, {
+    path: userExists ? 'known-user' : 'unknown-user',
+    status: isValid ? 'pass' : 'fail',
+  })
+  return isValid
+}
+
 const AuthenticationController = {
   serializeUser(user, callback) {
     if (!user._id || !user.email) {
@@ -335,17 +349,20 @@ const AuthenticationController = {
   },
 
   requireBasicAuth: function (userDetails) {
-    return basicAuth(function (user, pass) {
-      const expectedPassword = userDetails[user]
-      const isValid =
-        expectedPassword &&
-        expectedPassword.length === pass.length &&
-        crypto.timingSafeEqual(Buffer.from(expectedPassword), Buffer.from(pass))
-      if (!isValid) {
-        logger.err({ user }, 'invalid login details')
+    const userDetailsMap = new Map(Object.entries(userDetails))
+    return function (req, res, next) {
+      const credentials = basicAuth(req)
+      if (
+        !credentials ||
+        !checkCredentials(userDetailsMap, credentials.name, credentials.pass)
+      ) {
+        send401WithChallenge(res)
+        Metrics.inc('security.http-auth', 1, { status: 'reject' })
+      } else {
+        Metrics.inc('security.http-auth', 1, { status: 'accept' })
+        next()
       }
-      return isValid
-    })
+    }
   },
 
   requirePrivateApiAuth() {

services/web/package-lock.json

@@ -14084,11 +14084,6 @@
         }
       }
     },
-    "basic-auth-connect": {
-      "version": "1.0.0",
-      "resolved": "https://registry.npmjs.org/basic-auth-connect/-/basic-auth-connect-1.0.0.tgz",
-      "integrity": "sha512-kiV+/DTgVro4aZifY/hwRwALBISViL5NP4aReaR2EVJEObpbUBHIkdJh/YpcoEiYt7nBodZ6U2ajZeZvSxUCCg=="
-    },
     "batch": {
       "version": "0.6.1",
       "resolved": "https://registry.npmjs.org/batch/-/batch-0.6.1.tgz",
@@ -21109,7 +21104,7 @@
     "find-up": {
       "version": "2.1.0",
       "resolved": "https://registry.npmjs.org/find-up/-/find-up-2.1.0.tgz",
-      "integrity": "sha1-RdG35QbHF93UgndaK3eSCjwMV6c=",
+      "integrity": "sha512-NWzkk0jSJtTt08+FBFMvXoeZnOJD+jTtsRmBYbAIzJdX6l7dLgR7CTubCM5/eDdPUBvLCeVasP1brfVR/9/EZQ==",
       "dev": true,
       "requires": {
         "locate-path": "^2.0.0"
@@ -26182,7 +26177,7 @@
     "locate-path": {
       "version": "2.0.0",
       "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-2.0.0.tgz",
-      "integrity": "sha1-K1aLJl7slExtnA3pw9u7ygNUzY4=",
+      "integrity": "sha512-NCI2kiDkyR7VeEKm27Kda/iQHyKJe1Bu0FlTbYp3CqJu+9IFe9bLyAjMxf5ZDDbEg+iMPzB5zYyUTSm8wVTKmA==",
       "dev": true,
       "requires": {
         "p-locate": "^2.0.0",
@@ -29624,7 +29619,7 @@
     "p-locate": {
       "version": "2.0.0",
       "resolved": "https://registry.npmjs.org/p-locate/-/p-locate-2.0.0.tgz",
-      "integrity": "sha1-IKAQOyIqcMj9OcwuWAaA893l7EM=",
+      "integrity": "sha512-nQja7m7gSKuewoVRen45CtVfODR3crN3goVQ0DDZ9N3yHxgpkuBhZqsaiotSQRrADUrne346peY7kT3TSACykg==",
       "dev": true,
       "requires": {
         "p-limit": "^1.1.0"
@@ -29642,7 +29637,7 @@
         "p-try": {
           "version": "1.0.0",
           "resolved": "https://registry.npmjs.org/p-try/-/p-try-1.0.0.tgz",
-          "integrity": "sha1-y8ec26+P1CKOE/Yh8rGiN8GyB7M=",
+          "integrity": "sha512-U1etNYuMJoIz3ZXSrrySFjsXQTWOx2/jdi86L+2pRvph/qMKL6sbcCYdH23fqsbm8TH2Gn0OybpT4eSFlCVHww==",
           "dev": true
         }
       }

services/web/package.json

@@ -91,7 +91,7 @@
     "archiver": "4.0.1",
     "async": "0.6.2",
     "backbone": "^1.3.3",
-    "basic-auth-connect": "^1.0.0",
+    "basic-auth": "^2.0.1",
     "bcrypt": "^5.0.0",
     "body-parser": "^1.19.0",
     "bootstrap": "^3.4.1",

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

@@ -644,8 +644,8 @@ describe('AuthenticationController', function () {
         this.req.headers = {
           authorization: `Basic ${Buffer.from('user:nope').toString('base64')}`,
         }
-        this.req.end = status => {
-          expect(status).to.equal('Unauthorized')
+        this.req.sendStatus = status => {
+          expect(status).to.equal(401)
           done()
         }
         this.middleware(this.req, this.req)
@@ -657,8 +657,8 @@ describe('AuthenticationController', function () {
             'base64'
           )}`,
         }
-        this.req.end = status => {
-          expect(status).to.equal('Unauthorized')
+        this.req.sendStatus = status => {
+          expect(status).to.equal(401)
           done()
         }
         this.middleware(this.req, this.req)
@@ -670,8 +670,45 @@ describe('AuthenticationController', function () {
             'base64'
           )}`,
         }
-        this.req.end = status => {
-          expect(status).to.equal('Unauthorized')
+        this.req.sendStatus = status => {
+          expect(status).to.equal(401)
+          done()
+        }
+        this.middleware(this.req, this.req)
+      })
+
+      it('should fail with empty user and password of "undefined"', function (done) {
+        this.req.headers = {
+          authorization: `Basic ${Buffer.from(`:undefined`).toString(
+            'base64'
+          )}`,
+        }
+        this.req.sendStatus = status => {
+          expect(status).to.equal(401)
+          done()
+        }
+        this.middleware(this.req, this.req)
+      })
+
+      it('should fail with empty user and empty password', function (done) {
+        this.req.headers = {
+          authorization: `Basic ${Buffer.from(`:`).toString('base64')}`,
+        }
+        this.req.sendStatus = status => {
+          expect(status).to.equal(401)
+          done()
+        }
+        this.middleware(this.req, this.req)
+      })
+
+      it('should fail with a user that is not a valid property', function (done) {
+        this.req.headers = {
+          authorization: `Basic ${Buffer.from(
+            `constructor:[Function ]`
+          ).toString('base64')}`,
+        }
+        this.req.sendStatus = status => {
+          expect(status).to.equal(401)
           done()
         }
         this.middleware(this.req, this.req)