feat(downloader): Avoid downloading login pages

crates/symbolicator-service/src/download/http.rs

@@ -119,4 +119,38 @@ mod tests {
 
         assert_eq!(download_status, Err(CacheError::NotFound));
     }
+
+    #[tokio::test]
+    async fn test_download_azure_file() {
+        test::setup();
+
+        let tmpfile = tempfile::NamedTempFile::new().unwrap();
+        let dest = tmpfile.path();
+
+        // Create HttpSourceConfig directly
+        let azure_source = std::sync::Arc::new(symbolicator_sources::HttpSourceConfig {
+            id: symbolicator_sources::SourceId::new("azure"),
+            url: "https://dev.azure.com/".parse().unwrap(),
+            headers: Default::default(),
+            files: Default::default(),
+            accept_invalid_certs: false,
+        });
+
+        let loc = SourceLocation::new("foo/bar.cs");
+        let file_source = HttpRemoteFile::new(azure_source, loc);
+
+        let restricted_client = crate::utils::http::create_client(&Default::default(), true, false);
+        let no_ssl_client = crate::utils::http::create_client(&Default::default(), true, true);
+
+        let downloader = HttpDownloader::new(restricted_client, no_ssl_client, Default::default());
+        let mut destination = tokio::fs::File::create(&dest).await.unwrap();
+        let download_status = downloader
+            .download_source("", &file_source, &mut destination)
+            .await;
+
+        assert_eq!(
+            download_status,
+            Err(CacheError::DownloadError("302 Found".into()))
+        );
+    }
 }

crates/symbolicator-service/src/utils/http.rs

@@ -4,7 +4,7 @@ use std::time::Duration;
 
 use ipnetwork::Ipv4Network;
 use once_cell::sync::Lazy;
-use reqwest::Url;
+use reqwest::{redirect, StatusCode, Url};
 
 use crate::config::Config;
 
@@ -106,7 +106,27 @@ pub fn create_client(
         .connect_timeout(timeouts.connect)
         .timeout(timeouts.max_download)
         .pool_idle_timeout(Duration::from_secs(30))
-        .danger_accept_invalid_certs(accept_invalid_certs);
+        .danger_accept_invalid_certs(accept_invalid_certs)
+        .redirect(redirect::Policy::custom(|attempt: redirect::Attempt| {
+            // The default redirect policy allows to follow up to 10 redirects. This is problematic
+            // when symbolicator tries to fetch native source files from a web source, as a redirect
+            // might land us on a login page, which is then used for source context.
+            // To avoid this, symbolicator's redirect policy is to not follow temporary redirects
+            // on hosts that are known to redirect to login pages.
+
+            if attempt.status() == StatusCode::FOUND {
+                let is_from_azure = attempt
+                    .previous()
+                    .last()
+                    .and_then(|url| url.host_str())
+                    .map_or(false, |host| host == "dev.azure.com");
+
+                if is_from_azure {
+                    return attempt.stop();
+                }
+            }
+            redirect::Policy::default().redirect(attempt)
+        }));
 
     if !connect_to_reserved_ips {
         builder = builder.ip_filter(is_external_ip);
@@ -273,6 +293,23 @@ mod tests {
         assert_eq!(text, "OK");
     }
 
+    #[tokio::test]
+    async fn test_client_redirect_policy() {
+        let client = create_client(&Default::default(), false, false);
+
+        let response = client
+            .get("https://dev.azure.com/foo/bar.cs")
+            .send()
+            .await
+            .unwrap();
+
+        let status = response.status();
+
+        assert_eq!(status.as_u16(), 302);
+        assert!(status.is_redirection());
+        assert!(!status.is_success());
+    }
+
     fn is_valid_origin(origin: &str, allowed: &[&str]) -> bool {
         let allowed: Vec<_> = allowed.iter().map(|s| s.to_string()).collect();
         super::is_valid_origin(&origin.parse().unwrap(), &allowed)