Allow list options multiple times

example.ini

@@ -4,14 +4,17 @@ username_http_header = "X-Ldap-Authz-Username"
 
 ; Example LDAP server configuration. This is for Active Directory,
 ; and makes a recursive membership query to given group.
-;ldap_server_url = ldap://127.0.0.1:3890
 ldap_server_url = ldap://dc1.example.test:389
 ldap_conn_timeout = 10.0
 ldap_bind_dn = "CN=service,CN=Users,DC=example,DC=test"
 ldap_bind_password = "password123"
 ldap_search_base = "DC=example,DC=test"
 ldap_query = "(&(objectCategory=Person)(sAMAccountName=%USERNAME%)(memberOf:1.2.840.113556.1.4.1941:=CN=%MY_CUSTOM_VAR%,CN=Users,DC=example,DC=test))"
-ldap_attribs = "displayName, givenName, sn, mail"
+
+; Attributes to fetch from LDAP.
+; (You can specify this multiple times, or use a comma-separated list:)
+ldap_attribs = "givenName, sn, mail"
+ldap_attribs = "displayName"
 
 ; Cache size (these are defaults)
 cache_time = 30
@@ -28,12 +31,13 @@ http_path = "/users$"
 ; Ldap query references variable MY_CUSTOM_VAR above. Set it for this query:
 query_vars = "MY_CUSTOM_VAR = ACL_Users"
 ; Fetch additional attributes from LDAP my performing additional queries
-; if this one succeeds. See below for their definitions.
-sub_queries = "is_beta_tester, is_bug_reporter, is_peer_support"
-
+; if this one succeeds. See below for section definitions.
+sub_queries = "is_beta_tester, is_bug_reporter"
+sub_queries = "is_peer_support"
 
 [admins]
 http_path = "/admins$"
+http_path = "/admins$"
 query_vars = "MY_CUSTOM_VAR = ACL_Admins"
 ; Fictional example: instruct backend app to show debug info for admins
 set_attribs_on_success = "extraGroups = show_debug_info"
@@ -53,6 +57,7 @@ set_attribs_on_success = "extraGroups = beta_tester"
 
 [is_bug_reporter]
 query_vars = "MY_CUSTOM_VAR = Role_Bug_Reporters"
-set_attribs_on_success = "extraGroups = bug_reporter, extraGroups = show_debug_info"
+set_attribs_on_success = "extraGroups = bug_reporter"
+set_attribs_on_success = "extraGroups = show_debug_info"
 ; Circular references are pruned, so this nonsense won't crash - it's just useless:
 sub_queries = "is_bug_reporter, users"

run-tests.sh

