refactor(user_role): Change update user role request to take email …

…instead of `user_id` (#3530)

Co-authored-by: hyperswitch-bot[bot] <148525504+hyperswitch-bot[bot]@users.noreply.github.com>

crates/api_models/src/user.rs

@@ -142,7 +142,6 @@ pub struct GetUsersResponse(pub Vec<UserDetails>);
 
 #[derive(Debug, serde::Serialize)]
 pub struct UserDetails {
-    pub user_id: String,
     pub email: pii::Email,
     pub name: Secret<String>,
     pub role_id: String,

crates/api_models/src/user_role.rs

@@ -86,7 +86,7 @@ pub struct PermissionInfo {
 
 #[derive(Debug, serde::Deserialize, serde::Serialize)]
 pub struct UpdateUserRoleRequest {
-    pub user_id: String,
+    pub email: pii::Email,
     pub role_id: String,
 }
 

crates/router/src/core/errors/user.rs

@@ -60,6 +60,8 @@ pub enum UserErrors {
     MaxInvitationsError,
     #[error("RoleNotFound")]
     RoleNotFound,
+    #[error("InvalidRoleOperationWithMessage")]
+    InvalidRoleOperationWithMessage(String),
 }
 
 impl common_utils::errors::ErrorSwitch<api_models::errors::types::ApiErrorResponse> for UserErrors {
@@ -103,9 +105,12 @@ impl common_utils::errors::ErrorSwitch<api_models::errors::types::ApiErrorRespon
             Self::CompanyNameParsingError => {
                 AER::BadRequest(ApiError::new(sub_code, 14, self.get_error_message(), None))
             }
-            Self::MerchantAccountCreationError(error_message) => {
-                AER::InternalServerError(ApiError::new(sub_code, 15, error_message, None))
-            }
+            Self::MerchantAccountCreationError(_) => AER::InternalServerError(ApiError::new(
+                sub_code,
+                15,
+                self.get_error_message(),
+                None,
+            )),
             Self::InvalidEmailError => {
                 AER::BadRequest(ApiError::new(sub_code, 16, self.get_error_message(), None))
             }
@@ -151,6 +156,9 @@ impl common_utils::errors::ErrorSwitch<api_models::errors::types::ApiErrorRespon
             Self::RoleNotFound => {
                 AER::BadRequest(ApiError::new(sub_code, 32, self.get_error_message(), None))
             }
+            Self::InvalidRoleOperationWithMessage(_) => {
+                AER::BadRequest(ApiError::new(sub_code, 33, self.get_error_message(), None))
+            }
         }
     }
 }
@@ -184,6 +192,7 @@ impl UserErrors {
             Self::InvalidDeleteOperation => "Delete Operation Not Supported",
             Self::MaxInvitationsError => "Maximum invite count per request exceeded",
             Self::RoleNotFound => "Role Not Found",
+            Self::InvalidRoleOperationWithMessage(error_message) => error_message,
         }
     }
 }

crates/router/src/core/errors/utils.rs

@@ -509,3 +509,34 @@ impl RedisErrorExt for error_stack::Report<errors::RedisError> {
         }
     }
 }
+
+#[cfg(feature = "olap")]
+impl<T> StorageErrorExt<T, errors::UserErrors> for error_stack::Result<T, errors::StorageError> {
+    #[track_caller]
+    fn to_not_found_response(
+        self,
+        not_found_response: errors::UserErrors,
+    ) -> error_stack::Result<T, errors::UserErrors> {
+        self.map_err(|e| {
+            if e.current_context().is_db_not_found() {
+                e.change_context(not_found_response)
+            } else {
+                e.change_context(errors::UserErrors::InternalServerError)
+            }
+        })
+    }
+
+    #[track_caller]
+    fn to_duplicate_response(
+        self,
+        duplicate_response: errors::UserErrors,
+    ) -> error_stack::Result<T, errors::UserErrors> {
+        self.map_err(|e| {
+            if e.current_context().is_db_unique_violation() {
+                e.change_context(duplicate_response)
+            } else {
+                e.change_context(errors::UserErrors::InternalServerError)
+            }
+        })
+    }
+}

crates/router/src/core/user.rs

@@ -11,13 +11,15 @@ use router_env::env;
 #[cfg(feature = "email")]
 use router_env::logger;
 
-use super::errors::{UserErrors, UserResponse, UserResult};
+use super::errors::{StorageErrorExt, UserErrors, UserResponse, UserResult};
 #[cfg(feature = "email")]
 use crate::services::email::types as email_types;
 use crate::{
     consts,
     routes::AppState,
-    services::{authentication as auth, ApplicationResponse},
+    services::{
+        authentication as auth, authorization::predefined_permissions, ApplicationResponse,
+    },
     types::domain,
     utils,
 };
