Skip to content

Commit 6151a41

Browse files
arlosiarlosi
committed
Send all headers to cred provider
1 parent 2b39792 commit 6151a41

File tree

9 files changed

+60
-48
lines changed

9 files changed

+60
-48
lines changed

‎credential/cargo-credential/src/lib.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,9 @@ pub struct RegistryInfo<'a> {
6262
/// Name of the registry in configuration. May not be available.
6363
/// The crates.io registry will be `crates-io` (`CRATES_IO_REGISTRY`).
6464
pub name: Option<&'a str>,
65-
/// www-authenticate headers from attempting to access a sparse registry.
66-
pub www_authenticate: Option<Vec<String>>,
65+
/// Headers from attempting to access a registry that resulted in a HTTP 401.
66+
#[serde(skip_serializing_if = "Vec::is_empty")]
67+
pub headers: Vec<String>,
6768
}
6869

6970
#[derive(Serialize, Deserialize, Clone, Debug)]

‎src/cargo/core/package.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use crate::core::source::MaybePackage;
2525
use crate::core::{Dependency, Manifest, PackageId, SourceId, Target};
2626
use crate::core::{SourceMap, Summary, Workspace};
2727
use crate::util::config::PackageCacheLock;
28-
use crate::util::errors::{CargoResult, HttpNotSuccessful, DEBUG_HEADERS};
28+
use crate::util::errors::{CargoResult, HttpNotSuccessful};
2929
use crate::util::interning::InternedString;
3030
use crate::util::network::http::http_handle_and_timeout;
3131
use crate::util::network::http::HttpTimeout;
@@ -748,9 +748,7 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
748748
// Headers contain trailing \r\n, trim them to make it easier
749749
// to work with.
750750
let h = String::from_utf8_lossy(data).trim().to_string();
751-
if DEBUG_HEADERS.iter().any(|p| h.starts_with(p)) {
752-
downloads.pending[&token].0.headers.borrow_mut().push(h);
753-
}
751+
downloads.pending[&token].0.headers.borrow_mut().push(h);
754752
}
755753
});
756754
true

‎src/cargo/ops/registry/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ fn registry(
144144
&source_ids.original,
145145
None,
146146
operation,
147-
None,
147+
vec![],
148148
)?)
149149
} else {
150150
None

‎src/cargo/ops/registry/publish.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
160160
&reg_ids.original,
161161
None,
162162
operation,
163-
None,
163+
vec![],
164164
)?));
165165
}
166166

