From d45d886d5169cb97dd95253970805b8c32abf6da Mon Sep 17 00:00:00 2001
From: alxndrsn <alxndrsn>
Date: Mon, 28 Aug 2023 09:58:44 +0000
Subject: [PATCH 1/4] tests: always use real bcrypt lib

Instead of mocking bcrypt in some test scenarios, decrease the cost factor to
the fastest possible value.
---
 .circleci/config.yml                  |  2 +-
 .github/PULL_REQUEST_TEMPLATE.md      |  2 +-
 Makefile                              | 12 ++++--------
 lib/bin/run-server.js                 |  3 +--
 lib/http/preprocessors.js             |  5 +++--
 lib/model/query/users.js              |  5 +++--
 lib/resources/sessions.js             |  7 ++++---
 lib/resources/users.js                | 23 +++++++++++++++++++++--
 lib/task/task.js                      |  3 +--
 lib/util/crypto.js                    | 15 ++++++++++-----
 test/integration/fixtures/01-users.js |  5 +++--
 test/integration/setup.js             | 10 ++--------
 test/integration/task/account.js      | 13 +++++++------
 test/unit/http/preprocessors.js       |  9 ++++-----
 test/unit/util/crypto.js              | 18 +++++++++---------
 test/util/bcrypt-mock.js              |  5 -----
 16 files changed, 74 insertions(+), 63 deletions(-)
 delete mode 100644 test/util/bcrypt-mock.js

diff --git a/.circleci/config.yml b/.circleci/config.yml
index e7c7883bf..665c88edd 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -14,5 +14,5 @@ jobs:
       - run: make check-file-headers
       - run: npm ci --legacy-peer-deps
       - run: node lib/bin/create-docker-databases.js
-      - run: make test-full
+      - run: make test
 
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
index 231f088cd..b87ed0248 100644
--- a/.github/PULL_REQUEST_TEMPLATE.md
+++ b/.github/PULL_REQUEST_TEMPLATE.md
@@ -17,5 +17,5 @@ https://github.com/getodk/central-backend/blob/master/CONTRIBUTING.md
 
 #### Before submitting this PR, please make sure you have:
 
-- [ ] run `make test-full` and confirmed all checks still pass OR confirm CircleCI build passes
+- [ ] run `make test` and confirmed all checks still pass OR confirm CircleCI build passes
 - [ ] verified that any code from external sources are properly credited in comments or that everything is internally sourced
\ No newline at end of file
diff --git a/Makefile b/Makefile
index b1ca36002..e3665b108 100644
--- a/Makefile
+++ b/Makefile
@@ -60,11 +60,7 @@ debug: base
 
 .PHONY: test
 test: lint
-	BCRYPT=no npx mocha --recursive --exit
-
-.PHONY: test-full
-test-full: lint
-	npx mocha --recursive --exit
+	BCRYPT=insecure npx mocha --recursive --exit
 
 .PHONY: test-fast
 test-fast: node_version
@@ -72,11 +68,11 @@ test-fast: node_version
 
 .PHONY: test-integration
 test-integration: node_version
-	BCRYPT=no npx mocha --recursive test/integration --exit
+	BCRYPT=insecure npx mocha --recursive test/integration --exit
 
 .PHONY: test-unit
 test-unit: node_version
-	npx mocha --recursive test/unit --exit
+	BCRYPT=insecure npx mocha --recursive test/unit --exit
 
 .PHONY: test-coverage
 test-coverage: node_version
@@ -105,4 +101,4 @@ check-file-headers:
 .PHONY: api-docs
 api-docs:
 	(test "$(docker images -q odk-docs)" || docker build --file odk-docs.dockerfile -t odk-docs .) && \
-	docker run --rm -it -v ./docs/api.yaml:/docs/docs/_static/api-spec/central.yaml -p 8000:8000 odk-docs
\ No newline at end of file
+	docker run --rm -it -v ./docs/api.yaml:/docs/docs/_static/api-spec/central.yaml -p 8000:8000 odk-docs
diff --git a/lib/bin/run-server.js b/lib/bin/run-server.js
index 6d886692d..978730139 100644
--- a/lib/bin/run-server.js
+++ b/lib/bin/run-server.js
@@ -34,7 +34,6 @@ Error.stackTrackLimit = 20;
 
 // get an xlsform client and a password module.
 const xlsform = require('../external/xlsform').init(config.get('default.xlsform'));
