Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate registry token before operations that require it. #6854

Merged
merged 2 commits into from
Apr 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
opts.index.clone(),
opts.registry.clone(),
true,
!opts.dry_run
)?;
verify_dependencies(pkg, &registry, reg_id)?;

Expand Down Expand Up @@ -334,12 +335,13 @@ pub fn registry_configuration(
Ok(RegistryConfig { index, token })
}

pub fn registry(
fn registry(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fn registry isn't called outside this file and is already a bit gnarly and specific in its usage, so I thought it might be best made private.

config: &Config,
token: Option<String>,
index: Option<String>,
registry: Option<String>,
force_update: bool,
validate_token: bool
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the token validation to fn registry because it seemed useful for several callers, but also because once the token is wrapped in a Registry private field we can't read it without bulkier changes that didn't seem justified.

) -> CargoResult<(Registry, SourceId)> {
// Parse all configuration options
let RegistryConfig {
Expand All @@ -363,6 +365,9 @@ pub fn registry(
.ok_or_else(|| format_err!("{} does not support API commands", sid))?
};
let handle = http_handle(config)?;
if validate_token && token.is_none() {
bail!("no upload token found, please run `cargo login`");
};
Ok((Registry::new_handle(api_host, token, handle), sid))
}

Expand Down Expand Up @@ -536,7 +541,7 @@ pub fn registry_login(
token: Option<String>,
reg: Option<String>,
) -> CargoResult<()> {
let (registry, _) = registry(config, token.clone(), None, reg.clone(), false)?;
let (registry, _) = registry(config, token.clone(), None, reg.clone(), false, false)?;

let token = match token {
Some(token) => token,
Expand Down Expand Up @@ -604,6 +609,7 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> {
opts.index.clone(),
opts.registry.clone(),
true,
true
)?;

if let Some(ref v) = opts.to_add {
Expand Down Expand Up @@ -664,7 +670,7 @@ pub fn yank(
None => bail!("a version must be specified to yank"),
};

let (mut registry, _) = registry(config, token, index, reg, true)?;
let (mut registry, _) = registry(config, token, index, reg, true, true)?;

if undo {
config
Expand Down Expand Up @@ -720,7 +726,7 @@ pub fn search(
prefix
}

let (mut registry, source_id) = registry(config, None, index, reg, false)?;
let (mut registry, source_id) = registry(config, None, index, reg, false, false)?;
let (crates, total_crates) = registry
.search(query, limit)
.chain_err(|| "failed to retrieve search results from the registry")?;
Expand Down
3 changes: 3 additions & 0 deletions tests/testsuite/alt_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,9 @@ fn cannot_publish_to_crates_io_with_registry_dependency() {
)
.build();

// Login so that we have the token available
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test failed at an earlier point without access to the fake registry's token.

p.cargo("login --registry fakeio TOKEN").run();

p.cargo("publish --registry fakeio")
.with_status(101)
.with_stderr_contains("[ERROR] crates cannot be published to crates.io[..]")
Expand Down
36 changes: 36 additions & 0 deletions tests/testsuite/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -977,3 +977,39 @@ fn publish_with_patch() {
&["Cargo.toml", "Cargo.toml.orig", "src/main.rs"],
);
}

#[test]
fn publish_checks_for_token_before_verify() {
registry::init();

let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.0.1"
authors = []
license = "MIT"
description = "foo"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

let credentials = paths::home().join(".cargo/credentials");
fs::remove_file(&credentials).unwrap();

// Assert upload token error before the package is verified
p.cargo("publish")
.with_status(101)
.with_stderr_contains("[ERROR] no upload token found, please run `cargo login`")
.with_stderr_does_not_contain("[VERIFYING] foo v0.0.1 ([CWD])")
.run();

// Assert package verified successfully on dry run
p.cargo("publish --dry-run")
.with_status(0)
.with_stderr_contains("[VERIFYING] foo v0.0.1 ([CWD])")
.run();
}