feat: [#618] check current password before changing it

This is an extra security check. Before changing the password the user
must provide the current one.

src/errors.rs

@@ -30,6 +30,8 @@ pub enum ServiceError {
 
     #[display(fmt = "Invalid username/email or password")]
     WrongPasswordOrUsername,
+    #[display(fmt = "Invalid password")]
+    InvalidPassword,
     #[display(fmt = "Username not found")]
     UsernameNotFound,
     #[display(fmt = "User not found")]
@@ -273,6 +275,7 @@ pub fn http_status_code_for_service_error(error: &ServiceError) -> StatusCode {
         ServiceError::EmailInvalid => StatusCode::BAD_REQUEST,
         ServiceError::NotAUrl => StatusCode::BAD_REQUEST,
         ServiceError::WrongPasswordOrUsername => StatusCode::FORBIDDEN,
+        ServiceError::InvalidPassword => StatusCode::FORBIDDEN,
         ServiceError::UsernameNotFound => StatusCode::NOT_FOUND,
         ServiceError::UserNotFound => StatusCode::NOT_FOUND,
         ServiceError::AccountNotFound => StatusCode::NOT_FOUND,

src/services/authentication.rs

@@ -65,7 +65,7 @@ impl Service {
             .await
             .map_err(|_| ServiceError::InternalServerError)?;
 
-        verify_password(password.as_bytes(), &user_authentication)?;
+        verify_password(password.as_bytes(), &user_authentication).map_err(|_| ServiceError::WrongPasswordOrUsername)?;
 
         let settings = self.configuration.settings.read().await;
 
@@ -191,35 +191,35 @@ impl DbUserAuthenticationRepository {
     /// 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
 ///
 /// # Errors
 ///
 /// This function will return an error if unable to parse password hash from the stored user authentication value.
-/// This function will return a `ServiceError::WrongPasswordOrUsername` if unable to match the password with either `argon2id` or `pbkdf2-sha256`.
-fn verify_password(password: &[u8], user_authentication: &UserAuthentication) -> Result<(), ServiceError> {
+/// This function will return a `ServiceError::InvalidPassword` if unable to match the password with either `argon2id` or `pbkdf2-sha256`.
+pub fn verify_password(password: &[u8], user_authentication: &UserAuthentication) -> Result<(), ServiceError> {
     // wrap string of the hashed password into a PasswordHash struct for verification
     let parsed_hash = PasswordHash::new(&user_authentication.password_hash)?;
 
     match parsed_hash.algorithm.as_str() {
         "argon2id" => {
             if Argon2::default().verify_password(password, &parsed_hash).is_err() {
-                return Err(ServiceError::WrongPasswordOrUsername);
+                return Err(ServiceError::InvalidPassword);
             }
 
             Ok(())
         }
         "pbkdf2-sha256" => {
             if Pbkdf2.verify_password(password, &parsed_hash).is_err() {
-                return Err(ServiceError::WrongPasswordOrUsername);
+                return Err(ServiceError::InvalidPassword);
             }
 
             Ok(())
         }
-        _ => Err(ServiceError::WrongPasswordOrUsername),
+        _ => Err(ServiceError::InvalidPassword),
     }
 }
 

src/services/user.rs

@@ -17,6 +17,7 @@ use crate::errors::ServiceError;
 use crate::mailer;
 use crate::mailer::VerifyClaims;
 use crate::models::user::{UserCompact, UserId, UserProfile, Username};
+use crate::services::authentication::verify_password;
 use crate::utils::validation::validate_email_address;
 use crate::web::api::server::v1::contexts::user::forms::{ChangePasswordForm, RegistrationForm};
 
@@ -100,7 +101,7 @@ impl RegistrationService {
             max_password_length: settings.auth.max_password_length,
         };
 
-        validate_password(
+        validate_password_constrains(
             &registration_form.password,
             &registration_form.confirm_password,
             &password_constraints,
@@ -196,6 +197,7 @@ impl ProfileService {
     ///
     /// This function will return a:
     ///
+    /// * `ServiceError::InvalidPassword` if the current password supplied is invalid.
     /// * `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.
@@ -206,14 +208,19 @@ impl ProfileService {
 
         let settings = self.configuration.settings.read().await;
 
-        // todo: guard that current password matches the one provided in change password form
+        let user_authentication = self
+            .user_authentication_repository
+            .get_user_authentication_from_id(&user_id)
+            .await?;
+
+        verify_password(change_password_form.current_password.as_bytes(), &user_authentication)?;
 
         let password_constraints = PasswordConstraints {
             min_password_length: settings.auth.min_password_length,
             max_password_length: settings.auth.max_password_length,
         };
 
-        validate_password(
+        validate_password_constrains(
             &change_password_form.password,
             &change_password_form.confirm_password,
             &password_constraints,
@@ -413,7 +420,11 @@ struct PasswordConstraints {
     pub max_password_length: usize,
 }
 
-fn validate_password(password: &str, confirm_password: &str, password_rules: &PasswordConstraints) -> Result<(), ServiceError> {
+fn validate_password_constrains(
+    password: &str,
+    confirm_password: &str,
+    password_rules: &PasswordConstraints,
+) -> Result<(), ServiceError> {
     if password != confirm_password {
         return Err(ServiceError::PasswordsDontMatch);
     }

src/web/api/server/signals.rs

@@ -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(2)));
+    handle.graceful_shutdown(Some(Duration::from_secs(90)));
 
     println!("!! shuting down in 90 seconds !!");
 

tests/e2e/web/api/v1/contexts/user/contract.rs

@@ -118,6 +118,29 @@ mod authentication {
         assert_successful_login_response(&response, &logged_in_user.username);
     }
 
+    #[tokio::test]
+    async fn it_should_fail_changing_the_password_if_the_user_does_not_provide_the_current_password() {
+        let mut env = TestEnv::new();
+        env.start(api::Version::V1).await;
+
+        let logged_in_user = new_logged_in_user(&env).await;
+
+        let client = Client::authenticated(&env.server_socket_addr().unwrap(), &logged_in_user.token);
+
+        let response = client
+            .change_password(
+                Username::new(logged_in_user.username.clone()),
+                ChangePasswordForm {
+                    current_password: "INVALID PASSWORD".to_string(),
+                    password: VALID_PASSWORD.to_string(),
+                    confirm_password: VALID_PASSWORD.to_string(),
+                },
+            )
+            .await;
+
+        assert_eq!(response.status, 403);
+    }
+
     #[tokio::test]
     async fn it_should_allow_a_logged_in_user_to_verify_an_authentication_token() {
         let mut env = TestEnv::new();