bugfixes

- user and role deletion while still being used
- renamed resource types

Author: parmesant

src/handlers/http/rbac.rs

@@ -228,11 +228,19 @@ pub async fn get_role(username: web::Path<String>) -> Result<impl Responder, RBA
 // Handler for DELETE /api/v1/user/delete/{username}
 pub async fn delete_user(username: web::Path<String>) -> Result<impl Responder, RBACError> {
     let username = username.into_inner();
-    let _ = UPDATE_LOCK.lock().await;
+
+    // if user is a part of any groups then don't allow deletion
+    if !Users.get_user_groups(&username).is_empty() {
+        return Err(RBACError::InvalidDeletionRequest(format!(
+            "User: {username} should not be a part of any groups"
+        )));
+    }
     // fail this request if the user does not exists
     if !Users.contains(&username) {
         return Err(RBACError::UserDoesNotExist);
     };
+    let _ = UPDATE_LOCK.lock().await;
+
     // delete from parseable.json first
     let mut metadata = get_metadata().await?;
     metadata.users.retain(|user| user.username() != username);
@@ -408,6 +416,8 @@ pub enum RBACError {
     UserGroupNotEmpty(String),
     #[error("Resource in use: {0}")]
     ResourceInUse(String),
+    #[error("{0}")]
+    InvalidDeletionRequest(String),
 }
 
 impl actix_web::ResponseError for RBACError {
@@ -429,6 +439,7 @@ impl actix_web::ResponseError for RBACError {
             Self::InvalidSyncOperation(_) => StatusCode::BAD_REQUEST,
             Self::UserGroupNotEmpty(_) => StatusCode::BAD_REQUEST,
             Self::ResourceInUse(_) => StatusCode::BAD_REQUEST,
+            Self::InvalidDeletionRequest(_) => StatusCode::BAD_REQUEST,
         }
     }
 

src/handlers/http/role.rs

@@ -22,12 +22,11 @@ use actix_web::{
     HttpResponse, Responder,
 };
 use http::StatusCode;
-use itertools::Itertools;
 
 use crate::{
     parseable::PARSEABLE,
     rbac::{
-        map::{mut_roles, read_user_groups, write_user_groups, DEFAULT_ROLE},
+        map::{mut_roles, DEFAULT_ROLE},
         role::model::DefaultPrivilege,
     },
     storage::{self, ObjectStorageError, StorageMetadata},
@@ -78,46 +77,22 @@ pub async fn list_roles() -> Result<impl Responder, RoleError> {
 // Delete existing role
 pub async fn delete(name: web::Path<String>) -> Result<impl Responder, RoleError> {
     let name = name.into_inner();
+    // check if the role is being used by any user or group
     let mut metadata = get_metadata().await?;
     if metadata.users.iter().any(|user| user.roles.contains(&name)) {
         return Err(RoleError::RoleInUse);
     }
+    if metadata
+        .user_groups
+        .iter()
+        .any(|user_group| user_group.roles.contains(&name))
+    {
+        return Err(RoleError::RoleInUse);
+    }
     metadata.roles.remove(&name);
     put_metadata(&metadata).await?;
     mut_roles().remove(&name);
 
-    // also delete from user groups
-    let groups = read_user_groups().keys().cloned().collect_vec();
-    let mut group_names = Vec::new();
-
-    for user_group in groups {
-        if let Some(ug) = read_user_groups().get(&user_group) {
-            if ug.roles.contains(&name) {
-                return Err(RoleError::RoleInUse);
-            }
-            group_names.push(ug.name.clone());
-        } else {
-            continue;
-        };
-    }
-
-    // remove role from all user groups that have it
-    let mut groups_to_update = Vec::new();
-    for group in write_user_groups().values_mut() {
-        if group.roles.remove(&name) {
-            groups_to_update.push(group.clone());
-        }
-    }
-
-    // update metadata only if there are changes
-    if !groups_to_update.is_empty() {
-        metadata
-            .user_groups
-            .retain(|x| !groups_to_update.contains(x));
-        metadata.user_groups.extend(groups_to_update);
-    }
-    put_metadata(&metadata).await?;
-
     Ok(HttpResponse::Ok().finish())
 }
 

src/rbac/role.rs

@@ -79,9 +79,9 @@ pub enum Action {
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
 pub enum ParseableResourceType {
-    #[serde(rename = "stream")]
+    #[serde(rename = "dataset")]
     Stream(String),
-    #[serde(rename = "llm")]
+    #[serde(rename = "llmKey")]
     Llm(String),
     #[serde(rename = "all")]
     All,