From 5f9e0d33024b38468481e204c56c41adcda3c906 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 5 Mar 2024 21:12:00 -0600 Subject: [PATCH 01/10] refactor(credential): Pull our error deserialization logic --- credential/cargo-credential/src/error.rs | 43 ++++++++++++++---------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/credential/cargo-credential/src/error.rs b/credential/cargo-credential/src/error.rs index 8c5fe19e5ff..63b2096ff39 100644 --- a/credential/cargo-credential/src/error.rs +++ b/credential/cargo-credential/src/error.rs @@ -131,27 +131,34 @@ mod error_serialize { where D: Deserializer<'de>, { - #[derive(Deserialize)] - #[serde(rename_all = "kebab-case")] - struct ErrorData { - message: String, - caused_by: Option>, - } let data = ErrorData::deserialize(deserializer)?; - let mut prev = None; - if let Some(source) = data.caused_by { - for e in source.into_iter().rev() { - prev = Some(Box::new(StringTypedError { - message: e, - source: prev, - })); + let e = Box::new(StringTypedError::from(data)); + Ok(e) + } + + #[derive(Deserialize)] + #[serde(rename_all = "kebab-case")] + struct ErrorData { + message: String, + caused_by: Option>, + } + + impl From for StringTypedError { + fn from(data: ErrorData) -> Self { + let mut prev = None; + if let Some(source) = data.caused_by { + for e in source.into_iter().rev() { + prev = Some(Box::new(StringTypedError { + message: e, + source: prev, + })); + } + } + StringTypedError { + message: data.message, + source: prev, } } - let e = Box::new(StringTypedError { - message: data.message, - source: prev, - }); - Ok(e) } } From de77d20146cb8a285fbab04178d0ad6e48317e4d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Mar 2024 11:32:59 -0600 Subject: [PATCH 02/10] refactor(credential): Remove use of thiserror This isn't really providing much and gets in the way of tweaking the errors. --- Cargo.lock | 1 - Cargo.toml | 1 - credential/cargo-credential/Cargo.toml | 1 - credential/cargo-credential/src/error.rs | 35 ++++++++++++++++++------ 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ea8e4354359..9061a58d447 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -360,7 +360,6 @@ dependencies = [ "serde", "serde_json", "snapbox", - "thiserror", "time", "windows-sys 0.52.0", ] diff --git a/Cargo.toml b/Cargo.toml index b5b508c7b7f..ac6340850bc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" proptest = "1.4.0" pulldown-cmark = { version = "0.10.0", default-features = false, features = ["html"] } rand = "0.8.5" diff --git a/credential/cargo-credential/Cargo.toml b/credential/cargo-credential/Cargo.toml index 1b9ae5756f3..1fdf6c5f925 100644 --- a/credential/cargo-credential/Cargo.toml +++ b/credential/cargo-credential/Cargo.toml @@ -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] diff --git a/credential/cargo-credential/src/error.rs b/credential/cargo-credential/src/error.rs index 63b2096ff39..5c03c0860ba 100644 --- a/credential/cargo-credential/src/error.rs +++ b/credential/cargo-credential/src/error.rs @@ -1,6 +1,6 @@ -use serde::{Deserialize, Serialize}; use std::error::Error as StdError; -use thiserror::Error as ThisError; + +use serde::{Deserialize, Serialize}; /// Credential provider error type. /// @@ -9,38 +9,57 @@ use thiserror::Error as ThisError; /// variants are fatal. /// /// Note: Do not add a tuple variant, as it cannot be serialized. -#[derive(Serialize, Deserialize, ThisError, Debug)] +#[derive(Serialize, Deserialize, Debug)] #[serde(rename_all = "kebab-case", tag = "kind")] #[non_exhaustive] pub enum Error { /// Registry URL is not supported. This should be used if /// the provider only works for some registries. Cargo will /// try another provider, if available - #[error("registry not supported")] UrlNotSupported, /// Credentials could not be found. Cargo will try another /// provider, if available - #[error("credential not found")] NotFound, /// The provider doesn't support this operation, such as /// a provider that can't support 'login' / 'logout' - #[error("requested operation not supported")] OperationNotSupported, /// The provider failed to perform the operation. Other /// providers will not be attempted - #[error(transparent)] #[serde(with = "error_serialize")] Other(Box), /// A new variant was added to this enum since Cargo was built - #[error("unknown error kind; try updating Cargo?")] #[serde(other)] Unknown, } +impl StdError for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + Error::Other(transparent) => transparent.source(), + Error::UrlNotSupported + | Error::NotFound + | Error::OperationNotSupported + | Error::Unknown => None, + } + } +} + +impl std::fmt::Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Error::UrlNotSupported => f.write_str("registry not supported"), + Error::NotFound => f.write_str("credential not found"), + Error::OperationNotSupported => f.write_str("requested operation not supported"), + Error::Other(transparent) => transparent.fmt(f), + Error::Unknown => f.write_str("unknown error kind; try updating Cargo?"), + } + } +} + impl From for Error { fn from(message: String) -> Self { Box::new(StringTypedError { From 8de4ecb3f059eaeed614ea89fa3fcbbcef89715a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Mar 2024 11:49:46 -0600 Subject: [PATCH 03/10] test(credential): Verify existing serde behavior for NotSupported --- credential/cargo-credential/src/error.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/credential/cargo-credential/src/error.rs b/credential/cargo-credential/src/error.rs index 5c03c0860ba..e42f99455c1 100644 --- a/credential/cargo-credential/src/error.rs +++ b/credential/cargo-credential/src/error.rs @@ -186,7 +186,19 @@ mod tests { use super::Error; #[test] - pub fn unknown_kind() { + fn not_supported_roundtrip() { + let input = Error::UrlNotSupported; + + let expected_json = r#"{"kind":"url-not-supported"}"#; + let actual_json = serde_json::to_string(&input).unwrap(); + assert_eq!(actual_json, expected_json); + + let actual = serde_json::from_str(&actual_json).unwrap(); + assert!(matches!(actual, Error::UrlNotSupported)); + } + + #[test] + fn deserialize_to_unknown_kind() { let json = r#"{ "kind": "unexpected-kind", "unexpected-content": "test" @@ -196,7 +208,7 @@ mod tests { } #[test] - pub fn roundtrip() { + fn other_roundtrip() { // Construct an error with context let e = anyhow::anyhow!("E1").context("E2").context("E3"); // Convert to a string with contexts. From 321291f9b7571d3e3e92e43f55dcc53d11b36b4c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Mar 2024 14:22:44 -0600 Subject: [PATCH 04/10] refactor(credential): Pull out alias for BoxError --- credential/cargo-credential/src/error.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/credential/cargo-credential/src/error.rs b/credential/cargo-credential/src/error.rs index e42f99455c1..ba5f1a66176 100644 --- a/credential/cargo-credential/src/error.rs +++ b/credential/cargo-credential/src/error.rs @@ -2,6 +2,8 @@ use std::error::Error as StdError; use serde::{Deserialize, Serialize}; +type BoxError = Box; + /// Credential provider error type. /// /// `UrlNotSupported` and `NotFound` errors both cause Cargo @@ -29,7 +31,7 @@ pub enum Error { /// The provider failed to perform the operation. Other /// providers will not be attempted #[serde(with = "error_serialize")] - Other(Box), + Other(BoxError), /// A new variant was added to this enum since Cargo was built #[serde(other)] @@ -123,12 +125,10 @@ mod error_serialize { use serde::{ser::SerializeStruct, Deserialize, Deserializer, Serializer}; - use crate::error::StringTypedError; + use super::BoxError; + use super::StringTypedError; - pub fn serialize( - e: &Box, - serializer: S, - ) -> Result + pub fn serialize(e: &BoxError, serializer: S) -> Result where S: Serializer, { @@ -146,7 +146,7 @@ mod error_serialize { state.end() } - pub fn deserialize<'de, D>(deserializer: D) -> Result, D::Error> + pub fn deserialize<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, { From 459cab9a81011e6f1573f6c12f46b8bb7859b0f1 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Mar 2024 14:37:25 -0600 Subject: [PATCH 05/10] refactor(credential): Consolidate Other construction --- credential/cargo-credential/src/error.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/credential/cargo-credential/src/error.rs b/credential/cargo-credential/src/error.rs index ba5f1a66176..a342495dc15 100644 --- a/credential/cargo-credential/src/error.rs +++ b/credential/cargo-credential/src/error.rs @@ -80,14 +80,7 @@ impl From<&str> for Error { impl From for Error { fn from(value: anyhow::Error) -> Self { - let mut prev = None; - for e in value.chain().rev() { - prev = Some(Box::new(StringTypedError { - message: e.to_string(), - source: prev, - })); - } - Error::Other(prev.unwrap()) + Error::from(Box::new(StringTypedError::from(value))) } } @@ -116,6 +109,19 @@ impl std::fmt::Display for StringTypedError { } } +impl From for StringTypedError { + fn from(value: anyhow::Error) -> Self { + let mut prev = None; + for e in value.chain().rev() { + prev = Some(StringTypedError { + message: e.to_string(), + source: prev.map(Box::new), + }); + } + prev.unwrap() + } +} + /// Serializer / deserializer for any boxed error. /// The string representation of the error, and its `source` chain can roundtrip across /// the serialization. The actual types are lost (downcast will not work). From 52af0c63fc1a971bfb26cc6a38ec0cf72d850a73 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Mar 2024 15:11:27 -0600 Subject: [PATCH 06/10] refactor(credential): Switch to an Other constructor --- credential/cargo-credential/src/error.rs | 8 +++++++- credential/cargo-credential/src/lib.rs | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/credential/cargo-credential/src/error.rs b/credential/cargo-credential/src/error.rs index a342495dc15..1da184c5868 100644 --- a/credential/cargo-credential/src/error.rs +++ b/credential/cargo-credential/src/error.rs @@ -38,6 +38,12 @@ pub enum Error { Unknown, } +impl Error { + pub(crate) fn other(inner: BoxError) -> Self { + Self::Other(inner) + } +} + impl StdError for Error { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { @@ -86,7 +92,7 @@ impl From for Error { impl From> for Error { fn from(value: Box) -> Self { - Error::Other(value) + Error::other(value) } } diff --git a/credential/cargo-credential/src/lib.rs b/credential/cargo-credential/src/lib.rs index 0888fb402f7..84eab66f881 100644 --- a/credential/cargo-credential/src/lib.rs +++ b/credential/cargo-credential/src/lib.rs @@ -226,7 +226,7 @@ pub trait Credential { /// Runs the credential interaction pub fn main(credential: impl Credential) { - let result = doit(credential).map_err(|e| Error::Other(e)); + let result = doit(credential).map_err(|e| Error::other(e)); if result.is_err() { serde_json::to_writer(std::io::stdout(), &result) .expect("failed to serialize credential provider error"); From 0a56d63d900fe7b984b424ec75efa4f140d725ad Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 6 Mar 2024 12:25:10 -0600 Subject: [PATCH 07/10] fix(credential)!: Decouple `ErrorKind` from `BoxError` BREAKING CHANGE: `Error` because an opaque type with the enum broken out into `ErrorKind` - To construct an error from an `ErrorKind`, you can use `Into`. - To wrap an existing error, use `Error::other` --- Cargo.lock | 10 +- Cargo.toml | 8 +- .../cargo-credential-1password/Cargo.toml | 2 +- .../cargo-credential-1password/src/main.rs | 8 +- .../cargo-credential-libsecret/Cargo.toml | 2 +- .../cargo-credential-libsecret/src/lib.rs | 8 +- .../Cargo.toml | 2 +- .../src/lib.rs | 9 +- .../cargo-credential-wincred/Cargo.toml | 2 +- .../cargo-credential-wincred/src/lib.rs | 8 +- credential/cargo-credential/Cargo.toml | 2 +- .../examples/file-provider.rs | 12 +- .../examples/stdout-redirected.rs | 4 +- credential/cargo-credential/src/error.rs | 179 ++++++++++++------ credential/cargo-credential/src/lib.rs | 17 +- src/cargo/util/auth/mod.rs | 40 ++-- src/cargo/util/credential/adaptor.rs | 2 +- src/cargo/util/credential/paseto.rs | 11 +- src/cargo/util/credential/token.rs | 10 +- 19 files changed, 204 insertions(+), 132 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9061a58d447..88a12cfecec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -353,7 +353,7 @@ dependencies = [ [[package]] name = "cargo-credential" -version = "0.4.4" +version = "0.5.0" dependencies = [ "anyhow", "libc", @@ -366,7 +366,7 @@ dependencies = [ [[package]] name = "cargo-credential-1password" -version = "0.4.4" +version = "0.5.0" dependencies = [ "cargo-credential", "serde", @@ -375,7 +375,7 @@ dependencies = [ [[package]] name = "cargo-credential-libsecret" -version = "0.4.4" +version = "0.5.0" dependencies = [ "anyhow", "cargo-credential", @@ -384,7 +384,7 @@ dependencies = [ [[package]] name = "cargo-credential-macos-keychain" -version = "0.4.4" +version = "0.5.0" dependencies = [ "cargo-credential", "security-framework", @@ -392,7 +392,7 @@ dependencies = [ [[package]] name = "cargo-credential-wincred" -version = "0.4.4" +version = "0.5.0" dependencies = [ "cargo-credential", "windows-sys 0.52.0", diff --git a/Cargo.toml b/Cargo.toml index ac6340850bc..b215c4987cb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" } diff --git a/credential/cargo-credential-1password/Cargo.toml b/credential/cargo-credential-1password/Cargo.toml index 144b44070db..6a870205d41 100644 --- a/credential/cargo-credential-1password/Cargo.toml +++ b/credential/cargo-credential-1password/Cargo.toml @@ -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 diff --git a/credential/cargo-credential-1password/src/main.rs b/credential/cargo-credential-1password/src/main.rs index 38b567bf2d4..d6036dd549b 100644 --- a/credential/cargo-credential-1password/src/main.rs +++ b/credential/cargo-credential-1password/src/main.rs @@ -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; @@ -278,7 +278,7 @@ impl Credential for OnePasswordCredential { operation_independent: true, }) } else { - Err(Error::NotFound) + Err(ErrorKind::NotFound.into()) } } Action::Login(options) => { @@ -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()), } } } diff --git a/credential/cargo-credential-libsecret/Cargo.toml b/credential/cargo-credential-libsecret/Cargo.toml index 8a65d7f389f..fd7fde4588b 100644 --- a/credential/cargo-credential-libsecret/Cargo.toml +++ b/credential/cargo-credential-libsecret/Cargo.toml @@ -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 diff --git a/credential/cargo-credential-libsecret/src/lib.rs b/credential/cargo-credential-libsecret/src/lib.rs index ee179760539..82646926f4d 100644 --- a/credential/cargo-credential-libsecret/src/lib.rs +++ b/credential/cargo-credential-libsecret/src/lib.rs @@ -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}; @@ -153,7 +153,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) @@ -223,7 +223,7 @@ mod linux { } Ok(CredentialResponse::Logout) } - _ => Err(Error::OperationNotSupported), + _ => Err(ErrorKind::OperationNotSupported.into()), } } } diff --git a/credential/cargo-credential-macos-keychain/Cargo.toml b/credential/cargo-credential-macos-keychain/Cargo.toml index f0438e043cd..b43ef24ab34 100644 --- a/credential/cargo-credential-macos-keychain/Cargo.toml +++ b/credential/cargo-credential-macos-keychain/Cargo.toml @@ -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 diff --git a/credential/cargo-credential-macos-keychain/src/lib.rs b/credential/cargo-credential-macos-keychain/src/lib.rs index 8a702a3620c..a6ee2b9a1f7 100644 --- a/credential/cargo-credential-macos-keychain/src/lib.rs +++ b/credential/cargo-credential-macos-keychain/src/lib.rs @@ -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; @@ -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)?; @@ -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()), } } } diff --git a/credential/cargo-credential-wincred/Cargo.toml b/credential/cargo-credential-wincred/Cargo.toml index 46f5fb31ed1..fb297eee535 100644 --- a/credential/cargo-credential-wincred/Cargo.toml +++ b/credential/cargo-credential-wincred/Cargo.toml @@ -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 diff --git a/credential/cargo-credential-wincred/src/lib.rs b/credential/cargo-credential-wincred/src/lib.rs index 24b072ee28a..de454c9a695 100644 --- a/credential/cargo-credential-wincred/src/lib.rs +++ b/credential/cargo-credential-wincred/src/lib.rs @@ -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; @@ -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()); } @@ -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()), } } } diff --git a/credential/cargo-credential/Cargo.toml b/credential/cargo-credential/Cargo.toml index 1fdf6c5f925..2ded91d3268 100644 --- a/credential/cargo-credential/Cargo.toml +++ b/credential/cargo-credential/Cargo.toml @@ -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 diff --git a/credential/cargo-credential/examples/file-provider.rs b/credential/cargo-credential/examples/file-provider.rs index 3ed312cb833..2c021549a42 100644 --- a/credential/cargo-credential/examples/file-provider.rs +++ b/credential/cargo-credential/examples/file-provider.rs @@ -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(_) => { @@ -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) => { @@ -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) @@ -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()), } } } diff --git a/credential/cargo-credential/examples/stdout-redirected.rs b/credential/cargo-credential/examples/stdout-redirected.rs index 1986253dce5..f9e5012be3c 100644 --- a/credential/cargo-credential/examples/stdout-redirected.rs +++ b/credential/cargo-credential/examples/stdout-redirected.rs @@ -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; @@ -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()) } } diff --git a/credential/cargo-credential/src/error.rs b/credential/cargo-credential/src/error.rs index 1da184c5868..d7e1e366ac1 100644 --- a/credential/cargo-credential/src/error.rs +++ b/credential/cargo-credential/src/error.rs @@ -12,58 +12,49 @@ type BoxError = Box; /// /// Note: Do not add a tuple variant, as it cannot be serialized. #[derive(Serialize, Deserialize, Debug)] -#[serde(rename_all = "kebab-case", tag = "kind")] +#[serde(rename_all = "kebab-case")] #[non_exhaustive] -pub enum Error { - /// Registry URL is not supported. This should be used if - /// the provider only works for some registries. Cargo will - /// try another provider, if available - UrlNotSupported, - - /// Credentials could not be found. Cargo will try another - /// provider, if available - NotFound, - - /// The provider doesn't support this operation, such as - /// a provider that can't support 'login' / 'logout' - OperationNotSupported, +pub struct Error { + kind: ErrorKind, - /// The provider failed to perform the operation. Other - /// providers will not be attempted - #[serde(with = "error_serialize")] - Other(BoxError), - - /// A new variant was added to this enum since Cargo was built - #[serde(other)] - Unknown, + #[serde(flatten)] + inner: Option, } impl Error { - pub(crate) fn other(inner: BoxError) -> Self { - Self::Other(inner) + pub fn other(inner: BoxError) -> Self { + Self { + kind: ErrorKind::Other, + inner: Some(SerdeBoxError(inner)), + } + } + + pub fn kind(&self) -> ErrorKind { + self.kind } } impl StdError for Error { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Error::Other(transparent) => transparent.source(), - Error::UrlNotSupported - | Error::NotFound - | Error::OperationNotSupported - | Error::Unknown => None, - } + self.inner.as_ref().and_then(|e| e.source()) } } impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Error::UrlNotSupported => f.write_str("registry not supported"), - Error::NotFound => f.write_str("credential not found"), - Error::OperationNotSupported => f.write_str("requested operation not supported"), - Error::Other(transparent) => transparent.fmt(f), - Error::Unknown => f.write_str("unknown error kind; try updating Cargo?"), + if let Some(inner) = self.inner.as_ref() { + inner.fmt(f) + } else { + self.kind.fmt(f) + } + } +} + +impl From for Error { + fn from(kind: ErrorKind) -> Self { + Self { + kind: kind, + inner: None, } } } @@ -96,6 +87,53 @@ impl From> for Error { } } +/// Credential provider error kind. +/// +/// `UrlNotSupported` and `NotFound` errors both cause Cargo +/// to attempt another provider, if one is available. The other +/// variants are fatal. +/// +/// Note: Do not add a tuple variant, as it cannot be serialized. +#[derive(Copy, Clone, Serialize, Deserialize, Debug)] +#[serde(rename_all = "kebab-case")] +#[non_exhaustive] +pub enum ErrorKind { + /// Registry URL is not supported. This should be used if + /// the provider only works for some registries. Cargo will + /// try another provider, if available + UrlNotSupported, + + /// Credentials could not be found. Cargo will try another + /// provider, if available + NotFound, + + /// The provider doesn't support this operation, such as + /// a provider that can't support 'login' / 'logout' + OperationNotSupported, + + /// The provider failed to perform the operation. Other + /// providers will not be attempted + Other, + + /// A new variant was added to this enum since Cargo was built + #[serde(other)] + Unknown, +} + +impl StdError for ErrorKind {} + +impl std::fmt::Display for ErrorKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::UrlNotSupported => f.write_str("registry not supported"), + Self::NotFound => f.write_str("credential not found"), + Self::OperationNotSupported => f.write_str("requested operation not supported"), + Self::Other => f.write_str("credential action failed"), + Self::Unknown => f.write_str("unknown error kind; try updating Cargo?"), + } + } +} + /// String-based error type with an optional source #[derive(Debug)] struct StringTypedError { @@ -128,27 +166,22 @@ impl From for StringTypedError { } } -/// Serializer / deserializer for any boxed error. -/// The string representation of the error, and its `source` chain can roundtrip across -/// the serialization. The actual types are lost (downcast will not work). -mod error_serialize { - use std::error::Error as StdError; - use std::ops::Deref; - - use serde::{ser::SerializeStruct, Deserialize, Deserializer, Serializer}; - - use super::BoxError; - use super::StringTypedError; +#[derive(Debug)] +struct SerdeBoxError(BoxError); - pub fn serialize(e: &BoxError, serializer: S) -> Result +impl Serialize for SerdeBoxError { + fn serialize(&self, serializer: S) -> Result where - S: Serializer, + S: serde::Serializer, { + use serde::ser::SerializeStruct; + use std::ops::Deref as _; + let mut state = serializer.serialize_struct("StringTypedError", 2)?; - state.serialize_field("message", &format!("{}", e))?; + state.serialize_field("message", &format!("{}", self.0))?; // Serialize the source error chain recursively - let mut current_source: &dyn StdError = e.deref(); + let mut current_source: &dyn StdError = self.0.deref(); let mut sources = Vec::new(); while let Some(err) = current_source.source() { sources.push(err.to_string()); @@ -157,19 +190,42 @@ mod error_serialize { state.serialize_field("caused-by", &sources)?; state.end() } +} - pub fn deserialize<'de, D>(deserializer: D) -> Result +impl<'de> Deserialize<'de> for SerdeBoxError { + fn deserialize(deserializer: D) -> Result where - D: Deserializer<'de>, + D: serde::Deserializer<'de>, { - let data = ErrorData::deserialize(deserializer)?; - let e = Box::new(StringTypedError::from(data)); + let data = error_serialize::ErrorData::deserialize(deserializer)?; + let e = SerdeBoxError(Box::new(StringTypedError::from(data))); Ok(e) } +} + +impl StdError for SerdeBoxError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.0.source() + } +} + +impl std::fmt::Display for SerdeBoxError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + +/// Serializer / deserializer for any boxed error. +/// The string representation of the error, and its `source` chain can roundtrip across +/// the serialization. The actual types are lost (downcast will not work). +mod error_serialize { + use serde::Deserialize; + + use super::StringTypedError; #[derive(Deserialize)] #[serde(rename_all = "kebab-case")] - struct ErrorData { + pub struct ErrorData { message: String, caused_by: Option>, } @@ -196,17 +252,18 @@ mod error_serialize { #[cfg(test)] mod tests { use super::Error; + use super::ErrorKind; #[test] fn not_supported_roundtrip() { - let input = Error::UrlNotSupported; + let input = Error::from(ErrorKind::UrlNotSupported); let expected_json = r#"{"kind":"url-not-supported"}"#; let actual_json = serde_json::to_string(&input).unwrap(); assert_eq!(actual_json, expected_json); - let actual = serde_json::from_str(&actual_json).unwrap(); - assert!(matches!(actual, Error::UrlNotSupported)); + let actual: Error = serde_json::from_str(&actual_json).unwrap(); + assert!(matches!(actual.kind(), ErrorKind::UrlNotSupported)); } #[test] @@ -216,7 +273,7 @@ mod tests { "unexpected-content": "test" }"#; let e: Error = serde_json::from_str(&json).unwrap(); - assert!(matches!(e, Error::Unknown)); + assert!(matches!(e.kind(), ErrorKind::Unknown)); } #[test] diff --git a/credential/cargo-credential/src/lib.rs b/credential/cargo-credential/src/lib.rs index 84eab66f881..a7833ba3d34 100644 --- a/credential/cargo-credential/src/lib.rs +++ b/credential/cargo-credential/src/lib.rs @@ -13,22 +13,22 @@ //! active console. This allows credential providers to be interactive if necessary. //! //! ## Error handling -//! ### [`Error::UrlNotSupported`] +//! ### [`ErrorKind::UrlNotSupported`] //! A credential provider may only support some registry URLs. If this is the case //! and an unsupported index URL is passed to the provider, it should respond with -//! [`Error::UrlNotSupported`]. Other credential providers may be attempted by Cargo. +//! [`ErrorKind::UrlNotSupported`]. Other credential providers may be attempted by Cargo. //! -//! ### [`Error::NotFound`] +//! ### [`ErrorKind::NotFound`] //! When attempting an [`Action::Get`] or [`Action::Logout`], if a credential can not -//! be found, the provider should respond with [`Error::NotFound`]. Other credential +//! be found, the provider should respond with [`ErrorKind::NotFound`]. Other credential //! providers may be attempted by Cargo. //! -//! ### [`Error::OperationNotSupported`] +//! ### [`ErrorKind::OperationNotSupported`] //! A credential provider might not support all operations. For example if the provider -//! only supports [`Action::Get`], [`Error::OperationNotSupported`] should be returned +//! only supports [`Action::Get`], [`ErrorKind::OperationNotSupported`] should be returned //! for all other requests. //! -//! ### [`Error::Other`] +//! ### [`ErrorKind::Other`] //! All other errors go here. The error will be shown to the user in Cargo, including //! the full error chain using [`std::error::Error::source`]. //! @@ -49,6 +49,7 @@ mod secret; mod stdio; pub use error::Error; +pub use error::ErrorKind; pub use secret::Secret; use stdio::stdin_stdout_to_console; @@ -68,7 +69,7 @@ impl Credential for UnsupportedCredential { _action: &Action<'_>, _args: &[&str], ) -> Result { - Err(Error::UrlNotSupported) + Err(ErrorKind::UrlNotSupported.into()) } } diff --git a/src/cargo/util/auth/mod.rs b/src/cargo/util/auth/mod.rs index cf4cccb9753..c99959a681a 100644 --- a/src/cargo/util/auth/mod.rs +++ b/src/cargo/util/auth/mod.rs @@ -552,20 +552,22 @@ fn credential_action( })?; match provider.perform(®istry, &action, &args[1..]) { Ok(response) => return Ok(response), - Err(cargo_credential::Error::UrlNotSupported) => {} - Err(cargo_credential::Error::NotFound) => any_not_found = true, - e => { - return e.with_context(|| { - format!( - "credential provider `{}` failed action `{action}`", - args.join(" ") - ) - }) - } + Err(e) => match e.kind() { + cargo_credential::ErrorKind::UrlNotSupported => {} + cargo_credential::ErrorKind::NotFound => any_not_found = true, + _ => { + return Err(e).with_context(|| { + format!( + "credential provider `{}` failed action `{action}`", + args.join(" ") + ) + }) + } + }, } } if any_not_found { - Err(cargo_credential::Error::NotFound.into()) + Err(cargo_credential::ErrorKind::NotFound.into()) } else { anyhow::bail!("no credential providers could handle the request") } @@ -630,8 +632,12 @@ fn auth_token_optional( require_cred_provider_config, ); if let Some(e) = credential_response.as_ref().err() { - if let Some(e) = e.downcast_ref::() { - if matches!(e, cargo_credential::Error::NotFound) { + if let Some(e) = e + .downcast_ref::() + .map(|e| e.kind()) + .or_else(|| e.downcast_ref::().copied()) + { + if matches!(e, cargo_credential::ErrorKind::NotFound) { return Ok(None); } } @@ -669,8 +675,12 @@ fn auth_token_optional( pub fn logout(gctx: &GlobalContext, sid: &SourceId) -> CargoResult<()> { let credential_response = credential_action(gctx, sid, Action::Logout, vec![], &[], false); if let Some(e) = credential_response.as_ref().err() { - if let Some(e) = e.downcast_ref::() { - if matches!(e, cargo_credential::Error::NotFound) { + if let Some(e) = e + .downcast_ref::() + .map(|e| e.kind()) + .or_else(|| e.downcast_ref::().copied()) + { + if matches!(e, cargo_credential::ErrorKind::NotFound) { gctx.shell().status( "Logout", format!( diff --git a/src/cargo/util/credential/adaptor.rs b/src/cargo/util/credential/adaptor.rs index 693e653b5c6..d4a236be598 100644 --- a/src/cargo/util/credential/adaptor.rs +++ b/src/cargo/util/credential/adaptor.rs @@ -62,7 +62,7 @@ impl Credential for BasicProcessCredential { operation_independent: true, }) } - _ => Err(cargo_credential::Error::OperationNotSupported), + _ => Err(cargo_credential::ErrorKind::OperationNotSupported.into()), } } } diff --git a/src/cargo/util/credential/paseto.rs b/src/cargo/util/credential/paseto.rs index 0d552608c2a..0993677446c 100644 --- a/src/cargo/util/credential/paseto.rs +++ b/src/cargo/util/credential/paseto.rs @@ -2,7 +2,8 @@ use anyhow::Context as _; use cargo_credential::{ - Action, CacheControl, Credential, CredentialResponse, Error, Operation, RegistryInfo, Secret, + Action, CacheControl, Credential, CredentialResponse, Error, ErrorKind, Operation, + RegistryInfo, Secret, }; use clap::Command; use pasetors::{ @@ -82,10 +83,10 @@ impl<'a> Credential for PasetoCredential<'a> { match action { Action::Get(operation) => { let Some(reg_cfg) = reg_cfg else { - return Err(Error::NotFound); + return Err(ErrorKind::NotFound.into()); }; let Some(secret_key) = reg_cfg.secret_key.as_ref() else { - return Err(Error::NotFound); + return Err(ErrorKind::NotFound.into()); }; let secret_key_subject = reg_cfg.secret_key_subject; @@ -209,10 +210,10 @@ impl<'a> Credential for PasetoCredential<'a> { ); Ok(CredentialResponse::Logout) } else { - Err(Error::NotFound) + Err(ErrorKind::NotFound.into()) } } - _ => Err(Error::OperationNotSupported), + _ => Err(ErrorKind::OperationNotSupported.into()), } } } diff --git a/src/cargo/util/credential/token.rs b/src/cargo/util/credential/token.rs index c58c874177f..8b078b6902f 100644 --- a/src/cargo/util/credential/token.rs +++ b/src/cargo/util/credential/token.rs @@ -1,7 +1,9 @@ //! Credential provider that uses plaintext tokens in Cargo's config. use anyhow::Context as _; -use cargo_credential::{Action, CacheControl, Credential, CredentialResponse, Error, RegistryInfo}; +use cargo_credential::{ + Action, CacheControl, Credential, CredentialResponse, Error, ErrorKind, RegistryInfo, +}; use url::Url; use crate::{ @@ -38,7 +40,7 @@ impl<'a> Credential for TokenCredential<'a> { match action { Action::Get(_) => { - let token = previous_token.ok_or_else(|| Error::NotFound)?.val; + let token = previous_token.ok_or_else(|| ErrorKind::NotFound)?.val; Ok(CredentialResponse::Get { token, cache: CacheControl::Session, @@ -65,7 +67,7 @@ impl<'a> Credential for TokenCredential<'a> { } Action::Logout => { if previous_token.is_none() { - return Err(Error::NotFound); + return Err(ErrorKind::NotFound.into()); } let reg_name = sid.display_registry_name(); context::save_credentials(self.gctx, None, &sid)?; @@ -89,7 +91,7 @@ impl<'a> Credential for TokenCredential<'a> { ); Ok(CredentialResponse::Logout) } - _ => Err(Error::OperationNotSupported), + _ => Err(ErrorKind::OperationNotSupported.into()), } } } From 19e2165bafacbd30bd79bb6e2cf44afae04e4a78 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 7 Mar 2024 14:18:40 -0600 Subject: [PATCH 08/10] feat(credential): Allow any ErrorKind to have a custom error --- credential/cargo-credential/src/error.rs | 5 +++++ .../src/reference/credential-provider-protocol.md | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/credential/cargo-credential/src/error.rs b/credential/cargo-credential/src/error.rs index d7e1e366ac1..b93c6b7f9a5 100644 --- a/credential/cargo-credential/src/error.rs +++ b/credential/cargo-credential/src/error.rs @@ -29,6 +29,11 @@ impl Error { } } + pub fn with_kind(mut self, kind: ErrorKind) -> Self { + self.kind = kind; + self + } + pub fn kind(&self) -> ErrorKind { self.kind } diff --git a/src/doc/src/reference/credential-provider-protocol.md b/src/doc/src/reference/credential-provider-protocol.md index 31006abee00..6fe0b298f66 100644 --- a/src/doc/src/reference/credential-provider-protocol.md +++ b/src/doc/src/reference/credential-provider-protocol.md @@ -163,6 +163,10 @@ the provider wants to generate tokens that are scoped to specific operations. ```javascript {"Err":{ "kind":"url-not-supported" + // (Optional) Error message string to be displayed + "message": "free form string error message", + // (Optional) Detailed cause chain for the error (optional) + "caused-by": ["cause 1", "cause 2"] }} ``` Sent if the credential provider is designed @@ -177,6 +181,10 @@ available. {"Err":{ // Error: The credential could not be found in the provider. "kind":"not-found" + // (Optional) Error message string to be displayed + "message": "free form string error message", + // (Optional) Detailed cause chain for the error (optional) + "caused-by": ["cause 1", "cause 2"] }} ``` Sent if the credential could not be found. This is expected for @@ -190,6 +198,10 @@ requests where there is nothing found to erase. {"Err":{ // Error: The credential could not be found in the provider. "kind":"operation-not-supported" + // (Optional) Error message string to be displayed + "message": "free form string error message", + // (Optional) Detailed cause chain for the error (optional) + "caused-by": ["cause 1", "cause 2"] }} ``` Sent if the credential provider does not support the requested operation. From cea204f9e96d1d3b51ab6b2bea78dbefa6bf3af6 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 7 Mar 2024 14:51:13 -0600 Subject: [PATCH 09/10] fix(libsecret): Treat lack of libsecret as unsupported 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 #13343 --- .../cargo-credential-libsecret/src/lib.rs | 8 +++++--- credential/cargo-credential/src/error.rs | 5 +++++ src/cargo/util/auth/mod.rs | 18 +++++++++++++++++- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/credential/cargo-credential-libsecret/src/lib.rs b/credential/cargo-credential-libsecret/src/lib.rs index 82646926f4d..898f6ab328e 100644 --- a/credential/cargo-credential-libsecret/src/lib.rs +++ b/credential/cargo-credential-libsecret/src/lib.rs @@ -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)?; diff --git a/credential/cargo-credential/src/error.rs b/credential/cargo-credential/src/error.rs index b93c6b7f9a5..9a383ffbb9a 100644 --- a/credential/cargo-credential/src/error.rs +++ b/credential/cargo-credential/src/error.rs @@ -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 { diff --git a/src/cargo/util/auth/mod.rs b/src/cargo/util/auth/mod.rs index c99959a681a..73508cbee75 100644 --- a/src/cargo/util/auth/mod.rs +++ b/src/cargo/util/auth/mod.rs @@ -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() @@ -553,7 +554,20 @@ fn credential_action( match provider.perform(®istry, &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(|| { @@ -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") } From 0f3b391d60b0b70583cf0e3de387f1f458b8898b Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 7 Mar 2024 14:57:32 -0600 Subject: [PATCH 10/10] fix(token-from-stdout): Treat process spawn errors as unsupported Like with `libsecret` not being present, process spawn errors can be treated as an indication that that config entry isn't relevant for the current machine and we should try other entries. If there is nothing else to fallback to, we'll still report this error. --- src/cargo/util/credential/adaptor.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/cargo/util/credential/adaptor.rs b/src/cargo/util/credential/adaptor.rs index d4a236be598..edc02fe9835 100644 --- a/src/cargo/util/credential/adaptor.rs +++ b/src/cargo/util/credential/adaptor.rs @@ -7,7 +7,7 @@ use std::{ use anyhow::Context; use cargo_credential::{ - Action, CacheControl, Credential, CredentialResponse, RegistryInfo, Secret, + Action, CacheControl, Credential, CredentialResponse, Error, ErrorKind, RegistryInfo, Secret, }; pub struct BasicProcessCredential {} @@ -33,7 +33,10 @@ impl Credential for BasicProcessCredential { cmd.env("CARGO_REGISTRY_NAME_OPT", name); } cmd.stdout(Stdio::piped()); - let mut child = cmd.spawn().context("failed to spawn credential process")?; + let mut child = cmd + .spawn() + .context("failed to spawn credential process") + .map_err(|err| Error::from(err).with_kind(ErrorKind::UrlNotSupported))?; let mut buffer = String::new(); child .stdout