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

OAuth: Fix token refresh failure when custom SSL settings are configured for OAuth provider #27523

Merged
merged 5 commits into from Sep 11, 2020

Conversation

billoley
Copy link
Contributor

Fixes #27514

OAuth token refresh fails when custom SSL settings are configured for oauth provider

When using generic oauth with SSL configured and tls_skip_verify_insecure=true, I was able to log in with my oauth provider, but when the token expired and grafana tried to fetch a new token (using the refresh token), I saw http ssl errors.

When the original token is fetched in login_oauth.go, the code gets an http client based on the appropriate oauth settings and setit into the context to be used. This same functionality should be centralized and used from wherever oauth token http operations are initiated.

Moved code for creating an http client using the OAuth SSL setting to a new package oauthtoken.
This allows code to be refactored from login_oauth.go so that it can be used from there, ds_proxy.go, and anywhere else that calls OAuth APIs and needs to configure a client.

… oauth provider (grafana#27514)

OAuth token refresh fails when custom SSL settings are configured for oauth provider (grafana#27514)

OAuth token refresh fails when custom SSL settings are configured for oauth provider (grafana#27514)
@marefr marefr added area/backend pr/external This PR is from external contributor area/auth/oauth labels Sep 11, 2020
Copy link
Member

@marefr marefr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. However, I have some minor suggestions regarding code structure.

pkg/services/oauthtoken/oauth_http_client.go Outdated Show resolved Hide resolved
pkg/api/pluginproxy/ds_proxy.go Outdated Show resolved Hide resolved
Copy link
Member

@marefr marefr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. LGTM

@marefr marefr changed the title OAuth token refresh fails when custom SSL settings are configured for… OAuth: Fix token refresh failure when custom SSL settings are configured for OAuth provider Sep 11, 2020
@marefr marefr merged commit 19caa10 into grafana:master Sep 11, 2020
@marefr
Copy link
Member

marefr commented Sep 11, 2020

Thank you for contributing to Grafana!

@marefr marefr added this to the 7.3 milestone Sep 11, 2020
billoley added a commit to billoley/grafana that referenced this pull request Sep 14, 2020
…red for OAuth provider (grafana#27523)

OAuth token refresh fails when custom SSL settings are configured for
oauth provider. These changes makes sure that custom SSL settings
are applied for HTTP client before refreshing token.

Fixes grafana#27514
billoley added a commit to billoley/grafana that referenced this pull request Sep 25, 2020
…red for OAuth provider (grafana#27523)

OAuth token refresh fails when custom SSL settings are configured for
oauth provider. These changes makes sure that custom SSL settings
are applied for HTTP client before refreshing token.

Fixes grafana#27514
@wbrowne wbrowne mentioned this pull request Oct 15, 2020
ryantxu pushed a commit that referenced this pull request Nov 18, 2020
…red for OAuth provider (#27523)

OAuth token refresh fails when custom SSL settings are configured for 
oauth provider. These changes makes sure that custom SSL settings 
are applied for HTTP client before refreshing token.

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

Successfully merging this pull request may close these issues.

OAuth token refresh fails when custom SSL settings are configured for oauth provider
2 participants