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

add support for loading pypi cred from token and pypirc #374

Merged
merged 1 commit into from
Nov 10, 2020
Merged

add support for loading pypi cred from token and pypirc #374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
91 changes: 91 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,16 @@ toml = "0.5.6"
walkdir = "2.3.1"
zip = "0.5.5"
thiserror = "1.0.20"
dirs = { version = "3.0.1", optional = true }
configparser = { version = "1.0.0", optional = true }

[dev-dependencies]
indoc = "1.0.2"

[features]
default = ["auditwheel", "log", "upload", "rustls", "human-panic"]
auditwheel = []
upload = ["reqwest", "rpassword"]
upload = ["reqwest", "rpassword", "configparser", "dirs"]
password-storage = ["upload", "keyring"]
log = ["pretty_env_logger"]
# We use rustls for manylinux compliance and because unlike both dynamic and
Expand Down
48 changes: 45 additions & 3 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use anyhow::{bail, Context, Result};
#[cfg(feature = "upload")]
use bytesize::ByteSize;
use cargo_metadata::MetadataCommand;
#[cfg(feature = "upload")]
use configparser::ini::Ini;
#[cfg(feature = "human-panic")]
use human_panic::setup_panic;
#[cfg(feature = "password-storage")]
Expand Down Expand Up @@ -70,14 +72,54 @@ fn get_username() -> String {
}

#[cfg(feature = "upload")]
/// Asks for username and password for a registry account where missing.
fn complete_registry(opt: &PublishOpt) -> Result<(Registry, bool)> {
fn load_pypi_cred_from_config(package_name: &str) -> Option<(String, String)> {
if let Some(mut config_path) = dirs::home_dir() {
config_path.push(".pypirc");
if let Ok(pypirc) = fs::read_to_string(config_path.as_path()) {
let mut config = Ini::new();
if config.read(pypirc).is_err() {
return None;
}

if let (Some(username), Some(password)) = (
config.get(package_name, "username"),
Copy link
Member

Choose a reason for hiding this comment

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

@houqp Sorry for asking so late, but shouldn't this be the registry name rather than the package name?

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 is because pypi tokens can be scoped to packages. I was expecting users to set token config per package. perhaps we should fallback to the registry when per package config is not found?

Copy link
Member

Choose a reason for hiding this comment

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

Do you have an example file? I've read https://packaging.python.org/specifications/pypirc/ and I only see how you can set username/password per repository, but I haven't found a way to scope this per package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha, looking at the doc again, maybe i misused the config section name. They way i have it configured locally is to set the section name as package name:

[package-name]
repository = https://upload.pypi.org/legacy/
username = __token__
password = API_TOKEN

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 think the correct behavior here should be just picking the first repo from index-servers key instead of using package name?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I understood the header in square brackets as the name of the repository rather than name of the package, e.g. you could do twine upload -r first-repository myproject/dist/* and twine upload -r first-repository myproject/dist/*, given the example from the docs:

[distutils]
index-servers =
    first-repository
    second-repository

[first-repository]
repository = <first-repository URL>
username = <first-repository username>
password = <first-repository password>

[second-repository]
repository = <second-repository URL>
username = <second-repository username>
password = <second-repository password>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, based on the doc, section names are supposed to be mapped to repo name. although i think this is a design issue with pypi, which makes it hard to use package scoped tokens for security best practices, it's better to stick to the official spec to avoid surprises for now.

config.get(package_name, "password"),
) {
return Some((username, password));
}
}
}

None
}

#[cfg(feature = "upload")]
fn resolve_pypi_cred(opt: &PublishOpt, package_name: &str) -> (String, String, bool) {
// API token from environment variable takes priority
if let Ok(token) = env::var("MATURIN_PYPI_TOKEN") {
return ("__token__".to_string(), token, false);
}

// load creds from pypirc if found
if let Some((username, password)) = load_pypi_cred_from_config(package_name) {
println!("🔐 Using credential in pypirc for upload");
return (username, password, false);
}

// fallback to username and password
let username = opt.username.clone().unwrap_or_else(get_username);
let (password, reenter) = match opt.password {
Some(ref password) => (password.clone(), false),
None => get_password(&username),
};

(username, password, reenter)
}

#[cfg(feature = "upload")]
/// Asks for username and password for a registry account where missing.
fn complete_registry(opt: &PublishOpt, package_name: &str) -> Result<(Registry, bool)> {
let (username, password, reenter) = resolve_pypi_cred(opt, package_name);
let registry = Registry::new(username, password, Url::parse(&opt.registry)?);

Ok((registry, reenter))
Expand Down Expand Up @@ -321,7 +363,7 @@ fn upload_ui(build: BuildOptions, publish: &PublishOpt, no_sdist: bool) -> Resul
}
}

let (mut registry, reenter) = complete_registry(&publish)?;
let (mut registry, reenter) = complete_registry(&publish, &build_context.metadata21.name)?;

loop {
println!("🚀 Uploading {} packages", wheels.len());
Expand Down