diff --git a/src/apikeys.rs b/src/apikeys.rs index d139b351a..9f8cb3b1f 100644 --- a/src/apikeys.rs +++ b/src/apikeys.rs @@ -30,12 +30,24 @@ 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). +/// `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 api_key: Option, +} + +/// 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(()) } #[derive(Debug, thiserror::Error)] @@ -46,6 +58,12 @@ pub enum ApiKeyError { #[error("Duplicate key name: {0}")] DuplicateKeyName(String), + #[error("API key value is already registered")] + DuplicateApiKey, + + #[error("apiKey must be a UUID v4 in 8-4-4-4-12 format")] + InvalidApiKey, + #[error("Unauthorized: {0}")] Unauthorized(String), @@ -66,7 +84,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::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 7cf4fa99d..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}, + 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,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(api_key) = body.api_key { + validate_api_key_value(&api_key)?; + 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, diff --git a/src/handlers/http/modal/query/querier_rbac.rs b/src/handlers/http/modal/query/querier_rbac.rs index 576a500b8..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: HashSet = if let Some(body) = body { - serde_json::from_value(body.into_inner())? - } else { - HashSet::new() - }; + let (user_roles, password) = body.split_request(); let mut non_existent_roles = Vec::new(); for role in &user_roles { @@ -83,7 +79,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 password { + Some(password) => user::User::new_basic_with_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..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: HashSet = if let Some(body) = body { - serde_json::from_value(body.into_inner())? - } else { - HashSet::new() - }; + let (user_roles, password) = body.split_request(); let mut non_existent_roles = Vec::new(); for role in &user_roles { @@ -171,7 +167,12 @@ 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 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), + }; metadata.users.push(user.clone()); @@ -552,6 +553,8 @@ pub enum RBACError { ProtectedUser, #[error("Role is protected")] ProtectedRole, + #[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}")] @@ -590,6 +593,7 @@ impl actix_web::ResponseError for RBACError { Self::UserExists(_) => StatusCode::BAD_REQUEST, Self::ProtectedUser => StatusCode::BAD_REQUEST, Self::ProtectedRole => 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 76b62a6aa..f58ebc315 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 CreateUserOptions { + Roles(HashSet), + Details(CreateUserDetails), +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +pub struct CreateUserDetails { + #[serde(default)] + roles: HashSet, + password: Option, +} + +impl CreateUserOptions { + pub fn split_request(self) -> (HashSet, Option) { + match self { + Self::Roles(roles) => (roles, None), + Self::Details(details) => (details.roles, details.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_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::InvalidPassword); + } + + 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,47 @@ impl UserGroup { } } +#[cfg(test)] +mod password_request_tests { + use super::*; + + #[test] + fn accepts_legacy_roles_array_and_new_password_object() { + 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: 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#"{"providedPassword":"old"}"#).is_err() + ); + } + + #[test] + fn supplied_password_is_hashed_and_invalid_values_are_rejected() { + let (user, returned) = + 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") + }; + assert!(basic.verify_password(&returned)); + assert!(!serde_json::to_string(&user).unwrap().contains(&returned)); + + assert!(matches!( + User::new_basic_with_password("bob".into(), None, false, "short".into()), + Err(RBACError::InvalidPassword) + )); + } +} + #[cfg(test)] mod apikey_serde_tests { use super::*;