diff --git a/Cargo.lock b/Cargo.lock index ace603b..eead819 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2536,6 +2536,7 @@ dependencies = [ "ldap3_proto", "lettre", "lldap_auth", + "lldap_validation", "log", "mockall", "nix", @@ -2589,6 +2590,7 @@ dependencies = [ "indexmap 1.6.2", "jwt 0.13.0", "lldap_auth", + "lldap_validation", "rand 0.8.5", "serde", "serde_json", @@ -2653,6 +2655,10 @@ dependencies = [ "serde_json", ] +[[package]] +name = "lldap_validation" +version = "0.6.0" + [[package]] name = "local-channel" version = "0.1.5" diff --git a/app/Cargo.toml b/app/Cargo.toml index f594b5a..6e05553 100644 --- a/app/Cargo.toml +++ b/app/Cargo.toml @@ -60,6 +60,9 @@ features = [ path = "../auth" features = [ "opaque_client" ] +[dependencies.lldap_validation] +path = "../validation" + [dependencies.image] features = ["jpeg"] default-features = false diff --git a/app/src/components/create_group_attribute.rs b/app/src/components/create_group_attribute.rs index 1ae1562..a1ebd01 100644 --- a/app/src/components/create_group_attribute.rs +++ b/app/src/components/create_group_attribute.rs @@ -12,6 +12,7 @@ use crate::{ use anyhow::{bail, Result}; use gloo_console::log; use graphql_client::GraphQLQuery; +use lldap_validation::attributes::validate_attribute_name; use validator_derive::Validate; use yew::prelude::*; use yew_form_derive::Model; @@ -62,6 +63,13 @@ impl CommonComponent for CreateGroupAttributeForm { bail!("Check the form for errors"); } let model = self.form.model(); + validate_attribute_name(&model.attribute_name).or_else(|invalid_chars| { + let invalid = String::from_iter(invalid_chars); + bail!( + "Attribute name contains one or more invalid characters: {}", + invalid + ); + })?; let attribute_type = model.attribute_type.parse::().unwrap(); let req = create_group_attribute::Variables { name: model.attribute_name, diff --git a/app/src/components/create_user_attribute.rs b/app/src/components/create_user_attribute.rs index 88c78fe..0957b7c 100644 --- a/app/src/components/create_user_attribute.rs +++ b/app/src/components/create_user_attribute.rs @@ -12,6 +12,7 @@ use crate::{ use anyhow::{bail, Result}; use gloo_console::log; use graphql_client::GraphQLQuery; +use lldap_validation::attributes::validate_attribute_name; use validator_derive::Validate; use yew::prelude::*; use yew_form_derive::Model; @@ -66,6 +67,13 @@ impl CommonComponent for CreateUserAttributeForm { if model.is_editable && !model.is_visible { bail!("Editable attributes must also be visible"); } + validate_attribute_name(&model.attribute_name).or_else(|invalid_chars| { + let invalid = String::from_iter(invalid_chars); + bail!( + "Attribute name contains one or more invalid characters: {}", + invalid + ); + })?; let attribute_type = model.attribute_type.parse::().unwrap(); let req = create_user_attribute::Variables { name: model.attribute_name, diff --git a/app/src/components/fragments/attribute_schema.rs b/app/src/components/fragments/attribute_schema.rs index 7bc3ace..568334b 100644 --- a/app/src/components/fragments/attribute_schema.rs +++ b/app/src/components/fragments/attribute_schema.rs @@ -1,36 +1,52 @@ +use crate::infra::attributes::AttributeDescription; +use lldap_validation::attributes::{validate_attribute_name, ALLOWED_CHARACTERS_DESCRIPTION}; use yew::{html, Html}; -use crate::infra::attributes::AttributeDescription; - -pub fn render_attribute_name( - hardcoded: bool, - attribute_identifier: &String, - attribute_description: &AttributeDescription, -) -> Html { - if hardcoded { - html! { - <> - {&attribute_description.attribute_name} - { - if attribute_description.aliases.is_empty() { - html!{} - } else { - html!{ - <> -
- - {"Aliases: "} - {attribute_description.aliases.join(", ")} - - - } - } - } - - } +fn render_attribute_aliases(attribute_description: &AttributeDescription) -> Html { + if attribute_description.aliases.is_empty() { + html! {} } else { html! { - {&attribute_identifier} + <> +
+ + {"Aliases: "} + {attribute_description.aliases.join(", ")} + + } } } + +fn render_attribute_validation_warnings(attribute_name: &str) -> Html { + match validate_attribute_name(attribute_name) { + Ok(()) => { + html! {} + } + Err(_invalid_chars) => { + html! { + <> +
+ + {"Warning: This attribute uses one or more invalid characters "} + {"("}{ALLOWED_CHARACTERS_DESCRIPTION}{"). "} + {"Some clients may not support it."} + + + } + } + } +} + +pub fn render_attribute_name( + hardcoded: bool, + attribute_description: &AttributeDescription, +) -> Html { + html! { + <> + {&attribute_description.attribute_name} + {if hardcoded {render_attribute_aliases(attribute_description)} else {html!{}}} + {render_attribute_validation_warnings(attribute_description.attribute_name)} + + } +} diff --git a/app/src/components/group_schema_table.rs b/app/src/components/group_schema_table.rs index 7481253..1c4a504 100644 --- a/app/src/components/group_schema_table.rs +++ b/app/src/components/group_schema_table.rs @@ -157,7 +157,7 @@ impl GroupSchemaTable { let desc = group::resolve_group_attribute_description_or_default(&attribute.name); html! { - {render_attribute_name(hardcoded, &attribute.name, &desc)} + {render_attribute_name(hardcoded, &desc)} {if attribute.is_list { format!("List<{attribute_type}>")} else {attribute_type.to_string()}} {if attribute.is_visible {checkmark.clone()} else {html!{}}} { diff --git a/app/src/components/user_schema_table.rs b/app/src/components/user_schema_table.rs index 2e76cc0..2d8d7d2 100644 --- a/app/src/components/user_schema_table.rs +++ b/app/src/components/user_schema_table.rs @@ -156,7 +156,7 @@ impl UserSchemaTable { let desc = user::resolve_user_attribute_description_or_default(&attribute.name); html! { - {render_attribute_name(hardcoded, &attribute.name, &desc)} + {render_attribute_name(hardcoded, &desc)} {if attribute.is_list { format!("List<{attribute_type}>")} else {attribute_type.to_string()}} {if attribute.is_editable {checkmark.clone()} else {html!{}}} {if attribute.is_visible {checkmark.clone()} else {html!{}}} diff --git a/server/Cargo.toml b/server/Cargo.toml index 1a4addc..ab79b85 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -85,6 +85,9 @@ version = "0.10.1" path = "../auth" features = ["opaque_server", "opaque_client", "sea_orm"] +[dependencies.lldap_validation] +path = "../validation" + [dependencies.opaque-ke] version = "0.7" diff --git a/server/src/infra/graphql/mutation.rs b/server/src/infra/graphql/mutation.rs index e492585..ef15aa1 100644 --- a/server/src/infra/graphql/mutation.rs +++ b/server/src/infra/graphql/mutation.rs @@ -23,7 +23,8 @@ use crate::{ }; use anyhow::{anyhow, Context as AnyhowContext}; use base64::Engine; -use juniper::{graphql_object, FieldResult, GraphQLInputObject, GraphQLObject}; +use juniper::{graphql_object, FieldError, FieldResult, GraphQLInputObject, GraphQLObject}; +use lldap_validation::attributes::{validate_attribute_name, ALLOWED_CHARACTERS_DESCRIPTION}; use tracing::{debug, debug_span, Instrument, Span}; #[derive(PartialEq, Eq, Debug)] @@ -440,6 +441,22 @@ impl Mutation { span.in_scope(|| { debug!(?name, ?attribute_type, is_list, is_visible, is_editable); }); + validate_attribute_name(&name).map_err(|invalid_chars: Vec| -> FieldError { + let chars = String::from_iter(invalid_chars); + span.in_scope(|| { + debug!( + "Cannot create attribute with invalid name. Valid characters: {}. Invalid chars found: {}", + ALLOWED_CHARACTERS_DESCRIPTION, + chars + ) + }); + anyhow!( + "Cannot create attribute with invalid name. Valid characters: {}. Invalid chars found: {}", + ALLOWED_CHARACTERS_DESCRIPTION, + chars + ) + .into() + })?; let handler = context .get_admin_handler() .ok_or_else(field_error_callback( @@ -471,6 +488,20 @@ impl Mutation { span.in_scope(|| { debug!(?name, ?attribute_type, is_list, is_visible, is_editable); }); + validate_attribute_name(&name).map_err(|invalid_chars: Vec| -> FieldError { + let chars = String::from_iter(invalid_chars); + span.in_scope(|| { + debug!( + "Cannot create attribute with invalid name. Invalid chars found: {}", + chars + ) + }); + anyhow!( + "Cannot create attribute with invalid name. Valid characters: {}", + ALLOWED_CHARACTERS_DESCRIPTION + ) + .into() + })?; let handler = context .get_admin_handler() .ok_or_else(field_error_callback( @@ -695,3 +726,240 @@ fn deserialize_attribute( value: deserialized_values, }) } + +#[cfg(test)] +mod tests { + + use super::*; + use crate::{ + domain::types::{AttributeName, AttributeType}, + infra::{ + access_control::{Permission, ValidationResults}, + graphql::query::Query, + test_utils::MockTestBackendHandler, + }, + }; + use juniper::{ + execute, graphql_value, DefaultScalarValue, EmptySubscription, GraphQLType, InputValue, + RootNode, Variables, + }; + use mockall::predicate::eq; + use pretty_assertions::assert_eq; + + fn mutation_schema<'q, C, Q, M>( + query_root: Q, + mutation_root: M, + ) -> RootNode<'q, Q, M, EmptySubscription> + where + Q: GraphQLType + 'q, + M: GraphQLType + 'q, + { + RootNode::new(query_root, mutation_root, EmptySubscription::::new()) + } + + #[tokio::test] + async fn test_create_user_attribute_valid() { + const QUERY: &str = r#" + mutation CreateUserAttribute($name: String!, $attributeType: AttributeType!, $isList: Boolean!, $isVisible: Boolean!, $isEditable: Boolean!) { + addUserAttribute(name: $name, attributeType: $attributeType, isList: $isList, isVisible: $isVisible, isEditable: $isEditable) { + ok + } + } + "#; + let mut mock = MockTestBackendHandler::new(); + mock.expect_add_user_attribute() + .with(eq(CreateAttributeRequest { + name: AttributeName::new("AttrName0"), + attribute_type: AttributeType::String, + is_list: false, + is_visible: false, + is_editable: false, + })) + .return_once(|_| Ok(())); + let context = Context::::new_for_tests( + mock, + ValidationResults { + user: UserId::new("bob"), + permission: Permission::Admin, + }, + ); + let vars = Variables::from([ + ("name".to_string(), InputValue::scalar("AttrName0")), + ( + "attributeType".to_string(), + InputValue::enum_value("STRING"), + ), + ("isList".to_string(), InputValue::scalar(false)), + ("isVisible".to_string(), InputValue::scalar(false)), + ("isEditable".to_string(), InputValue::scalar(false)), + ]); + let schema = mutation_schema( + Query::::new(), + Mutation::::new(), + ); + assert_eq!( + execute(QUERY, None, &schema, &vars, &context).await, + Ok(( + graphql_value!( + { + "addUserAttribute": { + "ok": true + } + } ), + vec![] + )) + ); + } + + #[tokio::test] + async fn test_create_user_attribute_invalid() { + const QUERY: &str = r#" + mutation CreateUserAttribute($name: String!, $attributeType: AttributeType!, $isList: Boolean!, $isVisible: Boolean!, $isEditable: Boolean!) { + addUserAttribute(name: $name, attributeType: $attributeType, isList: $isList, isVisible: $isVisible, isEditable: $isEditable) { + ok + } + } + "#; + let mock = MockTestBackendHandler::new(); + let context = Context::::new_for_tests( + mock, + ValidationResults { + user: UserId::new("bob"), + permission: Permission::Admin, + }, + ); + let vars = Variables::from([ + ("name".to_string(), InputValue::scalar("AttrName_0")), + ( + "attributeType".to_string(), + InputValue::enum_value("STRING"), + ), + ("isList".to_string(), InputValue::scalar(false)), + ("isVisible".to_string(), InputValue::scalar(false)), + ("isEditable".to_string(), InputValue::scalar(false)), + ]); + let schema = mutation_schema( + Query::::new(), + Mutation::::new(), + ); + let result = execute(QUERY, None, &schema, &vars, &context).await; + match result { + Ok(res) => { + let (response, errors) = res; + assert!(response.is_null()); + let expected_error_msg = + "Cannot create attribute with invalid name. Valid characters: a-z, A-Z, 0-9, and dash (-). Invalid chars found: _" + .to_string(); + assert!(errors + .iter() + .all(|e| e.error().message() == expected_error_msg)); + } + Err(_) => { + panic!(); + } + } + } + + #[tokio::test] + async fn test_create_group_attribute_valid() { + const QUERY: &str = r#" + mutation CreateGroupAttribute($name: String!, $attributeType: AttributeType!, $isList: Boolean!, $isVisible: Boolean!) { + addGroupAttribute(name: $name, attributeType: $attributeType, isList: $isList, isVisible: $isVisible, isEditable: false) { + ok + } + } + "#; + let mut mock = MockTestBackendHandler::new(); + mock.expect_add_group_attribute() + .with(eq(CreateAttributeRequest { + name: AttributeName::new("AttrName0"), + attribute_type: AttributeType::String, + is_list: false, + is_visible: false, + is_editable: false, + })) + .return_once(|_| Ok(())); + let context = Context::::new_for_tests( + mock, + ValidationResults { + user: UserId::new("bob"), + permission: Permission::Admin, + }, + ); + let vars = Variables::from([ + ("name".to_string(), InputValue::scalar("AttrName0")), + ( + "attributeType".to_string(), + InputValue::enum_value("STRING"), + ), + ("isList".to_string(), InputValue::scalar(false)), + ("isVisible".to_string(), InputValue::scalar(false)), + ("isEditable".to_string(), InputValue::scalar(false)), + ]); + let schema = mutation_schema( + Query::::new(), + Mutation::::new(), + ); + assert_eq!( + execute(QUERY, None, &schema, &vars, &context).await, + Ok(( + graphql_value!( + { + "addGroupAttribute": { + "ok": true + } + } ), + vec![] + )) + ); + } + + #[tokio::test] + async fn test_create_group_attribute_invalid() { + const QUERY: &str = r#" + mutation CreateUserAttribute($name: String!, $attributeType: AttributeType!, $isList: Boolean!, $isVisible: Boolean!, $isEditable: Boolean!) { + addUserAttribute(name: $name, attributeType: $attributeType, isList: $isList, isVisible: $isVisible, isEditable: $isEditable) { + ok + } + } + "#; + let mock = MockTestBackendHandler::new(); + let context = Context::::new_for_tests( + mock, + ValidationResults { + user: UserId::new("bob"), + permission: Permission::Admin, + }, + ); + let vars = Variables::from([ + ("name".to_string(), InputValue::scalar("AttrName_0")), + ( + "attributeType".to_string(), + InputValue::enum_value("STRING"), + ), + ("isList".to_string(), InputValue::scalar(false)), + ("isVisible".to_string(), InputValue::scalar(false)), + ("isEditable".to_string(), InputValue::scalar(false)), + ]); + let schema = mutation_schema( + Query::::new(), + Mutation::::new(), + ); + let result = execute(QUERY, None, &schema, &vars, &context).await; + match result { + Ok(res) => { + let (response, errors) = res; + assert!(response.is_null()); + let expected_error_msg = + "Cannot create attribute with invalid name. Valid characters: a-z, A-Z, 0-9, and dash (-). Invalid chars found: _" + .to_string(); + assert!(errors + .iter() + .all(|e| e.error().message() == expected_error_msg)); + } + Err(_) => { + panic!(); + } + } + } +} diff --git a/validation/Cargo.toml b/validation/Cargo.toml new file mode 100644 index 0000000..cffcd35 --- /dev/null +++ b/validation/Cargo.toml @@ -0,0 +1,9 @@ +[package] +authors = ["Simon Broeng Jensen "] +description = "Validation logic for LLDAP" +edition = "2021" +homepage = "https://github.com/lldap/lldap" +license = "GPL-3.0-only" +name = "lldap_validation" +repository = "https://github.com/lldap/lldap" +version = "0.6.0" diff --git a/validation/src/attributes.rs b/validation/src/attributes.rs new file mode 100644 index 0000000..c76999d --- /dev/null +++ b/validation/src/attributes.rs @@ -0,0 +1,45 @@ +// Description of allowed characters. Intended for error messages. +pub const ALLOWED_CHARACTERS_DESCRIPTION: &str = "a-z, A-Z, 0-9, and dash (-)"; + +pub fn validate_attribute_name(attribute_name: &str) -> Result<(), Vec> { + let invalid_chars: Vec = attribute_name + .chars() + .filter(|c| !(c.is_alphanumeric() || *c == '-')) + .collect(); + if invalid_chars.is_empty() { + Ok(()) + } else { + Err(invalid_chars) + } +} + +mod tests { + + #[test] + fn test_valid_attribute_name() { + let valid1: String = "AttrName-01".to_string(); + let result = super::validate_attribute_name(&valid1); + assert!(result == Ok(())); + } + + #[test] + fn test_invalid_attribute_name_chars() { + fn test_invalid_char(c: char) { + let prefix: String = "AttrName".to_string(); + let suffix: String = "01".to_string(); + let name: String = format!("{prefix}{c}{suffix}"); + let result = super::validate_attribute_name(&name); + match result { + Ok(()) => { + panic!() + } + Err(invalid) => { + assert!(invalid == vec![c.to_owned()]); + } + } + } + test_invalid_char(' '); + test_invalid_char('_'); + test_invalid_char('#'); + } +} diff --git a/validation/src/lib.rs b/validation/src/lib.rs new file mode 100644 index 0000000..21e28e1 --- /dev/null +++ b/validation/src/lib.rs @@ -0,0 +1,3 @@ +#![forbid(non_ascii_idents)] + +pub mod attributes;