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: Fix signout redirect url #87631
Conversation
@@ -875,7 +875,6 @@ auto_sign_up = false | |||
url_login = false | |||
allow_assign_grafana_admin = false | |||
skip_org_role_sync = false | |||
signout_redirect_url = |
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.
this value is never read into jwt config
This PR must be merged before a backport PR will be created. |
1 similar comment
This PR must be merged before a backport PR will be created. |
@@ -254,6 +254,7 @@ func (hs *HTTPServer) Logout(c *contextmodel.ReqContext) { | |||
if err != nil { | |||
hs.log.Error("Failed perform proper logout", "error", err) | |||
c.Redirect(hs.Cfg.AppSubURL + "/login") | |||
return |
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.
Panic logs displayed in issues was from here. When something wen't wrong we did not return here and on line 262 we tried to use redirect.URL
i.e. de-referencing a nil pointer
I think this also fixes this issue: #79839 |
* Add missing return * Use sign out redirect url from auth config if configured * remove option from auth.jwt that is not used (cherry picked from commit 0f3080e)
What is this feature?
After #79635 we did not respect
signout_redirect_url
configured under[auth]
. It only worked with saml and oauth.To fix this we instead default to use
signout_redirect_url
if configured. A client that implementLogoutClient
will still take precedence. We also need to ignore the case when users don't have a session token so that the configured redirect url is used.Which issue(s) does this PR fix?:
Fixes #85128
Fixes #84108
Special notes for your reviewer:
Please check that: