Supporting service account key format OR user credential formats #76

msdrigg · 2023-08-18T13:53:08Z

This PR supports parsing both formats in either the
GOOGLE_APPLICATION_CREDENTIALS env variable or the
~/.config/gcloud/application_default_credentials.json file.

This is part 1 of the PRs addressing #56

For context see
#75 (comment)

This PR supports parsing both formats in either the `GOOGLE_APPLICATION_CREDENTIALS` env variable or the `~/.config/gcloud/application_default_credentials.json` file.

msdrigg · 2023-08-18T13:53:48Z

@djc I need to do real world testing on this part, but is this what you're looking for as of part 1?

src/default_authorized_user.rs

This PR adds a new `ServiceAccount` format that takes credentials from `source_credentials: ServiceAccount` and then makes a request to get a service account token using those credentials. This also adds the ability to parse the token format created by `gcloud auth application-default login --impersonate-service-account <service account>`

msdrigg · 2023-08-18T18:09:48Z

Tested this PR with real world keys

Tested with GOOGLE_APPLICATION_CREDENTIALS and ~/.config/gcloud/application_default_credentials.json and verified the new tagged enum is used in both cases
Tested with service account key and user creds, and verified that I can make API calls

djc

This is nice and focused, thanks. I propose we scale it down a little more.

src/flexible_credential_source.rs

djc · 2023-08-21T08:18:39Z

src/flexible_credential_source.rs

