crepererum commented on code in PR #4926:
URL: https://github.com/apache/arrow-rs/pull/4926#discussion_r1358104134
##########
object_store/src/gcp/credential.rs:
##########
@@ -404,62 +369,29 @@ impl TokenProvider for InstanceCredentialProvider {
}
}
-/// ApplicationDefaultCredentials
-/// <https://google.aip.dev/auth/4110>
-pub fn application_default_credentials(
- path: Option<&str>,
- client: &ClientOptions,
- retry: &RetryConfig,
-) -> crate::Result<Option<GcpCredentialProvider>> {
- let file = match ApplicationDefaultCredentialsFile::read(path)? {
- Some(x) => x,
- None => return Ok(None),
- };
-
- match file.type_.as_str() {
- // <https://google.aip.dev/auth/4113>
- "authorized_user" => {
- let token = AuthorizedUserCredentials {
- client_id: file.client_id,
- client_secret: file.client_secret,
- refresh_token: file.refresh_token,
- };
-
- Ok(Some(Arc::new(TokenCredentialProvider::new(
- token,
- client.client()?,
- retry.clone(),
- ))))
- }
- type_ => Err(UnsupportedCredentialsType {
- type_: type_.to_string(),
- }
- .into()),
- }
-}
-
/// A deserialized `application_default_credentials.json`-file.
+///
///
<https://cloud.google.com/docs/authentication/application-default-credentials#personal>
+/// <https://google.aip.dev/auth/4110>
#[derive(serde::Deserialize)]
-struct ApplicationDefaultCredentialsFile {
- #[serde(default)]
- client_id: String,
- #[serde(default)]
- client_secret: String,
- #[serde(default)]
- refresh_token: String,
- #[serde(rename = "type")]
- type_: String,
+#[serde(tag = "type")]
+pub enum ApplicationDefaultCredentials {
+ /// <https://google.aip.dev/auth/4112>
+ #[serde(rename = "service_account")]
+ ServiceAccount(ServiceAccountCredentials),
+ /// <https://google.aip.dev/auth/4113>
Review Comment:
```suggestion
/// Authorized user via "gcloud CLI Integration".
///
/// # References
/// - <https://google.aip.dev/auth/4113>
```
##########
object_store/src/gcp/credential.rs:
##########
@@ -404,62 +369,29 @@ impl TokenProvider for InstanceCredentialProvider {
}
}
-/// ApplicationDefaultCredentials
-/// <https://google.aip.dev/auth/4110>
-pub fn application_default_credentials(
- path: Option<&str>,
- client: &ClientOptions,
- retry: &RetryConfig,
-) -> crate::Result<Option<GcpCredentialProvider>> {
- let file = match ApplicationDefaultCredentialsFile::read(path)? {
- Some(x) => x,
- None => return Ok(None),
- };
-
- match file.type_.as_str() {
- // <https://google.aip.dev/auth/4113>
- "authorized_user" => {
- let token = AuthorizedUserCredentials {
- client_id: file.client_id,
- client_secret: file.client_secret,
- refresh_token: file.refresh_token,
- };
-
- Ok(Some(Arc::new(TokenCredentialProvider::new(
- token,
- client.client()?,
- retry.clone(),
- ))))
- }
- type_ => Err(UnsupportedCredentialsType {
- type_: type_.to_string(),
- }
- .into()),
- }
-}
-
/// A deserialized `application_default_credentials.json`-file.
+///
///
<https://cloud.google.com/docs/authentication/application-default-credentials#personal>
+/// <https://google.aip.dev/auth/4110>
Review Comment:
```suggestion
/// # References
/// -
<https://cloud.google.com/docs/authentication/application-default-credentials#personal>
/// - <https://google.aip.dev/auth/4110>
```
##########
object_store/src/gcp/credential.rs:
##########
@@ -290,17 +268,19 @@ impl ServiceAccountCredentials {
serde_json::from_str(key).context(DecodeCredentialsSnafu)
}
- /// Create an [`OAuthProvider`] from this credentials struct.
- pub fn oauth_provider(
- self,
- scope: &str,
- audience: &str,
- ) -> crate::Result<OAuthProvider> {
- Ok(OAuthProvider::new(
+ /// Create a [`SelfSignedJwt`] from this credentials struct.
+ ///
+ /// We use a scope of [`DEFAULT_SCOPE`] as opposed to an audience
+ /// as GCS appears to not support audience
+ ///
+ ///
<https://stackoverflow.com/questions/63222450/service-account-authorization-without-oauth-can-we-get-file-from-google-cloud/71834557#71834557>
+ ///
<https://www.codejam.info/2022/05/google-cloud-service-account-authorization-without-oauth.html>
Review Comment:
This goes for other reference lists as well: Since markdown is free to
render these on one line, I think they would be easier to read like this:
```suggestion
/// # References
/// -
<https://stackoverflow.com/questions/63222450/service-account-authorization-without-oauth-can-we-get-file-from-google-cloud/71834557#71834557>
/// -
<https://www.codejam.info/2022/05/google-cloud-service-account-authorization-without-oauth.html>
```
##########
object_store/src/gcp/credential.rs:
##########
@@ -144,61 +144,61 @@ struct TokenResponse {
expires_in: u64,
}
-/// Encapsulates the logic to perform an OAuth token challenge
+/// <https://google.aip.dev/auth/4111>
Review Comment:
```suggestion
/// Self-signed JWT (JSON Web Token).
///
/// # References
/// - <https://google.aip.dev/auth/4111>
```
##########
object_store/src/gcp/credential.rs:
##########
@@ -404,62 +369,29 @@ impl TokenProvider for InstanceCredentialProvider {
}
}
-/// ApplicationDefaultCredentials
-/// <https://google.aip.dev/auth/4110>
-pub fn application_default_credentials(
- path: Option<&str>,
- client: &ClientOptions,
- retry: &RetryConfig,
-) -> crate::Result<Option<GcpCredentialProvider>> {
- let file = match ApplicationDefaultCredentialsFile::read(path)? {
- Some(x) => x,
- None => return Ok(None),
- };
-
- match file.type_.as_str() {
- // <https://google.aip.dev/auth/4113>
- "authorized_user" => {
- let token = AuthorizedUserCredentials {
- client_id: file.client_id,
- client_secret: file.client_secret,
- refresh_token: file.refresh_token,
- };
-
- Ok(Some(Arc::new(TokenCredentialProvider::new(
- token,
- client.client()?,
- retry.clone(),
- ))))
- }
- type_ => Err(UnsupportedCredentialsType {
- type_: type_.to_string(),
- }
- .into()),
- }
-}
-
/// A deserialized `application_default_credentials.json`-file.
+///
///
<https://cloud.google.com/docs/authentication/application-default-credentials#personal>
+/// <https://google.aip.dev/auth/4110>
#[derive(serde::Deserialize)]
-struct ApplicationDefaultCredentialsFile {
- #[serde(default)]
- client_id: String,
- #[serde(default)]
- client_secret: String,
- #[serde(default)]
- refresh_token: String,
- #[serde(rename = "type")]
- type_: String,
+#[serde(tag = "type")]
+pub enum ApplicationDefaultCredentials {
+ /// <https://google.aip.dev/auth/4112>
Review Comment:
```suggestion
/// Service Account.
///
/// # References
/// - <https://google.aip.dev/auth/4112>
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
