diff --git a/src/externalldap.js b/src/externalldap.js index b8f527230..775fa335b 100644 --- a/src/externalldap.js +++ b/src/externalldap.js @@ -278,10 +278,10 @@ async function maybeCreateUser(identifier) { return await users.add(user.email, { username: user.username, password: null, displayName: user.displayName, source: 'ldap' }, AuditSource.EXTERNAL_LDAP); } -async function verifyPassword(username, password, totpToken) { +async function verifyPassword(username, password, options) { assert.strictEqual(typeof username, 'string'); assert.strictEqual(typeof password, 'string'); - assert(totpToken === null || typeof totpToken === 'string'); + assert.strictEqual(typeof options, 'object'); const config = await getConfig(); if (config.provider === 'noop') throw new BoxError(BoxError.BAD_STATE, 'not enabled'); @@ -293,10 +293,10 @@ async function verifyPassword(username, password, totpToken) { const client = await getClient(config, { bind: false }); let userAuthDn; - if (totpToken !== null) { + if (!options.skipTotpCheck) { // inject totptoken into first attribute. in ldap, '+' is the attribute separator in a RDNS const rdns = ldapUsers[0].dn.split(','); - userAuthDn = `${rdns[0]}+totptoken=${totpToken},` + rdns.slice(1).join(','); + userAuthDn = `${rdns[0]}+totptoken=${options.totpToken},` + rdns.slice(1).join(','); } else { userAuthDn = ldapUsers[0].dn; } diff --git a/src/oidc.js b/src/oidc.js index 1d5f618a1..f0061be9c 100644 --- a/src/oidc.js +++ b/src/oidc.js @@ -522,7 +522,7 @@ function interactionLogin(provider) { const verifyFunc = username.indexOf('@') === -1 ? users.verifyWithUsername : users.verifyWithEmail; - const [verifyError, user] = await safe(verifyFunc(username, password, users.AP_WEBADMIN, { totpToken })); + const [verifyError, user] = await safe(verifyFunc(username, password, users.AP_WEBADMIN, { totpToken, skipTotpCheck: false })); if (verifyError && verifyError.reason === BoxError.INVALID_CREDENTIALS) return next(new HttpError(401, verifyError.message)); if (verifyError && verifyError.reason === BoxError.NOT_FOUND) return next(new HttpError(401, 'Username and password does not match')); if (verifyError) return next(new HttpError(500, verifyError)); diff --git a/src/proxyauth.js b/src/proxyauth.js index d368e0ae8..b80a0f267 100644 --- a/src/proxyauth.js +++ b/src/proxyauth.js @@ -166,7 +166,7 @@ async function passwordAuth(req, res, next) { const { username, password, totpToken } = req.body; const verifyFunc = username.indexOf('@') !== -1 ? users.verifyWithEmail : users.verifyWithUsername; - const [error, user] = await safe(verifyFunc(username, password, appId, { totpToken })); + const [error, user] = await safe(verifyFunc(username, password, appId, { totpToken, skipTotpCheck: false })); if (error) return next(new HttpError(403, error.message)); req.user = user; diff --git a/src/test/externalldap-test.js b/src/test/externalldap-test.js index f7744c021..d0519c89a 100644 --- a/src/test/externalldap-test.js +++ b/src/test/externalldap-test.js @@ -331,39 +331,39 @@ describe('External LDAP', function () { }); it('fails for unknown user', async function () { - const [error] = await safe(externalLdap.verifyPassword('badusername', 'badpassword', null)); + const [error] = await safe(externalLdap.verifyPassword('badusername', 'badpassword', {})); expect(error.reason).to.be(BoxError.NOT_FOUND); }); it('fails for bad password', async function () { - const [error] = await safe(externalLdap.verifyPassword('maximus', 'badpassword', null)); + const [error] = await safe(externalLdap.verifyPassword('maximus', 'badpassword', {})); expect(error.reason).to.be(BoxError.INVALID_CREDENTIALS); }); it('succeeds for valid password', async function () { - const u = await externalLdap.verifyPassword('maximus', LDAP_SHARED_PASSWORD, null); + const u = await externalLdap.verifyPassword('maximus', LDAP_SHARED_PASSWORD, {}); expect(u.username).to.be('maximus'); }); it('enable totp', () => gLdapUsers[0].has2fa = true); - it('succeeds when totp required and no totp', async function () { - const u = await externalLdap.verifyPassword('maximus', LDAP_SHARED_PASSWORD, null); + it('succeeds when totp required and skip totp', async function () { + const u = await externalLdap.verifyPassword('maximus', LDAP_SHARED_PASSWORD, { skipTotpCheck: true }); expect(u.username).to.be('maximus'); }); it('fails when totp required and empty totp', async function () { - const [error] = await safe(externalLdap.verifyPassword('maximus', LDAP_SHARED_PASSWORD, '')); + const [error] = await safe(externalLdap.verifyPassword('maximus', LDAP_SHARED_PASSWORD, { skipTotpCheck: false, totpToken: '' })); expect(error.reason).to.be(BoxError.INVALID_CREDENTIALS); }); it('fails when totp required and bad totp', async function () { - const [error] = await safe(externalLdap.verifyPassword('maximus', LDAP_SHARED_PASSWORD, 'badtotp')); + const [error] = await safe(externalLdap.verifyPassword('maximus', LDAP_SHARED_PASSWORD, { skipTotpCheck: false, totpToken: 'badtotp' })); expect(error.reason).to.be(BoxError.INVALID_CREDENTIALS); }); it('succeeds when totp required and good totp', async function () { - const u = await externalLdap.verifyPassword('maximus', LDAP_SHARED_PASSWORD, LDAP_SHARED_TOTP_TOKEN); + const u = await externalLdap.verifyPassword('maximus', LDAP_SHARED_PASSWORD, { skipTotpCheck: false, totpToken: LDAP_SHARED_TOTP_TOKEN }); expect(u.username).to.be('maximus'); }); }); diff --git a/src/users.js b/src/users.js index bf31a9187..b456fe9f5 100644 --- a/src/users.js +++ b/src/users.js @@ -360,11 +360,8 @@ async function verify(userId, password, identifier, options) { return user; } - const skipTotpCheck = !!options.skipTotpCheck; - const totpToken = options.totpToken || null; - if (user.source === 'ldap') { - await externalLdap.verifyPassword(user.username, password, totpToken); + await externalLdap.verifyPassword(user.username, password, options); } else { const saltBinary = Buffer.from(user.salt, 'hex'); const [error, derivedKey] = await safe(pbkdf2Async(password, saltBinary, CRYPTO_ITERATIONS, CRYPTO_KEY_LENGTH, CRYPTO_DIGEST)); @@ -373,9 +370,9 @@ async function verify(userId, password, identifier, options) { const derivedKeyHex = Buffer.from(derivedKey, 'binary').toString('hex'); if (derivedKeyHex !== user.password) throw new BoxError(BoxError.INVALID_CREDENTIALS, 'Username and password does not match'); - if (!skipTotpCheck && user.twoFactorAuthenticationEnabled) { - if (!totpToken) throw new BoxError(BoxError.INVALID_CREDENTIALS, 'A totpToken must be provided'); - const verified = speakeasy.totp.verify({ secret: user.twoFactorAuthenticationSecret, encoding: 'base32', token: totpToken, window: 2 }); + if (!options.skipTotpCheck && user.twoFactorAuthenticationEnabled) { + if (!options.totpToken) throw new BoxError(BoxError.INVALID_CREDENTIALS, 'A totpToken must be provided'); + const verified = speakeasy.totp.verify({ secret: user.twoFactorAuthenticationSecret, encoding: 'base32', token: options.totpToken, window: 2 }); if (!verified) throw new BoxError(BoxError.INVALID_CREDENTIALS, 'Invalid totpToken'); } }