From af8fd1f270b225d98c8b1dadc420e49fc4ea8f95 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Sun, 31 Aug 2025 10:37:47 -0700 Subject: [PATCH 01/15] fix: use userid, username and method in a usergroup new structure for POST /usergroup ``` { "name": "parseable", "roles": [], "users": [ { "username": "nikhil", "method": "native" } ] } ``` new structure for add/remove user - PATCH /usergroup/{name}/add(remove) ``` { "users": [ { "username": "authentik Default Admin", "method": "oauth" } ] } ``` --- .../http/modal/ingest/ingestor_role.rs | 6 +- src/handlers/http/modal/query/querier_rbac.rs | 22 ++-- src/handlers/http/modal/query/querier_role.rs | 6 +- src/handlers/http/oidc.rs | 9 +- src/handlers/http/rbac.rs | 10 +- src/handlers/http/role.rs | 6 +- src/rbac/user.rs | 113 +++++++++++++++--- 7 files changed, 129 insertions(+), 43 deletions(-) diff --git a/src/handlers/http/modal/ingest/ingestor_role.rs b/src/handlers/http/modal/ingest/ingestor_role.rs index 5744eb3fe..68fb2f590 100644 --- a/src/handlers/http/modal/ingest/ingestor_role.rs +++ b/src/handlers/http/modal/ingest/ingestor_role.rs @@ -48,14 +48,14 @@ pub async fn put( // refresh the sessions of all users using this role // for this, iterate over all user_groups and users and create a hashset of users let mut session_refresh_users: HashSet = HashSet::new(); - for user_group in read_user_groups().values().cloned() { + for user_group in read_user_groups().values() { if user_group.roles.contains(&name) { - session_refresh_users.extend(user_group.users); + session_refresh_users.extend(user_group.get_usernames()); } } // iterate over all users to see if they have this role - for user in users().values().cloned() { + for user in users().values() { if user.roles.contains(&name) { session_refresh_users.insert(user.username().to_string()); } diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index c28a3c9f3..43fc7a7b9 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -103,7 +103,7 @@ pub async fn post_user( Ok(password) } -// Handler for DELETE /api/v1/user/delete/{username} +// Handler for DELETE /api/v1/user/{username} pub async fn delete_user(username: web::Path) -> Result { let username = username.into_inner(); let _ = UPDATE_LOCK.lock().await; @@ -120,19 +120,25 @@ pub async fn delete_user(username: web::Path) -> Result = HashSet::new(); - for user_group in read_user_groups().values().cloned() { + for user_group in read_user_groups().values() { if user_group.roles.contains(&name) { - session_refresh_users.extend(user_group.users); + session_refresh_users.extend(user_group.get_usernames()); } } // iterate over all users to see if they have this role - for user in users().values().cloned() { + for user in users().values() { if user.roles.contains(&name) { session_refresh_users.insert(user.username().to_string()); } diff --git a/src/handlers/http/oidc.rs b/src/handlers/http/oidc.rs index d54712670..bd73890c9 100644 --- a/src/handlers/http/oidc.rs +++ b/src/handlers/http/oidc.rs @@ -38,7 +38,7 @@ use crate::{ rbac::{ self, Users, map::{DEFAULT_ROLE, SessionKey}, - user::{self, User, UserType}, + user::{self, GroupUser, User, UserType}, }, storage::{self, ObjectStorageError, StorageMetadata}, utils::actix::extract_session_key_from_req, @@ -432,12 +432,11 @@ pub async fn update_user_if_changed( entry.clone_from(&user); // migrate user references inside user groups for group in metadata.user_groups.iter_mut() { - if group.users.remove(&old_username) { - group.users.insert(user.username().to_string()); - } + group.users.retain(|u| u.userid() != old_username); + group.users.insert(GroupUser::from_user(&user)); } - put_metadata(&metadata).await?; } + put_metadata(&metadata).await?; Users.delete_user(&old_username); Users.put_user(user.clone()); Ok(user) diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index 3bb320f4a..6f8f46dc4 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -42,7 +42,7 @@ use tokio::sync::Mutex; use super::modal::utils::rbac_utils::{get_metadata, put_metadata}; -// async aware lock for updating storage metadata and user map atomicically +// async aware lock for updating storage metadata and user map atomically static UPDATE_LOCK: Mutex<()> = Mutex::const_new(()); #[derive(serde::Serialize)] @@ -66,13 +66,13 @@ impl From<&user::User> for User { } // Handler for GET /api/v1/user -// returns list of all registerd users +// returns list of all registered users pub async fn list_users() -> impl Responder { web::Json(Users.collect_user::()) } /// Handler for GET /api/v1/users -/// returns list of all registerd users along with their roles and other info +/// returns list of all registered users along with their roles and other info pub async fn list_users_prism() -> impl Responder { // get all users let prism_users = rbac::map::users().values().map(to_prism_user).collect_vec(); @@ -350,7 +350,7 @@ pub async fn remove_roles_from_user( } #[derive(Debug, Serialize)] -#[serde(rename = "camelCase")] +#[serde(rename_all = "camelCase")] pub struct InvalidUserGroupError { pub valid_name: bool, pub non_existent_roles: Vec, @@ -390,7 +390,7 @@ pub enum RBACError { InvalidUserGroupRequest(Box), #[error("{0}")] InvalidSyncOperation(String), - #[error("User group `{0}` is still being used")] + #[error("UserGroup `{0}` is still in use")] UserGroupNotEmpty(String), #[error("Resource `{0}` is still in use")] ResourceInUse(String), diff --git a/src/handlers/http/role.rs b/src/handlers/http/role.rs index b2462dc08..fef46a5eb 100644 --- a/src/handlers/http/role.rs +++ b/src/handlers/http/role.rs @@ -50,14 +50,14 @@ pub async fn put( // refresh the sessions of all users using this role // for this, iterate over all user_groups and users and create a hashset of users let mut session_refresh_users: HashSet = HashSet::new(); - for user_group in read_user_groups().values().cloned() { + for user_group in read_user_groups().values() { if user_group.roles.contains(&name) { - session_refresh_users.extend(user_group.users); + session_refresh_users.extend(user_group.get_usernames()); } } // iterate over all users to see if they have this role - for user in users().values().cloned() { + for user in users().values() { if user.roles.contains(&name) { session_refresh_users.insert(user.username().to_string()); } diff --git a/src/rbac/user.rs b/src/rbac/user.rs index f901d25e9..620120273 100644 --- a/src/rbac/user.rs +++ b/src/rbac/user.rs @@ -203,6 +203,56 @@ impl From for UserInfo { } } +/// Represents a user in a UserGroup - simplified structure for both user types +#[derive(Debug, Clone, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)] +pub struct GroupUser { + pub userid: String, + pub username: String, + pub method: String, +} + +impl GroupUser { + pub fn username(&self) -> &str { + &self.username + } + + pub fn userid(&self) -> &str { + &self.userid + } + + pub fn is_oauth(&self) -> bool { + self.method == "oauth" + } + + pub fn user_type(&self) -> &str { + if self.is_oauth() { "oauth" } else { "native" } + } + + pub fn from_user(user: &User) -> Self { + match &user.ty { + UserType::Native(Basic { username, .. }) => GroupUser { + userid: username.clone(), // Same value for basic users + username: username.clone(), + method: "native".to_string(), + }, + UserType::OAuth(OAuth { userid, user_info }) => { + // For OAuth users, derive the display username from user_info + let display_username = user_info + .name + .clone() + .or_else(|| user_info.email.clone()) + .unwrap_or_else(|| userid.clone()); // fallback to userid if nothing else available + + GroupUser { + userid: userid.clone(), + username: display_username, + method: "oauth".to_string(), + } + } + } + } +} + /// Logically speaking, UserGroup is a collection of roles and is applied to a collection of users. /// /// The users present in a group inherit all the roles present in the group for as long as they are a part of the group. @@ -212,7 +262,7 @@ pub struct UserGroup { // #[serde(default = "crate::utils::uid::gen")] // pub id: Ulid, pub roles: HashSet, - pub users: HashSet, + pub users: HashSet, } fn is_valid_group_name(name: &str) -> bool { @@ -239,9 +289,9 @@ impl UserGroup { let mut non_existent_users = Vec::new(); if !self.users.is_empty() { // validate that the users exist - for user in &self.users { - if !users().contains_key(user) { - non_existent_users.push(user.clone()); + for group_user in &self.users { + if !users().contains_key(group_user.userid()) { + non_existent_users.push(group_user.username().to_string()); } } } @@ -266,7 +316,7 @@ impl UserGroup { Ok(()) } } - pub fn new(name: String, roles: HashSet, users: HashSet) -> Self { + pub fn new(name: String, roles: HashSet, users: HashSet) -> Self { UserGroup { name, roles, users } } @@ -276,20 +326,20 @@ impl UserGroup { } self.roles.extend(roles); // also refresh all user sessions - for username in &self.users { - mut_sessions().remove_user(username); + for group_user in &self.users { + mut_sessions().remove_user(group_user.userid()); } Ok(()) } - pub fn add_users(&mut self, users: HashSet) -> Result<(), RBACError> { + pub fn add_users(&mut self, users: HashSet) -> Result<(), RBACError> { if users.is_empty() { return Ok(()); } self.users.extend(users.clone()); // also refresh all user sessions - for username in &users { - mut_sessions().remove_user(username); + for group_user in &users { + mut_sessions().remove_user(group_user.userid()); } Ok(()) } @@ -307,30 +357,61 @@ impl UserGroup { self.roles.clone_from(&new_roles); // also refresh all user sessions - for username in &self.users { - mut_sessions().remove_user(username); + for group_user in &self.users { + mut_sessions().remove_user(group_user.userid()); } Ok(()) } - pub fn remove_users(&mut self, users: HashSet) -> Result<(), RBACError> { + pub fn remove_users(&mut self, users: HashSet) -> Result<(), RBACError> { if users.is_empty() { return Ok(()); } let new_users = HashSet::from_iter(self.users.difference(&users).cloned()); - let removed_users: HashSet = self.users.intersection(&users).cloned().collect(); + let removed_users: HashSet = self.users.intersection(&users).cloned().collect(); if removed_users.is_empty() { return Ok(()); } // also refresh all user sessions - for username in &removed_users { - mut_sessions().remove_user(username); + for group_user in &removed_users { + mut_sessions().remove_user(group_user.userid()); } self.users.clone_from(&new_users); Ok(()) } + /// Get all usernames in this group + pub fn get_usernames(&self) -> Vec { + self.users + .iter() + .map(|u| u.username().to_string()) + .collect() + } + + /// Add users by converting from User references + pub fn add_users_from_user_refs(&mut self, user_refs: &[&User]) -> Result<(), RBACError> { + let group_users: HashSet = + user_refs.iter().map(|u| GroupUser::from_user(u)).collect(); + self.add_users(group_users) + } + + /// Remove users by user ID + pub fn remove_users_by_user_ids(&mut self, user_ids: HashSet) -> Result<(), RBACError> { + if user_ids.is_empty() { + return Ok(()); + } + + let users_to_remove: HashSet = self + .users + .iter() + .filter(|u| user_ids.contains(u.userid())) + .cloned() + .collect(); + + self.remove_users(users_to_remove) + } + pub async fn update_in_metadata(&self) -> Result<(), RBACError> { let mut metadata = get_metadata().await?; metadata.user_groups.retain(|x| x.name != self.name); From 0693fe4d8906f454db314902379d443f461f6494 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Sun, 31 Aug 2025 11:30:17 -0700 Subject: [PATCH 02/15] code rabbit suggestions --- .../http/modal/ingest/ingestor_rbac.rs | 2 +- .../http/modal/ingest/ingestor_role.rs | 2 +- src/handlers/http/modal/query/querier_rbac.rs | 28 +++++++++++++------ src/handlers/http/modal/query/querier_role.rs | 2 +- src/handlers/http/rbac.rs | 6 ++-- src/handlers/http/role.rs | 2 +- src/rbac/user.rs | 14 +++++++++- 7 files changed, 39 insertions(+), 17 deletions(-) diff --git a/src/handlers/http/modal/ingest/ingestor_rbac.rs b/src/handlers/http/modal/ingest/ingestor_rbac.rs index 4f1862898..ff1b57d22 100644 --- a/src/handlers/http/modal/ingest/ingestor_rbac.rs +++ b/src/handlers/http/modal/ingest/ingestor_rbac.rs @@ -58,7 +58,7 @@ pub async fn post_user( // Handler for DELETE /api/v1/user/delete/{username} pub async fn delete_user(username: web::Path) -> Result { let username = username.into_inner(); - let _ = UPDATE_LOCK.lock().await; + let _guard = UPDATE_LOCK.lock().await; // fail this request if the user does not exists if !Users.contains(&username) { return Err(RBACError::UserDoesNotExist); diff --git a/src/handlers/http/modal/ingest/ingestor_role.rs b/src/handlers/http/modal/ingest/ingestor_role.rs index 68fb2f590..3c6fce821 100644 --- a/src/handlers/http/modal/ingest/ingestor_role.rs +++ b/src/handlers/http/modal/ingest/ingestor_role.rs @@ -50,7 +50,7 @@ pub async fn put( let mut session_refresh_users: HashSet = HashSet::new(); for user_group in read_user_groups().values() { if user_group.roles.contains(&name) { - session_refresh_users.extend(user_group.get_usernames()); + session_refresh_users.extend(user_group.users.iter().map(|u| u.userid().to_string())); } } diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index 43fc7a7b9..328ea978a 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -32,8 +32,8 @@ use crate::{ }, rbac::{ Users, - map::{roles, write_user_groups}, - user, + map::{roles, users, write_user_groups}, + user::{self, UserType}, }, validator, }; @@ -74,7 +74,7 @@ pub async fn post_user( return Err(RBACError::RolesDoNotExist(non_existant_roles)); } } - let _ = UPDATE_LOCK.lock().await; + let _guard = UPDATE_LOCK.lock().await; if Users.contains(&username) || metadata .users @@ -106,7 +106,7 @@ pub async fn post_user( // Handler for DELETE /api/v1/user/{username} pub async fn delete_user(username: web::Path) -> Result { let username = username.into_inner(); - let _ = UPDATE_LOCK.lock().await; + let _guard = UPDATE_LOCK.lock().await; // fail this request if the user does not exists if !Users.contains(&username) { return Err(RBACError::UserDoesNotExist); @@ -120,11 +120,21 @@ pub async fn delete_user(username: web::Path) -> Result basic.username.clone(), + UserType::OAuth(oauth) => oauth.userid.clone(), + }; + ug.remove_users_by_user_ids(HashSet::from_iter([user_id]))?; + groups_to_update.push(ug.clone()); + } else { + // User not found, skip or log as needed + continue; + } } else { continue; - }; + } } // For each updated group, replace in place if found; otherwise push @@ -134,7 +144,7 @@ pub async fn delete_user(username: web::Path) -> Result) -> Result = HashSet::new(); for user_group in read_user_groups().values() { if user_group.roles.contains(&name) { - session_refresh_users.extend(user_group.get_usernames()); + session_refresh_users.extend(user_group.users.iter().map(|u| u.userid().to_string())); } } diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index 6f8f46dc4..2bccd39cf 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -124,7 +124,7 @@ pub async fn post_user( return Err(RBACError::RolesDoNotExist(non_existent_roles)); } } - let _ = UPDATE_LOCK.lock().await; + let _guard = UPDATE_LOCK.lock().await; if Users.contains(&username) || metadata .users @@ -159,7 +159,7 @@ pub async fn post_gen_password(username: web::Path) -> Result) -> Result = HashSet::new(); for user_group in read_user_groups().values() { if user_group.roles.contains(&name) { - session_refresh_users.extend(user_group.get_usernames()); + session_refresh_users.extend(user_group.users.iter().map(|u| u.userid().to_string())); } } diff --git a/src/rbac/user.rs b/src/rbac/user.rs index 620120273..5d747bd66 100644 --- a/src/rbac/user.rs +++ b/src/rbac/user.rs @@ -204,13 +204,25 @@ impl From for UserInfo { } /// Represents a user in a UserGroup - simplified structure for both user types -#[derive(Debug, Clone, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)] +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct GroupUser { pub userid: String, pub username: String, pub method: String, } +impl PartialEq for GroupUser { + fn eq(&self, other: &Self) -> bool { + self.userid == other.userid + } +} +impl Eq for GroupUser {} +impl std::hash::Hash for GroupUser { + fn hash(&self, state: &mut H) { + self.userid.hash(state) + } +} + impl GroupUser { pub fn username(&self) -> &str { &self.username From 576cbcf77eacf1ae2460dec6e39c9d6d87ae259a Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Sun, 31 Aug 2025 11:42:56 -0700 Subject: [PATCH 03/15] shared lock among all rbac operations --- src/handlers/http/modal/ingest/ingestor_rbac.rs | 9 ++++----- src/handlers/http/modal/query/querier_rbac.rs | 6 +----- src/handlers/http/rbac.rs | 2 +- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/handlers/http/modal/ingest/ingestor_rbac.rs b/src/handlers/http/modal/ingest/ingestor_rbac.rs index ff1b57d22..2949555f8 100644 --- a/src/handlers/http/modal/ingest/ingestor_rbac.rs +++ b/src/handlers/http/modal/ingest/ingestor_rbac.rs @@ -19,10 +19,12 @@ use std::collections::HashSet; use actix_web::{Responder, web}; -use tokio::sync::Mutex; use crate::{ - handlers::http::{modal::utils::rbac_utils::get_metadata, rbac::RBACError}, + handlers::http::{ + modal::utils::rbac_utils::get_metadata, + rbac::{RBACError, UPDATE_LOCK}, + }, rbac::{ Users, map::roles, @@ -31,9 +33,6 @@ use crate::{ storage, }; -// async aware lock for updating storage metadata and user map atomicically -static UPDATE_LOCK: Mutex<()> = Mutex::const_new(()); - // Handler for POST /api/v1/user/{username} // Creates a new user by username if it does not exists pub async fn post_user( diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index 328ea978a..e7362175a 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -19,7 +19,6 @@ use std::collections::HashSet; use actix_web::{Responder, web}; -use tokio::sync::Mutex; use crate::{ handlers::http::{ @@ -28,7 +27,7 @@ use crate::{ sync_user_deletion_with_ingestors, sync_users_with_roles_with_ingestors, }, modal::utils::rbac_utils::{get_metadata, put_metadata}, - rbac::RBACError, + rbac::{RBACError, UPDATE_LOCK}, }, rbac::{ Users, @@ -38,9 +37,6 @@ use crate::{ validator, }; -// async aware lock for updating storage metadata and user map atomically -static UPDATE_LOCK: Mutex<()> = Mutex::const_new(()); - // Handler for POST /api/v1/user/{username} // Creates a new user by username if it does not exists pub async fn post_user( diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index 2bccd39cf..3254e2fe5 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -43,7 +43,7 @@ use tokio::sync::Mutex; use super::modal::utils::rbac_utils::{get_metadata, put_metadata}; // async aware lock for updating storage metadata and user map atomically -static UPDATE_LOCK: Mutex<()> = Mutex::const_new(()); +pub(crate) static UPDATE_LOCK: Mutex<()> = Mutex::const_new(()); #[derive(serde::Serialize)] struct User { From 520a0e98d8683688364565b1b36f946bf24e32e1 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Thu, 4 Sep 2025 22:38:24 -0700 Subject: [PATCH 04/15] update username to userid --- src/handlers/http/cluster/mod.rs | 6 +- .../http/modal/ingest/ingestor_rbac.rs | 64 +++++++++----- .../http/modal/ingest/ingestor_role.rs | 6 +- src/handlers/http/modal/ingest_server.rs | 10 +-- src/handlers/http/modal/query/querier_rbac.rs | 82 +++++++++++------- src/handlers/http/modal/query/querier_role.rs | 6 +- src/handlers/http/modal/query_server.rs | 12 +-- src/handlers/http/oidc.rs | 10 +-- src/handlers/http/rbac.rs | 85 +++++++++++-------- src/handlers/http/role.rs | 6 +- src/rbac/map.rs | 10 +-- src/rbac/mod.rs | 56 ++++++------ src/rbac/user.rs | 26 ++++-- src/rbac/utils.rs | 10 +-- src/utils/mod.rs | 2 +- 15 files changed, 230 insertions(+), 161 deletions(-) diff --git a/src/handlers/http/cluster/mod.rs b/src/handlers/http/cluster/mod.rs index aac5d91f9..3cd756b56 100644 --- a/src/handlers/http/cluster/mod.rs +++ b/src/handlers/http/cluster/mod.rs @@ -331,21 +331,21 @@ pub async fn sync_user_creation_with_ingestors( if let Some(role) = role { user.roles.clone_from(role); } - let username = user.username(); + let userid = user.userid(); let user_data = to_vec(&user).map_err(|err| { error!("Fatal: failed to serialize user: {:?}", err); RBACError::SerdeError(err) })?; - let username = username.to_string(); + let userid = userid.to_string(); for_each_live_ingestor(move |ingestor| { let url = format!( "{}{}/user/{}/sync", ingestor.domain_name, base_path_without_preceding_slash(), - username + userid ); let user_data = user_data.clone(); diff --git a/src/handlers/http/modal/ingest/ingestor_rbac.rs b/src/handlers/http/modal/ingest/ingestor_rbac.rs index 2949555f8..a46e4a974 100644 --- a/src/handlers/http/modal/ingest/ingestor_rbac.rs +++ b/src/handlers/http/modal/ingest/ingestor_rbac.rs @@ -27,7 +27,7 @@ use crate::{ }, rbac::{ Users, - map::roles, + map::{roles, users}, user::{self, User as ParseableUser}, }, storage, @@ -54,34 +54,49 @@ pub async fn post_user( Ok(generated_password) } -// Handler for DELETE /api/v1/user/delete/{username} -pub async fn delete_user(username: web::Path) -> Result { - let username = username.into_inner(); +// Handler for DELETE /api/v1/user/delete/{userid} +pub async fn delete_user(userid: web::Path) -> Result { + let userid = userid.into_inner(); let _guard = UPDATE_LOCK.lock().await; // fail this request if the user does not exists - if !Users.contains(&username) { + if !Users.contains(&userid) { + return Err(RBACError::UserDoesNotExist); + }; + + // find username by userid, for native users, username is userid, for oauth users, we need to look up + let username = if let Some(user) = users().get(&userid) { + user.username_by_userid() + } else { return Err(RBACError::UserDoesNotExist); }; + // delete from parseable.json first let mut metadata = get_metadata().await?; - metadata.users.retain(|user| user.username() != username); + metadata.users.retain(|user| user.userid() != userid); let _ = storage::put_staging_metadata(&metadata); // update in mem table - Users.delete_user(&username); + Users.delete_user(&userid); Ok(format!("deleted user: {username}")) } -// Handler PATCH /user/{username}/role/sync/add => Add roles to a user +// Handler PATCH /user/{userid}/role/sync/add => Add roles to a user pub async fn add_roles_to_user( - username: web::Path, + userid: web::Path, roles_to_add: web::Json>, ) -> Result { - let username = username.into_inner(); + let userid = userid.into_inner(); let roles_to_add = roles_to_add.into_inner(); - if !Users.contains(&username) { + if !Users.contains(&userid) { + return Err(RBACError::UserDoesNotExist); + }; + + // find username by userid, for native users, username is userid, for oauth users, we need to look up + let username = if let Some(user) = users().get(&userid) { + user.username_by_userid() + } else { return Err(RBACError::UserDoesNotExist); }; @@ -102,7 +117,7 @@ pub async fn add_roles_to_user( if let Some(user) = metadata .users .iter_mut() - .find(|user| user.username() == username) + .find(|user| user.userid() == userid) { user.roles.extend(roles_to_add.clone()); } else { @@ -112,20 +127,27 @@ pub async fn add_roles_to_user( let _ = storage::put_staging_metadata(&metadata); // update in mem table - Users.add_roles(&username.clone(), roles_to_add.clone()); + Users.add_roles(&userid.clone(), roles_to_add.clone()); Ok(format!("Roles updated successfully for {username}")) } -// Handler PATCH /user/{username}/role/sync/add => Add roles to a user +// Handler PATCH /user/{userid}/role/sync/add => Add roles to a user pub async fn remove_roles_from_user( - username: web::Path, + userid: web::Path, roles_to_remove: web::Json>, ) -> Result { - let username = username.into_inner(); + let userid = userid.into_inner(); let roles_to_remove = roles_to_remove.into_inner(); - if !Users.contains(&username) { + if !Users.contains(&userid) { + return Err(RBACError::UserDoesNotExist); + }; + + // find username by userid, for native users, username is userid, for oauth users, we need to look up + let username = if let Some(user) = users().get(&userid) { + user.username_by_userid() + } else { return Err(RBACError::UserDoesNotExist); }; @@ -142,7 +164,7 @@ pub async fn remove_roles_from_user( } // check that user actually has these roles - let user_roles: HashSet = HashSet::from_iter(Users.get_role(&username)); + let user_roles: HashSet = HashSet::from_iter(Users.get_role(&userid)); let roles_not_with_user: HashSet = HashSet::from_iter(roles_to_remove.difference(&user_roles).cloned()); @@ -152,12 +174,12 @@ pub async fn remove_roles_from_user( ))); } - // update parseable.json first + // update parseable.json in staging first let mut metadata = get_metadata().await?; if let Some(user) = metadata .users .iter_mut() - .find(|user| user.username() == username) + .find(|user| user.userid() == userid) { let diff: HashSet = HashSet::from_iter(user.roles.difference(&roles_to_remove).cloned()); @@ -169,7 +191,7 @@ pub async fn remove_roles_from_user( let _ = storage::put_staging_metadata(&metadata); // update in mem table - Users.remove_roles(&username.clone(), roles_to_remove.clone()); + Users.remove_roles(&userid.clone(), roles_to_remove.clone()); Ok(format!("Roles updated successfully for {username}")) } diff --git a/src/handlers/http/modal/ingest/ingestor_role.rs b/src/handlers/http/modal/ingest/ingestor_role.rs index 3c6fce821..89fe2b1f8 100644 --- a/src/handlers/http/modal/ingest/ingestor_role.rs +++ b/src/handlers/http/modal/ingest/ingestor_role.rs @@ -57,12 +57,12 @@ pub async fn put( // iterate over all users to see if they have this role for user in users().values() { if user.roles.contains(&name) { - session_refresh_users.insert(user.username().to_string()); + session_refresh_users.insert(user.userid().to_string()); } } - for username in session_refresh_users { - mut_sessions().remove_user(&username); + for userid in session_refresh_users { + mut_sessions().remove_user(&userid); } Ok(HttpResponse::Ok().finish()) diff --git a/src/handlers/http/modal/ingest_server.rs b/src/handlers/http/modal/ingest_server.rs index 04ba8f981..e13ba4ea1 100644 --- a/src/handlers/http/modal/ingest_server.rs +++ b/src/handlers/http/modal/ingest_server.rs @@ -190,7 +190,7 @@ impl IngestServer { .to(ingestor_rbac::post_user) .authorize(Action::PutUser), ) - // DELETE /user/{username} => Sync deletion of a user + // DELETE /user/{userid} => Sync deletion of a user .route( web::delete() .to(ingestor_rbac::delete_user) @@ -199,8 +199,8 @@ impl IngestServer { .wrap(DisAllowRootUser), ) .service( - web::resource("/{username}/role/sync/add") - // PATCH /user/{username}/role/sync/add => Add roles to a user + web::resource("/{userid}/role/sync/add") + // PATCH /user/{userid}/role/sync/add => Add roles to a user .route( web::patch() .to(ingestor_rbac::add_roles_to_user) @@ -209,8 +209,8 @@ impl IngestServer { ), ) .service( - web::resource("/{username}/role/sync/remove") - // PATCH /user/{username}/role/sync/remove => Remove roles from a user + web::resource("/{userid}/role/sync/remove") + // PATCH /user/{userid}/role/sync/remove => Remove roles from a user .route( web::patch() .to(ingestor_rbac::remove_roles_from_user) diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index e7362175a..dfb7fe360 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -71,12 +71,7 @@ pub async fn post_user( } } let _guard = UPDATE_LOCK.lock().await; - if Users.contains(&username) - || metadata - .users - .iter() - .any(|user| user.username() == username) - { + if Users.contains(&username) || metadata.users.iter().any(|user| user.userid() == username) { return Err(RBACError::UserExists); } @@ -99,30 +94,39 @@ pub async fn post_user( Ok(password) } -// Handler for DELETE /api/v1/user/{username} -pub async fn delete_user(username: web::Path) -> Result { - let username = username.into_inner(); +// Handler for DELETE /api/v1/user/{userid} +pub async fn delete_user(userid: web::Path) -> Result { + let userid = userid.into_inner(); + let _guard = UPDATE_LOCK.lock().await; // fail this request if the user does not exists - if !Users.contains(&username) { + if !Users.contains(&userid) { return Err(RBACError::UserDoesNotExist); }; + + // find username by userid, for native users, username is userid, for oauth users, we need to look up + let username = if let Some(user) = users().get(&userid) { + user.username_by_userid() + } else { + return Err(RBACError::UserDoesNotExist); + }; + // delete from parseable.json first let mut metadata = get_metadata().await?; - metadata.users.retain(|user| user.username() != username); + metadata.users.retain(|user| user.userid() != userid); // also delete from user groups - let user_groups = Users.get_user_groups(&username); + let user_groups = Users.get_user_groups(&userid); let mut groups_to_update = Vec::new(); for user_group in user_groups { if let Some(ug) = write_user_groups().get_mut(&user_group) { // Look up the user by display name to get the correct userid - if let Some(user) = users().get(&username) { - let user_id = match &user.ty { + if let Some(user) = users().get(&userid) { + let userid = match &user.ty { UserType::Native(basic) => basic.username.clone(), UserType::OAuth(oauth) => oauth.userid.clone(), }; - ug.remove_users_by_user_ids(HashSet::from_iter([user_id]))?; + ug.remove_users_by_user_ids(HashSet::from_iter([userid]))?; groups_to_update.push(ug.clone()); } else { // User not found, skip or log as needed @@ -147,22 +151,29 @@ pub async fn delete_user(username: web::Path) -> Result Add roles to a user +// Handler PATCH /user/{userid}/role/add => Add roles to a user pub async fn add_roles_to_user( - username: web::Path, + userid: web::Path, roles_to_add: web::Json>, ) -> Result { - let username = username.into_inner(); + let userid = userid.into_inner(); let roles_to_add = roles_to_add.into_inner(); - if !Users.contains(&username) { + if !Users.contains(&userid) { + return Err(RBACError::UserDoesNotExist); + }; + + // find username by userid, for native users, username is userid, for oauth users, we need to look up + let username = if let Some(user) = users().get(&userid) { + user.username_by_userid() + } else { return Err(RBACError::UserDoesNotExist); }; @@ -184,7 +195,7 @@ pub async fn add_roles_to_user( if let Some(user) = metadata .users .iter_mut() - .find(|user| user.username() == username) + .find(|user| user.userid() == userid) { user.roles.extend(roles_to_add.clone()); } else { @@ -194,22 +205,29 @@ pub async fn add_roles_to_user( put_metadata(&metadata).await?; // update in mem table - Users.add_roles(&username.clone(), roles_to_add.clone()); + Users.add_roles(&userid.clone(), roles_to_add.clone()); - sync_users_with_roles_with_ingestors(&username, &roles_to_add, "add").await?; + sync_users_with_roles_with_ingestors(&userid, &roles_to_add, "add").await?; Ok(format!("Roles updated successfully for {username}")) } -// Handler PATCH /user/{username}/role/remove => Remove roles from a user +// Handler PATCH /user/{userid}/role/remove => Remove roles from a user pub async fn remove_roles_from_user( - username: web::Path, + userid: web::Path, roles_to_remove: web::Json>, ) -> Result { - let username = username.into_inner(); + let userid = userid.into_inner(); let roles_to_remove = roles_to_remove.into_inner(); - if !Users.contains(&username) { + if !Users.contains(&userid) { + return Err(RBACError::UserDoesNotExist); + }; + + // find username by userid, for native users, username is userid, for oauth users, we need to look up + let username = if let Some(user) = users().get(&userid) { + user.username_by_userid() + } else { return Err(RBACError::UserDoesNotExist); }; @@ -227,7 +245,7 @@ pub async fn remove_roles_from_user( } // check for role not present with user - let user_roles: HashSet = HashSet::from_iter(Users.get_role(&username)); + let user_roles: HashSet = HashSet::from_iter(Users.get_role(&userid)); let roles_not_with_user: HashSet = HashSet::from_iter(roles_to_remove.difference(&user_roles).cloned()); if !roles_not_with_user.is_empty() { @@ -241,7 +259,7 @@ pub async fn remove_roles_from_user( if let Some(user) = metadata .users .iter_mut() - .find(|user| user.username() == username) + .find(|user| user.userid() == userid) { let diff: HashSet = HashSet::from_iter(user.roles.difference(&roles_to_remove).cloned()); @@ -253,9 +271,9 @@ pub async fn remove_roles_from_user( put_metadata(&metadata).await?; // update in mem table - Users.remove_roles(&username.clone(), roles_to_remove.clone()); + Users.remove_roles(&userid.clone(), roles_to_remove.clone()); - sync_users_with_roles_with_ingestors(&username, &roles_to_remove, "remove").await?; + sync_users_with_roles_with_ingestors(&userid, &roles_to_remove, "remove").await?; Ok(format!("Roles updated successfully for {username}")) } diff --git a/src/handlers/http/modal/query/querier_role.rs b/src/handlers/http/modal/query/querier_role.rs index cb740b116..2889d9d94 100644 --- a/src/handlers/http/modal/query/querier_role.rs +++ b/src/handlers/http/modal/query/querier_role.rs @@ -60,12 +60,12 @@ pub async fn put( // iterate over all users to see if they have this role for user in users().values() { if user.roles.contains(&name) { - session_refresh_users.insert(user.username().to_string()); + session_refresh_users.insert(user.userid().to_string()); } } - for username in session_refresh_users { - mut_sessions().remove_user(&username); + for userid in session_refresh_users { + mut_sessions().remove_user(&userid); } sync_role_update_with_ingestors(name.clone(), privileges.clone()).await?; diff --git a/src/handlers/http/modal/query_server.rs b/src/handlers/http/modal/query_server.rs index 8533f392b..60c0b155b 100644 --- a/src/handlers/http/modal/query_server.rs +++ b/src/handlers/http/modal/query_server.rs @@ -200,7 +200,7 @@ impl QueryServer { .to(querier_rbac::post_user) .authorize(Action::PutUser), ) - // DELETE /user/{username} => Delete a user + // DELETE /user/{userid} => Delete a user .route( web::delete() .to(querier_rbac::delete_user) @@ -209,15 +209,15 @@ impl QueryServer { .wrap(DisAllowRootUser), ) .service( - web::resource("/{username}/role").route( + web::resource("/{userid}/role").route( web::get() .to(rbac::get_role) .authorize_for_user(Action::GetUserRoles), ), ) .service( - web::resource("/{username}/role/add") - // PATCH /user/{username}/role/add => Add roles to a user + web::resource("/{userid}/role/add") + // PATCH /user/{userid}/role/add => Add roles to a user .route( web::patch() .to(rbac::add_roles_to_user) @@ -226,8 +226,8 @@ impl QueryServer { ), ) .service( - web::resource("/{username}/role/remove") - // PATCH /user/{username}/role/remove => Remove roles from a user + web::resource("/{userid}/role/remove") + // PATCH /user/{userid}/role/remove => Remove roles from a user .route( web::patch() .to(rbac::remove_roles_from_user) diff --git a/src/handlers/http/oidc.rs b/src/handlers/http/oidc.rs index bd73890c9..84a7b79b7 100644 --- a/src/handlers/http/oidc.rs +++ b/src/handlers/http/oidc.rs @@ -367,7 +367,7 @@ pub async fn request_token( // put new user in metadata if does not exits // update local cache pub async fn put_user( - username: &str, + userid: &str, group: HashSet, user_info: user::UserInfo, ) -> Result { @@ -376,10 +376,10 @@ pub async fn put_user( let user = metadata .users .iter() - .find(|user| user.username() == username) + .find(|user| user.userid() == userid) .cloned() .unwrap_or_else(|| { - let user = User::new_oauth(username.to_owned(), group, user_info); + let user = User::new_oauth(userid.to_owned(), group, user_info); metadata.users.push(user.clone()); user }); @@ -395,7 +395,7 @@ pub async fn update_user_if_changed( user_info: user::UserInfo, ) -> Result { // Store the old username before modifying the user object - let old_username = user.username().to_string(); + let old_username = user.userid().to_string(); let User { ty, roles, .. } = &mut user; let UserType::OAuth(oauth_user) = ty else { unreachable!() @@ -427,7 +427,7 @@ pub async fn update_user_if_changed( if let Some(entry) = metadata .users .iter_mut() - .find(|x| x.username() == old_username) + .find(|x| x.userid() == old_username) { entry.clone_from(&user); // migrate user references inside user groups diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index 3254e2fe5..2f3a72911 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -21,7 +21,7 @@ use std::collections::{HashMap, HashSet}; use crate::{ rbac::{ self, Users, - map::{read_user_groups, roles}, + map::{read_user_groups, roles, users}, role::model::DefaultPrivilege, user, utils::to_prism_user, @@ -59,7 +59,7 @@ impl From<&user::User> for User { }; User { - id: user.username().to_owned(), + id: user.userid().to_owned(), method, } } @@ -125,12 +125,7 @@ pub async fn post_user( } } let _guard = UPDATE_LOCK.lock().await; - if Users.contains(&username) - || metadata - .users - .iter() - .any(|user| user.username() == username) - { + if Users.contains(&username) || metadata.users.iter().any(|user| user.userid() == username) { return Err(RBACError::UserExists); } @@ -182,14 +177,14 @@ pub async fn post_gen_password(username: web::Path) -> Result) -> Result { - if !Users.contains(&username) { +pub async fn get_role(userid: web::Path) -> Result { + if !Users.contains(&userid) { return Err(RBACError::UserDoesNotExist); }; let direct_roles: HashMap> = Users - .get_role(&username) + .get_role(&userid) .iter() .filter_map(|role_name| { roles() @@ -200,7 +195,7 @@ pub async fn get_role(username: web::Path) -> Result>> = HashMap::new(); // user might be part of some user groups, fetch the roles from there as well - for user_group in Users.get_user_groups(&username) { + for user_group in Users.get_user_groups(&userid) { if let Some(group) = read_user_groups().get(&user_group) { let ug_roles: HashMap> = group .roles @@ -221,41 +216,56 @@ pub async fn get_role(username: web::Path) -> Result) -> Result { - let username = username.into_inner(); +// Handler for DELETE /api/v1/user/delete/{userid} +pub async fn delete_user(userid: web::Path) -> Result { + let userid = userid.into_inner(); // if user is a part of any groups then don't allow deletion - if !Users.get_user_groups(&username).is_empty() { + if !Users.get_user_groups(&userid).is_empty() { return Err(RBACError::InvalidDeletionRequest(format!( - "User: {username} should not be a part of any groups" + "User: {userid} should not be a part of any groups" ))); } // fail this request if the user does not exists - if !Users.contains(&username) { + if !Users.contains(&userid) { + return Err(RBACError::UserDoesNotExist); + }; + + // find username by userid, for native users, username is userid, for oauth users, we need to look up + let username = if let Some(user) = users().get(&userid) { + user.username_by_userid() + } else { return Err(RBACError::UserDoesNotExist); }; + let _guard = UPDATE_LOCK.lock().await; // delete from parseable.json first let mut metadata = get_metadata().await?; - metadata.users.retain(|user| user.username() != username); + metadata.users.retain(|user| user.userid() != userid); put_metadata(&metadata).await?; // update in mem table - Users.delete_user(&username); + Users.delete_user(&userid); Ok(format!("deleted user: {username}")) } -// Handler PATCH /user/{username}/role/add => Add roles to a user +// Handler PATCH /user/{userid}/role/add => Add roles to a user pub async fn add_roles_to_user( - username: web::Path, + userid: web::Path, roles_to_add: web::Json>, ) -> Result { - let username = username.into_inner(); + let userid = userid.into_inner(); let roles_to_add = roles_to_add.into_inner(); - if !Users.contains(&username) { + if !Users.contains(&userid) { + return Err(RBACError::UserDoesNotExist); + }; + + // find username by userid, for native users, username is userid, for oauth users, we need to look up + let username = if let Some(user) = users().get(&userid) { + user.username_by_userid() + } else { return Err(RBACError::UserDoesNotExist); }; @@ -277,7 +287,7 @@ pub async fn add_roles_to_user( if let Some(user) = metadata .users .iter_mut() - .find(|user| user.username() == username) + .find(|user| user.userid() == userid) { user.roles.extend(roles_to_add.clone()); } else { @@ -287,20 +297,27 @@ pub async fn add_roles_to_user( put_metadata(&metadata).await?; // update in mem table - Users.add_roles(&username.clone(), roles_to_add); + Users.add_roles(&userid.clone(), roles_to_add); Ok(format!("Roles updated successfully for {username}")) } -// Handler PATCH /user/{username}/role/remove => Remove roles from a user +// Handler PATCH /user/{userid}/role/remove => Remove roles from a user pub async fn remove_roles_from_user( - username: web::Path, + userid: web::Path, roles_to_remove: web::Json>, ) -> Result { - let username = username.into_inner(); + let userid = userid.into_inner(); let roles_to_remove = roles_to_remove.into_inner(); - if !Users.contains(&username) { + if !Users.contains(&userid) { + return Err(RBACError::UserDoesNotExist); + }; + + // find username by userid, for native users, username is userid, for oauth users, we need to look up + let username = if let Some(user) = users().get(&userid) { + user.username_by_userid() + } else { return Err(RBACError::UserDoesNotExist); }; @@ -318,7 +335,7 @@ pub async fn remove_roles_from_user( } // check for role not present with user - let user_roles: HashSet = HashSet::from_iter(Users.get_role(&username)); + let user_roles: HashSet = HashSet::from_iter(Users.get_role(&userid)); let roles_not_with_user: HashSet = HashSet::from_iter(roles_to_remove.difference(&user_roles).cloned()); if !roles_not_with_user.is_empty() { @@ -332,7 +349,7 @@ pub async fn remove_roles_from_user( if let Some(user) = metadata .users .iter_mut() - .find(|user| user.username() == username) + .find(|user| user.userid() == userid) { let diff: HashSet = HashSet::from_iter(user.roles.difference(&roles_to_remove).cloned()); @@ -344,7 +361,7 @@ pub async fn remove_roles_from_user( put_metadata(&metadata).await?; // update in mem table - Users.remove_roles(&username.clone(), roles_to_remove); + Users.remove_roles(&userid.clone(), roles_to_remove); Ok(format!("Roles updated successfully for {username}")) } diff --git a/src/handlers/http/role.rs b/src/handlers/http/role.rs index 95a6e9351..3db3a6f42 100644 --- a/src/handlers/http/role.rs +++ b/src/handlers/http/role.rs @@ -59,12 +59,12 @@ pub async fn put( // iterate over all users to see if they have this role for user in users().values() { if user.roles.contains(&name) { - session_refresh_users.insert(user.username().to_string()); + session_refresh_users.insert(user.userid().to_string()); } } - for username in session_refresh_users { - mut_sessions().remove_user(&username); + for userid in session_refresh_users { + mut_sessions().remove_user(&userid); } Ok(HttpResponse::Ok().finish()) diff --git a/src/rbac/map.rs b/src/rbac/map.rs index 7fd5ed703..5377d10d7 100644 --- a/src/rbac/map.rs +++ b/src/rbac/map.rs @@ -123,7 +123,7 @@ pub fn init(metadata: &StorageMetadata) { let mut users = Users::from(users); let admin = user::get_admin_user(); - let admin_username = admin.username().to_owned(); + let admin_username = admin.userid().to_owned(); users.insert(admin); let mut sessions = Sessions::default(); @@ -274,8 +274,8 @@ impl Sessions { }) } - pub fn get_username(&self, key: &SessionKey) -> Option<&String> { - self.active_sessions.get(key).map(|(username, _)| username) + pub fn get_userid(&self, key: &SessionKey) -> Option<&String> { + self.active_sessions.get(key).map(|(userid, _)| userid) } } @@ -286,7 +286,7 @@ pub struct Users(HashMap); impl Users { pub fn insert(&mut self, user: User) { - self.0.insert(user.username().to_owned(), user); + self.0.insert(user.userid().to_owned(), user); } } @@ -296,7 +296,7 @@ impl From> for Users { map.extend( users .into_iter() - .map(|user| (user.username().to_owned(), user)), + .map(|user| (user.userid().to_owned(), user)), ); map } diff --git a/src/rbac/mod.rs b/src/rbac/mod.rs index 256564b77..4eb115778 100644 --- a/src/rbac/mod.rs +++ b/src/rbac/mod.rs @@ -50,80 +50,80 @@ pub struct Users; impl Users { pub fn put_user(&self, user: User) { - mut_sessions().remove_user(user.username()); + mut_sessions().remove_user(user.userid()); mut_users().insert(user); } - pub fn get_user_groups(&self, username: &str) -> HashSet { + pub fn get_user_groups(&self, userid: &str) -> HashSet { users() - .get(username) + .get(userid) .map(|user| user.user_groups.clone()) .unwrap_or_default() } - pub fn get_user(&self, username: &str) -> Option { - users().get(username).cloned() + pub fn get_user(&self, userid: &str) -> Option { + users().get(userid).cloned() } - pub fn is_oauth(&self, username: &str) -> Option { - users().get(username).map(|user| user.is_oauth()) + pub fn is_oauth(&self, userid: &str) -> Option { + users().get(userid).map(|user| user.is_oauth()) } pub fn collect_user From<&'a User> + 'static>(&self) -> Vec { users().values().map(|user| user.into()).collect_vec() } - pub fn get_role(&self, username: &str) -> Vec { + pub fn get_role(&self, userid: &str) -> Vec { users() - .get(username) + .get(userid) .map(|user| user.roles.iter().cloned().collect()) .unwrap_or_default() } - pub fn delete_user(&self, username: &str) { - mut_users().remove(username); - mut_sessions().remove_user(username); + pub fn delete_user(&self, userid: &str) { + mut_users().remove(userid); + mut_sessions().remove_user(userid); } // caller ensures that this operation is valid for the user - pub fn change_password_hash(&self, username: &str, hash: &String) { + pub fn change_password_hash(&self, userid: &str, hash: &String) { if let Some(User { ty: UserType::Native(user), .. - }) = mut_users().get_mut(username) + }) = mut_users().get_mut(userid) { user.password_hash.clone_from(hash); - mut_sessions().remove_user(username); + mut_sessions().remove_user(userid); }; } - pub fn add_roles(&self, username: &str, roles: HashSet) { - if let Some(user) = mut_users().get_mut(username) { + pub fn add_roles(&self, userid: &str, roles: HashSet) { + if let Some(user) = mut_users().get_mut(userid) { user.roles.extend(roles); - mut_sessions().remove_user(username) + mut_sessions().remove_user(userid) }; } - pub fn remove_roles(&self, username: &str, roles: HashSet) { - if let Some(user) = mut_users().get_mut(username) { + pub fn remove_roles(&self, userid: &str, roles: HashSet) { + if let Some(user) = mut_users().get_mut(userid) { let diff = HashSet::from_iter(user.roles.difference(&roles).cloned()); user.roles = diff; - mut_sessions().remove_user(username) + mut_sessions().remove_user(userid) }; } - pub fn contains(&self, username: &str) -> bool { - users().contains_key(username) + pub fn contains(&self, userid: &str) -> bool { + users().contains_key(userid) } pub fn get_permissions(&self, session: &SessionKey) -> Vec { let mut permissions = sessions().get(session).cloned().unwrap_or_default(); - let Some(username) = self.get_username_from_session(session) else { + let Some(userid) = self.get_userid_from_session(session) else { return permissions.into_iter().collect_vec(); }; - let user_groups = self.get_user_groups(&username); + let user_groups = self.get_user_groups(&userid); for group in user_groups { if let Some(group) = read_user_groups().get(&group) { let group_roles = &group.roles; @@ -149,7 +149,7 @@ impl Users { pub fn new_session(&self, user: &User, session: SessionKey) { mut_sessions().track_new( - user.username().to_owned(), + user.userid().to_owned(), session, Utc::now() + Days::new(7), roles_to_permission(user.roles()), @@ -199,8 +199,8 @@ impl Users { Response::UnAuthorized } - pub fn get_username_from_session(&self, session: &SessionKey) -> Option { - sessions().get_username(session).cloned() + pub fn get_userid_from_session(&self, session: &SessionKey) -> Option { + sessions().get_userid(session).cloned() } } diff --git a/src/rbac/user.rs b/src/rbac/user.rs index 5d747bd66..575ea716d 100644 --- a/src/rbac/user.rs +++ b/src/rbac/user.rs @@ -66,14 +66,14 @@ impl User { ) } - pub fn new_oauth(username: String, roles: HashSet, user_info: UserInfo) -> Self { + pub fn new_oauth(userid: String, roles: HashSet, user_info: UserInfo) -> Self { Self { ty: UserType::OAuth(OAuth { userid: user_info .sub .clone() .or_else(|| user_info.name.clone()) - .unwrap_or(username), + .unwrap_or(userid), user_info, }), roles, @@ -81,13 +81,25 @@ impl User { } } - pub fn username(&self) -> &str { + pub fn userid(&self) -> &str { match self.ty { UserType::Native(Basic { ref username, .. }) => username, - UserType::OAuth(OAuth { - userid: ref username, - .. - }) => username, + UserType::OAuth(OAuth { ref userid, .. }) => userid, + } + } + + pub fn username_by_userid(&self) -> String { + match &self.ty { + UserType::Native(basic) => basic.username.clone(), + UserType::OAuth(oauth) => { + let user_info = oauth.user_info.clone(); + user_info.name.clone().unwrap_or_else(|| { + user_info + .email + .clone() + .unwrap_or_else(|| oauth.userid.clone()) + }) + } } } diff --git a/src/rbac/utils.rs b/src/rbac/utils.rs index 55223f691..83ac17e26 100644 --- a/src/rbac/utils.rs +++ b/src/rbac/utils.rs @@ -30,12 +30,12 @@ use super::{ pub fn to_prism_user(user: &User) -> UsersPrism { let (id, username, method, email, picture) = match &user.ty { - UserType::Native(_) => (user.username(), user.username(), "native", None, None), + UserType::Native(_) => (user.userid(), user.userid(), "native", None, None), UserType::OAuth(oauth) => { - let username = user.username(); - let display_name = oauth.user_info.name.as_deref().unwrap_or(username); + let userid = user.userid(); + let display_name = oauth.user_info.name.as_deref().unwrap_or(userid); ( - username, + userid, display_name, "oauth", oauth.user_info.email.clone(), @@ -56,7 +56,7 @@ pub fn to_prism_user(user: &User) -> UsersPrism { let mut group_roles: HashMap>> = HashMap::new(); let mut user_groups = HashSet::new(); // user might be part of some user groups, fetch the roles from there as well - for user_group in Users.get_user_groups(user.username()) { + for user_group in Users.get_user_groups(user.userid()) { if let Some(group) = read_user_groups().get(&user_group) { let ug_roles: HashMap> = group .roles diff --git a/src/utils/mod.rs b/src/utils/mod.rs index bf24e0277..fc3290ab4 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -59,7 +59,7 @@ pub fn extract_datetime(path: &str) -> Option { pub fn get_user_from_request(req: &HttpRequest) -> Result { let session_key = extract_session_key_from_req(req).unwrap(); - let user_id = Users.get_username_from_session(&session_key); + let user_id = Users.get_userid_from_session(&session_key); if user_id.is_none() { return Err(RBACError::UserDoesNotExist); } From 3a7a06e539f31b7c84b16c5f64ef294c45b66c0c Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Thu, 4 Sep 2025 22:48:43 -0700 Subject: [PATCH 05/15] update logic for user existence --- src/handlers/http/modal/query/querier_rbac.rs | 11 +++++++++-- src/handlers/http/rbac.rs | 17 ++++++++++------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index dfb7fe360..b544cd26c 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -71,8 +71,15 @@ pub async fn post_user( } } let _guard = UPDATE_LOCK.lock().await; - if Users.contains(&username) || metadata.users.iter().any(|user| user.userid() == username) { - return Err(RBACError::UserExists); + if Users.contains(&username) { + if Users.contains(&username) + || metadata.users.iter().any(|user| match &user.ty { + UserType::Native(basic) => basic.username == username, + UserType::OAuth(_) => false, // OAuth users should be created differently + }) + { + return Err(RBACError::UserExists); + } } let (user, password) = user::User::new_basic(username.clone()); diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index 2f3a72911..ccd566a32 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -20,11 +20,7 @@ use std::collections::{HashMap, HashSet}; use crate::{ rbac::{ - self, Users, - map::{read_user_groups, roles, users}, - role::model::DefaultPrivilege, - user, - utils::to_prism_user, + self, map::{read_user_groups, roles, users}, role::model::DefaultPrivilege, user::{self, UserType}, utils::to_prism_user, Users }, storage::ObjectStorageError, validator::{self, error::UsernameValidationError}, @@ -125,8 +121,15 @@ pub async fn post_user( } } let _guard = UPDATE_LOCK.lock().await; - if Users.contains(&username) || metadata.users.iter().any(|user| user.userid() == username) { - return Err(RBACError::UserExists); + if Users.contains(&username) { + if Users.contains(&username) + || metadata.users.iter().any(|user| match &user.ty { + UserType::Native(basic) => basic.username == username, + UserType::OAuth(_) => false, // OAuth users should be created differently + }) + { + return Err(RBACError::UserExists); + } } let (user, password) = user::User::new_basic(username.clone()); From 7f3f9adcba77725ec8dbef0d7cb51b7c5822a649 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Thu, 4 Sep 2025 22:54:05 -0700 Subject: [PATCH 06/15] fmt change --- src/handlers/http/rbac.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index ccd566a32..48dc01a29 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -20,7 +20,11 @@ use std::collections::{HashMap, HashSet}; use crate::{ rbac::{ - self, map::{read_user_groups, roles, users}, role::model::DefaultPrivilege, user::{self, UserType}, utils::to_prism_user, Users + self, Users, + map::{read_user_groups, roles, users}, + role::model::DefaultPrivilege, + user::{self, UserType}, + utils::to_prism_user, }, storage::ObjectStorageError, validator::{self, error::UsernameValidationError}, From a55dac710e7454f94c9e577569cdff14cbd887f1 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Thu, 4 Sep 2025 23:18:41 -0700 Subject: [PATCH 07/15] clippy changes --- src/handlers/http/modal/query/querier_rbac.rs | 39 ++++++++----------- src/handlers/http/rbac.rs | 16 ++++---- src/rbac/user.rs | 8 ++-- 3 files changed, 28 insertions(+), 35 deletions(-) diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index b544cd26c..f2df1e441 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -71,15 +71,13 @@ pub async fn post_user( } } let _guard = UPDATE_LOCK.lock().await; - if Users.contains(&username) { - if Users.contains(&username) - || metadata.users.iter().any(|user| match &user.ty { - UserType::Native(basic) => basic.username == username, - UserType::OAuth(_) => false, // OAuth users should be created differently - }) - { - return Err(RBACError::UserExists); - } + if Users.contains(&username) && Users.contains(&username) + || metadata.users.iter().any(|user| match &user.ty { + UserType::Native(basic) => basic.username == username, + UserType::OAuth(_) => false, // OAuth users should be created differently + }) + { + return Err(RBACError::UserExists); } let (user, password) = user::User::new_basic(username.clone()); @@ -126,20 +124,17 @@ pub async fn delete_user(userid: web::Path) -> Result basic.username.clone(), - UserType::OAuth(oauth) => oauth.userid.clone(), - }; - ug.remove_users_by_user_ids(HashSet::from_iter([userid]))?; - groups_to_update.push(ug.clone()); - } else { - // User not found, skip or log as needed - continue; - } + if let Some(ug) = write_user_groups().get_mut(&user_group) + && let Some(user) = users().get(&userid) + { + let userid = match &user.ty { + UserType::Native(basic) => basic.username.clone(), + UserType::OAuth(oauth) => oauth.userid.clone(), + }; + ug.remove_users_by_user_ids(HashSet::from_iter([userid]))?; + groups_to_update.push(ug.clone()); } else { + // User not found, skip or log as needed continue; } } diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index 48dc01a29..b2f6c6b39 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -125,15 +125,13 @@ pub async fn post_user( } } let _guard = UPDATE_LOCK.lock().await; - if Users.contains(&username) { - if Users.contains(&username) - || metadata.users.iter().any(|user| match &user.ty { - UserType::Native(basic) => basic.username == username, - UserType::OAuth(_) => false, // OAuth users should be created differently - }) - { - return Err(RBACError::UserExists); - } + if Users.contains(&username) && Users.contains(&username) + || metadata.users.iter().any(|user| match &user.ty { + UserType::Native(basic) => basic.username == username, + UserType::OAuth(_) => false, // OAuth users should be created differently + }) + { + return Err(RBACError::UserExists); } let (user, password) = user::User::new_basic(username.clone()); diff --git a/src/rbac/user.rs b/src/rbac/user.rs index 575ea716d..6e70f7bf6 100644 --- a/src/rbac/user.rs +++ b/src/rbac/user.rs @@ -315,7 +315,7 @@ impl UserGroup { // validate that the users exist for group_user in &self.users { if !users().contains_key(group_user.userid()) { - non_existent_users.push(group_user.username().to_string()); + non_existent_users.push(group_user.userid().to_string()); } } } @@ -405,11 +405,11 @@ impl UserGroup { Ok(()) } - /// Get all usernames in this group - pub fn get_usernames(&self) -> Vec { + /// Get all user IDs in this group + pub fn get_userids(&self) -> Vec { self.users .iter() - .map(|u| u.username().to_string()) + .map(|u| u.userid().to_string()) .collect() } From 33ccd470fc8c3036c8ff7f053c84603b8cd0777d Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Thu, 4 Sep 2025 23:26:47 -0700 Subject: [PATCH 08/15] code rabbit suggestions --- src/handlers/http/rbac.rs | 4 +--- src/rbac/user.rs | 5 +---- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index b2f6c6b39..a9b9f53dc 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -224,7 +224,7 @@ pub async fn get_role(userid: web::Path) -> Result) -> Result { let userid = userid.into_inner(); - + let _guard = UPDATE_LOCK.lock().await; // if user is a part of any groups then don't allow deletion if !Users.get_user_groups(&userid).is_empty() { return Err(RBACError::InvalidDeletionRequest(format!( @@ -243,8 +243,6 @@ pub async fn delete_user(userid: web::Path) -> Result Vec { - self.users - .iter() - .map(|u| u.userid().to_string()) - .collect() + self.users.iter().map(|u| u.userid().to_string()).collect() } /// Add users by converting from User references From f4019b47e8dc311ba8da22a47a1bd73be4d0a2b6 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Thu, 4 Sep 2025 23:47:00 -0700 Subject: [PATCH 09/15] refactor --- src/handlers/http/modal/query/querier_rbac.rs | 10 +++++----- src/handlers/http/rbac.rs | 1 + src/rbac/user.rs | 6 +----- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index f2df1e441..1d5589dba 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -71,11 +71,11 @@ pub async fn post_user( } } let _guard = UPDATE_LOCK.lock().await; - if Users.contains(&username) && Users.contains(&username) - || metadata.users.iter().any(|user| match &user.ty { - UserType::Native(basic) => basic.username == username, - UserType::OAuth(_) => false, // OAuth users should be created differently - }) + if Users.contains(&username) + || metadata + .users + .iter() + .any(|user| matches!(&user.ty, UserType::Native(basic) if basic.username == username)) { return Err(RBACError::UserExists); } diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index a9b9f53dc..aa8f6ba0b 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -185,6 +185,7 @@ pub async fn post_gen_password(username: web::Path) -> Result) -> Result { + let userid = userid.into_inner(); if !Users.contains(&userid) { return Err(RBACError::UserDoesNotExist); }; diff --git a/src/rbac/user.rs b/src/rbac/user.rs index 638362f6e..300bf90d9 100644 --- a/src/rbac/user.rs +++ b/src/rbac/user.rs @@ -69,11 +69,7 @@ impl User { pub fn new_oauth(userid: String, roles: HashSet, user_info: UserInfo) -> Self { Self { ty: UserType::OAuth(OAuth { - userid: user_info - .sub - .clone() - .or_else(|| user_info.name.clone()) - .unwrap_or(userid), + userid: user_info.sub.clone().unwrap_or(userid), user_info, }), roles, From 978a85e2e7e932ab3c99cab557adc185e84402ca Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Fri, 5 Sep 2025 00:08:45 -0700 Subject: [PATCH 10/15] add guard before update --- src/handlers/http/modal/query/querier_rbac.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index 1d5589dba..f18f26e2c 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -168,6 +168,8 @@ pub async fn add_roles_to_user( let userid = userid.into_inner(); let roles_to_add = roles_to_add.into_inner(); + let _guard = UPDATE_LOCK.lock().await; + if !Users.contains(&userid) { return Err(RBACError::UserDoesNotExist); }; @@ -222,6 +224,8 @@ pub async fn remove_roles_from_user( let userid = userid.into_inner(); let roles_to_remove = roles_to_remove.into_inner(); + let _guard = UPDATE_LOCK.lock().await; + if !Users.contains(&userid) { return Err(RBACError::UserDoesNotExist); }; From c9028ef84f2395755756dca21b88a9ea8f4d86f2 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Fri, 5 Sep 2025 01:25:49 -0700 Subject: [PATCH 11/15] remove lock as lock held in caller --- src/handlers/http/modal/query/querier_rbac.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index f18f26e2c..05aec6ce1 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -168,8 +168,6 @@ pub async fn add_roles_to_user( let userid = userid.into_inner(); let roles_to_add = roles_to_add.into_inner(); - let _guard = UPDATE_LOCK.lock().await; - if !Users.contains(&userid) { return Err(RBACError::UserDoesNotExist); }; @@ -225,7 +223,7 @@ pub async fn remove_roles_from_user( let roles_to_remove = roles_to_remove.into_inner(); let _guard = UPDATE_LOCK.lock().await; - + if !Users.contains(&userid) { return Err(RBACError::UserDoesNotExist); }; From 19967fd142d065327b7856fd55bff3ff501a3124 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Fri, 5 Sep 2025 01:41:54 -0700 Subject: [PATCH 12/15] change string response to json with status 200 --- src/handlers/http/modal/ingest/ingestor_rbac.rs | 10 ++++------ src/handlers/http/modal/query/querier_rbac.rs | 8 ++++---- src/handlers/http/rbac.rs | 12 ++++++------ 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/handlers/http/modal/ingest/ingestor_rbac.rs b/src/handlers/http/modal/ingest/ingestor_rbac.rs index a46e4a974..9d456e75a 100644 --- a/src/handlers/http/modal/ingest/ingestor_rbac.rs +++ b/src/handlers/http/modal/ingest/ingestor_rbac.rs @@ -18,7 +18,7 @@ use std::collections::HashSet; -use actix_web::{Responder, web}; +use actix_web::{HttpResponse, Responder, web}; use crate::{ handlers::http::{ @@ -51,7 +51,7 @@ pub async fn post_user( Users.add_roles(&username, created_role.clone()); } - Ok(generated_password) + Ok(HttpResponse::Ok().json(generated_password)) } // Handler for DELETE /api/v1/user/delete/{userid} @@ -78,7 +78,7 @@ pub async fn delete_user(userid: web::Path) -> Result Add roles to a user @@ -128,7 +128,6 @@ pub async fn add_roles_to_user( let _ = storage::put_staging_metadata(&metadata); // update in mem table Users.add_roles(&userid.clone(), roles_to_add.clone()); - Ok(format!("Roles updated successfully for {username}")) } @@ -218,6 +217,5 @@ pub async fn post_gen_password(username: web::Path) -> Result) -> Result Add roles to a user @@ -218,7 +218,7 @@ pub async fn add_roles_to_user( pub async fn remove_roles_from_user( userid: web::Path, roles_to_remove: web::Json>, -) -> Result { +) -> Result { let userid = userid.into_inner(); let roles_to_remove = roles_to_remove.into_inner(); @@ -279,7 +279,7 @@ pub async fn remove_roles_from_user( sync_users_with_roles_with_ingestors(&userid, &roles_to_remove, "remove").await?; - Ok(format!("Roles updated successfully for {username}")) + Ok(HttpResponse::Ok().json(format!("Roles updated successfully for {username}"))) } // Handler for POST /api/v1/user/{username}/generate-new-password diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index aa8f6ba0b..acf7f912b 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -30,7 +30,7 @@ use crate::{ validator::{self, error::UsernameValidationError}, }; use actix_web::{ - Responder, + HttpResponse, Responder, http::header::ContentType, web::{self, Path}, }; @@ -251,14 +251,14 @@ pub async fn delete_user(userid: web::Path) -> Result Add roles to a user pub async fn add_roles_to_user( userid: web::Path, roles_to_add: web::Json>, -) -> Result { +) -> Result { let userid = userid.into_inner(); let roles_to_add = roles_to_add.into_inner(); @@ -303,14 +303,14 @@ pub async fn add_roles_to_user( // update in mem table Users.add_roles(&userid.clone(), roles_to_add); - Ok(format!("Roles updated successfully for {username}")) + Ok(HttpResponse::Ok().json(format!("Roles updated successfully for {username}"))) } // Handler PATCH /user/{userid}/role/remove => Remove roles from a user pub async fn remove_roles_from_user( userid: web::Path, roles_to_remove: web::Json>, -) -> Result { +) -> Result { let userid = userid.into_inner(); let roles_to_remove = roles_to_remove.into_inner(); @@ -367,7 +367,7 @@ pub async fn remove_roles_from_user( // update in mem table Users.remove_roles(&userid.clone(), roles_to_remove); - Ok(format!("Roles updated successfully for {username}")) + Ok(HttpResponse::Ok().json(format!("Roles updated successfully for {username}"))) } #[derive(Debug, Serialize)] From 0fe2ca62bca0dd55d7b8c670d2ca9066bf6c5e3d Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Fri, 5 Sep 2025 02:07:54 -0700 Subject: [PATCH 13/15] status code in response --- .../http/modal/ingest/ingestor_rbac.rs | 47 +++++-------------- src/handlers/http/modal/ingest_server.rs | 2 +- 2 files changed, 14 insertions(+), 35 deletions(-) diff --git a/src/handlers/http/modal/ingest/ingestor_rbac.rs b/src/handlers/http/modal/ingest/ingestor_rbac.rs index 9d456e75a..99a5838df 100644 --- a/src/handlers/http/modal/ingest/ingestor_rbac.rs +++ b/src/handlers/http/modal/ingest/ingestor_rbac.rs @@ -18,7 +18,8 @@ use std::collections::HashSet; -use actix_web::{HttpResponse, Responder, web}; +use actix_web::{HttpResponse, web}; +use http::StatusCode; use crate::{ handlers::http::{ @@ -27,7 +28,7 @@ use crate::{ }, rbac::{ Users, - map::{roles, users}, + map::roles, user::{self, User as ParseableUser}, }, storage, @@ -38,10 +39,9 @@ use crate::{ pub async fn post_user( username: web::Path, body: Option>, -) -> Result { +) -> Result { let username = username.into_inner(); - let generated_password = String::default(); let metadata = get_metadata().await?; if let Some(body) = body { let user: ParseableUser = serde_json::from_value(body.into_inner())?; @@ -51,11 +51,11 @@ pub async fn post_user( Users.add_roles(&username, created_role.clone()); } - Ok(HttpResponse::Ok().json(generated_password)) + Ok(HttpResponse::Ok().status(StatusCode::OK).finish()) } // Handler for DELETE /api/v1/user/delete/{userid} -pub async fn delete_user(userid: web::Path) -> Result { +pub async fn delete_user(userid: web::Path) -> Result { let userid = userid.into_inner(); let _guard = UPDATE_LOCK.lock().await; // fail this request if the user does not exists @@ -63,13 +63,6 @@ pub async fn delete_user(userid: web::Path) -> Result) -> Result Add roles to a user pub async fn add_roles_to_user( userid: web::Path, roles_to_add: web::Json>, -) -> Result { +) -> Result { let userid = userid.into_inner(); let roles_to_add = roles_to_add.into_inner(); @@ -93,13 +86,6 @@ pub async fn add_roles_to_user( return Err(RBACError::UserDoesNotExist); }; - // find username by userid, for native users, username is userid, for oauth users, we need to look up - let username = if let Some(user) = users().get(&userid) { - user.username_by_userid() - } else { - return Err(RBACError::UserDoesNotExist); - }; - // check if all roles exist let mut non_existent_roles = Vec::new(); roles_to_add.iter().for_each(|r| { @@ -128,14 +114,14 @@ pub async fn add_roles_to_user( let _ = storage::put_staging_metadata(&metadata); // update in mem table Users.add_roles(&userid.clone(), roles_to_add.clone()); - Ok(format!("Roles updated successfully for {username}")) + Ok(HttpResponse::Ok().status(StatusCode::OK).finish()) } // Handler PATCH /user/{userid}/role/sync/add => Add roles to a user pub async fn remove_roles_from_user( userid: web::Path, roles_to_remove: web::Json>, -) -> Result { +) -> Result { let userid = userid.into_inner(); let roles_to_remove = roles_to_remove.into_inner(); @@ -143,13 +129,6 @@ pub async fn remove_roles_from_user( return Err(RBACError::UserDoesNotExist); }; - // find username by userid, for native users, username is userid, for oauth users, we need to look up - let username = if let Some(user) = users().get(&userid) { - user.username_by_userid() - } else { - return Err(RBACError::UserDoesNotExist); - }; - // check if all roles exist let mut non_existent_roles = Vec::new(); roles_to_remove.iter().for_each(|r| { @@ -192,12 +171,12 @@ pub async fn remove_roles_from_user( // update in mem table Users.remove_roles(&userid.clone(), roles_to_remove.clone()); - Ok(format!("Roles updated successfully for {username}")) + Ok(HttpResponse::Ok().status(StatusCode::OK).finish()) } // Handler for POST /api/v1/user/{username}/generate-new-password // Resets password for the user to a newly generated one and returns it -pub async fn post_gen_password(username: web::Path) -> Result { +pub async fn post_gen_password(username: web::Path) -> Result { let username = username.into_inner(); let mut new_hash = String::default(); let mut metadata = get_metadata().await?; @@ -217,5 +196,5 @@ pub async fn post_gen_password(username: web::Path) -> Result Sync creation of a new user + // POST /user/{username}/sync => Sync creation of a new user .route( web::post() .to(ingestor_rbac::post_user) From 9ce51244db5b8721a05fafe4ba135763b0c75ba9 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Fri, 5 Sep 2025 02:17:52 -0700 Subject: [PATCH 14/15] update method in sync with ingestors --- src/handlers/http/cluster/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/handlers/http/cluster/mod.rs b/src/handlers/http/cluster/mod.rs index 3cd756b56..c64daba99 100644 --- a/src/handlers/http/cluster/mod.rs +++ b/src/handlers/http/cluster/mod.rs @@ -253,7 +253,7 @@ pub async fn sync_users_with_roles_with_ingestors( async move { let res = INTRA_CLUSTER_CLIENT - .put(url) + .patch(url) .header(header::AUTHORIZATION, &ingestor.token) .header(header::CONTENT_TYPE, "application/json") .body(role_data) From 08378194e1748c38105639670b3b98eb36f8c514 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Fri, 5 Sep 2025 02:27:46 -0700 Subject: [PATCH 15/15] rename to userid, update comment --- src/handlers/http/cluster/mod.rs | 12 ++++++------ src/handlers/http/modal/ingest/ingestor_rbac.rs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/handlers/http/cluster/mod.rs b/src/handlers/http/cluster/mod.rs index c64daba99..7a0fc3a8d 100644 --- a/src/handlers/http/cluster/mod.rs +++ b/src/handlers/http/cluster/mod.rs @@ -222,7 +222,7 @@ pub async fn get_demo_data_from_ingestor(action: &str) -> Result<(), PostError> // forward the role update request to all ingestors to keep them in sync pub async fn sync_users_with_roles_with_ingestors( - username: &str, + userid: &str, role: &HashSet, operation: &str, ) -> Result<(), RBACError> { @@ -236,7 +236,7 @@ pub async fn sync_users_with_roles_with_ingestors( RBACError::SerdeError(err) })?; - let username = username.to_owned(); + let userid = userid.to_owned(); let op = operation.to_string(); @@ -245,7 +245,7 @@ pub async fn sync_users_with_roles_with_ingestors( "{}{}/user/{}/role/sync/{}", ingestor.domain_name, base_path_without_preceding_slash(), - username, + userid, op ); @@ -282,15 +282,15 @@ pub async fn sync_users_with_roles_with_ingestors( } // forward the delete user request to all ingestors to keep them in sync -pub async fn sync_user_deletion_with_ingestors(username: &str) -> Result<(), RBACError> { - let username = username.to_owned(); +pub async fn sync_user_deletion_with_ingestors(userid: &str) -> Result<(), RBACError> { + let userid = userid.to_owned(); for_each_live_ingestor(move |ingestor| { let url = format!( "{}{}/user/{}/sync", ingestor.domain_name, base_path_without_preceding_slash(), - username + userid ); async move { diff --git a/src/handlers/http/modal/ingest/ingestor_rbac.rs b/src/handlers/http/modal/ingest/ingestor_rbac.rs index 99a5838df..8eb39cda2 100644 --- a/src/handlers/http/modal/ingest/ingestor_rbac.rs +++ b/src/handlers/http/modal/ingest/ingestor_rbac.rs @@ -117,7 +117,7 @@ pub async fn add_roles_to_user( Ok(HttpResponse::Ok().status(StatusCode::OK).finish()) } -// Handler PATCH /user/{userid}/role/sync/add => Add roles to a user +// Handler PATCH /user/{userid}/role/sync/remove => Remove roles to a user pub async fn remove_roles_from_user( userid: web::Path, roles_to_remove: web::Json>,