@@ -84,6 +84,9 @@ function do_tests() {
     test "user-page"  "charlie:BADPASSWORD" "401 c eg:"
 
     test "bad-page"   "alice:alice123" "404 c eg:"
+
+    # Test username quoting with malicious characters, should give 401, not 500
+    test "user-page"  ")=&%)):password" "401 c eg:"
 
     echo "(Repeat and check that query came from cache)"
     test "user-page"  "alice:alice123" "200Alice Alison c1 eg:beta_tester"

src/config.rs

@@ -13,14 +13,25 @@ use crate::SubQueryJoin;
 
 
 macro_rules! config_options {
-    ($($name:ident: $type:ty = $default:expr ; $help:expr),*) => {
+    ($($name:ident $([$multi:tt])? : $type:ty = $default:expr ; $help:expr),*) => {
+
         #[derive(Debug, Clone)]
         pub(crate) struct ConfigSection {
             $(
                 pub(crate) $name: $type,
             )*
         }
 
+        fn is_multiline(opt: &str) -> bool {
+            match opt {
+                $(
+                    $( stringify!($name) => { assert!(stringify!($multi) == "MULTILINE"); true } )?
+                )*
+                _ => false,
+            }
+        }
+
+
         const CONFIG_OPTIONS: &[(&str, &str, Option<&str>)] = &[
             $(
                 (stringify!($name), $help, $default),
@@ -31,26 +42,25 @@ macro_rules! config_options {
 Configuration file in in INI format:
 
     [default]
-    ; Default values for all sections (can be overridden in the sections)
-    key = value1
-    key = value2
+    ; Default values for all other sections
+    option1 = value1
+    option2 = value2
     ...
 
 
     [section1]
-    ; (same keys as in [default], if values differ)
+    ; (any options that differ from [default])
 
     [section2]
-    ; (same keys as in [default], if values differ)
+    ; (any options that differ from [default])
 
     ...
 
-You can define defaults for other sections in the [default] section.
-It's not used for anything else.
-
-Every section must have a unique name, and the name 'default' is reserved.
+Every section must have a unique name.
 
-Descriptions of the config keys:
+Descriptions of possible config options follows. Options marked with '(+)'
+are comma-separated lists - if specified multiple times, their values are
+concatenated.
 
 "##;
 
@@ -63,7 +73,12 @@ Descriptions of the config keys:
             }
             CONFIG_HELP_INTRO.to_string() + &CONFIG_OPTIONS.iter()
                 .filter(|(key, _, _)| *key != "section")
-                .map(|(key, help, def)| format!("  {}  {}\n\n    {}\n\n\n", key, fmt_def(def), help.replace("\n", "\n    ") ))
+                .map(|(key, help, def)| format!("  {}{}  {}\n\n    {}\n\n\n",
+                    key,
+                    if is_multiline(key) { " (+)" } else { "" },
+                    fmt_def(def),
+                    help.replace("\n", "\n    ")
+                ))
                 .collect::<String>()
         }
 
@@ -97,9 +112,9 @@ config_options! {
     ldap_search_base: String = None; "LDAP base DN to search in (e.g. 'OU=users,DC=example,DC=com')",
     ldap_scope: ldap3::Scope = Some("subtree"); "LDAP search scope. Must be 'subtree', 'onelevel' or 'base')",
     ldap_query: String = None; "LDAP query to use. May contain '%USERNAME%', which will be quoted and replaced.\nExample: '(&(objectClass=person)(sAMAccountName=%USERNAME%))",
-    ldap_attribs: Vec<String> = Some("CN"); "LDAP attributes to return (e.g. 'displayName, givenName, sn, mail'). Must not be empty.",
+    ldap_attribs [MULTILINE]: Vec<String> = Some("CN"); "LDAP attributes to return (e.g. 'displayName, givenName, sn, mail'). Must not be empty.",
 
-    query_vars: HashMap<String, String> = Some(""); concat!(
+    query_vars [MULTILINE]: HashMap<String, String> = Some(""); concat!(
         "Extra variables to use in the query, in addition to %USERNAME%.\n",
         "You can use these to avoid repeating long query strings in different sections.\n",
         "\n",
@@ -117,11 +132,11 @@ config_options! {
     attrib_delimiter: String = Some(";"); "Delimiter to use when concatenating multiple values of an attribute",
     deduplicate_attribs: bool = Some("true"); "Whether to deduplicate attribute values.\nExample: 'someAttr=foo,bar,foo,foo' becomes 'someAttr=foo,bar')",
 
-    set_attribs_on_success: Vec<(String, Vec<String>)> = Some(""); concat!(
+    set_attribs_on_success [MULTILINE] : Vec<(String, Vec<String>)> = Some(""); concat!(
         "Attributes to set manually if the main query succeeds.\n",
         "If empty, only the attributes returned by LDAP queries are set.\n",
         "Format: 'attribute=value1, attribute=value2, attribute2= ...'"),
-    sub_queries: Vec<String> = Some(""); concat!(
+    sub_queries [MULTILINE]: Vec<String> = Some(""); concat!(
         "Section names of optional sub-queries.'.\n",
         "\n",
         "Sub-queries can check for additional conditions and/or set additional attributes.\n",
@@ -190,6 +205,13 @@ pub(crate) fn parse_config(config_file: &str) -> Result<Vec<ConfigSection>, Erro
             bail!("Unknown key(s) in section [{}]: {}", &section_name, unknown_keys.join(", "));
         }
 
+        // Only allow certain keys to appear multiple times
+        for (key, _) in sect_props.iter() {
+            if sect_props.get_all(key).count() > 1 && !is_multiline(key) {
+                bail!("Key '{}' (in section [{}]) is not a list, and therefore cannot appear mutiple times.", key, section_name);
+            }
+        }
+
         if section_name == "default" {
             continue;
         }
@@ -213,8 +235,13 @@ pub(crate) fn parse_config(config_file: &str) -> Result<Vec<ConfigSection>, Erro
             bail!("Config option(s) not set in section [{}]: {}", section_name, missing_keys.join(", "));
         }
 
-        let get = |key: &str| sect_props.get(key)
-            .unwrap_or_else(|| panic!("BUG: missing key '{key}' after checking that it's not missing?!"))
+        // Helper function to get a value from the section. Combines multiple values with a comma.
+        let get = |key: &str| sect_props.get_all(key)
+            .map(|v| v.trim())
+            .filter(|v| !v.is_empty())
+            .collect::<Vec<_>>()            
+            .join(", ")
+            .trim()
             .to_string();
 
         // Compile regex
@@ -226,6 +253,8 @@ pub(crate) fn parse_config(config_file: &str) -> Result<Vec<ConfigSection>, Erro
             anyhow!("Invalid value for option '{key}' in section [{section_name}]: {}.\n -- {}", get(key), help_for_key(key))
         };
 
+        /// Parse a comma-separated list of assignments.
+        /// E.g. "key1=value1, key2=value2, key3=value3"
         fn split_assignments(s: &str) -> Result<Vec<(String, String)>, Error> {
             let mut res = Vec::new();
             for assignment in s.split(',')

src/main.rs

@@ -170,7 +170,7 @@ async fn ldap_query(
                 }
             };
         };
-        panic!("BUG: ldap_query() retry loop should never have reached this point");
+        unreachable!();
     }.await;
 
     let result_entries = match res {