From 4e039018e449da31eca0edc17600a737a394bd64 Mon Sep 17 00:00:00 2001 From: Stefan Hausotte Date: Fri, 23 Apr 2021 21:06:49 +0200 Subject: [PATCH 1/3] fix type inference error --- src/cargo/util/config/de.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/util/config/de.rs b/src/cargo/util/config/de.rs index 758f5c23f13..e8238678297 100644 --- a/src/cargo/util/config/de.rs +++ b/src/cargo/util/config/de.rs @@ -527,7 +527,7 @@ impl<'de, 'config> de::MapAccess<'de> for ValueDeserializer<'config> { seed.deserialize(Tuple2Deserializer(0i32, path.to_string_lossy())) } Definition::Environment(env) => { - seed.deserialize(Tuple2Deserializer(1i32, env.as_ref())) + seed.deserialize(Tuple2Deserializer(1i32, env.as_ref() as &str)) } Definition::Cli => seed.deserialize(Tuple2Deserializer(2i32, "")), } From a18cc4bb69184d1a69b841ca082d2f17c27ea4c0 Mon Sep 17 00:00:00 2001 From: Stefan Hausotte Date: Sat, 24 Apr 2021 17:10:17 +0200 Subject: [PATCH 2/3] add token to HttpRegistry --- src/cargo/core/source/source_id.rs | 4 ++ src/cargo/sources/config.rs | 23 +++++++ src/cargo/sources/registry/http_remote.rs | 14 +++++ tests/testsuite/http_registry.rs | 77 +++++++++++++++++++++++ 4 files changed, 118 insertions(+) diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index e91f21a9c18..ee277642ff9 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -365,6 +365,10 @@ impl SourceId { pub fn full_hash(self, into: &mut S) { ptr::NonNull::from(self.inner).hash(into) } + + pub fn get_name(&self) -> Option { + self.inner.name.clone() + } } impl PartialEq for SourceId { diff --git a/src/cargo/sources/config.rs b/src/cargo/sources/config.rs index 71a9a7194c0..6cc63da696c 100644 --- a/src/cargo/sources/config.rs +++ b/src/cargo/sources/config.rs @@ -33,6 +33,8 @@ struct SourceConfigDef { directory: Option, /// A registry source. Value is a URL. registry: OptValue, + /// An optional authorization token + token: Option, /// A local registry source. local_registry: Option, /// A git source. Value is a URL. @@ -51,6 +53,7 @@ struct SourceConfigDef { /// [source.crates-io] /// registry = 'https://github.com/rust-lang/crates.io-index' /// replace-with = 'foo' # optional +/// token = 'authorization_token' # optional /// ``` #[derive(Clone)] struct SourceConfig { @@ -64,6 +67,17 @@ struct SourceConfig { /// this configuration key was defined (such as the `.cargo/config` path /// or the environment variable name). replace_with: Option<(String, String)>, + + /// Optional authorization token. + /// + /// Use to authorize any request to a private registry. + token: Option, +} + +impl SourceConfig { + pub fn get_token(&self) -> Option { + self.token.clone() + } } impl<'cfg> SourceConfigMap<'cfg> { @@ -89,11 +103,18 @@ impl<'cfg> SourceConfigMap<'cfg> { SourceConfig { id: SourceId::crates_io(config)?, replace_with: None, + token: None, }, )?; Ok(base) } + pub fn get_token(&self, source_id: &SourceId) -> Option { + source_id.get_name() + .and_then(|n| self.cfgs.get(&n)) + .and_then(|c| c.get_token()) + } + pub fn config(&self) -> &'cfg Config { self.config } @@ -269,11 +290,13 @@ restore the source replacement configuration to continue the build .replace_with .map(|val| (val.val, val.definition.to_string())); + self.add( &name, SourceConfig { id: src, replace_with, + token: def.token, }, )?; diff --git a/src/cargo/sources/registry/http_remote.rs b/src/cargo/sources/registry/http_remote.rs index e729bbaa53a..d8e94dae003 100644 --- a/src/cargo/sources/registry/http_remote.rs +++ b/src/cargo/sources/registry/http_remote.rs @@ -61,6 +61,9 @@ pub struct HttpRegistry<'cfg> { /// Store the server URL without the protocol prefix (sparse+) url: Url, + /// Store optional authorization token + token: Option, + /// Cached HTTP handle for synchronous requests (RegistryData::load). http: RefCell>, @@ -160,12 +163,20 @@ impl<'cfg> HttpRegistry<'cfg> { .into_url() .expect("a url with the protocol stripped should still be valid"); + let token = match crate::sources::SourceConfigMap::new(&config) { + Err(_) => None, + Ok(sm) => { + sm.get_token(&source_id) + } + }; + HttpRegistry { index_path: config.registry_index_path().join(name), cache_path: config.registry_cache_path().join(name), source_id, config, url, + token, http: RefCell::new(None), prefetch: Multi::new(), multiplexing: false, @@ -463,6 +474,9 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { handle.http_headers(list)?; } + // Add authorization token, if set. + let auth_token = self.config; + // We're going to have a bunch of downloads all happening "at the same time". // So, we need some way to track what headers/data/responses are for which request. // We do that through this token. Each request (and associated response) gets one. diff --git a/tests/testsuite/http_registry.rs b/tests/testsuite/http_registry.rs index e639b7a69dd..bee97b313d3 100644 --- a/tests/testsuite/http_registry.rs +++ b/tests/testsuite/http_registry.rs @@ -43,6 +43,83 @@ fn setup() -> RegistryServer { server } +fn setup_authorized() -> RegistryServer { + let server = serve_registry(registry_path()); + + let root = paths::root(); + t!(fs::create_dir(&root.join(".cargo"))); + t!(fs::write( + root.join(".cargo/config"), + format!( + " + [source.crates-io] + registry = 'https://wut' + replace-with = 'my-awesome-http-registry' + + [source.my-awesome-http-registry] + registry = 'sparse+http://{}' + token = 'some_authorization_token' + ", + server.addr() + ) + )); + + server +} + + +#[cargo_test] +fn simple_authorized() { + let server = setup_authorized(); + let url = format!("sparse+http://{}", server.addr()); + let p = project() + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = ">= 0.0.0" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + Package::new("bar", "0.0.1").publish(); + + cargo(&p, "build") + .with_stderr(&format!( + "\ +[UPDATING] `{reg}` index +[PREFETCHING] index files ... +[DOWNLOADING] crates ... +[DOWNLOADED] bar v0.0.1 (registry `{reg}`) +[COMPILING] bar v0.0.1 +[COMPILING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s +", + reg = url + )) + .run(); + + cargo(&p, "clean").run(); + + // Don't download a second time + cargo(&p, "build") + .with_stderr( + "\ +[PREFETCHING] index files ... +[COMPILING] bar v0.0.1 +[COMPILING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s +", + ) + .run(); +} + #[cargo_test] fn not_on_stable() { let _server = setup(); From c63949a8c2ceea62e48f3002279e2cb7c100ae85 Mon Sep 17 00:00:00 2001 From: Stefan Hausotte Date: Sun, 25 Apr 2021 14:02:53 +0200 Subject: [PATCH 3/3] authorization works --- crates/cargo-test-support/src/registry.rs | 64 ++++++++++---- src/cargo/sources/config.rs | 13 +-- src/cargo/sources/registry/http_remote.rs | 42 ++++++--- tests/testsuite/http_registry.rs | 100 +++++++++++++++++++--- 4 files changed, 170 insertions(+), 49 deletions(-) diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index 60fb78504a1..1cbbb9dec6f 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -15,6 +15,7 @@ use std::sync::Arc; use std::thread; use tar::{Builder, Header}; use url::Url; +use std::ops::Deref; /// Gets the path to the local index pretending to be crates.io. This is a Git repo /// initialized with a `config.json` file pointing to `dl_path` for downloads @@ -22,44 +23,55 @@ use url::Url; pub fn registry_path() -> PathBuf { generate_path("registry") } + pub fn registry_url() -> Url { generate_url("registry") } + /// Gets the path for local web API uploads. Cargo will place the contents of a web API /// request here. For example, `api/v1/crates/new` is the result of publishing a crate. pub fn api_path() -> PathBuf { generate_path("api") } + pub fn api_url() -> Url { generate_url("api") } + /// Gets the path where crates can be downloaded using the web API endpoint. Crates /// should be organized as `{name}/{version}/download` to match the web API /// endpoint. This is rarely used and must be manually set up. pub fn dl_path() -> PathBuf { generate_path("dl") } + pub fn dl_url() -> Url { generate_url("dl") } + /// Gets the alternative-registry version of `registry_path`. pub fn alt_registry_path() -> PathBuf { generate_path("alternative-registry") } + pub fn alt_registry_url() -> Url { generate_url("alternative-registry") } + /// Gets the alternative-registry version of `dl_path`. pub fn alt_dl_path() -> PathBuf { generate_path("alt_dl") } + pub fn alt_dl_url() -> String { generate_alt_dl_url("alt_dl") } + /// Gets the alternative-registry version of `api_path`. pub fn alt_api_path() -> PathBuf { generate_path("alt_api") } + pub fn alt_api_url() -> Url { generate_url("alt_api") } @@ -67,9 +79,11 @@ pub fn alt_api_url() -> Url { pub fn generate_path(name: &str) -> PathBuf { paths::root().join(name) } + pub fn generate_url(name: &str) -> Url { Url::from_file_path(generate_path(name)).ok().unwrap() } + pub fn generate_alt_dl_url(name: &str) -> String { let base = Url::from_file_path(generate_path(name)).ok().unwrap(); format!("{}/{{crate}}/{{version}}/{{crate}}-{{version}}.crate", base) @@ -239,7 +253,7 @@ impl Drop for RegistryServer { } #[must_use] -pub fn serve_registry(registry_path: PathBuf) -> RegistryServer { +pub fn serve_registry(registry_path: PathBuf, auth_token: Option) -> RegistryServer { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let addr = listener.local_addr().unwrap(); let done = Arc::new(AtomicBool::new(false)); @@ -274,6 +288,7 @@ pub fn serve_registry(registry_path: PathBuf) -> RegistryServer { // Grab some other headers we may care about. let mut if_modified_since = None; let mut if_none_match = None; + let mut token = None; loop { line.clear(); if buf.read_line(&mut line).unwrap() == 0 { @@ -297,9 +312,21 @@ pub fn serve_registry(registry_path: PathBuf) -> RegistryServer { if_modified_since = Some(value.to_owned()); } else if line.starts_with("If-None-Match:") { if_none_match = Some(value.trim_matches('"').to_owned()); + // It would make sense to check the auth. header before the "file.exists()", as + // everything after can be skipped. For the tests, it should be enough to check it here. + } else if line.starts_with("Authorization:") { + token = Some(value.to_owned()) } } + // Check if authorization is expected and + // an authorization token is provided. + let authorized = match (&auth_token, token) { + (Some(expected), Some(actual)) => expected.deref() == actual, + (Some(expected), None) => false, + (_, _) => true + }; + // Now grab info about the file. let data = fs::read(&file).unwrap(); let etag = Sha256::new().update(&data).finish_hex(); @@ -325,7 +352,12 @@ pub fn serve_registry(registry_path: PathBuf) -> RegistryServer { } // Write out the main response line. - if any_match && all_match { + if !authorized { + buf.get_mut() + .write_all(b"HTTP/1.1 401 Unauthorized\r\n\r\n") + .unwrap(); + } + else if any_match && all_match { buf.get_mut() .write_all(b"HTTP/1.1 304 Not Modified\r\n") .unwrap(); @@ -335,19 +367,21 @@ pub fn serve_registry(registry_path: PathBuf) -> RegistryServer { // TODO: Support 451 for crate index deletions. // Write out other headers. - buf.get_mut() - .write_all(format!("Content-Length: {}\r\n", data.len()).as_bytes()) - .unwrap(); - buf.get_mut() - .write_all(format!("ETag: \"{}\"\r\n", etag).as_bytes()) - .unwrap(); - buf.get_mut() - .write_all(format!("Last-Modified: {}\r\n", last_modified).as_bytes()) - .unwrap(); + if authorized { + buf.get_mut() + .write_all(format!("Content-Length: {}\r\n", data.len()).as_bytes()) + .unwrap(); + buf.get_mut() + .write_all(format!("ETag: \"{}\"\r\n", etag).as_bytes()) + .unwrap(); + buf.get_mut() + .write_all(format!("Last-Modified: {}\r\n", last_modified).as_bytes()) + .unwrap(); - // And finally, write out the body. - buf.get_mut().write_all(b"\r\n").unwrap(); - buf.get_mut().write_all(&data).unwrap(); + // And finally, write out the body. + buf.get_mut().write_all(b"\r\n").unwrap(); + buf.get_mut().write_all(&data).unwrap(); + } } else { loop { line.clear(); @@ -593,7 +627,7 @@ impl Package { "yanked": self.yanked, "links": self.links, }) - .to_string(); + .to_string(); let file = match self.name.len() { 1 => format!("1/{}", self.name), diff --git a/src/cargo/sources/config.rs b/src/cargo/sources/config.rs index 6cc63da696c..acc68f78c6b 100644 --- a/src/cargo/sources/config.rs +++ b/src/cargo/sources/config.rs @@ -74,12 +74,6 @@ struct SourceConfig { token: Option, } -impl SourceConfig { - pub fn get_token(&self) -> Option { - self.token.clone() - } -} - impl<'cfg> SourceConfigMap<'cfg> { pub fn new(config: &'cfg Config) -> CargoResult> { let mut base = SourceConfigMap::empty(config)?; @@ -109,10 +103,9 @@ impl<'cfg> SourceConfigMap<'cfg> { Ok(base) } - pub fn get_token(&self, source_id: &SourceId) -> Option { - source_id.get_name() - .and_then(|n| self.cfgs.get(&n)) - .and_then(|c| c.get_token()) + pub fn token(&self, source_id: &SourceId) -> Option { + let name: &str = self.id2name[source_id].as_ref(); + self.cfgs.get(name).and_then(|c| c.token.clone()) } pub fn config(&self) -> &'cfg Config { diff --git a/src/cargo/sources/registry/http_remote.rs b/src/cargo/sources/registry/http_remote.rs index d8e94dae003..c2c6ea8b180 100644 --- a/src/cargo/sources/registry/http_remote.rs +++ b/src/cargo/sources/registry/http_remote.rs @@ -163,12 +163,8 @@ impl<'cfg> HttpRegistry<'cfg> { .into_url() .expect("a url with the protocol stripped should still be valid"); - let token = match crate::sources::SourceConfigMap::new(&config) { - Err(_) => None, - Ok(sm) => { - sm.get_token(&source_id) - } - }; + // I bet there is better way to get to the token that I'm not aware of + let token=crate::sources::SourceConfigMap::new(&config).unwrap().token(&source_id); HttpRegistry { index_path: config.registry_index_path().join(name), @@ -463,19 +459,21 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { try_old_curl!(handle.pipewait(true), "pipewait"); // Make sure we don't send data back if it's the same as we have in the index. + let mut list = List::new(); if let Some((ref etag, ref last_modified, _)) = was { - let mut list = List::new(); if !etag.is_empty() { list.append(&format!("If-None-Match: {}", etag))?; } if !last_modified.is_empty() { list.append(&format!("If-Modified-Since: {}", last_modified))?; } - handle.http_headers(list)?; } - // Add authorization token, if set. - let auth_token = self.config; + // If an authorization token is set, add it to the headers. + if let Some(ref auth_token) = self.token { + list.append(&format!("Authorization: {}", auth_token))?; + } + handle.http_headers(list)?; // We're going to have a bunch of downloads all happening "at the same time". // So, we need some way to track what headers/data/responses are for which request. @@ -746,6 +744,21 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { paths::remove_file(pkg)?; } } + 401 => { + // Not Authorized + // + // If the registry supports authorization and the authorization + // fails for some reason, e.g. missing or wrong token. + eprintln!("error: authorization failed. Request needs to be authorized with a token."); // Not sure what the correct why of displaying a meaningful error message is + + // Let cargo handle the "Not Authorized" case the same way, it handles a "Not Found", as in the end + // the package is not available to use. + let path = self.config.assert_package_cache_locked(&self.index_path); + let pkg = path.join(&fetched.path); + if pkg.exists() { + paths::remove_file(pkg)?; + } + } code => { anyhow::bail!( "prefetch: server returned unexpected HTTP status code {} for {}{}: {}", @@ -894,13 +907,18 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { debug!("fetch {}{}", self.url, path.display()); handle.url(&format!("{}{}", self.url, path.display()))?; + let mut list = List::new(); if let Some((ref etag, ref last_modified, _)) = was { - let mut list = List::new(); list.append(&format!("If-None-Match: {}", etag))?; list.append(&format!("If-Modified-Since: {}", last_modified))?; - handle.http_headers(list)?; } + // If an authorization token is set, add it to the headers. + if let Some(ref auth_token) = self.token { + list.append(&format!("Authorization: {}", auth_token))?; + } + handle.http_headers(list)?; + let mut contents = Vec::new(); let mut etag = None; let mut last_modified = None; diff --git a/tests/testsuite/http_registry.rs b/tests/testsuite/http_registry.rs index bee97b313d3..9b2abd119bd 100644 --- a/tests/testsuite/http_registry.rs +++ b/tests/testsuite/http_registry.rs @@ -21,7 +21,7 @@ fn cargo(p: &cargo_test_support::Project, s: &str) -> cargo_test_support::Execs } fn setup() -> RegistryServer { - let server = serve_registry(registry_path()); + let server = serve_registry(registry_path(), None); let root = paths::root(); t!(fs::create_dir(&root.join(".cargo"))); @@ -43,8 +43,8 @@ fn setup() -> RegistryServer { server } -fn setup_authorized() -> RegistryServer { - let server = serve_registry(registry_path()); +fn setup_authorized(expected_token: Option, actual_token: Option) -> RegistryServer { + let server = serve_registry(registry_path(), expected_token); let root = paths::root(); t!(fs::create_dir(&root.join(".cargo"))); @@ -58,9 +58,13 @@ fn setup_authorized() -> RegistryServer { [source.my-awesome-http-registry] registry = 'sparse+http://{}' - token = 'some_authorization_token' + {} ", - server.addr() + server.addr(), + match actual_token { + Some(t) => format!("token = '{}'", t), + None => String::new() + } ) )); @@ -70,7 +74,10 @@ fn setup_authorized() -> RegistryServer { #[cargo_test] fn simple_authorized() { - let server = setup_authorized(); + let server = setup_authorized( + Some("auth_token".to_string()), + Some("auth_token".to_string()), + ); let url = format!("sparse+http://{}", server.addr()); let p = project() .file( @@ -106,20 +113,89 @@ fn simple_authorized() { .run(); cargo(&p, "clean").run(); +} + + +#[cargo_test] +fn simple_wrong_authorization_token() { + let server = setup_authorized( + Some("auth_token".to_string()), + Some("wrong_auth_token".to_string()), + ); + let url = format!("sparse+http://{}", server.addr()); + let p = project() + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = ">= 0.0.0" + "#, + ) + .file("src/main.rs", "fn main() {}") + .build(); + + Package::new("bar", "0.0.1").publish(); - // Don't download a second time cargo(&p, "build") - .with_stderr( + .with_status(101) + .with_stderr_contains(&format!( "\ +[UPDATING] `{reg}` index [PREFETCHING] index files ... -[COMPILING] bar v0.0.1 -[COMPILING] foo v0.0.1 ([CWD]) -[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s -", +error: authorization failed. Request needs to be authorized with a token. +error: no matching package named `bar` found", + reg = url)) + .run(); + + cargo(&p, "clean").run(); +} + + +#[cargo_test] +fn simple_authorization_token_missing() { + let server = setup_authorized( + Some("auth_token".to_string()), + None, + ); + let url = format!("sparse+http://{}", server.addr()); + let p = project() + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + bar = ">= 0.0.0" + "#, ) + .file("src/main.rs", "fn main() {}") + .build(); + + Package::new("bar", "0.0.1").publish(); + + cargo(&p, "build") + .with_status(101) + .with_stderr_contains(&format!( + "\ +[UPDATING] `{reg}` index +[PREFETCHING] index files ... +error: authorization failed. Request needs to be authorized with a token. +error: no matching package named `bar` found", + reg = url)) .run(); + + cargo(&p, "clean").run(); } + #[cargo_test] fn not_on_stable() { let _server = setup();