Support empty OAuth2 inline secrets#547
Conversation
bwplotka
left a comment
There was a problem hiding this comment.
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 <thespiritxiii@gmail.com>
a429950 to
c9b5e71
Compare
bwplotka
left a comment
There was a problem hiding this comment.
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_filealso 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_secretis an empty string, that will be used for the secret.This also adds a subtest on
client_secret_filefor testing with an empty file.cc @roidelapluie who seems to have written most of the original code. :)