@@ -150,7 +152,7 @@ pub async fn signin(
             let preferred_role = user_from_db
                 .get_role_from_db_by_merchant_id(&state, preferred_merchant_id.as_str())
                 .await
-                .change_context(UserErrors::InternalServerError)
+                .to_not_found_response(UserErrors::InternalServerError)
                 .attach_printable("User role with preferred_merchant_id not found")?;
             domain::SignInWithRoleStrategyType::SingleRole(domain::SignInWithSingleRoleStrategy {
                 user: user_from_db,
@@ -408,11 +410,17 @@ pub async fn invite_user(
         .change_context(UserErrors::InternalServerError)?;
 
     if inviter_user.email == request.email {
-        return Err(UserErrors::InvalidRoleOperation.into())
-            .attach_printable("User Inviting themself");
+        return Err(UserErrors::InvalidRoleOperationWithMessage(
+            "User Inviting themselves".to_string(),
+        )
+        .into());
+    }
+
+    if !predefined_permissions::is_role_invitable(request.role_id.as_str())? {
+        return Err(UserErrors::InvalidRoleId.into())
+            .attach_printable(format!("role_id = {} is not invitable", request.role_id));
     }
 
-    utils::user_role::validate_role_id(request.role_id.as_str())?;
     let invitee_email = domain::UserEmail::from_pii_email(request.email.clone())?;
 
     let invitee_user = state
@@ -561,14 +569,20 @@ async fn handle_invitation(
     user_from_token: &auth::UserFromToken,
     request: &user_api::InviteUserRequest,
 ) -> UserResult<InviteMultipleUserResponse> {
-    let inviter_user = user_from_token.get_user(state.clone()).await?;
+    let inviter_user = user_from_token.get_user(state).await?;
 
     if inviter_user.email == request.email {
-        return Err(UserErrors::InvalidRoleOperation.into())
-            .attach_printable("User Inviting themself");
+        return Err(UserErrors::InvalidRoleOperationWithMessage(
+            "User Inviting themselves".to_string(),
+        )
+        .into());
+    }
+
+    if !predefined_permissions::is_role_invitable(request.role_id.as_str())? {
+        return Err(UserErrors::InvalidRoleId.into())
+            .attach_printable(format!("role_id = {} is not invitable", request.role_id));
     }
 
-    utils::user_role::validate_role_id(request.role_id.as_str())?;
     let invitee_email = domain::UserEmail::from_pii_email(request.email.clone())?;
     let invitee_user = state
         .store
@@ -733,15 +747,19 @@ pub async fn resend_invite(
         .map_err(|e| {
             if e.current_context().is_db_not_found() {
                 e.change_context(UserErrors::InvalidRoleOperation)
-                    .attach_printable("User role with given UserId MerchantId not found")
+                    .attach_printable(format!(
+                        "User role with user_id = {} and org_id = {} is not found",
+                        user.get_user_id(),
+                        user_from_token.merchant_id
+                    ))
             } else {
                 e.change_context(UserErrors::InternalServerError)
             }
         })?;
 
     if !matches!(user_role.status, UserStatus::InvitationSent) {
         return Err(UserErrors::InvalidRoleOperation.into())
-            .attach_printable("Invalid Status for Reinvitation");
+            .attach_printable("User status is not InvitationSent".to_string());
     }
 
     let email_contents = email_types::InviteUser {
@@ -832,8 +850,10 @@ pub async fn switch_merchant_id(
     user_from_token: auth::UserFromToken,
 ) -> UserResponse<user_api::SwitchMerchantResponse> {
     if user_from_token.merchant_id == request.merchant_id {
-        return Err(UserErrors::InvalidRoleOperation.into())
-            .attach_printable("User switch to same merchant id.");
+        return Err(UserErrors::InvalidRoleOperationWithMessage(
+            "User switching to same merchant id".to_string(),
+        )
+        .into());
     }
 
     let user_roles = state
@@ -847,7 +867,7 @@ pub async fn switch_merchant_id(
         .filter(|role| role.status == UserStatus::Active)
         .collect::<Vec<_>>();
 
-    let user = user_from_token.get_user(state.clone()).await?.into();
+    let user = user_from_token.get_user(&state).await?.into();
 
     let (token, role_id) = if utils::user_role::is_internal_role(&user_from_token.role_id) {
         let key_store = state
@@ -916,8 +936,7 @@ pub async fn create_merchant_account(
     user_from_token: auth::UserFromToken,
     req: user_api::UserMerchantCreate,
 ) -> UserResponse<()> {
-    let user_from_db: domain::UserFromStorage =
-        user_from_token.get_user(state.clone()).await?.into();
+    let user_from_db: domain::UserFromStorage = user_from_token.get_user(&state).await?.into();
 
     let new_user = domain::NewUser::try_from((user_from_db, req, user_from_token))?;
     let new_merchant = new_user.get_new_merchant();

crates/router/src/core/user_role.rs

@@ -5,7 +5,7 @@ use masking::ExposeInterface;
 use router_env::logger;
 
 use crate::{
-    core::errors::{UserErrors, UserResponse},
+    core::errors::{StorageErrorExt, UserErrors, UserResponse},
     routes::AppState,
     services::{
         authentication::{self as auth},
@@ -33,6 +33,7 @@ pub async fn list_roles(_state: AppState) -> UserResponse<user_role_api::ListRol
     Ok(ApplicationResponse::Json(user_role_api::ListRolesResponse(
         predefined_permissions::PREDEFINED_PERMISSIONS
             .iter()
+            .filter(|(_, role_info)| role_info.is_invitable())
             .filter_map(|(role_id, role_info)| {
                 utils::user_role::get_role_name_and_permission_response(role_info).map(
                     |(permissions, role_name)| user_role_api::RoleInfoResponse {
@@ -87,34 +88,49 @@ pub async fn update_user_role(
     user_from_token: auth::UserFromToken,
     req: user_role_api::UpdateUserRoleRequest,
 ) -> UserResponse<()> {
-    let merchant_id = user_from_token.merchant_id;
-    let role_id = req.role_id.clone();
-    utils::user_role::validate_role_id(role_id.as_str())?;
+    if !predefined_permissions::is_role_updatable(&req.role_id)? {
+        return Err(UserErrors::InvalidRoleOperation.into())
+            .attach_printable(format!("User role cannot be updated to {}", req.role_id));
+    }
+
+    let user_to_be_updated =
+        utils::user::get_user_from_db_by_email(&state, domain::UserEmail::try_from(req.email)?)
+            .await
+            .to_not_found_response(UserErrors::InvalidRoleOperation)
+            .attach_printable("User not found in our records".to_string())?;
 
-    if user_from_token.user_id == req.user_id {
+    if user_from_token.user_id == user_to_be_updated.get_user_id() {
         return Err(UserErrors::InvalidRoleOperation.into())
-            .attach_printable("Admin User Changing their role");
+            .attach_printable("User Changing their own role");
+    }
+
+    let user_role_to_be_updated = user_to_be_updated
+        .get_role_from_db_by_merchant_id(&state, &user_from_token.merchant_id)
+        .await
+        .to_not_found_response(UserErrors::InvalidRoleOperation)?;
+
+    if !predefined_permissions::is_role_updatable(&user_role_to_be_updated.role_id)? {
+        return Err(UserErrors::InvalidRoleOperation.into()).attach_printable(format!(
+            "User role cannot be updated from {}",
+            user_role_to_be_updated.role_id
+        ));
     }
 
     state
         .store
         .update_user_role_by_user_id_merchant_id(
-            req.user_id.as_str(),
-            merchant_id.as_str(),
+            user_to_be_updated.get_user_id(),
+            user_role_to_be_updated.merchant_id.as_str(),
             UserRoleUpdate::UpdateRole {
-                role_id,
+                role_id: req.role_id.clone(),
                 modified_by: user_from_token.user_id,
             },
         )
         .await
-        .map_err(|e| {
-            if e.current_context().is_db_not_found() {
-                return e
-                    .change_context(UserErrors::InvalidRoleOperation)
-                    .attach_printable("UserId MerchantId not found");
-            }
-            e.change_context(UserErrors::InternalServerError)
-        })?;
+        .to_not_found_response(UserErrors::InvalidRoleOperation)
+        .attach_printable("User with given email is not found in the organization")?;
+
+    auth::blacklist::insert_user_in_blacklist(&state, user_to_be_updated.get_user_id()).await?;
 
     Ok(ApplicationResponse::StatusOk)
 }
@@ -181,7 +197,7 @@ pub async fn delete_user_role(
         .map_err(|e| {
             if e.current_context().is_db_not_found() {
                 e.change_context(UserErrors::InvalidRoleOperation)
-                    .attach_printable("User not found in records")
+                    .attach_printable("User not found in our records")
             } else {
                 e.change_context(UserErrors::InternalServerError)
             }
@@ -204,9 +220,9 @@ pub async fn delete_user_role(
         .find(|&role| role.merchant_id == user_from_token.merchant_id.as_str())
     {
         Some(user_role) => {
-            if !predefined_permissions::is_role_deletable(&user_role.role_id) {
-                return Err(UserErrors::InvalidRoleId.into())
-                    .attach_printable("Deletion not allowed for users with specific role id");
+            if !predefined_permissions::is_role_deletable(&user_role.role_id)? {
+                return Err(UserErrors::InvalidDeleteOperation.into())
+                    .attach_printable(format!("role_id = {} is not deletable", user_role.role_id));
             }
         }
         None => {