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.

Sign up for GitHub

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

Jump to bottom

Validate registry token before operations that require it. #6854

Merged
merged 2 commits into from
Apr 17, 2019
Merged

Validate registry token before operations that require it. #6854

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d9bc8c0
Validate registry token before operations that require it.
Apr 15, 2019
f7c424f
Add check token before verify tests.
Apr 16, 2019
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
View file
Open in desktop
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
View file
Open in desktop
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