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

gcpkms: Add support for oauth tokens using env var #1188

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

warwick-mitchell1
Copy link

Add support for CLOUDSDK_AUTH_ACCESS_TOKEN environment to pass though an oauth access token directly or via a file.

Add support for CLOUDSDK_AUTH_ACCESS_TOKEN environment to pass though an
oauth access token directly or via a file.
credentials := []byte(envCredentials)
if _, err := os.Stat(envCredentials); err == nil {
if credentials, err = os.ReadFile(envCredentials); err != nil {
return nil, err

Choose a reason for hiding this comment

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

Perhaps don't exit here, but instead check SopsGoogleCredentialsOAuthEnv as well?

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

You maybe right, this isn't the part I'm really adding with this PR though, this was already like this. I'm adding the Oauth token below which isn't covered by application-defaultcreds

Choose a reason for hiding this comment

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

I understand this. However, with your added functionality I believe it makes sense to also adjust the old behavior. If any fail condition happens with the credentials by file lookup, continue to check whether credentials by token area available before failing completely.

@moritzschmitz-oviva
Copy link

Couldn't find anything in the Google docs, but the Terraform provider documentation provides a few more (and different) environment variables: https://registry.terraform.io/providers/hashicorp/google/latest/docs/guides/provider_reference#authentication-configuration

@hiddeco hiddeco added this to the v3.9.0 milestone Jul 3, 2023
@moritzschmitz-oviva
Copy link

Let's do this!

Comment on lines +252 to +263
if envToken, isSet := os.LookupEnv(SopsGoogleCredentialsOAuthEnv); isSet {
token := []byte(envToken)
if _, err := os.Stat(envToken); err == nil {
if token, err = os.ReadFile(envToken); err != nil {
return nil, err
}
}
tokenSource := oauth2.StaticTokenSource(
&oauth2.Token{AccessToken: string(token)},
)
return option.WithTokenSource(tokenSource), nil
}
Copy link
Member

Choose a reason for hiding this comment

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

I think we should not check for a file here, but rather assume what's documented in https://cloud.google.com/sdk/docs/authorizing. Which only mentions:

Set the CLOUDSDK_AUTH_ACCESS_TOKEN environment variable to the access token value.

@dirsigler
Copy link

are there any blockers for this?

I currently also facing issues with SOPS using Googles access token.

@mincua
Copy link

mincua commented Sep 7, 2024

bumping this up, what's blocking it? need support for ODIC token auth

@moritzschmitz-oviva
Copy link

@warwick-mitchell1 Do you plan on continuing the PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants