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

Generic OAuth Authentication send_client_credentials_via_post unexpected behaviour #15408

Closed
ivoadf opened this issue Feb 13, 2019 · 3 comments
Closed

Comments

@ivoadf
Copy link

ivoadf commented Feb 13, 2019

What Grafana version are you using?

v5.4.3

What OS are you running grafana on?

CentOS-7

What did you do?

Configured generic OAuth to send client_id and client_secret via POST.
Using the parameter: send_client_credentials_via_post = true as is described on the documentation here.

What was the expected result?

Expected the client_id and client_secret to be sent via POST body.

What happened instead?

Activating the flag had no effect. The client credentials are still sent in the header.

Problem origin

After digging around the code, I saw that setting the flag to true would result on the function oauth2.RegisterBrokenAuthHeaderProvider(info.TokenUrl) being called, see here.

After checking oauth2 documentation I verified that this function is deprecated and as of the moment has no effect (explaining the behavior observed). See function documentation.

Problem solution

To restore the expected behavior to the send_client_credentials_via_post a small change is required. To define the way authentication credentials are sent to the server oauth2 uses the AuthStyle parameter on the Endpoint struct. So looking at the code we need to change this line to set the AuthStyle value to 1.

    // AuthStyleInParams sends the "client_id" and "client_secret"
    // in the POST body as application/x-www-form-urlencoded parameters.
    AuthStyleInParams AuthStyle = 1

and then make sure that this AuthStyle value is set in the Endpoint structs here and here.

In your opinion how long will it take to include this fix on a new grafana version?

PS: I am not familiar with the go language or with development of the grafana project, but I will allocate some time to try and fix this simple bug and send a pull request. Just sharing this so that if someone more familiar with the project wants to quickly fix this can do so and not be waiting for my pull request.

@marefr
Copy link
Member

marefr commented Feb 13, 2019

This feature was fixed for #14562 and will be included in Grafana v6.0 stable. Will make sure to update the documentation to reflect this.

@marefr marefr closed this as completed Feb 13, 2019
@ivoadf
Copy link
Author

ivoadf commented Feb 13, 2019

I am sorry but I do not understand what you mean by "fixed for".
I can see that the branch v5.4x does not contain any changes relating to send_client_credentials_via_post, this means it's an error on the documentation of v.5.4x?

But ignoring that mistake. From what I can tell, in #14562 the problem I highlighted is present. The function used is deprecated in the oauth2 library. Am I missing something trivial?

@marefr
Copy link
Member

marefr commented Feb 13, 2019

I meant that I will make sure to update the documentation to reflect that 5.4.x documentation has a version note regarding this and that Grafana v6.0 is required.

Oauth2 deprecated that recently and we're not using the latest version in Grafana (yet), see code. But good point that this could be a potential future problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants