fix: Non-letter payment method identifiers are accepted

.changelog/reject-invalid-method-identifiers.md

@@ -0,0 +1,7 @@
+---
+"mpp": patch
+---
+
+Reject payment challenges whose method identifier contains characters other
+than lowercase ASCII letters. Reject payment challenges reached through a
+cross-origin redirect before a credential can be created or sent.

src/client/error.rs

@@ -21,6 +21,9 @@ pub enum HttpError {
     /// Request could not be cloned (required for retry)
     CloneFailed,
 
+    /// A redirect changed the request origin before returning a payment challenge
+    CrossOriginRedirect,
+
     /// Payment provider error
     Payment(MppError),
 
@@ -41,6 +44,9 @@ impl fmt::Display for HttpError {
             Self::InvalidChallenge(msg) => write!(f, "invalid challenge: {}", msg),
             Self::InvalidCredential(msg) => write!(f, "invalid credential: {}", msg),
             Self::CloneFailed => write!(f, "request could not be cloned for retry"),
+            Self::CrossOriginRedirect => {
+                write!(f, "Refusing to send payment credential across redirect")
+            }
             Self::Payment(e) => write!(f, "payment failed: {}", e),
             #[cfg(feature = "client")]
             Self::Request(e) => write!(f, "HTTP request failed: {}", e),

src/client/fetch.rs

@@ -253,6 +253,17 @@ async fn send_with_payment<P: PaymentProvider>(
             return Ok(resp);
         }
 
+        if url
+            .as_ref()
+            .is_some_and(|request_url| request_url.origin() != resp.url().origin())
+        {
+            pending_payments
+                .rollback()
+                .await
+                .map_err(HttpError::Payment)?;
+            return Err(HttpError::CrossOriginRedirect);
+        }
+
         let www_auth_values: Vec<&str> = resp
             .headers()
             .get_all(WWW_AUTHENTICATE)
@@ -794,6 +805,60 @@ mod tests {
             assert_eq!(call_count.load(Ordering::SeqCst), 2); // initial 402 + retry
         }
 
+        #[tokio::test]
+        async fn cross_origin_redirect_before_402_is_rejected() {
+            let (_, www_auth) = test_challenge();
+            let authorization_observed = Arc::new(AtomicU32::new(0));
+            let observed = authorization_observed.clone();
+            let target = Router::new().route(
+                "/paid",
+                get(move |req: axum::http::Request<axum::body::Body>| {
+                    let www_auth = www_auth.clone();
+                    let observed = observed.clone();
+                    async move {
+                        if req.headers().contains_key("authorization") {
+                            observed.fetch_add(1, Ordering::SeqCst);
+                        }
+                        (
+                            AxumStatusCode::PAYMENT_REQUIRED,
+                            [(WWW_AUTH_NAME, www_auth)],
+                            "pay up",
+                        )
+                    }
+                }),
+            );
+            let target_url = spawn_server(target).await;
+            let source = Router::new().route(
+                "/paid",
+                get(move || {
+                    let target_url = target_url.clone();
+                    async move {
+                        (
+                            AxumStatusCode::TEMPORARY_REDIRECT,
+                            [(axum::http::header::LOCATION, format!("{target_url}/paid"))],
+                            "redirect",
+                        )
+                    }
+                }),
+            );
+            let source_url = spawn_server(source).await;
+            let provider = MockProvider::new();
+
+            let err = reqwest::Client::new()
+                .get(format!("{source_url}/paid"))
+                .send_with_payment(&provider)
+                .await
+                .unwrap_err();
+
+            assert!(matches!(err, HttpError::CrossOriginRedirect));
+            assert_eq!(
+                err.to_string(),
+                "Refusing to send payment credential across redirect"
+            );
+            assert_eq!(provider.call_count(), 0);
+            assert_eq!(authorization_observed.load(Ordering::SeqCst), 0);
+        }
+
         #[tokio::test]
         async fn dropped_paid_request_abandons_transient_provider_state() {
             let (_, www_auth) = test_challenge();

src/protocol/core/headers.rs

@@ -252,7 +252,7 @@ pub fn parse_www_authenticate(header: &str) -> Result<PaymentChallenge> {
     }
     let realm = require_param!(params, "realm").clone();
     let method_raw = require_param!(params, "method").clone();
-    if method_raw.is_empty() || !method_raw.chars().all(|c| c.is_ascii_lowercase()) {
+    if method_raw.is_empty() || !method_raw.bytes().all(|c| c.is_ascii_lowercase()) {
         return Err(MppError::invalid_challenge_reason(format!(
             "Invalid method: \"{}\". Must match method-name ABNF.",
             method_raw
@@ -1022,6 +1022,17 @@ mod tests {
         assert!(err.to_string().contains("Invalid method"));
     }
 
+    #[test]
+    fn test_parse_www_authenticate_rejects_non_letter_method_names() {
+        for method in ["123", "*", "tempo!"] {
+            let header = format!(
+                r#"Payment id="abc", realm="api", method="{method}", intent="charge", request="e30""#
+            );
+            let err = parse_www_authenticate(&header).unwrap_err();
+            assert!(err.to_string().contains("Invalid method"));
+        }
+    }
+
     #[test]
     fn test_parse_www_authenticate_rejects_mixed_case_method_name() {
         let header =