Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(security): RN-1303: Update password storage to use argon2 #5872

Open
wants to merge 24 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/auth/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 8 additions & 5 deletions packages/auth/src/Authenticator.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
/**
* 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';

import { DatabaseError, UnauthenticatedError, UnverifiedError } from '@tupaia/utils';
import { AccessPolicyBuilder } from './AccessPolicyBuilder';
import { mergeAccessPolicies } from './mergeAccessPolicies';
import { encryptPassword } from './utils';
import { verifyPassword } from './passwordEncryption';
import { getTokenClaims } from './userAuth';

const REFRESH_TOKEN_LENGTH = 40;
Expand Down Expand Up @@ -47,12 +47,15 @@ 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,
);
if (!verified) {
throw new UnauthenticatedError('Could not authenticate Api Client');
}
const user = await apiClient.getUser();
Expand Down
10 changes: 7 additions & 3 deletions packages/auth/src/index.js
Original file line number Diff line number Diff line change
@@ -1,11 +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 * from './utils';
export {
encryptPassword,
verifyPassword,
hashAndSaltPassword,
sha256EncryptPassword,
} from './passwordEncryption';
export { getJwtToken, extractRefreshTokenFromReq, generateSecretKey } from './security';
export {
getTokenClaimsFromBearerAuth,
Expand Down
49 changes: 49 additions & 0 deletions packages/auth/src/passwordEncryption.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* 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<string>}
*/
export function encryptPassword(password, salt) {
return hash(`${password}${salt}`);
passcod marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Helper function to verify passwords using argon2
* @param password
* @param salt {string}
* @param hash {string}
* @returns {Promise<boolean>}
*/
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 };
}

/**
* Helper function to encrypt passwords using sha256
* @param password {string}
* @param salt {string}
* @returns {string}
*/
export function sha256EncryptPassword(password, salt) {
return sha256(`${password}${salt}`);
}
23 changes: 0 additions & 23 deletions packages/auth/src/utils.js

This file was deleted.

5 changes: 3 additions & 2 deletions packages/central-server/src/apiV2/changePassword.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
}

Expand All @@ -53,8 +53,9 @@ export async function changePassword(req, res, next) {
throw new FormValidationError(error.message, ['password', 'passwordConfirm']);
}

const passwordAndSalt = await hashAndSaltPassword(passwordParam);
tcaiger marked this conversation as resolved.
Show resolved Hide resolved
await models.user.updateById(userId, {
...hashAndSaltPassword(passwordParam),
...passwordAndSalt,
});

respond(res, { message: 'Password successfully updated' });
Expand Down
4 changes: 3 additions & 1 deletion packages/central-server/src/apiV2/import/importUsers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
12 changes: 5 additions & 7 deletions packages/central-server/src/apiV2/middleware/auth/clientAuth.js
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
});
}

Expand Down Expand Up @@ -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,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
}

Expand Down
17 changes: 14 additions & 3 deletions packages/central-server/src/apiV2/utilities/emailVerification.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -24,7 +24,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, signOff, platformName } = EMAILS[platform];
const TUPAIA_FRONT_END_URL = requireEnv('TUPAIA_FRONT_END_URL');
Expand Down Expand Up @@ -59,5 +59,16 @@ 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 verified = await verifyPassword(
`${user.email}${user.password_hash}`,
user.password_salt,
token,
);
passcod marked this conversation as resolved.
Show resolved Hide resolved

if (verified) {
return user;
}
}
return null;
};
6 changes: 4 additions & 2 deletions packages/central-server/src/dataAccessors/createUser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand All @@ -61,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),
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,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');
}
Loading
Loading