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

fix(credential)!: Fallback when an auth method isn't available on the current machine #13558

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
11 changes: 5 additions & 6 deletions Cargo.lock

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

9 changes: 4 additions & 5 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ anyhow = "1.0.80"
base64 = "0.21.7"
bytesize = "1.3"
cargo = { path = "" }
cargo-credential = { version = "0.4.2", path = "credential/cargo-credential" }
cargo-credential-libsecret = { version = "0.4.2", path = "credential/cargo-credential-libsecret" }
cargo-credential-macos-keychain = { version = "0.4.2", path = "credential/cargo-credential-macos-keychain" }
cargo-credential-wincred = { version = "0.4.2", path = "credential/cargo-credential-wincred" }
cargo-credential = { version = "0.5.0", path = "credential/cargo-credential" }
cargo-credential-libsecret = { version = "0.5.0", path = "credential/cargo-credential-libsecret" }
cargo-credential-macos-keychain = { version = "0.5.0", path = "credential/cargo-credential-macos-keychain" }
cargo-credential-wincred = { version = "0.5.0", path = "credential/cargo-credential-wincred" }
cargo-platform = { path = "crates/cargo-platform", version = "0.1.5" }
cargo-test-macro = { path = "crates/cargo-test-macro" }
cargo-test-support = { path = "crates/cargo-test-support" }
Expand Down Expand Up @@ -72,7 +72,6 @@ os_info = "3.7.0"
pasetors = { version = "0.6.8", features = ["v3", "paserk", "std", "serde"] }
pathdiff = "0.2"
percent-encoding = "2.3"
pkg-config = "0.3.30"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see this referenced anywhere else in the change. Is it just an unneeded dependency?

proptest = "1.4.0"
pulldown-cmark = { version = "0.10.0", default-features = false, features = ["html"] }
rand = "0.8.5"
Expand Down
2 changes: 1 addition & 1 deletion credential/cargo-credential-1password/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-credential-1password"
version = "0.4.4"
version = "0.5.0"
rust-version.workspace = true
edition.workspace = true
license.workspace = true
Expand Down
8 changes: 4 additions & 4 deletions credential/cargo-credential-1password/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#![allow(clippy::print_stderr)]

