diff --git a/services/web/app/src/Features/Authentication/AuthenticationManager.js b/services/web/app/src/Features/Authentication/AuthenticationManager.js index 9195352313..1249ea7dcf 100644 --- a/services/web/app/src/Features/Authentication/AuthenticationManager.js +++ b/services/web/app/src/Features/Authentication/AuthenticationManager.js @@ -14,10 +14,21 @@ const util = require('util') const HaveIBeenPwned = require('./HaveIBeenPwned') const UserAuditLogHandler = require('../User/UserAuditLogHandler') const logger = require('@overleaf/logger') +const DiffHelper = require('../Helpers/DiffHelper') const Metrics = require('@overleaf/metrics') const BCRYPT_ROUNDS = Settings.security.bcryptRounds || 12 const BCRYPT_MINOR_VERSION = Settings.security.bcryptMinorVersion || 'a' +const MAX_SIMILARITY = 0.7 + +function _exceedsMaximumLengthRatio(password, maxSimilarity, value) { + const passwordLength = password.length + const lengthBoundSimilarity = (maxSimilarity / 2) * passwordLength + const valueLength = value.length + return ( + passwordLength >= 10 * valueLength && valueLength < lengthBoundSimilarity + ) +} const _checkWriteResult = function (result, callback) { // for MongoDB @@ -203,6 +214,7 @@ 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 || @@ -213,6 +225,18 @@ const AuthenticationManager = { info: { code: 'contains_email' }, }) } + try { + const passwordTooSimilarError = + AuthenticationManager._validatePasswordNotTooSimilar(password, email) + if (passwordTooSimilarError) { + Metrics.inc('password-too-similar-to-email') + } + } catch (error) { + logger.error( + { error }, + 'error while checking password similarity to email' + ) + } } return null }, @@ -342,6 +366,33 @@ const AuthenticationManager = { } return true }, + + /** + * Check if the password is similar to (parts of) the email address. + * For now, this merely sends a metric when the password and + * email address are deemed to be too similar to each other. + * Later we will reject passwords that fail this check. + * + * This logic was borrowed from the django project: + * https://github.com/django/django/blob/fa3afc5d86f1f040922cca2029d6a34301597a70/django/contrib/auth/password_validation.py#L159-L214 + */ + _validatePasswordNotTooSimilar(password, email) { + password = password.toLowerCase() + email = email.toLowerCase() + const stringsToCheck = [email].concat(email.split(/\W+/)) + for (const emailPart of stringsToCheck) { + if (!_exceedsMaximumLengthRatio(password, MAX_SIMILARITY, emailPart)) { + const similarity = DiffHelper.stringSimilarity(password, emailPart) + if (similarity > MAX_SIMILARITY) { + logger.warn( + { email, emailPart, similarity, maxSimilarity: MAX_SIMILARITY }, + 'Password too similar to email' + ) + return new Error('password is too similar to email') + } + } + } + }, } AuthenticationManager.promises = { diff --git a/services/web/app/src/Features/Helpers/DiffHelper.js b/services/web/app/src/Features/Helpers/DiffHelper.js new file mode 100644 index 0000000000..c2d1d73fd3 --- /dev/null +++ b/services/web/app/src/Features/Helpers/DiffHelper.js @@ -0,0 +1,55 @@ +const MAX_LENGTH = 254 + +function _calculateRatio(matches, length) { + if (length) { + const ratio = (2.0 * matches) / length + const rounded = Math.floor(ratio * 100) / 100 + return rounded + } + return 1.0 +} + +/** + * Ported from python's `difflib`: + * https://github.com/python/cpython/blob/0415cf895f96ae3f896f1f25f0c030a820845e13/Lib/difflib.py#L622-L649 + * + * Accepts two strings, `a` and `b`, and returns a float ratio + * corresponding (approximatey) to the overlap between the strings. + * Identical strings produce 1.0, completely different strings produce 0.0 + * */ +function stringSimilarity(a, b) { + if ( + typeof a !== 'string' || + typeof b !== 'string' || + a.length > MAX_LENGTH || + b.length > MAX_LENGTH + ) { + throw new Error('Invalid input to quickMatchRatio') + } + // Count how many times each character occurs in `b` + const fullBCount = {} + b.split('').forEach(e => { + fullBCount[e] = (fullBCount[e] || 0) + 1 + }) + // avail[x] is the number of times x appears in 'b' less the + // number of times we've seen it in 'a' so far ... kinda + const avail = {} + let matches = 0 + a.split('').forEach(e => { + let n = null + if (Object.hasOwn(avail, e)) { + n = avail[e] + } else { + n = fullBCount[e] || 0 + } + avail[e] = n - 1 + if (n > 0) { + matches = matches + 1 + } + }) + return _calculateRatio(matches, a.length + b.length) +} + +module.exports = { + stringSimilarity, +} diff --git a/services/web/test/unit/src/Authentication/AuthenticationManagerTests.js b/services/web/test/unit/src/Authentication/AuthenticationManagerTests.js index 34f30fd051..494414783a 100644 --- a/services/web/test/unit/src/Authentication/AuthenticationManagerTests.js +++ b/services/web/test/unit/src/Authentication/AuthenticationManagerTests.js @@ -672,6 +672,52 @@ describe('AuthenticationManager', function () { }) }) + describe('_validatePasswordNotTooSimilar', function () { + beforeEach(function () { + this.metrics.inc.reset() + }) + + it('should return an error when the password is too similar to email', function () { + const password = 'someuser1234' + const email = 'someuser@example.com' + const error = this.AuthenticationManager._validatePasswordNotTooSimilar( + password, + email + ) + expect(error).to.exist + }) + + 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 + }) + + it('should return nothing when the password different from email', function () { + const password = '58WyLvr' + const email = 'someuser@example.com' + const error = this.AuthenticationManager._validatePasswordNotTooSimilar( + password, + email + ) + expect(error).to.not.exist + }) + + it('should return nothing when the password is much longer than parts of the email', function () { + const password = new Array(30).fill('a').join('') + const email = 'a@cd.com' + const error = this.AuthenticationManager._validatePasswordNotTooSimilar( + password, + email + ) + expect(error).to.not.exist + }) + }) + describe('setUserPassword', function () { beforeEach(function () { this.user_id = ObjectId() @@ -813,8 +859,45 @@ describe('AuthenticationManager', function () { }) }) + describe('password too similar to email', function () { + beforeEach(function () { + this.user.email = 'foobarbazquux@example.com' + this.password = 'foobarbaz' + this.metrics.inc.reset() + }) + + it('should send a metric when the password is too similar to the email', function (done) { + this.AuthenticationManager.setUserPassword( + this.user, + this.password, + err => { + expect(err).to.not.exist + expect( + this.metrics.inc.calledWith('password-too-similar-to-email') + ).to.equal(true) + done() + } + ) + }) + + it('should send a metric when the password is too similar to the email, regardless of case', function (done) { + this.AuthenticationManager.setUserPassword( + this.user, + this.password.toUpperCase(), + err => { + expect(err).to.not.exist + expect( + this.metrics.inc.calledWith('password-too-similar-to-email') + ).to.equal(true) + done() + } + ) + }) + }) + describe('successful password set attempt', function () { beforeEach(function () { + this.metrics.inc.reset() this.UserGetter.getUser = sinon.stub().yields(null, { overleaf: null }) this.AuthenticationManager.setUserPassword( this.user, @@ -843,6 +926,12 @@ describe('AuthenticationManager', function () { this.bcrypt.hash.calledWith(this.password, this.salt).should.equal(true) }) + it('should not send a metric for password-too-similar-to-email', function () { + expect( + this.metrics.inc.calledWith('password-too-similar-to-email') + ).to.equal(false) + }) + it('should call the callback', function () { this.callback.called.should.equal(true) }) diff --git a/services/web/test/unit/src/HelperFiles/DiffHelperTests.js b/services/web/test/unit/src/HelperFiles/DiffHelperTests.js new file mode 100644 index 0000000000..518bffc534 --- /dev/null +++ b/services/web/test/unit/src/HelperFiles/DiffHelperTests.js @@ -0,0 +1,28 @@ +const { expect } = require('chai') +const { + stringSimilarity, +} = require('../../../../app/src/Features/Helpers/DiffHelper') + +describe('DiffHelper', function () { + describe('stringSimilarity', function () { + it('should have a ratio of 1 for identical strings', function () { + expect(stringSimilarity('abcdef', 'abcdef')).to.equal(1.0) + }) + + it('should have a ratio of 0 for completely different strings', function () { + expect(stringSimilarity('abcdef', 'qmglzxv')).to.equal(0.0) + }) + + it('should have a ratio of between 0 and 1 for strings that are similar', function () { + const ratio = stringSimilarity('abcdef', 'abcdef@zxvkp') + expect(ratio).to.equal(0.66) + }) + + it('should reject non-string inputs', function () { + expect(() => stringSimilarity(1, 'abc')).to.throw + expect(() => stringSimilarity('abc', 2)).to.throw + expect(() => stringSimilarity('abc', new Array(1000).fill('a').join(''))) + .to.throw + }) + }) +})