Skip to content

Commit

Permalink
refactor(users): Make profile_id in the JWT non-optional (#6537)
Browse files Browse the repository at this point in the history
  • Loading branch information
ThisIsMani authored Nov 18, 2024
1 parent df32e82 commit d32397f
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 110 deletions.
7 changes: 2 additions & 5 deletions crates/diesel_models/src/query/user_role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl UserRole {
user_id: String,
org_id: id_type::OrganizationId,
merchant_id: id_type::MerchantId,
profile_id: Option<id_type::ProfileId>,
profile_id: id_type::ProfileId,
version: UserRoleVersion,
) -> StorageResult<Self> {
// Checking in user roles, for a user in token hierarchy, only one of the relation will be true, either org level, merchant level or profile level
Expand All @@ -103,7 +103,6 @@ impl UserRole {
.or(dsl::org_id.eq(org_id).and(
dsl::merchant_id
.eq(merchant_id)
//TODO: In case of None, profile_id = NULL its unexpected behaviour, after V1 profile id will not be option
.and(dsl::profile_id.eq(profile_id)),
));

Expand Down Expand Up @@ -137,7 +136,6 @@ impl UserRole {
.or(dsl::org_id.eq(org_id).and(
dsl::merchant_id
.eq(merchant_id)
//TODO: In case of None, profile_id = NULL its unexpected behaviour, after V1 profile id will not be option
.and(dsl::profile_id.eq(profile_id)),
));

Expand All @@ -160,7 +158,7 @@ impl UserRole {
user_id: String,
org_id: id_type::OrganizationId,
merchant_id: id_type::MerchantId,
profile_id: Option<id_type::ProfileId>,
profile_id: id_type::ProfileId,
version: UserRoleVersion,
) -> StorageResult<Self> {
// Checking in user roles, for a user in token hierarchy, only one of the relation will be true, either org level, merchant level or profile level
Expand All @@ -176,7 +174,6 @@ impl UserRole {
.or(dsl::org_id.eq(org_id).and(
dsl::merchant_id
.eq(merchant_id)
//TODO: In case of None, profile_id = NULL its unexpected behaviour, after V1 profile id will not be option
.and(dsl::profile_id.eq(profile_id)),
));

Expand Down
61 changes: 18 additions & 43 deletions crates/router/src/core/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,7 @@ pub async fn get_user_details(
org_id: user_from_token.org_id,
is_two_factor_auth_setup: user.get_totp_status() == TotpStatus::Set,
recovery_codes_left: user.get_recovery_codes().map(|codes| codes.len()),
profile_id: user_from_token
.profile_id
.ok_or(UserErrors::JwtProfileIdMissing)?,
profile_id: user_from_token.profile_id,
entity_type: role_info.get_entity_type(),
},
))
Expand Down Expand Up @@ -603,7 +601,7 @@ async fn handle_existing_user_invitation(
invitee_user_from_db.get_user_id(),
&user_from_token.org_id,
&user_from_token.merchant_id,
user_from_token.profile_id.as_ref(),
&user_from_token.profile_id,
UserRoleVersion::V1,
)
.await
Expand All @@ -619,7 +617,7 @@ async fn handle_existing_user_invitation(
invitee_user_from_db.get_user_id(),
&user_from_token.org_id,
&user_from_token.merchant_id,
user_from_token.profile_id.as_ref(),
&user_from_token.profile_id,
UserRoleVersion::V2,
)
.await
Expand Down Expand Up @@ -673,10 +671,6 @@ async fn handle_existing_user_invitation(
.await?
}
EntityType::Profile => {
let profile_id = user_from_token
.profile_id
.clone()
.ok_or(UserErrors::InternalServerError)?;
user_role
.add_entity(domain::ProfileLevel {
tenant_id: user_from_token
Expand All @@ -685,7 +679,7 @@ async fn handle_existing_user_invitation(
.unwrap_or(state.tenant.tenant_id.clone()),
org_id: user_from_token.org_id.clone(),
merchant_id: user_from_token.merchant_id.clone(),
profile_id: profile_id.clone(),
profile_id: user_from_token.profile_id.clone(),
})
.insert_in_v2(state)
.await?
Expand All @@ -705,16 +699,10 @@ async fn handle_existing_user_invitation(
entity_id: user_from_token.merchant_id.get_string_repr().to_owned(),
entity_type: EntityType::Merchant,
},
EntityType::Profile => {
let profile_id = user_from_token
.profile_id
.clone()
.ok_or(UserErrors::InternalServerError)?;
email_types::Entity {
entity_id: profile_id.get_string_repr().to_owned(),
entity_type: EntityType::Profile,
}
}
EntityType::Profile => email_types::Entity {
entity_id: user_from_token.profile_id.get_string_repr().to_owned(),
entity_type: EntityType::Profile,
},
};

let email_contents = email_types::InviteUser {
Expand Down Expand Up @@ -812,10 +800,6 @@ async fn handle_new_user_invitation(
.await?
}
EntityType::Profile => {
let profile_id = user_from_token
.profile_id
.clone()
.ok_or(UserErrors::InternalServerError)?;
user_role
.add_entity(domain::ProfileLevel {
tenant_id: user_from_token
Expand All @@ -824,7 +808,7 @@ async fn handle_new_user_invitation(
.unwrap_or(state.tenant.tenant_id.clone()),
org_id: user_from_token.org_id.clone(),
merchant_id: user_from_token.merchant_id.clone(),
profile_id: profile_id.clone(),
profile_id: user_from_token.profile_id.clone(),
})
.insert_in_v2(state)
.await?
Expand All @@ -848,16 +832,10 @@ async fn handle_new_user_invitation(
entity_id: user_from_token.merchant_id.get_string_repr().to_owned(),
entity_type: EntityType::Merchant,
},
EntityType::Profile => {
let profile_id = user_from_token
.profile_id
.clone()
.ok_or(UserErrors::InternalServerError)?;
email_types::Entity {
entity_id: profile_id.get_string_repr().to_owned(),
entity_type: EntityType::Profile,
}
}
EntityType::Profile => email_types::Entity {
entity_id: user_from_token.profile_id.get_string_repr().to_owned(),
entity_type: EntityType::Profile,
},
};

let email_contents = email_types::InviteUser {
Expand Down Expand Up @@ -887,7 +865,7 @@ async fn handle_new_user_invitation(
merchant_id: user_from_token.merchant_id.clone(),
org_id: user_from_token.org_id.clone(),
role_id: request.role_id.clone(),
profile_id: None,
profile_id: user_from_token.profile_id.clone(),
tenant_id: user_from_token.tenant_id.clone(),
};

Expand Down Expand Up @@ -939,7 +917,7 @@ pub async fn resend_invite(
user.get_user_id(),
&user_from_token.org_id,
&user_from_token.merchant_id,
user_from_token.profile_id.as_ref(),
&user_from_token.profile_id,
UserRoleVersion::V2,
)
.await
Expand All @@ -962,7 +940,7 @@ pub async fn resend_invite(
user.get_user_id(),
&user_from_token.org_id,
&user_from_token.merchant_id,
user_from_token.profile_id.as_ref(),
&user_from_token.profile_id,
UserRoleVersion::V1,
)
.await
Expand Down Expand Up @@ -1235,10 +1213,7 @@ pub async fn list_user_roles_details(
merchant_id: (requestor_role_info.get_entity_type() <= EntityType::Merchant)
.then_some(&user_from_token.merchant_id),
profile_id: (requestor_role_info.get_entity_type() <= EntityType::Profile)
.then_some(&user_from_token.profile_id)
.cloned()
.flatten()
.as_ref(),
.then_some(&user_from_token.profile_id),
entity_id: None,
version: None,
status: None,
Expand Down Expand Up @@ -2865,7 +2840,7 @@ pub async fn switch_profile_for_user_in_org_and_merchant(
request: user_api::SwitchProfileRequest,
user_from_token: auth::UserFromToken,
) -> UserResponse<user_api::TokenResponse> {
if user_from_token.profile_id == Some(request.profile_id.clone()) {
if user_from_token.profile_id == request.profile_id {
return Err(UserErrors::InvalidRoleOperationWithMessage(
"User switching to same profile".to_string(),
)
Expand Down
22 changes: 9 additions & 13 deletions crates/router/src/core/user_role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub async fn update_user_role(
user_to_be_updated.get_user_id(),
&user_from_token.org_id,
&user_from_token.merchant_id,
user_from_token.profile_id.as_ref(),
&user_from_token.profile_id,
UserRoleVersion::V2,
)
.await
Expand Down Expand Up @@ -215,7 +215,7 @@ pub async fn update_user_role(
user_to_be_updated.get_user_id(),
&user_from_token.org_id,
Some(&user_from_token.merchant_id),
user_from_token.profile_id.as_ref(),
Some(&user_from_token.profile_id),
UserRoleUpdate::UpdateRole {
role_id: req.role_id.clone(),
modified_by: user_from_token.user_id.clone(),
Expand All @@ -234,7 +234,7 @@ pub async fn update_user_role(
user_to_be_updated.get_user_id(),
&user_from_token.org_id,
&user_from_token.merchant_id,
user_from_token.profile_id.as_ref(),
&user_from_token.profile_id,
UserRoleVersion::V1,
)
.await
Expand Down Expand Up @@ -288,7 +288,7 @@ pub async fn update_user_role(
user_to_be_updated.get_user_id(),
&user_from_token.org_id,
Some(&user_from_token.merchant_id),
user_from_token.profile_id.as_ref(),
Some(&user_from_token.profile_id),
UserRoleUpdate::UpdateRole {
role_id: req.role_id.clone(),
modified_by: user_from_token.user_id,
Expand Down Expand Up @@ -475,7 +475,7 @@ pub async fn delete_user_role(
user_from_db.get_user_id(),
&user_from_token.org_id,
&user_from_token.merchant_id,
user_from_token.profile_id.as_ref(),
&user_from_token.profile_id,
UserRoleVersion::V2,
)
.await
Expand Down Expand Up @@ -522,7 +522,7 @@ pub async fn delete_user_role(
user_from_db.get_user_id(),
&user_from_token.org_id,
&user_from_token.merchant_id,
user_from_token.profile_id.as_ref(),
&user_from_token.profile_id,
UserRoleVersion::V2,
)
.await
Expand All @@ -537,7 +537,7 @@ pub async fn delete_user_role(
user_from_db.get_user_id(),
&user_from_token.org_id,
&user_from_token.merchant_id,
user_from_token.profile_id.as_ref(),
&user_from_token.profile_id,
UserRoleVersion::V1,
)
.await
Expand Down Expand Up @@ -584,7 +584,7 @@ pub async fn delete_user_role(
user_from_db.get_user_id(),
&user_from_token.org_id,
&user_from_token.merchant_id,
user_from_token.profile_id.as_ref(),
&user_from_token.profile_id,
UserRoleVersion::V1,
)
.await
Expand Down Expand Up @@ -676,17 +676,13 @@ pub async fn list_users_in_lineage(
.await?
}
EntityType::Profile => {
let Some(profile_id) = user_from_token.profile_id.as_ref() else {
return Err(UserErrors::JwtProfileIdMissing.into());
};

utils::user_role::fetch_user_roles_by_payload(
&state,
ListUserRolesByOrgIdPayload {
user_id: None,
org_id: &user_from_token.org_id,
merchant_id: Some(&user_from_token.merchant_id),
profile_id: Some(profile_id),
profile_id: Some(&user_from_token.profile_id),
version: None,
limit: None,
},
Expand Down
4 changes: 2 additions & 2 deletions crates/router/src/db/kafka_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3030,7 +3030,7 @@ impl UserRoleInterface for KafkaStore {
user_id: &str,
org_id: &id_type::OrganizationId,
merchant_id: &id_type::MerchantId,
profile_id: Option<&id_type::ProfileId>,
profile_id: &id_type::ProfileId,
version: enums::UserRoleVersion,
) -> CustomResult<storage::UserRole, errors::StorageError> {
self.diesel_store
Expand Down Expand Up @@ -3070,7 +3070,7 @@ impl UserRoleInterface for KafkaStore {
user_id: &str,
org_id: &id_type::OrganizationId,
merchant_id: &id_type::MerchantId,
profile_id: Option<&id_type::ProfileId>,
profile_id: &id_type::ProfileId,
version: enums::UserRoleVersion,
) -> CustomResult<storage::UserRole, errors::StorageError> {
self.diesel_store
Expand Down
20 changes: 10 additions & 10 deletions crates/router/src/db/user_role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub trait UserRoleInterface {
user_id: &str,
org_id: &id_type::OrganizationId,
merchant_id: &id_type::MerchantId,
profile_id: Option<&id_type::ProfileId>,
profile_id: &id_type::ProfileId,
version: enums::UserRoleVersion,
) -> CustomResult<storage::UserRole, errors::StorageError>;

Expand All @@ -64,7 +64,7 @@ pub trait UserRoleInterface {
user_id: &str,
org_id: &id_type::OrganizationId,
merchant_id: &id_type::MerchantId,
profile_id: Option<&id_type::ProfileId>,
profile_id: &id_type::ProfileId,
version: enums::UserRoleVersion,
) -> CustomResult<storage::UserRole, errors::StorageError>;

Expand Down Expand Up @@ -100,7 +100,7 @@ impl UserRoleInterface for Store {
user_id: &str,
org_id: &id_type::OrganizationId,
merchant_id: &id_type::MerchantId,
profile_id: Option<&id_type::ProfileId>,
profile_id: &id_type::ProfileId,
version: enums::UserRoleVersion,
) -> CustomResult<storage::UserRole, errors::StorageError> {
let conn = connection::pg_connection_read(self).await?;
Expand All @@ -109,7 +109,7 @@ impl UserRoleInterface for Store {
user_id.to_owned(),
org_id.to_owned(),
merchant_id.to_owned(),
profile_id.cloned(),
profile_id.to_owned(),
version,
)
.await
Expand Down Expand Up @@ -146,7 +146,7 @@ impl UserRoleInterface for Store {
user_id: &str,
org_id: &id_type::OrganizationId,
merchant_id: &id_type::MerchantId,
profile_id: Option<&id_type::ProfileId>,
profile_id: &id_type::ProfileId,
version: enums::UserRoleVersion,
) -> CustomResult<storage::UserRole, errors::StorageError> {
let conn = connection::pg_connection_write(self).await?;
Expand All @@ -155,7 +155,7 @@ impl UserRoleInterface for Store {
user_id.to_owned(),
org_id.to_owned(),
merchant_id.to_owned(),
profile_id.cloned(),
profile_id.to_owned(),
version,
)
.await
Expand Down Expand Up @@ -245,7 +245,7 @@ impl UserRoleInterface for MockDb {
user_id: &str,
org_id: &id_type::OrganizationId,
merchant_id: &id_type::MerchantId,
profile_id: Option<&id_type::ProfileId>,
profile_id: &id_type::ProfileId,
version: enums::UserRoleVersion,
) -> CustomResult<storage::UserRole, errors::StorageError> {
let user_roles = self.user_roles.lock().await;
Expand All @@ -261,7 +261,7 @@ impl UserRoleInterface for MockDb {

let profile_level_check = user_role.org_id.as_ref() == Some(org_id)
&& user_role.merchant_id.as_ref() == Some(merchant_id)
&& user_role.profile_id.as_ref() == profile_id;
&& user_role.profile_id.as_ref() == Some(profile_id);

// Check if any condition matches and the version matches
if user_role.user_id == user_id
Expand Down Expand Up @@ -338,7 +338,7 @@ impl UserRoleInterface for MockDb {
user_id: &str,
org_id: &id_type::OrganizationId,
merchant_id: &id_type::MerchantId,
profile_id: Option<&id_type::ProfileId>,
profile_id: &id_type::ProfileId,
version: enums::UserRoleVersion,
) -> CustomResult<storage::UserRole, errors::StorageError> {
let mut user_roles = self.user_roles.lock().await;
Expand All @@ -355,7 +355,7 @@ impl UserRoleInterface for MockDb {

let profile_level_check = role.org_id.as_ref() == Some(org_id)
&& role.merchant_id.as_ref() == Some(merchant_id)
&& role.profile_id.as_ref() == profile_id;
&& role.profile_id.as_ref() == Some(profile_id);

// Check if the user role matches the conditions and the version matches
role.user_id == user_id
Expand Down
Loading

0 comments on commit d32397f

Please sign in to comment.