+    pub(crate) async fn from_default_credentials() -> Result<Self, Error> {
+        tracing::debug!("Loading user credentials file");
+        let mut home = dirs_next::home_dir().ok_or(Error::NoHomeDir)?;
+        home.push(Self::USER_CREDENTIALS_PATH);


Is it an actually expected use case that we find ApplicationCredentials in the USER_CREDENTIALS_PATH? That doesn't seem like something that makes sense.

I agree that this isn't a typical pattern, but Google's documentation refers to both the GOOGLE_APPLICATION_CREDENTIALS and the ~/.config/gcloud/application_default_credentials.json file as types of application default credentials, so it feels reasonable that we accept both behaviors.

https://cloud.google.com/docs/authentication/application-default-credentials#order

Additionally in the go source I have been referencing, all these credential types are contained in one struct and there isn't any runtime checks to restrict usage between certain types.

One more thing: gcloud accepts this behavior. I can run

gcloud iam service-accounts keys create ~/.config/gcloud/application_default_credentials.json --iam-account=<service act> gcloud auth application-default print-access-token

and the service account's token will get printed.

Okay, in that case I suggest that we do this at the top level:

Check for the GOOGLE_APPLICATION_CREDENTIALS environment variable

If that doesn't exist, check the USER_CREDENTIALS_PATH

For both of these, deserialize and pass the results to the appropriate trait impl

.. other options

Are you saying to return an error (don't try other options) if USER_CREDENTIALS_PATH exists but fails to parse as a valid ServiceAccount for some reason. This is the current behavior for GOOGLE_APPLICATION_CREDENTIALS but not USER_CREDENTIALS_PATH

I hadn't really thought about it that way, but it seems kinda reasonable to me? Or do you think the USER_CREDENTIALS_PATH could contain other things? What does the Go code do in this case?

Hmm. The gcloud isn't open source, and the referenced go code doesn't cover that situation. I think keeping the current behavior is fine. I don't see a reason to change since we already have been using the current behavior -- I just wanted to clarify if you were expecting that.

src/flexible_credential_source.rs

src/authentication_manager.rs

msdrigg · 2023-08-24T23:27:34Z

This should be ready for final checks at this point

djc · 2023-08-25T08:27:35Z

This is on my list to review, but it's been busy. Hope to get to it soon.

msdrigg · 2023-08-25T13:52:30Z

No rush. Just wanted to make sure it wasn't waiting on me.

valkum · 2024-10-28T16:38:46Z

Hey there, running into this as well when trying to run a Rust binary in a Docker container that is not running on GCP and has no gcloud CLI installed.
The way to do this is by creating application default credentials using gcloud auth application-default login (maybe even impersonating a specific service account) and then passing the GOOGLE_APPLICATION_CREDENTIALS env var and file to the docker container.
See https://cloud.google.com/run/docs/testing/local#docker-with-google-cloud-access and https://medium.com/google-cloud/use-google-cloud-user-credentials-when-testing-containers-locally-acb57cd4e4da
This results in the error in the linked issue.

I don't think this PR supports impersonation via GOOGLE_APPLICATION_CREDENTIALS.
The content of application_default_credentials.json for gcloud auth application-default login --impersonate-service-account test-sa@test.iam.gserviceaccount.com looks like:

{
  "delegates": [],
  "service_account_impersonation_url": "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/
test@test.iam.gserviceaccount.com:generateAccessToken",
  "source_credentials": {
    "account": "",
    "client_id": "...",
    "client_secret": "...,
    "refresh_token": "...",
    "type": "authorized_user",
    "universe_domain": "googleapis.com"
  },
  "type": "impersonated_service_account"
}

The source for processing this content can be found here: https://github.com/golang/oauth2/blob/master/google/google.go#L223

@djc @msdrigg If you like I can pick work on this up again.

msdrigg · 2024-10-28T16:40:38Z

I wouldnt mind. I likely won't have time for a while to finish this

djc · 2024-10-29T09:06:02Z

Happy to review a PR for this.

Supporting service account key format OR user credential formats

925dbba

This PR supports parsing both formats in either the `GOOGLE_APPLICATION_CREDENTIALS` env variable or the `~/.config/gcloud/application_default_credentials.json` file.

msdrigg commented Aug 18, 2023

View reviewed changes

src/default_authorized_user.rs Show resolved Hide resolved

msdrigg mentioned this pull request Aug 18, 2023

Supporting impersonated service count (builds on #76) #77

Open

djc reviewed Aug 21, 2023

View reviewed changes

src/flexible_credential_source.rs Outdated Show resolved Hide resolved

Adding recommended changes

d452a85

msdrigg commented Aug 21, 2023

View reviewed changes

src/authentication_manager.rs Show resolved Hide resolved

msdrigg commented Aug 21, 2023

View reviewed changes

src/authentication_manager.rs Show resolved Hide resolved

Fixing rust 1.72 fmt check

7cc714a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Supporting service account key format OR user credential formats #76

Supporting service account key format OR user credential formats #76

msdrigg commented Aug 18, 2023

msdrigg commented Aug 18, 2023

msdrigg commented Aug 18, 2023

djc left a comment

djc Aug 21, 2023

msdrigg Aug 21, 2023 •

edited

Loading

msdrigg Aug 21, 2023 •

edited

Loading

djc Aug 21, 2023

msdrigg Aug 21, 2023 •

edited

Loading

djc Aug 21, 2023

msdrigg Aug 21, 2023

msdrigg commented Aug 24, 2023

djc commented Aug 25, 2023

msdrigg commented Aug 25, 2023

valkum commented Oct 28, 2024

msdrigg commented Oct 28, 2024

djc commented Oct 29, 2024

Supporting service account key format OR user credential formats #76

Are you sure you want to change the base?

Supporting service account key format OR user credential formats #76

Conversation

msdrigg commented Aug 18, 2023

msdrigg commented Aug 18, 2023

msdrigg commented Aug 18, 2023

djc left a comment

Choose a reason for hiding this comment

djc Aug 21, 2023

Choose a reason for hiding this comment

msdrigg Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

msdrigg Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

djc Aug 21, 2023

Choose a reason for hiding this comment

msdrigg Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

djc Aug 21, 2023

Choose a reason for hiding this comment

msdrigg Aug 21, 2023

Choose a reason for hiding this comment

msdrigg commented Aug 24, 2023

djc commented Aug 25, 2023

msdrigg commented Aug 25, 2023

valkum commented Oct 28, 2024

msdrigg commented Oct 28, 2024

djc commented Oct 29, 2024

msdrigg Aug 21, 2023 •

edited

Loading

msdrigg Aug 21, 2023 •

edited

Loading

msdrigg Aug 21, 2023 •

edited

Loading