Skip to content

Conversation

@eliroca
Copy link

@eliroca eliroca commented Dec 4, 2025

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.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 4, 2025
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files docs-update-needed The document needs to be updated synchronously labels Dec 4, 2025
m.Post("/recover_account", auth.ResetPasswdPost)
m.Get("/forgot_password", auth.ForgotPasswd)
m.Post("/forgot_password", auth.ForgotPasswdPost)
m.Get("/logout", auth.SignOut)
Copy link
Member

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?

Copy link
Author

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 setting href. As I understand it, a GET request is needed to also cleanly redirect the user to the external URL.

Copy link
Author

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.

Copy link
Member

@silverwind silverwind Dec 4, 2025

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.

Copy link
Member

@silverwind silverwind Dec 4, 2025

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.

Copy link
Member

Choose a reason for hiding this comment

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

I think we can create a small inline <form> to trigger the POST and change the link to a button with type=submit, so it works without JS.

Not sure how others feel but I think this case does not warrant the additional route.

Copy link
Member

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?

Copy link
Author

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

Copy link
Member

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 contain redirect=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:
image
image

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
image

Copy link
Contributor

@wxiaoguang wxiaoguang Dec 5, 2025

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-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

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.

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 to fully logout.
@eliroca eliroca force-pushed the logout_redirect_main branch from ceb6e88 to 23fd5f9 Compare December 4, 2025 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-update-needed The document needs to be updated synchronously lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants