Implement RFC 3139: alternative registry authentication support

Author: arlosi

CHANGELOG.md

@@ -326,7 +326,7 @@
 - Added `-Zcheck-cfg=output` to support build-scripts declaring their
   supported set of `cfg` values with `cargo:rustc-check-cfg`.
   [#10539](https://github.com/rust-lang/cargo/pull/10539)
-- `-Z http-registry` now uses https://index.crates.io/ when accessing crates-io.
+- `-Z sparse-registry` now uses https://index.crates.io/ when accessing crates-io.
   [#10725](https://github.com/rust-lang/cargo/pull/10725)
 - Fixed formatting of `.workspace` key in `cargo add` for workspace inheritance.
   [#10705](https://github.com/rust-lang/cargo/pull/10705)

Cargo.toml

@@ -33,6 +33,7 @@ git2-curl = "0.16.0"
 glob = "0.3.0"
 hex = "0.4"
 home = "0.5"
+http-auth = { version = "0.1.6", default-features = false }
 humantime = "2.0.0"
 indexmap = "1"
 ignore = "0.4.7"

crates/cargo-test-support/src/registry.rs

@@ -11,7 +11,7 @@ use std::fs::{self, File};
 use std::io::{BufRead, BufReader, Read, Write};
 use std::net::{SocketAddr, TcpListener, TcpStream};
 use std::path::PathBuf;
-use std::thread;
+use std::thread::{self, JoinHandle};
 use tar::{Builder, Header};
 use url::Url;
 
@@ -61,6 +61,8 @@ pub struct RegistryBuilder {
     alternative: Option<String>,
     /// If set, the authorization token for the registry.
     token: Option<String>,
+    /// If set, the registry requires authorization for all operations.
+    auth_required: bool,
     /// If set, serves the index over http.
     http_index: bool,
     /// If set, serves the API over http.
@@ -76,7 +78,7 @@ pub struct RegistryBuilder {
 }
 
 pub struct TestRegistry {
-    _server: Option<HttpServerHandle>,
+    server: Option<HttpServerHandle>,
     index_url: Url,
     path: PathBuf,
     api_url: Url,
@@ -98,6 +100,17 @@ impl TestRegistry {
             .as_deref()
             .expect("registry was not configured with a token")
     }
+
+    /// Shutdown the server thread and wait for it to stop.
+    /// `Drop` automatically stops the server, but this additionally
+    /// waits for the thread to stop.
+    pub fn join(self) {
+        if let Some(mut server) = self.server {
+            server.stop();
+            let handle = server.handle.take().unwrap();
+            handle.join().unwrap();
+        }
+    }
 }
 
 impl RegistryBuilder {
@@ -106,6 +119,7 @@ impl RegistryBuilder {
         RegistryBuilder {
             alternative: None,
             token: None,
+            auth_required: false,
             http_api: false,
             http_index: false,
             api: true,
@@ -160,6 +174,14 @@ impl RegistryBuilder {
         self
     }
 
+    /// Sets this registry to require the authentication token for
+    /// all operations.
+    #[must_use]
+    pub fn auth_required(mut self) -> Self {
+        self.auth_required = true;
+        self
+    }
+
     /// Operate the index over http
     #[must_use]
     pub fn http_index(mut self) -> Self {
@@ -207,6 +229,7 @@ impl RegistryBuilder {
                 registry_path.clone(),
                 dl_path,
                 token.clone(),
+                self.auth_required,
                 self.custom_responders,
             );
             let index_url = if self.http_index {
@@ -226,7 +249,7 @@ impl RegistryBuilder {
         let registry = TestRegistry {
             api_url,
             index_url,
-            _server: server,
+            server,
             dl_url,
             path: registry_path,
             token,
@@ -293,6 +316,11 @@ impl RegistryBuilder {
             }
         }
 
+        let auth = if self.auth_required {
+            r#","auth-required":true"#
+        } else {
+            ""
+        };
         let api = if self.api {
             format!(r#","api":"{}""#, registry.api_url)
         } else {
@@ -302,7 +330,7 @@ impl RegistryBuilder {
         repo(&registry.path)
             .file(
                 "config.json",
-                &format!(r#"{{"dl":"{}"{api}}}"#, registry.dl_url),
+                &format!(r#"{{"dl":"{}"{api}{auth}}}"#, registry.dl_url),
             )
             .build();
         fs::create_dir_all(api_path.join("api/v1/crates")).unwrap();
@@ -442,6 +470,7 @@ pub fn alt_init() -> TestRegistry {
 
 pub struct HttpServerHandle {
     addr: SocketAddr,
+    handle: Option<JoinHandle<()>>,
 }
 
 impl HttpServerHandle {
@@ -456,10 +485,8 @@ impl HttpServerHandle {
     pub fn dl_url(&self) -> Url {
         Url::parse(&format!("http://{}/dl", self.addr.to_string())).unwrap()
     }
-}
 
-impl Drop for HttpServerHandle {
-    fn drop(&mut self) {
+    fn stop(&self) {
         if let Ok(mut stream) = TcpStream::connect(self.addr) {
             // shutdown the server
             let _ = stream.write_all(b"stop");
@@ -468,6 +495,12 @@ impl Drop for HttpServerHandle {
     }
 }
 
+impl Drop for HttpServerHandle {
+    fn drop(&mut self) {
+        self.stop();
+    }
+}
+
 /// Request to the test http server
 #[derive(Clone)]
 pub struct Request {
@@ -504,6 +537,7 @@ pub struct HttpServer {
     registry_path: PathBuf,
     dl_path: PathBuf,
     token: Option<String>,
+    auth_required: bool,
     custom_responders: HashMap<&'static str, Box<dyn Send + Fn(&Request, &HttpServer) -> Response>>,
 }
 
@@ -512,6 +546,7 @@ impl HttpServer {
         registry_path: PathBuf,
         dl_path: PathBuf,
         token: Option<String>,
+        auth_required: bool,
         api_responders: HashMap<
             &'static str,
             Box<dyn Send + Fn(&Request, &HttpServer) -> Response>,
@@ -524,10 +559,11 @@ impl HttpServer {
             registry_path,
             dl_path,
             token,
+            auth_required,
             custom_responders: api_responders,
         };
-        thread::spawn(move || server.start());
-        HttpServerHandle { addr }
+        let handle = Some(thread::spawn(move || server.start()));
+        HttpServerHandle { addr, handle }
     }
 
     fn start(&self) {
@@ -615,7 +651,7 @@ impl HttpServer {
     /// Route the request
     fn route(&self, req: &Request) -> Response {
         let authorized = |mutatation: bool| {
-            if mutatation {
+            if mutatation || self.auth_required {
                 self.token == req.authorization
             } else {
                 assert!(req.authorization.is_none(), "unexpected token");
@@ -676,7 +712,9 @@ impl HttpServer {
     pub fn unauthorized(&self, _req: &Request) -> Response {
         Response {
             code: 401,
-            headers: vec![],
+            headers: vec![
+                r#"WWW-Authenticate: Cargo login_url="https://test-registry-login/me""#.to_string(),
+            ],
             body: b"Unauthorized message from server.".to_vec(),
         }
     }

crates/crates-io/lib.rs

@@ -21,6 +21,8 @@ pub struct Registry {
     token: Option<String>,
     /// Curl handle for issuing requests.
     handle: Easy,
+    /// Whether to include the authorization token with all requests.
+    auth_required: bool,
 }
 
 #[derive(PartialEq, Clone, Copy)]
@@ -199,11 +201,17 @@ impl Registry {
     /// handle.useragent("my_crawler (example.com/info)");
     /// let mut reg = Registry::new_handle(String::from("https://crates.io"), None, handle);
     /// ```
-    pub fn new_handle(host: String, token: Option<String>, handle: Easy) -> Registry {
+    pub fn new_handle(
+        host: String,
+        token: Option<String>,
+        handle: Easy,
+        auth_required: bool,
+    ) -> Registry {
         Registry {
             host,
             token,
             handle,
+            auth_required,
         }
     }
 
@@ -377,7 +385,7 @@ impl Registry {
         headers.append("Accept: application/json")?;
         headers.append("Content-Type: application/json")?;
 
-        if authorized == Auth::Authorized {
+        if self.auth_required || authorized == Auth::Authorized {
             let token = match self.token.as_ref() {
                 Some(s) => s,
                 None => bail!("no upload token found, please run `cargo login`"),

crates/credential/cargo-credential-1password/Cargo.toml

@@ -1,12 +1,12 @@
 [package]
 name = "cargo-credential-1password"
-version = "0.1.0"
+version = "0.2.0"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/rust-lang/cargo"
 description = "A Cargo credential process that stores tokens in a 1password vault."
 
 [dependencies]
-cargo-credential = { version = "0.1.0", path = "../cargo-credential" }
+cargo-credential = { version = "0.2.0", path = "../cargo-credential" }
 serde = { version = "1.0.117", features = ["derive"] }
 serde_json = "1.0.59"

crates/credential/cargo-credential-1password/src/main.rs

@@ -41,7 +41,7 @@ struct ListItem {
 
 #[derive(Deserialize)]
 struct Overview {
-    title: String,
+    url: String,
 }
 
 impl OnePasswordKeychain {
@@ -175,11 +175,7 @@ impl OnePasswordKeychain {
         Ok(buffer)
     }
 
-    fn search(
-        &self,
-        session: &Option<String>,
-        registry_name: &str,
-    ) -> Result<Option<String>, Error> {
+    fn search(&self, session: &Option<String>, index_url: &str) -> Result<Option<String>, Error> {
         let cmd = self.make_cmd(
             session,
             &[
@@ -196,15 +192,15 @@ impl OnePasswordKeychain {
             .map_err(|e| format!("failed to deserialize JSON from 1password list: {}", e))?;
         let mut matches = items
             .into_iter()
-            .filter(|item| item.overview.title == registry_name);
+            .filter(|item| item.overview.url == index_url);
         match matches.next() {
             Some(login) => {
                 // Should this maybe just sort on `updatedAt` and return the newest one?
                 if matches.next().is_some() {
                     return Err(format!(
-                        "too many 1password logins match registry name {}, \
+                        "too many 1password logins match registry `{}`, \
                         consider deleting the excess entries",
-                        registry_name
+                        index_url
                     )
                     .into());
                 }
@@ -214,7 +210,13 @@ impl OnePasswordKeychain {
         }
     }
 
-    fn modify(&self, session: &Option<String>, uuid: &str, token: &str) -> Result<(), Error> {
+    fn modify(
+        &self,
+        session: &Option<String>,
+        uuid: &str,
+        token: &str,
+        _name: Option<&str>,
+    ) -> Result<(), Error> {
         let cmd = self.make_cmd(
             session,
             &["edit", "item", uuid, &format!("password={}", token)],
@@ -226,20 +228,24 @@ impl OnePasswordKeychain {
     fn create(
         &self,
         session: &Option<String>,
-        registry_name: &str,
-        api_url: &str,
+        index_url: &str,
         token: &str,
+        name: Option<&str>,
     ) -> Result<(), Error> {
+        let title = match name {
+            Some(name) => format!("Cargo registry token for {}", name),
+            None => "Cargo registry token".to_string(),
+        };
         let cmd = self.make_cmd(
             session,
             &[
                 "create",
                 "item",
                 "Login",
                 &format!("password={}", token),
-                &format!("url={}", api_url),
+                &format!("url={}", index_url),
                 "--title",
-                registry_name,
+                &title,
                 "--tags",
                 CARGO_TAG,
             ],
@@ -276,36 +282,36 @@ impl Credential for OnePasswordKeychain {
         env!("CARGO_PKG_NAME")
     }
 
-    fn get(&self, registry_name: &str, _api_url: &str) -> Result<String, Error> {
+    fn get(&self, index_url: &str) -> Result<String, Error> {
         let session = self.signin()?;
-        if let Some(uuid) = self.search(&session, registry_name)? {
+        if let Some(uuid) = self.search(&session, index_url)? {
             self.get_token(&session, &uuid)
         } else {
             return Err(format!(
                 "no 1password entry found for registry `{}`, try `cargo login` to add a token",
-                registry_name
+                index_url
             )
             .into());
         }
     }
 
-    fn store(&self, registry_name: &str, api_url: &str, token: &str) -> Result<(), Error> {
+    fn store(&self, index_url: &str, token: &str, name: Option<&str>) -> Result<(), Error> {
         let session = self.signin()?;
         // Check if an item already exists.
-        if let Some(uuid) = self.search(&session, registry_name)? {
-            self.modify(&session, &uuid, token)
+        if let Some(uuid) = self.search(&session, index_url)? {
+            self.modify(&session, &uuid, token, name)
         } else {
-            self.create(&session, registry_name, api_url, token)
+            self.create(&session, index_url, token, name)
         }
     }
 
-    fn erase(&self, registry_name: &str, _api_url: &str) -> Result<(), Error> {
+    fn erase(&self, index_url: &str) -> Result<(), Error> {
         let session = self.signin()?;
         // Check if an item already exists.
-        if let Some(uuid) = self.search(&session, registry_name)? {
+        if let Some(uuid) = self.search(&session, index_url)? {
             self.delete(&session, &uuid)?;
         } else {
-            eprintln!("not currently logged in to `{}`", registry_name);
+            eprintln!("not currently logged in to `{}`", index_url);
         }
         Ok(())
     }