-
Notifications
You must be signed in to change notification settings - Fork 326
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
Support empty OAuth2 inline secrets #547
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Epic work, LGTM!
Waiting for @roidelapluie for the final approve.
NOTE @TheSpiritXIII works in my Google Team
What is the use case? |
0e741ca
to
22046cf
Compare
My use-case is this that I am working on the Google Prometheus Operator for Kubernetes (which we call Managed Prometheus). We want to provide OAuth2 support and there's essentially two flows: User provides the client secret in a Kubernetes secret or they don't. If they do not specify a Kubernetes secret, the only way to get an empty client secret is to create an empty file and use that, and that involves a lot of setup: create a new empty directory, then use bash script to create an empty file, then mount it to the Prometheus instances and finally update the Prometheus configuration. If the inline client secrets was consistent and allowed empty client secrets like the file option, that would simplify my use-case dramatically. It would essentially be 1 step which is to just not provide the field in the Prometheus configuration. Why allow users to provide an empty client secret? Because the spec allows for it and some users might want it. For example, they may want to create a simple OAuth2 server which skips secret validation. If you Google-search for "oauth2 empty client string" you will see a plethora of users asking for the feature in various frameworks. It seems it's a common issue that applications do not implement the OAuth2 specification fully. |
22046cf
to
d67a57d
Compare
Hi @roidelapluie friendly ping :) do you have any thoughts on the motivating statements? As an additional motivating factor, BasicAuth |
d67a57d
to
a429950
Compare
Signed-off-by: Daniel Hrabovcak <[email protected]>
a429950
to
c9b5e71
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the consensus on the last DevSummit to support secret providers let's merging this so the work on providers is unblocked 🤗
If no objection, will merge later today.
Resolves #528.
client_secret_file
also supports using an empty file for no client secret. The same is not true forclient_secret
. This change makes it so that if neither are set or ifclient_secret
is an empty string, that will be used for the secret.This also adds a subtest on
client_secret_file
for testing with an empty file.cc @roidelapluie who seems to have written most of the original code. :)