From 13d9b066d5b6885b135c190ad0e7fa636b76742f Mon Sep 17 00:00:00 2001 From: Pratik Jadhav Date: Wed, 7 Oct 2026 15:45:49 +0530 Subject: [PATCH 1/3] feat: accept caller-provided API keys --- src/apikeys.rs | 26 +++++++++++++++++++++++++- src/handlers/http/apikeys.rs | 16 ++++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/apikeys.rs b/src/apikeys.rs index d139b351a..b1db6879f 100644 --- a/src/apikeys.rs +++ b/src/apikeys.rs @@ -30,12 +30,27 @@ use serde::Deserialize; /// Request body for creating a new API key. `roles` is a set of role names /// that must already exist in the tenant; permissions for the backing user /// are derived from these roles (same flow as native/OAuth users). +/// `providedApiKey` lets a caller register their own key; omitting it preserves +/// the existing server-generated key behavior. #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub struct CreateApiKeyRequest { pub key_name: String, #[serde(default)] pub roles: HashSet, + pub provided_api_key: Option, +} + +/// Reject short or malformed caller-supplied credentials before persisting them. +pub fn validate_provided_api_key(value: &str) -> Result<(), ApiKeyError> { + if !(32..=256).contains(&value.len()) + || !value + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-' || byte == b'_') + { + return Err(ApiKeyError::InvalidProvidedApiKey); + } + Ok(()) } #[derive(Debug, thiserror::Error)] @@ -46,6 +61,12 @@ pub enum ApiKeyError { #[error("Duplicate key name: {0}")] DuplicateKeyName(String), + #[error("API key value is already registered")] + DuplicateApiKey, + + #[error("providedApiKey must be 32-256 ASCII letters, digits, hyphens, or underscores")] + InvalidProvidedApiKey, + #[error("Unauthorized: {0}")] Unauthorized(String), @@ -66,7 +87,10 @@ impl actix_web::ResponseError for ApiKeyError { fn status_code(&self) -> actix_web::http::StatusCode { match self { ApiKeyError::KeyNotFound(_) => actix_web::http::StatusCode::NOT_FOUND, - ApiKeyError::DuplicateKeyName(_) => actix_web::http::StatusCode::CONFLICT, + ApiKeyError::DuplicateKeyName(_) | ApiKeyError::DuplicateApiKey => { + actix_web::http::StatusCode::CONFLICT + } + ApiKeyError::InvalidProvidedApiKey => actix_web::http::StatusCode::BAD_REQUEST, ApiKeyError::Unauthorized(_) => actix_web::http::StatusCode::FORBIDDEN, ApiKeyError::RevocationSyncFailed(_) => { actix_web::http::StatusCode::SERVICE_UNAVAILABLE diff --git a/src/handlers/http/apikeys.rs b/src/handlers/http/apikeys.rs index 7cf4fa99d..4c0974b4a 100644 --- a/src/handlers/http/apikeys.rs +++ b/src/handlers/http/apikeys.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use ulid::Ulid; use crate::{ - apikeys::{ApiKeyError, CreateApiKeyRequest}, + apikeys::{ApiKeyError, CreateApiKeyRequest, validate_provided_api_key}, handlers::http::{ modal::utils::rbac_utils::{get_metadata, put_metadata}, rbac::{RBACError, UPDATE_LOCK}, @@ -187,7 +187,19 @@ pub async fn create_api_key( } let key_id = Ulid::new(); - let api_key_value = generate_api_key_value(); + let api_key_value = if let Some(provided_api_key) = body.provided_api_key { + validate_provided_api_key(&provided_api_key)?; + provided_api_key + } else { + generate_api_key_value() + }; + if existing + .iter() + .filter_map(|user: &User| user.as_api_key()) + .any(|key| key.api_key == api_key_value) + { + return Err(ApiKeyError::DuplicateApiKey); + } let user = User::new_api_key( key_id, api_key_value, From 3397c9acdaabbb08642256a14497b9060498b190 Mon Sep 17 00:00:00 2001 From: Pratik Jadhav Date: Thu, 8 Oct 2026 13:32:47 +0530 Subject: [PATCH 2/3] feat: accept caller-provided user passwords --- src/handlers/http/modal/query/querier_rbac.rs | 20 ++-- src/handlers/http/rbac.rs | 23 ++-- src/rbac/user.rs | 100 +++++++++++++++++- 3 files changed, 130 insertions(+), 13 deletions(-) diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index 576a500b8..9a2c6d16d 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -53,11 +53,11 @@ pub async fn post_user( validator::user_role_name(&username)?; let mut metadata = get_metadata(&tenant_id).await?; - let user_roles: HashSet = if let Some(body) = body { - serde_json::from_value(body.into_inner())? - } else { - HashSet::new() - }; + let (user_roles, provided_password) = body + .map(|body| serde_json::from_value::(body.into_inner())) + .transpose()? + .map(user::CreateUserRequest::split_request) + .unwrap_or_default(); let mut non_existent_roles = Vec::new(); for role in &user_roles { @@ -83,7 +83,15 @@ pub async fn post_user( return Err(RBACError::UserExists(username)); } - let (mut user, password) = user::User::new_basic(username.clone(), tenant_id.clone(), false); + let (mut user, password) = match provided_password { + Some(password) => user::User::new_basic_with_provided_password( + username.clone(), + tenant_id.clone(), + false, + password, + )?, + None => user::User::new_basic(username.clone(), tenant_id.clone(), false), + }; // add user roles user.roles.clone_from(&user_roles); metadata.users.push(user.clone()); diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index 6d5bf1896..7ffcf4e53 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -142,11 +142,11 @@ pub async fn post_user( validator::user_role_name(&userid)?; let mut metadata = get_metadata(&tenant_id).await?; - let user_roles: HashSet = if let Some(body) = body { - serde_json::from_value(body.into_inner())? - } else { - HashSet::new() - }; + let (user_roles, provided_password) = body + .map(|body| serde_json::from_value::(body.into_inner())) + .transpose()? + .map(user::CreateUserRequest::split_request) + .unwrap_or_default(); let mut non_existent_roles = Vec::new(); for role in &user_roles { @@ -171,7 +171,15 @@ pub async fn post_user( return Err(RBACError::UserExists(userid)); } - let (user, password) = user::User::new_basic(userid.clone(), tenant_id.clone(), false); + let (user, password) = match provided_password { + Some(password) => user::User::new_basic_with_provided_password( + userid.clone(), + tenant_id.clone(), + false, + password, + )?, + None => user::User::new_basic(userid.clone(), tenant_id.clone(), false), + }; metadata.users.push(user.clone()); @@ -552,6 +560,8 @@ pub enum RBACError { ProtectedUser, #[error("Role is protected")] ProtectedRole, + #[error("providedPassword must be 8-256 bytes with no control characters")] + InvalidProvidedPassword, #[error("{0}")] SerdeError(#[from] serde_json::Error), #[error("Failed to connect to storage: {0}")] @@ -590,6 +600,7 @@ impl actix_web::ResponseError for RBACError { Self::UserExists(_) => StatusCode::BAD_REQUEST, Self::ProtectedUser => StatusCode::BAD_REQUEST, Self::ProtectedRole => StatusCode::BAD_REQUEST, + Self::InvalidProvidedPassword => StatusCode::BAD_REQUEST, Self::UserDoesNotExist => StatusCode::NOT_FOUND, Self::SerdeError(_) => StatusCode::BAD_REQUEST, Self::ValidationError(_) => StatusCode::BAD_REQUEST, diff --git a/src/rbac/user.rs b/src/rbac/user.rs index 76b62a6aa..17f6c7a74 100644 --- a/src/rbac/user.rs +++ b/src/rbac/user.rs @@ -103,10 +103,65 @@ pub struct User { pub protected: bool, } +/// Accept the UI's existing roles array or an object with a caller-supplied password. +#[derive(Deserialize)] +#[serde(untagged)] +pub enum CreateUserRequest { + Roles(HashSet), + Options(CreateUserOptions), +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct CreateUserOptions { + #[serde(default)] + roles: HashSet, + provided_password: Option, +} + +impl CreateUserRequest { + pub fn split_request(self) -> (HashSet, Option) { + match self { + Self::Roles(roles) => (roles, None), + Self::Options(options) => (options.roles, options.provided_password), + } + } +} + impl User { // create a new User and return self with password generated for said user. pub fn new_basic(username: String, tenant: Option, protected: bool) -> (Self, String) { - let PassCode { password, hash } = Basic::gen_new_password(); + Self::new_basic_with_passcode(username, tenant, protected, Basic::gen_new_password()) + } + + pub fn new_basic_with_provided_password( + username: String, + tenant: Option, + protected: bool, + password: String, + ) -> Result<(Self, String), RBACError> { + if !(8..=256).contains(&password.len()) + || password.trim() != password + || password.chars().any(char::is_control) + { + return Err(RBACError::InvalidProvidedPassword); + } + + let hash = gen_hash(&password); + Ok(Self::new_basic_with_passcode( + username, + tenant, + protected, + PassCode { password, hash }, + )) + } + + fn new_basic_with_passcode( + username: String, + tenant: Option, + protected: bool, + PassCode { password, hash }: PassCode, + ) -> (Self, String) { ( Self { ty: UserType::Native(Basic { @@ -705,6 +760,49 @@ impl UserGroup { } } +#[cfg(test)] +mod provided_password_tests { + use super::*; + + #[test] + fn accepts_legacy_roles_array_and_new_password_object() { + let legacy: CreateUserRequest = serde_json::from_str(r#"["reader"]"#).unwrap(); + let (roles, password) = legacy.split_request(); + assert!(roles.contains("reader")); + assert!(password.is_none()); + + let modern: CreateUserRequest = serde_json::from_str( + r#"{"roles":["reader"],"providedPassword":"a_valid_password_123"}"#, + ) + .unwrap(); + let (roles, password) = modern.split_request(); + assert!(roles.contains("reader")); + assert_eq!(password.as_deref(), Some("a_valid_password_123")); + + assert!( + serde_json::from_str::(r#"{"providedPasswrod":"typo"}"#).is_err() + ); + } + + #[test] + fn supplied_password_is_hashed_and_invalid_values_are_rejected() { + let (user, returned) = + User::new_basic_with_provided_password("alice".into(), None, false, "Pa$$w0rd".into()) + .unwrap(); + assert_eq!(returned, "Pa$$w0rd"); + let UserType::Native(basic) = &user.ty else { + panic!("expected native user") + }; + assert!(basic.verify_password(&returned)); + assert!(!serde_json::to_string(&user).unwrap().contains(&returned)); + + assert!(matches!( + User::new_basic_with_provided_password("bob".into(), None, false, "short".into()), + Err(RBACError::InvalidProvidedPassword) + )); + } +} + #[cfg(test)] mod apikey_serde_tests { use super::*; From 98883881fb6a15c903712e26dd3003d88c7ecf45 Mon Sep 17 00:00:00 2001 From: Pratik Jadhav Date: Thu, 8 Oct 2026 16:32:57 +0530 Subject: [PATCH 3/3] feat: accept UUID v4 API keys and simplify user credentials --- src/apikeys.rs | 25 ++++++------- src/handlers/http/apikeys.rs | 8 ++--- src/handlers/http/modal/query/querier_rbac.rs | 12 +++---- src/handlers/http/rbac.rs | 25 +++++-------- src/rbac/user.rs | 36 +++++++++---------- 5 files changed, 45 insertions(+), 61 deletions(-) diff --git a/src/apikeys.rs b/src/apikeys.rs index b1db6879f..9f8cb3b1f 100644 --- a/src/apikeys.rs +++ b/src/apikeys.rs @@ -30,25 +30,22 @@ use serde::Deserialize; /// Request body for creating a new API key. `roles` is a set of role names /// that must already exist in the tenant; permissions for the backing user /// are derived from these roles (same flow as native/OAuth users). -/// `providedApiKey` lets a caller register their own key; omitting it preserves +/// `apiKey` lets a caller register their own key; omitting it preserves /// the existing server-generated key behavior. #[derive(Debug, Deserialize)] -#[serde(rename_all = "camelCase")] +#[serde(rename_all = "camelCase", deny_unknown_fields)] pub struct CreateApiKeyRequest { pub key_name: String, #[serde(default)] pub roles: HashSet, - pub provided_api_key: Option, + pub api_key: Option, } -/// Reject short or malformed caller-supplied credentials before persisting them. -pub fn validate_provided_api_key(value: &str) -> Result<(), ApiKeyError> { - if !(32..=256).contains(&value.len()) - || !value - .bytes() - .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-' || byte == b'_') - { - return Err(ApiKeyError::InvalidProvidedApiKey); +/// Require a hyphenated UUID v4, allowing either letter case. +pub fn validate_api_key(value: &str) -> Result<(), ApiKeyError> { + let uuid = uuid::Uuid::parse_str(value).map_err(|_| ApiKeyError::InvalidApiKey)?; + if value.len() != 36 || uuid.get_version_num() != 4 { + return Err(ApiKeyError::InvalidApiKey); } Ok(()) } @@ -64,8 +61,8 @@ pub enum ApiKeyError { #[error("API key value is already registered")] DuplicateApiKey, - #[error("providedApiKey must be 32-256 ASCII letters, digits, hyphens, or underscores")] - InvalidProvidedApiKey, + #[error("apiKey must be a UUID v4 in 8-4-4-4-12 format")] + InvalidApiKey, #[error("Unauthorized: {0}")] Unauthorized(String), @@ -90,7 +87,7 @@ impl actix_web::ResponseError for ApiKeyError { ApiKeyError::DuplicateKeyName(_) | ApiKeyError::DuplicateApiKey => { actix_web::http::StatusCode::CONFLICT } - ApiKeyError::InvalidProvidedApiKey => actix_web::http::StatusCode::BAD_REQUEST, + ApiKeyError::InvalidApiKey => actix_web::http::StatusCode::BAD_REQUEST, ApiKeyError::Unauthorized(_) => actix_web::http::StatusCode::FORBIDDEN, ApiKeyError::RevocationSyncFailed(_) => { actix_web::http::StatusCode::SERVICE_UNAVAILABLE diff --git a/src/handlers/http/apikeys.rs b/src/handlers/http/apikeys.rs index 4c0974b4a..8efb7160b 100644 --- a/src/handlers/http/apikeys.rs +++ b/src/handlers/http/apikeys.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use ulid::Ulid; use crate::{ - apikeys::{ApiKeyError, CreateApiKeyRequest, validate_provided_api_key}, + apikeys::{ApiKeyError, CreateApiKeyRequest, validate_api_key as validate_api_key_value}, handlers::http::{ modal::utils::rbac_utils::{get_metadata, put_metadata}, rbac::{RBACError, UPDATE_LOCK}, @@ -187,9 +187,9 @@ pub async fn create_api_key( } let key_id = Ulid::new(); - let api_key_value = if let Some(provided_api_key) = body.provided_api_key { - validate_provided_api_key(&provided_api_key)?; - provided_api_key + let api_key_value = if let Some(api_key) = body.api_key { + validate_api_key_value(&api_key)?; + api_key } else { generate_api_key_value() }; diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index 9a2c6d16d..0638f43ec 100644 --- a/src/handlers/http/modal/query/querier_rbac.rs +++ b/src/handlers/http/modal/query/querier_rbac.rs @@ -45,7 +45,7 @@ use crate::{ pub async fn post_user( req: HttpRequest, userid: web::Path, - body: Option>, + web::Json(body): web::Json, ) -> Result { let username = userid.into_inner(); let tenant_id = get_tenant_id_from_request(&req); @@ -53,11 +53,7 @@ pub async fn post_user( validator::user_role_name(&username)?; let mut metadata = get_metadata(&tenant_id).await?; - let (user_roles, provided_password) = body - .map(|body| serde_json::from_value::(body.into_inner())) - .transpose()? - .map(user::CreateUserRequest::split_request) - .unwrap_or_default(); + let (user_roles, password) = body.split_request(); let mut non_existent_roles = Vec::new(); for role in &user_roles { @@ -83,8 +79,8 @@ pub async fn post_user( return Err(RBACError::UserExists(username)); } - let (mut user, password) = match provided_password { - Some(password) => user::User::new_basic_with_provided_password( + let (mut user, password) = match password { + Some(password) => user::User::new_basic_with_password( username.clone(), tenant_id.clone(), false, diff --git a/src/handlers/http/rbac.rs b/src/handlers/http/rbac.rs index 7ffcf4e53..a8e68a0e4 100644 --- a/src/handlers/http/rbac.rs +++ b/src/handlers/http/rbac.rs @@ -135,18 +135,14 @@ pub fn get_user_internal( pub async fn post_user( req: HttpRequest, userid: web::Path, - body: Option>, + web::Json(body): web::Json, ) -> Result { let userid = userid.into_inner(); let tenant_id = get_tenant_id_from_request(&req); validator::user_role_name(&userid)?; let mut metadata = get_metadata(&tenant_id).await?; - let (user_roles, provided_password) = body - .map(|body| serde_json::from_value::(body.into_inner())) - .transpose()? - .map(user::CreateUserRequest::split_request) - .unwrap_or_default(); + let (user_roles, password) = body.split_request(); let mut non_existent_roles = Vec::new(); for role in &user_roles { @@ -171,13 +167,10 @@ pub async fn post_user( return Err(RBACError::UserExists(userid)); } - let (user, password) = match provided_password { - Some(password) => user::User::new_basic_with_provided_password( - userid.clone(), - tenant_id.clone(), - false, - password, - )?, + let (user, password) = match password { + Some(password) => { + user::User::new_basic_with_password(userid.clone(), tenant_id.clone(), false, password)? + } None => user::User::new_basic(userid.clone(), tenant_id.clone(), false), }; @@ -560,8 +553,8 @@ pub enum RBACError { ProtectedUser, #[error("Role is protected")] ProtectedRole, - #[error("providedPassword must be 8-256 bytes with no control characters")] - InvalidProvidedPassword, + #[error("password must be 8-256 bytes with no control characters or surrounding whitespace")] + InvalidPassword, #[error("{0}")] SerdeError(#[from] serde_json::Error), #[error("Failed to connect to storage: {0}")] @@ -600,7 +593,7 @@ impl actix_web::ResponseError for RBACError { Self::UserExists(_) => StatusCode::BAD_REQUEST, Self::ProtectedUser => StatusCode::BAD_REQUEST, Self::ProtectedRole => StatusCode::BAD_REQUEST, - Self::InvalidProvidedPassword => StatusCode::BAD_REQUEST, + Self::InvalidPassword => StatusCode::BAD_REQUEST, Self::UserDoesNotExist => StatusCode::NOT_FOUND, Self::SerdeError(_) => StatusCode::BAD_REQUEST, Self::ValidationError(_) => StatusCode::BAD_REQUEST, diff --git a/src/rbac/user.rs b/src/rbac/user.rs index 17f6c7a74..f58ebc315 100644 --- a/src/rbac/user.rs +++ b/src/rbac/user.rs @@ -106,24 +106,24 @@ pub struct User { /// Accept the UI's existing roles array or an object with a caller-supplied password. #[derive(Deserialize)] #[serde(untagged)] -pub enum CreateUserRequest { +pub enum CreateUserOptions { Roles(HashSet), - Options(CreateUserOptions), + Details(CreateUserDetails), } #[derive(Deserialize)] #[serde(rename_all = "camelCase", deny_unknown_fields)] -pub struct CreateUserOptions { +pub struct CreateUserDetails { #[serde(default)] roles: HashSet, - provided_password: Option, + password: Option, } -impl CreateUserRequest { +impl CreateUserOptions { pub fn split_request(self) -> (HashSet, Option) { match self { Self::Roles(roles) => (roles, None), - Self::Options(options) => (options.roles, options.provided_password), + Self::Details(details) => (details.roles, details.password), } } } @@ -134,7 +134,7 @@ impl User { Self::new_basic_with_passcode(username, tenant, protected, Basic::gen_new_password()) } - pub fn new_basic_with_provided_password( + pub fn new_basic_with_password( username: String, tenant: Option, protected: bool, @@ -144,7 +144,7 @@ impl User { || password.trim() != password || password.chars().any(char::is_control) { - return Err(RBACError::InvalidProvidedPassword); + return Err(RBACError::InvalidPassword); } let hash = gen_hash(&password); @@ -761,34 +761,32 @@ impl UserGroup { } #[cfg(test)] -mod provided_password_tests { +mod password_request_tests { use super::*; #[test] fn accepts_legacy_roles_array_and_new_password_object() { - let legacy: CreateUserRequest = serde_json::from_str(r#"["reader"]"#).unwrap(); + let legacy: CreateUserOptions = serde_json::from_str(r#"["reader"]"#).unwrap(); let (roles, password) = legacy.split_request(); assert!(roles.contains("reader")); assert!(password.is_none()); - let modern: CreateUserRequest = serde_json::from_str( - r#"{"roles":["reader"],"providedPassword":"a_valid_password_123"}"#, - ) - .unwrap(); + let modern: CreateUserOptions = + serde_json::from_str(r#"{"roles":["reader"],"password":"a_valid_password_123"}"#) + .unwrap(); let (roles, password) = modern.split_request(); assert!(roles.contains("reader")); assert_eq!(password.as_deref(), Some("a_valid_password_123")); assert!( - serde_json::from_str::(r#"{"providedPasswrod":"typo"}"#).is_err() + serde_json::from_str::(r#"{"providedPassword":"old"}"#).is_err() ); } #[test] fn supplied_password_is_hashed_and_invalid_values_are_rejected() { let (user, returned) = - User::new_basic_with_provided_password("alice".into(), None, false, "Pa$$w0rd".into()) - .unwrap(); + User::new_basic_with_password("alice".into(), None, false, "Pa$$w0rd".into()).unwrap(); assert_eq!(returned, "Pa$$w0rd"); let UserType::Native(basic) = &user.ty else { panic!("expected native user") @@ -797,8 +795,8 @@ mod provided_password_tests { assert!(!serde_json::to_string(&user).unwrap().contains(&returned)); assert!(matches!( - User::new_basic_with_provided_password("bob".into(), None, false, "short".into()), - Err(RBACError::InvalidProvidedPassword) + User::new_basic_with_password("bob".into(), None, false, "short".into()), + Err(RBACError::InvalidPassword) )); } }