From a5b72a051df3b983d34c376d3c58ea030ef4e99b Mon Sep 17 00:00:00 2001 From: Saksham Goyal Date: Tue, 4 Aug 2026 17:36:03 +0530 Subject: [PATCH] fix(sessions): clear stale cookie parts Signed-off-by: Saksham Goyal --- CHANGELOG.md | 2 + pkg/sessions/cookie/session_store.go | 29 ++++++++++ pkg/sessions/cookie/session_store_test.go | 69 +++++++++++++++++++++++ 3 files changed, 100 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 788e82c2..443d49ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ## Changes since v7.15.3 +- fix: clear stale cookie session parts after session size changes + # V7.15.3 ## Release Highlights diff --git a/pkg/sessions/cookie/session_store.go b/pkg/sessions/cookie/session_store.go index a4da3734..e32d7f7c 100644 --- a/pkg/sessions/cookie/session_store.go +++ b/pkg/sessions/cookie/session_store.go @@ -121,6 +121,35 @@ func (s *SessionStore) setSessionCookie(rw http.ResponseWriter, req *http.Reques if err != nil { return err } + + newCookieNames := make(map[string]struct{}, len(cookies)) + for _, c := range cookies { + newCookieNames[c.Name] = struct{}{} + } + + sessionCookieName := regexp.QuoteMeta(s.Cookie.Name) + sessionCookiePattern := regexp.MustCompile(fmt.Sprintf("^%s(_\\d+)?$", sessionCookieName)) + for _, c := range req.Cookies() { + if !sessionCookiePattern.MatchString(c.Name) { + continue + } + if _, ok := newCookieNames[c.Name]; ok { + continue + } + + clearCookieOptions := &pkgcookies.CookieOptions{ + Name: c.Name, + Value: "", + Domains: s.Cookie.Domains, + Expiration: time.Hour * -1, + SameSite: s.Cookie.SameSite, + Path: s.Cookie.Path, + HTTPOnly: s.Cookie.HTTPOnly, + Secure: s.Cookie.Secure, + } + http.SetCookie(rw, pkgcookies.MakeCookieFromOptions(req, clearCookieOptions)) + } + for _, c := range cookies { http.SetCookie(rw, c) } diff --git a/pkg/sessions/cookie/session_store_test.go b/pkg/sessions/cookie/session_store_test.go index 5fc1ad78..6da70a00 100644 --- a/pkg/sessions/cookie/session_store_test.go +++ b/pkg/sessions/cookie/session_store_test.go @@ -4,6 +4,7 @@ import ( "fmt" mathrand "math/rand" "net/http" + "net/http/httptest" "strings" "testing" "time" @@ -140,6 +141,74 @@ func Test_splitCookieName(t *testing.T) { } } +func Test_setSessionCookieClearsStaleCookies(t *testing.T) { + store := &SessionStore{ + Cookie: &options.Cookie{ + Name: "_oauth2_proxy", + Secret: "0123456789abcdef", + Path: "/", + Expire: time.Hour, + }, + } + now := time.Now() + + testCases := map[string]struct { + value []byte + existingCookies []string + wantSessionParts int + wantClearedCookie []string + }{ + "split session becomes a single cookie": { + value: []byte("small session"), + existingCookies: []string{"_oauth2_proxy_0", "_oauth2_proxy_1", "_oauth2_proxy_csrf"}, + wantSessionParts: 1, + wantClearedCookie: []string{"_oauth2_proxy_0", "_oauth2_proxy_1"}, + }, + "split session uses fewer parts": { + value: []byte(strings.Repeat("v", 4500)), + existingCookies: []string{"_oauth2_proxy_0", "_oauth2_proxy_1", "_oauth2_proxy_2", "_oauth2_proxy_3"}, + wantSessionParts: 2, + wantClearedCookie: []string{"_oauth2_proxy_2", "_oauth2_proxy_3"}, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "https://example.com/", nil) + for _, cookieName := range tc.existingCookies { + req.AddCookie(&http.Cookie{Name: cookieName, Value: "old"}) + } + + newCookies, err := store.makeSessionCookie(req, tc.value, now) + assert.NoError(t, err) + assert.Len(t, newCookies, tc.wantSessionParts) + + rw := httptest.NewRecorder() + err = store.setSessionCookie(rw, req, tc.value, now) + assert.NoError(t, err) + + responseCookies := make(map[string]*http.Cookie) + for _, cookie := range rw.Result().Cookies() { + responseCookies[cookie.Name] = cookie + } + for _, cookieName := range tc.wantClearedCookie { + cookie := responseCookies[cookieName] + if assert.NotNil(t, cookie, "stale cookie %q was not cleared", cookieName) { + assert.Equal(t, -1, cookie.MaxAge) + } + } + for _, wantCookie := range newCookies { + cookie := responseCookies[wantCookie.Name] + if assert.NotNil(t, cookie, "current cookie %q was not set", wantCookie.Name) { + assert.Greater(t, cookie.MaxAge, 0) + } + } + assert.Len(t, responseCookies, len(newCookies)+len(tc.wantClearedCookie)) + assert.NotContains(t, responseCookies, "_oauth2_proxy_csrf") + }) + } +} + func Test_splitCookie_joinCookies(t *testing.T) { const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"