From 425d0e47b29abd0bd99b5f81158f2742132be73e Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Mon, 2 Sep 2024 15:57:24 +1200 Subject: [PATCH 01/19] install argon --- packages/auth/package.json | 1 + packages/auth/src/argon2.js | 22 ++++ yarn.lock | 200 ++++++++++++++++++++++++++++++++++++ 3 files changed, 223 insertions(+) create mode 100644 packages/auth/src/argon2.js diff --git a/packages/auth/package.json b/packages/auth/package.json index 583dc92b4c..5018f2a297 100644 --- a/packages/auth/package.json +++ b/packages/auth/package.json @@ -23,6 +23,7 @@ "test:coverage": "yarn test --coverage" }, "dependencies": { + "@node-rs/argon2": "^1.8.3", "@tupaia/server-utils": "workspace:*", "@tupaia/utils": "workspace:*", "jsonwebtoken": "^9.0.0", diff --git a/packages/auth/src/argon2.js b/packages/auth/src/argon2.js new file mode 100644 index 0000000000..1483827e35 --- /dev/null +++ b/packages/auth/src/argon2.js @@ -0,0 +1,22 @@ +/* + * Tupaia + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd + */ + +const argon2 = require('argon2'); + +/** + * Helper function to encrypt passwords using sha256 + */ +export function encryptPassword(password) { + return argon2.hashAndSaltPassword(password); +} + +/** + * Returns an object containing the encrypted form of a password, along with its random salt. + */ +export function hashAndSaltPassword(password) { + const salt = crypto.randomBytes(16).toString('base64'); // Generate a random salt + const encryptedPassword = encryptPassword(password, salt); + return { password_hash: encryptedPassword, password_salt: salt }; +} diff --git a/yarn.lock b/yarn.lock index 8a83fa8007..b615e34507 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7174,6 +7174,34 @@ __metadata: languageName: node linkType: hard +"@emnapi/core@npm:^1.1.0": + version: 1.2.0 + resolution: "@emnapi/core@npm:1.2.0" + dependencies: + "@emnapi/wasi-threads": 1.0.1 + tslib: ^2.4.0 + checksum: b3b61bd01de93346f05803151eee9dc308262065034d835db95a46842ea75867c43745c227577f19fa0542fcb3883a752477eb012bf9e4b72f540f4e23f63cbe + languageName: node + linkType: hard + +"@emnapi/runtime@npm:^1.1.0": + version: 1.2.0 + resolution: "@emnapi/runtime@npm:1.2.0" + dependencies: + tslib: ^2.4.0 + checksum: c9f5814f65a7851eda3fae96320b7ebfaf3b7e0db4e1ac2d77b55f5c0785e56b459a029413dbfc0abb1b23f059b850169888f92833150a28cdf24b9a53e535c5 + languageName: node + linkType: hard + +"@emnapi/wasi-threads@npm:1.0.1": + version: 1.0.1 + resolution: "@emnapi/wasi-threads@npm:1.0.1" + dependencies: + tslib: ^2.4.0 + checksum: e154880440ff9bfe67b417f30134f0ff6fee28913dbf4a22de2e67dda5bf5b51055647c5d1565281df17ef5dfcc89256546bdf9b8ccfd07e07566617e7ce1498 + languageName: node + linkType: hard + "@emotion/cache@npm:^10.0.27": version: 10.0.29 resolution: "@emotion/cache@npm:10.0.29" @@ -8534,6 +8562,17 @@ __metadata: languageName: node linkType: hard +"@napi-rs/wasm-runtime@npm:^0.2.3": + version: 0.2.4 + resolution: "@napi-rs/wasm-runtime@npm:0.2.4" + dependencies: + "@emnapi/core": ^1.1.0 + "@emnapi/runtime": ^1.1.0 + "@tybys/wasm-util": ^0.9.0 + checksum: 976eeca9c411724bf004f92a94707f1c78b6a5932a354e8b456eaae16c476dd6b96244c4afec60a3f621c922fca3ef2c6c3f6a900bd6b79f509dd4c0c2b3376d + languageName: node + linkType: hard + "@ndelangen/get-tarball@npm:^3.0.7": version: 3.0.9 resolution: "@ndelangen/get-tarball@npm:3.0.9" @@ -8563,6 +8602,157 @@ __metadata: languageName: node linkType: hard +"@node-rs/argon2-android-arm-eabi@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-android-arm-eabi@npm:1.8.3" + conditions: os=android & cpu=arm + languageName: node + linkType: hard + +"@node-rs/argon2-android-arm64@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-android-arm64@npm:1.8.3" + conditions: os=android & cpu=arm64 + languageName: node + linkType: hard + +"@node-rs/argon2-darwin-arm64@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-darwin-arm64@npm:1.8.3" + conditions: os=darwin & cpu=arm64 + languageName: node + linkType: hard + +"@node-rs/argon2-darwin-x64@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-darwin-x64@npm:1.8.3" + conditions: os=darwin & cpu=x64 + languageName: node + linkType: hard + +"@node-rs/argon2-freebsd-x64@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-freebsd-x64@npm:1.8.3" + conditions: os=freebsd & cpu=x64 + languageName: node + linkType: hard + +"@node-rs/argon2-linux-arm-gnueabihf@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-linux-arm-gnueabihf@npm:1.8.3" + conditions: os=linux & cpu=arm + languageName: node + linkType: hard + +"@node-rs/argon2-linux-arm64-gnu@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-linux-arm64-gnu@npm:1.8.3" + conditions: os=linux & cpu=arm64 & libc=glibc + languageName: node + linkType: hard + +"@node-rs/argon2-linux-arm64-musl@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-linux-arm64-musl@npm:1.8.3" + conditions: os=linux & cpu=arm64 & libc=musl + languageName: node + linkType: hard + +"@node-rs/argon2-linux-x64-gnu@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-linux-x64-gnu@npm:1.8.3" + conditions: os=linux & cpu=x64 & libc=glibc + languageName: node + linkType: hard + +"@node-rs/argon2-linux-x64-musl@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-linux-x64-musl@npm:1.8.3" + conditions: os=linux & cpu=x64 & libc=musl + languageName: node + linkType: hard + +"@node-rs/argon2-wasm32-wasi@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-wasm32-wasi@npm:1.8.3" + dependencies: + "@napi-rs/wasm-runtime": ^0.2.3 + conditions: cpu=wasm32 + languageName: node + linkType: hard + +"@node-rs/argon2-win32-arm64-msvc@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-win32-arm64-msvc@npm:1.8.3" + conditions: os=win32 & cpu=arm64 + languageName: node + linkType: hard + +"@node-rs/argon2-win32-ia32-msvc@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-win32-ia32-msvc@npm:1.8.3" + conditions: os=win32 & cpu=ia32 + languageName: node + linkType: hard + +"@node-rs/argon2-win32-x64-msvc@npm:1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2-win32-x64-msvc@npm:1.8.3" + conditions: os=win32 & cpu=x64 + languageName: node + linkType: hard + +"@node-rs/argon2@npm:^1.8.3": + version: 1.8.3 + resolution: "@node-rs/argon2@npm:1.8.3" + dependencies: + "@node-rs/argon2-android-arm-eabi": 1.8.3 + "@node-rs/argon2-android-arm64": 1.8.3 + "@node-rs/argon2-darwin-arm64": 1.8.3 + "@node-rs/argon2-darwin-x64": 1.8.3 + "@node-rs/argon2-freebsd-x64": 1.8.3 + "@node-rs/argon2-linux-arm-gnueabihf": 1.8.3 + "@node-rs/argon2-linux-arm64-gnu": 1.8.3 + "@node-rs/argon2-linux-arm64-musl": 1.8.3 + "@node-rs/argon2-linux-x64-gnu": 1.8.3 + "@node-rs/argon2-linux-x64-musl": 1.8.3 + "@node-rs/argon2-wasm32-wasi": 1.8.3 + "@node-rs/argon2-win32-arm64-msvc": 1.8.3 + "@node-rs/argon2-win32-ia32-msvc": 1.8.3 + "@node-rs/argon2-win32-x64-msvc": 1.8.3 + dependenciesMeta: + "@node-rs/argon2-android-arm-eabi": + optional: true + "@node-rs/argon2-android-arm64": + optional: true + "@node-rs/argon2-darwin-arm64": + optional: true + "@node-rs/argon2-darwin-x64": + optional: true + "@node-rs/argon2-freebsd-x64": + optional: true + "@node-rs/argon2-linux-arm-gnueabihf": + optional: true + "@node-rs/argon2-linux-arm64-gnu": + optional: true + "@node-rs/argon2-linux-arm64-musl": + optional: true + "@node-rs/argon2-linux-x64-gnu": + optional: true + "@node-rs/argon2-linux-x64-musl": + optional: true + "@node-rs/argon2-wasm32-wasi": + optional: true + "@node-rs/argon2-win32-arm64-msvc": + optional: true + "@node-rs/argon2-win32-ia32-msvc": + optional: true + "@node-rs/argon2-win32-x64-msvc": + optional: true + checksum: ee42315e94205f22205d6ba3569e4a2f6290f3f97e6c89ac2d2a6cca3e9f1fd9d9423bb096970d2238a34ee6a0df90b1a49878194db6cb9ae9150d4eee45688e + languageName: node + linkType: hard + "@nodelib/fs.scandir@npm:2.1.3": version: 2.1.3 resolution: "@nodelib/fs.scandir@npm:2.1.3" @@ -11832,6 +12022,7 @@ __metadata: resolution: "@tupaia/auth@workspace:packages/auth" dependencies: "@beyondessential/tupaia-access-policy": ^2.5.1 + "@node-rs/argon2": ^1.8.3 "@tupaia/access-policy": "workspace:*" "@tupaia/database": "workspace:*" "@tupaia/server-utils": "workspace:*" @@ -12832,6 +13023,15 @@ __metadata: languageName: unknown linkType: soft +"@tybys/wasm-util@npm:^0.9.0": + version: 0.9.0 + resolution: "@tybys/wasm-util@npm:0.9.0" + dependencies: + tslib: ^2.4.0 + checksum: 8d44c64e64e39c746e45b5dff7b534716f20e1f6e8fc206f8e4c8ac454ec0eb35b65646e446dd80745bc898db37a4eca549a936766d447c2158c9c43d44e7708 + languageName: node + linkType: hard + "@types/anymatch@npm:*": version: 1.3.1 resolution: "@types/anymatch@npm:1.3.1" From 2373cd10be5b5bf364250913f9630f4544744917 Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Mon, 2 Sep 2024 17:49:02 +1200 Subject: [PATCH 02/19] wip --- packages/auth/src/argon2.js | 30 +++++++++++--- packages/auth/src/index.js | 1 + packages/auth/src/utils.js | 8 +--- packages/datatrak-web-server/package.json | 1 + .../src/routes/LeaderboardRoute.ts | 40 +++++++++++++++++++ 5 files changed, 68 insertions(+), 12 deletions(-) diff --git a/packages/auth/src/argon2.js b/packages/auth/src/argon2.js index 1483827e35..bb08a2f809 100644 --- a/packages/auth/src/argon2.js +++ b/packages/auth/src/argon2.js @@ -3,20 +3,38 @@ * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ -const argon2 = require('argon2'); +import { randomBytes } from 'crypto'; +import { hash, verify } from '@node-rs/argon2'; /** - * Helper function to encrypt passwords using sha256 + * Helper function to encrypt passwords using argon2 + * @param password + * @param salt + * @returns {Promise} */ -export function encryptPassword(password) { - return argon2.hashAndSaltPassword(password); +export async function encryptPassword(password, salt) { + return hash(password); +} + +/** + * Helper function to verify passwords using argon2 + * @param password + * @param salt + * @param hash + * @returns {Promise} + */ +export async function verifyPassword(password, salt, hash) { + // Try to verify the password using argon2 + return verify(hash, password); } /** * Returns an object containing the encrypted form of a password, along with its random salt. + * @param password + * @returns {{password_salt: string, password_hash: Promise}} */ -export function hashAndSaltPassword(password) { - const salt = crypto.randomBytes(16).toString('base64'); // Generate a random salt +export async function hashAndSaltPassword(password) { + const salt = randomBytes(16).toString('base64'); // Generate a random salt const encryptedPassword = encryptPassword(password, salt); return { password_hash: encryptedPassword, password_salt: salt }; } diff --git a/packages/auth/src/index.js b/packages/auth/src/index.js index cef10e51d6..97e10eb498 100644 --- a/packages/auth/src/index.js +++ b/packages/auth/src/index.js @@ -5,6 +5,7 @@ export { Authenticator } from './Authenticator'; export { AccessPolicyBuilder } from './AccessPolicyBuilder'; +// export { encryptPassword, verifyPassword, hashAndSaltPassword } from './argon2'; export * from './utils'; export { getJwtToken, extractRefreshTokenFromReq, generateSecretKey } from './security'; export { diff --git a/packages/auth/src/utils.js b/packages/auth/src/utils.js index 5a7fdb706c..c45b0413a3 100644 --- a/packages/auth/src/utils.js +++ b/packages/auth/src/utils.js @@ -6,16 +6,12 @@ import sha256 from 'sha256'; import crypto from 'crypto'; -/** - * Helper function to encrypt passwords using sha256 - */ +// Superseded by argon2.js/ export function encryptPassword(password, salt) { return sha256(`${password}${salt}`); } -/** - * Returns an object containing the encrypted form of a password, along with its random salt. - */ +// Superseded by argon2.js/ export function hashAndSaltPassword(password) { const salt = crypto.randomBytes(16).toString('base64'); // Generate a random salt const encryptedPassword = encryptPassword(password, salt); diff --git a/packages/datatrak-web-server/package.json b/packages/datatrak-web-server/package.json index c5e62eb60a..1a2034abf8 100644 --- a/packages/datatrak-web-server/package.json +++ b/packages/datatrak-web-server/package.json @@ -25,6 +25,7 @@ "test": "yarn package:test" }, "dependencies": { + "@tupaia/auth": "workspace:*", "@tupaia/access-policy": "workspace:*", "@tupaia/api-client": "workspace:*", "@tupaia/database": "workspace:*", diff --git a/packages/datatrak-web-server/src/routes/LeaderboardRoute.ts b/packages/datatrak-web-server/src/routes/LeaderboardRoute.ts index 253cf04ee1..af228dda48 100644 --- a/packages/datatrak-web-server/src/routes/LeaderboardRoute.ts +++ b/packages/datatrak-web-server/src/routes/LeaderboardRoute.ts @@ -7,6 +7,8 @@ import { Request } from 'express'; import camelcaseKeys from 'camelcase-keys'; import { Route } from '@tupaia/server-boilerplate'; import { DatatrakWebLeaderboardRequest } from '@tupaia/types'; +import { hash, verify } from '@node-rs/argon2'; +import { encryptPassword as sha256Encrypt } from '@tupaia/auth'; export type LeaderboardRequest = Request< DatatrakWebLeaderboardRequest.Params, @@ -15,9 +17,47 @@ export type LeaderboardRequest = Request< DatatrakWebLeaderboardRequest.ReqQuery >; +export function encryptPassword(password: string, salt: string) { + return hash(`${password}${salt}`); +} + +export async function verifyPasswordOld(password: string, salt: string, hash: string) { + return verify(hash, `${password}${salt}`); +} + +export async function verifyPassword(password: string, salt: string, hash: string) { + // Try to verify password using argon2 directly + const verify1 = await verify(hash, `${password}${salt}`); + if (verify1) { + return true; + } + + // Try to verify password using sha256 plus argon2 + const hashedUserInput = sha256Encrypt(password, salt); + + return verify(hash, `${hashedUserInput}${salt}`); +} export class LeaderboardRoute extends Route { public async buildResponse() { const { models, query = {} } = this.req; + console.log('LeaderboardRoute:buildResponse'); + + const password = 'mysecretpassword'; + const salt = 'mysalt'; + const hashed = await encryptPassword(password, salt); + const verified = await verifyPassword(password, salt, hashed); + + // Test with sha256 module + const sha256Hash = sha256Encrypt(password, salt); + console.log('INPUT', sha256Hash); + const superHash = await encryptPassword(sha256Hash, salt); + const verified2 = await verifyPassword(password, salt, superHash); + + console.log('verified super hash', verified2); + + // Wrong password + // const wrong = await verifyPasswordOld('123', salt, hashed); + // console.log('wrong', wrong); const leaderboard = await models.surveyResponse.getLeaderboard(query.projectId); return camelcaseKeys(leaderboard, { deep: true }); From 9f6a8c3f7d61bc7a296efc3c585306edddbdfe1b Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Tue, 3 Sep 2024 11:24:16 +1200 Subject: [PATCH 03/19] refactor api clients --- packages/auth/src/Authenticator.js | 14 ++++--- packages/auth/src/argon2.js | 37 +++++++++++------ packages/auth/src/index.js | 4 +- .../central-server/src/apiV2/authenticate.js | 1 + .../src/apiV2/changePassword.js | 5 ++- .../src/apiV2/import/importUsers.js | 4 +- .../src/apiV2/middleware/auth/clientAuth.js | 12 +++--- .../apiV2/userAccounts/CreateUserAccounts.js | 6 ++- .../apiV2/userAccounts/EditUserAccounts.js | 3 +- .../src/apiV2/utilities/emailVerification.js | 13 +++++- .../src/dataAccessors/createUser.js | 4 +- .../apiV2/authenticate/authenticate.test.js | 4 +- ...02224836-argon2-passwords-modifies-data.js | 27 +++++++++++++ packages/database/src/modelClasses/User.js | 4 +- .../src/routes/LeaderboardRoute.ts | 40 ------------------- .../src/__tests__/testUtilities/setup.ts | 4 +- .../src/__tests__/utilities/setupTestUser.ts | 3 +- .../__integration__/testUtilities/setup.ts | 4 +- .../src/utils/initialiseApiClient.ts | 4 +- .../src/__tests__/testUtilities/setup.ts | 7 +++- .../src/authSession/getUserFromAuthHeader.js | 11 +++-- 21 files changed, 122 insertions(+), 89 deletions(-) create mode 100644 packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js diff --git a/packages/auth/src/Authenticator.js b/packages/auth/src/Authenticator.js index 86ca198796..1dd782e55b 100644 --- a/packages/auth/src/Authenticator.js +++ b/packages/auth/src/Authenticator.js @@ -1,6 +1,6 @@ /** * Tupaia - * Copyright (c) 2017 - 2020 Beyond Essential Systems Pty Ltd + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ import randomToken from 'rand-token'; import compareVersions from 'semver-compare'; @@ -8,7 +8,7 @@ import compareVersions from 'semver-compare'; import { DatabaseError, UnauthenticatedError, UnverifiedError } from '@tupaia/utils'; import { AccessPolicyBuilder } from './AccessPolicyBuilder'; import { mergeAccessPolicies } from './mergeAccessPolicies'; -import { encryptPassword } from './utils'; +import { verifyPassword } from './argon2'; import { getTokenClaims } from './userAuth'; const REFRESH_TOKEN_LENGTH = 40; @@ -47,12 +47,16 @@ export class Authenticator { * @param {{ username: string, secretKey: string }} apiClientCredentials */ async authenticateApiClient({ username, secretKey }) { - const secretKeyHash = encryptPassword(secretKey, process.env.API_CLIENT_SALT); const apiClient = await this.models.apiClient.findOne({ username, - secret_key_hash: secretKeyHash, }); - if (!apiClient) { + const verified = await verifyPassword( + secretKey, + process.env.API_CLIENT_SALT, + apiClient.secret_key_hash, + ); + console.log('verified', verified); + if (!verified) { throw new UnauthenticatedError('Could not authenticate Api Client'); } const user = await apiClient.getUser(); diff --git a/packages/auth/src/argon2.js b/packages/auth/src/argon2.js index bb08a2f809..65ba5320f1 100644 --- a/packages/auth/src/argon2.js +++ b/packages/auth/src/argon2.js @@ -5,36 +5,49 @@ import { randomBytes } from 'crypto'; import { hash, verify } from '@node-rs/argon2'; +import { encryptPassword as sha256Encrypt } from './utils'; /** * Helper function to encrypt passwords using argon2 - * @param password - * @param salt + * @param password {string} + * @param salt {string} * @returns {Promise} */ -export async function encryptPassword(password, salt) { - return hash(password); +export function encryptPassword(password, salt) { + return hash(`${password}${salt}`); } /** * Helper function to verify passwords using argon2 * @param password - * @param salt - * @param hash + * @param salt {string} + * @param hash {string} * @returns {Promise} */ export async function verifyPassword(password, salt, hash) { - // Try to verify the password using argon2 - return verify(hash, password); + // Try to verify password using argon2 directly + const isVerified = await verify(hash, `${password}${salt}`); + if (isVerified) { + return true; + } + + // Try to verify password using sha256 plus argon2 + const hashedUserInput = sha256Encrypt(password, salt); + + if (hashedUserInput === hash) { + console.warn('There was an error with the password migrations. Still using old password hash.'); + } + + return verify(hash, `${hashedUserInput}${salt}`); } /** - * Returns an object containing the encrypted form of a password, along with its random salt. - * @param password - * @returns {{password_salt: string, password_hash: Promise}} + * Helper function to hash and salt passwords using argon2 + * @param password {string} + * @returns {Promise<{password_salt: string, password_hash: string}>} */ export async function hashAndSaltPassword(password) { const salt = randomBytes(16).toString('base64'); // Generate a random salt - const encryptedPassword = encryptPassword(password, salt); + const encryptedPassword = await encryptPassword(password, salt); return { password_hash: encryptedPassword, password_salt: salt }; } diff --git a/packages/auth/src/index.js b/packages/auth/src/index.js index 97e10eb498..8d03931b36 100644 --- a/packages/auth/src/index.js +++ b/packages/auth/src/index.js @@ -5,8 +5,8 @@ export { Authenticator } from './Authenticator'; export { AccessPolicyBuilder } from './AccessPolicyBuilder'; -// export { encryptPassword, verifyPassword, hashAndSaltPassword } from './argon2'; -export * from './utils'; +export { encryptPassword, verifyPassword, hashAndSaltPassword } from './argon2'; +// export * from './utils'; export { getJwtToken, extractRefreshTokenFromReq, generateSecretKey } from './security'; export { getTokenClaimsFromBearerAuth, diff --git a/packages/central-server/src/apiV2/authenticate.js b/packages/central-server/src/apiV2/authenticate.js index 660ab3deb2..177823c92b 100644 --- a/packages/central-server/src/apiV2/authenticate.js +++ b/packages/central-server/src/apiV2/authenticate.js @@ -91,6 +91,7 @@ const checkApiClientAuthentication = async req => { } try { + console.log('authenticating api client', username, secretKey); return await authenticator.authenticateApiClient({ username, secretKey }); } catch (error) { winston.warn('Invalid Api Client Basic Auth header provided'); diff --git a/packages/central-server/src/apiV2/changePassword.js b/packages/central-server/src/apiV2/changePassword.js index 58af82c541..e7c1128c14 100644 --- a/packages/central-server/src/apiV2/changePassword.js +++ b/packages/central-server/src/apiV2/changePassword.js @@ -39,7 +39,7 @@ export async function changePassword(req, res, next) { if (!isTokenValid) { throw new FormValidationError('One time login is invalid'); } - } else if (!user.checkPassword(oldPassword)) { + } else if (!(await user.checkPassword(oldPassword))) { throw new FormValidationError('Incorrect current password', ['oldPassword']); } @@ -53,8 +53,9 @@ export async function changePassword(req, res, next) { throw new FormValidationError(error.message, ['password', 'passwordConfirm']); } + const passwordAndSalt = await hashAndSaltPassword(passwordParam); await models.user.updateById(userId, { - ...hashAndSaltPassword(passwordParam), + ...passwordAndSalt, }); respond(res, { message: 'Password successfully updated' }); diff --git a/packages/central-server/src/apiV2/import/importUsers.js b/packages/central-server/src/apiV2/import/importUsers.js index a55041e9b9..fd4aefa12e 100644 --- a/packages/central-server/src/apiV2/import/importUsers.js +++ b/packages/central-server/src/apiV2/import/importUsers.js @@ -53,9 +53,11 @@ export async function importUsers(req, res) { } emails.push(userObject.email); const { password, permission_group: permissionGroupName, ...restOfUser } = userObject; + const passwordAndSalt = await hashAndSaltPassword(password); + const userToUpsert = { ...restOfUser, - ...hashAndSaltPassword(password), + ...passwordAndSalt, }; const user = await transactingModels.user.updateOrCreate( { email: userObject.email }, diff --git a/packages/central-server/src/apiV2/middleware/auth/clientAuth.js b/packages/central-server/src/apiV2/middleware/auth/clientAuth.js index 0af89b46d1..d464dfe1dd 100644 --- a/packages/central-server/src/apiV2/middleware/auth/clientAuth.js +++ b/packages/central-server/src/apiV2/middleware/auth/clientAuth.js @@ -1,10 +1,9 @@ /** - * Tupaia MediTrak - * Copyright (c) 2017 Beyond Essential Systems Pty Ltd + * Tupaia + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ - import { UnauthenticatedError, requireEnv } from '@tupaia/utils'; -import { encryptPassword, getUserAndPassFromBasicAuth } from '@tupaia/auth'; +import { verifyPassword, getUserAndPassFromBasicAuth } from '@tupaia/auth'; export async function getAPIClientUser(authHeader, models) { const { username, password: secretKey } = getUserAndPassFromBasicAuth(authHeader); @@ -15,12 +14,11 @@ export async function getAPIClientUser(authHeader, models) { const API_CLIENT_SALT = requireEnv('API_CLIENT_SALT'); // We always need a valid client; throw if none is found - const secretKeyHash = encryptPassword(secretKey, API_CLIENT_SALT); const apiClient = await models.apiClient.findOne({ username, - secret_key_hash: secretKeyHash, }); - if (!apiClient) { + const verified = await verifyPassword(secretKey, API_CLIENT_SALT, apiClient.secret_key_hash); + if (!verified) { throw new UnauthenticatedError('Incorrect client username or secret'); } return apiClient.getUser(); diff --git a/packages/central-server/src/apiV2/userAccounts/CreateUserAccounts.js b/packages/central-server/src/apiV2/userAccounts/CreateUserAccounts.js index 541e584638..a3dac06784 100644 --- a/packages/central-server/src/apiV2/userAccounts/CreateUserAccounts.js +++ b/packages/central-server/src/apiV2/userAccounts/CreateUserAccounts.js @@ -46,7 +46,7 @@ export class CreateUserAccounts extends CreateHandler { await transactingModels.apiClient.create({ username: user.email, user_account_id: user.id, - secret_key_hash: encryptPassword(secretKey, process.env.API_CLIENT_SALT), + secret_key_hash: await encryptPassword(secretKey, process.env.API_CLIENT_SALT), }); } @@ -103,13 +103,15 @@ export class CreateUserAccounts extends CreateHandler { ...restOfUser }, ) { + const passwordAndSalt = await hashAndSaltPassword(password); + return transactingModels.user.create({ first_name: firstName, last_name: lastName, email: emailAddress, mobile_number: contactNumber, primary_platform: primaryPlatform, - ...hashAndSaltPassword(password), + ...passwordAndSalt, verified_email: verifiedEmail, ...restOfUser, }); diff --git a/packages/central-server/src/apiV2/userAccounts/EditUserAccounts.js b/packages/central-server/src/apiV2/userAccounts/EditUserAccounts.js index 10fbc9d812..a6cc7d9ffc 100644 --- a/packages/central-server/src/apiV2/userAccounts/EditUserAccounts.js +++ b/packages/central-server/src/apiV2/userAccounts/EditUserAccounts.js @@ -49,11 +49,12 @@ export class EditUserAccounts extends EditHandler { ...restOfUpdatedFields } = this.updatedFields; let updatedFields = restOfUpdatedFields; + const passwordAndSalt = await hashAndSaltPassword(password); if (password) { updatedFields = { ...updatedFields, - ...hashAndSaltPassword(password), + ...passwordAndSalt, }; } diff --git a/packages/central-server/src/apiV2/utilities/emailVerification.js b/packages/central-server/src/apiV2/utilities/emailVerification.js index 96080eba74..4c54116ebb 100644 --- a/packages/central-server/src/apiV2/utilities/emailVerification.js +++ b/packages/central-server/src/apiV2/utilities/emailVerification.js @@ -36,7 +36,7 @@ const EMAILS = { }; export const sendEmailVerification = async user => { - const token = encryptPassword(user.email + user.password_hash, user.password_salt); + const token = await encryptPassword(user.email + user.password_hash, user.password_salt); const platform = user.primary_platform ? user.primary_platform : 'tupaia'; const { subject, body, signOff } = EMAILS[platform]; const TUPAIA_FRONT_END_URL = requireEnv('TUPAIA_FRONT_END_URL'); @@ -57,5 +57,14 @@ export const verifyEmailHelper = async (models, searchCondition, token) => { verified_email: searchCondition, }); - return users.find(x => encryptPassword(x.email + x.password_hash, x.password_salt) === token); + for (const user of users) { + const encryptedValue = await encryptPassword( + `${user.email}${user.password_hash}`, + user.password_salt, + ); + if (encryptedValue === token) { + return user; + } + } + return null; }; diff --git a/packages/central-server/src/dataAccessors/createUser.js b/packages/central-server/src/dataAccessors/createUser.js index adb502d880..c0426f1593 100644 --- a/packages/central-server/src/dataAccessors/createUser.js +++ b/packages/central-server/src/dataAccessors/createUser.js @@ -39,12 +39,14 @@ export const createUser = async ( throw new Error(`No such country: ${countryName}`); } + const passwordAndSalt = await hashAndSaltPassword(password); + const user = await transactingModels.user.create({ first_name: firstName, last_name: lastName, email: emailAddress, mobile_number: contactNumber, - ...hashAndSaltPassword(password), + ...passwordAndSalt, verified_email: verifiedEmail, ...restOfUser, }); diff --git a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js index 4d0084bbef..da3fcdf271 100644 --- a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js +++ b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js @@ -39,11 +39,13 @@ describe('Authenticate', function () { }); // Create test users + const passwordAndSalt = await hashAndSaltPassword(userAccountPassword); + userAccount = await findOrCreateDummyRecord(models.user, { first_name: 'Ash', last_name: 'Ketchum', email: 'ash-ketchum@pokemon.org', - ...hashAndSaltPassword(userAccountPassword), + ...passwordAndSalt, verified_email: VERIFIED, }); diff --git a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js new file mode 100644 index 0000000000..3d2184c2b3 --- /dev/null +++ b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js @@ -0,0 +1,27 @@ +'use strict'; + +var dbm; +var type; +var seed; + +/** + * We receive the dbmigrate dependency from dbmigrate initially. + * This enables us to not have to rely on NODE_PATH. + */ +exports.setup = function(options, seedLink) { + dbm = options.dbmigrate; + type = dbm.dataType; + seed = seedLink; +}; + +exports.up = function(db) { + return null; +}; + +exports.down = function(db) { + return null; +}; + +exports._meta = { + "version": 1 +}; diff --git a/packages/database/src/modelClasses/User.js b/packages/database/src/modelClasses/User.js index 79a61e0ba4..1a486096a6 100644 --- a/packages/database/src/modelClasses/User.js +++ b/packages/database/src/modelClasses/User.js @@ -20,8 +20,8 @@ export class UserRecord extends DatabaseRecord { } // Checks if the provided non-encrypted password corresponds to this user - checkPassword(password) { - return encryptPassword(password, this.password_salt) === this.password_hash; + async checkPassword(password) { + return await encryptPassword(password, this.password_salt) === this.password_hash; } checkIsEmailUnverified() { diff --git a/packages/datatrak-web-server/src/routes/LeaderboardRoute.ts b/packages/datatrak-web-server/src/routes/LeaderboardRoute.ts index af228dda48..253cf04ee1 100644 --- a/packages/datatrak-web-server/src/routes/LeaderboardRoute.ts +++ b/packages/datatrak-web-server/src/routes/LeaderboardRoute.ts @@ -7,8 +7,6 @@ import { Request } from 'express'; import camelcaseKeys from 'camelcase-keys'; import { Route } from '@tupaia/server-boilerplate'; import { DatatrakWebLeaderboardRequest } from '@tupaia/types'; -import { hash, verify } from '@node-rs/argon2'; -import { encryptPassword as sha256Encrypt } from '@tupaia/auth'; export type LeaderboardRequest = Request< DatatrakWebLeaderboardRequest.Params, @@ -17,47 +15,9 @@ export type LeaderboardRequest = Request< DatatrakWebLeaderboardRequest.ReqQuery >; -export function encryptPassword(password: string, salt: string) { - return hash(`${password}${salt}`); -} - -export async function verifyPasswordOld(password: string, salt: string, hash: string) { - return verify(hash, `${password}${salt}`); -} - -export async function verifyPassword(password: string, salt: string, hash: string) { - // Try to verify password using argon2 directly - const verify1 = await verify(hash, `${password}${salt}`); - if (verify1) { - return true; - } - - // Try to verify password using sha256 plus argon2 - const hashedUserInput = sha256Encrypt(password, salt); - - return verify(hash, `${hashedUserInput}${salt}`); -} export class LeaderboardRoute extends Route { public async buildResponse() { const { models, query = {} } = this.req; - console.log('LeaderboardRoute:buildResponse'); - - const password = 'mysecretpassword'; - const salt = 'mysalt'; - const hashed = await encryptPassword(password, salt); - const verified = await verifyPassword(password, salt, hashed); - - // Test with sha256 module - const sha256Hash = sha256Encrypt(password, salt); - console.log('INPUT', sha256Hash); - const superHash = await encryptPassword(sha256Hash, salt); - const verified2 = await verifyPassword(password, salt, superHash); - - console.log('verified super hash', verified2); - - // Wrong password - // const wrong = await verifyPasswordOld('123', salt, hashed); - // console.log('wrong', wrong); const leaderboard = await models.surveyResponse.getLeaderboard(query.projectId); return camelcaseKeys(leaderboard, { deep: true }); diff --git a/packages/entity-server/src/__tests__/testUtilities/setup.ts b/packages/entity-server/src/__tests__/testUtilities/setup.ts index ac2b0f936f..42194c66b1 100644 --- a/packages/entity-server/src/__tests__/testUtilities/setup.ts +++ b/packages/entity-server/src/__tests__/testUtilities/setup.ts @@ -45,6 +45,8 @@ export const setupTestData = async () => { const { VERIFIED } = models.user.emailVerifiedStatuses; + const passwordAndSalt = await hashAndSaltPassword(userAccountPassword); + await findOrCreateDummyRecord( models.user, { @@ -53,7 +55,7 @@ export const setupTestData = async () => { { first_name: 'Ash', last_name: 'Ketchum', - ...hashAndSaltPassword(userAccountPassword), + ...passwordAndSalt, verified_email: VERIFIED, }, ); diff --git a/packages/meditrak-app-server/src/__tests__/utilities/setupTestUser.ts b/packages/meditrak-app-server/src/__tests__/utilities/setupTestUser.ts index 6bcc11aee7..9614507449 100644 --- a/packages/meditrak-app-server/src/__tests__/utilities/setupTestUser.ts +++ b/packages/meditrak-app-server/src/__tests__/utilities/setupTestUser.ts @@ -13,6 +13,7 @@ const models = getTestModels() as TestModelRegistry; export const setupTestUser = async () => { const { VERIFIED } = models.user.emailVerifiedStatuses; const { email, firstName, lastName, password } = CAT_USER; + const passwordAndSalt = await hashAndSaltPassword(password); return findOrCreateDummyRecord( models.user, @@ -22,7 +23,7 @@ export const setupTestUser = async () => { { first_name: firstName, last_name: lastName, - ...hashAndSaltPassword(password), + ...passwordAndSalt, verified_email: VERIFIED, }, ); diff --git a/packages/report-server/src/__tests__/__integration__/testUtilities/setup.ts b/packages/report-server/src/__tests__/__integration__/testUtilities/setup.ts index 71eac262b6..05aca6c9e4 100644 --- a/packages/report-server/src/__tests__/__integration__/testUtilities/setup.ts +++ b/packages/report-server/src/__tests__/__integration__/testUtilities/setup.ts @@ -58,6 +58,8 @@ export const setupTestData = async () => { // add user const { VERIFIED } = models.user.emailVerifiedStatuses; + const passwordAndSalt = await hashAndSaltPassword(userAccountPassword); + await findOrCreateDummyRecord( models.user, { @@ -66,7 +68,7 @@ export const setupTestData = async () => { { first_name: 'Ash', last_name: 'Ketchum', - ...hashAndSaltPassword(userAccountPassword), + ...passwordAndSalt, verified_email: VERIFIED, }, ); diff --git a/packages/server-boilerplate/src/utils/initialiseApiClient.ts b/packages/server-boilerplate/src/utils/initialiseApiClient.ts index 0e5bf859c0..ea3d9f4ceb 100644 --- a/packages/server-boilerplate/src/utils/initialiseApiClient.ts +++ b/packages/server-boilerplate/src/utils/initialiseApiClient.ts @@ -21,7 +21,7 @@ const upsertUserAccount = async ({ password: string; salt: string; }): Promise => { - const passwordHash = encryptPassword(password, salt); + const passwordHash = await encryptPassword(password, salt); const firstName = email; const lastName = 'API Client'; @@ -66,7 +66,7 @@ const upsertApiClient = async ({ password: string; salt: string; }) => { - const secretKeyHash = encryptPassword(password, salt); + const secretKeyHash = await encryptPassword(password, salt); const existingApiClient = await models.apiClient.findOne({ username: username, diff --git a/packages/tupaia-web-server/src/__tests__/testUtilities/setup.ts b/packages/tupaia-web-server/src/__tests__/testUtilities/setup.ts index 157ea45c1a..5d906dcf60 100644 --- a/packages/tupaia-web-server/src/__tests__/testUtilities/setup.ts +++ b/packages/tupaia-web-server/src/__tests__/testUtilities/setup.ts @@ -48,6 +48,7 @@ export const setupTestData = async () => { hierarchyCacher.stopListeningForChanges(); const { VERIFIED } = models.user.emailVerifiedStatuses; + const userAccountPasswordAndSalt = await hashAndSaltPassword(userAccountPassword); await findOrCreateDummyRecord( models.user, @@ -57,13 +58,15 @@ export const setupTestData = async () => { { first_name: 'Ash', last_name: 'Ketchum', - ...hashAndSaltPassword(userAccountPassword), + ...userAccountPasswordAndSalt, verified_email: VERIFIED, }, ); const apiClientEmail = requireEnv('API_CLIENT_NAME'); const apiClientPassword = requireEnv('API_CLIENT_PASSWORD'); + const apiClientPasswordAndSalt = await hashAndSaltPassword(apiClientPassword); + const apiClient = await findOrCreateDummyRecord( models.user, { @@ -72,7 +75,7 @@ export const setupTestData = async () => { { first_name: 'API', last_name: 'Client', - ...hashAndSaltPassword(apiClientPassword), + ...apiClientPasswordAndSalt, verified_email: VERIFIED, }, ); diff --git a/packages/web-config-server/src/authSession/getUserFromAuthHeader.js b/packages/web-config-server/src/authSession/getUserFromAuthHeader.js index a31f0658f7..5da6afad97 100644 --- a/packages/web-config-server/src/authSession/getUserFromAuthHeader.js +++ b/packages/web-config-server/src/authSession/getUserFromAuthHeader.js @@ -3,21 +3,24 @@ * Copyright (c) 2017 - 2021 Beyond Essential Systems Pty Ltd */ import { - encryptPassword, getUserAndPassFromBasicAuth, getTokenClaimsFromBearerAuth, + verifyPassword, } from '@tupaia/auth'; const getApiClientUserFromBasicAuth = async (models, authHeader) => { const { username, password: secretKey } = getUserAndPassFromBasicAuth(authHeader); // first attempt to authenticate as an api client, in case a secret key was used in the auth header - const secretKeyHash = encryptPassword(secretKey, process.env.API_CLIENT_SALT); const apiClient = await models.apiClient.findOne({ username, - secret_key_hash: secretKeyHash, }); - return apiClient?.getUser(); + const verified = await verifyPassword( + secretKey, + process.env.API_CLIENT_SALT, + apiClient.secret_key_hash, + ); + return verified ? apiClient?.getUser() : undefined; }; const getUserFromBearerAuth = async (models, authHeader) => { From 3bbd575c7d34d6499618b483377c52cb25bfa491 Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Tue, 3 Sep 2024 14:11:28 +1200 Subject: [PATCH 04/19] migrate passwords to argon --- packages/auth/src/argon2.js | 10 ++-- ...02224836-argon2-passwords-modifies-data.js | 46 +++++++++++++++---- packages/database/src/modelClasses/User.js | 11 ++--- 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/packages/auth/src/argon2.js b/packages/auth/src/argon2.js index 65ba5320f1..b79c2d3d8d 100644 --- a/packages/auth/src/argon2.js +++ b/packages/auth/src/argon2.js @@ -2,7 +2,6 @@ * Tupaia * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ - import { randomBytes } from 'crypto'; import { hash, verify } from '@node-rs/argon2'; import { encryptPassword as sha256Encrypt } from './utils'; @@ -34,11 +33,12 @@ export async function verifyPassword(password, salt, hash) { // Try to verify password using sha256 plus argon2 const hashedUserInput = sha256Encrypt(password, salt); - if (hashedUserInput === hash) { - console.warn('There was an error with the password migrations. Still using old password hash.'); + const isVerifiedSha256 = verify(hash, `${hashedUserInput}${salt}`); + if (isVerifiedSha256) { + // Move password to argon2 + console.log('Password was verified using sha256 plus argon2', password); } - - return verify(hash, `${hashedUserInput}${salt}`); + return true; } /** diff --git a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js index 3d2184c2b3..746da04d95 100644 --- a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js +++ b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js @@ -1,27 +1,55 @@ 'use strict'; +import { hash } from '@node-rs/argon2'; + var dbm; var type; var seed; /** - * We receive the dbmigrate dependency from dbmigrate initially. - * This enables us to not have to rely on NODE_PATH. - */ -exports.setup = function(options, seedLink) { + * We receive the dbmigrate dependency from dbmigrate initially. + * This enables us to not have to rely on NODE_PATH. + */ +exports.setup = function (options, seedLink) { dbm = options.dbmigrate; type = dbm.dataType; seed = seedLink; }; -exports.up = function(db) { - return null; +exports.up = async function (db) { + await db.runSql(` + ALTER TABLE user_account + RENAME COLUMN password_hash TO password_hash_old; + + ALTER TABLE user_account + ADD COLUMN password_hash TEXT; + `); + console.log('ALTERED TABLE'); + + const users = await db.runSql('SELECT id, password_hash_old, password_salt FROM user_account'); + + console.log('UPDATING PASSWORDS'); + let count = 0; + for (let user of users.rows) { + const { id, password_hash_old, password_salt } = user; + const hashedValue = await hash(`${password_hash_old}${password_salt}`); + await db.runSql('UPDATE user_account SET password_hash = $1 WHERE id = $2', [hashedValue, id]); + count++; + console.log('Updated ', count, ' passwords'); + } + console.log('DONE'); }; -exports.down = function(db) { - return null; +exports.down = function (db) { + return db.runSql(` + ALTER TABLE user_account + DROP COLUMN password_hash; + + ALTER TABLE user_account + RENAME COLUMN password_hash_old TO password_hash; + `); }; exports._meta = { - "version": 1 + version: 1, }; diff --git a/packages/database/src/modelClasses/User.js b/packages/database/src/modelClasses/User.js index 1a486096a6..2e1218ce0a 100644 --- a/packages/database/src/modelClasses/User.js +++ b/packages/database/src/modelClasses/User.js @@ -1,9 +1,8 @@ -/** - * Tupaia MediTrak - * Copyright (c) 2017 Beyond Essential Systems Pty Ltd +/* + * Tupaia + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ -import { encryptPassword } from '@tupaia/auth'; - +import { verifyPassword } from '@tupaia/auth'; import { DatabaseModel } from '../DatabaseModel'; import { DatabaseRecord } from '../DatabaseRecord'; import { RECORDS } from '../records'; @@ -21,7 +20,7 @@ export class UserRecord extends DatabaseRecord { // Checks if the provided non-encrypted password corresponds to this user async checkPassword(password) { - return await encryptPassword(password, this.password_salt) === this.password_hash; + return verifyPassword(password, this.password_salt, this.password_hash); } checkIsEmailUnverified() { From d086c0fe8547e64043e1dafa091e51f00a7d55a9 Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Tue, 3 Sep 2024 15:09:14 +1200 Subject: [PATCH 05/19] updating passwords on login --- packages/auth/src/Authenticator.js | 2 +- packages/auth/src/index.js | 11 ++++--- .../src/{argon2.js => passwordEncryption.js} | 29 +++++++++---------- packages/auth/src/utils.js | 19 ------------ packages/database/src/modelClasses/User.js | 24 +++++++++++++-- 5 files changed, 43 insertions(+), 42 deletions(-) rename packages/auth/src/{argon2.js => passwordEncryption.js} (65%) delete mode 100644 packages/auth/src/utils.js diff --git a/packages/auth/src/Authenticator.js b/packages/auth/src/Authenticator.js index 1dd782e55b..441117b5ad 100644 --- a/packages/auth/src/Authenticator.js +++ b/packages/auth/src/Authenticator.js @@ -8,7 +8,7 @@ import compareVersions from 'semver-compare'; import { DatabaseError, UnauthenticatedError, UnverifiedError } from '@tupaia/utils'; import { AccessPolicyBuilder } from './AccessPolicyBuilder'; import { mergeAccessPolicies } from './mergeAccessPolicies'; -import { verifyPassword } from './argon2'; +import { verifyPassword } from './passwordEncryption'; import { getTokenClaims } from './userAuth'; const REFRESH_TOKEN_LENGTH = 40; diff --git a/packages/auth/src/index.js b/packages/auth/src/index.js index 8d03931b36..0402e2f8d8 100644 --- a/packages/auth/src/index.js +++ b/packages/auth/src/index.js @@ -1,12 +1,15 @@ /** * Tupaia - * Copyright (c) 2017 - 2020 Beyond Essential Systems Pty Ltd + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ - export { Authenticator } from './Authenticator'; export { AccessPolicyBuilder } from './AccessPolicyBuilder'; -export { encryptPassword, verifyPassword, hashAndSaltPassword } from './argon2'; -// export * from './utils'; +export { + encryptPassword, + verifyPassword, + hashAndSaltPassword, + sha256EncryptPassword, +} from './passwordEncryption'; export { getJwtToken, extractRefreshTokenFromReq, generateSecretKey } from './security'; export { getTokenClaimsFromBearerAuth, diff --git a/packages/auth/src/argon2.js b/packages/auth/src/passwordEncryption.js similarity index 65% rename from packages/auth/src/argon2.js rename to packages/auth/src/passwordEncryption.js index b79c2d3d8d..4a1b459f51 100644 --- a/packages/auth/src/argon2.js +++ b/packages/auth/src/passwordEncryption.js @@ -4,7 +4,7 @@ */ import { randomBytes } from 'crypto'; import { hash, verify } from '@node-rs/argon2'; -import { encryptPassword as sha256Encrypt } from './utils'; +import sha256 from 'sha256'; /** * Helper function to encrypt passwords using argon2 @@ -24,21 +24,8 @@ export function encryptPassword(password, salt) { * @returns {Promise} */ export async function verifyPassword(password, salt, hash) { - // Try to verify password using argon2 directly - const isVerified = await verify(hash, `${password}${salt}`); - if (isVerified) { - return true; - } - - // Try to verify password using sha256 plus argon2 - const hashedUserInput = sha256Encrypt(password, salt); - - const isVerifiedSha256 = verify(hash, `${hashedUserInput}${salt}`); - if (isVerifiedSha256) { - // Move password to argon2 - console.log('Password was verified using sha256 plus argon2', password); - } - return true; + console.log('Verifying password...'); + return verify(hash, `${password}${salt}`); } /** @@ -51,3 +38,13 @@ export async function hashAndSaltPassword(password) { const encryptedPassword = await encryptPassword(password, salt); return { password_hash: encryptedPassword, password_salt: salt }; } + +/** + * Helper function to encrypt passwords using sha256 + * @param password {string} + * @param salt {string} + * @returns {string} + */ +export function sha256EncryptPassword(password, salt) { + return sha256(`${password}${salt}`); +} diff --git a/packages/auth/src/utils.js b/packages/auth/src/utils.js deleted file mode 100644 index c45b0413a3..0000000000 --- a/packages/auth/src/utils.js +++ /dev/null @@ -1,19 +0,0 @@ -/** - * Tupaia - * Copyright (c) 2017 - 2020 Beyond Essential Systems Pty Ltd - */ - -import sha256 from 'sha256'; -import crypto from 'crypto'; - -// Superseded by argon2.js/ -export function encryptPassword(password, salt) { - return sha256(`${password}${salt}`); -} - -// Superseded by argon2.js/ -export function hashAndSaltPassword(password) { - const salt = crypto.randomBytes(16).toString('base64'); // Generate a random salt - const encryptedPassword = encryptPassword(password, salt); - return { password_hash: encryptedPassword, password_salt: salt }; -} diff --git a/packages/database/src/modelClasses/User.js b/packages/database/src/modelClasses/User.js index 2e1218ce0a..4f7a3b3c0a 100644 --- a/packages/database/src/modelClasses/User.js +++ b/packages/database/src/modelClasses/User.js @@ -2,7 +2,8 @@ * Tupaia * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ -import { verifyPassword } from '@tupaia/auth'; +import { verify } from '@node-rs/argon2'; +import { verifyPassword, sha256EncryptPassword, hashAndSaltPassword } from '@tupaia/auth'; import { DatabaseModel } from '../DatabaseModel'; import { DatabaseRecord } from '../DatabaseRecord'; import { RECORDS } from '../records'; @@ -20,7 +21,26 @@ export class UserRecord extends DatabaseRecord { // Checks if the provided non-encrypted password corresponds to this user async checkPassword(password) { - return verifyPassword(password, this.password_salt, this.password_hash); + const salt = this.password_salt; + const hash = this.password_hash; + + // Try to verify password using argon2 directly + const isVerified = await verifyPassword(password, this.password_salt, this.password_hash); + if (isVerified) { + return true; + } + + // Try to verify password using sha256 plus argon2 + const hashedUserInput = sha256EncryptPassword(password, salt); + const isVerifiedSha256 = await verify(hash, `${hashedUserInput}${salt}`); + if (isVerifiedSha256) { + // Move password to argon2 + const passwordAndSalt = await hashAndSaltPassword(password); + await this.model.updateById(this.id, passwordAndSalt); + return true; + } + + return false; } checkIsEmailUnverified() { From af5fcec81a2aeba70b85442e72757a1356267c62 Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Tue, 3 Sep 2024 16:15:34 +1200 Subject: [PATCH 06/19] use same salt --- packages/auth/src/passwordEncryption.js | 1 - packages/database/src/modelClasses/User.js | 7 ++++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/auth/src/passwordEncryption.js b/packages/auth/src/passwordEncryption.js index 4a1b459f51..e04c8eaecf 100644 --- a/packages/auth/src/passwordEncryption.js +++ b/packages/auth/src/passwordEncryption.js @@ -24,7 +24,6 @@ export function encryptPassword(password, salt) { * @returns {Promise} */ export async function verifyPassword(password, salt, hash) { - console.log('Verifying password...'); return verify(hash, `${password}${salt}`); } diff --git a/packages/database/src/modelClasses/User.js b/packages/database/src/modelClasses/User.js index 4f7a3b3c0a..b182795589 100644 --- a/packages/database/src/modelClasses/User.js +++ b/packages/database/src/modelClasses/User.js @@ -3,10 +3,11 @@ * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ import { verify } from '@node-rs/argon2'; -import { verifyPassword, sha256EncryptPassword, hashAndSaltPassword } from '@tupaia/auth'; +import { verifyPassword, sha256EncryptPassword } from '@tupaia/auth'; import { DatabaseModel } from '../DatabaseModel'; import { DatabaseRecord } from '../DatabaseRecord'; import { RECORDS } from '../records'; +import { encryptPassword } from '@tupaia/auth/src'; export class UserRecord extends DatabaseRecord { static databaseRecord = RECORDS.USER_ACCOUNT; @@ -35,8 +36,8 @@ export class UserRecord extends DatabaseRecord { const isVerifiedSha256 = await verify(hash, `${hashedUserInput}${salt}`); if (isVerifiedSha256) { // Move password to argon2 - const passwordAndSalt = await hashAndSaltPassword(password); - await this.model.updateById(this.id, passwordAndSalt); + const encryptedPassword = await encryptPassword(password, salt); + await this.model.updateById(this.id, { password_hash: encryptedPassword }); return true; } From 3f8e591b78a537db8079c0443544f195dad54522 Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Thu, 5 Sep 2024 13:34:25 +1200 Subject: [PATCH 07/19] Update package.json --- packages/datatrak-web-server/package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/datatrak-web-server/package.json b/packages/datatrak-web-server/package.json index 1a2034abf8..c5e62eb60a 100644 --- a/packages/datatrak-web-server/package.json +++ b/packages/datatrak-web-server/package.json @@ -25,7 +25,6 @@ "test": "yarn package:test" }, "dependencies": { - "@tupaia/auth": "workspace:*", "@tupaia/access-policy": "workspace:*", "@tupaia/api-client": "workspace:*", "@tupaia/database": "workspace:*", From ac7d97ca921ea4513dc56a7ea659a057a2345f5b Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Thu, 5 Sep 2024 13:46:25 +1200 Subject: [PATCH 08/19] Update User.js --- packages/database/src/modelClasses/User.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/database/src/modelClasses/User.js b/packages/database/src/modelClasses/User.js index b182795589..5d3470eee4 100644 --- a/packages/database/src/modelClasses/User.js +++ b/packages/database/src/modelClasses/User.js @@ -3,11 +3,10 @@ * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ import { verify } from '@node-rs/argon2'; -import { verifyPassword, sha256EncryptPassword } from '@tupaia/auth'; +import { encryptPassword, verifyPassword, sha256EncryptPassword } from '@tupaia/auth'; import { DatabaseModel } from '../DatabaseModel'; import { DatabaseRecord } from '../DatabaseRecord'; import { RECORDS } from '../records'; -import { encryptPassword } from '@tupaia/auth/src'; export class UserRecord extends DatabaseRecord { static databaseRecord = RECORDS.USER_ACCOUNT; From abaa7d02175d95ac87a3e7b39d8dc2fa6fa7bb81 Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Thu, 5 Sep 2024 16:49:42 +1200 Subject: [PATCH 09/19] await password encryption --- packages/auth/src/Authenticator.js | 1 - packages/central-server/src/apiV2/authenticate.js | 1 - packages/central-server/src/dataAccessors/createUser.js | 2 +- .../src/database/models/UserEntityPermission.js | 2 +- .../src/tests/apiV2/authenticate/authenticate.test.js | 2 +- packages/central-server/src/tests/apiV2/verifyEmail.test.js | 2 +- .../src/tests/testUtilities/database/addBaselineTestData.js | 2 +- .../20240902224836-argon2-passwords-modifies-data.js | 5 ----- 8 files changed, 5 insertions(+), 12 deletions(-) diff --git a/packages/auth/src/Authenticator.js b/packages/auth/src/Authenticator.js index 441117b5ad..ad7e612373 100644 --- a/packages/auth/src/Authenticator.js +++ b/packages/auth/src/Authenticator.js @@ -55,7 +55,6 @@ export class Authenticator { process.env.API_CLIENT_SALT, apiClient.secret_key_hash, ); - console.log('verified', verified); if (!verified) { throw new UnauthenticatedError('Could not authenticate Api Client'); } diff --git a/packages/central-server/src/apiV2/authenticate.js b/packages/central-server/src/apiV2/authenticate.js index 177823c92b..660ab3deb2 100644 --- a/packages/central-server/src/apiV2/authenticate.js +++ b/packages/central-server/src/apiV2/authenticate.js @@ -91,7 +91,6 @@ const checkApiClientAuthentication = async req => { } try { - console.log('authenticating api client', username, secretKey); return await authenticator.authenticateApiClient({ username, secretKey }); } catch (error) { winston.warn('Invalid Api Client Basic Auth header provided'); diff --git a/packages/central-server/src/dataAccessors/createUser.js b/packages/central-server/src/dataAccessors/createUser.js index c0426f1593..93f2dfe931 100644 --- a/packages/central-server/src/dataAccessors/createUser.js +++ b/packages/central-server/src/dataAccessors/createUser.js @@ -63,7 +63,7 @@ export const createUser = async ( await transactingModels.apiClient.create({ username: user.email, user_account_id: user.id, - secret_key_hash: encryptPassword(secretKey, process.env.API_CLIENT_SALT), + secret_key_hash: await encryptPassword(secretKey, process.env.API_CLIENT_SALT), }); } diff --git a/packages/central-server/src/database/models/UserEntityPermission.js b/packages/central-server/src/database/models/UserEntityPermission.js index 29f7fa60c3..a66bd309f7 100644 --- a/packages/central-server/src/database/models/UserEntityPermission.js +++ b/packages/central-server/src/database/models/UserEntityPermission.js @@ -67,5 +67,5 @@ async function onUpsertSendPermissionGrantEmail( async function expireAccess({ new_record: newRecord, old_record: oldRecord }, models) { const userId = newRecord?.user_id || oldRecord.user_id; const user = await models.user.findById(userId); - await user.expireSessionToken('tupaia_web'); + await user?.expireSessionToken('tupaia_web'); } diff --git a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js index da3fcdf271..2c99bd1560 100644 --- a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js +++ b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js @@ -58,7 +58,7 @@ describe('Authenticate', function () { await findOrCreateDummyRecord(models.apiClient, { username: apiClientUserAccount.email, user_account_id: apiClientUserAccount.id, - secret_key_hash: encryptPassword(apiClientSecret, process.env.API_CLIENT_SALT), + secret_key_hash: await encryptPassword(apiClientSecret, process.env.API_CLIENT_SALT), }); // Public Demo Land Permission diff --git a/packages/central-server/src/tests/apiV2/verifyEmail.test.js b/packages/central-server/src/tests/apiV2/verifyEmail.test.js index fab279bd50..be54513090 100644 --- a/packages/central-server/src/tests/apiV2/verifyEmail.test.js +++ b/packages/central-server/src/tests/apiV2/verifyEmail.test.js @@ -43,7 +43,7 @@ describe('Verify Email', () => { const verifyEmail = async userId => { const user = await models.user.findById(userId); - const token = encryptPassword(user.email + user.password_hash, user.password_salt); + const token = await encryptPassword(user.email + user.password_hash, user.password_salt); return app.post('auth/verifyEmail', { headers, diff --git a/packages/central-server/src/tests/testUtilities/database/addBaselineTestData.js b/packages/central-server/src/tests/testUtilities/database/addBaselineTestData.js index 9dcc5bac83..ea93bcd65f 100644 --- a/packages/central-server/src/tests/testUtilities/database/addBaselineTestData.js +++ b/packages/central-server/src/tests/testUtilities/database/addBaselineTestData.js @@ -90,7 +90,7 @@ export async function addBaselineTestData() { }, { user_account_id: apiUser.userId, - secret_key_hash: encryptPassword(TEST_API_USER_PASSWORD, process.env.API_CLIENT_SALT), + secret_key_hash: await encryptPassword(TEST_API_USER_PASSWORD, process.env.API_CLIENT_SALT), }, ); } diff --git a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js index 746da04d95..9e3d8616d3 100644 --- a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js +++ b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js @@ -24,20 +24,15 @@ exports.up = async function (db) { ALTER TABLE user_account ADD COLUMN password_hash TEXT; `); - console.log('ALTERED TABLE'); - const users = await db.runSql('SELECT id, password_hash_old, password_salt FROM user_account'); - console.log('UPDATING PASSWORDS'); let count = 0; for (let user of users.rows) { const { id, password_hash_old, password_salt } = user; const hashedValue = await hash(`${password_hash_old}${password_salt}`); await db.runSql('UPDATE user_account SET password_hash = $1 WHERE id = $2', [hashedValue, id]); count++; - console.log('Updated ', count, ' passwords'); } - console.log('DONE'); }; exports.down = function (db) { From e30d0fd28618dcc38238e7f3bb254487d51db080 Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Fri, 6 Sep 2024 15:06:23 +1200 Subject: [PATCH 10/19] fix verify email test --- .../src/apiV2/utilities/emailVerification.js | 10 ++++++---- .../central-server/src/tests/apiV2/verifyEmail.test.js | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/central-server/src/apiV2/utilities/emailVerification.js b/packages/central-server/src/apiV2/utilities/emailVerification.js index 4c54116ebb..6264011629 100644 --- a/packages/central-server/src/apiV2/utilities/emailVerification.js +++ b/packages/central-server/src/apiV2/utilities/emailVerification.js @@ -3,7 +3,7 @@ * Copyright (c) 2017 - 2021 Beyond Essential Systems Pty Ltd */ -import { encryptPassword } from '@tupaia/auth'; +import { encryptPassword, verifyPassword } from '@tupaia/auth'; import { sendEmail } from '@tupaia/server-utils'; import { requireEnv } from '@tupaia/utils'; @@ -36,7 +36,7 @@ const EMAILS = { }; export const sendEmailVerification = async user => { - const token = await encryptPassword(user.email + user.password_hash, user.password_salt); + const token = await encryptPassword(`${user.email}${user.password_hash}`, user.password_salt); const platform = user.primary_platform ? user.primary_platform : 'tupaia'; const { subject, body, signOff } = EMAILS[platform]; const TUPAIA_FRONT_END_URL = requireEnv('TUPAIA_FRONT_END_URL'); @@ -58,11 +58,13 @@ export const verifyEmailHelper = async (models, searchCondition, token) => { }); for (const user of users) { - const encryptedValue = await encryptPassword( + const verified = await verifyPassword( `${user.email}${user.password_hash}`, user.password_salt, + token, ); - if (encryptedValue === token) { + + if (verified) { return user; } } diff --git a/packages/central-server/src/tests/apiV2/verifyEmail.test.js b/packages/central-server/src/tests/apiV2/verifyEmail.test.js index be54513090..8f1fdd29c5 100644 --- a/packages/central-server/src/tests/apiV2/verifyEmail.test.js +++ b/packages/central-server/src/tests/apiV2/verifyEmail.test.js @@ -43,7 +43,7 @@ describe('Verify Email', () => { const verifyEmail = async userId => { const user = await models.user.findById(userId); - const token = await encryptPassword(user.email + user.password_hash, user.password_salt); + const token = await encryptPassword(`${user.email}${user.password_hash}`, user.password_salt); return app.post('auth/verifyEmail', { headers, From f5718699203fdab7b915630298e514426fcf6798 Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Fri, 6 Sep 2024 16:10:06 +1200 Subject: [PATCH 11/19] add tests --- .../apiV2/authenticate/authenticate.test.js | 78 ++++++++++++++++++- packages/database/src/modelClasses/User.js | 8 +- 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js index 2c99bd1560..19be129556 100644 --- a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js +++ b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js @@ -5,7 +5,13 @@ import { expect } from 'chai'; -import { encryptPassword, hashAndSaltPassword, getTokenClaims } from '@tupaia/auth'; +import { + encryptPassword, + hashAndSaltPassword, + getTokenClaims, + sha256EncryptPassword, + verifyPassword, +} from '@tupaia/auth'; import { findOrCreateDummyRecord, findOrCreateDummyCountryEntity } from '@tupaia/database'; import { createBasicHeader } from '@tupaia/utils'; @@ -105,4 +111,74 @@ describe('Authenticate', function () { expect(userId).to.equal(userAccount.id); expect(apiClientUserId).to.equal(apiClientUserAccount.id); }); + + it('Should authenticate user who has been migrated to argon2 password hashing', async () => { + const email = 'peeka@pokemon.org'; + const password = 'oldPassword123!'; + const salt = 'xyz123^'; + const sha256Hash = await sha256EncryptPassword(password, salt); + const argon2Hash = await encryptPassword(sha256Hash, salt); + const migratedUser = await findOrCreateDummyRecord(models.user, { + first_name: 'Peeka', + last_name: 'Chu', + email: email, + password_hash: argon2Hash, + password_salt: salt, + verified_email: VERIFIED, + }); + + const authResponse = await app.post('auth?grantType=password', { + headers: { + authorization: createBasicHeader(apiClientUserAccount.email, apiClientSecret), + }, + body: { + emailAddress: email, + password: password, + deviceName: 'test_device', + }, + }); + + expect(authResponse.status).to.equal(200); + const { accessToken, refreshToken, user: userDetails } = authResponse.body; + expect(accessToken).to.be.a('string'); + expect(refreshToken).to.be.a('string'); + expect(userDetails.id).to.equal(migratedUser.id); + expect(userDetails.email).to.equal(migratedUser.email); + }); + + it("Should migrate user's password to argon2 after successful login", async () => { + const email = 'peeka@pokemon.org'; + const password = 'oldPassword123!'; + const salt = 'xyz123^'; + const sha256Hash = await sha256EncryptPassword(password, salt); + const argon2Hash = await encryptPassword(sha256Hash, salt); + const migratedUser = await findOrCreateDummyRecord(models.user, { + first_name: 'Peeka', + last_name: 'Chu', + email: email, + password_hash: argon2Hash, + password_salt: salt, + verified_email: VERIFIED, + }); + + const isVerifiedBefore = await verifyPassword(password, salt, migratedUser.password_hash); + expect(isVerifiedBefore).to.be.false; + + const authResponse = await app.post('auth?grantType=password', { + headers: { + authorization: createBasicHeader(apiClientUserAccount.email, apiClientSecret), + }, + body: { + emailAddress: email, + password: password, + deviceName: 'test_device', + }, + }); + + expect(authResponse.status).to.equal(200); + + const userDetails = await models.user.findById(migratedUser.id); + const isVerifiedAfter = await verifyPassword(password, salt, userDetails.password_hash); + expect(isVerifiedAfter).to.be.true; + }); }); diff --git a/packages/database/src/modelClasses/User.js b/packages/database/src/modelClasses/User.js index 5d3470eee4..4dee8d2f51 100644 --- a/packages/database/src/modelClasses/User.js +++ b/packages/database/src/modelClasses/User.js @@ -19,7 +19,13 @@ export class UserRecord extends DatabaseRecord { return userFullName; } - // Checks if the provided non-encrypted password corresponds to this user + /** + * Attempts to verify the password using argon2, if that fails, it tries to verify the password + * using sha256 plus argon2. If the password is verified using sha256, the password is moved to + * argon2. + * @param password {string} + * @returns {Promise} + */ async checkPassword(password) { const salt = this.password_salt; const hash = this.password_hash; From 0d0e7dd5a4ae628d42c5a8ece59deeb4721ab8ce Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Fri, 6 Sep 2024 16:13:09 +1200 Subject: [PATCH 12/19] Update 20240902224836-argon2-passwords-modifies-data.js --- ...0902224836-argon2-passwords-modifies-data.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js index 9e3d8616d3..1d6b6360b8 100644 --- a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js +++ b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js @@ -26,13 +26,16 @@ exports.up = async function (db) { `); const users = await db.runSql('SELECT id, password_hash_old, password_salt FROM user_account'); - let count = 0; - for (let user of users.rows) { - const { id, password_hash_old, password_salt } = user; - const hashedValue = await hash(`${password_hash_old}${password_salt}`); - await db.runSql('UPDATE user_account SET password_hash = $1 WHERE id = $2', [hashedValue, id]); - count++; - } + await Promise.all( + users.rows.map(async user => { + const { id, password_hash_old, password_salt } = user; + const hashedValue = await hash(`${password_hash_old}${password_salt}`); + await db.runSql('UPDATE user_account SET password_hash = $1 WHERE id = $2', [ + hashedValue, + id, + ]); + }), + ); }; exports.down = function (db) { From 81bee7d7db06f9e3c85cff92d746cc78daf735ab Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Mon, 9 Sep 2024 11:17:24 +1200 Subject: [PATCH 13/19] Update 20240902224836-argon2-passwords-modifies-data.js --- .../20240902224836-argon2-passwords-modifies-data.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js index 1d6b6360b8..5e9d4da089 100644 --- a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js +++ b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js @@ -20,9 +20,10 @@ exports.up = async function (db) { await db.runSql(` ALTER TABLE user_account RENAME COLUMN password_hash TO password_hash_old; - + ALTER TABLE user_account ALTER COLUMN password_hash_old DROP NOT NULL; + ALTER TABLE user_account - ADD COLUMN password_hash TEXT; + ADD COLUMN password_hash TEXT NOT NULL; `); const users = await db.runSql('SELECT id, password_hash_old, password_salt FROM user_account'); From 56d6498f954a8de93d9ca47e9b967055805ec0b8 Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Mon, 9 Sep 2024 12:16:07 +1200 Subject: [PATCH 14/19] fix test --- .../src/tests/apiV2/authenticate/authenticate.test.js | 4 ++-- .../20240902224836-argon2-passwords-modifies-data.js | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js index 19be129556..859a1766cb 100644 --- a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js +++ b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js @@ -146,8 +146,8 @@ describe('Authenticate', function () { expect(userDetails.email).to.equal(migratedUser.email); }); - it("Should migrate user's password to argon2 after successful login", async () => { - const email = 'peeka@pokemon.org'; + it.only("Should migrate user's password to argon2 after successful login", async () => { + const email = 'squirtle@pokemon.org'; const password = 'oldPassword123!'; const salt = 'xyz123^'; const sha256Hash = await sha256EncryptPassword(password, salt); diff --git a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js index 5e9d4da089..51c0636e28 100644 --- a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js +++ b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js @@ -23,7 +23,7 @@ exports.up = async function (db) { ALTER TABLE user_account ALTER COLUMN password_hash_old DROP NOT NULL; ALTER TABLE user_account - ADD COLUMN password_hash TEXT NOT NULL; + ADD COLUMN password_hash TEXT; `); const users = await db.runSql('SELECT id, password_hash_old, password_salt FROM user_account'); @@ -37,6 +37,10 @@ exports.up = async function (db) { ]); }), ); + + await db.runSql(` + ALTER TABLE user_account ALTER COLUMN password_hash SET NOT NULL; + `); }; exports.down = function (db) { From fcbd3d891ed3d887be5245c3a1c0f5c07cbd3a31 Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Mon, 9 Sep 2024 13:32:56 +1200 Subject: [PATCH 15/19] Update authenticate.test.js --- .../src/tests/apiV2/authenticate/authenticate.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js index 859a1766cb..5bceb4ad5b 100644 --- a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js +++ b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js @@ -146,7 +146,7 @@ describe('Authenticate', function () { expect(userDetails.email).to.equal(migratedUser.email); }); - it.only("Should migrate user's password to argon2 after successful login", async () => { + it("Should migrate user's password to argon2 after successful login", async () => { const email = 'squirtle@pokemon.org'; const password = 'oldPassword123!'; const salt = 'xyz123^'; From b5c929df18c3c24f4a5e517ab67809fbbba4727f Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Thu, 3 Oct 2024 14:54:20 +1300 Subject: [PATCH 16/19] remove salt param from password encryption --- packages/auth/src/Authenticator.js | 6 +---- packages/auth/src/index.js | 7 +----- packages/auth/src/passwordEncryption.js | 22 ++++------------- .../src/apiV2/changePassword.js | 6 ++--- .../src/apiV2/import/importUsers.js | 7 +++--- .../src/apiV2/middleware/auth/clientAuth.js | 2 +- .../apiV2/userAccounts/CreateUserAccounts.js | 14 +++++------ .../apiV2/userAccounts/EditUserAccounts.js | 8 +++---- .../src/apiV2/utilities/emailVerification.js | 12 ++++------ .../src/dataAccessors/createUser.js | 14 +++++------ .../apiV2/authenticate/authenticate.test.js | 24 ++++++++----------- .../src/tests/apiV2/verifyEmail.test.js | 9 ++++--- .../database/addBaselineTestData.js | 8 +++---- ...02224836-argon2-passwords-modifies-data.js | 7 +++--- packages/database/src/modelClasses/User.js | 6 ++--- .../src/__tests__/testUtilities/setup.ts | 9 ++++--- .../src/__tests__/utilities/setupTestUser.ts | 8 +++---- .../__integration__/testUtilities/setup.ts | 6 ++--- .../src/utils/initialiseApiClient.ts | 13 ++-------- .../src/__tests__/testUtilities/setup.ts | 13 +++++----- .../src/authSession/getUserFromAuthHeader.js | 11 ++++----- 21 files changed, 84 insertions(+), 128 deletions(-) diff --git a/packages/auth/src/Authenticator.js b/packages/auth/src/Authenticator.js index ad7e612373..eabf9f8c0b 100644 --- a/packages/auth/src/Authenticator.js +++ b/packages/auth/src/Authenticator.js @@ -50,11 +50,7 @@ export class Authenticator { const apiClient = await this.models.apiClient.findOne({ username, }); - const verified = await verifyPassword( - secretKey, - process.env.API_CLIENT_SALT, - apiClient.secret_key_hash, - ); + const verified = await verifyPassword(secretKey, apiClient.secret_key_hash); if (!verified) { throw new UnauthenticatedError('Could not authenticate Api Client'); } diff --git a/packages/auth/src/index.js b/packages/auth/src/index.js index 0402e2f8d8..c7c1e86b4a 100644 --- a/packages/auth/src/index.js +++ b/packages/auth/src/index.js @@ -4,12 +4,7 @@ */ export { Authenticator } from './Authenticator'; export { AccessPolicyBuilder } from './AccessPolicyBuilder'; -export { - encryptPassword, - verifyPassword, - hashAndSaltPassword, - sha256EncryptPassword, -} from './passwordEncryption'; +export { encryptPassword, verifyPassword, sha256EncryptPassword } from './passwordEncryption'; export { getJwtToken, extractRefreshTokenFromReq, generateSecretKey } from './security'; export { getTokenClaimsFromBearerAuth, diff --git a/packages/auth/src/passwordEncryption.js b/packages/auth/src/passwordEncryption.js index e04c8eaecf..e88f851811 100644 --- a/packages/auth/src/passwordEncryption.js +++ b/packages/auth/src/passwordEncryption.js @@ -2,40 +2,26 @@ * Tupaia * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ -import { randomBytes } from 'crypto'; import { hash, verify } from '@node-rs/argon2'; import sha256 from 'sha256'; /** * Helper function to encrypt passwords using argon2 * @param password {string} - * @param salt {string} * @returns {Promise} */ -export function encryptPassword(password, salt) { - return hash(`${password}${salt}`); +export function encryptPassword(password) { + return hash(password); } /** * Helper function to verify passwords using argon2 * @param password - * @param salt {string} * @param hash {string} * @returns {Promise} */ -export async function verifyPassword(password, salt, hash) { - return verify(hash, `${password}${salt}`); -} - -/** - * Helper function to hash and salt passwords using argon2 - * @param password {string} - * @returns {Promise<{password_salt: string, password_hash: string}>} - */ -export async function hashAndSaltPassword(password) { - const salt = randomBytes(16).toString('base64'); // Generate a random salt - const encryptedPassword = await encryptPassword(password, salt); - return { password_hash: encryptedPassword, password_salt: salt }; +export async function verifyPassword(password, hash) { + return verify(hash, password); } /** diff --git a/packages/central-server/src/apiV2/changePassword.js b/packages/central-server/src/apiV2/changePassword.js index e7c1128c14..f3e1ccf6f9 100644 --- a/packages/central-server/src/apiV2/changePassword.js +++ b/packages/central-server/src/apiV2/changePassword.js @@ -3,7 +3,7 @@ * Copyright (c) 2017 Beyond Essential Systems Pty Ltd */ import { DatabaseError, FormValidationError, isValidPassword, respond } from '@tupaia/utils'; -import { hashAndSaltPassword } from '@tupaia/auth'; +import { encryptPassword } from '@tupaia/auth'; import { allowNoPermissions } from '../permissions'; export async function changePassword(req, res, next) { @@ -53,9 +53,9 @@ export async function changePassword(req, res, next) { throw new FormValidationError(error.message, ['password', 'passwordConfirm']); } - const passwordAndSalt = await hashAndSaltPassword(passwordParam); + const newPasswordHash = await encryptPassword(passwordParam); await models.user.updateById(userId, { - ...passwordAndSalt, + password_hash: newPasswordHash, }); respond(res, { message: 'Password successfully updated' }); diff --git a/packages/central-server/src/apiV2/import/importUsers.js b/packages/central-server/src/apiV2/import/importUsers.js index fd4aefa12e..0dff588711 100644 --- a/packages/central-server/src/apiV2/import/importUsers.js +++ b/packages/central-server/src/apiV2/import/importUsers.js @@ -14,10 +14,9 @@ import { constructIsOneOf, constructIsEmptyOr, } from '@tupaia/utils'; -import { hashAndSaltPassword } from '@tupaia/auth'; +import { encryptPassword } from '@tupaia/auth'; import { VerifiedEmail } from '@tupaia/types'; import { - TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, assertAdminPanelAccessToCountry, assertAnyPermissions, assertBESAdminAccess, @@ -53,11 +52,11 @@ export async function importUsers(req, res) { } emails.push(userObject.email); const { password, permission_group: permissionGroupName, ...restOfUser } = userObject; - const passwordAndSalt = await hashAndSaltPassword(password); + const newPasswordHash = await encryptPassword(password); const userToUpsert = { ...restOfUser, - ...passwordAndSalt, + password_hash: newPasswordHash, }; const user = await transactingModels.user.updateOrCreate( { email: userObject.email }, diff --git a/packages/central-server/src/apiV2/middleware/auth/clientAuth.js b/packages/central-server/src/apiV2/middleware/auth/clientAuth.js index d464dfe1dd..3feaf69a69 100644 --- a/packages/central-server/src/apiV2/middleware/auth/clientAuth.js +++ b/packages/central-server/src/apiV2/middleware/auth/clientAuth.js @@ -17,7 +17,7 @@ export async function getAPIClientUser(authHeader, models) { const apiClient = await models.apiClient.findOne({ username, }); - const verified = await verifyPassword(secretKey, API_CLIENT_SALT, apiClient.secret_key_hash); + const verified = await verifyPassword(secretKey, apiClient.secret_key_hash); if (!verified) { throw new UnauthenticatedError('Incorrect client username or secret'); } diff --git a/packages/central-server/src/apiV2/userAccounts/CreateUserAccounts.js b/packages/central-server/src/apiV2/userAccounts/CreateUserAccounts.js index a3dac06784..bb0998274b 100644 --- a/packages/central-server/src/apiV2/userAccounts/CreateUserAccounts.js +++ b/packages/central-server/src/apiV2/userAccounts/CreateUserAccounts.js @@ -1,17 +1,15 @@ -/** +/* * Tupaia - * Copyright (c) 2017 - 2020 Beyond Essential Systems Pty Ltd + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ -import { hashAndSaltPassword, encryptPassword, generateSecretKey } from '@tupaia/auth'; +import { encryptPassword, generateSecretKey } from '@tupaia/auth'; import { CreateHandler } from '../CreateHandler'; import { - TUPAIA_ADMIN_PANEL_PERMISSION_GROUP, assertAdminPanelAccess, assertAdminPanelAccessToCountry, assertAnyPermissions, assertBESAdminAccess, - hasTupaiaAdminPanelAccessToCountry, } from '../../permissions'; /** @@ -46,7 +44,7 @@ export class CreateUserAccounts extends CreateHandler { await transactingModels.apiClient.create({ username: user.email, user_account_id: user.id, - secret_key_hash: await encryptPassword(secretKey, process.env.API_CLIENT_SALT), + secret_key_hash: await encryptPassword(secretKey), }); } @@ -103,7 +101,7 @@ export class CreateUserAccounts extends CreateHandler { ...restOfUser }, ) { - const passwordAndSalt = await hashAndSaltPassword(password); + const passwordHash = await encryptPassword(password); return transactingModels.user.create({ first_name: firstName, @@ -111,7 +109,7 @@ export class CreateUserAccounts extends CreateHandler { email: emailAddress, mobile_number: contactNumber, primary_platform: primaryPlatform, - ...passwordAndSalt, + password_hash: passwordHash, verified_email: verifiedEmail, ...restOfUser, }); diff --git a/packages/central-server/src/apiV2/userAccounts/EditUserAccounts.js b/packages/central-server/src/apiV2/userAccounts/EditUserAccounts.js index a6cc7d9ffc..ddc2658d53 100644 --- a/packages/central-server/src/apiV2/userAccounts/EditUserAccounts.js +++ b/packages/central-server/src/apiV2/userAccounts/EditUserAccounts.js @@ -1,9 +1,9 @@ -/** +/* * Tupaia - * Copyright (c) 2017 - 2020 Beyond Essential Systems Pty Ltd + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ -import { hashAndSaltPassword } from '@tupaia/auth'; +import { encryptPassword } from '@tupaia/auth'; import { S3Client, S3 } from '@tupaia/server-utils'; import { EditHandler } from '../EditHandler'; import { @@ -49,7 +49,7 @@ export class EditUserAccounts extends EditHandler { ...restOfUpdatedFields } = this.updatedFields; let updatedFields = restOfUpdatedFields; - const passwordAndSalt = await hashAndSaltPassword(password); + const passwordAndSalt = await encryptPassword(password); if (password) { updatedFields = { diff --git a/packages/central-server/src/apiV2/utilities/emailVerification.js b/packages/central-server/src/apiV2/utilities/emailVerification.js index d1189755fa..8080bcf26f 100644 --- a/packages/central-server/src/apiV2/utilities/emailVerification.js +++ b/packages/central-server/src/apiV2/utilities/emailVerification.js @@ -1,6 +1,6 @@ /* * Tupaia - * Copyright (c) 2017 - 2021 Beyond Essential Systems Pty Ltd + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ import { encryptPassword, verifyPassword } from '@tupaia/auth'; @@ -23,8 +23,10 @@ const EMAILS = { }, }; +const getEmailVerificationToken = user => `${user.email}${user.password_hash}`; + export const sendEmailVerification = async user => { - const token = await encryptPassword(`${user.email}${user.password_hash}`, user.password_salt); + const token = await encryptPassword(getEmailVerificationToken(user)); const platform = user.primary_platform ? user.primary_platform : 'tupaia'; const { subject, signOff, platformName } = EMAILS[platform]; const TUPAIA_FRONT_END_URL = requireEnv('TUPAIA_FRONT_END_URL'); @@ -60,11 +62,7 @@ export const verifyEmailHelper = async (models, searchCondition, token) => { }); for (const user of users) { - const verified = await verifyPassword( - `${user.email}${user.password_hash}`, - user.password_salt, - token, - ); + const verified = await verifyPassword(getEmailVerificationToken(user), token); if (verified) { return user; diff --git a/packages/central-server/src/dataAccessors/createUser.js b/packages/central-server/src/dataAccessors/createUser.js index 93f2dfe931..b5fb217b86 100644 --- a/packages/central-server/src/dataAccessors/createUser.js +++ b/packages/central-server/src/dataAccessors/createUser.js @@ -1,10 +1,10 @@ -/** - * Tupaia MediTrak - * Copyright (c) 2017 Beyond Essential Systems Pty Ltd +/* + * Tupaia + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ import { DatabaseError } from '@tupaia/utils'; -import { hashAndSaltPassword, encryptPassword, generateSecretKey } from '@tupaia/auth'; +import { encryptPassword, generateSecretKey } from '@tupaia/auth'; export const createUser = async ( models, @@ -39,14 +39,14 @@ export const createUser = async ( throw new Error(`No such country: ${countryName}`); } - const passwordAndSalt = await hashAndSaltPassword(password); + const newPasswordHash = await encryptPassword(password); const user = await transactingModels.user.create({ first_name: firstName, last_name: lastName, email: emailAddress, mobile_number: contactNumber, - ...passwordAndSalt, + password_hash: newPasswordHash, verified_email: verifiedEmail, ...restOfUser, }); @@ -63,7 +63,7 @@ export const createUser = async ( await transactingModels.apiClient.create({ username: user.email, user_account_id: user.id, - secret_key_hash: await encryptPassword(secretKey, process.env.API_CLIENT_SALT), + secret_key_hash: await encryptPassword(secretKey), }); } diff --git a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js index 5bceb4ad5b..d4a2b0aa45 100644 --- a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js +++ b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js @@ -1,13 +1,10 @@ -/** - * Tupaia MediTrak - * Copyright (c) 2017 Beyond Essential Systems Pty Ltd +/* + * Tupaia + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ - import { expect } from 'chai'; - import { encryptPassword, - hashAndSaltPassword, getTokenClaims, sha256EncryptPassword, verifyPassword, @@ -45,13 +42,13 @@ describe('Authenticate', function () { }); // Create test users - const passwordAndSalt = await hashAndSaltPassword(userAccountPassword); + const passwordHash = await encryptPassword(userAccountPassword); userAccount = await findOrCreateDummyRecord(models.user, { first_name: 'Ash', last_name: 'Ketchum', email: 'ash-ketchum@pokemon.org', - ...passwordAndSalt, + password_hash: passwordHash, verified_email: VERIFIED, }); @@ -64,7 +61,7 @@ describe('Authenticate', function () { await findOrCreateDummyRecord(models.apiClient, { username: apiClientUserAccount.email, user_account_id: apiClientUserAccount.id, - secret_key_hash: await encryptPassword(apiClientSecret, process.env.API_CLIENT_SALT), + secret_key_hash: await encryptPassword(apiClientSecret), }); // Public Demo Land Permission @@ -117,13 +114,12 @@ describe('Authenticate', function () { const password = 'oldPassword123!'; const salt = 'xyz123^'; const sha256Hash = await sha256EncryptPassword(password, salt); - const argon2Hash = await encryptPassword(sha256Hash, salt); + const argon2Hash = await encryptPassword(sha256Hash); const migratedUser = await findOrCreateDummyRecord(models.user, { first_name: 'Peeka', last_name: 'Chu', email: email, password_hash: argon2Hash, - password_salt: salt, verified_email: VERIFIED, }); @@ -146,7 +142,7 @@ describe('Authenticate', function () { expect(userDetails.email).to.equal(migratedUser.email); }); - it("Should migrate user's password to argon2 after successful login", async () => { + it.only("Should migrate user's password to argon2 after successful login", async () => { const email = 'squirtle@pokemon.org'; const password = 'oldPassword123!'; const salt = 'xyz123^'; @@ -161,7 +157,7 @@ describe('Authenticate', function () { verified_email: VERIFIED, }); - const isVerifiedBefore = await verifyPassword(password, salt, migratedUser.password_hash); + const isVerifiedBefore = await verifyPassword(password, migratedUser.password_hash); expect(isVerifiedBefore).to.be.false; const authResponse = await app.post('auth?grantType=password', { @@ -178,7 +174,7 @@ describe('Authenticate', function () { expect(authResponse.status).to.equal(200); const userDetails = await models.user.findById(migratedUser.id); - const isVerifiedAfter = await verifyPassword(password, salt, userDetails.password_hash); + const isVerifiedAfter = await verifyPassword(password, userDetails.password_hash); expect(isVerifiedAfter).to.be.true; }); }); diff --git a/packages/central-server/src/tests/apiV2/verifyEmail.test.js b/packages/central-server/src/tests/apiV2/verifyEmail.test.js index 8f1fdd29c5..e6fae77da0 100644 --- a/packages/central-server/src/tests/apiV2/verifyEmail.test.js +++ b/packages/central-server/src/tests/apiV2/verifyEmail.test.js @@ -1,9 +1,8 @@ -/** - * Tupaia MediTrak - * Copyright (c) 2017 Beyond Essential Systems Pty Ltd +/* + * Tupaia + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ import { expect } from 'chai'; - import { encryptPassword } from '@tupaia/auth'; import { randomEmail } from '@tupaia/utils'; import { getAuthorizationHeader, TestableApp } from '../testUtilities'; @@ -43,7 +42,7 @@ describe('Verify Email', () => { const verifyEmail = async userId => { const user = await models.user.findById(userId); - const token = await encryptPassword(`${user.email}${user.password_hash}`, user.password_salt); + const token = await encryptPassword(`${user.email}${user.password_hash}`); return app.post('auth/verifyEmail', { headers, diff --git a/packages/central-server/src/tests/testUtilities/database/addBaselineTestData.js b/packages/central-server/src/tests/testUtilities/database/addBaselineTestData.js index ea93bcd65f..567134b592 100644 --- a/packages/central-server/src/tests/testUtilities/database/addBaselineTestData.js +++ b/packages/central-server/src/tests/testUtilities/database/addBaselineTestData.js @@ -1,6 +1,6 @@ -/** - * Tupaia MediTrak - * Copyright (c) 2019 Beyond Essential Systems Pty Ltd +/* + * Tupaia + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ import { encryptPassword } from '@tupaia/auth'; @@ -90,7 +90,7 @@ export async function addBaselineTestData() { }, { user_account_id: apiUser.userId, - secret_key_hash: await encryptPassword(TEST_API_USER_PASSWORD, process.env.API_CLIENT_SALT), + secret_key_hash: await encryptPassword(TEST_API_USER_PASSWORD), }, ); } diff --git a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js index 51c0636e28..eec0c24503 100644 --- a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js +++ b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js @@ -21,16 +21,17 @@ exports.up = async function (db) { ALTER TABLE user_account RENAME COLUMN password_hash TO password_hash_old; ALTER TABLE user_account ALTER COLUMN password_hash_old DROP NOT NULL; + ALTER TABLE user_account ALTER COLUMN password_salt DROP NOT NULL; ALTER TABLE user_account ADD COLUMN password_hash TEXT; `); - const users = await db.runSql('SELECT id, password_hash_old, password_salt FROM user_account'); + const users = await db.runSql('SELECT id, password_hash_old FROM user_account'); await Promise.all( users.rows.map(async user => { - const { id, password_hash_old, password_salt } = user; - const hashedValue = await hash(`${password_hash_old}${password_salt}`); + const { id, password_hash_old } = user; + const hashedValue = await hash(password_hash_old); await db.runSql('UPDATE user_account SET password_hash = $1 WHERE id = $2', [ hashedValue, id, diff --git a/packages/database/src/modelClasses/User.js b/packages/database/src/modelClasses/User.js index e96278372c..5291427e1a 100644 --- a/packages/database/src/modelClasses/User.js +++ b/packages/database/src/modelClasses/User.js @@ -31,17 +31,17 @@ export class UserRecord extends DatabaseRecord { const hash = this.password_hash; // Try to verify password using argon2 directly - const isVerified = await verifyPassword(password, this.password_salt, this.password_hash); + const isVerified = await verifyPassword(password, this.password_hash); if (isVerified) { return true; } // Try to verify password using sha256 plus argon2 const hashedUserInput = sha256EncryptPassword(password, salt); - const isVerifiedSha256 = await verify(hash, `${hashedUserInput}${salt}`); + const isVerifiedSha256 = await verify(hash, hashedUserInput); if (isVerifiedSha256) { // Move password to argon2 - const encryptedPassword = await encryptPassword(password, salt); + const encryptedPassword = await encryptPassword(password); await this.model.updateById(this.id, { password_hash: encryptedPassword }); return true; } diff --git a/packages/entity-server/src/__tests__/testUtilities/setup.ts b/packages/entity-server/src/__tests__/testUtilities/setup.ts index 42194c66b1..8cc482418b 100644 --- a/packages/entity-server/src/__tests__/testUtilities/setup.ts +++ b/packages/entity-server/src/__tests__/testUtilities/setup.ts @@ -1,11 +1,10 @@ /** * Tupaia - * Copyright (c) 2017 - 2021 Beyond Essential Systems Pty Ltd + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ -import { hashAndSaltPassword } from '@tupaia/auth'; +import { encryptPassword } from '@tupaia/auth'; import { TestableServer } from '@tupaia/server-boilerplate'; - import { findOrCreateDummyRecord, buildAndInsertProjectsAndHierarchies, @@ -45,7 +44,7 @@ export const setupTestData = async () => { const { VERIFIED } = models.user.emailVerifiedStatuses; - const passwordAndSalt = await hashAndSaltPassword(userAccountPassword); + const passwordHash = await encryptPassword(userAccountPassword); await findOrCreateDummyRecord( models.user, @@ -55,7 +54,7 @@ export const setupTestData = async () => { { first_name: 'Ash', last_name: 'Ketchum', - ...passwordAndSalt, + password_hash: passwordHash, verified_email: VERIFIED, }, ); diff --git a/packages/meditrak-app-server/src/__tests__/utilities/setupTestUser.ts b/packages/meditrak-app-server/src/__tests__/utilities/setupTestUser.ts index 9614507449..4463c9f9d1 100644 --- a/packages/meditrak-app-server/src/__tests__/utilities/setupTestUser.ts +++ b/packages/meditrak-app-server/src/__tests__/utilities/setupTestUser.ts @@ -1,9 +1,9 @@ /** * Tupaia - * Copyright (c) 2017 - 2022 Beyond Essential Systems Pty Ltd + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ -import { hashAndSaltPassword } from '@tupaia/auth'; +import { encryptPassword } from '@tupaia/auth'; import { findOrCreateDummyRecord, getTestModels } from '@tupaia/database'; import { TestModelRegistry } from '../types'; import { CAT_USER } from '../__integration__/fixtures'; @@ -13,7 +13,7 @@ const models = getTestModels() as TestModelRegistry; export const setupTestUser = async () => { const { VERIFIED } = models.user.emailVerifiedStatuses; const { email, firstName, lastName, password } = CAT_USER; - const passwordAndSalt = await hashAndSaltPassword(password); + const passwordHash = await encryptPassword(password); return findOrCreateDummyRecord( models.user, @@ -23,7 +23,7 @@ export const setupTestUser = async () => { { first_name: firstName, last_name: lastName, - ...passwordAndSalt, + password_hash: passwordHash, verified_email: VERIFIED, }, ); diff --git a/packages/report-server/src/__tests__/__integration__/testUtilities/setup.ts b/packages/report-server/src/__tests__/__integration__/testUtilities/setup.ts index 05aca6c9e4..98e387caf0 100644 --- a/packages/report-server/src/__tests__/__integration__/testUtilities/setup.ts +++ b/packages/report-server/src/__tests__/__integration__/testUtilities/setup.ts @@ -2,7 +2,7 @@ * Tupaia * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ -import { hashAndSaltPassword } from '@tupaia/auth'; +import { encryptPassword } from '@tupaia/auth'; import { TestableServer } from '@tupaia/server-boilerplate'; import { getTestModels, getTestDatabase, findOrCreateDummyRecord } from '@tupaia/database'; import { createBasicHeader } from '@tupaia/utils'; @@ -58,7 +58,7 @@ export const setupTestData = async () => { // add user const { VERIFIED } = models.user.emailVerifiedStatuses; - const passwordAndSalt = await hashAndSaltPassword(userAccountPassword); + const passwordHash = await encryptPassword(userAccountPassword); await findOrCreateDummyRecord( models.user, @@ -68,7 +68,7 @@ export const setupTestData = async () => { { first_name: 'Ash', last_name: 'Ketchum', - ...passwordAndSalt, + password_hash: passwordHash, verified_email: VERIFIED, }, ); diff --git a/packages/server-boilerplate/src/utils/initialiseApiClient.ts b/packages/server-boilerplate/src/utils/initialiseApiClient.ts index ea3d9f4ceb..403b3249b5 100644 --- a/packages/server-boilerplate/src/utils/initialiseApiClient.ts +++ b/packages/server-boilerplate/src/utils/initialiseApiClient.ts @@ -14,14 +14,12 @@ const upsertUserAccount = async ({ models, email, password, - salt, }: { models: ServerBoilerplateModelRegistry; email: string; password: string; - salt: string; }): Promise => { - const passwordHash = await encryptPassword(password, salt); + const passwordHash = await encryptPassword(password); const firstName = email; const lastName = 'API Client'; @@ -35,7 +33,6 @@ const upsertUserAccount = async ({ last_name: lastName, email: email, password_hash: passwordHash, - password_salt: salt, }, ); return existingUserAccount.id; @@ -48,7 +45,6 @@ const upsertUserAccount = async ({ last_name: lastName, email: email, password_hash: passwordHash, - password_salt: salt, }); return newId; }; @@ -58,15 +54,13 @@ const upsertApiClient = async ({ userAccountId, username, password, - salt, }: { models: ServerBoilerplateModelRegistry; userAccountId: string; username: string; password: string; - salt: string; }) => { - const secretKeyHash = await encryptPassword(password, salt); + const secretKeyHash = await encryptPassword(password); const existingApiClient = await models.apiClient.findOne({ username: username, @@ -142,21 +136,18 @@ export const initialiseApiClient = async ( ) => { const API_CLIENT_NAME = requireEnv('API_CLIENT_NAME'); const API_CLIENT_PASSWORD = requireEnv('API_CLIENT_PASSWORD'); - const API_CLIENT_SALT = requireEnv('API_CLIENT_SALT'); await models.wrapInTransaction(async (transactingModels: ServerBoilerplateModelRegistry) => { const userAccountId = await upsertUserAccount({ models: transactingModels, email: API_CLIENT_NAME, password: API_CLIENT_PASSWORD, - salt: API_CLIENT_SALT, }); await upsertApiClient({ models: transactingModels, userAccountId, username: API_CLIENT_NAME, password: API_CLIENT_PASSWORD, - salt: API_CLIENT_SALT, }); await upsertPermissions({ models: transactingModels, diff --git a/packages/tupaia-web-server/src/__tests__/testUtilities/setup.ts b/packages/tupaia-web-server/src/__tests__/testUtilities/setup.ts index 5d906dcf60..0d1ee6f0d5 100644 --- a/packages/tupaia-web-server/src/__tests__/testUtilities/setup.ts +++ b/packages/tupaia-web-server/src/__tests__/testUtilities/setup.ts @@ -1,9 +1,9 @@ /** * Tupaia - * Copyright (c) 2017 - 2023 Beyond Essential Systems Pty Ltd + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ -import { hashAndSaltPassword } from '@tupaia/auth'; +import { encryptPassword } from '@tupaia/auth'; import { createBasicHeader, requireEnv } from '@tupaia/utils'; import { TestableServer } from '@tupaia/server-boilerplate'; @@ -13,7 +13,6 @@ import { getTestModels, EntityHierarchyCacher, getTestDatabase, - findOrCreateDummyCountryEntity, } from '@tupaia/database'; import { createApp } from '../../app'; @@ -48,7 +47,7 @@ export const setupTestData = async () => { hierarchyCacher.stopListeningForChanges(); const { VERIFIED } = models.user.emailVerifiedStatuses; - const userAccountPasswordAndSalt = await hashAndSaltPassword(userAccountPassword); + const newPasswordHash = await encryptPassword(userAccountPassword); await findOrCreateDummyRecord( models.user, @@ -58,14 +57,14 @@ export const setupTestData = async () => { { first_name: 'Ash', last_name: 'Ketchum', - ...userAccountPasswordAndSalt, + password_hash: newPasswordHash, verified_email: VERIFIED, }, ); const apiClientEmail = requireEnv('API_CLIENT_NAME'); const apiClientPassword = requireEnv('API_CLIENT_PASSWORD'); - const apiClientPasswordAndSalt = await hashAndSaltPassword(apiClientPassword); + const newApiClientPassword = await encryptPassword(apiClientPassword); const apiClient = await findOrCreateDummyRecord( models.user, @@ -75,7 +74,7 @@ export const setupTestData = async () => { { first_name: 'API', last_name: 'Client', - ...apiClientPasswordAndSalt, + password_hash: newApiClientPassword, verified_email: VERIFIED, }, ); diff --git a/packages/web-config-server/src/authSession/getUserFromAuthHeader.js b/packages/web-config-server/src/authSession/getUserFromAuthHeader.js index 5da6afad97..242873d030 100644 --- a/packages/web-config-server/src/authSession/getUserFromAuthHeader.js +++ b/packages/web-config-server/src/authSession/getUserFromAuthHeader.js @@ -1,6 +1,6 @@ /** * Tupaia - * Copyright (c) 2017 - 2021 Beyond Essential Systems Pty Ltd + * Copyright (c) 2017 - 2024 Beyond Essential Systems Pty Ltd */ import { getUserAndPassFromBasicAuth, @@ -15,11 +15,10 @@ const getApiClientUserFromBasicAuth = async (models, authHeader) => { const apiClient = await models.apiClient.findOne({ username, }); - const verified = await verifyPassword( - secretKey, - process.env.API_CLIENT_SALT, - apiClient.secret_key_hash, - ); + if (!apiClient) { + return undefined; + } + const verified = await verifyPassword(secretKey, apiClient.secret_key_hash); return verified ? apiClient?.getUser() : undefined; }; From 39ce693340f69b6e9ef1f3867662bb55374dd6ac Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Fri, 4 Oct 2024 11:10:06 +1300 Subject: [PATCH 17/19] rename migration --- ...data.js => 20240902224836-argon2-passwords-modifies-schema.js} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename packages/database/src/migrations/{20240902224836-argon2-passwords-modifies-data.js => 20240902224836-argon2-passwords-modifies-schema.js} (100%) diff --git a/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js b/packages/database/src/migrations/20240902224836-argon2-passwords-modifies-schema.js similarity index 100% rename from packages/database/src/migrations/20240902224836-argon2-passwords-modifies-data.js rename to packages/database/src/migrations/20240902224836-argon2-passwords-modifies-schema.js From e5812d9b8b7f60011d468aca2353720089cd14d2 Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Fri, 4 Oct 2024 11:19:08 +1300 Subject: [PATCH 18/19] Update authenticate.test.js --- .../src/tests/apiV2/authenticate/authenticate.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js index d4a2b0aa45..ad7f586679 100644 --- a/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js +++ b/packages/central-server/src/tests/apiV2/authenticate/authenticate.test.js @@ -142,7 +142,7 @@ describe('Authenticate', function () { expect(userDetails.email).to.equal(migratedUser.email); }); - it.only("Should migrate user's password to argon2 after successful login", async () => { + it("Should migrate user's password to argon2 after successful login", async () => { const email = 'squirtle@pokemon.org'; const password = 'oldPassword123!'; const salt = 'xyz123^'; From d429633471d56da75c1315b68e922af1c15fb612 Mon Sep 17 00:00:00 2001 From: Tom Caiger Date: Fri, 4 Oct 2024 11:31:57 +1300 Subject: [PATCH 19/19] update types --- packages/types/src/schemas/schemas.ts | 13 ++++++++++--- packages/types/src/types/models.ts | 9 ++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/types/src/schemas/schemas.ts b/packages/types/src/schemas/schemas.ts index 2e15614c2b..8df5acb115 100644 --- a/packages/types/src/schemas/schemas.ts +++ b/packages/types/src/schemas/schemas.ts @@ -84412,6 +84412,9 @@ export const UserAccountSchema = { "password_hash": { "type": "string" }, + "password_hash_old": { + "type": "string" + }, "password_salt": { "type": "string" }, @@ -84459,7 +84462,6 @@ export const UserAccountSchema = { "email", "id", "password_hash", - "password_salt", "preferences" ] } @@ -84492,6 +84494,9 @@ export const UserAccountCreateSchema = { "password_hash": { "type": "string" }, + "password_hash_old": { + "type": "string" + }, "password_salt": { "type": "string" }, @@ -84537,8 +84542,7 @@ export const UserAccountCreateSchema = { "additionalProperties": false, "required": [ "email", - "password_hash", - "password_salt" + "password_hash" ] } @@ -84573,6 +84577,9 @@ export const UserAccountUpdateSchema = { "password_hash": { "type": "string" }, + "password_hash_old": { + "type": "string" + }, "password_salt": { "type": "string" }, diff --git a/packages/types/src/types/models.ts b/packages/types/src/types/models.ts index 4126baeaf6..deaace6efc 100644 --- a/packages/types/src/types/models.ts +++ b/packages/types/src/types/models.ts @@ -1639,7 +1639,8 @@ export interface UserAccount { 'last_name'?: string | null; 'mobile_number'?: string | null; 'password_hash': string; - 'password_salt': string; + 'password_hash_old'?: string | null; + 'password_salt'?: string | null; 'position'?: string | null; 'preferences': UserAccountPreferences; 'primary_platform'?: PrimaryPlatform | null; @@ -1655,7 +1656,8 @@ export interface UserAccountCreate { 'last_name'?: string | null; 'mobile_number'?: string | null; 'password_hash': string; - 'password_salt': string; + 'password_hash_old'?: string | null; + 'password_salt'?: string | null; 'position'?: string | null; 'preferences'?: UserAccountPreferences; 'primary_platform'?: PrimaryPlatform | null; @@ -1672,7 +1674,8 @@ export interface UserAccountUpdate { 'last_name'?: string | null; 'mobile_number'?: string | null; 'password_hash'?: string; - 'password_salt'?: string; + 'password_hash_old'?: string | null; + 'password_salt'?: string | null; 'position'?: string | null; 'preferences'?: UserAccountPreferences; 'primary_platform'?: PrimaryPlatform | null;