diff --git a/backend/internal/token.js b/backend/internal/token.js index 67255b213..d3a178d4f 100644 --- a/backend/internal/token.js +++ b/backend/internal/token.js @@ -34,18 +34,20 @@ export default { .andWhere("is_disabled", 0) .first(); - if (!user) { - throw new errs.AuthError(ERROR_MESSAGE_INVALID_AUTH); - } + // An unknown, disabled or passwordless account goes through the same queries and the same password check + // as a real one, and fails with the same error: answering any sooner or any differently would tell anyone + // trying addresses which ones have an account. There is no user 0, so its lookup finds nothing. An empty + // secret, which creating a user through the API can store, is no hash: bcrypt would reject it at once. + const auth = await authModel + .query() + .where("user_id", "=", user?.id ?? 0) + .where("type", "=", "password") + .first(); - const auth = await authModel.query().where("user_id", "=", user.id).where("type", "=", "password").first(); - - if (!auth) { - throw new errs.AuthError(ERROR_MESSAGE_INVALID_AUTH); - } - - const valid = await auth.verifyPassword(data.secret); - if (!valid) { + const valid = auth?.secret + ? await auth.verifyPassword(data.secret) + : await authModel.verifyDummyPassword(data.secret); + if (!user || !valid) { throw new errs.AuthError(ERROR_MESSAGE_INVALID_AUTH, ERROR_MESSAGE_INVALID_AUTH_I18N); } diff --git a/backend/models/auth.js b/backend/models/auth.js index e8af582d5..c6228feeb 100644 --- a/backend/models/auth.js +++ b/backend/models/auth.js @@ -1,6 +1,7 @@ // Objection Docs: // http://vincit.github.io/objection.js/ +import crypto from "node:crypto"; import bcrypt from "bcrypt"; import { Model } from "objection"; import db from "../db.js"; @@ -11,10 +12,15 @@ import User from "./user.js"; Model.knex(db()); const boolFields = ["is_deleted"]; +const saltRounds = 13; + +// A hash of a random value, for a login that names an account with no password to check it against. Hashed +// with the same cost as real passwords, and made at startup so the first such login takes no longer either. +const dummyHash = bcrypt.hash(crypto.randomBytes(32).toString("hex"), saltRounds); function encryptPassword() { if (this.type === "password" && this.secret) { - return bcrypt.hash(this.secret, 13).then((hash) => { + return bcrypt.hash(this.secret, saltRounds).then((hash) => { this.secret = hash; }); } @@ -60,6 +66,18 @@ class Auth extends Model { return bcrypt.compare(password, this.secret); } + /** + * Take as long as verifyPassword does, then fail. bcrypt is nearly all of the time a login takes, so a login + * that skipped it for an unknown account would answer far sooner, and say which accounts exist. + * + * @param {String} password + * @returns {Promise} + */ + static async verifyDummyPassword(password) { + await bcrypt.compare(password, await dummyHash); + return false; + } + static get name() { return "Auth"; } diff --git a/test/cypress/e2e/api/Tokens.cy.js b/test/cypress/e2e/api/Tokens.cy.js new file mode 100644 index 000000000..c5bd55c41 --- /dev/null +++ b/test/cypress/e2e/api/Tokens.cy.js @@ -0,0 +1,61 @@ +/// + +describe('Tokens endpoints', () => { + const user = { + email: 'cypress@example.com', + secret: 'changeme', + }; + const disabledUser = { + email: 'disabled@example.com', + secret: 'changeme', + }; + const unknownEmail = 'nobody@example.com'; + + const login = (identity, secret) => { + return cy.request({ + method: 'POST', + url: '/api/tokens', + body: {identity, secret}, + failOnStatusCode: false, + }); + }; + + before(() => { + cy.resetUsers(); + cy.getToken().then((token) => { + cy.task('backendApiPost', { + token: token, + path: '/api/users', + data: { + name: 'Disabled User', + nickname: 'Disabled', + email: disabledUser.email, + is_disabled: true, + auth: { + type: 'password', + secret: disabledUser.secret, + }, + }, + }); + }); + }); + + it('Should give the same answer for an unknown email, a disabled account and a wrong password', () => { + // Only the error is compared: CI runs the backend in debug mode, which adds a stack trace that a real + // client never gets. + login(user.email, 'not-the-password').then((wrongPassword) => { + expect(wrongPassword.status).to.equal(400); + expect(wrongPassword.body.error.message_i18n).to.equal('error.invalid-auth'); + + login(unknownEmail, user.secret).then((unknown) => { + expect(unknown.status).to.equal(wrongPassword.status); + expect(unknown.body.error).to.deep.equal(wrongPassword.body.error); + }); + + login(disabledUser.email, disabledUser.secret).then((disabled) => { + expect(disabled.status).to.equal(wrongPassword.status); + expect(disabled.body.error).to.deep.equal(wrongPassword.body.error); + }); + }); + }); +});