From 706df11f15df82c3e4b6e72a9227585d22935883 Mon Sep 17 00:00:00 2001
From: goose <goose@block.dev>
Date: Sun, 19 Jul 2026 02:29:09 -0300
Subject: [PATCH] fix(auth): wrong password returns Ok(false), not Err
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

verify_password() was mapping every password-hash failure — including a
plain wrong password (password_hash::Error::Password) — to Err, which
propagated to the login handler's Err arm and returned 500
'authentication error' instead of 401 'invalid credentials'.

All three callers (login, change_password, recover_password) already
match on Ok(true)/Ok(false)/Err and expect Ok(false) for a mismatch;
the function just wasn't honoring that contract. Map
Error::Password -> Ok(false) and propagate only genuine failures as Err.

This was a latent bug: CI's auth tests never ran before because the
Mongo service container couldn't schedule (port collision, fixed in
#8). Now that CI runs them for real, login_with_wrong_password_is_rejected
exposed it. Behavior improves on all three call sites — a wrong recovery
phrase now also correctly 401s instead of 500ing.
---
 backend/src/auth/password.rs | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/backend/src/auth/password.rs b/backend/src/auth/password.rs
index 04e3bbd..efe5383 100644
--- a/backend/src/auth/password.rs
+++ b/backend/src/auth/password.rs
@@ -1,6 +1,9 @@
 use anyhow::Result;
 use pbkdf2::{
-    password_hash::{rand_core::OsRng, PasswordHash, PasswordHasher, PasswordVerifier, SaltString},
+    password_hash::{
+        rand_core::OsRng, Error as PasswordHashError, PasswordHash, PasswordHasher,
+        PasswordVerifier, SaltString,
+    },
     Pbkdf2,
 };
 
@@ -15,14 +18,24 @@ impl PasswordService {
         Ok(password_hash.to_string())
     }
 
+    /// Verify a password against a stored PHC string.
+    ///
+    /// Returns `Ok(true)` on a match, `Ok(false)` on a mismatch (wrong
+    /// password — an expected outcome, not an error), and `Err(...)` only for
+    /// genuine failures (unparseable hash, malformed input, etc.). Returning
+    /// `Ok(false)` for a wrong password lets the login handler distinguish
+    /// "invalid credentials" (401) from a real backend fault (500).
     pub fn verify_password(password: &str, hash: &str) -> Result<bool> {
         let parsed_hash = PasswordHash::new(hash)
             .map_err(|e| anyhow::anyhow!("Failed to parse password hash: {}", e))?;
 
-        Pbkdf2
-            .verify_password(password.as_bytes(), &parsed_hash)
-            .map(|_| true)
-            .map_err(|e| anyhow::anyhow!("Password verification failed: {}", e))
+        match Pbkdf2.verify_password(password.as_bytes(), &parsed_hash) {
+            Ok(()) => Ok(true),
+            // `Error::Password` is the password-hash crate's signal for a
+            // mismatch — the one expected "failure" during normal auth.
+            Err(PasswordHashError::Password) => Ok(false),
+            Err(e) => Err(anyhow::anyhow!("Password verification failed: {}", e)),
+        }
     }
 }