Skip to content

Commit

Permalink
feat: [#618] allow users to change the password
Browse files Browse the repository at this point in the history
TODO:

- Validate new password.
- Extract duplicate code for pass validation.
- Tests
  • Loading branch information
josecelano committed Jun 4, 2024
1 parent df848de commit 44030f8
Show file tree
Hide file tree
Showing 10 changed files with 131 additions and 9 deletions.
5 changes: 5 additions & 0 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ pub async fn run(configuration: Configuration, api_version: &Version) -> Running
user_repository.clone(),
user_profile_repository.clone(),
));
let profile_service = Arc::new(user::ProfileService::new(
configuration.clone(),
user_authentication_repository.clone(),
));
let ban_service = Arc::new(user::BanService::new(
user_repository.clone(),
user_profile_repository.clone(),
Expand Down Expand Up @@ -164,6 +168,7 @@ pub async fn run(configuration: Configuration, api_version: &Version) -> Running
settings_service,
torrent_index,
registration_service,
profile_service,
ban_service,
));

Expand Down
3 changes: 3 additions & 0 deletions src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ pub struct AppData {
pub settings_service: Arc<settings::Service>,
pub torrent_service: Arc<torrent::Index>,
pub registration_service: Arc<user::RegistrationService>,
pub profile_service: Arc<user::ProfileService>,
pub ban_service: Arc<user::BanService>,
}

