fix: Fix to delete cookie when AppSubURL is non-empty (#30375)

Cookies may exist on "/subpath" and "/subpath/" for some legacy reasons (eg: changed CookiePath behavior in code). The legacy cookie should be removed correctly.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Kyle D <kdumontnu@gmail.com>
(cherry picked from commit b18c04ebde94e23d97da4958173faea843d5344f)
This commit is contained in:
Jonathan Tran 2024-04-14 00:46:56 -04:00 committed by Gergely Nagy
parent bf933f5ca4
commit 7ff74a47c8
No known key found for this signature in database
3 changed files with 37 additions and 7 deletions

View file

@ -6,6 +6,9 @@ package session
import ( import (
"net/http" "net/http"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/web/middleware"
"gitea.com/go-chi/session" "gitea.com/go-chi/session"
) )
@ -18,6 +21,10 @@ type Store interface {
// RegenerateSession regenerates the underlying session and returns the new store // RegenerateSession regenerates the underlying session and returns the new store
func RegenerateSession(resp http.ResponseWriter, req *http.Request) (Store, error) { func RegenerateSession(resp http.ResponseWriter, req *http.Request) (Store, error) {
// Ensure that a cookie with a trailing slash does not take precedence over
// the cookie written by the middleware.
middleware.DeleteLegacySiteCookie(resp, setting.SessionConfig.CookieName)
s, err := session.RegenerateSession(resp, req) s, err := session.RegenerateSession(resp, req)
return s, err return s, err
} }

View file

@ -45,10 +45,32 @@ func SetSiteCookie(resp http.ResponseWriter, name, value string, maxAge int) {
SameSite: setting.SessionConfig.SameSite, SameSite: setting.SessionConfig.SameSite,
} }
resp.Header().Add("Set-Cookie", cookie.String()) resp.Header().Add("Set-Cookie", cookie.String())
if maxAge < 0 { // Previous versions would use a cookie path with a trailing /.
// There was a bug in "setting.SessionConfig.CookiePath" code, the old default value of it was empty "". // These are more specific than cookies without a trailing /, so
// So we have to delete the cookie on path="" again, because some old code leaves cookies on path="". // we need to delete these if they exist.
cookie.Path = strings.TrimSuffix(setting.SessionConfig.CookiePath, "/") DeleteLegacySiteCookie(resp, name)
resp.Header().Add("Set-Cookie", cookie.String()) }
}
// DeleteLegacySiteCookie deletes the cookie with the given name at the cookie
// path with a trailing /, which would unintentionally override the cookie.
func DeleteLegacySiteCookie(resp http.ResponseWriter, name string) {
if setting.SessionConfig.CookiePath == "" || strings.HasSuffix(setting.SessionConfig.CookiePath, "/") {
// If the cookie path ends with /, no legacy cookies will take
// precedence, so do nothing. The exception is that cookies with no
// path could override other cookies, but it's complicated and we don't
// currently handle that.
return
}
cookie := &http.Cookie{
Name: name,
Value: "",
MaxAge: -1,
Path: setting.SessionConfig.CookiePath + "/",
Domain: setting.SessionConfig.Domain,
Secure: setting.SessionConfig.Secure,
HttpOnly: true,
SameSite: setting.SessionConfig.SameSite,
}
resp.Header().Add("Set-Cookie", cookie.String())
} }

View file

@ -9,6 +9,7 @@ import (
"net/http" "net/http"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
session_module "code.gitea.io/gitea/modules/session"
chiSession "gitea.com/go-chi/session" chiSession "gitea.com/go-chi/session"
"github.com/gorilla/sessions" "github.com/gorilla/sessions"
@ -65,7 +66,7 @@ func (st *SessionsStore) Save(r *http.Request, w http.ResponseWriter, session *s
chiStore := chiSession.GetSession(r) chiStore := chiSession.GetSession(r)
if session.IsNew { if session.IsNew {
_, _ = chiSession.RegenerateSession(w, r) _, _ = session_module.RegenerateSession(w, r)
session.IsNew = false session.IsNew = false
} }