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

auth: Fail on invalid Google Service Account JSON #10429

Closed
whs opened this issue Jun 25, 2024 · 5 comments
Closed

auth: Fail on invalid Google Service Account JSON #10429

whs opened this issue Jun 25, 2024 · 5 comments
Assignees
Labels
status: investigating The issue is under investigation, which is determined to be non-trivial. triage me I really want to be triaged.

Comments

@whs
Copy link

whs commented Jun 25, 2024

Is your feature request related to a problem? Please describe.
In the previous oauth2 library, if the Google Service Account JSON is invalid, an error is thrown.

In the new oauth2 library, the error is silently dropped. This make troubleshooting service account JSON near impossible.

We're using external account JSON with emulated Google STS server that calls to HashiCorp Vault, so the JSON is written by hand.

Describe the solution you'd like
If GOOGLE_APPLICATION_CREDENTIALS is present and the JSON is invalid in anyway (unreadable, unparsable, invalid schema, etc.), then it should return an error message

Describe alternatives you've considered
Use a debugger to check the err, which is not possible in production.

Other context
In our case the audience field is handled by Vault, so we leave it empty but the new SDK do not allow that. Perhaps that might not be all of the problems though since there's no concrete error message.

@whs whs added the triage me I really want to be triaged. label Jun 25, 2024
@codyoss codyoss self-assigned this Jun 25, 2024
@codyoss codyoss added the status: investigating The issue is under investigation, which is determined to be non-trivial. label Jun 25, 2024
@codyoss
Copy link
Member

codyoss commented Jun 25, 2024

@whs In your case, what was the old error message in these cases? Go's default json unmarshalling is very permissive in general.

@whs
Copy link
Author

whs commented Jun 25, 2024

In my case the old library work just fine. The new library do require the audience field to not be empty:

if o.Audience == "" {
return fmt.Errorf("externalaccount: Audience must be set")
}

@whs
Copy link
Author

whs commented Jun 25, 2024

Looking at the source it seems that the check was added when externalaccount was moved from internal to exported

https://cs.opensource.google/go/x/oauth2/+/refs/tags/v0.18.0:google/externalaccount/basecredentials.go;l=205-207;bpv=1;bpt=0

I think the previous application that used our STS service was deployed last year, so it was before this was added.

@codyoss
Copy link
Member

codyoss commented Jun 25, 2024

Should have a patch shortly. Audience should have always been required before I believe looking at the spec: https://google.aip.dev/auth/4117#expected-behavior . This was a bug in the old library I would say.

@codyoss
Copy link
Member

codyoss commented Jun 25, 2024

fixed in #10431. Should be released today yet.

@codyoss codyoss closed this as completed Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: investigating The issue is under investigation, which is determined to be non-trivial. triage me I really want to be triaged.
Projects
None yet
Development

No branches or pull requests

2 participants