Skip to content

Commit 3397c9a

Browse files
committed
feat: accept caller-provided user passwords
1 parent 13d9b06 commit 3397c9a

3 files changed

Lines changed: 130 additions & 13 deletions

File tree

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,11 @@ pub async fn post_user(
5353
validator::user_role_name(&username)?;
5454
let mut metadata = get_metadata(&tenant_id).await?;
5555

56-
let user_roles: HashSet<String> = if let Some(body) = body {
57-
serde_json::from_value(body.into_inner())?
58-
} else {
59-
HashSet::new()
60-
};
56+
let (user_roles, provided_password) = body
57+
.map(|body| serde_json::from_value::<user::CreateUserRequest>(body.into_inner()))
58+
.transpose()?
59+
.map(user::CreateUserRequest::split_request)
60+
.unwrap_or_default();
6161

6262
let mut non_existent_roles = Vec::new();
6363
for role in &user_roles {
@@ -83,7 +83,15 @@ pub async fn post_user(
8383
return Err(RBACError::UserExists(username));
8484
}
8585

86-
let (mut user, password) = user::User::new_basic(username.clone(), tenant_id.clone(), false);
86+
let (mut user, password) = match provided_password {
87+
Some(password) => user::User::new_basic_with_provided_password(
88+
username.clone(),
89+
tenant_id.clone(),
90+
false,
91+
password,
92+
)?,
93+
None => user::User::new_basic(username.clone(), tenant_id.clone(), false),
94+
};
8795
// add user roles
8896
user.roles.clone_from(&user_roles);
8997
metadata.users.push(user.clone());

‎src/handlers/http/rbac.rs‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,11 @@ pub async fn post_user(
142142
validator::user_role_name(&userid)?;
143143
let mut metadata = get_metadata(&tenant_id).await?;
144144

145-
let user_roles: HashSet<String> = if let Some(body) = body {
146-
serde_json::from_value(body.into_inner())?
147-
} else {
148-
HashSet::new()
149-
};
145+
let (user_roles, provided_password) = body
146+
.map(|body| serde_json::from_value::<user::CreateUserRequest>(body.into_inner()))
147+
.transpose()?
148+
.map(user::CreateUserRequest::split_request)
149+
.unwrap_or_default();
150150

151151
let mut non_existent_roles = Vec::new();
152152
for role in &user_roles {
@@ -171,7 +171,15 @@ pub async fn post_user(
171171
return Err(RBACError::UserExists(userid));
172172
}
173173

174-
let (user, password) = user::User::new_basic(userid.clone(), tenant_id.clone(), false);
174+
let (user, password) = match provided_password {
175+
Some(password) => user::User::new_basic_with_provided_password(
176+
userid.clone(),
177+
tenant_id.clone(),
178+
false,
179+
password,
180+
)?,
181+
None => user::User::new_basic(userid.clone(), tenant_id.clone(), false),
182+
};
175183

176184
metadata.users.push(user.clone());
177185

@@ -552,6 +560,8 @@ pub enum RBACError {
552560
ProtectedUser,
553561
#[error("Role is protected")]
554562
ProtectedRole,
563+
#[error("providedPassword must be 8-256 bytes with no control characters")]
564+
InvalidProvidedPassword,
555565
#[error("{0}")]
556566
SerdeError(#[from] serde_json::Error),
557567
#[error("Failed to connect to storage: {0}")]
@@ -590,6 +600,7 @@ impl actix_web::ResponseError for RBACError {
590600
Self::UserExists(_) => StatusCode::BAD_REQUEST,
591601
Self::ProtectedUser => StatusCode::BAD_REQUEST,
592602
Self::ProtectedRole => StatusCode::BAD_REQUEST,
603+
Self::InvalidProvidedPassword => StatusCode::BAD_REQUEST,
593604
Self::UserDoesNotExist => StatusCode::NOT_FOUND,
594605
Self::SerdeError(_) => StatusCode::BAD_REQUEST,
595606
Self::ValidationError(_) => StatusCode::BAD_REQUEST,

‎src/rbac/user.rs‎

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,65 @@ pub struct User {
103103
pub protected: bool,
104104
}
105105

106+
/// Accept the UI's existing roles array or an object with a caller-supplied password.
107+
#[derive(Deserialize)]
108+
#[serde(untagged)]
109+
pub enum CreateUserRequest {
110+
Roles(HashSet<String>),
111+
Options(CreateUserOptions),
112+
}
113+
114+
#[derive(Deserialize)]
115+
#[serde(rename_all = "camelCase", deny_unknown_fields)]
116+
pub struct CreateUserOptions {
117+
#[serde(default)]
118+
roles: HashSet<String>,
119+
provided_password: Option<String>,
120+
}
121+
122+
impl CreateUserRequest {
123+
pub fn split_request(self) -> (HashSet<String>, Option<String>) {
124+
match self {
125+
Self::Roles(roles) => (roles, None),
126+
Self::Options(options) => (options.roles, options.provided_password),
127+
}
128+
}
129+
}
130+
106131
impl User {
107132
// create a new User and return self with password generated for said user.
108133
pub fn new_basic(username: String, tenant: Option<String>, protected: bool) -> (Self, String) {
109-
let PassCode { password, hash } = Basic::gen_new_password();
134+
Self::new_basic_with_passcode(username, tenant, protected, Basic::gen_new_password())
135+
}
136+
137+
pub fn new_basic_with_provided_password(
138+
username: String,
139+
tenant: Option<String>,
140+
protected: bool,
141+
password: String,
142+
) -> Result<(Self, String), RBACError> {
143+
if !(8..=256).contains(&password.len())
144+
|| password.trim() != password
145+
|| password.chars().any(char::is_control)
146+
{
147+
return Err(RBACError::InvalidProvidedPassword);
148+
}
149+
150+
let hash = gen_hash(&password);
151+
Ok(Self::new_basic_with_passcode(
152+
username,
153+
tenant,
154+
protected,
155+
PassCode { password, hash },
156+
))
157+
}
158+
159+
fn new_basic_with_passcode(
160+
username: String,
161+
tenant: Option<String>,
162+
protected: bool,
163+
PassCode { password, hash }: PassCode,
164+
) -> (Self, String) {
110165
(
111166
Self {
112167
ty: UserType::Native(Basic {
@@ -705,6 +760,49 @@ impl UserGroup {
705760
}
706761
}
707762

763+
#[cfg(test)]
764+
mod provided_password_tests {
765+
use super::*;
766+
767+
#[test]
768+
fn accepts_legacy_roles_array_and_new_password_object() {
769+
let legacy: CreateUserRequest = serde_json::from_str(r#"["reader"]"#).unwrap();
770+
let (roles, password) = legacy.split_request();
771+
assert!(roles.contains("reader"));
772+
assert!(password.is_none());
773+
774+
let modern: CreateUserRequest = serde_json::from_str(
775+
r#"{"roles":["reader"],"providedPassword":"a_valid_password_123"}"#,
776+
)
777+
.unwrap();
778+
let (roles, password) = modern.split_request();
779+
assert!(roles.contains("reader"));
780+
assert_eq!(password.as_deref(), Some("a_valid_password_123"));
781+
782+
assert!(
783+
serde_json::from_str::<CreateUserRequest>(r#"{"providedPasswrod":"typo"}"#).is_err()
784+
);
785+
}
786+
787+
#[test]
788+
fn supplied_password_is_hashed_and_invalid_values_are_rejected() {
789+
let (user, returned) =
790+
User::new_basic_with_provided_password("alice".into(), None, false, "Pa$$w0rd".into())
791+
.unwrap();
792+
assert_eq!(returned, "Pa$$w0rd");
793+
let UserType::Native(basic) = &user.ty else {
794+
panic!("expected native user")
795+
};
796+
assert!(basic.verify_password(&returned));
797+
assert!(!serde_json::to_string(&user).unwrap().contains(&returned));
798+
799+
assert!(matches!(
800+
User::new_basic_with_provided_password("bob".into(), None, false, "short".into()),
801+
Err(RBACError::InvalidProvidedPassword)
802+
));
803+
}
804+
}
805+
708806
#[cfg(test)]
709807
mod apikey_serde_tests {
710808
use super::*;

0 commit comments

Comments
 (0)