From 98cd68d085aacc6a4ff63802cc825a27412b94d4 Mon Sep 17 00:00:00 2001 From: June Kelly Date: Fri, 17 Mar 2023 09:49:51 +0000 Subject: [PATCH] Merge pull request #12261 from overleaf/jk-alter-password-similarity [web] Alter password-similarity check/metric GitOrigin-RevId: e9a55b4a86d2b69d6f34c1e2339d32321e08341d --- .../Authentication/AuthenticationManager.js | 9 ++-- .../AuthenticationManagerTests.js | 47 +++++++++++++++---- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/services/web/app/src/Features/Authentication/AuthenticationManager.js b/services/web/app/src/Features/Authentication/AuthenticationManager.js index bf5dff47df..39053a6d14 100644 --- a/services/web/app/src/Features/Authentication/AuthenticationManager.js +++ b/services/web/app/src/Features/Authentication/AuthenticationManager.js @@ -239,8 +239,9 @@ const AuthenticationManager = { // 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 + password.includes(email) || + password.includes(startOfEmail) || + email.includes(password) ) { return new InvalidPasswordError({ message: 'password contains part of email address', @@ -389,7 +390,9 @@ const AuthenticationManager = { _validatePasswordNotTooSimilar(password, email) { password = password.toLowerCase() email = email.toLowerCase() - const stringsToCheck = [email].concat(email.split(/\W+/)) + const stringsToCheck = [email] + .concat(email.split(/\W+/)) + .concat(email.split(/@/)) let largestSimilarity = 0 let err = null for (const emailPart of stringsToCheck) { diff --git a/services/web/test/unit/src/Authentication/AuthenticationManagerTests.js b/services/web/test/unit/src/Authentication/AuthenticationManagerTests.js index dd56d493f9..f9669ebdef 100644 --- a/services/web/test/unit/src/Authentication/AuthenticationManagerTests.js +++ b/services/web/test/unit/src/Authentication/AuthenticationManagerTests.js @@ -747,7 +747,7 @@ describe('AuthenticationManager', function () { }) it('should return an error when the password is too similar to email', function () { - const password = 'someuser1234' + const password = '12someuser34' const email = 'someuser@example.com' const error = this.AuthenticationManager._validatePasswordNotTooSimilar( password, @@ -757,13 +757,25 @@ describe('AuthenticationManager', function () { }) it('should return an error when the password is re-arranged elements of the email', function () { - const password = 'su2oe1em3re' - const email = 'someuser@example.com' - const error = this.AuthenticationManager._validatePasswordNotTooSimilar( - password, - email - ) - expect(error).to.exist + const badPasswords = [ + 'su2oe1em3oolc', + 'someone.cool', + 'someonecool', + 'cool.someone', + 'coolsomeone', + 'example.com', + 'examplecom', + 'com.example', + 'comexample', + ] + const email = 'someone.cool@example.com' + for (const password of badPasswords) { + const error = this.AuthenticationManager._validatePasswordNotTooSimilar( + password, + email + ) + expect(error).to.exist + } }) it('should send a metric with a rounded similarity score when password is too similar to email', function () { @@ -908,6 +920,23 @@ describe('AuthenticationManager', function () { }) }) + describe('email contains password', function () { + let user, password + beforeEach(function () { + password = 'somedomain' + user = { _id: 'some-user-id', email: 'someuser@somedomain.com' } + }) + + it('should reject the password', function (done) { + this.AuthenticationManager.setUserPassword(user, password, err => { + expect(err).to.exist + expect(err.name).to.equal('InvalidPasswordError') + expect(err?.info?.code).to.equal('contains_email') + done() + }) + }) + }) + describe('too short', function () { beforeEach(function () { this.settings.passwordStrengthOptions = { @@ -946,7 +975,7 @@ describe('AuthenticationManager', function () { describe('password too similar to email', function () { beforeEach(function () { this.user.email = 'foobarbazquux@example.com' - this.password = 'foobarbaz' + this.password = 'foo21barbaz' this.metrics.inc.reset() })