Fix partner registration validation and index consistency

Author: ChristianPavilonis

crates/trusted-server-core/src/ec/admin.rs

@@ -7,6 +7,8 @@
 use error_stack::{Report, ResultExt};
 use fastly::{Request, Response};
 use serde::{Deserialize, Serialize};
+use std::collections::HashSet;
+use url::Host;
 
 use crate::error::TrustedServerError;
 
@@ -64,6 +66,64 @@ fn default_pull_sync_rate_limit() -> u32 {
     10
 }
 
+fn bad_request(message: impl Into<String>) -> Report<TrustedServerError> {
+    Report::new(TrustedServerError::BadRequest {
+        message: message.into(),
+    })
+}
+
+fn normalize_required_text(
+    value: &str,
+    field_name: &str,
+) -> Result<String, Report<TrustedServerError>> {
+    let trimmed = value.trim();
+    if trimmed.is_empty() {
+        return Err(bad_request(format!("{field_name} is required")));
+    }
+    Ok(trimmed.to_owned())
+}
+
+fn normalize_hostname(value: &str, field_name: &str) -> Result<String, Report<TrustedServerError>> {
+    let trimmed = value.trim().trim_end_matches('.');
+    if trimmed.is_empty() {
+        return Err(bad_request(format!("{field_name} is required")));
+    }
+
+    let normalized = trimmed.to_ascii_lowercase();
+    Host::parse(&normalized)
+        .map_err(|_| bad_request(format!("{field_name} must be a valid hostname")))?;
+
+    Ok(normalized)
+}
+
+fn normalize_hostname_list(
+    values: Vec<String>,
+    field_name: &str,
+) -> Result<Vec<String>, Report<TrustedServerError>> {
+    let mut normalized_values = Vec::with_capacity(values.len());
+    let mut seen = HashSet::with_capacity(values.len());
+
+    for value in values {
+        let trimmed = value.trim().trim_end_matches('.');
+        if trimmed.is_empty() {
+            return Err(bad_request(format!(
+                "{field_name} entries must not be empty"
+            )));
+        }
+
+        let normalized = trimmed.to_ascii_lowercase();
+        Host::parse(&normalized).map_err(|_| {
+            bad_request(format!("{field_name} contains invalid hostname '{value}'"))
+        })?;
+
+        if seen.insert(normalized.clone()) {
+            normalized_values.push(normalized);
+        }
+    }
+
+    Ok(normalized_values)
+}
+
 /// Response body for `POST /admin/partners/register`.
 ///
 /// Echoes key fields without exposing sensitive data (`api_key_hash`,
@@ -97,54 +157,65 @@ pub fn handle_register_partner(
             message: "Invalid JSON in request body".to_owned(),
         })?;
 
+    let RegisterPartnerRequest {
+        id,
+        name,
+        allowed_return_domains,
+        api_key,
+        bidstream_enabled,
+        source_domain,
+        openrtb_atype,
+        sync_rate_limit,
+        batch_rate_limit,
+        pull_sync_enabled,
+        pull_sync_url,
+        pull_sync_allowed_domains,
+        pull_sync_ttl_sec,
+        pull_sync_rate_limit,
+        ts_pull_token,
+    } = request;
+
     // Validate partner ID.
