Require a User-Agent header, take 2

We deployed this back in October, and quickly realized that old versions
of Cargo (Rust 1.15 and earlier) didn't set a User-Agent header.
Notably, the version of Cargo used for bootstrapping the compiler back
then was this old, so we accidentally broke the build for rustc.

We still see build traffic for these old versions, and we likely will
never be willing to break `cargo build` for old versions of Cargo.
However, as we discussed back in early November, we're fine with doing
this for all other endpoints.

This change will break versions of Cargo prior to Rust 1.16 for all
operations that talk to crates.io, except for `cargo build`. We no
longer receive any traffic for publish, yank, unyank, or owners from
these old versions. (`cargo search` hits an endpoint that is also hit by
bots, so I can't say for sure if it's coming from cargo or random
crawlers, which is kinda the point of this requirement in the first
place)

Author: sgrif

src/middleware.rs

@@ -93,10 +93,8 @@ pub fn build_middleware(app: Arc<App>, endpoints: R404) -> MiddlewareBuilder {
         let ips = ip_list.split(',').map(String::from).collect();
         m.around(block_ips::BlockIps::new(ips));
     }
-    // Note: Temporarily disabled because cargo-vendor doesn't include a
-    // User-Agent header and Rust's CI broke. If this is still commented out
-    // by Nov 7, 2018 ping the crates.io team.
-    // m.around(require_user_agent::RequireUserAgent::default());
+
+    m.around(require_user_agent::RequireUserAgent::default());
 
     if env != Env::Test {
         m.around(log_request::LogRequests::default());

src/middleware/require_user_agent.rs

@@ -22,7 +22,8 @@ impl AroundMiddleware for RequireUserAgent {
 impl Handler for RequireUserAgent {
     fn call(&self, req: &mut dyn Request) -> Result<Response, Box<dyn Error + Send>> {
         let has_user_agent = request_header(req, "User-Agent") != "";
-        if !has_user_agent {
+        let is_download = req.path().ends_with("download");
+        if !has_user_agent && !is_download {
             let body = format!(
                 include_str!("no_user_agent_message.txt"),
                 request_header(req, "X-Request-Id"),

src/tests/server.rs

@@ -1,18 +1,28 @@
-// Note: Temporarily disabled because cargo-vendor doesn't include a
-// User-Agent header and Rust's CI broke. If this is still commented out
-// by Nov 7, 2018 ping the crates.io team.
+use conduit::Method;
 
-// use conduit::{Handler, Method};
-//
-// use {app, req};
-//
-//
-// #[test]
-// fn user_agent_is_required() {
-//     let (_b, _app, middle) = app();
-//
-//     let mut req = req(Method::Get, "/api/v1/crates");
-//     req.header("User-Agent", "");
-//     let resp = t!(middle.call(&mut req));
-//     assert_eq!(resp.status.0, 403);
-// }
+use crate::builders::*;
+use crate::util::*;
+
+#[test]
+fn user_agent_is_required() {
+    let (_app, anon) = TestApp::init().empty();
+
+    let mut req = anon.request_builder(Method::Get, "/api/v1/crates");
+    req.header("User-Agent", "");
+    let resp = anon.run::<()>(req);
+    resp.assert_status(403);
+}
+
+#[test]
+fn user_agent_is_not_required_for_download() {
+    let (app, anon, user) = TestApp::init().with_user();
+
+    app.db(|conn| {
+        CrateBuilder::new("dl_no_ua", user.as_model().id).expect_build(conn);
+    });
+
+    let mut req = anon.request_builder(Method::Get, "/api/v1/crates/dl_no_ua/0.99.0/download");
+    req.header("User-Agent", "");
+    let resp = anon.run::<()>(req);
+    resp.assert_status(302);
+}

src/tests/util.rs

@@ -145,13 +145,21 @@ pub trait RequestHelper {
     fn request_builder(&self, method: Method, path: &str) -> MockRequest;
     fn app(&self) -> &TestApp;
 
+    /// Run a request
+    fn run<T>(&self, mut request: MockRequest) -> Response<T>
+    where
+        T: serde::de::DeserializeOwned,
+    {
+        Response::new(self.app().0.middle.call(&mut request))
+    }
+
     /// Issue a GET request
     fn get<T>(&self, path: &str) -> Response<T>
     where
         for<'de> T: serde::Deserialize<'de>,
     {
-        let mut request = self.request_builder(Method::Get, path);
-        Response::new(self.app().0.middle.call(&mut request))
+        let request = self.request_builder(Method::Get, path);
+        self.run(request)
     }
 
     /// Issue a GET request that includes query parameters
@@ -161,36 +169,36 @@ pub trait RequestHelper {
     {
         let mut request = self.request_builder(Method::Get, path);
         request.with_query(query);
-        Response::new(self.app().0.middle.call(&mut request))
+        self.run(request)
     }
 
     /// Issue a PUT request
     fn put<T>(&self, path: &str, body: &[u8]) -> Response<T>
     where
         for<'de> T: serde::Deserialize<'de>,
     {
-        let mut builder = self.request_builder(Method::Put, path);
-        let request = builder.with_body(body);
-        Response::new(self.app().0.middle.call(request))
+        let mut request = self.request_builder(Method::Put, path);
+        request.with_body(body);
+        self.run(request)
     }
 
     /// Issue a DELETE request
     fn delete<T>(&self, path: &str) -> Response<T>
     where
         for<'de> T: serde::Deserialize<'de>,
     {
-        let mut request = self.request_builder(Method::Delete, path);
-        Response::new(self.app().0.middle.call(&mut request))
+        let request = self.request_builder(Method::Delete, path);
+        self.run(request)
     }
 
     /// Issue a DELETE request with a body... yes we do it, for crate owner removal
     fn delete_with_body<T>(&self, path: &str, body: &[u8]) -> Response<T>
     where
         for<'de> T: serde::Deserialize<'de>,
     {
-        let mut builder = self.request_builder(Method::Delete, path);
-        let request = builder.with_body(body);
-        Response::new(self.app().0.middle.call(request))
+        let mut request = self.request_builder(Method::Delete, path);
+        request.with_body(body);
+        self.run(request)
     }
 
     /// Search for crates matching a query string