use cargo_credential::{
Action, CacheControl, Credential, CredentialResponse, Error, RegistryInfo, Secret,
Action, CacheControl, Credential, CredentialResponse, Error, ErrorKind, RegistryInfo, Secret,
};
use serde::Deserialize;
use std::io::Read;
Expand Down Expand Up @@ -278,7 +278,7 @@ impl Credential for OnePasswordCredential {
operation_independent: true,
})
} else {
Err(Error::NotFound)
Err(ErrorKind::NotFound.into())
}
}
Action::Login(options) => {
Expand All @@ -301,10 +301,10 @@ impl Credential for OnePasswordCredential {
op.delete(&session, &id)?;
Ok(CredentialResponse::Logout)
} else {
Err(Error::NotFound)
Err(ErrorKind::NotFound.into())
}
}
_ => Err(Error::OperationNotSupported),
_ => Err(ErrorKind::OperationNotSupported.into()),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion credential/cargo-credential-libsecret/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-credential-libsecret"
version = "0.4.4"
version = "0.5.0"
rust-version = "1.76.0" # MSRV:1
edition.workspace = true
license.workspace = true
Expand Down
16 changes: 9 additions & 7 deletions credential/cargo-credential-libsecret/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ mod linux {

use anyhow::Context;
use cargo_credential::{
read_token, Action, CacheControl, Credential, CredentialResponse, Error, RegistryInfo,
Secret,
read_token, Action, CacheControl, Credential, CredentialResponse, Error, ErrorKind,
RegistryInfo, Secret,
};
use libloading::{Library, Symbol};
use std::ffi::{CStr, CString};
Expand Down Expand Up @@ -115,10 +115,12 @@ mod linux {
let secret_password_store_sync: Symbol<'_, SecretPasswordStoreSync>;
let secret_password_clear_sync: Symbol<'_, SecretPasswordClearSync>;
unsafe {
lib = Library::new("libsecret-1.so").context(
"failed to load libsecret: try installing the `libsecret` \
lib = Library::new("libsecret-1.so")
.context(
"failed to load libsecret: try installing the `libsecret` \
or `libsecret-1-0` package with the system package manager",
)?;
)
.map_err(|err| Error::from(err).with_kind(ErrorKind::UrlNotSupported))?;
secret_password_lookup_sync = lib
.get(b"secret_password_lookup_sync\0")
.map_err(Box::new)?;
Expand Down Expand Up @@ -153,7 +155,7 @@ mod linux {
.into());
}
if token_c.is_null() {
return Err(Error::NotFound);
return Err(ErrorKind::NotFound.into());
}
let token = Secret::from(
CStr::from_ptr(token_c)
Expand Down Expand Up @@ -223,7 +225,7 @@ mod linux {
}
Ok(CredentialResponse::Logout)
}
_ => Err(Error::OperationNotSupported),
_ => Err(ErrorKind::OperationNotSupported.into()),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion credential/cargo-credential-macos-keychain/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-credential-macos-keychain"
version = "0.4.4"
version = "0.5.0"
rust-version = "1.76.0" # MSRV:1
edition.workspace = true
license.workspace = true
Expand Down
9 changes: 5 additions & 4 deletions credential/cargo-credential-macos-keychain/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
#[cfg(target_os = "macos")]
mod macos {
use cargo_credential::{
read_token, Action, CacheControl, Credential, CredentialResponse, Error, RegistryInfo,
read_token, Action, CacheControl, Credential, CredentialResponse, Error, ErrorKind,
RegistryInfo,
};
use security_framework::os::macos::keychain::SecKeychain;

Expand All @@ -31,7 +32,7 @@ mod macos {
let not_found = security_framework::base::Error::from(NOT_FOUND).code();
match action {
Action::Get(_) => match keychain.find_generic_password(&service_name, ACCOUNT) {
Err(e) if e.code() == not_found => Err(Error::NotFound),
Err(e) if e.code() == not_found => Err(ErrorKind::NotFound.into()),
Err(e) => Err(Box::new(e).into()),
Ok((pass, _)) => {
let token = String::from_utf8(pass.as_ref().to_vec()).map_err(Box::new)?;
Expand Down Expand Up @@ -64,14 +65,14 @@ mod macos {
Ok(CredentialResponse::Login)
}
Action::Logout => match keychain.find_generic_password(&service_name, ACCOUNT) {
Err(e) if e.code() == not_found => Err(Error::NotFound),
Err(e) if e.code() == not_found => Err(ErrorKind::NotFound.into()),
Err(e) => Err(Box::new(e).into()),
Ok((_, item)) => {
item.delete();
Ok(CredentialResponse::Logout)
}
},
_ => Err(Error::OperationNotSupported),
_ => Err(ErrorKind::OperationNotSupported.into()),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion credential/cargo-credential-wincred/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-credential-wincred"
version = "0.4.4"
version = "0.5.0"
rust-version = "1.76.0" # MSRV:1
edition.workspace = true
license.workspace = true
Expand Down
8 changes: 4 additions & 4 deletions credential/cargo-credential-wincred/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#[cfg(windows)]
mod win {
use cargo_credential::{read_token, Action, CacheControl, CredentialResponse, RegistryInfo};
use cargo_credential::{Credential, Error};
use cargo_credential::{Credential, Error, ErrorKind};
use std::ffi::OsStr;

use std::os::windows::ffi::OsStrExt;
Expand Down Expand Up @@ -56,7 +56,7 @@ mod win {
{
let err = std::io::Error::last_os_error();
if err.raw_os_error() == Some(ERROR_NOT_FOUND as i32) {
return Err(Error::NotFound);
return Err(ErrorKind::NotFound.into());
}
return Err(Box::new(err).into());
}
Expand Down Expand Up @@ -107,13 +107,13 @@ mod win {
if result != TRUE {
let err = std::io::Error::last_os_error();
if err.raw_os_error() == Some(ERROR_NOT_FOUND as i32) {
return Err(Error::NotFound);
return Err(ErrorKind::NotFound.into());
}
return Err(Box::new(err).into());
}
Ok(CredentialResponse::Logout)
}
_ => Err(Error::OperationNotSupported),
_ => Err(ErrorKind::OperationNotSupported.into()),
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions credential/cargo-credential/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-credential"
version = "0.4.4"
version = "0.5.0"
rust-version.workspace = true
edition.workspace = true
license.workspace = true
Expand All @@ -13,7 +13,6 @@ anyhow.workspace = true
libc.workspace = true
serde = { workspace = true, features = ["derive"] }
serde_json.workspace = true
thiserror.workspace = true
time.workspace = true

[target.'cfg(windows)'.dependencies]
Expand Down
12 changes: 6 additions & 6 deletions credential/cargo-credential/examples/file-provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ impl Credential for FileCredential {
// another provider for any other registry.
//
// If a provider supports any registry, then this check should be omitted.
return Err(cargo_credential::Error::UrlNotSupported);
return Err(cargo_credential::ErrorKind::UrlNotSupported.into());
}

// `Error::Other` takes a boxed `std::error::Error` type that causes Cargo to show the error.
let mut creds = FileCredential::read().map_err(cargo_credential::Error::Other)?;
let mut creds = FileCredential::read().map_err(cargo_credential::Error::other)?;

match action {
Action::Get(_) => {
Expand All @@ -39,7 +39,7 @@ impl Credential for FileCredential {
} else {
// Credential providers should respond with `NotFound` when a credential can not be
// found, allowing Cargo to attempt another provider.
Err(cargo_credential::Error::NotFound)
Err(cargo_credential::ErrorKind::NotFound.into())
}
}
Action::Login(login_options) => {
Expand All @@ -50,7 +50,7 @@ impl Credential for FileCredential {
let token = cargo_credential::read_token(login_options, registry)?;
creds.insert(registry.index_url.to_string(), token);

FileCredential::write(&creds).map_err(cargo_credential::Error::Other)?;
FileCredential::write(&creds).map_err(cargo_credential::Error::other)?;

// Credentials were successfully stored.
Ok(CredentialResponse::Login)
Expand All @@ -59,14 +59,14 @@ impl Credential for FileCredential {
if creds.remove(registry.index_url).is_none() {
// If the user attempts to log out from a registry that has no credentials
// stored, then NotFound is the appropriate error.
Err(cargo_credential::Error::NotFound)
Err(cargo_credential::ErrorKind::NotFound.into())
} else {
// Credentials were successfully erased.
Ok(CredentialResponse::Logout)
}
}
// If a credential provider doesn't support a given operation, it should respond with `OperationNotSupported`.
_ => Err(cargo_credential::Error::OperationNotSupported),
_ => Err(cargo_credential::ErrorKind::OperationNotSupported.into()),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions credential/cargo-credential/examples/stdout-redirected.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#![allow(clippy::print_stderr)]
#![allow(clippy::print_stdout)]

use cargo_credential::{Action, Credential, CredentialResponse, Error, RegistryInfo};
use cargo_credential::{Action, Credential, CredentialResponse, Error, ErrorKind, RegistryInfo};

struct MyCredential;

Expand All @@ -19,7 +19,7 @@ impl Credential for MyCredential {

// Reading from stdin and writing to stdout will go to the attached console (tty).
println!("message from test credential provider");
Err(Error::OperationNotSupported)
Err(ErrorKind::OperationNotSupported.into())
}
}

Expand Down
Loading