fix(auth): wrong password returns Ok(false), not Err
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.
This commit is contained in:
parent
eb2c2aa546
commit
706df11f15
1 changed files with 18 additions and 5 deletions
|
|
@ -1,6 +1,9 @@
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
use pbkdf2::{
|
use pbkdf2::{
|
||||||
password_hash::{rand_core::OsRng, PasswordHash, PasswordHasher, PasswordVerifier, SaltString},
|
password_hash::{
|
||||||
|
rand_core::OsRng, Error as PasswordHashError, PasswordHash, PasswordHasher,
|
||||||
|
PasswordVerifier, SaltString,
|
||||||
|
},
|
||||||
Pbkdf2,
|
Pbkdf2,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
@ -15,14 +18,24 @@ impl PasswordService {
|
||||||
Ok(password_hash.to_string())
|
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> {
|
pub fn verify_password(password: &str, hash: &str) -> Result<bool> {
|
||||||
let parsed_hash = PasswordHash::new(hash)
|
let parsed_hash = PasswordHash::new(hash)
|
||||||
.map_err(|e| anyhow::anyhow!("Failed to parse password hash: {}", e))?;
|
.map_err(|e| anyhow::anyhow!("Failed to parse password hash: {}", e))?;
|
||||||
|
|
||||||
Pbkdf2
|
match Pbkdf2.verify_password(password.as_bytes(), &parsed_hash) {
|
||||||
.verify_password(password.as_bytes(), &parsed_hash)
|
Ok(()) => Ok(true),
|
||||||
.map(|_| true)
|
// `Error::Password` is the password-hash crate's signal for a
|
||||||
.map_err(|e| anyhow::anyhow!("Password verification failed: {}", e))
|
// mismatch — the one expected "failure" during normal auth.
|
||||||
|
Err(PasswordHashError::Password) => Ok(false),
|
||||||
|
Err(e) => Err(anyhow::anyhow!("Password verification failed: {}", e)),
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue