chore: add validation to user and role creation (#1435)

below validations are added to both user and role names at creation -

1. the length should be between 3-64 characters
2. the name should not be any of these -
   admin, user, role, null, undefined, none, empty, password, username
3. first and the last character must be alphanumeric
4. allow any of these special characters - `_, -,.`
5. no two consecutive characters should be special character

Author: nikhilsinhaparseable

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

@@ -44,10 +44,9 @@ pub async fn post_user(
     body: Option<web::Json<serde_json::Value>>,
 ) -> Result<impl Responder, RBACError> {
     let username = username.into_inner();
-
+    validator::user_role_name(&username)?;
     let mut metadata = get_metadata().await?;
 
-    validator::user_name(&username)?;
     let user_roles: HashSet<String> = if let Some(body) = body {
         serde_json::from_value(body.into_inner())?
     } else {
@@ -173,17 +172,17 @@ pub async fn add_roles_to_user(
         return Err(RBACError::UserDoesNotExist);
     };
 
-    let mut non_existant_roles = Vec::new();
+    let mut non_existent_roles = Vec::new();
 
     // check if the role exists
     roles_to_add.iter().for_each(|r| {
         if roles().get(r).is_none() {
-            non_existant_roles.push(r.clone());
+            non_existent_roles.push(r.clone());
         }
     });
 
-    if !non_existant_roles.is_empty() {
-        return Err(RBACError::RolesDoNotExist(non_existant_roles));
+    if !non_existent_roles.is_empty() {
+        return Err(RBACError::RolesDoNotExist(non_existent_roles));
     }
 
     // update parseable.json first
@@ -229,17 +228,17 @@ pub async fn remove_roles_from_user(
         return Err(RBACError::UserDoesNotExist);
     };
 
-    let mut non_existant_roles = Vec::new();
+    let mut non_existent_roles = Vec::new();
 
     // check if the role exists
     roles_to_remove.iter().for_each(|r| {
         if roles().get(r).is_none() {
-            non_existant_roles.push(r.clone());
+            non_existent_roles.push(r.clone());
         }
     });
 
-    if !non_existant_roles.is_empty() {
-        return Err(RBACError::RolesDoNotExist(non_existant_roles));
+    if !non_existent_roles.is_empty() {
+        return Err(RBACError::RolesDoNotExist(non_existent_roles));
     }
 
     // check for role not present with user

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

@@ -33,6 +33,7 @@ use crate::{
         map::{mut_roles, mut_sessions, read_user_groups, users},
         role::model::DefaultPrivilege,
     },
+    validator,
 };
 
 // Handler for PUT /api/v1/role/{name}
@@ -42,6 +43,8 @@ pub async fn put(
     Json(privileges): Json<Vec<DefaultPrivilege>>,
 ) -> Result<impl Responder, RoleError> {
     let name = name.into_inner();
+    // validate the role name
+    validator::user_role_name(&name).map_err(RoleError::ValidationError)?;
     let mut metadata = get_metadata().await?;
     metadata.roles.insert(name.clone(), privileges.clone());
 

src/handlers/http/rbac.rs

@@ -101,10 +101,9 @@ pub async fn post_user(
     body: Option<web::Json<serde_json::Value>>,
 ) -> Result<impl Responder, RBACError> {
     let username = username.into_inner();
-
+    validator::user_role_name(&username)?;
     let mut metadata = get_metadata().await?;
 
-    validator::user_name(&username)?;
     let user_roles: HashSet<String> = if let Some(body) = body {
         serde_json::from_value(body.into_inner())?
     } else {
@@ -121,7 +120,7 @@ pub async fn post_user(
         return Err(RBACError::RolesDoNotExist(non_existent_roles));
     }
     let _guard = UPDATE_LOCK.lock().await;
-    if Users.contains(&username) && 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

src/handlers/http/role.rs

@@ -32,6 +32,7 @@ use crate::{
         role::model::DefaultPrivilege,
     },
     storage::{self, ObjectStorageError, StorageMetadata},
+    validator::{self, error::UsernameValidationError},
 };
 
 // Handler for PUT /api/v1/role/{name}
@@ -41,6 +42,8 @@ pub async fn put(
     Json(privileges): Json<Vec<DefaultPrivilege>>,
 ) -> Result<impl Responder, RoleError> {
     let name = name.into_inner();
+    // validate the role name
+    validator::user_role_name(&name).map_err(RoleError::ValidationError)?;
     let mut metadata = get_metadata().await?;
     metadata.roles.insert(name.clone(), privileges.clone());
 
@@ -168,6 +171,8 @@ pub enum RoleError {
     SerdeError(#[from] serde_json::Error),
     #[error("Network Error: {0}")]
     Network(#[from] reqwest::Error),
+    #[error("Validation Error: {0}")]
+    ValidationError(#[from] UsernameValidationError),
 }
 
 impl actix_web::ResponseError for RoleError {
@@ -178,6 +183,7 @@ impl actix_web::ResponseError for RoleError {
             Self::Anyhow(_) => StatusCode::INTERNAL_SERVER_ERROR,
             Self::SerdeError(_) => StatusCode::BAD_REQUEST,
             Self::Network(_) => StatusCode::BAD_GATEWAY,
+            Self::ValidationError(_) => StatusCode::BAD_REQUEST,
         }
     }
 

src/validator.rs

@@ -16,7 +16,10 @@
  *
  */
 
+use std::collections::HashSet;
+
 use error::HotTierValidationError;
+use once_cell::sync::Lazy;
 
 use self::error::{StreamNameValidationError, UsernameValidationError};
 use crate::hottier::MIN_STREAM_HOT_TIER_SIZE_BYTES;
@@ -67,21 +70,65 @@ pub fn stream_name(
     Ok(())
 }
 
-// validate if username is valid
-// username should be between 3 and 64 characters long
-// username should contain only alphanumeric characters or underscores
-// username should be lowercase
-pub fn user_name(username: &str) -> Result<(), UsernameValidationError> {
-    // Check if the username meets the required criteria
-    if username.len() < 3 || username.len() > 64 {
+static RESERVED_NAMES: Lazy<HashSet<&'static str>> = Lazy::new(|| {
+    [
+        "admin",
+        "user",
+        "role",
+        "null",
+        "undefined",
+        "none",
+        "empty",
+        "password",
+        "username",
+    ]
+    .into_iter()
+    .collect()
+});
+
+pub fn user_role_name(name: &str) -> Result<(), UsernameValidationError> {
+    // Normalize username to lowercase for validation
+    let name = name.to_lowercase();
+
+    // Check length constraints
+    if name.len() < 3 || name.len() > 64 {
         return Err(UsernameValidationError::InvalidLength);
     }
-    // Username should contain only alphanumeric characters or underscores
-    if !username
-        .chars()
-        .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '_')
+
+    // Check if name is reserved
+    if RESERVED_NAMES.contains(name.as_str()) {
+        return Err(UsernameValidationError::ReservedName);
+    }
+
+    let chars: Vec<char> = name.chars().collect();
+
+    // Check last character (must be alphanumeric)
+    if let Some(last_char) = chars.last()
+        && !last_char.is_ascii_alphanumeric()
     {
-        return Err(UsernameValidationError::SpecialChar);
+        return Err(UsernameValidationError::InvalidEndChar);
+    }
+
+    // Check all characters and consecutive special chars
+    let mut prev_was_special = false;
+    for &ch in &chars {
+        match ch {
+            // Allow alphanumeric
+            c if c.is_ascii_alphanumeric() => {
+                prev_was_special = false;
+            }
+            // Allow specific special characters
+            '_' | '-' | '.' => {
+                if prev_was_special {
+                    return Err(UsernameValidationError::ConsecutiveSpecialChars);
+                }
+                prev_was_special = true;
+            }
+            // Reject any other character
+            _ => {
+                return Err(UsernameValidationError::InvalidCharacter);
+            }
+        }
     }
 
     Ok(())
@@ -138,9 +185,21 @@ pub mod error {
         #[error("Username should be between 3 and 64 chars long")]
         InvalidLength,
         #[error(
-            "Username contains invalid characters. Please use lowercase, alphanumeric or underscore"
+            "Username contains invalid characters. Only alphanumeric characters and special characters (underscore, hyphen and dot) are allowed"
         )]
         SpecialChar,
+        #[error("Username should start with an alphanumeric character")]
+        InvalidStartChar,
+        #[error("Username should end with an alphanumeric character")]
+        InvalidEndChar,
+        #[error(
+            "Username contains invalid characters. Only alphanumeric characters and special characters (underscore, hyphen and dot) are allowed"
+        )]
+        InvalidCharacter,
+        #[error("Username contains consecutive special characters")]
+        ConsecutiveSpecialChars,
+        #[error("Username is reserved")]
+        ReservedName,
     }
 
     #[derive(Debug, thiserror::Error)]