From 6151a4152ba880f1b61958abbb2734966cf6b835 Mon Sep 17 00:00:00 2001 From: Arlo Siemsen Date: Fri, 21 Jul 2023 17:30:01 -0500 Subject: [PATCH] Send all headers to cred provider --- credential/cargo-credential/src/lib.rs | 5 ++-- src/cargo/core/package.rs | 6 ++--- src/cargo/ops/registry/mod.rs | 2 +- src/cargo/ops/registry/publish.rs | 2 +- src/cargo/sources/registry/download.rs | 2 +- src/cargo/sources/registry/http_remote.rs | 27 +++++++++---------- src/cargo/util/auth/mod.rs | 17 ++++++------ src/cargo/util/errors.rs | 15 +++++++++-- tests/testsuite/credential_process.rs | 32 ++++++++++++++--------- 9 files changed, 60 insertions(+), 48 deletions(-) diff --git a/credential/cargo-credential/src/lib.rs b/credential/cargo-credential/src/lib.rs index fbc6d8cf8bf..3bae2f40a50 100644 --- a/credential/cargo-credential/src/lib.rs +++ b/credential/cargo-credential/src/lib.rs @@ -62,8 +62,9 @@ pub struct RegistryInfo<'a> { /// Name of the registry in configuration. May not be available. /// The crates.io registry will be `crates-io` (`CRATES_IO_REGISTRY`). pub name: Option<&'a str>, - /// www-authenticate headers from attempting to access a sparse registry. - pub www_authenticate: Option>, + /// Headers from attempting to access a registry that resulted in a HTTP 401. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub headers: Vec, } #[derive(Serialize, Deserialize, Clone, Debug)] diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index f4ab448d28d..093c64d72d3 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -25,7 +25,7 @@ use crate::core::source::MaybePackage; use crate::core::{Dependency, Manifest, PackageId, SourceId, Target}; use crate::core::{SourceMap, Summary, Workspace}; use crate::util::config::PackageCacheLock; -use crate::util::errors::{CargoResult, HttpNotSuccessful, DEBUG_HEADERS}; +use crate::util::errors::{CargoResult, HttpNotSuccessful}; use crate::util::interning::InternedString; use crate::util::network::http::http_handle_and_timeout; use crate::util::network::http::HttpTimeout; @@ -748,9 +748,7 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> { // Headers contain trailing \r\n, trim them to make it easier // to work with. let h = String::from_utf8_lossy(data).trim().to_string(); - if DEBUG_HEADERS.iter().any(|p| h.starts_with(p)) { - downloads.pending[&token].0.headers.borrow_mut().push(h); - } + downloads.pending[&token].0.headers.borrow_mut().push(h); } }); true diff --git a/src/cargo/ops/registry/mod.rs b/src/cargo/ops/registry/mod.rs index 28aae140452..94ef1ead271 100644 --- a/src/cargo/ops/registry/mod.rs +++ b/src/cargo/ops/registry/mod.rs @@ -144,7 +144,7 @@ fn registry( &source_ids.original, None, operation, - None, + vec![], )?) } else { None diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index bba87cc146c..40ca9fd16f5 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -160,7 +160,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> { ®_ids.original, None, operation, - None, + vec![], )?)); } diff --git a/src/cargo/sources/registry/download.rs b/src/cargo/sources/registry/download.rs index 50b39fa876d..08940b3a1db 100644 --- a/src/cargo/sources/registry/download.rs +++ b/src/cargo/sources/registry/download.rs @@ -84,7 +84,7 @@ pub(super) fn download( &pkg.source_id(), None, Operation::Read, - None, + vec![], )?) } else { None diff --git a/src/cargo/sources/registry/http_remote.rs b/src/cargo/sources/registry/http_remote.rs index f43d73c0895..05920eab11e 100644 --- a/src/cargo/sources/registry/http_remote.rs +++ b/src/cargo/sources/registry/http_remote.rs @@ -4,7 +4,7 @@ use crate::core::{PackageId, SourceId}; use crate::sources::registry::download; use crate::sources::registry::MaybeLock; use crate::sources::registry::{LoadResponse, RegistryConfig, RegistryData}; -use crate::util::errors::{CargoResult, HttpNotSuccessful, DEBUG_HEADERS}; +use crate::util::errors::{CargoResult, HttpNotSuccessful}; use crate::util::network::http::http_handle; use crate::util::network::retry::{Retry, RetryResult}; use crate::util::network::sleep::SleepTracker; @@ -97,8 +97,8 @@ pub struct HttpRegistry<'cfg> { /// Url to get a token for the registry. login_url: Option, - /// WWW-Authenticate header received with an HTTP 401. - www_authenticate: Option>, + /// Headers received with an HTTP 401. + auth_error_headers: Vec, /// Disables status messages. quiet: bool, @@ -153,8 +153,8 @@ struct Headers { last_modified: Option, etag: Option, www_authenticate: Vec, - /// We don't care about these headers. Put them here for debugging purpose. - others: Vec, + /// All headers, including explicit headers above. + all: Vec, } /// HTTP status code [`HttpRegistry`] cares about. @@ -225,7 +225,7 @@ impl<'cfg> HttpRegistry<'cfg> { registry_config: None, auth_required: false, login_url: None, - www_authenticate: None, + auth_error_headers: vec![], quiet: false, }) } @@ -321,7 +321,7 @@ impl<'cfg> HttpRegistry<'cfg> { &mut handle, &url, data, - download.header_map.take().others, + download.header_map.take().all, ) .into()); } @@ -574,7 +574,7 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { } } } - self.www_authenticate = Some(result.header_map.www_authenticate); + self.auth_error_headers = result.header_map.all; } StatusCode::Unauthorized => { let err = Err(HttpNotSuccessful { @@ -582,7 +582,7 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { body: result.data, url: self.full_url(path), ip: None, - headers: result.header_map.others, + headers: result.header_map.all, } .into()); if self.auth_required { @@ -650,7 +650,7 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { &self.source_id, self.login_url.as_ref(), Operation::Read, - self.www_authenticate.clone(), + self.auth_error_headers.clone(), )?; headers.append(&format!("Authorization: {}", authorization))?; trace!("including authorization for {}", full_url); @@ -690,15 +690,12 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { tls::with(|downloads| { if let Some(downloads) = downloads { let mut header_map = downloads.pending[&token].0.header_map.borrow_mut(); + header_map.all.push(format!("{tag}: {value}")); match tag.to_ascii_lowercase().as_str() { LAST_MODIFIED => header_map.last_modified = Some(value.to_string()), ETAG => header_map.etag = Some(value.to_string()), WWW_AUTHENTICATE => header_map.www_authenticate.push(value.to_string()), - _ => { - if DEBUG_HEADERS.iter().any(|prefix| tag.starts_with(prefix)) { - header_map.others.push(format!("{tag}: {value}")); - } - } + _ => {} } } }); diff --git a/src/cargo/util/auth/mod.rs b/src/cargo/util/auth/mod.rs index 0d72b467c66..1e221a2c7b7 100644 --- a/src/cargo/util/auth/mod.rs +++ b/src/cargo/util/auth/mod.rs @@ -412,7 +412,7 @@ fn credential_action( config: &Config, sid: &SourceId, action: Action<'_>, - www_authenticate: Option>, + headers: Vec, ) -> CargoResult { let name = if sid.is_crates_io() { Some(CRATES_IO_REGISTRY) @@ -422,7 +422,7 @@ fn credential_action( let registry = RegistryInfo { index_url: sid.url().as_str(), name, - www_authenticate, + headers, }; let providers = credential_provider(config, sid)?; for provider in providers { @@ -473,9 +473,9 @@ pub fn auth_token( sid: &SourceId, login_url: Option<&Url>, operation: Operation<'_>, - www_authenticate: Option>, + headers: Vec, ) -> CargoResult { - match auth_token_optional(config, sid, operation, www_authenticate)? { + match auth_token_optional(config, sid, operation, headers)? { Some(token) => Ok(token.expose()), None => Err(AuthorizationError { sid: sid.clone(), @@ -492,7 +492,7 @@ fn auth_token_optional( config: &Config, sid: &SourceId, operation: Operation<'_>, - www_authenticate: Option>, + headers: Vec, ) -> CargoResult>> { log::trace!("token requested for {}", sid.display_registry_name()); let mut cache = config.credential_cache(); @@ -513,8 +513,7 @@ fn auth_token_optional( } } - let credential_response = - credential_action(config, sid, Action::Get(operation), www_authenticate); + let credential_response = credential_action(config, sid, Action::Get(operation), headers); if let Some(e) = credential_response.as_ref().err() { if let Some(e) = e.downcast_ref::() { if matches!(e, cargo_credential::Error::NotFound) { @@ -553,7 +552,7 @@ fn auth_token_optional( /// Log out from the given registry. pub fn logout(config: &Config, sid: &SourceId) -> CargoResult<()> { - let credential_response = credential_action(config, sid, Action::Logout, None); + let credential_response = credential_action(config, sid, Action::Logout, vec![]); if let Some(e) = credential_response.as_ref().err() { if let Some(e) = e.downcast_ref::() { if matches!(e, cargo_credential::Error::NotFound) { @@ -577,7 +576,7 @@ pub fn logout(config: &Config, sid: &SourceId) -> CargoResult<()> { /// Log in to the given registry. pub fn login(config: &Config, sid: &SourceId, options: LoginOptions<'_>) -> CargoResult<()> { - let credential_response = credential_action(config, sid, Action::Login(options), None)?; + let credential_response = credential_action(config, sid, Action::Login(options), vec![])?; let CredentialResponse::Login = credential_response else { bail!("credential provider produced unexpected response for `login` request: {credential_response:?}") }; diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index 5c7eebcdb71..91258c53c08 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -83,8 +83,19 @@ impl HttpNotSuccessful { } write!(result, ", got {}\n", self.code).unwrap(); if show_headers { - if !self.headers.is_empty() { - write!(result, "debug headers:\n{}\n", self.headers.join("\n")).unwrap(); + let headers: Vec<_> = self + .headers + .iter() + .filter(|header| { + let Some((name, _)) = header.split_once(":") else { return false }; + DEBUG_HEADERS.contains(&name.to_ascii_lowercase().trim()) + }) + .collect(); + if !headers.is_empty() { + writeln!(result, "debug headers:").unwrap(); + for header in headers { + writeln!(result, "{header}").unwrap(); + } } } write!(result, "body:\n{body}").unwrap(); diff --git a/tests/testsuite/credential_process.rs b/tests/testsuite/credential_process.rs index fbdf1967900..c04ddcf421d 100644 --- a/tests/testsuite/credential_process.rs +++ b/tests/testsuite/credential_process.rs @@ -83,11 +83,13 @@ fn get_token_test() -> (Project, TestRegistry) { )) .alternative() .http_api() + .http_index() + .auth_required() .build(); let provider = build_provider( "test-cred", - r#"{"Ok":{"kind":"get","token":"sekrit","cache":"session","operation_independent":true}}"#, + r#"{"Ok":{"kind":"get","token":"sekrit","cache":"session","operation_independent":false}}"#, ); let p = project() @@ -123,13 +125,14 @@ fn publish() { // Checks that credential-process is used for `cargo publish`. let (p, _t) = get_token_test(); - p.cargo("publish --no-verify --registry alternative -Z credential-process") + p.cargo("publish --no-verify --registry alternative -Z credential-process -Z registry-auth") .masquerade_as_nightly_cargo(&["credential-process"]) .with_stderr( r#"[UPDATING] [..] -{"v":1,"registry":{"index-url":"[..]","name":"alternative","www-authenticate":null},"kind":"get","operation":"read","args":[]} +{"v":1,"registry":{"index-url":"[..]","name":"alternative","headers":[..]},"kind":"get","operation":"read","args":[]} [PACKAGING] foo v0.1.0 [..] [PACKAGED] [..] +{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"publish","name":"foo","vers":"0.1.0","cksum":"[..]","args":[]} [UPLOADING] foo v0.1.0 [..] [UPLOADED] foo v0.1.0 [..] note: Waiting [..] @@ -190,7 +193,7 @@ fn login() { .replace_crates_io(registry.index_url()) .with_stderr( r#"[UPDATING] [..] -{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io","www-authenticate":null},"kind":"login","token":"abcdefg","login-url":"[..]","args":[]} +{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io"},"kind":"login","token":"abcdefg","login-url":"[..]","args":[]} "#, ) .run(); @@ -210,7 +213,7 @@ fn logout() { .masquerade_as_nightly_cargo(&["credential-process"]) .replace_crates_io(server.index_url()) .with_stderr( - r#"{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io","www-authenticate":null},"kind":"logout","args":[]} + r#"{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io"},"kind":"logout","args":[]} "#, ) .run(); @@ -220,11 +223,12 @@ fn logout() { fn yank() { let (p, _t) = get_token_test(); - p.cargo("yank --version 0.1.0 --registry alternative -Z credential-process") + p.cargo("yank --version 0.1.0 --registry alternative -Zcredential-process -Zregistry-auth") .masquerade_as_nightly_cargo(&["credential-process"]) .with_stderr( r#"[UPDATING] [..] -{"v":1,"registry":{"index-url":"[..]","name":"alternative","www-authenticate":null},"kind":"get","operation":"yank","name":"foo","vers":"0.1.0","args":[]} +{"v":1,"registry":{"index-url":"[..]","name":"alternative","headers":[..]},"kind":"get","operation":"read","args":[]} +{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"yank","name":"foo","vers":"0.1.0","args":[]} [YANK] foo@0.1.0 "#, ) @@ -235,11 +239,12 @@ fn yank() { fn owner() { let (p, _t) = get_token_test(); - p.cargo("owner --add username --registry alternative -Z credential-process") + p.cargo("owner --add username --registry alternative -Zcredential-process -Zregistry-auth") .masquerade_as_nightly_cargo(&["credential-process"]) .with_stderr( r#"[UPDATING] [..] -{"v":1,"registry":{"index-url":"[..]","name":"alternative","www-authenticate":null},"kind":"get","operation":"owners","name":"foo","args":[]} +{"v":1,"registry":{"index-url":"[..]","name":"alternative","headers":[..]},"kind":"get","operation":"read","args":[]} +{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"owners","name":"foo","args":[]} [OWNER] completed! "#, ) @@ -345,9 +350,9 @@ fn multiple_providers() { .with_stderr( r#"[UPDATING] [..] [CREDENTIAL] [..]url_not_supported[..] login crates-io -{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io","www-authenticate":null},"kind":"login","token":"abcdefg","login-url":"[..]","args":[]} +{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io"},"kind":"login","token":"abcdefg","login-url":"[..]","args":[]} [CREDENTIAL] [..]success_provider[..] login crates-io -{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io","www-authenticate":null},"kind":"login","token":"abcdefg","login-url":"[..]","args":[]} +{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io"},"kind":"login","token":"abcdefg","login-url":"[..]","args":[]} "#, ) .run(); @@ -423,6 +428,7 @@ fn token_caching() { )) .alternative() .http_api() + .http_index() .build(); // Token should not be re-used if it is expired @@ -464,10 +470,10 @@ fn token_caching() { .build(); let output = r#"[UPDATING] `alternative` index -{"v":1,"registry":{"index-url":"[..]","name":"alternative","www-authenticate":null},"kind":"get","operation":"read","args":[]} +{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"read","args":[]} [PACKAGING] foo v0.1.0 [..] [PACKAGED] [..] -{"v":1,"registry":{"index-url":"[..]","name":"alternative","www-authenticate":null},"kind":"get","operation":"publish","name":"foo","vers":"0.1.0","cksum":"[..]","args":[]} +{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"publish","name":"foo","vers":"0.1.0","cksum":"[..]","args":[]} [UPLOADING] foo v0.1.0 [..] [UPLOADED] foo v0.1.0 [..] note: Waiting [..]