From 114d416de0b5b5f3635503a44ed9e4454102b7f7 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Sun, 25 Dec 2022 15:49:06 +0100 Subject: [PATCH] server: refactor password hashing & comparison to module For easier replacement should the hash algorithm ever be changed. --- packages/backend/src/misc/password.ts | 10 ++++++++++ packages/backend/src/server/api/common/signup.ts | 6 ++---- .../server/api/endpoints/admin/reset-password.ts | 13 ++++--------- .../src/server/api/endpoints/i/2fa/key-done.ts | 7 ++----- .../src/server/api/endpoints/i/2fa/register-key.ts | 7 ++----- .../src/server/api/endpoints/i/2fa/register.ts | 7 ++----- .../src/server/api/endpoints/i/2fa/remove-key.ts | 7 ++----- .../src/server/api/endpoints/i/2fa/unregister.ts | 7 ++----- .../src/server/api/endpoints/i/change-password.ts | 13 +++---------- .../src/server/api/endpoints/i/delete-account.ts | 7 ++----- .../src/server/api/endpoints/i/regenerate-token.ts | 7 ++----- .../src/server/api/endpoints/i/update-email.ts | 9 ++++----- .../src/server/api/endpoints/reset-password.ts | 8 ++------ packages/backend/src/server/api/private/signin.ts | 4 ++-- packages/backend/src/server/api/private/signup.ts | 8 ++------ packages/backend/src/services/create-system-user.ts | 10 +++------- 16 files changed, 46 insertions(+), 84 deletions(-) create mode 100644 packages/backend/src/misc/password.ts diff --git a/packages/backend/src/misc/password.ts b/packages/backend/src/misc/password.ts new file mode 100644 index 000000000..4e5a3d1be --- /dev/null +++ b/packages/backend/src/misc/password.ts @@ -0,0 +1,10 @@ +import bcrypt from 'bcryptjs'; + +export async function hashPassword(password: string): Promise { + const salt = await bcrypt.genSalt(8); + return await bcrypt.hash(password, salt); +} + +export async function comparePassword(password: string, hash: string): Promise { + return await bcrypt.compare(password, hash); +} diff --git a/packages/backend/src/server/api/common/signup.ts b/packages/backend/src/server/api/common/signup.ts index 150d27218..7ae1c09b0 100644 --- a/packages/backend/src/server/api/common/signup.ts +++ b/packages/backend/src/server/api/common/signup.ts @@ -1,11 +1,11 @@ import { generateKeyPair } from 'node:crypto'; -import bcrypt from 'bcryptjs'; import { IsNull } from 'typeorm'; import { User } from '@/models/entities/user.js'; import { Users, UsedUsernames } from '@/models/index.js'; import { UserProfile } from '@/models/entities/user-profile.js'; import { genId } from '@/misc/gen-id.js'; import { toPunyNullable } from '@/misc/convert-host.js'; +import { hashPassword } from '@/misc/password.js'; import { UserKeypair } from '@/models/entities/user-keypair.js'; import { usersChart } from '@/services/chart/index.js'; import { UsedUsername } from '@/models/entities/used-username.js'; @@ -33,9 +33,7 @@ export async function signup(opts: { throw new ApiError('INVALID_PASSWORD'); } - // Generate hash of password - const salt = await bcrypt.genSalt(8); - hash = await bcrypt.hash(password, salt); + hash = await hashPassword(password); } // Generate secret diff --git a/packages/backend/src/server/api/endpoints/admin/reset-password.ts b/packages/backend/src/server/api/endpoints/admin/reset-password.ts index 4a54a96e3..5561b1507 100644 --- a/packages/backend/src/server/api/endpoints/admin/reset-password.ts +++ b/packages/backend/src/server/api/endpoints/admin/reset-password.ts @@ -1,4 +1,4 @@ -import bcrypt from 'bcryptjs'; +import { hashPassword } from '@/misc/password.js'; import { secureRndstr } from '@/misc/secure-rndstr.js'; import { Users, UserProfiles } from '@/models/index.js'; import { ApiError } from '@/server/api/error.js'; @@ -17,8 +17,6 @@ export const meta = { password: { type: 'string', optional: false, nullable: false, - minLength: 8, - maxLength: 8, }, }, }, @@ -46,18 +44,15 @@ export default define(meta, paramDef, async (ps) => { throw new ApiError('IS_ADMIN'); } - const passwd = secureRndstr(8, true); - - // Generate hash of password - const hash = bcrypt.hashSync(passwd); + const password = secureRndstr(8, true); await UserProfiles.update({ userId: user.id, }, { - password: hash, + password: await hashPassword(password), }); return { - password: passwd, + password, }; }); diff --git a/packages/backend/src/server/api/endpoints/i/2fa/key-done.ts b/packages/backend/src/server/api/endpoints/i/2fa/key-done.ts index 809efe814..73773c8c5 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/key-done.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/key-done.ts @@ -1,7 +1,7 @@ import { promisify } from 'node:util'; -import bcrypt from 'bcryptjs'; import * as cbor from 'cbor'; import { MINUTE } from '@/const.js'; +import { comparePassword } from '@/misc/password.js'; import { UserProfiles, UserSecurityKeys, @@ -41,10 +41,7 @@ export const paramDef = { export default define(meta, paramDef, async (ps, user) => { const profile = await UserProfiles.findOneByOrFail({ userId: user.id }); - // Compare password - const same = await bcrypt.compare(ps.password, profile.password!); - - if (!same) { + if (!(await comparePassword(ps.password, profile.password!))) { throw new ApiError('ACCESS_DENIED'); } diff --git a/packages/backend/src/server/api/endpoints/i/2fa/register-key.ts b/packages/backend/src/server/api/endpoints/i/2fa/register-key.ts index 65dd4804f..32e85463c 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/register-key.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/register-key.ts @@ -1,8 +1,8 @@ import { promisify } from 'node:util'; import * as crypto from 'node:crypto'; -import bcrypt from 'bcryptjs'; import { UserProfiles, AttestationChallenges } from '@/models/index.js'; import { genId } from '@/misc/gen-id.js'; +import { comparePassword } from '@/misc/password.js'; import { ApiError } from '@/server/api/error.js'; import define from '../../../define.js'; import { hash } from '../../../2fa.js'; @@ -29,10 +29,7 @@ export const paramDef = { export default define(meta, paramDef, async (ps, user) => { const profile = await UserProfiles.findOneByOrFail({ userId: user.id }); - // Compare password - const same = await bcrypt.compare(ps.password, profile.password!); - - if (!same) { + if (!(await comparePassword(ps.password, profile.password!))) { throw new ApiError('ACCESS_DENIED'); } diff --git a/packages/backend/src/server/api/endpoints/i/2fa/register.ts b/packages/backend/src/server/api/endpoints/i/2fa/register.ts index db80a850f..59a7f5d8b 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/register.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/register.ts @@ -1,7 +1,7 @@ -import bcrypt from 'bcryptjs'; import * as speakeasy from 'speakeasy'; import * as QRCode from 'qrcode'; import config from '@/config/index.js'; +import { comparePassword } from '@/misc/password.js'; import { UserProfiles } from '@/models/index.js'; import { ApiError } from '@/server/api/error.js'; import define from '../../../define.js'; @@ -26,10 +26,7 @@ export const paramDef = { export default define(meta, paramDef, async (ps, user) => { const profile = await UserProfiles.findOneByOrFail({ userId: user.id }); - // Compare password - const same = await bcrypt.compare(ps.password, profile.password!); - - if (!same) { + if (!(await comparePassword(ps.password, profile.password!))) { throw new ApiError('ACCESS_DENIED'); } diff --git a/packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts b/packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts index 4e704ab63..4164e114c 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/remove-key.ts @@ -1,4 +1,4 @@ -import bcrypt from 'bcryptjs'; +import { comparePassword } from '@/misc/password.js'; import { UserProfiles, UserSecurityKeys, Users } from '@/models/index.js'; import { publishMainStream } from '@/services/stream.js'; import { ApiError } from '@/server/api/error.js'; @@ -25,10 +25,7 @@ export const paramDef = { export default define(meta, paramDef, async (ps, user) => { const profile = await UserProfiles.findOneByOrFail({ userId: user.id }); - // Compare password - const same = await bcrypt.compare(ps.password, profile.password!); - - if (!same) { + if (!(await comparePassword(ps.password, profile.password!))) { throw new ApiError('ACCESS_DENIED'); } diff --git a/packages/backend/src/server/api/endpoints/i/2fa/unregister.ts b/packages/backend/src/server/api/endpoints/i/2fa/unregister.ts index d9730bde9..c82fde0b3 100644 --- a/packages/backend/src/server/api/endpoints/i/2fa/unregister.ts +++ b/packages/backend/src/server/api/endpoints/i/2fa/unregister.ts @@ -1,4 +1,4 @@ -import bcrypt from 'bcryptjs'; +import { comparePassword } from '@/misc/password.js'; import { UserProfiles } from '@/models/index.js'; import { ApiError } from '@/server/api/error.js'; import define from '../../../define.js'; @@ -23,10 +23,7 @@ export const paramDef = { export default define(meta, paramDef, async (ps, user) => { const profile = await UserProfiles.findOneByOrFail({ userId: user.id }); - // Compare password - const same = await bcrypt.compare(ps.password, profile.password!); - - if (!same) { + if (!(await comparePassword(ps.password, profile.password!))) { throw new ApiError('ACCESS_DENIED'); } diff --git a/packages/backend/src/server/api/endpoints/i/change-password.ts b/packages/backend/src/server/api/endpoints/i/change-password.ts index 3df7b0237..e7fd70391 100644 --- a/packages/backend/src/server/api/endpoints/i/change-password.ts +++ b/packages/backend/src/server/api/endpoints/i/change-password.ts @@ -1,4 +1,4 @@ -import bcrypt from 'bcryptjs'; +import { comparePassword, hashPassword } from '@/misc/password.js'; import { UserProfiles } from '@/models/index.js'; import { ApiError } from '@/server/api/error.js'; import define from '../../define.js'; @@ -24,18 +24,11 @@ export const paramDef = { export default define(meta, paramDef, async (ps, user) => { const profile = await UserProfiles.findOneByOrFail({ userId: user.id }); - // Compare password - const same = await bcrypt.compare(ps.currentPassword, profile.password!); - - if (!same) { + if (!(await comparePassword(ps.currentPassword, profile.password!))) { throw new ApiError('ACCESS_DENIED'); } - // Generate hash of password - const salt = await bcrypt.genSalt(8); - const hash = await bcrypt.hash(ps.newPassword, salt); - await UserProfiles.update(user.id, { - password: hash, + password: await hashPassword(ps.newPassword), }); }); diff --git a/packages/backend/src/server/api/endpoints/i/delete-account.ts b/packages/backend/src/server/api/endpoints/i/delete-account.ts index 01268373d..8db1455e5 100644 --- a/packages/backend/src/server/api/endpoints/i/delete-account.ts +++ b/packages/backend/src/server/api/endpoints/i/delete-account.ts @@ -1,4 +1,4 @@ -import bcrypt from 'bcryptjs'; +import { comparePassword } from '@/misc/password.js'; import { UserProfiles, Users } from '@/models/index.js'; import { deleteAccount } from '@/services/delete-account.js'; import { ApiError } from '@/server/api/error.js'; @@ -28,10 +28,7 @@ export default define(meta, paramDef, async (ps, user) => { return; } - // Compare password - const same = await bcrypt.compare(ps.password, profile.password!); - - if (!same) { + if (!(await comparePassword(ps.password, profile.password!))) { throw new ApiError('ACCESS_DENIED'); } diff --git a/packages/backend/src/server/api/endpoints/i/regenerate-token.ts b/packages/backend/src/server/api/endpoints/i/regenerate-token.ts index 50bd73f4a..2bec1ed17 100644 --- a/packages/backend/src/server/api/endpoints/i/regenerate-token.ts +++ b/packages/backend/src/server/api/endpoints/i/regenerate-token.ts @@ -1,4 +1,4 @@ -import bcrypt from 'bcryptjs'; +import { comparePassword } from '@/misc/password.js'; import { publishInternalEvent, publishMainStream, publishUserEvent } from '@/services/stream.js'; import { Users, UserProfiles } from '@/models/index.js'; import generateUserToken from '../../common/generate-native-user-token.js'; @@ -28,10 +28,7 @@ export default define(meta, paramDef, async (ps, user) => { const profile = await UserProfiles.findOneByOrFail({ userId: user.id }); - // Compare password - const same = await bcrypt.compare(ps.password, profile.password!); - - if (!same) { + if (!(await comparePassword(ps.password, profile.password!))) { throw new ApiError('ACCESS_DENIED'); } diff --git a/packages/backend/src/server/api/endpoints/i/update-email.ts b/packages/backend/src/server/api/endpoints/i/update-email.ts index 057ad5cf3..2a2413730 100644 --- a/packages/backend/src/server/api/endpoints/i/update-email.ts +++ b/packages/backend/src/server/api/endpoints/i/update-email.ts @@ -1,6 +1,6 @@ -import bcrypt from 'bcryptjs'; import { publishMainStream } from '@/services/stream.js'; import config from '@/config/index.js'; +import { comparePassword } from '@/misc/password.js'; import { secureRndstr } from '@/misc/secure-rndstr.js'; import { Users, UserProfiles } from '@/models/index.js'; import { sendEmail } from '@/services/send-email.js'; @@ -37,10 +37,9 @@ export const paramDef = { export default define(meta, paramDef, async (ps, user) => { const profile = await UserProfiles.findOneByOrFail({ userId: user.id }); - // Compare password - const same = await bcrypt.compare(ps.password, profile.password!); - - if (!same) throw new ApiError('ACCESS_DENIED'); + if (!(await comparePassword(ps.password, profile.password!))) { + throw new ApiError('ACCESS_DENIED'); + } if (ps.email != null) { const available = await validateEmailForAccount(ps.email); diff --git a/packages/backend/src/server/api/endpoints/reset-password.ts b/packages/backend/src/server/api/endpoints/reset-password.ts index 51b26a2b0..c46fcb6fd 100644 --- a/packages/backend/src/server/api/endpoints/reset-password.ts +++ b/packages/backend/src/server/api/endpoints/reset-password.ts @@ -1,4 +1,4 @@ -import bcrypt from 'bcryptjs'; +import { hashPassword } from '@/misc/password.js'; import { UserProfiles, PasswordResetRequests } from '@/models/index.js'; import { DAY, MINUTE } from '@/const.js'; import define from '../define.js'; @@ -43,12 +43,8 @@ export default define(meta, paramDef, async (ps, user) => { throw new ApiError('NO_SUCH_RESET_REQUEST'); } - // Generate hash of password - const salt = await bcrypt.genSalt(8); - const hash = await bcrypt.hash(ps.password, salt); - await UserProfiles.update(req.userId, { - password: hash, + password: await hashPassword(ps.password), }); await PasswordResetRequests.delete(req.id); diff --git a/packages/backend/src/server/api/private/signin.ts b/packages/backend/src/server/api/private/signin.ts index f5cc7af08..6ca957056 100644 --- a/packages/backend/src/server/api/private/signin.ts +++ b/packages/backend/src/server/api/private/signin.ts @@ -1,7 +1,6 @@ import { randomBytes } from 'node:crypto'; import { IsNull } from 'typeorm'; import Koa from 'koa'; -import bcrypt from 'bcryptjs'; import * as speakeasy from 'speakeasy'; import { SECOND, MINUTE, HOUR } from '@/const.js'; import config from '@/config/index.js'; @@ -9,6 +8,7 @@ import { Users, Signins, UserProfiles, UserSecurityKeys, AttestationChallenges } import { ILocalUser } from '@/models/entities/user.js'; import { genId } from '@/misc/gen-id.js'; import { getIpHash } from '@/misc/get-ip-hash.js'; +import { comparePassword } from '@/misc/password.js'; import signin from '../common/signin.js'; import { verifyLogin, hash } from '../2fa.js'; import { limiter } from '../limiter.js'; @@ -67,7 +67,7 @@ export default async (ctx: Koa.Context) => { const profile = await UserProfiles.findOneByOrFail({ userId: user.id }); // Compare password - const same = await bcrypt.compare(password, profile.password!); + const same = await comparePassword(password, profile.password!); async function fail(): void { // Append signin history diff --git a/packages/backend/src/server/api/private/signup.ts b/packages/backend/src/server/api/private/signup.ts index e20fb9abb..fafa96362 100644 --- a/packages/backend/src/server/api/private/signup.ts +++ b/packages/backend/src/server/api/private/signup.ts @@ -1,7 +1,7 @@ import Koa from 'koa'; -import bcrypt from 'bcryptjs'; import { fetchMeta } from '@/misc/fetch-meta.js'; import { verifyHcaptcha, verifyRecaptcha } from '@/misc/captcha.js'; +import { hashPassword } from '@/misc/password.js'; import { Users, RegistrationTickets, UserPendings } from '@/models/index.js'; import config from '@/config/index.js'; import { sendEmail } from '@/services/send-email.js'; @@ -71,17 +71,13 @@ export default async (ctx: Koa.Context) => { if (instance.emailRequiredForSignup) { const code = secureRndstr(16); - // Generate hash of password - const salt = await bcrypt.genSalt(8); - const hash = await bcrypt.hash(password, salt); - await UserPendings.insert({ id: genId(), createdAt: new Date(), code, email: emailAddress, username, - password: hash, + password: await hashPassword(password), }); const link = `${config.url}/signup-complete/${code}`; diff --git a/packages/backend/src/services/create-system-user.ts b/packages/backend/src/services/create-system-user.ts index 2285f1586..f965e82d7 100644 --- a/packages/backend/src/services/create-system-user.ts +++ b/packages/backend/src/services/create-system-user.ts @@ -1,7 +1,7 @@ -import bcrypt from 'bcryptjs'; import { v4 as uuid } from 'uuid'; import { IsNull } from 'typeorm'; import { genRsaKeyPair } from '@/misc/gen-key-pair.js'; +import { hashPassword } from '@/misc/password.js'; import { User } from '@/models/entities/user.js'; import { UserProfile } from '@/models/entities/user-profile.js'; import { genId } from '@/misc/gen-id.js'; @@ -11,11 +11,7 @@ import { db } from '@/db/postgre.js'; import generateNativeUserToken from '@/server/api/common/generate-native-user-token.js'; export async function createSystemUser(username: string): Promise { - const password = uuid(); - - // Generate hash of password - const salt = await bcrypt.genSalt(8); - const hash = await bcrypt.hash(password, salt); + const password = await hashPassword(uuid()); // Generate secret const secret = generateNativeUserToken(); @@ -55,7 +51,7 @@ export async function createSystemUser(username: string): Promise { await transactionalEntityManager.insert(UserProfile, { userId: account.id, autoAcceptFollowed: false, - password: hash, + password, }); await transactionalEntityManager.insert(UsedUsername, {