Expand Down Expand Up @@ -85,6 +86,7 @@ impl AppData {
settings_service: Arc<settings::Service>,
torrent_service: Arc<torrent::Index>,
registration_service: Arc<user::RegistrationService>,
profile_service: Arc<user::ProfileService>,
ban_service: Arc<user::BanService>,
) -> AppData {
AppData {
Expand Down Expand Up @@ -118,6 +120,7 @@ impl AppData {
settings_service,
torrent_service,
registration_service,
profile_service,
ban_service,
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/databases/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ pub trait Database: Sync + Send {
/// Add new user and return the newly inserted `user_id`.
async fn insert_user_and_get_id(&self, username: &str, email: &str, password: &str) -> Result<UserId, Error>;

/// Change user's password.
async fn change_user_password(&self, user_id: i64, new_password: &str) -> Result<(), Error>;

/// Get `User` from `user_id`.
async fn get_user_from_id(&self, user_id: i64) -> Result<User, Error>;

Expand Down
17 changes: 17 additions & 0 deletions src/databases/mysql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,23 @@ impl Database for Mysql {
}
}

/// Change user's password.
async fn change_user_password(&self, user_id: i64, new_password: &str) -> Result<(), database::Error> {
query("UPDATE torrust_user_authentication SET password_hash = ? WHERE user_id = ?")
.bind(new_password)
.bind(user_id)
.execute(&self.pool)
.await
.map_err(|_| database::Error::Error)
.and_then(|v| {
if v.rows_affected() > 0 {
Ok(())
} else {
Err(database::Error::UserNotFound)
}
})
}

async fn get_user_from_id(&self, user_id: i64) -> Result<User, database::Error> {
query_as::<_, User>("SELECT * FROM torrust_users WHERE user_id = ?")
.bind(user_id)
Expand Down
17 changes: 17 additions & 0 deletions src/databases/sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,23 @@ impl Database for Sqlite {
}
}

/// Change user's password.
async fn change_user_password(&self, user_id: i64, new_password: &str) -> Result<(), database::Error> {
query("UPDATE torrust_user_authentication SET password_hash = ? WHERE user_id = ?")
.bind(new_password)
.bind(user_id)
.execute(&self.pool)
.await
.map_err(|_| database::Error::Error)
.and_then(|v| {
if v.rows_affected() > 0 {
Ok(())
} else {
Err(database::Error::UserNotFound)
}
})
}

async fn get_user_from_id(&self, user_id: i64) -> Result<User, database::Error> {
query_as::<_, User>("SELECT * FROM torrust_users WHERE user_id = ?")
.bind(user_id)
Expand Down
9 changes: 9 additions & 0 deletions src/services/authentication.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,15 @@ impl DbUserAuthenticationRepository {
pub async fn get_user_authentication_from_id(&self, user_id: &UserId) -> Result<UserAuthentication, Error> {
self.database.get_user_authentication_from_id(*user_id).await
}

/// It changes the user's password.
///
/// # Errors
///
/// It returns an error if there is a database error.
pub async fn change_password(&self, user_id: UserId, password_hash: &str) -> Result<(), Error> {
self.database.change_user_password(user_id, password_hash).await
}
}

/// Verify if the user supplied and the database supplied passwords match
Expand Down
67 changes: 66 additions & 1 deletion src/services/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,15 @@ use log::{debug, info};
use mockall::automock;
use pbkdf2::password_hash::rand_core::OsRng;

use super::authentication::DbUserAuthenticationRepository;
use crate::config::{Configuration, EmailOnSignup};
use crate::databases::database::{Database, Error};
use crate::errors::ServiceError;
use crate::mailer;
use crate::mailer::VerifyClaims;
use crate::models::user::{UserCompact, UserId, UserProfile, Username};
use crate::utils::validation::validate_email_address;
use crate::web::api::server::v1::contexts::user::forms::RegistrationForm;
use crate::web::api::server::v1::contexts::user::forms::{ChangePasswordForm, RegistrationForm};

/// Since user email could be optional, we need a way to represent "no email"
/// in the database. This function returns the string that should be used for
Expand Down Expand Up @@ -186,6 +187,70 @@ impl RegistrationService {
}
}

pub struct ProfileService {
configuration: Arc<Configuration>,
user_authentication_repository: Arc<DbUserAuthenticationRepository>,
}

impl ProfileService {
#[must_use]
pub fn new(configuration: Arc<Configuration>, user_repository: Arc<DbUserAuthenticationRepository>) -> Self {
Self {
configuration,
user_authentication_repository: user_repository,
}
}

/// It registers a new user.
///
/// # Errors
///
/// This function will return a:
///
/// * `ServiceError::PasswordsDontMatch` if the supplied passwords do not match.
/// * `ServiceError::PasswordTooShort` if the supplied password is too short.
/// * `ServiceError::PasswordTooLong` if the supplied password is too long.
/// * An error if unable to successfully hash the password.
/// * An error if unable to change the password in the database.
pub async fn change_password(&self, user_id: UserId, change_password_form: &ChangePasswordForm) -> Result<(), ServiceError> {
info!("changing user password for user ID: {user_id}");

let settings = self.configuration.settings.read().await;

// todo:
// - Validate current password
// - Remove duplicate code for password validation and hashing

if change_password_form.password != change_password_form.confirm_password {
return Err(ServiceError::PasswordsDontMatch);
}

let password_length = change_password_form.password.len();

if password_length <= settings.auth.min_password_length {
return Err(ServiceError::PasswordTooShort);
}

if password_length >= settings.auth.max_password_length {
return Err(ServiceError::PasswordTooLong);
}

let salt = SaltString::generate(&mut OsRng);

// Argon2 with default params (Argon2id v19)
let argon2 = Argon2::default();

// Hash password to PHC string ($argon2id$v=19$...)
let new_password_hash = argon2
.hash_password(change_password_form.password.as_bytes(), &salt)?
.to_string();

self.user_authentication_repository.change_password(user_id, &new_password_hash).await?;

Ok(())
}
}

pub struct BanService {
user_repository: Arc<Box<dyn Repository>>,
user_profile_repository: Arc<DbUserProfileRepository>,
Expand Down
2 changes: 1 addition & 1 deletion src/web/api/server/signals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub async fn graceful_shutdown(handle: axum_server::Handle, rx_halt: tokio::sync
shutdown_signal_with_message(rx_halt, message).await;

info!("Sending graceful shutdown signal");
handle.graceful_shutdown(Some(Duration::from_secs(90)));
handle.graceful_shutdown(Some(Duration::from_secs(2)));

println!("!! shuting down in 90 seconds !!");

Expand Down
2 changes: 1 addition & 1 deletion src/web/api/server/v1/contexts/user/forms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub struct JsonWebToken {

#[derive(Clone, Debug, Deserialize, Serialize)]
pub struct ChangePasswordForm {
pub current_password: String,
pub password: String,
pub new_password: String,
pub confirm_password: String,
}
15 changes: 9 additions & 6 deletions src/web/api/server/v1/contexts/user/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use serde::Deserialize;
use super::forms::{ChangePasswordForm, JsonWebToken, LoginForm, RegistrationForm};
use super::responses::{self};
use crate::common::AppData;
use crate::errors::ServiceError;
use crate::web::api::server::v1::extractors::user_id::ExtractLoggedInUser;
use crate::web::api::server::v1::responses::OkResponseData;

Expand Down Expand Up @@ -133,13 +132,17 @@ pub async fn renew_token_handler(
/// - The user account is not found.
#[allow(clippy::unused_async)]
pub async fn change_password_handler(
State(_app_data): State<Arc<AppData>>,
State(app_data): State<Arc<AppData>>,
ExtractLoggedInUser(user_id): ExtractLoggedInUser,
extract::Json(change_password_form): extract::Json<ChangePasswordForm>,
) -> Response {

println!("change pass form: {change_password_form:#?}");

ServiceError::AccountNotFound.into_response()
match app_data.profile_service.change_password(user_id, &change_password_form).await {
Ok(()) => Json(OkResponseData {
data: format!("Password changed for user with ID: {user_id}"),
})
.into_response(),
Err(error) => error.into_response(),
}
}

/// It bans a user from the index.
Expand Down

0 comments on commit 44030f8

Please sign in to comment.