diff --git a/crates/diesel_models/src/schema.rs b/crates/diesel_models/src/schema.rs index 15e662fe43..a7d5a8c02b 100644 --- a/crates/diesel_models/src/schema.rs +++ b/crates/diesel_models/src/schema.rs @@ -1223,7 +1223,7 @@ diesel::table! { #[max_length = 255] name -> Varchar, #[max_length = 255] - password -> Varchar, + password -> Nullable, is_verified -> Bool, created_at -> Timestamp, last_modified_at -> Timestamp, diff --git a/crates/diesel_models/src/user.rs b/crates/diesel_models/src/user.rs index 6a040e4146..b1bbb66256 100644 --- a/crates/diesel_models/src/user.rs +++ b/crates/diesel_models/src/user.rs @@ -17,7 +17,7 @@ pub struct User { pub user_id: String, pub email: pii::Email, pub name: Secret, - pub password: Secret, + pub password: Option>, pub is_verified: bool, pub created_at: PrimitiveDateTime, pub last_modified_at: PrimitiveDateTime, @@ -37,7 +37,7 @@ pub struct UserNew { pub user_id: String, pub email: pii::Email, pub name: Secret, - pub password: Secret, + pub password: Option>, pub is_verified: bool, pub created_at: Option, pub last_modified_at: Option, @@ -76,7 +76,7 @@ pub enum UserUpdate { totp_recovery_codes: Option>>, }, PasswordUpdate { - password: Option>, + password: Secret, }, } @@ -127,7 +127,7 @@ impl From for UserUpdateInternal { }, UserUpdate::PasswordUpdate { password } => Self { name: None, - password, + password: Some(password), is_verified: None, last_modified_at, preferred_merchant_id: None, diff --git a/crates/router/src/core/user.rs b/crates/router/src/core/user.rs index a4bb4fb4b0..c78f2c8080 100644 --- a/crates/router/src/core/user.rs +++ b/crates/router/src/core/user.rs @@ -368,7 +368,7 @@ pub async fn change_password( .update_user_by_user_id( user.get_user_id(), diesel_models::user::UserUpdate::PasswordUpdate { - password: Some(new_password_hash), + password: new_password_hash, }, ) .await @@ -459,7 +459,7 @@ pub async fn rotate_password( .update_user_by_user_id( &user_token.user_id, storage_user::UserUpdate::PasswordUpdate { - password: Some(hash_password), + password: hash_password, }, ) .await @@ -510,7 +510,7 @@ pub async fn reset_password_token_only_flow( .get_email() .change_context(UserErrors::InternalServerError)?, storage_user::UserUpdate::PasswordUpdate { - password: Some(hash_password), + password: hash_password, }, ) .await @@ -548,7 +548,7 @@ pub async fn reset_password( .get_email() .change_context(UserErrors::InternalServerError)?, storage_user::UserUpdate::PasswordUpdate { - password: Some(hash_password), + password: hash_password, }, ) .await @@ -838,11 +838,9 @@ async fn handle_new_user_invitation( Ok(InviteMultipleUserResponse { is_email_sent, - password: if cfg!(not(feature = "email")) { - Some(new_user.get_password().get_secret()) - } else { - None - }, + password: new_user + .get_password() + .map(|password| password.get_secret()), email: request.email.clone(), error: None, }) diff --git a/crates/router/src/db/user.rs b/crates/router/src/db/user.rs index e8104e3a15..742944bd1e 100644 --- a/crates/router/src/db/user.rs +++ b/crates/router/src/db/user.rs @@ -244,7 +244,7 @@ impl UserInterface for MockDb { ..user.to_owned() }, storage::UserUpdate::PasswordUpdate { password } => storage::User { - password: password.clone().unwrap_or(user.password.clone()), + password: Some(password.clone()), last_password_modified_at: Some(common_utils::date_time::now()), ..user.to_owned() }, @@ -299,7 +299,7 @@ impl UserInterface for MockDb { ..user.to_owned() }, storage::UserUpdate::PasswordUpdate { password } => storage::User { - password: password.clone().unwrap_or(user.password.clone()), + password: Some(password.clone()), last_password_modified_at: Some(common_utils::date_time::now()), ..user.to_owned() }, diff --git a/crates/router/src/types/domain/user.rs b/crates/router/src/types/domain/user.rs index d9ef736347..47f26ce04e 100644 --- a/crates/router/src/types/domain/user.rs +++ b/crates/router/src/types/domain/user.rs @@ -1,8 +1,4 @@ -use std::{ - collections::HashSet, - ops::{self, Not}, - str::FromStr, -}; +use std::{collections::HashSet, ops, str::FromStr}; use api_models::{ admin as admin_api, organization as api_org, user as user_api, user_role as user_role_api, @@ -517,9 +513,8 @@ pub struct NewUser { user_id: String, name: UserName, email: UserEmail, - password: UserPassword, + password: Option, new_merchant: NewUserMerchant, - is_temporary_password: bool, } impl NewUser { @@ -539,7 +534,7 @@ impl NewUser { self.new_merchant.clone() } - pub fn get_password(&self) -> UserPassword { + pub fn get_password(&self) -> Option { self.password.clone() } @@ -623,7 +618,12 @@ impl TryFrom for storage_user::UserNew { type Error = error_stack::Report; fn try_from(value: NewUser) -> UserResult { - let hashed_password = password::generate_password_hash(value.password.get_secret())?; + let hashed_password = value + .password + .as_ref() + .map(|password| password::generate_password_hash(password.get_secret())) + .transpose()?; + let now = common_utils::date_time::now(); Ok(Self { user_id: value.get_user_id(), @@ -637,7 +637,7 @@ impl TryFrom for storage_user::UserNew { totp_status: TotpStatus::NotSet, totp_secret: None, totp_recovery_codes: None, - last_password_modified_at: value.is_temporary_password.not().then_some(now), + last_password_modified_at: value.password.is_some().then_some(now), }) } } @@ -655,10 +655,9 @@ impl TryFrom for NewUser { Ok(Self { name, email, - password, + password: Some(password), user_id, new_merchant, - is_temporary_password: false, }) } } @@ -677,9 +676,8 @@ impl TryFrom for NewUser { user_id, name, email, - password, + password: Some(password), new_merchant, - is_temporary_password: false, }) } } @@ -691,16 +689,14 @@ impl TryFrom for NewUser { let user_id = uuid::Uuid::new_v4().to_string(); let email = value.email.clone().try_into()?; let name = UserName::try_from(value.email.clone())?; - let password = UserPassword::new(password::get_temp_password())?; let new_merchant = NewUserMerchant::try_from(value)?; Ok(Self { user_id, name, email, - password, + password: None, new_merchant, - is_temporary_password: true, }) } } @@ -721,9 +717,8 @@ impl TryFrom<(user_api::CreateInternalUserRequest, String)> for NewUser { user_id, name, email, - password, + password: Some(password), new_merchant, - is_temporary_password: false, }) } } @@ -739,11 +734,12 @@ impl TryFrom for NewUser { user_id: user.0.user_id, name: UserName::new(user.0.name)?, email: user.0.email.clone().try_into()?, - password: UserPassword::new_password_without_validation(user.0.password)?, + password: user + .0 + .password + .map(UserPassword::new_password_without_validation) + .transpose()?, new_merchant, - // This is true because we are not creating a user with this request. And if it is set - // to false, last_password_modified_at will be overwritten if this user is inserted. - is_temporary_password: true, }) } } @@ -754,7 +750,8 @@ impl TryFrom for NewUser { let user_id = uuid::Uuid::new_v4().to_string(); let email = value.0.email.clone().try_into()?; let name = UserName::new(value.0.name.clone())?; - let password = UserPassword::new(password::get_temp_password())?; + let password = cfg!(not(feature = "email")) + .then_some(UserPassword::new(password::get_temp_password())?); let new_merchant = NewUserMerchant::try_from(value)?; Ok(Self { @@ -763,7 +760,6 @@ impl TryFrom for NewUser { email, password, new_merchant, - is_temporary_password: true, }) } } @@ -783,10 +779,14 @@ impl UserFromStorage { } pub fn compare_password(&self, candidate: &Secret) -> UserResult<()> { - match password::is_correct_password(candidate, &self.0.password) { - Ok(true) => Ok(()), - Ok(false) => Err(UserErrors::InvalidCredentials.into()), - Err(e) => Err(e), + if let Some(password) = self.0.password.as_ref() { + match password::is_correct_password(candidate, password) { + Ok(true) => Ok(()), + Ok(false) => Err(UserErrors::InvalidCredentials.into()), + Err(e) => Err(e), + } + } else { + Err(UserErrors::InvalidCredentials.into()) } } diff --git a/migrations/2024-06-06-101812_user_optional_password/down.sql b/migrations/2024-06-06-101812_user_optional_password/down.sql new file mode 100644 index 0000000000..acd0b1efca --- /dev/null +++ b/migrations/2024-06-06-101812_user_optional_password/down.sql @@ -0,0 +1,2 @@ +-- This file should undo anything in `up.sql` +ALTER TABLE users ALTER COLUMN password SET NOT NULL; diff --git a/migrations/2024-06-06-101812_user_optional_password/up.sql b/migrations/2024-06-06-101812_user_optional_password/up.sql new file mode 100644 index 0000000000..e48b1b751a --- /dev/null +++ b/migrations/2024-06-06-101812_user_optional_password/up.sql @@ -0,0 +1,2 @@ +-- Your SQL goes here +ALTER TABLE users ALTER COLUMN password DROP NOT NULL;