diff --git a/apps/api/src/modules/auth/auth.service.spec.ts b/apps/api/src/modules/auth/auth.service.spec.ts index e58d604..cd268b8 100644 --- a/apps/api/src/modules/auth/auth.service.spec.ts +++ b/apps/api/src/modules/auth/auth.service.spec.ts @@ -9,7 +9,18 @@ import * as bcrypt from 'bcryptjs'; */ jest.mock('@nestjs/jwt', () => ({ JwtService: class JwtService {} })); -import { AuthService } from './auth.service'; +/** + * The real bcryptjs, with `compare` wrapped so a test can assert it was never + * reached. It calls straight through, so every other test still runs against + * the real implementation. (A namespace import's properties cannot be + * re-defined, so jest.spyOn is not an option here.) + */ +jest.mock('bcryptjs', () => { + const actual = jest.requireActual('bcryptjs'); + return { ...actual, compare: jest.fn(actual.compare) }; +}); + +import { AuthService, compareLocalHash, OTP_ONLY_PASSWORD_HASH } from './auth.service'; import type { AdminSessionService } from '../sessions/admin-session.service'; import { IamInvalidCredentialsError, @@ -258,6 +269,58 @@ describe('AuthService login', () => { ); }); + /* ─────────────── accounts with no usable password ─────────────── */ + + describe('accounts with no usable password', () => { + const noPassword = ['', OTP_ONLY_PASSWORD_HASH]; + + it.each(noPassword)('refuses login for hash %j, whatever is typed', async (hash) => { + const { service } = build(disabledIam(), { ...adminRow(), passwordHash: hash }); + + // Including the empty string and the sentinel itself: neither may open it + for (const typed of ['', 'x', OTP_ONLY_PASSWORD_HASH]) { + await expect( + service.login({ phone: '09120000000', password: typed }), + ).rejects.toThrow('نام کاربری یا رمز عبور اشتباه است'); + } + }); + + it.each(noPassword)('refuses hash %j on the IAM fallback path too', async (hash) => { + const verifyCredentials = jest.fn().mockRejectedValue(new IamUnavailableError('down')); + const { service } = build( + { enabled: true, allowsLocalFallback: true, verifyCredentials }, + { ...adminRow(), passwordHash: hash }, + ); + + await expect( + service.login({ phone: '09120000000', password: '' }), + ).rejects.toBeInstanceOf(UnauthorizedException); + }); + + it('never hands a malformed hash to bcrypt', async () => { + const compare = bcrypt.compare as unknown as jest.Mock; + compare.mockClear(); + + expect(await compareLocalHash('x', OTP_ONLY_PASSWORD_HASH)).toBe(false); + expect(await compareLocalHash('x', '')).toBe(false); + expect(await compareLocalHash('x', null)).toBe(false); + expect(compare).not.toHaveBeenCalled(); + + // …while a real hash still goes through it + await compareLocalHash(PASSWORD, passwordHash); + expect(compare).toHaveBeenCalledTimes(1); + }); + + it("still accepts the legacy server's PHP $2y$ hashes", async () => { + // PHP's $2y$ is the same algorithm as $2b$; the legacy passwords + // (customers and the sales experts moving to AdminUser) use it. + const legacy = '$2y$' + (await bcrypt.hash(PASSWORD, 4)).slice(4); + + expect(await compareLocalHash(PASSWORD, legacy)).toBe(true); + expect(await compareLocalHash('wrong', legacy)).toBe(false); + }); + }); + /* ─────────────── two-factor ─────────────── */ it('explains that a two-factor account cannot log in through this path', async () => { diff --git a/apps/api/src/modules/auth/auth.service.ts b/apps/api/src/modules/auth/auth.service.ts index 7614430..5006a4f 100644 --- a/apps/api/src/modules/auth/auth.service.ts +++ b/apps/api/src/modules/auth/auth.service.ts @@ -11,6 +11,30 @@ import { import { AdminSessionService } from '../sessions/admin-session.service'; import type { LoginDto } from './dto/login.dto'; +/** + * Value written into `passwordHash` for accounts that must never log in with a + * password — the ~1,590 legacy customers who only ever used OTP + * (docs/DATA-MIGRATION-MAP.md §10.4). The `!` prefix is the Django/Rails + * "unusable password" convention: readable by anyone looking at the row, and + * matched by no password. + */ +export const OTP_ONLY_PASSWORD_HASH = '!OTP_ONLY_NO_PASSWORD_LOGIN'; + +/** + * A real bcrypt hash always starts with `$2` ($2a$, $2b$, or PHP's $2y$ from + * the legacy server — all three verify with bcryptjs). Anything else is not a + * password: '' on accounts created without one, or OTP_ONLY_PASSWORD_HASH. + * + * Such hashes are refused here, BEFORE bcrypt is called, so the outcome never + * depends on how a particular bcrypt build treats a malformed hash. bcryptjs 3 + * happens to return false today; a future version or a native binding might + * throw and turn a wrong password into a 500 (owner ruling 2026-10-07). + */ +export async function compareLocalHash(password: string, passwordHash: string | null | undefined) { + if (typeof passwordHash !== 'string' || !passwordHash.startsWith('$2')) return false; + return bcrypt.compare(password, passwordHash); +} + @Injectable() export class AuthService { private readonly logger = new Logger(AuthService.name); @@ -100,7 +124,7 @@ export class AuthService { clientIp?: string, ): Promise<{ ok: boolean; identity?: IamIdentity }> { if (!this.iam.enabled) { - return { ok: await bcrypt.compare(dto.password, passwordHash) }; + return { ok: await compareLocalHash(dto.password, passwordHash) }; } try { @@ -132,7 +156,7 @@ export class AuthService { } // fallback mode: the account may not have been migrated to the IAM yet - return { ok: await bcrypt.compare(dto.password, passwordHash) }; + return { ok: await compareLocalHash(dto.password, passwordHash) }; } }