From 7ff74a47c8d3222160437affcf56b4ab7a819aa5 Mon Sep 17 00:00:00 2001
From: Jonathan Tran <jonnytran@gmail.com>
Date: Sun, 14 Apr 2024 00:46:56 -0400
Subject: [PATCH] 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)
---
 modules/session/store.go             |  7 ++++++
 modules/web/middleware/cookie.go     | 34 +++++++++++++++++++++++-----
 services/auth/source/oauth2/store.go |  3 ++-
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/modules/session/store.go b/modules/session/store.go
index 4fa4d2848f..2f7ab7760b 100644
--- a/modules/session/store.go
+++ b/modules/session/store.go
@@ -6,6 +6,9 @@ package session
 import (
 	"net/http"
 
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/web/middleware"
+
 	"gitea.com/go-chi/session"
 )
 
@@ -18,6 +21,10 @@ type Store interface {
 
 // RegenerateSession regenerates the underlying session and returns the new store
 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)
 	return s, err
 }
diff --git a/modules/web/middleware/cookie.go b/modules/web/middleware/cookie.go
index 621640895b..0bed726793 100644
--- a/modules/web/middleware/cookie.go
+++ b/modules/web/middleware/cookie.go
@@ -45,10 +45,32 @@ func SetSiteCookie(resp http.ResponseWriter, name, value string, maxAge int) {
 		SameSite: setting.SessionConfig.SameSite,
 	}
 	resp.Header().Add("Set-Cookie", cookie.String())
-	if maxAge < 0 {
-		// There was a bug in "setting.SessionConfig.CookiePath" code, the old default value of it was empty "".
-		// So we have to delete the cookie on path="" again, because some old code leaves cookies on path="".
-		cookie.Path = strings.TrimSuffix(setting.SessionConfig.CookiePath, "/")
-		resp.Header().Add("Set-Cookie", cookie.String())
-	}
+	// Previous versions would use a cookie path with a trailing /.
+	// These are more specific than cookies without a trailing /, so
+	// we need to delete these if they exist.
+	DeleteLegacySiteCookie(resp, name)
+}
+
+// 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())
 }
diff --git a/services/auth/source/oauth2/store.go b/services/auth/source/oauth2/store.go
index 394bf99463..90fa965602 100644
--- a/services/auth/source/oauth2/store.go
+++ b/services/auth/source/oauth2/store.go
@@ -9,6 +9,7 @@ import (
 	"net/http"
 
 	"code.gitea.io/gitea/modules/log"
+	session_module "code.gitea.io/gitea/modules/session"
 
 	chiSession "gitea.com/go-chi/session"
 	"github.com/gorilla/sessions"
@@ -65,7 +66,7 @@ func (st *SessionsStore) Save(r *http.Request, w http.ResponseWriter, session *s
 	chiStore := chiSession.GetSession(r)
 
 	if session.IsNew {
-		_, _ = chiSession.RegenerateSession(w, r)
+		_, _ = session_module.RegenerateSession(w, r)
 		session.IsNew = false
 	}