Skip to content

Commit

Permalink
fix(libsecret): Treat lack of libsecret as unsupported
Browse files Browse the repository at this point in the history
This changes cargo-credential-libsecret to report `UrlNotSupported` errors when `libsecret` can't be loaded.

The goal with this change is to make it easier to support a
cross-platform, cross-machine config file.
Before, someone couldn't enable `libsecret` for the machines that
supported it and then fallback to something else on machines that don't.

After this change, we should be able to fallback to other providers.

To help with debugability, we preserve the "`libsecret` can't be loaded"
message by reporting the first "rich" `UrlNotSupported` error message.
This is a newly invented construct as part of this PR.

This was discussed on
[zulip](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/reg.20auth.20and.20libsecret)
and in the cargo team meeting.

This is to improve the cross-platform config support in an effort to
deprecate having config be unset as part of rust-lang#13343
  • Loading branch information
epage committed Mar 7, 2024
1 parent bef68b7 commit 2997467
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 4 deletions.
8 changes: 5 additions & 3 deletions credential/cargo-credential-libsecret/src/lib.rs
Original file line number Diff line number Diff line change
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
5 changes: 5 additions & 0 deletions credential/cargo-credential/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ impl Error {
pub fn kind(&self) -> ErrorKind {
self.kind
}

pub fn as_inner(&self) -> Option<&(dyn StdError + Sync + Send)> {
use std::ops::Deref as _;
self.inner.as_ref().map(|e| e.0.deref())
}
}

impl StdError for Error {
Expand Down
18 changes: 17 additions & 1 deletion src/cargo/util/auth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ fn credential_action(
};
let providers = credential_provider(gctx, sid, require_cred_provider_config, true)?;
let mut any_not_found = false;
let mut custom_not_supported = None;
for provider in providers {
let args: Vec<&str> = provider
.iter()
Expand Down Expand Up @@ -553,7 +554,20 @@ fn credential_action(
match provider.perform(&registry, &action, &args[1..]) {
Ok(response) => return Ok(response),
Err(e) => match e.kind() {
cargo_credential::ErrorKind::UrlNotSupported => {}
cargo_credential::ErrorKind::UrlNotSupported => {
if e.as_inner().is_some() {
custom_not_supported.get_or_insert_with(|| {
Err::<(), _>(e)
.with_context(|| {
format!(
"credential provider `{}` could not handle the request",
args.join(" ")
)
})
.unwrap_err()
});
}
}
cargo_credential::ErrorKind::NotFound => any_not_found = true,
_ => {
return Err(e).with_context(|| {
Expand All @@ -568,6 +582,8 @@ fn credential_action(
}
if any_not_found {
Err(cargo_credential::ErrorKind::NotFound.into())
} else if let Some(custom_not_supported) = custom_not_supported {
Err(custom_not_supported)
} else {
anyhow::bail!("no credential providers could handle the request")
}
Expand Down

0 comments on commit 2997467

Please sign in to comment.