From 77a71787bdebb9c12bd1749d20a7d4ee4032cd34 Mon Sep 17 00:00:00 2001 From: Max Hohlfeld Date: Mon, 9 Sep 2024 23:21:07 +0200 Subject: [PATCH] refactor: use same logic for all password reset locations --- src/auth/utils.rs | 2 +- src/endpoints/user/get_reset.rs | 2 +- src/endpoints/user/mod.rs | 106 +++++++++++++++++++++- src/endpoints/user/post_changepassword.rs | 1 + src/endpoints/user/post_register.rs | 69 +++----------- src/endpoints/user/post_reset.rs | 101 +++++---------------- src/models/mod.rs | 2 +- src/models/password_reset.rs | 30 ++++-- src/models/registration.rs | 8 +- src/utils/application_error.rs | 7 +- src/utils/email.rs | 1 - src/utils/password_help.rs | 12 ++- templates/user/profile.html | 2 +- 13 files changed, 185 insertions(+), 158 deletions(-) diff --git a/src/auth/utils.rs b/src/auth/utils.rs index e50a0cb4..0b35e79f 100644 --- a/src/auth/utils.rs +++ b/src/auth/utils.rs @@ -14,7 +14,7 @@ pub fn generate_salt_and_hash_plain_password(plain: &str) -> anyhow::Result<(Str Ok((hash, salt.to_string())) } -pub fn hash_plain_password_with_salt(plain: &str, salt_string: &str) -> anyhow::Result { +pub fn hash_plain_password_with_salt(plain: &str, salt_string: &str) -> Result { let salt = SaltString::from_b64(salt_string)?; let hash = Argon2::default() diff --git a/src/endpoints/user/get_reset.rs b/src/endpoints/user/get_reset.rs index 99f65024..4881c4a5 100644 --- a/src/endpoints/user/get_reset.rs +++ b/src/endpoints/user/get_reset.rs @@ -34,7 +34,7 @@ pub async fn get( token, title: "Brass - Passwort zurücksetzen", endpoint: "/reset-password", - new_password_label: "/reset-password", + new_password_label: "neues Passwort:", retype_label: "neues Passwort wiederholen:", submit_button_label: "Passwort zurücksetzen", }; diff --git a/src/endpoints/user/mod.rs b/src/endpoints/user/mod.rs index ff8f8b08..91f8674b 100644 --- a/src/endpoints/user/mod.rs +++ b/src/endpoints/user/mod.rs @@ -1,6 +1,11 @@ -use crate::filters; -use crate::models::{Area, Role, User}; +use crate::auth::utils::hash_plain_password_with_salt; +use crate::models::{Area, Role, Token, User}; +use crate::utils::{password_help, ApplicationError}; +use crate::{auth, filters}; +use actix_web::HttpResponse; use askama::Template; +use sqlx::PgPool; +use zxcvbn::{zxcvbn, Score}; pub mod delete; pub mod get_changepassword; @@ -10,15 +15,15 @@ pub mod get_logout; pub mod get_new; pub mod get_overview; pub mod get_profile; +pub mod get_register; pub mod get_reset; pub mod post_changepassword; pub mod post_edit; pub mod post_login; pub mod post_new; +pub mod post_register; pub mod post_reset; pub mod post_toggle; -pub mod get_register; -pub mod post_register; #[derive(Template)] #[template(path = "user/new_or_edit.html")] @@ -41,5 +46,96 @@ struct ResetPasswordTemplate<'a> { endpoint: &'a str, new_password_label: &'a str, retype_label: &'a str, - submit_button_label: &'a str + submit_button_label: &'a str, +} + +async fn handle_password_change_request( + pool: &PgPool, + token: Option<&impl Token>, + user_id: i32, + password: &str, + password_retyped: &str, + current_password: Option<&str>, + generate_message: bool, +) -> Result { + let no_message = HttpResponse::NoContent().finish(); + + if password.chars().count() > 256 { + if generate_message { + return Ok( + HttpResponse::BadRequest().body(password_help::format_message( + "danger", + "Password darf nicht länger als 256 Zeichen sein.", + )), + ); + } + + return Ok(no_message); + } + + let user = User::read_by_id(pool, user_id).await?; + let mut split_names: Vec<&str> = user.name.as_str().split_whitespace().collect(); + let mut user_inputs = vec![user.email.as_str()]; + user_inputs.append(&mut split_names); + let entropy = zxcvbn(password, &user_inputs); + + if entropy.score() < Score::Three { + if generate_message { + let message = password_help::generate_for_entropy(&entropy); + return Ok(HttpResponse::BadRequest().body(message)); + } + + return Ok(no_message); + } + + if generate_message { + if entropy.score() == Score::Three { + return Ok(HttpResponse::Ok().body(password_help::format_message( + "success", + "Sicheres Passwort.", + ))); + } else { + return Ok(HttpResponse::Ok().body(password_help::format_message( + "success", + "Sehr sicheres Passwort.", + ))); + } + } + + if let Some(current_password) = current_password { + // unwraps are safe, as login only works with password and salt and this call site requires login + let hash = user.password.as_ref().unwrap(); + let salt = user.salt.as_ref().unwrap(); + + if hash != &hash_plain_password_with_salt(current_password, salt)? { + return Ok(HttpResponse::BadRequest().body("Aktuelles Passwort ist falsch.")); + } + } + + if password != password_retyped { + return Ok(HttpResponse::BadRequest().body("Passwörter stimmen nicht überein.")); + } + + let (hash, salt) = auth::utils::generate_salt_and_hash_plain_password(password).unwrap(); + + User::update( + pool, + user_id, + None, + None, + Some(&hash), + Some(&salt), + None, + None, + None, + None, + None, + ) + .await?; + + if let Some(token) = token { + token.delete(pool).await?; + } + + Ok(HttpResponse::Ok().body(r#"
Registrierung abgeschlossen.
Zum Login"#)) } diff --git a/src/endpoints/user/post_changepassword.rs b/src/endpoints/user/post_changepassword.rs index 6c6e949d..edb7f8f7 100644 --- a/src/endpoints/user/post_changepassword.rs +++ b/src/endpoints/user/post_changepassword.rs @@ -17,6 +17,7 @@ async fn post( form: web::Form, pool: web::Data, ) -> impl Responder { + // TODO: hier weiter gleichziehen mit post reset und post register if user.password.as_ref().is_some_and(|p| p == &utils::hash_plain_password_with_salt( &form.currentpassword, diff --git a/src/endpoints/user/post_register.rs b/src/endpoints/user/post_register.rs index f639cd77..129ad1b1 100644 --- a/src/endpoints/user/post_register.rs +++ b/src/endpoints/user/post_register.rs @@ -1,12 +1,9 @@ use actix_web::{post, web, HttpResponse, Responder}; use serde::Deserialize; use sqlx::PgPool; -use zxcvbn::{zxcvbn, Score}; use crate::{ - auth, - models::{Registration, User}, - utils::{password_help, ApplicationError}, + endpoints::user::handle_password_change_request, models::Registration, utils::ApplicationError, }; #[derive(Deserialize)] @@ -22,65 +19,25 @@ async fn post( form: web::Form, pool: web::Data, ) -> Result { + // TODO: refactor into check if HX-TARGET = #password-strength exists let is_dry = form.dry.unwrap_or(false); - // TODO: flip unauthorized with not found or unwrap result in a other way - let token = Registration::does_token_exist(pool.get_ref(), &form.token).await?.ok_or(ApplicationError::Unauthorized)?; - - if form.password.chars().count() > 256 { - if is_dry { - return Ok(HttpResponse::BadRequest().body("
Password darf nicht länger als 256 Zeichen sein.
")); + let token = + if let Some(token) = Registration::does_token_exist(pool.get_ref(), &form.token).await? { + token } else { return Ok(HttpResponse::NoContent().finish()); - } - } + }; - let user = User::read_by_id(pool.get_ref(), token.userid).await?; - let mut split_names: Vec<&str> = user.name.as_str().split_whitespace().collect(); - let mut user_inputs = vec![user.email.as_str()]; - user_inputs.append(&mut split_names); - let entropy = zxcvbn(&form.password, &user_inputs); - - if entropy.score() < Score::Three { - if is_dry { - let message = password_help::generate_for_entropy(&entropy); - - return Ok(HttpResponse::BadRequest().body(message)); - } else { - return Ok(HttpResponse::NoContent().finish()); - } - } - - if is_dry { - if entropy.score() == Score::Three { - return Ok(HttpResponse::Ok() - .body("
Sicheres Passwort.
")); - } else { - return Ok(HttpResponse::Ok() - .body("
Sehr sicheres Passwort.
")); - } - } - - if form.password != form.passwordretyped { - return Ok(HttpResponse::BadRequest().body("Passwörter stimmen nicht überein!")); - } - let (hash, salt) = auth::utils::generate_salt_and_hash_plain_password(&form.password).unwrap(); - - User::update( + let response = handle_password_change_request( pool.get_ref(), - token.userid, - None, - None, - Some(&hash), - Some(&salt), - None, - None, - None, - None, + Some(&token), + token.id, + &form.password, + &form.passwordretyped, None, + is_dry, ) .await?; - Registration::delete(pool.get_ref(), &token.token).await?; - - Ok(HttpResponse::Ok().body(r#"
Registrierung abgeschlossen.
Zum Login"#)) + Ok(response) } diff --git a/src/endpoints/user/post_reset.rs b/src/endpoints/user/post_reset.rs index 906569e0..016b1a6d 100644 --- a/src/endpoints/user/post_reset.rs +++ b/src/endpoints/user/post_reset.rs @@ -2,12 +2,11 @@ use actix_web::{web, HttpResponse, Responder}; use lettre::{SmtpTransport, Transport}; use serde::Deserialize; use sqlx::PgPool; -use zxcvbn::{zxcvbn, Score}; use crate::{ - auth::{self}, + endpoints::user::handle_password_change_request, models::{PasswordReset, User}, - utils::{email, password_help}, + utils::{email, ApplicationError}, }; #[derive(Deserialize, Debug)] @@ -24,104 +23,50 @@ async fn post( form: web::Form, pool: web::Data, mailer: web::Data, -) -> impl Responder { +) -> Result { if form.email.is_some() && form.token.is_none() && form.password.is_none() && form.passwordretyped.is_none() { if let Ok(user) = User::read_for_login(pool.get_ref(), form.email.as_ref().unwrap()).await { - let reset = PasswordReset::insert_new_for_user(pool.get_ref(), user.id) - .await - .unwrap(); + let reset = PasswordReset::insert_new_for_user(pool.get_ref(), user.id).await?; let message = email::build_forgot_password_message(&user, &reset.token); - mailer.send(&message).unwrap(); + mailer.send(&message)?; } - return HttpResponse::Ok().body("E-Mail versandt!"); + return Ok(HttpResponse::Ok().body("E-Mail versandt!")); } else if form.email.is_none() && form.token.is_some() && form.password.is_some() && form.passwordretyped.is_some() { - let password = form.password.as_ref().unwrap(); + // TODO: refactor into check if HX-TARGET = #password-strength exists let is_dry = form.dry.is_some_and(|b| b); - let token = - PasswordReset::does_token_exist(pool.get_ref(), form.token.as_ref().unwrap()).await; + let token = if let Some(token) = + PasswordReset::does_token_exist(pool.get_ref(), &form.token.as_ref().unwrap()).await? + { + token + } else { + return Ok(HttpResponse::NoContent().finish()); + }; - if token.is_err() { - return HttpResponse::BadRequest().body("Token existiert nicht bzw. ist abgelaufen!"); - } - - if password.chars().count() > 256 { - if is_dry { - return HttpResponse::BadRequest().body("
Password darf nicht länger als 256 Zeichen sein.
"); - } else { - return HttpResponse::NoContent().finish(); - } - } - - let user = User::read_by_id(pool.get_ref(), token.as_ref().unwrap().userid) - .await - .unwrap(); - let mut split_names: Vec<&str> = user.name.as_str().split_whitespace().collect(); - let mut user_inputs = vec![user.email.as_str()]; - user_inputs.append(&mut split_names); - let entropy = zxcvbn(password, &user_inputs); - - if entropy.score() < Score::Three { - if is_dry { - let message = password_help::generate_for_entropy(&entropy); - - return HttpResponse::BadRequest().body(message); - } else { - return HttpResponse::NoContent().finish(); - } - } - - if is_dry { - if entropy.score() == Score::Three { - return HttpResponse::Ok() - .body("
Sicheres Passwort.
"); - } else { - return HttpResponse::Ok() - .body("
Sehr sicheres Passwort.
"); - } - } - - if password != form.passwordretyped.as_ref().unwrap() { - return HttpResponse::BadRequest().body("Passwörter stimmen nicht überein!"); - } - - let (hash, salt) = - auth::utils::generate_salt_and_hash_plain_password(form.password.as_ref().unwrap()) - .unwrap(); - - User::update( + let response = handle_password_change_request( pool.get_ref(), - token.as_ref().unwrap().userid, - None, - None, - Some(&hash), - Some(&salt), - None, - None, - None, - None, + Some(&token), + token.id, + form.password.as_ref().unwrap(), + form.passwordretyped.as_ref().unwrap(), None, + is_dry, ) - .await - .unwrap(); + .await?; - PasswordReset::delete(pool.get_ref(), &token.unwrap().token) - .await - .unwrap(); - - return HttpResponse::Ok().body(r#"
Passwort wurde geändert.
Zum Login"#); + return Ok(response); } else { - return HttpResponse::BadRequest().finish(); + return Ok(HttpResponse::BadRequest().finish()); } } diff --git a/src/models/mod.rs b/src/models/mod.rs index 4e6afa40..65b96494 100644 --- a/src/models/mod.rs +++ b/src/models/mod.rs @@ -18,7 +18,7 @@ pub use location::Location; pub use role::Role; pub use user::User; pub use assignement::Assignment; -pub use password_reset::PasswordReset; +pub use password_reset::{PasswordReset, Token}; pub use registration::Registration; type Result = std::result::Result; diff --git a/src/models/password_reset.rs b/src/models/password_reset.rs index 61b5b787..b3f802c8 100644 --- a/src/models/password_reset.rs +++ b/src/models/password_reset.rs @@ -1,19 +1,23 @@ -use anyhow::Result; use chrono::{NaiveDateTime, TimeDelta}; use sqlx::{query_as, PgPool}; +use super::Result; use crate::utils::token_generation::generate_token_and_expiration; +pub trait Token { + async fn delete(&self, pool: &PgPool) -> Result<()>; +} + #[derive(Debug)] pub struct PasswordReset { pub id: i32, pub token: String, pub userid: i32, - pub expires: NaiveDateTime + pub expires: NaiveDateTime, } impl PasswordReset { - pub async fn insert_new_for_user(pool: &PgPool, user_id: i32) -> Result{ + pub async fn insert_new_for_user(pool: &PgPool, user_id: i32) -> Result { let (token, expires) = generate_token_and_expiration(64, TimeDelta::hours(24)); let inserted = query_as!( @@ -29,17 +33,19 @@ impl PasswordReset { Ok(inserted) } - pub async fn does_token_exist(pool: &PgPool, token: &str) -> Result { - let result = query_as!(PasswordReset, - "SELECT * FROM passwordReset WHERE token = $1 AND expires > NOW();", token + pub async fn does_token_exist(pool: &PgPool, token: &str) -> Result> { + let result = query_as!( + PasswordReset, + "SELECT * FROM passwordReset WHERE token = $1 AND expires > NOW();", + token ) - .fetch_one(pool) - .await?; + .fetch_optional(pool) + .await?; Ok(result) } - pub async fn delete(pool: &PgPool, token: &str) -> anyhow::Result<()> { + pub async fn delete(pool: &PgPool, token: &str) -> Result<()> { sqlx::query!("DELETE FROM passwordReset WHERE token = $1;", token) .execute(pool) .await?; @@ -47,3 +53,9 @@ impl PasswordReset { Ok(()) } } + +impl Token for PasswordReset { + async fn delete(&self, pool: &PgPool) -> Result<()> { + PasswordReset::delete(pool, &self.token).await + } +} diff --git a/src/models/registration.rs b/src/models/registration.rs index 3192697d..a7a5aa8a 100644 --- a/src/models/registration.rs +++ b/src/models/registration.rs @@ -11,7 +11,7 @@ pub struct Registration { pub expires: NaiveDateTime, } -use super::Result; +use super::{password_reset::Token, Result}; impl Registration { pub async fn insert_new_for_user(pool: &PgPool, user_id: i32) -> Result { @@ -48,3 +48,9 @@ impl Registration { Ok(()) } } + +impl Token for Registration { + async fn delete(&self, pool: &PgPool) -> Result<()> { + Registration::delete(pool, &self.token).await + } +} diff --git a/src/utils/application_error.rs b/src/utils/application_error.rs index 47d47ca6..dd53c8c3 100644 --- a/src/utils/application_error.rs +++ b/src/utils/application_error.rs @@ -17,6 +17,8 @@ pub enum ApplicationError { Email(#[from] lettre::error::Error), #[error("email transport not working")] EmailTransport(#[from] lettre::transport::smtp::Error), + #[error("hashfunction failed")] + Hash(#[from] argon2::password_hash::Error) } impl actix_web::error::ResponseError for ApplicationError { @@ -29,6 +31,7 @@ impl actix_web::error::ResponseError for ApplicationError { ApplicationError::EmailAdress(_) => StatusCode::INTERNAL_SERVER_ERROR, ApplicationError::Email(_) => StatusCode::INTERNAL_SERVER_ERROR, ApplicationError::EmailTransport(_) => StatusCode::INTERNAL_SERVER_ERROR, + ApplicationError::Hash(_) => StatusCode::INTERNAL_SERVER_ERROR, } } @@ -36,13 +39,11 @@ impl actix_web::error::ResponseError for ApplicationError { let mut response = HttpResponse::build(self.status_code()); match self { - ApplicationError::UnsupportedEnumValue { .. } => response.body(self.to_string()), - ApplicationError::Unauthorized { .. } => response.body(self.to_string()), ApplicationError::Database(e) => response.body(format!("{self} - {e}")), - ApplicationError::EnvVariable(_) => response.body(self.to_string()), ApplicationError::EmailAdress(e) => response.body(format!("{self} - {e}")), ApplicationError::Email(e) => response.body(format!("{self} - {e}")), ApplicationError::EmailTransport(e) => response.body(format!("{self} - {e}")), + _ => response.body(self.to_string()), } } } diff --git a/src/utils/email.rs b/src/utils/email.rs index 9a45b8fc..0a7ede00 100644 --- a/src/utils/email.rs +++ b/src/utils/email.rs @@ -84,7 +84,6 @@ Viele Grüße"##, user.name)) pub fn build_registration_message(user: &User, token: &str) -> Result { let hostname = env::var("HOSTNAME")?; let register_url = format!("https://{hostname}/register?token={token}"); - println!("{user:?}"); let message = Message::builder() .from("noreply ".parse()?) diff --git a/src/utils/password_help.rs b/src/utils/password_help.rs index 8a677512..b777aaa0 100644 --- a/src/utils/password_help.rs +++ b/src/utils/password_help.rs @@ -1,4 +1,7 @@ -use zxcvbn::{feedback::{Suggestion, Warning}, Entropy}; +use zxcvbn::{ + feedback::{Suggestion, Warning}, + Entropy, +}; pub fn generate_for_entropy(entropy: &Entropy) -> String { let feedback = entropy.feedback().unwrap(); @@ -72,3 +75,10 @@ pub fn generate_for_entropy(entropy: &Entropy) -> String { format!("

{warning}

{vorschlag_text}:
    {suggestion}
") } + +pub fn format_message(level: &str, message: &str) -> String { + format!( + r#"
{}
"#, + level, message + ) +} diff --git a/templates/user/profile.html b/templates/user/profile.html index 58d8a81e..71d340fa 100644 --- a/templates/user/profile.html +++ b/templates/user/profile.html @@ -28,7 +28,7 @@
- +