refactor(users): Make password nullable in users table (#4902)

crates/diesel_models/src/schema.rs

@@ -1223,7 +1223,7 @@ diesel::table! {
         #[max_length = 255]
         name -> Varchar,
         #[max_length = 255]
-        password -> Varchar,
+        password -> Nullable<Varchar>,
         is_verified -> Bool,
         created_at -> Timestamp,
         last_modified_at -> Timestamp,

crates/diesel_models/src/user.rs

@@ -17,7 +17,7 @@ pub struct User {
     pub user_id: String,
     pub email: pii::Email,
     pub name: Secret<String>,
-    pub password: Secret<String>,
+    pub password: Option<Secret<String>>,
     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<String>,
-    pub password: Secret<String>,
+    pub password: Option<Secret<String>>,
     pub is_verified: bool,
     pub created_at: Option<PrimitiveDateTime>,
     pub last_modified_at: Option<PrimitiveDateTime>,
@@ -76,7 +76,7 @@ pub enum UserUpdate {
         totp_recovery_codes: Option<Vec<Secret<String>>>,
     },
     PasswordUpdate {
-        password: Option<Secret<String>>,
+        password: Secret<String>,
     },
 }
 
@@ -127,7 +127,7 @@ impl From<UserUpdate> for UserUpdateInternal {
             },
             UserUpdate::PasswordUpdate { password } => Self {
                 name: None,
-                password,
+                password: Some(password),
                 is_verified: None,
                 last_modified_at,
                 preferred_merchant_id: None,

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")) {
+        password: new_user
-            Some(new_user.get_password().get_secret())
+            .get_password()
-        } else {
+            .map(|password| password.get_secret()),
-            None
-        },
         email: request.email.clone(),
         error: None,
     })

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()
                     },

crates/router/src/types/domain/user.rs

@@ -1,8 +1,4 @@
-use std::{
+use std::{collections::HashSet, ops, str::FromStr};
-    collections::HashSet,
-    ops::{self, Not},
-    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<UserPassword>,
     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<UserPassword> {
         self.password.clone()
     }
 
@@ -623,7 +618,12 @@ impl TryFrom<NewUser> for storage_user::UserNew {
     type Error = error_stack::Report<UserErrors>;
 
     fn try_from(value: NewUser) -> UserResult<Self> {
-        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<NewUser> 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<user_api::SignUpWithMerchantIdRequest> for NewUser {
         Ok(Self {
             name,
             email,
-            password,
+            password: Some(password),
             user_id,
             new_merchant,
-            is_temporary_password: false,
         })
     }
 }
@@ -677,9 +676,8 @@ impl TryFrom<user_api::SignUpRequest> for NewUser {
             user_id,
             name,
             email,
-            password,
+            password: Some(password),
             new_merchant,
-            is_temporary_password: false,
         })
     }
 }
@@ -691,16 +689,14 @@ impl TryFrom<user_api::ConnectAccountRequest> 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<UserMerchantCreateRequestWithToken> 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<InviteeUserRequestWithInvitedUserToken> 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<InviteeUserRequestWithInvitedUserToken> for NewUser {
             email,
             password,
             new_merchant,
-            is_temporary_password: true,
         })
     }
 }
@@ -783,10 +779,14 @@ impl UserFromStorage {
     }
 
     pub fn compare_password(&self, candidate: &Secret<String>) -> UserResult<()> {
-        match password::is_correct_password(candidate, &self.0.password) {
+        if let Some(password) = self.0.password.as_ref() {
-            Ok(true) => Ok(()),
+            match password::is_correct_password(candidate, password) {
-            Ok(false) => Err(UserErrors::InvalidCredentials.into()),
+                Ok(true) => Ok(()),
-            Err(e) => Err(e),
+                Ok(false) => Err(UserErrors::InvalidCredentials.into()),
+                Err(e) => Err(e),
+            }
+        } else {
+            Err(UserErrors::InvalidCredentials.into())
         }
     }
 

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;

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;