-const bcrypt = require('../util/crypto').password(require('bcrypt'));
 
 // get an Enketo client
 const enketo = require('../external/enketo').init(config.get('default.enketo'));
@@ -45,7 +44,7 @@ const enketo = require('../external/enketo').init(config.get('default.enketo'));
 
 // initialize our container, then generate an http service out of it.
 const container = require('../model/container')
-  .withDefaults({ db, mail, env, Sentry, bcrypt, xlsform, enketo });
+  .withDefaults({ db, mail, env, Sentry, xlsform, enketo });
 const service = require('../http/service')(container);
 
 // insert the graceful exit middleware.
diff --git a/lib/http/preprocessors.js b/lib/http/preprocessors.js
index 0c00bbac2..4874c6e5a 100644
--- a/lib/http/preprocessors.js
+++ b/lib/http/preprocessors.js
@@ -7,6 +7,7 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
+const { verifyPassword } = require('../util/crypto');
 const { isBlank, isPresent, noop, without } = require('../util/util');
 const { isTrue } = require('../util/http');
 const oidc = require('../util/oidc');
@@ -27,7 +28,7 @@ const emptyAuthInjector = ({ Auth }, context) => context.with({ auth: Auth.by(nu
 // in conjunction with this function!
 //
 // TODO?: repetitive, but deduping it makes it even harder to understand.
-const authHandler = ({ Sessions, Users, Auth, bcrypt }, context) => {
+const authHandler = ({ Sessions, Users, Auth }, context) => {
   const authBySessionToken = (token, onFailure = noop) => Sessions.getByBearerToken(token)
     .then((session) => {
       if (!session.isDefined()) return onFailure();
@@ -87,7 +88,7 @@ const authHandler = ({ Sessions, Users, Auth, bcrypt }, context) => {
     // TODO: email existence timing attack on whether bcrypt runs or not.
     return Users.getByEmail(email)
       .then(getOrReject(Problem.user.authenticationFailed()))
-      .then((user) => bcrypt.verify(password, user.password)
+      .then((user) => verifyPassword(password, user.password)
         .then((verified) => {
           if (verified === true)
             return context.with({ auth: Auth.by(user.actor) });
diff --git a/lib/model/query/users.js b/lib/model/query/users.js
index 77a7bf7bf..e3c8fa43f 100644
--- a/lib/model/query/users.js
+++ b/lib/model/query/users.js
@@ -10,6 +10,7 @@
 const { sql } = require('slonik');
 const { map } = require('ramda');
 const { Actor, User } = require('../frames');
+const { hashPassword } = require('../../util/crypto');
 const { unjoiner, page, equals, QueryOptions } = require('../../util/db');
 const { reject } = require('../../util/promise');
 const Problem = require('../../util/problem');
@@ -32,10 +33,10 @@ const update = (user, data) => ({ run, one }) => {
 
 update.audit = (user, data) => (log) => log('user.update', user.actor, { data: data.with(data.actor) });
 
-const updatePassword = (user, cleartext) => ({ run, bcrypt }) =>
+const updatePassword = (user, cleartext) => ({ run }) =>
   (cleartext.length < 10
     ? reject(Problem.user.passwordTooShort())
-    : bcrypt.hash(cleartext))
+    : hashPassword(cleartext))
     .then((hash) => run(sql`update users set password=${hash} where "actorId"=${user.actor.id}`));
 updatePassword.audit = (user) => (log) => log('user.update', user.actor, { data: { password: true } });
 
diff --git a/lib/resources/sessions.js b/lib/resources/sessions.js
index 859ee563d..d60ce33f4 100644
--- a/lib/resources/sessions.js
+++ b/lib/resources/sessions.js
@@ -7,6 +7,7 @@
 // including this file, may be copied, modified, propagated, or distributed
 // except according to the terms contained in the LICENSE file.
 
+const { verifyPassword } = require('../util/crypto');
 const Problem = require('../util/problem');
 const { isBlank, noargs } = require('../util/util');
 const { getOrReject, rejectIf } = require('../util/promise');
@@ -17,9 +18,9 @@ const oidc = require('../util/oidc');
 module.exports = (service, endpoint) => {
 
   if (!oidc.isEnabled()) {
-    service.post('/sessions', endpoint(({ Audits, Users, Sessions, bcrypt }, { body, headers }) => {
+    service.post('/sessions', endpoint(({ Audits, Users, Sessions }, { body, headers }) => {
       // TODO if we're planning to offer multiple authN methods, we should be looking for
-      // any calls to bcrypt.verify(), and blocking them if that authN method is not
+      // any calls to verifyPassword(), and blocking them if that authN method is not
       // appropriate for the current user.
       //
       // It may be useful to re-use the sessions resources for other authN methods.
@@ -31,7 +32,7 @@ module.exports = (service, endpoint) => {
 
       return Users.getByEmail(email)
         .then(getOrReject(Problem.user.authenticationFailed()))
-        .then((user) => bcrypt.verify(password, user.password)
+        .then((user) => verifyPassword(password, user.password)
           .then(rejectIf(
             (verified) => (verified !== true),
             noargs(Problem.user.authenticationFailed)
diff --git a/lib/resources/users.js b/lib/resources/users.js
index 468ff3990..55f97f989 100644
--- a/lib/resources/users.js
+++ b/lib/resources/users.js
@@ -9,6 +9,7 @@
 
 const { always } = require('ramda');
 const { User } = require('../model/frames');
+const { verifyPassword } = require('../util/crypto');
 const { success, isTrue } = require('../util/http');
 const Option = require('../util/option');
 const Problem = require('../util/problem');
@@ -90,10 +91,10 @@ module.exports = (service, endpoint) => {
 
     // TODO: infosec debate around 404 vs 403 if insufficient privs but record DNE.
     // TODO: exact endpoint naming.
-    service.put('/users/:id/password', endpoint(async ({ Sessions, Users, mail, bcrypt }, { params, body, auth }) => {
+    service.put('/users/:id/password', endpoint(async ({ Sessions, Users, mail }, { params, body, auth }) => {
       const user = await Users.getByActorId(params.id).then(getOrNotFound);
       await auth.canOrReject('user.update', user.actor);
-      const verified = await bcrypt.verify(body.old, user.password);
+      const verified = await verifyPassword(body.old, user.password);
       if (verified !== true) return Problem.user.authenticationFailed();
       await Promise.all([
         Users.updatePassword(user, body.new),
@@ -152,6 +153,24 @@ module.exports = (service, endpoint) => {
             : resolve())
             .then(always(result)))))));
 
+  // TODO: ditto infosec debate.
+  // TODO: exact endpoint naming.
+  service.put('/users/:id/password', endpoint(async ({ Sessions, Users, mail }, { params, body, auth }) => {
+    const user = await Users.getByActorId(params.id).then(getOrNotFound);
+    await auth.canOrReject('user.update', user.actor);
+    const verified = await verifyPassword(body.old, user.password);
+    if (verified !== true) return Problem.user.authenticationFailed();
+    await Promise.all([
+      Users.updatePassword(user, body.new),
+      Sessions.terminateByActorId(
+        user.actorId,
+        auth.session.map(({ token }) => token).orNull()
+      )
+    ]);
+    await mail(user.email, 'accountPasswordChanged');
+    return success();
+  }));
+
   service.delete('/users/:id', endpoint(({ Actors, Users }, { params, auth }) =>
     Users.getByActorId(params.id)
       .then(getOrNotFound)
diff --git a/lib/task/task.js b/lib/task/task.js
index 3456b6091..8e1230e84 100644
--- a/lib/task/task.js
+++ b/lib/task/task.js
@@ -26,7 +26,6 @@ const { serialize } = require('../util/http');
 
 
 // initialize modules we need to put in the container:
-const bcrypt = require('../util/crypto').password(require('bcrypt'));
 const env = config.get('default.env');
 const { mailer } = require('../external/mail');
 const mail = mailer(mergeRight(config.get('default.email'), { env }));
@@ -47,7 +46,7 @@ const task = {
   // not thread-safe! but we don't have threads..
   withContainer: (taskdef) => (...args) => {
     const needsContainer = (task._container == null);
-    if (needsContainer) task._container = container.withDefaults({ db: slonikPool(config.get('default.database')), bcrypt, env, mail, task: true, odkAnalytics });
+    if (needsContainer) task._container = container.withDefaults({ db: slonikPool(config.get('default.database')), env, mail, task: true, odkAnalytics });
 
     const result = taskdef(task._container)(...args);
 
diff --git a/lib/util/crypto.js b/lib/util/crypto.js
index 577fc1991..a4a259c94 100644
--- a/lib/util/crypto.js
+++ b/lib/util/crypto.js
@@ -10,6 +10,7 @@
 // Various useful cryptography functions, for doing things like hashing passwords,
 // generating random tokens, and encrypting/decrypting data.
 
+const bcrypt = require('bcrypt');
 const digest = require('digest-stream');
 const { createHash, randomBytes, generateKeyPair, pbkdf2, createPrivateKey, createPublicKey, createCipheriv, createDecipheriv, publicEncrypt, privateDecrypt } = require('crypto');
 const { RSA_NO_PADDING } = require('crypto').constants;
@@ -25,13 +26,17 @@ const { isBlank } = require('./util');
 ////////////////////////////////////////////////////////////////////////////////
 // PASSWORD/AUTH UTIL
 
+// In tests, allow an insecure cost-factor for bcrypt.  In fact the minimum salt
+// is 4, but this is only documented in the bcrypt lib's issue tracker.
+// See: https://github.com/kelektiv/node.bcrypt.js/issues/870
+const BCRYPT_COST_FACTOR = process.env.BCRYPT === 'insecure' ? 1 : 12;
+
 // These functions call into bcrypt to hash or verify passwords.
-const hashPassword = (bcrypt) => (plain) =>
-  (isBlank(plain) ? resolve(null) : bcrypt.hash(plain, 12));
-const verifyPassword = (bcrypt) => (plain, hash) => ((isBlank(plain) || (isBlank(hash)))
+const hashPassword = (plain) =>
+  (isBlank(plain) ? resolve(null) : bcrypt.hash(plain, BCRYPT_COST_FACTOR));
+const verifyPassword = (plain, hash) => ((isBlank(plain) || (isBlank(hash)))
   ? resolve(false)
   : bcrypt.compare(plain, hash));
-const password = (bcrypt) => ({ hash: hashPassword(bcrypt), verify: verifyPassword(bcrypt) });
 
 // Returns a cryptographically random base64 string of a given length.
 const generateToken = (bytes = 48) => randomBytes(bytes).toString('base64')
@@ -302,7 +307,7 @@ const submissionDecryptor = (keys, passphrases) =>
 
 
 module.exports = {
-  hashPassword, verifyPassword, password, generateToken,
+  hashPassword, verifyPassword, generateToken,
   shasum, sha256sum, md5sum, digestWith,
   getPrivate, generateManagedKey, generateVersionSuffix,
   stripPemEnvelope,
diff --git a/test/integration/fixtures/01-users.js b/test/integration/fixtures/01-users.js
index 00729f313..7e8e345c2 100644
--- a/test/integration/fixtures/01-users.js
+++ b/test/integration/fixtures/01-users.js
@@ -1,8 +1,9 @@
 const appRoot = require('app-root-path');
 const { User, Actor, Project } = require(appRoot + '/lib/model/frames');
+const { hashPassword } = require(appRoot + '/lib/util/crypto');
 const { mapSequential } = require(appRoot + '/test/util/util');
 
-module.exports = ({ Assignments, Users, Projects, bcrypt }) => {
+module.exports = ({ Assignments, Users, Projects }) => {
   const proj = new Project({ name: 'Default Project' });
 
   const users = [
@@ -13,7 +14,7 @@ module.exports = ({ Assignments, Users, Projects, bcrypt }) => {
 
   // hash the passwords, create our three test users, then add grant Alice and Bob their rights.
   const withPasswords = Promise.all(users.map((user) =>
-    bcrypt.hash(user.password).then((password) => user.with({ password }))));
+    hashPassword(user.password).then((password) => user.with({ password }))));
 
   return Projects.create(proj)
     .then(() => withPasswords)
diff --git a/test/integration/setup.js b/test/integration/setup.js
index 49de57b2f..1d19e3678 100644
--- a/test/integration/setup.js
+++ b/test/integration/setup.js
@@ -33,12 +33,6 @@ const xlsform = require(appRoot + '/test/util/xlsform');
 // set up our sentry mock.
 const Sentry = require(appRoot + '/lib/external/sentry').init();
 
-// set up our bcrypt module; possibly mock or not based on params.
-const _bcrypt = (process.env.BCRYPT === 'no')
-  ? require('../util/bcrypt-mock')
-  : require('bcrypt');
-const bcrypt = require(appRoot + '/lib/util/crypto').password(_bcrypt);
-
 // set up our enketo mock.
 const { reset: resetEnketo, ...enketo } = require(appRoot + '/test/util/enketo');
 // Initialize the mock before other setup that uses the mock, then reset the
@@ -87,7 +81,7 @@ const initialize = async () => {
     await migrator.destroy();
   }
 
-  return withDefaults({ db, bcrypt, context, enketo, env }).transacting(populate);
+  return withDefaults({ db, context, enketo, env }).transacting(populate);
 };
 
 // eslint-disable-next-line func-names, space-before-function-paren
@@ -143,7 +137,7 @@ const augment = (service) => {
 // FINAL TEST WRAPPERS
 
 
-const baseContainer = withDefaults({ db, mail, env, xlsform, bcrypt, enketo, Sentry, odkAnalytics, context });
+const baseContainer = withDefaults({ db, mail, env, xlsform, enketo, Sentry, odkAnalytics, context });
 
 // called to get a service context per request. we do some work to hijack the
 // transaction system so that each test runs in a single transaction that then
diff --git a/test/integration/task/account.js b/test/integration/task/account.js
index 3ab383173..a2d1f8bc9 100644
--- a/test/integration/task/account.js
+++ b/test/integration/task/account.js
@@ -1,6 +1,7 @@
 const appRoot = require('app-root-path');
 const should = require('should');
 const { testTask } = require('../setup');
+const { verifyPassword } = require(appRoot + '/lib/util/crypto');
 const { getOrNotFound } = require(appRoot + '/lib/util/promise');
 const { createUser, promoteUser, setUserPassword } = require(appRoot + '/lib/task/account');
 const { User } = require(appRoot + '/lib/model/frames');
@@ -35,18 +36,18 @@ describe('task: accounts', () => {
           should(log.details.data.password).equal(null);
         })));
 
-    it('should set the password if given', testTask(({ Users, bcrypt }) =>
+    it('should set the password if given', testTask(({ Users }) =>
       createUser('testuser@getodk.org', 'aoeuidhtns')
         .then(() => Users.getByEmail('testuser@getodk.org'))
         .then(getOrNotFound)
-        .then((user) => bcrypt.verify('aoeuidhtns', user.password))
+        .then((user) => verifyPassword('aoeuidhtns', user.password))
         .then((verified) => verified.should.equal(true))));
 
-    it('should not verify a null password', testTask(({ Users, bcrypt }) =>
+    it('should not verify a null password', testTask(({ Users }) =>
       createUser('testuser@getodk.org', null)
         .then(() => Users.getByEmail('testuser@getodk.org'))
         .then(getOrNotFound)
-        .then((user) => bcrypt.verify(null, user.password))
+        .then((user) => verifyPassword(null, user.password))
         .then((verified) => verified.should.equal(false))));
 
     it('should complain if the password is too short', testTask(() =>
@@ -85,12 +86,12 @@ describe('task: accounts', () => {
   });
 
   describe('setUserPassword', () => {
-    it('should set a user password', testTask(({ Users, bcrypt }) =>
+    it('should set a user password', testTask(({ Users }) =>
       Users.create(User.fromApi({ email: 'testuser@getodk.org', displayName: 'test user' }))
         .then(() => setUserPassword('testuser@getodk.org', 'aoeuidhtns'))
         .then(() => Users.getByEmail('testuser@getodk.org'))
         .then(getOrNotFound)
-        .then((user) => bcrypt.verify('aoeuidhtns', user.password))
+        .then((user) => verifyPassword('aoeuidhtns', user.password))
         .then((verified) => verified.should.equal(true))));
 
     it('should complain about a password that is too short', testTask(({ Users }) =>
diff --git a/test/unit/http/preprocessors.js b/test/unit/http/preprocessors.js
index 6aabdff34..8df4a2fbf 100644
--- a/test/unit/http/preprocessors.js
+++ b/test/unit/http/preprocessors.js
@@ -6,11 +6,10 @@ const preprocessors = require(appRoot + '/lib/http/preprocessors');
 const { Context } = require(appRoot + '/lib/http/endpoint');
 const { Session, User, Actor } = require(appRoot + '/lib/model/frames');
 const { by } = require(appRoot + '/lib/model/query/auth');
+const { hashPassword } = require(appRoot + '/lib/util/crypto');
 const Problem = require(appRoot + '/lib/util/problem');
 const Option = require(appRoot + '/lib/util/option');
 
-const bcrypt = require(appRoot + '/lib/util/crypto').password(require('bcrypt'));
-
 describe('preprocessors', () => {
   // some mock helpers to simplify testing this module in isolation:
   const Auth = { by: (x) => by(x)({}) };
@@ -115,7 +114,7 @@ describe('preprocessors', () => {
 
       it('should fail the request if the Basic auth credentials are not right', () =>
         Promise.resolve(authHandler(
-          { Auth, Users: mockUsers('alice@getodk.org', 'willnevermatch'), bcrypt },
+          { Auth, Users: mockUsers('alice@getodk.org', 'willnevermatch') },
           new Context(
             createRequest({ headers: {
               Authorization: `Basic ${Buffer.from('alice@getodk.org:alice', 'utf8').toString('base64')}`,
@@ -126,9 +125,9 @@ describe('preprocessors', () => {
         )).should.be.rejectedWith(Problem, { problemCode: 401.2 }));
 
       it('should set the appropriate session if valid Basic auth credentials are given @slow', () =>
-        bcrypt.hash('alice').then((hashed) =>
+        hashPassword('alice').then((hashed) =>
           Promise.resolve(authHandler(
-            { Auth, Users: mockUsers('alice@getodk.org', hashed), bcrypt },
+            { Auth, Users: mockUsers('alice@getodk.org', hashed) },
             new Context(
               createRequest({ headers: {
                 Authorization: `Basic ${Buffer.from('alice@getodk.org:alice', 'utf8').toString('base64')}`,
diff --git a/test/unit/util/crypto.js b/test/unit/util/crypto.js
index 2a42baff2..5caf9809f 100644
--- a/test/unit/util/crypto.js
+++ b/test/unit/util/crypto.js
@@ -2,38 +2,38 @@ const appRoot = require('app-root-path');
 const { readFileSync } = require('fs');
 const should = require('should');
 const streamTest = require('streamtest').v2;
-const bcrypt = require('bcrypt');
 const crypto = require(appRoot + '/lib/util/crypto');
 
 describe('util/crypto', () => {
-  describe('hashPassword/verifyPassword @slow', () => {
-    const password = crypto.password(bcrypt);
+  describe('hashPassword/verifyPassword', () => {
+    const { hashPassword, verifyPassword } = crypto;
+
     // we do not actually verify the hashing itself, as:
     // 1. it is entirely performed by bcrypt, which has is own tests.
     // 2. bcrypt is intentionally slow, and we would like unit tests to be fast.
 
     it('should always return a Promise', () => {
-      password.hash('').should.be.a.Promise();
-      password.hash('password').should.be.a.Promise();
-      password.verify('password', 'hashhash').should.be.a.Promise();
+      hashPassword('').should.be.a.Promise();
+      hashPassword('password').should.be.a.Promise();
+      hashPassword('password', 'hashhash').should.be.a.Promise();
     });
 
     it('should return a Promise of null given a blank plaintext', (done) => {
-      password.hash('').then((result) => {
+      hashPassword('').then((result) => {
         should(result).equal(null);
         done();
       });
     });
 
     it('should not attempt to verify empty plaintext', (done) => {
-      password.verify('', '$2a$12$hCRUXz/7Hx2iKPLCduvrWugC5Q/j5e3bX9KvaYvaIvg/uvFYEpzSy').then((result) => {
+      verifyPassword('', '$2a$12$hCRUXz/7Hx2iKPLCduvrWugC5Q/j5e3bX9KvaYvaIvg/uvFYEpzSy').then((result) => {
         result.should.equal(false);
         done();
       });
     });
 
     it('should not attempt to verify empty hash', (done) => {
-      password.verify('password', '').then((result) => {
+      verifyPassword('password', '').then((result) => {
         result.should.equal(false);
         done();
       });
diff --git a/test/util/bcrypt-mock.js b/test/util/bcrypt-mock.js
deleted file mode 100644
index a6dc0c175..000000000
--- a/test/util/bcrypt-mock.js
+++ /dev/null
@@ -1,5 +0,0 @@
-module.exports = {
-  hash: (x) => Promise.resolve(x),
-  compare: (x, y) => Promise.resolve(x === y)
-};
-

From ec220a47c98b3b5c4baeb88a793f788ce7c2b766 Mon Sep 17 00:00:00 2001
From: alxndrsn <alxndrsn>
Date: Fri, 3 Nov 2023 12:40:10 +0000
Subject: [PATCH 2/4] gha: fix make target

---
 .github/workflows/standard-suite.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/standard-suite.yml b/.github/workflows/standard-suite.yml
index 4226943ef..35efdee01 100644
--- a/.github/workflows/standard-suite.yml
+++ b/.github/workflows/standard-suite.yml
@@ -29,4 +29,4 @@ jobs:
         cache: 'npm'
     - run: npm ci --legacy-peer-deps
     - run: node lib/bin/create-docker-databases.js
-    - run: make test-full
+    - run: make test

From 594b1f10daad00ee292b5e97cf02d8f04e7db7df Mon Sep 17 00:00:00 2001
From: alxndrsn <alxndrsn>
Date: Fri, 3 Nov 2023 12:45:34 +0000
Subject: [PATCH 3/4] lib/resources/users: remove accidental extra endpoint

---
 lib/resources/users.js | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/lib/resources/users.js b/lib/resources/users.js
index 55f97f989..91477effa 100644
--- a/lib/resources/users.js
+++ b/lib/resources/users.js
@@ -153,24 +153,6 @@ module.exports = (service, endpoint) => {
             : resolve())
             .then(always(result)))))));
 
-  // TODO: ditto infosec debate.
-  // TODO: exact endpoint naming.
-  service.put('/users/:id/password', endpoint(async ({ Sessions, Users, mail }, { params, body, auth }) => {
-    const user = await Users.getByActorId(params.id).then(getOrNotFound);
-    await auth.canOrReject('user.update', user.actor);
-    const verified = await verifyPassword(body.old, user.password);
-    if (verified !== true) return Problem.user.authenticationFailed();
-    await Promise.all([
-      Users.updatePassword(user, body.new),
-      Sessions.terminateByActorId(
-        user.actorId,
-        auth.session.map(({ token }) => token).orNull()
-      )
-    ]);
-    await mail(user.email, 'accountPasswordChanged');
-    return success();
-  }));
-
   service.delete('/users/:id', endpoint(({ Actors, Users }, { params, auth }) =>
     Users.getByActorId(params.id)
       .then(getOrNotFound)

From 9130f80a2613479406de6c20063fce1c2b8c8e90 Mon Sep 17 00:00:00 2001
From: alxndrsn <alxndrsn>
Date: Fri, 3 Nov 2023 12:46:36 +0000
Subject: [PATCH 4/4] Makefile: remove trailing newline

---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index e3665b108..c37a6f4b8 100644
--- a/Makefile
+++ b/Makefile
@@ -101,4 +101,4 @@ check-file-headers:
 .PHONY: api-docs
 api-docs:
 	(test "$(docker images -q odk-docs)" || docker build --file odk-docs.dockerfile -t odk-docs .) && \
-	docker run --rm -it -v ./docs/api.yaml:/docs/docs/_static/api-spec/central.yaml -p 8000:8000 odk-docs
+	docker run --rm -it -v ./docs/api.yaml:/docs/docs/_static/api-spec/central.yaml -p 8000:8000 odk-docs
\ No newline at end of file