-    validate_partner_id(&request.id)
-        .map_err(|msg| Report::new(TrustedServerError::BadRequest { message: msg }))?;
-
-    // Validate required fields.
-    if request.name.is_empty() {
-        return Err(Report::new(TrustedServerError::BadRequest {
-            message: "name is required".to_owned(),
-        }));
-    }
-    if request.api_key.is_empty() {
-        return Err(Report::new(TrustedServerError::BadRequest {
-            message: "api_key is required".to_owned(),
-        }));
-    }
-    if request.source_domain.is_empty() {
-        return Err(Report::new(TrustedServerError::BadRequest {
-            message: "source_domain is required".to_owned(),
-        }));
+    validate_partner_id(&id).map_err(bad_request)?;
+
+    // Validate and normalize required fields.
+    let name = normalize_required_text(&name, "name")?;
+    if api_key.trim().is_empty() {
+        return Err(bad_request("api_key is required"));
     }
-    if request.allowed_return_domains.is_empty() {
-        return Err(Report::new(TrustedServerError::BadRequest {
-            message: "allowed_return_domains must have at least one entry".to_owned(),
-        }));
+    let source_domain = normalize_hostname(&source_domain, "source_domain")?;
+
+    if allowed_return_domains.is_empty() {
+        return Err(bad_request(
+            "allowed_return_domains must have at least one entry",
+        ));
     }
+    let allowed_return_domains =
+        normalize_hostname_list(allowed_return_domains, "allowed_return_domains")?;
+    let pull_sync_allowed_domains =
+        normalize_hostname_list(pull_sync_allowed_domains, "pull_sync_allowed_domains")?;
 
     // Build the PartnerRecord with hashed API key.
     let record = PartnerRecord {
-        id: request.id.clone(),
-        name: request.name.clone(),
-        allowed_return_domains: request.allowed_return_domains,
-        api_key_hash: hash_api_key(&request.api_key),
-        bidstream_enabled: request.bidstream_enabled,
-        source_domain: request.source_domain,
-        openrtb_atype: request.openrtb_atype,
-        sync_rate_limit: request.sync_rate_limit,
-        batch_rate_limit: request.batch_rate_limit,
-        pull_sync_enabled: request.pull_sync_enabled,
-        pull_sync_url: request.pull_sync_url,
-        pull_sync_allowed_domains: request.pull_sync_allowed_domains,
-        pull_sync_ttl_sec: request.pull_sync_ttl_sec,
-        pull_sync_rate_limit: request.pull_sync_rate_limit,
-        ts_pull_token: request.ts_pull_token,
+        id,
+        name,
+        allowed_return_domains,
+        api_key_hash: hash_api_key(&api_key),
+        bidstream_enabled,
+        source_domain,
+        openrtb_atype,
+        sync_rate_limit,
+        batch_rate_limit,
+        pull_sync_enabled,
+        pull_sync_url,
+        pull_sync_allowed_domains,
+        pull_sync_ttl_sec,
+        pull_sync_rate_limit,
+        ts_pull_token,
     };
 
     // Validate pull sync configuration.
-    validate_pull_sync_config(&record)
-        .map_err(|msg| Report::new(TrustedServerError::BadRequest { message: msg }))?;
+    validate_pull_sync_config(&record).map_err(bad_request)?;
 
     // Persist to KV store.
     let created = partner_store.upsert(&record)?;
@@ -258,4 +329,52 @@ mod tests {
             Some("https://sync.example-ssp.com/pull")
         );
     }
+
+    #[test]
+    fn normalize_required_text_rejects_whitespace_only() {
+        let err = normalize_required_text("   ", "name")
+            .expect_err("should reject whitespace-only required field");
+        assert!(
+            err.to_string().contains("name is required"),
+            "should mention required field"
+        );
+    }
+
+    #[test]
+    fn normalize_hostname_normalizes_case_and_trailing_dot() {
+        let normalized = normalize_hostname("  Sync.Example.COM.  ", "source_domain")
+            .expect("should parse host");
+        assert_eq!(normalized, "sync.example.com");
+    }
+
+    #[test]
+    fn normalize_hostname_list_rejects_empty_entry() {
+        let err = normalize_hostname_list(
+            vec!["sync.example.com".to_owned(), "   ".to_owned()],
+            "allowed_return_domains",
+        )
+        .expect_err("should reject empty domain entries");
+        assert!(
+            err.to_string()
+                .contains("allowed_return_domains entries must not be empty"),
+            "should surface empty-entry error"
+        );
+    }
+
+    #[test]
+    fn normalize_hostname_list_deduplicates_normalized_values() {
+        let normalized = normalize_hostname_list(
+            vec![
+                "Sync.Example.com".to_owned(),
+                "sync.example.com.".to_owned(),
+                "cdn.example.com".to_owned(),
+            ],
+            "allowed_return_domains",
+        )
+        .expect("should normalize hostnames");
+        assert_eq!(
+            normalized,
+            vec!["sync.example.com".to_owned(), "cdn.example.com".to_owned()]
+        );
+    }
 }