‎src/cargo/sources/registry/download.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ pub(super) fn download(
8484
&pkg.source_id(),
8585
None,
8686
Operation::Read,
87-
None,
87+
vec![],
8888
)?)
8989
} else {
9090
None

‎src/cargo/sources/registry/http_remote.rs

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::core::{PackageId, SourceId};
44
use crate::sources::registry::download;
55
use crate::sources::registry::MaybeLock;
66
use crate::sources::registry::{LoadResponse, RegistryConfig, RegistryData};
7-
use crate::util::errors::{CargoResult, HttpNotSuccessful, DEBUG_HEADERS};
7+
use crate::util::errors::{CargoResult, HttpNotSuccessful};
88
use crate::util::network::http::http_handle;
99
use crate::util::network::retry::{Retry, RetryResult};
1010
use crate::util::network::sleep::SleepTracker;
@@ -97,8 +97,8 @@ pub struct HttpRegistry<'cfg> {
9797
/// Url to get a token for the registry.
9898
login_url: Option<Url>,
9999

100-
/// WWW-Authenticate header received with an HTTP 401.
101-
www_authenticate: Option<Vec<String>>,
100+
/// Headers received with an HTTP 401.
101+
auth_error_headers: Vec<String>,
102102

103103
/// Disables status messages.
104104
quiet: bool,
@@ -153,8 +153,8 @@ struct Headers {
153153
last_modified: Option<String>,
154154
etag: Option<String>,
155155
www_authenticate: Vec<String>,
156-
/// We don't care about these headers. Put them here for debugging purpose.
157-
others: Vec<String>,
156+
/// All headers, including explicit headers above.
157+
all: Vec<String>,
158158
}
159159

160160
/// HTTP status code [`HttpRegistry`] cares about.
@@ -225,7 +225,7 @@ impl<'cfg> HttpRegistry<'cfg> {
225225
registry_config: None,
226226
auth_required: false,
227227
login_url: None,
228-
www_authenticate: None,
228+
auth_error_headers: vec![],
229229
quiet: false,
230230
})
231231
}
@@ -321,7 +321,7 @@ impl<'cfg> HttpRegistry<'cfg> {
321321
&mut handle,
322322
&url,
323323
data,
324-
download.header_map.take().others,
324+
download.header_map.take().all,
325325
)
326326
.into());
327327
}
@@ -574,15 +574,15 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
574574
}
575575
}
576576
}
577-
self.www_authenticate = Some(result.header_map.www_authenticate);
577+
self.auth_error_headers = result.header_map.all;
578578
}
579579
StatusCode::Unauthorized => {
580580
let err = Err(HttpNotSuccessful {
581581
code: 401,
582582
body: result.data,
583583
url: self.full_url(path),
584584
ip: None,
585-
headers: result.header_map.others,
585+
headers: result.header_map.all,
586586
}
587587
.into());
588588
if self.auth_required {
@@ -650,7 +650,7 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
650650
&self.source_id,
651651
self.login_url.as_ref(),
652652
Operation::Read,
653-
self.www_authenticate.clone(),
653+
self.auth_error_headers.clone(),
654654
)?;
655655
headers.append(&format!("Authorization: {}", authorization))?;
656656
trace!("including authorization for {}", full_url);
@@ -690,15 +690,12 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
690690
tls::with(|downloads| {
691691
if let Some(downloads) = downloads {
692692
let mut header_map = downloads.pending[&token].0.header_map.borrow_mut();
693+
header_map.all.push(format!("{tag}: {value}"));
693694
match tag.to_ascii_lowercase().as_str() {
694695
LAST_MODIFIED => header_map.last_modified = Some(value.to_string()),
695696
ETAG => header_map.etag = Some(value.to_string()),
696697
WWW_AUTHENTICATE => header_map.www_authenticate.push(value.to_string()),
697-
_ => {
698-
if DEBUG_HEADERS.iter().any(|prefix| tag.starts_with(prefix)) {
699-
header_map.others.push(format!("{tag}: {value}"));
700-
}
701-
}
698+
_ => {}
702699
}
703700
}
704701
});

‎src/cargo/util/auth/mod.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ fn credential_action(
412412
config: &Config,
413413
sid: &SourceId,
414414
action: Action<'_>,
415-
www_authenticate: Option<Vec<String>>,
415+
headers: Vec<String>,
416416
) -> CargoResult<CredentialResponse> {
417417
let name = if sid.is_crates_io() {
418418
Some(CRATES_IO_REGISTRY)
@@ -422,7 +422,7 @@ fn credential_action(
422422
let registry = RegistryInfo {
423423
index_url: sid.url().as_str(),
424424
name,
425-
www_authenticate,
425+
headers,
426426
};
427427
let providers = credential_provider(config, sid)?;
428428
for provider in providers {
@@ -473,9 +473,9 @@ pub fn auth_token(
473473
sid: &SourceId,
474474
login_url: Option<&Url>,
475475
operation: Operation<'_>,
476-
www_authenticate: Option<Vec<String>>,
476+
headers: Vec<String>,
477477
) -> CargoResult<String> {
478-
match auth_token_optional(config, sid, operation, www_authenticate)? {
478+
match auth_token_optional(config, sid, operation, headers)? {
479479
Some(token) => Ok(token.expose()),
480480
None => Err(AuthorizationError {
481481
sid: sid.clone(),
@@ -492,7 +492,7 @@ fn auth_token_optional(
492492
config: &Config,
493493
sid: &SourceId,
494494
operation: Operation<'_>,
495-
www_authenticate: Option<Vec<String>>,
495+
headers: Vec<String>,
496496
) -> CargoResult<Option<Secret<String>>> {
497497
log::trace!("token requested for {}", sid.display_registry_name());
498498
let mut cache = config.credential_cache();
@@ -513,8 +513,7 @@ fn auth_token_optional(
513513
}
514514
}
515515

516-
let credential_response =
517-
credential_action(config, sid, Action::Get(operation), www_authenticate);
516+
let credential_response = credential_action(config, sid, Action::Get(operation), headers);
518517
if let Some(e) = credential_response.as_ref().err() {
519518
if let Some(e) = e.downcast_ref::<cargo_credential::Error>() {
520519
if matches!(e, cargo_credential::Error::NotFound) {
@@ -553,7 +552,7 @@ fn auth_token_optional(
553552

554553
/// Log out from the given registry.
555554
pub fn logout(config: &Config, sid: &SourceId) -> CargoResult<()> {
556-
let credential_response = credential_action(config, sid, Action::Logout, None);
555+
let credential_response = credential_action(config, sid, Action::Logout, vec![]);
557556
if let Some(e) = credential_response.as_ref().err() {
558557
if let Some(e) = e.downcast_ref::<cargo_credential::Error>() {
559558
if matches!(e, cargo_credential::Error::NotFound) {
@@ -577,7 +576,7 @@ pub fn logout(config: &Config, sid: &SourceId) -> CargoResult<()> {
577576

578577
/// Log in to the given registry.
579578
pub fn login(config: &Config, sid: &SourceId, options: LoginOptions<'_>) -> CargoResult<()> {
580-
let credential_response = credential_action(config, sid, Action::Login(options), None)?;
579+
let credential_response = credential_action(config, sid, Action::Login(options), vec![])?;
581580
let CredentialResponse::Login = credential_response else {
582581
bail!("credential provider produced unexpected response for `login` request: {credential_response:?}")
583582
};

‎src/cargo/util/errors.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,19 @@ impl HttpNotSuccessful {
8383
}
8484
write!(result, ", got {}\n", self.code).unwrap();
8585
if show_headers {
86-
if !self.headers.is_empty() {
87-
write!(result, "debug headers:\n{}\n", self.headers.join("\n")).unwrap();
86+
let headers: Vec<_> = self
87+
.headers
88+
.iter()
89+
.filter(|header| {
90+
let Some((name, _)) = header.split_once(":") else { return false };
91+
DEBUG_HEADERS.contains(&name.to_ascii_lowercase().trim())
92+
})
93+
.collect();
94+
if !headers.is_empty() {
95+
writeln!(result, "debug headers:").unwrap();
96+
for header in headers {
97+
writeln!(result, "{header}").unwrap();
98+
}
8899
}
89100
}
90101
write!(result, "body:\n{body}").unwrap();

‎tests/testsuite/credential_process.rs

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,13 @@ fn get_token_test() -> (Project, TestRegistry) {
8383
))
8484
.alternative()
8585
.http_api()
86+
.http_index()
87+
.auth_required()
8688
.build();
8789

8890
let provider = build_provider(
8991
"test-cred",
90-
r#"{"Ok":{"kind":"get","token":"sekrit","cache":"session","operation_independent":true}}"#,
92+
r#"{"Ok":{"kind":"get","token":"sekrit","cache":"session","operation_independent":false}}"#,
9193
);
9294

9395
let p = project()
@@ -123,13 +125,14 @@ fn publish() {
123125
// Checks that credential-process is used for `cargo publish`.
124126
let (p, _t) = get_token_test();
125127

126-
p.cargo("publish --no-verify --registry alternative -Z credential-process")
128+
p.cargo("publish --no-verify --registry alternative -Z credential-process -Z registry-auth")
127129
.masquerade_as_nightly_cargo(&["credential-process"])
128130
.with_stderr(
129131
r#"[UPDATING] [..]
130-
{"v":1,"registry":{"index-url":"[..]","name":"alternative","www-authenticate":null},"kind":"get","operation":"read","args":[]}
132+
{"v":1,"registry":{"index-url":"[..]","name":"alternative","headers":[..]},"kind":"get","operation":"read","args":[]}
131133
[PACKAGING] foo v0.1.0 [..]
132134
[PACKAGED] [..]
135+
{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"publish","name":"foo","vers":"0.1.0","cksum":"[..]","args":[]}
133136
[UPLOADING] foo v0.1.0 [..]
134137
[UPLOADED] foo v0.1.0 [..]
135138
note: Waiting [..]
@@ -190,7 +193,7 @@ fn login() {
190193
.replace_crates_io(registry.index_url())
191194
.with_stderr(
192195
r#"[UPDATING] [..]
193-
{"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":[]}
196+
{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io"},"kind":"login","token":"abcdefg","login-url":"[..]","args":[]}
194197
"#,
195198
)
196199
.run();
@@ -210,7 +213,7 @@ fn logout() {
210213
.masquerade_as_nightly_cargo(&["credential-process"])
211214
.replace_crates_io(server.index_url())
212215
.with_stderr(
213-
r#"{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io","www-authenticate":null},"kind":"logout","args":[]}
216+
r#"{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io"},"kind":"logout","args":[]}
214217
"#,
215218
)
216219
.run();
@@ -220,11 +223,12 @@ fn logout() {
220223
fn yank() {
221224
let (p, _t) = get_token_test();
222225

223-
p.cargo("yank --version 0.1.0 --registry alternative -Z credential-process")
226+
p.cargo("yank --version 0.1.0 --registry alternative -Zcredential-process -Zregistry-auth")
224227
.masquerade_as_nightly_cargo(&["credential-process"])
225228
.with_stderr(
226229
r#"[UPDATING] [..]
227-
{"v":1,"registry":{"index-url":"[..]","name":"alternative","www-authenticate":null},"kind":"get","operation":"yank","name":"foo","vers":"0.1.0","args":[]}
230+
{"v":1,"registry":{"index-url":"[..]","name":"alternative","headers":[..]},"kind":"get","operation":"read","args":[]}
231+
{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"yank","name":"foo","vers":"0.1.0","args":[]}
228232
[YANK] [email protected]
229233
"#,
230234
)
@@ -235,11 +239,12 @@ fn yank() {
235239
fn owner() {
236240
let (p, _t) = get_token_test();
237241

238-
p.cargo("owner --add username --registry alternative -Z credential-process")
242+
p.cargo("owner --add username --registry alternative -Zcredential-process -Zregistry-auth")
239243
.masquerade_as_nightly_cargo(&["credential-process"])
240244
.with_stderr(
241245
r#"[UPDATING] [..]
242-
{"v":1,"registry":{"index-url":"[..]","name":"alternative","www-authenticate":null},"kind":"get","operation":"owners","name":"foo","args":[]}
246+
{"v":1,"registry":{"index-url":"[..]","name":"alternative","headers":[..]},"kind":"get","operation":"read","args":[]}
247+
{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"owners","name":"foo","args":[]}
243248
[OWNER] completed!
244249
"#,
245250
)
@@ -345,9 +350,9 @@ fn multiple_providers() {
345350
.with_stderr(
346351
r#"[UPDATING] [..]
347352
[CREDENTIAL] [..]url_not_supported[..] login crates-io
348-
{"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":[]}
353+
{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io"},"kind":"login","token":"abcdefg","login-url":"[..]","args":[]}
349354
[CREDENTIAL] [..]success_provider[..] login crates-io
350-
{"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":[]}
355+
{"v":1,"registry":{"index-url":"https://github.com/rust-lang/crates.io-index","name":"crates-io"},"kind":"login","token":"abcdefg","login-url":"[..]","args":[]}
351356
"#,
352357
)
353358
.run();
@@ -423,6 +428,7 @@ fn token_caching() {
423428
))
424429
.alternative()
425430
.http_api()
431+
.http_index()
426432
.build();
427433

428434
// Token should not be re-used if it is expired
@@ -464,10 +470,10 @@ fn token_caching() {
464470
.build();
465471

466472
let output = r#"[UPDATING] `alternative` index
467-
{"v":1,"registry":{"index-url":"[..]","name":"alternative","www-authenticate":null},"kind":"get","operation":"read","args":[]}
473+
{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"read","args":[]}
468474
[PACKAGING] foo v0.1.0 [..]
469475
[PACKAGED] [..]
470-
{"v":1,"registry":{"index-url":"[..]","name":"alternative","www-authenticate":null},"kind":"get","operation":"publish","name":"foo","vers":"0.1.0","cksum":"[..]","args":[]}
476+
{"v":1,"registry":{"index-url":"[..]","name":"alternative"},"kind":"get","operation":"publish","name":"foo","vers":"0.1.0","cksum":"[..]","args":[]}
471477
[UPLOADING] foo v0.1.0 [..]
472478
[UPLOADED] foo v0.1.0 [..]
473479
note: Waiting [..]

0 commit comments

Comments
 (0)