From f96a87d08ca003411d63dcd9ef4dda6439d20e07 Mon Sep 17 00:00:00 2001 From: Riddhiagrawal001 <50551695+Riddhiagrawal001@users.noreply.github.com> Date: Tue, 10 Dec 2024 16:49:17 +0530 Subject: [PATCH] refactor(users): remove lineage checks in roles get operations (#6701) Co-authored-by: hyperswitch-bot[bot] <148525504+hyperswitch-bot[bot]@users.noreply.github.com> --- crates/diesel_models/src/query/role.rs | 36 ++++++++++-- crates/diesel_models/src/user_role.rs | 11 ++-- crates/router/src/analytics.rs | 30 ++++------ crates/router/src/core/user.rs | 48 +++++----------- crates/router/src/core/user_role.rs | 26 ++++----- crates/router/src/core/user_role/role.rs | 34 +++++------ crates/router/src/db/kafka_store.rs | 16 +++++- crates/router/src/db/role.rs | 57 +++++++++++++++++-- crates/router/src/services/authorization.rs | 6 +- .../src/services/authorization/roles.rs | 8 +-- .../src/types/domain/user/decision_manager.rs | 6 +- crates/router/src/utils/user.rs | 11 +--- crates/router/src/utils/user_role.rs | 36 ++++-------- .../down.sql | 2 + .../up.sql | 10 ++++ 15 files changed, 188 insertions(+), 149 deletions(-) create mode 100644 migrations/2024-12-02-110129_update-user-role-entity-type/down.sql create mode 100644 migrations/2024-12-02-110129_update-user-role-entity-type/up.sql diff --git a/crates/diesel_models/src/query/role.rs b/crates/diesel_models/src/query/role.rs index 065a5b6e114e..6f6a1404ee2c 100644 --- a/crates/diesel_models/src/query/role.rs +++ b/crates/diesel_models/src/query/role.rs @@ -26,6 +26,7 @@ impl Role { .await } + // TODO: Remove once find_by_role_id_in_lineage is stable pub async fn find_by_role_id_in_merchant_scope( conn: &PgPooledConn, role_id: &str, @@ -43,7 +44,27 @@ impl Role { .await } - pub async fn find_by_role_id_in_org_scope( + pub async fn find_by_role_id_in_lineage( + conn: &PgPooledConn, + role_id: &str, + merchant_id: &id_type::MerchantId, + org_id: &id_type::OrganizationId, + ) -> StorageResult { + generics::generic_find_one::<::Table, _, _>( + conn, + dsl::role_id + .eq(role_id.to_owned()) + .and(dsl::org_id.eq(org_id.to_owned())) + .and( + dsl::scope.eq(RoleScope::Organization).or(dsl::merchant_id + .eq(merchant_id.to_owned()) + .and(dsl::scope.eq(RoleScope::Merchant))), + ), + ) + .await + } + + pub async fn find_by_role_id_and_org_id( conn: &PgPooledConn, role_id: &str, org_id: &id_type::OrganizationId, @@ -88,9 +109,11 @@ impl Role { merchant_id: &id_type::MerchantId, org_id: &id_type::OrganizationId, ) -> StorageResult> { - let predicate = dsl::merchant_id.eq(merchant_id.to_owned()).or(dsl::org_id - .eq(org_id.to_owned()) - .and(dsl::scope.eq(RoleScope::Organization))); + let predicate = dsl::org_id.eq(org_id.to_owned()).and( + dsl::scope.eq(RoleScope::Organization).or(dsl::merchant_id + .eq(merchant_id.to_owned()) + .and(dsl::scope.eq(RoleScope::Merchant))), + ); generics::generic_filter::<::Table, _, _, _>( conn, @@ -115,9 +138,10 @@ impl Role { if let Some(merchant_id) = merchant_id { query = query.filter( - dsl::merchant_id + (dsl::merchant_id .eq(merchant_id) - .or(dsl::scope.eq(RoleScope::Organization)), + .and(dsl::scope.eq(RoleScope::Merchant))) + .or(dsl::scope.eq(RoleScope::Organization)), ); } diff --git a/crates/diesel_models/src/user_role.rs b/crates/diesel_models/src/user_role.rs index 04f3264b45e3..08449685b291 100644 --- a/crates/diesel_models/src/user_role.rs +++ b/crates/diesel_models/src/user_role.rs @@ -29,16 +29,19 @@ pub struct UserRole { impl UserRole { pub fn get_entity_id_and_type(&self) -> Option<(String, EntityType)> { - match (self.version, self.role_id.as_str()) { - (enums::UserRoleVersion::V1, consts::ROLE_ID_ORGANIZATION_ADMIN) => { + match (self.version, self.entity_type, self.role_id.as_str()) { + (enums::UserRoleVersion::V1, None, consts::ROLE_ID_ORGANIZATION_ADMIN) => { let org_id = self.org_id.clone()?.get_string_repr().to_string(); Some((org_id, EntityType::Organization)) } - (enums::UserRoleVersion::V1, _) => { + (enums::UserRoleVersion::V1, None, _) => { let merchant_id = self.merchant_id.clone()?.get_string_repr().to_string(); Some((merchant_id, EntityType::Merchant)) } - (enums::UserRoleVersion::V2, _) => self.entity_id.clone().zip(self.entity_type), + (enums::UserRoleVersion::V1, Some(_), _) => { + self.entity_id.clone().zip(self.entity_type) + } + (enums::UserRoleVersion::V2, _, _) => self.entity_id.clone().zip(self.entity_type), } } } diff --git a/crates/router/src/analytics.rs b/crates/router/src/analytics.rs index 752c3a52284a..ff2744b824ed 100644 --- a/crates/router/src/analytics.rs +++ b/crates/router/src/analytics.rs @@ -1847,15 +1847,10 @@ pub mod routes { json_payload.into_inner(), |state, auth: UserFromToken, req, _| async move { let role_id = auth.role_id; - let role_info = RoleInfo::from_role_id_in_merchant_scope( - &state, - &role_id, - &auth.merchant_id, - &auth.org_id, - ) - .await - .change_context(UserErrors::InternalServerError) - .change_context(OpenSearchError::UnknownError)?; + let role_info = RoleInfo::from_role_id_and_org_id(&state, &role_id, &auth.org_id) + .await + .change_context(UserErrors::InternalServerError) + .change_context(OpenSearchError::UnknownError)?; let permission_groups = role_info.get_permission_groups(); if !permission_groups.contains(&common_enums::PermissionGroup::OperationsView) { return Err(OpenSearchError::AccessForbiddenError)?; @@ -1887,7 +1882,7 @@ pub mod routes { let role_id = user_role.role_id.clone(); let org_id = user_role.org_id.clone().unwrap_or_default(); async move { - RoleInfo::from_role_id_in_org_scope(&state, &role_id, &org_id) + RoleInfo::from_role_id_and_org_id(&state, &role_id, &org_id) .await .change_context(UserErrors::InternalServerError) .change_context(OpenSearchError::UnknownError) @@ -1974,15 +1969,10 @@ pub mod routes { indexed_req, |state, auth: UserFromToken, req, _| async move { let role_id = auth.role_id; - let role_info = RoleInfo::from_role_id_in_merchant_scope( - &state, - &role_id, - &auth.merchant_id, - &auth.org_id, - ) - .await - .change_context(UserErrors::InternalServerError) - .change_context(OpenSearchError::UnknownError)?; + let role_info = RoleInfo::from_role_id_and_org_id(&state, &role_id, &auth.org_id) + .await + .change_context(UserErrors::InternalServerError) + .change_context(OpenSearchError::UnknownError)?; let permission_groups = role_info.get_permission_groups(); if !permission_groups.contains(&common_enums::PermissionGroup::OperationsView) { return Err(OpenSearchError::AccessForbiddenError)?; @@ -2013,7 +2003,7 @@ pub mod routes { let role_id = user_role.role_id.clone(); let org_id = user_role.org_id.clone().unwrap_or_default(); async move { - RoleInfo::from_role_id_in_org_scope(&state, &role_id, &org_id) + RoleInfo::from_role_id_and_org_id(&state, &role_id, &org_id) .await .change_context(UserErrors::InternalServerError) .change_context(OpenSearchError::UnknownError) diff --git a/crates/router/src/core/user.rs b/crates/router/src/core/user.rs index f99c44cf2980..629476b2591e 100644 --- a/crates/router/src/core/user.rs +++ b/crates/router/src/core/user.rs @@ -104,10 +104,9 @@ pub async fn get_user_details( ) -> UserResponse { let user = user_from_token.get_user_from_db(&state).await?; let verification_days_left = utils::user::get_verification_days_left(&state, &user)?; - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_info = roles::RoleInfo::from_role_id_and_org_id( &state, &user_from_token.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -553,7 +552,7 @@ async fn handle_invitation( .into()); } - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_info = roles::RoleInfo::from_role_id_in_lineage( state, &request.role_id, &user_from_token.merchant_id, @@ -1371,10 +1370,9 @@ pub async fn list_user_roles_details( .await .to_not_found_response(UserErrors::InvalidRoleOperation)?; - let requestor_role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let requestor_role_info = roles::RoleInfo::from_role_id_and_org_id( &state, &user_from_token.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -1526,7 +1524,7 @@ pub async fn list_user_roles_details( .collect::>() .into_iter() .map(|role_id| async { - let role_info = roles::RoleInfo::from_role_id_in_org_scope( + let role_info = roles::RoleInfo::from_role_id_and_org_id( &state, &role_id, &user_from_token.org_id, @@ -2533,10 +2531,9 @@ pub async fn list_orgs_for_user( state: SessionState, user_from_token: auth::UserFromToken, ) -> UserResponse> { - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_info = roles::RoleInfo::from_role_id_and_org_id( &state, &user_from_token.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -2611,10 +2608,9 @@ pub async fn list_merchants_for_user_in_org( state: SessionState, user_from_token: auth::UserFromToken, ) -> UserResponse> { - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_info = roles::RoleInfo::from_role_id_and_org_id( &state, &user_from_token.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -2687,10 +2683,9 @@ pub async fn list_profiles_for_user_in_org_and_merchant_account( state: SessionState, user_from_token: auth::UserFromToken, ) -> UserResponse> { - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_info = roles::RoleInfo::from_role_id_and_org_id( &state, &user_from_token.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -2780,10 +2775,9 @@ pub async fn switch_org_for_user( .into()); } - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_info = roles::RoleInfo::from_role_id_and_org_id( &state, &user_from_token.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -2876,13 +2870,8 @@ pub async fn switch_org_for_user( ) .await?; - utils::user_role::set_role_permissions_in_cache_by_role_id_merchant_id_org_id( - &state, - &role_id, - &merchant_id, - &request.org_id, - ) - .await; + utils::user_role::set_role_info_in_cache_by_role_id_org_id(&state, &role_id, &request.org_id) + .await; let response = user_api::TokenResponse { token: token.clone(), @@ -2905,10 +2894,9 @@ pub async fn switch_merchant_for_user_in_org( } let key_manager_state = &(&state).into(); - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_info = roles::RoleInfo::from_role_id_and_org_id( &state, &user_from_token.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -3065,13 +3053,7 @@ pub async fn switch_merchant_for_user_in_org( ) .await?; - utils::user_role::set_role_permissions_in_cache_by_role_id_merchant_id_org_id( - &state, - &role_id, - &merchant_id, - &org_id, - ) - .await; + utils::user_role::set_role_info_in_cache_by_role_id_org_id(&state, &role_id, &org_id).await; let response = user_api::TokenResponse { token: token.clone(), @@ -3094,10 +3076,9 @@ pub async fn switch_profile_for_user_in_org_and_merchant( } let key_manager_state = &(&state).into(); - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_info = roles::RoleInfo::from_role_id_and_org_id( &state, &user_from_token.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -3175,10 +3156,9 @@ pub async fn switch_profile_for_user_in_org_and_merchant( ) .await?; - utils::user_role::set_role_permissions_in_cache_by_role_id_merchant_id_org_id( + utils::user_role::set_role_info_in_cache_by_role_id_org_id( &state, &role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await; diff --git a/crates/router/src/core/user_role.rs b/crates/router/src/core/user_role.rs index 31ec665b2ab2..d8fdff0e6233 100644 --- a/crates/router/src/core/user_role.rs +++ b/crates/router/src/core/user_role.rs @@ -83,10 +83,9 @@ pub async fn get_parent_group_info( state: SessionState, user_from_token: auth::UserFromToken, ) -> UserResponse> { - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_info = roles::RoleInfo::from_role_id_and_org_id( &state, &user_from_token.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -119,7 +118,7 @@ pub async fn update_user_role( req: user_role_api::UpdateUserRoleRequest, _req_state: ReqState, ) -> UserResponse<()> { - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_info = roles::RoleInfo::from_role_id_in_lineage( &state, &req.role_id, &user_from_token.merchant_id, @@ -144,10 +143,9 @@ pub async fn update_user_role( .attach_printable("User Changing their own role"); } - let updator_role = roles::RoleInfo::from_role_id_in_merchant_scope( + let updator_role = roles::RoleInfo::from_role_id_and_org_id( &state, &user_from_token.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -181,10 +179,9 @@ pub async fn update_user_role( }; if let Some(user_role) = v2_user_role_to_be_updated { - let role_to_be_updated = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_to_be_updated = roles::RoleInfo::from_role_id_and_org_id( &state, &user_role.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -262,10 +259,9 @@ pub async fn update_user_role( }; if let Some(user_role) = v1_user_role_to_be_updated { - let role_to_be_updated = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_to_be_updated = roles::RoleInfo::from_role_id_and_org_id( &state, &user_role.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -489,10 +485,9 @@ pub async fn delete_user_role( .attach_printable("User deleting himself"); } - let deletion_requestor_role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let deletion_requestor_role_info = roles::RoleInfo::from_role_id_and_org_id( &state, &user_from_token.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -527,7 +522,7 @@ pub async fn delete_user_role( }; if let Some(role_to_be_deleted) = user_role_v2 { - let target_role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let target_role_info = roles::RoleInfo::from_role_id_in_lineage( &state, &role_to_be_deleted.role_id, &user_from_token.merchant_id, @@ -597,7 +592,7 @@ pub async fn delete_user_role( }; if let Some(role_to_be_deleted) = user_role_v1 { - let target_role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let target_role_info = roles::RoleInfo::from_role_id_in_lineage( &state, &role_to_be_deleted.role_id, &user_from_token.merchant_id, @@ -685,10 +680,9 @@ pub async fn list_users_in_lineage( user_from_token: auth::UserFromToken, request: user_role_api::ListUsersInEntityRequest, ) -> UserResponse> { - let requestor_role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let requestor_role_info = roles::RoleInfo::from_role_id_and_org_id( &state, &user_from_token.role_id, - &user_from_token.merchant_id, &user_from_token.org_id, ) .await @@ -783,7 +777,7 @@ pub async fn list_users_in_lineage( let role_info_map = futures::future::try_join_all(user_roles_set.iter().map(|user_role| async { - roles::RoleInfo::from_role_id_in_org_scope( + roles::RoleInfo::from_role_id_and_org_id( &state, &user_role.role_id, &user_from_token.org_id, diff --git a/crates/router/src/core/user_role/role.rs b/crates/router/src/core/user_role/role.rs index 9f251f55bdf0..714bf9fed3cc 100644 --- a/crates/router/src/core/user_role/role.rs +++ b/crates/router/src/core/user_role/role.rs @@ -76,8 +76,10 @@ pub async fn create_role( ) .await?; + let user_role_info = user_from_token.get_role_info_from_db(&state).await?; + if matches!(req.role_scope, RoleScope::Organization) - && user_from_token.role_id != common_utils::consts::ROLE_ID_ORGANIZATION_ADMIN + && user_role_info.get_entity_type() != EntityType::Organization { return Err(report!(UserErrors::InvalidRoleOperation)) .attach_printable("Non org admin user creating org level role"); @@ -116,14 +118,10 @@ pub async fn get_role_with_groups( user_from_token: UserFromToken, role: role_api::GetRoleRequest, ) -> UserResponse { - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( - &state, - &role.role_id, - &user_from_token.merchant_id, - &user_from_token.org_id, - ) - .await - .to_not_found_response(UserErrors::InvalidRoleId)?; + let role_info = + roles::RoleInfo::from_role_id_and_org_id(&state, &role.role_id, &user_from_token.org_id) + .await + .to_not_found_response(UserErrors::InvalidRoleId)?; if role_info.is_internal() { return Err(UserErrors::InvalidRoleId.into()); @@ -144,14 +142,10 @@ pub async fn get_parent_info_for_role( user_from_token: UserFromToken, role: role_api::GetRoleRequest, ) -> UserResponse { - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( - &state, - &role.role_id, - &user_from_token.merchant_id, - &user_from_token.org_id, - ) - .await - .to_not_found_response(UserErrors::InvalidRoleId)?; + let role_info = + roles::RoleInfo::from_role_id_and_org_id(&state, &role.role_id, &user_from_token.org_id) + .await + .to_not_found_response(UserErrors::InvalidRoleId)?; if role_info.is_internal() { return Err(UserErrors::InvalidRoleId.into()); @@ -207,7 +201,7 @@ pub async fn update_role( utils::user_role::validate_role_groups(groups)?; } - let role_info = roles::RoleInfo::from_role_id_in_merchant_scope( + let role_info = roles::RoleInfo::from_role_id_in_lineage( &state, role_id, &user_from_token.merchant_id, @@ -216,8 +210,10 @@ pub async fn update_role( .await .to_not_found_response(UserErrors::InvalidRoleOperation)?; + let user_role_info = user_from_token.get_role_info_from_db(&state).await?; + if matches!(role_info.get_scope(), RoleScope::Organization) - && user_from_token.role_id != common_utils::consts::ROLE_ID_ORGANIZATION_ADMIN + && user_role_info.get_entity_type() != EntityType::Organization { return Err(report!(UserErrors::InvalidRoleOperation)) .attach_printable("Non org admin user changing org level role"); diff --git a/crates/router/src/db/kafka_store.rs b/crates/router/src/db/kafka_store.rs index 47c9adafb718..2fd30a3610b5 100644 --- a/crates/router/src/db/kafka_store.rs +++ b/crates/router/src/db/kafka_store.rs @@ -3521,6 +3521,7 @@ impl RoleInterface for KafkaStore { self.diesel_store.find_role_by_role_id(role_id).await } + //TODO:Remove once find_by_role_id_in_lineage is stable async fn find_role_by_role_id_in_merchant_scope( &self, role_id: &str, @@ -3532,13 +3533,24 @@ impl RoleInterface for KafkaStore { .await } - async fn find_role_by_role_id_in_org_scope( + async fn find_role_by_role_id_in_lineage( + &self, + role_id: &str, + merchant_id: &id_type::MerchantId, + org_id: &id_type::OrganizationId, + ) -> CustomResult { + self.diesel_store + .find_role_by_role_id_in_lineage(role_id, merchant_id, org_id) + .await + } + + async fn find_by_role_id_and_org_id( &self, role_id: &str, org_id: &id_type::OrganizationId, ) -> CustomResult { self.diesel_store - .find_role_by_role_id_in_org_scope(role_id, org_id) + .find_by_role_id_and_org_id(role_id, org_id) .await } diff --git a/crates/router/src/db/role.rs b/crates/router/src/db/role.rs index d13508356e5a..877a4c540774 100644 --- a/crates/router/src/db/role.rs +++ b/crates/router/src/db/role.rs @@ -23,6 +23,7 @@ pub trait RoleInterface { role_id: &str, ) -> CustomResult; + //TODO:Remove once find_by_role_id_in_lineage is stable async fn find_role_by_role_id_in_merchant_scope( &self, role_id: &str, @@ -30,7 +31,14 @@ pub trait RoleInterface { org_id: &id_type::OrganizationId, ) -> CustomResult; - async fn find_role_by_role_id_in_org_scope( + async fn find_role_by_role_id_in_lineage( + &self, + role_id: &str, + merchant_id: &id_type::MerchantId, + org_id: &id_type::OrganizationId, + ) -> CustomResult; + + async fn find_by_role_id_and_org_id( &self, role_id: &str, org_id: &id_type::OrganizationId, @@ -86,6 +94,7 @@ impl RoleInterface for Store { .map_err(|error| report!(errors::StorageError::from(error))) } + //TODO:Remove once find_by_role_id_in_lineage is stable #[instrument(skip_all)] async fn find_role_by_role_id_in_merchant_scope( &self, @@ -100,13 +109,26 @@ impl RoleInterface for Store { } #[instrument(skip_all)] - async fn find_role_by_role_id_in_org_scope( + async fn find_role_by_role_id_in_lineage( &self, role_id: &str, + merchant_id: &id_type::MerchantId, org_id: &id_type::OrganizationId, ) -> CustomResult { let conn = connection::pg_connection_read(self).await?; - storage::Role::find_by_role_id_in_org_scope(&conn, role_id, org_id) + storage::Role::find_by_role_id_in_lineage(&conn, role_id, merchant_id, org_id) + .await + .map_err(|error| report!(errors::StorageError::from(error))) + } + + #[instrument(skip_all)] + async fn find_by_role_id_and_org_id( + &self, + role_id: &str, + org_id: &id_type::OrganizationId, + ) -> CustomResult { + let conn = connection::pg_connection_read(self).await?; + storage::Role::find_by_role_id_and_org_id(&conn, role_id, org_id) .await .map_err(|error| report!(errors::StorageError::from(error))) } @@ -217,6 +239,7 @@ impl RoleInterface for MockDb { ) } + // TODO: Remove once find_by_role_id_in_lineage is stable async fn find_role_by_role_id_in_merchant_scope( &self, role_id: &str, @@ -241,7 +264,33 @@ impl RoleInterface for MockDb { ) } - async fn find_role_by_role_id_in_org_scope( + async fn find_role_by_role_id_in_lineage( + &self, + role_id: &str, + merchant_id: &id_type::MerchantId, + org_id: &id_type::OrganizationId, + ) -> CustomResult { + let roles = self.roles.lock().await; + roles + .iter() + .find(|role| { + role.role_id == role_id + && role.org_id == *org_id + && ((role.scope == enums::RoleScope::Organization) + || (role.merchant_id == *merchant_id + && role.scope == enums::RoleScope::Merchant)) + }) + .cloned() + .ok_or( + errors::StorageError::ValueNotFound(format!( + "No role available in merchant scope for role_id = {role_id}, \ + merchant_id = {merchant_id:?} and org_id = {org_id:?}" + )) + .into(), + ) + } + + async fn find_by_role_id_and_org_id( &self, role_id: &str, org_id: &id_type::OrganizationId, diff --git a/crates/router/src/services/authorization.rs b/crates/router/src/services/authorization.rs index e0feaa5e817a..2b7e8bd73406 100644 --- a/crates/router/src/services/authorization.rs +++ b/crates/router/src/services/authorization.rs @@ -33,8 +33,7 @@ where return Ok(role_info.clone()); } - let role_info = - get_role_info_from_db(state, &token.role_id, &token.merchant_id, &token.org_id).await?; + let role_info = get_role_info_from_db(state, &token.role_id, &token.org_id).await?; let token_expiry = i64::try_from(token.exp).change_context(ApiErrorResponse::InternalServerError)?; @@ -68,7 +67,6 @@ pub fn get_cache_key_from_role_id(role_id: &str) -> String { async fn get_role_info_from_db( state: &A, role_id: &str, - merchant_id: &id_type::MerchantId, org_id: &id_type::OrganizationId, ) -> RouterResult where @@ -76,7 +74,7 @@ where { state .store() - .find_role_by_role_id_in_merchant_scope(role_id, merchant_id, org_id) + .find_by_role_id_and_org_id(role_id, org_id) .await .map(roles::RoleInfo::from) .to_not_found_response(ApiErrorResponse::InvalidJwtToken) diff --git a/crates/router/src/services/authorization/roles.rs b/crates/router/src/services/authorization/roles.rs index a6c5cee1c4ea..dcffa3107c91 100644 --- a/crates/router/src/services/authorization/roles.rs +++ b/crates/router/src/services/authorization/roles.rs @@ -116,7 +116,7 @@ impl RoleInfo { acl } - pub async fn from_role_id_in_merchant_scope( + pub async fn from_role_id_in_lineage( state: &SessionState, role_id: &str, merchant_id: &id_type::MerchantId, @@ -127,13 +127,13 @@ impl RoleInfo { } else { state .store - .find_role_by_role_id_in_merchant_scope(role_id, merchant_id, org_id) + .find_role_by_role_id_in_lineage(role_id, merchant_id, org_id) .await .map(Self::from) } } - pub async fn from_role_id_in_org_scope( + pub async fn from_role_id_and_org_id( state: &SessionState, role_id: &str, org_id: &id_type::OrganizationId, @@ -143,7 +143,7 @@ impl RoleInfo { } else { state .store - .find_role_by_role_id_in_org_scope(role_id, org_id) + .find_by_role_id_and_org_id(role_id, org_id) .await .map(Self::from) } diff --git a/crates/router/src/types/domain/user/decision_manager.rs b/crates/router/src/types/domain/user/decision_manager.rs index 519edf4e9ce5..bf5556dd9a75 100644 --- a/crates/router/src/types/domain/user/decision_manager.rs +++ b/crates/router/src/types/domain/user/decision_manager.rs @@ -337,8 +337,7 @@ impl NextFlow { .change_context(UserErrors::InternalServerError)? .pop() .ok_or(UserErrors::InternalServerError)?; - utils::user_role::set_role_permissions_in_cache_by_user_role(state, &user_role) - .await; + utils::user_role::set_role_info_in_cache_by_user_role(state, &user_role).await; jwt_flow.generate_jwt(state, self, &user_role).await } @@ -357,8 +356,7 @@ impl NextFlow { { self.user.get_verification_days_left(state)?; } - utils::user_role::set_role_permissions_in_cache_by_user_role(state, user_role) - .await; + utils::user_role::set_role_info_in_cache_by_user_role(state, user_role).await; jwt_flow.generate_jwt(state, self, user_role).await } diff --git a/crates/router/src/utils/user.rs b/crates/router/src/utils/user.rs index 9886705ec66b..abd59684243c 100644 --- a/crates/router/src/utils/user.rs +++ b/crates/router/src/utils/user.rs @@ -77,14 +77,9 @@ impl UserFromToken { } pub async fn get_role_info_from_db(&self, state: &SessionState) -> UserResult { - RoleInfo::from_role_id_in_merchant_scope( - state, - &self.role_id, - &self.merchant_id, - &self.org_id, - ) - .await - .change_context(UserErrors::InternalServerError) + RoleInfo::from_role_id_and_org_id(state, &self.role_id, &self.org_id) + .await + .change_context(UserErrors::InternalServerError) } } diff --git a/crates/router/src/utils/user_role.rs b/crates/router/src/utils/user_role.rs index 3412219d0e97..b8f93bff943c 100644 --- a/crates/router/src/utils/user_role.rs +++ b/crates/router/src/utils/user_role.rs @@ -71,55 +71,43 @@ pub async fn validate_role_name( Ok(()) } -pub async fn set_role_permissions_in_cache_by_user_role( +pub async fn set_role_info_in_cache_by_user_role( state: &SessionState, user_role: &UserRole, ) -> bool { - let Some(ref merchant_id) = user_role.merchant_id else { - return false; - }; - let Some(ref org_id) = user_role.org_id else { return false; }; - set_role_permissions_in_cache_if_required( - state, - user_role.role_id.as_str(), - merchant_id, - org_id, - ) - .await - .map_err(|e| logger::error!("Error setting permissions in cache {:?}", e)) - .is_ok() + set_role_info_in_cache_if_required(state, user_role.role_id.as_str(), org_id) + .await + .map_err(|e| logger::error!("Error setting permissions in cache {:?}", e)) + .is_ok() } -pub async fn set_role_permissions_in_cache_by_role_id_merchant_id_org_id( +pub async fn set_role_info_in_cache_by_role_id_org_id( state: &SessionState, role_id: &str, - merchant_id: &id_type::MerchantId, org_id: &id_type::OrganizationId, ) -> bool { - set_role_permissions_in_cache_if_required(state, role_id, merchant_id, org_id) + set_role_info_in_cache_if_required(state, role_id, org_id) .await .map_err(|e| logger::error!("Error setting permissions in cache {:?}", e)) .is_ok() } -pub async fn set_role_permissions_in_cache_if_required( +pub async fn set_role_info_in_cache_if_required( state: &SessionState, role_id: &str, - merchant_id: &id_type::MerchantId, org_id: &id_type::OrganizationId, ) -> UserResult<()> { if roles::predefined_roles::PREDEFINED_ROLES.contains_key(role_id) { return Ok(()); } - let role_info = - roles::RoleInfo::from_role_id_in_merchant_scope(state, role_id, merchant_id, org_id) - .await - .change_context(UserErrors::InternalServerError) - .attach_printable("Error getting role_info from role_id")?; + let role_info = roles::RoleInfo::from_role_id_and_org_id(state, role_id, org_id) + .await + .change_context(UserErrors::InternalServerError) + .attach_printable("Error getting role_info from role_id")?; authz::set_role_info_in_cache( state, diff --git a/migrations/2024-12-02-110129_update-user-role-entity-type/down.sql b/migrations/2024-12-02-110129_update-user-role-entity-type/down.sql new file mode 100644 index 000000000000..c7c9cbeb4017 --- /dev/null +++ b/migrations/2024-12-02-110129_update-user-role-entity-type/down.sql @@ -0,0 +1,2 @@ +-- This file should undo anything in `up.sql` +SELECT 1; \ No newline at end of file diff --git a/migrations/2024-12-02-110129_update-user-role-entity-type/up.sql b/migrations/2024-12-02-110129_update-user-role-entity-type/up.sql new file mode 100644 index 000000000000..f2759f030d50 --- /dev/null +++ b/migrations/2024-12-02-110129_update-user-role-entity-type/up.sql @@ -0,0 +1,10 @@ +-- Your SQL goes here +UPDATE user_roles +SET + entity_type = CASE + WHEN role_id = 'org_admin' THEN 'organization' + ELSE 'merchant' + END +WHERE + version = 'v1' + AND entity_type IS NULL; \ No newline at end of file