-
Notifications
You must be signed in to change notification settings - Fork 12.3k
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
AuthN: Support reloading SSO config after the sso settings have changed #80734
Conversation
…svc-reload-support
f07ca07
to
14d3b34
Compare
14d3b34
to
a7d61b5
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.
Great work! Left some comments 👍
aa792c0
to
78c24d8
Compare
78c24d8
to
95728a9
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.
LGTM, left one last comment
func (s *SocialGenericOAuth) Reload(ctx context.Context, settings ssoModels.SSOSettings) error { | ||
newInfo, err := CreateOAuthInfoFromKeyValues(settings.Settings) | ||
if err != nil { | ||
return fmt.Errorf("SSO settings map cannot be converted to OAuthInfo: %v", err) |
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.
We repeat this error in all providers, extract it?
What is this feature?
This PR adds the ability to reload OAuth provider configuration when SSO settings feature is enabled. Besides, it changes how the AuthN service handles OAuth clients (all of the clients will be registered on startup).
Why do we need this feature?
To update Grafana's OAuth client to support reloading configuration without restarting Grafana.
Who is this feature for?
Which issue(s) does this PR fix?:
Fixes #
Special notes for your reviewer:
Please check that: