Merge pull request #11943 from overleaf/jk-another-password-similarity-metric

[web] Add another metric for password similarity

GitOrigin-RevId: 6d44796a63f3be85bfee86056e03cfd3bb47066c

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

@@ -216,18 +216,15 @@ const AuthenticationManager = {
       })
     }
     if (typeof email === 'string' && email !== '') {
-      // TODO: remove this check once the password-too-similar check below is active
-      const startOfEmail = email.split('@')[0]
-      if (
-        password.indexOf(email) !== -1 ||
-        password.indexOf(startOfEmail) !== -1
-      ) {
-        return new InvalidPasswordError({
-          message: 'password contains part of email address',
-          info: { code: 'contains_email' },
-        })
-      }
       try {
+        const substringError =
+          AuthenticationManager._validatePasswordNotContainsEmailSubstrings(
+            password,
+            email
+          )
+        if (substringError) {
+          Metrics.inc('password-contains-substring-of-email')
+        }
         const passwordTooSimilarError =
           AuthenticationManager._validatePasswordNotTooSimilar(password, email)
         if (passwordTooSimilarError) {
@@ -239,6 +236,17 @@ const AuthenticationManager = {
           'error while checking password similarity to email'
         )
       }
+      // TODO: remove this check once the password-too-similar checks are active?
+      const startOfEmail = email.split('@')[0]
+      if (
+        password.indexOf(email) !== -1 ||
+        password.indexOf(startOfEmail) !== -1
+      ) {
+        return new InvalidPasswordError({
+          message: 'password contains part of email address',
+          info: { code: 'contains_email' },
+        })
+      }
     }
     return null
   },
@@ -406,6 +414,27 @@ const AuthenticationManager = {
     })
     return err
   },
+
+  _validatePasswordNotContainsEmailSubstrings(password, email) {
+    password = password.toLowerCase()
+    email = email.toLowerCase()
+    const chunkLength = 4
+
+    if (email.length < chunkLength) {
+      return
+    }
+
+    let chunk
+    for (let i = 0; i <= email.length - chunkLength; i++) {
+      chunk = email.slice(i, i + chunkLength)
+      if (password.indexOf(chunk) !== -1) {
+        return new InvalidPasswordError({
+          message: 'password contains part of email address',
+          info: { code: 'contains_email' },
+        })
+      }
+    }
+  },
 }
 
 AuthenticationManager.promises = {

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

@@ -683,6 +683,62 @@ describe('AuthenticationManager', function () {
     })
   })
 
+  describe('_validatePasswordNotContainsEmailSubstrings', function () {
+    it('should return nothing for a dissimilar password', function () {
+      const password = 'fublmqgaeohhvd8'
+      const email = 'someuser@example.com'
+      const error =
+        this.AuthenticationManager._validatePasswordNotContainsEmailSubstrings(
+          password,
+          email
+        )
+      expect(error).to.not.exist
+    })
+
+    it('should return an error for password that is same as email', function () {
+      const email = 'someuser@example.com'
+      const error =
+        this.AuthenticationManager._validatePasswordNotContainsEmailSubstrings(
+          email,
+          email
+        )
+      expect(error).to.exist
+    })
+
+    it('should return an error for a password with a substring of email', function () {
+      const password = 'cooluser1253'
+      const email = 'somecooluser@example.com'
+      const error =
+        this.AuthenticationManager._validatePasswordNotContainsEmailSubstrings(
+          password,
+          email
+        )
+      expect(error).to.exist
+    })
+
+    it('should return an error for a password with a substring of email, regardless of case', function () {
+      const password = 'coOLUSer1253'
+      const email = 'somecooluser@example.com'
+      const error =
+        this.AuthenticationManager._validatePasswordNotContainsEmailSubstrings(
+          password,
+          email
+        )
+      expect(error).to.exist
+    })
+
+    it('should return nothing for a password containing first two characters of email', function () {
+      const password = 'lmgaesopxzqg'
+      const email = 'someuser@example.com'
+      const error =
+        this.AuthenticationManager._validatePasswordNotContainsEmailSubstrings(
+          password,
+          email
+        )
+      expect(error).to.not.exist
+    })
+  })
+
   describe('_validatePasswordNotTooSimilar', function () {
     beforeEach(function () {
       this.metrics.inc.reset()
@@ -921,6 +977,30 @@ describe('AuthenticationManager', function () {
       })
     })
 
+    describe('password contains substring of email', function () {
+      beforeEach(function () {
+        this.user.email = 'somecooluser@example.com'
+        this.password = 'somecoolfhzxk'
+        this.metrics.inc.reset()
+      })
+
+      it('should send a metric when the password contains substring of the email', function (done) {
+        this.AuthenticationManager.setUserPassword(
+          this.user,
+          this.password,
+          err => {
+            expect(err).to.not.exist
+            expect(
+              this.metrics.inc.calledWith(
+                'password-contains-substring-of-email'
+              )
+            ).to.equal(true)
+            done()
+          }
+        )
+      })
+    })
+
     describe('successful password set attempt', function () {
       beforeEach(function () {
         this.metrics.inc.reset()
@@ -958,6 +1038,12 @@ describe('AuthenticationManager', function () {
         ).to.equal(false)
       })
 
+      it('should not send a metric for password-contains-substring-of-email', function () {
+        expect(
+          this.metrics.inc.calledWith('password-contains-substring-of-email')
+        ).to.equal(false)
+      })
+
       it('should call the callback', function () {
         this.callback.called.should.equal(true)
       })