Fix: Password validation requires data and hash arguments

[sonarly]

Automated fix for bug 5360

Severity: critical

Summary

When an existing user created via OAuth (with no password hash) attempts to sign in through the signInUp password flow, their null passwordHash is passed directly to bcrypt.compare, which throws an unhandled 'data and hash arguments required' error instead of a user-friendly message.

User Impact

Users who originally registered via Google or Microsoft OAuth and then attempt to sign in using email/password through the signInUp flow cannot authenticate. They see a server error instead of a helpful message like 'User was not created with email/password'. This blocks the user from accessing the CRM entirely.

Root Cause

Proximate cause: bcrypt.compare() at auth.util.ts:19 throws 'data and hash arguments required' because the passwordHash argument is null.

1. Why did bcrypt throw? Because compareHash() was called with a null passwordHash value. The compareHash function at auth.util.ts:19 passes its arguments directly to bcrypt.compare without any null check.

2. Why was passwordHash null? Because SignInUpService.validatePassword() at sign-in-up.service.ts:154 received a null passwordHash from AuthService.validatePassword() at auth.service.ts:232. The value userData.existingUser.passwordHash is null for users who were originally created via an OAuth provider (Google, Microsoft) and never set a password.

3. Why was there no null check before calling bcrypt? Because AuthService.validatePassword() at line 229-233 does not check whether userData.existingUser.passwordHash is null before passing it to signInUpService.validatePassword(). The TypeScript type annotation says passwordHash is string, but the UserEntity column is declared nullable: true (user.entity.ts:73), so the runtime value can be null despite the type system.

4. Why does this code path lack the null check when another path has it? The validateLoginWithPassword() method at auth.service.ts:177 correctly checks if (!user.passwordHash) and throws a user-friendly AuthException 'Incorrect login method'. This guard was added by Thomas Trompette on 2024-08-07 (commit 2abb6ad). However, the signInUp flow's validatePassword() method was introduced later by Antoine Moreaux on 2025-03-17 (commit bda835b) without replicating this same defensive check.

5. Why was this not caught? The signInUp validatePassword method was written assuming the existingUser would always have a passwordHash when the auth provider is Password. This assumption is incorrect because the signInUp flow can match an existing OAuth-created user (who has no passwordHash) when someone tries to sign up with email/password using an email already registered via OAuth. No test covers this cross-provider scenario in the signInUp path.

Root cause: The AuthService.validatePassword() method (auth.service.ts:229-233) in the signInUp code path is missing a null guard on userData.existingUser.passwordHash before passing it to bcrypt.compare. The fix should check for null/undefined passwordHash and throw a descriptive AuthException, mirroring the existing guard in validateLoginWithPassword at line 177.

Introduced by: Antoine Moreaux on 2025-03-17 in commit bda835b

Suggested Fix

Added a null check on userData.existingUser.passwordHash inside the validatePassword private method of AuthService, immediately before calling signInUpService.validatePassword. When passwordHash is null or undefined (i.e. the user was created via OAuth and never set a password), an AuthException with code INVALID_INPUT is thrown with the user-friendly message 'User was not created with email/password'. This mirrors the identical guard that already exists in validateLoginWithPassword at line 177, preventing bcrypt.compare from receiving a null argument and crashing with an unhandled error.

Evidence

• Code: packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts:229 - if (userData.type === 'existingUser') {

---

Generated by Sonarly

[cubic-dev-ai]

No issues found across 1 file

[greptile-apps]

Greptile Summary

Adds a defensive null check for passwordHash in the validatePassword method to prevent bcrypt crashes when OAuth users attempt to sign in with email/password.

• Mirrors the existing guard pattern from validateLoginWithPassword at line 177
• Throws a user-friendly AuthException with message "User was not created with email/password" when passwordHash is null
• Prevents unhandled bcrypt error "data and hash arguments required" from reaching users

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• The fix correctly addresses a critical bug by adding a defensive null check that mirrors an existing pattern in the codebase (validateLoginWithPassword at line 177). The change is minimal, well-scoped, and prevents a runtime crash while providing appropriate user feedback.
• No files require special attention

Important Files Changed

Filename	Overview
packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts	Added null check for passwordHash before bcrypt.compare to prevent crashes when OAuth users attempt password login

_{Last reviewed commit: 9540337}

[greptile-apps]

_{1 file reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[twenty-eng-sync]

Hey @FelixMalfait! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

[github-actions]

	Fails
🚫	node failed.

Log

Details

�[31mError: �[39m SyntaxError: Unexpected end of JSON input
    at JSON.parse (<anonymous>)
�[90m    at parseJSONFromBytes (node:internal/deps/undici/undici:6433:19)�[39m
�[90m    at successSteps (node:internal/deps/undici/undici:6414:27)�[39m
�[90m    at readAllBytes (node:internal/deps/undici/undici:5380:13)�[39m
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:103:5)�[39m
danger-results://tmp/danger-results-e8e97e46.json

Generated by 🚫 dangerJS against 9540337