-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(users): add support for tenant level users #6708
base: main
Are you sure you want to change the base?
Conversation
crates/router/src/core/user.rs
Outdated
state: SessionState, | ||
req: user_api::UserOrgCreateRequest, | ||
) -> UserResponse<()> { | ||
let db_organization = crate::types::transformers::ForeignFrom::foreign_from(req.clone()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clean up import.
crates/router/src/core/user.rs
Outdated
let orgs = if matches!(role_info.get_entity_type(), EntityType::Tenant) { | ||
let key_manager_state = &(&state).into(); | ||
state | ||
.store | ||
.list_all_merchant_accounts(key_manager_state, None, None) | ||
.await | ||
.change_context(UserErrors::InternalServerError)? | ||
.into_iter() | ||
.map(|merchant_account| merchant_account.get_org_id().to_owned()) | ||
.collect::<HashSet<_>>() | ||
} else { | ||
state | ||
.global_store | ||
.list_user_roles_by_user_id(ListUserRolesByUserIdPayload { | ||
user_id: user_from_token.user_id.as_str(), | ||
tenant_id: user_from_token | ||
.tenant_id | ||
.as_ref() | ||
.unwrap_or(&state.tenant.tenant_id), | ||
org_id: None, | ||
merchant_id: None, | ||
profile_id: None, | ||
entity_id: None, | ||
version: None, | ||
status: Some(UserStatus::Active), | ||
limit: None, | ||
}) | ||
.await | ||
.change_context(UserErrors::InternalServerError)? | ||
.into_iter() | ||
.filter_map(|user_role| user_role.org_id) | ||
.collect::<HashSet<_>>() | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: Use match instead.
crates/router/src/core/user.rs
Outdated
.change_context(UserErrors::InternalServerError) | ||
.attach_printable("Failed to get merchant list for org")? | ||
.first() | ||
.ok_or(UserErrors::InternalServerError) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be InternalServerError
.
crates/router/src/core/user.rs
Outdated
.await | ||
.change_context(UserErrors::InternalServerError) | ||
.attach_printable("Failed to get merchant list for org")? | ||
.first() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use pop()
.
crates/router/src/core/user.rs
Outdated
|
||
let token = utils::user::generate_jwt_auth_token_with_attributes( | ||
&state, | ||
user_from_token.user_id, | ||
merchant_id.clone(), | ||
request.org_id.clone(), | ||
user_role.role_id.clone(), | ||
user_from_token.role_id.clone(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should come from user_role
.
EntityType::Tenant => { | ||
return Err(UserErrors::InvalidRoleOperationWithMessage( | ||
"Tenant roles are not allowed for this operation".to_string(), | ||
) | ||
.into()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we throw error here? Is dashboard handling this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The query above it will fetch 0 user_roles for entity type tenant where status is invitation_sent
, so we should throw error here instead of sending empty vector, if we have such user role.
conn: &PgPooledConn, | ||
limit: Option<u32>, | ||
offset: Option<u32>, | ||
) -> StorageResult<Vec<Self>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of Self
, can we get only merchant_id
and org_id
, which we don't have to decrypt?
is_invitable: true, | ||
is_deletable: true, | ||
is_updatable: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be false.
impl ForeignFrom<api_models::user::UserOrgCreateRequest> | ||
for diesel_models::organization::OrganizationNew | ||
{ | ||
fn foreign_from(item: api_models::user::UserOrgCreateRequest) -> Self { | ||
let org_new = api_models::organization::OrganizationNew::new(None); | ||
let api_models::user::UserOrgCreateRequest { | ||
organization_name, | ||
organization_details, | ||
metadata, | ||
.. | ||
} = item; | ||
let mut org_new_db = Self::new(org_new.org_id, Some(organization_name)); | ||
org_new_db.organization_details = organization_details; | ||
org_new_db.metadata = metadata; | ||
org_new_db | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check if we should we keep it here?
let key_manager_state = &(&*state).into(); | ||
|
||
let merchant_account = state | ||
.store | ||
.list_all_merchant_accounts(key_manager_state, Some(1), None) | ||
.await | ||
.change_context(UserErrors::InternalServerError)? | ||
.pop() | ||
.ok_or(UserErrors::InternalServerError)?; | ||
|
||
let merchant_id = merchant_account.get_id().to_owned(); | ||
let org_id = merchant_account.get_org_id().to_owned(); | ||
|
||
let key_store = state | ||
.store | ||
.get_merchant_key_store_by_merchant_id( | ||
&state.into(), | ||
&merchant_id, | ||
&state.store.get_master_key().to_vec().into(), | ||
) | ||
.await | ||
.change_context(UserErrors::InternalServerError)?; | ||
|
||
let profile_id = state | ||
.store | ||
.list_profile_by_merchant_id(&state.into(), &key_store, &merchant_id) | ||
.await | ||
.change_context(UserErrors::InternalServerError)? | ||
.pop() | ||
.ok_or(UserErrors::InternalServerError)? | ||
.get_id() | ||
.to_owned(); | ||
(org_id, merchant_id, profile_id) | ||
} | ||
_ => { | ||
let org_id = user_role | ||
.org_id | ||
.clone() | ||
.ok_or(report!(UserErrors::InternalServerError)) | ||
.attach_printable("org_id not found")?; | ||
|
||
let (merchant_id, profile_id) = | ||
utils::user_role::get_single_merchant_id_and_profile_id(state, user_role) | ||
.await?; | ||
(org_id, merchant_id, profile_id) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to use util function.
Type of Change
Description
Additional Changes
Motivation and Context
Closes #6707
How did you test it?
Api for create tenant:
Response: 200 Ok, if tenant_admin got created successfully
A tenant can login with email and password and continue with 2FA, he will land into any one of organization existing for tenant.
Create Org Api, (works for tenant users only)
Response will be 200 OK if the org got created success fully (1 org with 1 merchant and 1 profile)
After getting the login token tenant admin can switch to any org in the tenancy.
Checklist
cargo +nightly fmt --all
cargo clippy