From 50331545a2571de63b9b6331bcd95ef9780c3380 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Wed, 17 Sep 2025 21:21:19 -0700 Subject: [PATCH 1/5] chore: add validation to user and role creation 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 --- src/handlers/http/modal/query/querier_rbac.rs | 3 +- src/handlers/http/modal/query/querier_role.rs | 3 + src/handlers/http/rbac.rs | 3 +- src/handlers/http/role.rs | 3 + src/validator.rs | 90 ++++++++++++++++--- 5 files changed, 86 insertions(+), 16 deletions(-) diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index 52d4da057..9bf0eaae2 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -44,10 +44,9 @@ pub async fn post_user( body: Option>, ) -> Result { let username = username.into_inner(); - + validator::user_role_name(&username)?; let mut metadata = get_metadata().await?; - validator::user_name(&username)?; let user_roles: HashSet = if let Some(body) = body { serde_json::from_value(body.into_inner())? } else { diff --git a/src/handlers/http/modal/query/querier_role.rs b/src/handlers/http/modal/query/querier_role.rs index 2889d9d94..10893e556 100644 --- a/src/handlers/http/modal/query/querier_role.rs +++ b/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>, ) -> Result { let name = name.into_inner(); + // validate the role name + validator::user_role_name(&name).map_err(|e| RoleError::Anyhow(e.into()))?; let mut metadata = get_metadata().await?; metadata.roles.insert(name.clone(), privileges.clone()); diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index 0147d7410..33e693861 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -101,10 +101,9 @@ pub async fn post_user( body: Option>, ) -> Result { let username = username.into_inner(); - + validator::user_role_name(&username)?; let mut metadata = get_metadata().await?; - validator::user_name(&username)?; let user_roles: HashSet = if let Some(body) = body { serde_json::from_value(body.into_inner())? } else { diff --git a/src/handlers/http/role.rs b/src/handlers/http/role.rs index 2e6b19710..1746903f6 100644 --- a/src/handlers/http/role.rs +++ b/src/handlers/http/role.rs @@ -32,6 +32,7 @@ use crate::{ role::model::DefaultPrivilege, }, storage::{self, ObjectStorageError, StorageMetadata}, + validator, }; // Handler for PUT /api/v1/role/{name} @@ -41,6 +42,8 @@ pub async fn put( Json(privileges): Json>, ) -> Result { let name = name.into_inner(); + // validate the role name + validator::user_role_name(&name).map_err(|e| RoleError::Anyhow(e.into()))?; let mut metadata = get_metadata().await?; metadata.roles.insert(name.clone(), privileges.clone()); diff --git a/src/validator.rs b/src/validator.rs index 797a14a35..dd0c3b082 100644 --- a/src/validator.rs +++ b/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,72 @@ 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> = 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 = name.chars().collect(); + + // Check first character (must be alphanumeric) + if let Some(first_char) = chars.first() + && !first_char.is_ascii_alphanumeric() + { + return Err(UsernameValidationError::InvalidStartChar); + } + + // 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(()) @@ -141,6 +195,18 @@ pub mod error { "Username contains invalid characters. Please use lowercase, alphanumeric or underscore" )] 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 _, -, . are allowed" + )] + InvalidCharacter, + #[error("Username contains consecutive special characters")] + ConsecutiveSpecialChars, + #[error("Username is reserved")] + ReservedName, } #[derive(Debug, thiserror::Error)] From 06a75e8bd2b1303c775e6a34d59db9601fdab9a9 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Wed, 17 Sep 2025 21:41:35 -0700 Subject: [PATCH 2/5] add , to the validation, update validation error status to 400 --- src/handlers/http/modal/query/querier_role.rs | 2 +- src/handlers/http/role.rs | 7 +++++-- src/validator.rs | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/handlers/http/modal/query/querier_role.rs b/src/handlers/http/modal/query/querier_role.rs index 10893e556..4a9c77547 100644 --- a/src/handlers/http/modal/query/querier_role.rs +++ b/src/handlers/http/modal/query/querier_role.rs @@ -44,7 +44,7 @@ pub async fn put( ) -> Result { let name = name.into_inner(); // validate the role name - validator::user_role_name(&name).map_err(|e| RoleError::Anyhow(e.into()))?; + validator::user_role_name(&name).map_err(RoleError::ValidationError)?; let mut metadata = get_metadata().await?; metadata.roles.insert(name.clone(), privileges.clone()); diff --git a/src/handlers/http/role.rs b/src/handlers/http/role.rs index 1746903f6..6b2a3dbbd 100644 --- a/src/handlers/http/role.rs +++ b/src/handlers/http/role.rs @@ -32,7 +32,7 @@ use crate::{ role::model::DefaultPrivilege, }, storage::{self, ObjectStorageError, StorageMetadata}, - validator, + validator::{self, error::UsernameValidationError}, }; // Handler for PUT /api/v1/role/{name} @@ -43,7 +43,7 @@ pub async fn put( ) -> Result { let name = name.into_inner(); // validate the role name - validator::user_role_name(&name).map_err(|e| RoleError::Anyhow(e.into()))?; + validator::user_role_name(&name).map_err(RoleError::ValidationError)?; let mut metadata = get_metadata().await?; metadata.roles.insert(name.clone(), privileges.clone()); @@ -171,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 { @@ -181,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, } } diff --git a/src/validator.rs b/src/validator.rs index dd0c3b082..8836171a4 100644 --- a/src/validator.rs +++ b/src/validator.rs @@ -125,7 +125,7 @@ pub fn user_role_name(name: &str) -> Result<(), UsernameValidationError> { prev_was_special = false; } // Allow specific special characters - '_' | '-' | '.' => { + '_' | '-' | '.' | ',' => { if prev_was_special { return Err(UsernameValidationError::ConsecutiveSpecialChars); } @@ -200,7 +200,7 @@ pub mod error { #[error("Username should end with an alphanumeric character")] InvalidEndChar, #[error( - "Username contains invalid characters. Only alphanumeric characters and _, -, . are allowed" + "Username contains invalid characters. Only alphanumeric characters and _, -, ., , are allowed" )] InvalidCharacter, #[error("Username contains consecutive special characters")] From ec36175d2485601d6dbfba3dacb3ca2963317376 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Wed, 17 Sep 2025 21:47:27 -0700 Subject: [PATCH 3/5] refactor --- src/handlers/http/modal/query/querier_rbac.rs | 16 ++++++++-------- src/handlers/http/rbac.rs | 2 +- src/validator.rs | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index 9bf0eaae2..8cbf78e83 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -172,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 @@ -228,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 diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index 33e693861..0ad06a6e2 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -120,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 diff --git a/src/validator.rs b/src/validator.rs index 8836171a4..c224b27e5 100644 --- a/src/validator.rs +++ b/src/validator.rs @@ -192,7 +192,7 @@ 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 _, -, ., , are allowed" )] SpecialChar, #[error("Username should start with an alphanumeric character")] From 180266e8b1dedc5b8e2682d0aafd6acbce14a38e Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Wed, 17 Sep 2025 21:53:28 -0700 Subject: [PATCH 4/5] do not allow comma in names --- src/validator.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/validator.rs b/src/validator.rs index c224b27e5..d27b81fac 100644 --- a/src/validator.rs +++ b/src/validator.rs @@ -125,7 +125,7 @@ pub fn user_role_name(name: &str) -> Result<(), UsernameValidationError> { prev_was_special = false; } // Allow specific special characters - '_' | '-' | '.' | ',' => { + '_' | '-' | '.' => { if prev_was_special { return Err(UsernameValidationError::ConsecutiveSpecialChars); } @@ -192,7 +192,7 @@ pub mod error { #[error("Username should be between 3 and 64 chars long")] InvalidLength, #[error( - "Username contains invalid characters. Only alphanumeric characters and _, -, ., , are allowed" + "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")] @@ -200,7 +200,7 @@ pub mod error { #[error("Username should end with an alphanumeric character")] InvalidEndChar, #[error( - "Username contains invalid characters. Only alphanumeric characters and _, -, ., , are allowed" + "Username contains invalid characters. Only alphanumeric characters and special characters (underscore, hyphen and dot) are allowed" )] InvalidCharacter, #[error("Username contains consecutive special characters")] From d7a8b7fff5dd34f15917f84749fe3a296f59d392 Mon Sep 17 00:00:00 2001 From: Nikhil Sinha Date: Thu, 18 Sep 2025 02:49:32 -0700 Subject: [PATCH 5/5] removed check of 1st character --- src/validator.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/validator.rs b/src/validator.rs index d27b81fac..f095fc032 100644 --- a/src/validator.rs +++ b/src/validator.rs @@ -102,13 +102,6 @@ pub fn user_role_name(name: &str) -> Result<(), UsernameValidationError> { let chars: Vec = name.chars().collect(); - // Check first character (must be alphanumeric) - if let Some(first_char) = chars.first() - && !first_char.is_ascii_alphanumeric() - { - return Err(UsernameValidationError::InvalidStartChar); - } - // Check last character (must be alphanumeric) if let Some(last_char) = chars.last() && !last_char.is_ascii_alphanumeric()