feat(option): Deprecate unsafe credentials JSON loading options#3356
feat(option): Deprecate unsafe credentials JSON loading options#3356quartzmo merged 13 commits intogoogleapis:mainfrom
Conversation
quartzmo
force-pushed
the
option-credentials-json
branch
3 times, most recently
from
6a6e6f3 to
acb5230
Compare
quartzmo
changed the title
Add safer credentials JSON loading options. Add option.WithAuthCredentialsFile and option.WithAuthCredentialsJSON to mitigate a security vulnerability where credential configurations from untrusted sources could be used without validation. These new functions require the credential type to be explicitly specified. Deprecate the less safe option.WithCredentialsFile and option.WithCredentialsJSON functions.
quartzmo
force-pushed
the
option-credentials-json
branch
from
acb5230 to
84794e0
Compare
shollyman
left a comment
shollyman
left a comment
There was a problem hiding this comment.
Should there be tests in this PR where the file/json contents and the credential type are mismatched? I suspect they belong closer to wherever the validation logic lives, but I wanted to doublecheck with you.
Great question. The current plan is to pass the credentials type from this options layer to the auth packages ( |
quartzmo
added
the
do not merge
|
Add |
| // more information, refer to [Validate credential configurations from | ||
| // external sources](https://cloud.google.com/docs/authentication/external/externally-sourced-credentials). | ||
| // | ||
| // Deprecated: This function is being deprecated because of a potential security risk. |
There was a problem hiding this comment.
Is this go's way of deprecating?
No tags or something like that?
There was a problem hiding this comment.
Yes, from Gemini:
How to Deprecate a Function in Go:
Add a "Deprecated:" paragraph to the doc comment: Within the godoc comment for the function, include a paragraph that starts with the literal string "Deprecated:". This paragraph can be placed anywhere within the doc comment, not necessarily at the end.
Provide context and alternatives: Following "Deprecated:", explain why the function is deprecated and, if applicable, recommend what users should use instead. This helps users understand the reason for the change and provides a clear path forward.
quartzmo
force-pushed
the
option-credentials-json
branch
from
afa1abe to
ea9bbd2
Compare
quartzmo
removed
the
do not merge
| } | ||
| switch allowedType { | ||
| case serviceAccount: | ||
| switch credType { |
There was a problem hiding this comment.
Do we need to worry about other types like GDCHServiceAccount or they simply can't be used via a byte slice?
There was a problem hiding this comment.
This limited set of types preserves existing behavior. Also, this idtoken package is legacy, and users should migrate to cloud.google.com/go/auth/idtoken.
There was a problem hiding this comment.
I don't see mention in https://github.com/googleapis/google-api-go-client/blob/main/idtoken/doc.go about recommending the migration. Should we include that guidance (either in this PR or a followup)?
shollyman
added
the
kokoro:force-run
kokoro-team
removed
the
kokoro:force-run
4 participants
Add safer credentials JSON loading options.
Add option.WithAuthCredentialsFile and option.WithAuthCredentialsJSON
to mitigate a security vulnerability where credential configurations
from untrusted sources could be used without validation. These new
functions require the credential type to be explicitly specified.
Deprecate the less safe option.WithCredentialsFile and
option.WithCredentialsJSON functions.
refs: googleapis/google-cloud-go#13397