Skip to content

Commit

Permalink
Auto merge of #11545 - kylematsuda:secret-type, r=Eh2406
Browse files Browse the repository at this point in the history
Wrapper type for data that should never be logged

Fixes #11519.

So far this is just creating the new wrapper type. If this looks okay, I'll start adding this wrapper in places where tokens and secret keys are held or passed.
  • Loading branch information
bors committed Jan 20, 2023
2 parents d73b935 + efb972a commit ba6217e
Show file tree
Hide file tree
Showing 7 changed files with 191 additions and 77 deletions.
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub fn cli() -> Command {
pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
ops::registry_login(
config,
args.get_one("token").map(String::as_str),
args.get_one::<String>("token").map(|s| s.as_str().into()),
args.get_one("registry").map(String::as_str),
args.flag("generate-keypair"),
args.flag("secret-key"),
Expand Down
3 changes: 2 additions & 1 deletion src/bin/cargo/commands/owner.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::command_prelude::*;

use cargo::ops::{self, OwnersOptions};
use cargo::util::auth::Secret;

pub fn cli() -> Command {
subcommand("owner")
Expand Down Expand Up @@ -34,7 +35,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
let registry = args.registry(config)?;
let opts = OwnersOptions {
krate: args.get_one::<String>("crate").cloned(),
token: args.get_one::<String>("token").cloned(),
token: args.get_one::<String>("token").cloned().map(Secret::from),
index: args.get_one::<String>("index").cloned(),
to_add: args
.get_many::<String>("add")
Expand Down
4 changes: 3 additions & 1 deletion src/bin/cargo/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
&ws,
&PublishOpts {
config,
token: args.get_one::<String>("token").map(|s| s.to_string()),
token: args
.get_one::<String>("token")
.map(|s| s.to_string().into()),
index,
verify: !args.flag("no-verify"),
allow_dirty: args.flag("allow-dirty"),
Expand Down
3 changes: 2 additions & 1 deletion src/bin/cargo/commands/yank.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::command_prelude::*;

use cargo::ops;
use cargo::util::auth::Secret;

pub fn cli() -> Command {
subcommand("yank")
Expand Down Expand Up @@ -37,7 +38,7 @@ pub fn exec(config: &mut Config, args: &ArgMatches) -> CliResult {
config,
krate.map(|s| s.to_string()),
version.map(|s| s.to_string()),
args.get_one::<String>("token").cloned(),
args.get_one::<String>("token").cloned().map(Secret::from),
args.get_one::<String>("index").cloned(),
args.flag("undo"),
registry,
Expand Down
62 changes: 33 additions & 29 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use crate::ops;
use crate::ops::Packages;
use crate::sources::{RegistrySource, SourceConfigMap, CRATES_IO_DOMAIN, CRATES_IO_REGISTRY};
use crate::util::auth::{
paserk_public_from_paserk_secret, {self, AuthorizationError},
paserk_public_from_paserk_secret, Secret, {self, AuthorizationError},
};
use crate::util::config::{Config, SslVersionConfig, SslVersionConfigRange};
use crate::util::errors::CargoResult;
Expand All @@ -45,11 +45,11 @@ use crate::{drop_print, drop_println, version};
pub enum RegistryCredentialConfig {
None,
/// The authentication token.
Token(String),
Token(Secret<String>),
/// Process used for fetching a token.
Process((PathBuf, Vec<String>)),
/// Secret Key and subject for Asymmetric tokens.
AsymmetricKey((String, Option<String>)),
AsymmetricKey((Secret<String>, Option<String>)),
}

impl RegistryCredentialConfig {
Expand All @@ -71,9 +71,9 @@ impl RegistryCredentialConfig {
pub fn is_asymmetric_key(&self) -> bool {
matches!(self, Self::AsymmetricKey(..))
}
pub fn as_token(&self) -> Option<&str> {
pub fn as_token(&self) -> Option<Secret<&str>> {
if let Self::Token(v) = self {
Some(&*v)
Some(v.as_deref())
} else {
None
}
Expand All @@ -85,7 +85,7 @@ impl RegistryCredentialConfig {
None
}
}
pub fn as_asymmetric_key(&self) -> Option<&(String, Option<String>)> {
pub fn as_asymmetric_key(&self) -> Option<&(Secret<String>, Option<String>)> {
if let Self::AsymmetricKey(v) = self {
Some(v)
} else {
Expand All @@ -96,7 +96,7 @@ impl RegistryCredentialConfig {

pub struct PublishOpts<'cfg> {
pub config: &'cfg Config,
pub token: Option<String>,
pub token: Option<Secret<String>>,
pub index: Option<String>,
pub verify: bool,
pub allow_dirty: bool,
Expand Down Expand Up @@ -174,7 +174,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {

let (mut registry, reg_ids) = registry(
opts.config,
opts.token.as_deref(),
opts.token.as_ref().map(Secret::as_deref),
opts.index.as_deref(),
publish_registry.as_deref(),
true,
Expand Down Expand Up @@ -512,7 +512,7 @@ fn wait_for_publish(
/// * `token_required`: If `true`, the token will be set.
fn registry(
config: &Config,
token_from_cmdline: Option<&str>,
token_from_cmdline: Option<Secret<&str>>,
index: Option<&str>,
registry: Option<&str>,
force_update: bool,
Expand Down Expand Up @@ -786,7 +786,7 @@ fn http_proxy_exists(config: &Config) -> CargoResult<bool> {

pub fn registry_login(
config: &Config,
token: Option<&str>,
token: Option<Secret<&str>>,
reg: Option<&str>,
generate_keypair: bool,
secret_key_required: bool,
Expand All @@ -795,7 +795,7 @@ pub fn registry_login(
let source_ids = get_source_id(config, None, reg)?;
let reg_cfg = auth::registry_credential_config(config, &source_ids.original)?;

let login_url = match registry(config, token, None, reg, false, None) {
let login_url = match registry(config, token.clone(), None, reg, false, None) {
Ok((registry, _)) => Some(format!("{}/me", registry.host())),
Err(e) if e.is::<AuthorizationError>() => e
.downcast::<AuthorizationError>()
Expand Down Expand Up @@ -830,29 +830,33 @@ pub fn registry_login(
}
_ => (None, None),
};
let secret_key: String;
let secret_key: Secret<String>;
if generate_keypair {
assert!(!secret_key_required);
let kp = AsymmetricKeyPair::<pasetors::version3::V3>::generate().unwrap();
let mut key = String::new();
FormatAsPaserk::fmt(&kp.secret, &mut key).unwrap();
secret_key = key;
secret_key = Secret::default().map(|mut key| {
FormatAsPaserk::fmt(&kp.secret, &mut key).unwrap();
key
});
} else if secret_key_required {
assert!(!generate_keypair);
drop_println!(config, "please paste the API secret key below");
let mut line = String::new();
let input = io::stdin();
input
.lock()
.read_line(&mut line)
.with_context(|| "failed to read stdin")?;
secret_key = line.trim().to_string();
secret_key = Secret::default()
.map(|mut line| {
let input = io::stdin();
input
.lock()
.read_line(&mut line)
.with_context(|| "failed to read stdin")
.map(|_| line.trim().to_string())
})
.transpose()?;
} else {
secret_key = old_secret_key
.cloned()
.ok_or_else(|| anyhow!("need a secret_key to set a key_subject"))?;
}
if let Some(p) = paserk_public_from_paserk_secret(&secret_key) {
if let Some(p) = paserk_public_from_paserk_secret(secret_key.as_deref()) {
drop_println!(config, "{}", &p);
} else {
bail!("not a validly formatted PASERK secret key");
Expand All @@ -866,7 +870,7 @@ pub fn registry_login(
));
} else {
new_token = RegistryCredentialConfig::Token(match token {
Some(token) => token.to_string(),
Some(token) => token.owned(),
None => {
if let Some(login_url) = login_url {
drop_println!(
Expand All @@ -890,7 +894,7 @@ pub fn registry_login(
.with_context(|| "failed to read stdin")?;
// Automatically remove `cargo login` from an inputted token to
// allow direct pastes from `registry.host()`/me.
line.replace("cargo login", "").trim().to_string()
Secret::from(line.replace("cargo login", "").trim().to_string())
}
});

Expand Down Expand Up @@ -938,7 +942,7 @@ pub fn registry_logout(config: &Config, reg: Option<&str>) -> CargoResult<()> {

pub struct OwnersOptions {
pub krate: Option<String>,
pub token: Option<String>,
pub token: Option<Secret<String>>,
pub index: Option<String>,
pub to_add: Option<Vec<String>>,
pub to_remove: Option<Vec<String>>,
Expand All @@ -960,7 +964,7 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> {

let (mut registry, _) = registry(
config,
opts.token.as_deref(),
opts.token.as_ref().map(Secret::as_deref),
opts.index.as_deref(),
opts.registry.as_deref(),
true,
Expand Down Expand Up @@ -1019,7 +1023,7 @@ pub fn yank(
config: &Config,
krate: Option<String>,
version: Option<String>,
token: Option<String>,
token: Option<Secret<String>>,
index: Option<String>,
undo: bool,
reg: Option<String>,
Expand Down Expand Up @@ -1051,7 +1055,7 @@ pub fn yank(

let (mut registry, _) = registry(
config,
token.as_deref(),
token.as_ref().map(Secret::as_deref),
index.as_deref(),
reg.as_deref(),
true,
Expand Down
Loading

0 comments on commit ba6217e

Please sign in to comment.