Skip to content

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.

Sign up for GitHub

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

Open
eliroca wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Enable logout redirection for reverse proxy setups #36085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions custom/conf/app.example.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,11 @@ INTERNAL_TOKEN =
;; Name of cookie used to store authentication information.
;COOKIE_REMEMBER_NAME = gitea_incredible
;;
;; URL or path that Gitea should redirect users to *after* performing its own logout.
;; Use this to redirect user to the external logout endpoint, if needed, when authentication is handled by a reverse proxy or SSO.
;; Mellon example: REVERSE_PROXY_LOGOUT_REDIRECT = /mellon/logout?ReturnTo=/
;REVERSE_PROXY_LOGOUT_REDIRECT =
;;
;; Reverse proxy authentication header name of user name, email, and full name
;REVERSE_PROXY_AUTHENTICATION_USER = X-WEBAUTH-USER
;REVERSE_PROXY_AUTHENTICATION_EMAIL = X-WEBAUTH-EMAIL
Expand Down
2 changes: 2 additions & 0 deletions modules/setting/security.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ var (
ReverseProxyAuthEmail string
ReverseProxyAuthFullName string
ReverseProxyLimit int
ReverseProxyLogoutRedirect string
ReverseProxyTrustedProxies []string
MinPasswordLength int
ImportLocalPaths bool
Expand Down Expand Up @@ -121,6 +122,7 @@ func loadSecurityFrom(rootCfg ConfigProvider) {
ReverseProxyAuthFullName = sec.Key("REVERSE_PROXY_AUTHENTICATION_FULL_NAME").MustString("X-WEBAUTH-FULLNAME")

ReverseProxyLimit = sec.Key("REVERSE_PROXY_LIMIT").MustInt(1)
ReverseProxyLogoutRedirect = sec.Key("REVERSE_PROXY_LOGOUT_REDIRECT").MustString("")
ReverseProxyTrustedProxies = sec.Key("REVERSE_PROXY_TRUSTED_PROXIES").Strings(",")
if len(ReverseProxyTrustedProxies) == 0 {
ReverseProxyTrustedProxies = []string{"127.0.0.0/8", "::1/128"}
Expand Down
8 changes: 8 additions & 0 deletions routers/web/auth/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,14 @@ func SignOut(ctx *context.Context) {
})
}
HandleSignOut(ctx)
if setting.ReverseProxyLogoutRedirect != "" {
ctx.Redirect(setting.ReverseProxyLogoutRedirect)
return
}
if ctx.Req.Method == http.MethodGet {
ctx.Redirect(setting.AppSubURL + "/")
return
}
ctx.JSONRedirect(setting.AppSubURL + "/")
}

Expand Down
1 change: 1 addition & 0 deletions routers/web/web.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,7 @@ func registerWebRoutes(m *web.Router) {
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

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

What's the point of this route? Convenience for testing?

Copy link
Author

@eliroca eliroca Dec 4, 2025

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

@eliroca eliroca Dec 4, 2025

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 •
edited
Loading

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 •
edited
Loading

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

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

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

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

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

@eliroca eliroca Dec 4, 2025

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

@a1012112796 a1012112796 Dec 5, 2025

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 •
edited
Loading

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.

m.Post("/logout", auth.SignOut)
m.Get("/stopwatches", reqSignIn, user.GetStopwatches)
m.Get("/search_candidates", optExploreSignIn, user.SearchCandidates)
Expand Down
2 changes: 2 additions & 0 deletions services/context/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,8 @@ func Contexter() func(next http.Handler) http.Handler {
ctx.Data["ManifestData"] = setting.ManifestData
ctx.Data["AllLangs"] = translation.AllLangs()

ctx.Data["ReverseProxyLogoutRedirect"] = setting.ReverseProxyLogoutRedirect != ""

next.ServeHTTP(ctx.Resp, ctx.Req)
})
}
Expand Down
4 changes: 2 additions & 2 deletions templates/base/head_navbar.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
</div>

<div class="divider"></div>
<a class="item link-action" href data-url="{{AppSubUrl}}/user/logout">
<a class="item{{if not .ReverseProxyLogoutRedirect}} link-action" data-url={{else}}" href={{end}}"{{AppSubUrl}}/user/logout">
{{svg "octicon-sign-out"}}
{{ctx.Locale.Tr "sign_out"}}
</a>
Expand Down Expand Up @@ -128,7 +128,7 @@
</a>
{{end}}
<div class="divider"></div>
<a class="item link-action" href data-url="{{AppSubUrl}}/user/logout">
<a class="item{{if not .ReverseProxyLogoutRedirect}} link-action" data-url={{else}}" href={{end}}"{{AppSubUrl}}/user/logout">
{{svg "octicon-sign-out"}}
{{ctx.Locale.Tr "sign_out"}}
</a>
Expand Down
43 changes: 42 additions & 1 deletion tests/integration/signout_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import (
"net/http"
"testing"

"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/tests"
)

func TestSignOut(t *testing.T) {
func TestSignOut_Post(t *testing.T) {
defer tests.PrepareTestEnv(t)()

session := loginUser(t, "user2")
Expand All @@ -22,3 +24,42 @@ func TestSignOut(t *testing.T) {
req = NewRequest(t, "GET", "/user2/repo2")
session.MakeRequest(t, req, http.StatusNotFound)
}

func TestSignOut_Get(t *testing.T) {
defer tests.PrepareTestEnv(t)()

session := loginUser(t, "user2")

req := NewRequest(t, "GET", "/user/logout")
resp := session.MakeRequest(t, req, http.StatusSeeOther)

location := resp.Header().Get("Location")
if location != "/" {
t.Fatalf("expected redirect Location to '/', got %q", location)
}

// try to view a private repo, should fail
req = NewRequest(t, "GET", "/user2/repo2")
session.MakeRequest(t, req, http.StatusNotFound)
}

func TestSignOut_ReverseProxyLogoutRedirect(t *testing.T) {
defer tests.PrepareTestEnv(t)()

defer test.MockVariableValue(&setting.ReverseProxyLogoutRedirect, "/mellon/logout?ReturnTo=/")()

session := loginUser(t, "user2")

req := NewRequest(t, "GET", "/user/logout")
resp := session.MakeRequest(t, req, http.StatusSeeOther)

expected := "/mellon/logout?ReturnTo=/"
loc := resp.Header().Get("Location")
if loc != expected {
t.Fatalf("expected redirect to %q, got %q", expected, loc)
}

// try to view a private repo, should fail
req = NewRequest(t, "GET", "/user2/repo2")
session.MakeRequest(t, req, http.StatusNotFound)
}