fix(sessions): clear stale cookie parts
Signed-off-by: Saksham Goyal <sakshamgoyal1510@gmail.com>
This commit is contained in:
parent
14af2951e5
commit
a5b72a051d
|
|
@ -8,6 +8,8 @@
|
||||||
|
|
||||||
## Changes since v7.15.3
|
## Changes since v7.15.3
|
||||||
|
|
||||||
|
- fix: clear stale cookie session parts after session size changes
|
||||||
|
|
||||||
# V7.15.3
|
# V7.15.3
|
||||||
|
|
||||||
## Release Highlights
|
## Release Highlights
|
||||||
|
|
|
||||||
|
|
@ -121,6 +121,35 @@ func (s *SessionStore) setSessionCookie(rw http.ResponseWriter, req *http.Reques
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
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 {
|
for _, c := range cookies {
|
||||||
http.SetCookie(rw, c)
|
http.SetCookie(rw, c)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -4,6 +4,7 @@ import (
|
||||||
"fmt"
|
"fmt"
|
||||||
mathrand "math/rand"
|
mathrand "math/rand"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"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) {
|
func Test_splitCookie_joinCookies(t *testing.T) {
|
||||||
const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
|
const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue