Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork6.3k
Enable logout redirection for reverse proxy setups#36085
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
Uh oh!
There was an error while loading.Please reload this page.
routers/web/web.go Outdated
| m.Post("/recover_account",auth.ResetPasswdPost) | ||
| m.Get("/forgot_password",auth.ForgotPasswd) | ||
| m.Post("/forgot_password",auth.ForgotPasswdPost) | ||
| m.Get("/logout",auth.SignOut) |
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.
What's the point of this route? Convenience for testing?
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.
The button gets also changed to do a GET request by settinghref. As I understand it, a GET request is needed to also cleanly redirect the user to the external 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.
We could think about simplifying the "Sign out" button to just do a GET /user/logout always, to not have to check whether we have this new config setting.
silverwindDec 4, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Hmm I think we ought to keep the POST for compatibilty reasons. Some users may have special integrations with the POST route in their reverse proxy.
silverwindDec 4, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
As I understand it, a GET request is needed to also cleanly redirect the user to the external URL.
If I understand HTTP correctly, one can also redirect in a POST response, can you try that? Logging out is a action that likely changes state on the server so a GET is semantically incorrect.
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.
Thought that does present a challenge in the styling because we don't have CSS to style a button like a link. Could you show a screenshot of how this link currently looks?
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.
Screencast.From.2025-12-04.22-27-40.mp4
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.
after checking the code, maybe I know why you think post atcion can't be used for redirect action, first, we should comfirm curent logout steps: first do POST/logout by js, backed will response a json struct wich containredirect=xxxx, then js will call a speciall post action to/-/fetch-redirect with a form contain redirect link, then the backen will do a redirect.
ref:
but sadly, in current design,/-/fetch-redirect will will block link to ather service, maybe we can remove this block, or render a warning page with a link for it. /cc@wxiaoguang
wxiaoguangDec 5, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
but sadly, in current design,
/-/fetch-redirectwill will block link to ather service, maybe we can remove this block, or render a warning page with a link for it. /cc@wxiaoguang
No, the design is right. Otherwise "open redirect" security vulnerabilities.
You can allow more URL patterns which are known to be safe, but not remove it or bypass it.
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.
Updated the PR so that the "Sign Out" link does an actual POST, please review.
d093cd4 to8d868acCompareeliroca commentedDec 9, 2025
Updated the test, POST on |
When authentication is handled externally by a reverse proxy or SSO provider,users can be redirected to an external logout URL or relative pathdefined on the reverse proxy.
63a3a81 tofa7ee5aCompare| <divclass="divider"></div> | ||
| <aclass="item link-action" hrefdata-url="{{AppSubUrl}}/user/logout"> | ||
| <formid="logout-form"method="post"action="{{AppSubUrl}}/user/logout"></form> | ||
| <aclass="item"onclick="document.getElementById('logout-form').submit();"> |
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.
It’s not recommended to use an event handler in this style.
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.
I'm open to suggestions.@silverwind, you wrote:
inline
<form>to trigger the POST and change the link to a button with type=submit, so it works without JS.
Thought that does present a challenge in the styling because we don't have CSS to style a button like a link.
Could you help with styling a button?
silverwindDec 11, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
Yeah I guess I can take a look a bit later. We need to move the event handler to TS (inline scripts are a no-go for strict CSP) and do the styling.
When authentication is handled externally by a reverse proxy or SSO provider, users can be redirected to an external logout URL or relative path defined on the reverse proxy.