-
Notifications
You must be signed in to change notification settings - Fork 345
Add option to select oauth auth method #436
Conversation
@apinnecke could you please file a Jira (https://issues.jboss.org/browse/KEYCLOAK), add the affected version and steps to reproduce the issue? It helps us to plan/prioritize the next releases. |
@abstractj this is a PR, not an issue. And you obviously didn‘t even read the PR, so please do so first an then merce the PR please. 🤷🏻♂️ |
@apinnecke Jiras are not only for bugs, but for features, tasks and other things as well. Please, read https://github.com/keycloak/keycloak-gatekeeper#help-and-documentation. |
The fact that client credentials are sent both in the basic header and the body is a bug for sure. Please file a JIRA as @abstractj has requested. |
@@ -49,6 +49,7 @@ func newDefaultConfig() *Config { | |||
MaxIdleConns: 100, | |||
MaxIdleConnsPerHost: 50, | |||
OAuthURI: "/oauth", | |||
OAuthAuthMethod: "basic", |
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.
Rename to 'ClientAuthMethod'. Supported values should be 'secret-basic' and 'secret-body'. It should be prefixed with 'secret-' to differentiate between other authentication mechanisms that may be added in the future. Also, it should be 'body' not 'post' (both basic and body are after all sent in a post).
It has been a while since we requested changes to this PR. While the submission is appreciated, we are not able to merge it without the changes request and a Jira as well. Because it has been so long, we are going to close the issue. Feel free to reopen with the changes included, if your time permits. |
Hi,
TL;DR:
This PR implements a config option to choose between basic and post authentication during communication between OAuth client and OAuth server. ✌️
Background:
I am currently building a kubernetes infrastructure for one of my clients and stumbled across this issue yesterday during implementing this gatekeeper as an OIDC proxy for https://onelogin.com . The issue I ran into is the following: https://stackoverflow.com/questions/52635508/what-is-the-meaning-of-this-error-client-authentication-must-only-be-provided , which states to use either basic or post authentication. 🤔
Due to the fact that the gatekeeper is still using go-oidc v1 (as decided and argued about in #407) it is still having a bug as described in coreos/go-oidc#181 which sets the post authentication even when basic is used, so the only quick chance for me to get this one up and running is to implement a config option to let the user choose between basic and post auth (which fixes the problem for me, tested with custom build in my cluster). 😑