chore: allow user to be created without role (#1428)

this is helpful when an admin wants to create a user
and assign group to it

Author: nikhilsinhaparseable

src/handlers/http/modal/query/querier_rbac.rs

@@ -51,33 +51,26 @@ pub async fn post_user(
     let user_roles: HashSet<String> = if let Some(body) = body {
         serde_json::from_value(body.into_inner())?
     } else {
-        return Err(RBACError::RoleValidationError);
+        HashSet::new()
     };
 
-    if user_roles.is_empty() {
-        return Err(RBACError::RoleValidationError);
-    } else {
-        let mut non_existant_roles = Vec::new();
-        user_roles
-            .iter()
-            .map(|r| {
-                if !roles().contains_key(r) {
-                    non_existant_roles.push(r.clone());
-                }
-            })
-            .for_each(drop);
-        if !non_existant_roles.is_empty() {
-            return Err(RBACError::RolesDoNotExist(non_existant_roles));
+    let mut non_existent_roles = Vec::new();
+    for role in &user_roles {
+        if !roles().contains_key(role) {
+            non_existent_roles.push(role.clone());
         }
     }
+    if !non_existent_roles.is_empty() {
+        return Err(RBACError::RolesDoNotExist(non_existent_roles));
+    }
     let _guard = UPDATE_LOCK.lock().await;
     if Users.contains(&username)
         || metadata
             .users
             .iter()
             .any(|user| matches!(&user.ty, UserType::Native(basic) if basic.username == username))
     {
-        return Err(RBACError::UserExists);
+        return Err(RBACError::UserExists(username));
     }
 
     let (user, password) = user::User::new_basic(username.clone());
@@ -89,12 +82,13 @@ pub async fn post_user(
     Users.put_user(user.clone());
 
     sync_user_creation_with_ingestors(user, &Some(user_roles)).await?;
-
-    add_roles_to_user(
-        web::Path::<String>::from(username.clone()),
-        web::Json(created_role),
-    )
-    .await?;
+    if !created_role.is_empty() {
+        add_roles_to_user(
+            web::Path::<String>::from(username.clone()),
+            web::Json(created_role),
+        )
+        .await?;
+    }
 
     Ok(password)
 }

src/handlers/http/modal/query_server.rs

@@ -194,7 +194,7 @@ impl QueryServer {
             )
             .service(
                 web::resource("/{username}")
-                    // PUT /user/{username} => Create a new user
+                    // POST /user/{username} => Create a new user
                     .route(
                         web::post()
                             .to(querier_rbac::post_user)

src/handlers/http/modal/server.rs

@@ -641,7 +641,7 @@ impl Server {
             )
             .service(
                 web::resource("/{username}")
-                    // PUT /user/{username} => Create a new user
+                    // POST /user/{username} => Create a new user
                     .route(
                         web::post()
                             .to(http::rbac::post_user)

src/handlers/http/rbac.rs

@@ -108,30 +108,26 @@ pub async fn post_user(
     let user_roles: HashSet<String> = if let Some(body) = body {
         serde_json::from_value(body.into_inner())?
     } else {
-        return Err(RBACError::RoleValidationError);
+        HashSet::new()
     };
 
-    if user_roles.is_empty() {
-        return Err(RBACError::RoleValidationError);
-    } else {
-        let mut non_existent_roles = Vec::new();
-        for role in &user_roles {
-            if !roles().contains_key(role) {
-                non_existent_roles.push(role.clone());
-            }
-        }
-        if !non_existent_roles.is_empty() {
-            return Err(RBACError::RolesDoNotExist(non_existent_roles));
+    let mut non_existent_roles = Vec::new();
+    for role in &user_roles {
+        if !roles().contains_key(role) {
+            non_existent_roles.push(role.clone());
         }
     }
+    if !non_existent_roles.is_empty() {
+        return Err(RBACError::RolesDoNotExist(non_existent_roles));
+    }
     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
         })
     {
-        return Err(RBACError::UserExists);
+        return Err(RBACError::UserExists(username));
     }
 
     let (user, password) = user::User::new_basic(username.clone());
@@ -141,12 +137,13 @@ pub async fn post_user(
     put_metadata(&metadata).await?;
     let created_role = user_roles.clone();
     Users.put_user(user.clone());
-
-    add_roles_to_user(
-        web::Path::<String>::from(username.clone()),
-        web::Json(created_role),
-    )
-    .await?;
+    if !created_role.is_empty() {
+        add_roles_to_user(
+            web::Path::<String>::from(username.clone()),
+            web::Json(created_role),
+        )
+        .await?;
+    }
 
     Ok(password)
 }
@@ -383,8 +380,8 @@ pub struct InvalidUserGroupError {
 
 #[derive(Debug, thiserror::Error)]
 pub enum RBACError {
-    #[error("User exists already")]
-    UserExists,
+    #[error("User {0} already exists")]
+    UserExists(String),
     #[error("User does not exist")]
     UserDoesNotExist,
     #[error("{0}")]
@@ -422,7 +419,7 @@ pub enum RBACError {
 impl actix_web::ResponseError for RBACError {
     fn status_code(&self) -> http::StatusCode {
         match self {
-            Self::UserExists => StatusCode::BAD_REQUEST,
+            Self::UserExists(_) => StatusCode::BAD_REQUEST,
             Self::UserDoesNotExist => StatusCode::NOT_FOUND,
             Self::SerdeError(_) => StatusCode::BAD_REQUEST,
             Self::ValidationError(_) => StatusCode::BAD_REQUEST,