diff --git a/pkg/auth/sessions/combined_sessions.go b/pkg/auth/sessions/combined_sessions.go index 8a1aaf03827..48819a44091 100644 --- a/pkg/auth/sessions/combined_sessions.go +++ b/pkg/auth/sessions/combined_sessions.go @@ -47,6 +47,21 @@ func (cs *CombinedSessionStore) AddSession(w http.ResponseWriter, r *http.Reques cs.sessionLock.Lock() defer cs.sessionLock.Unlock() + // Clean up old session cookies from previous pods before creating new session + // This prevents cookie accumulation when users are load-balanced across multiple pods + currentCookieName := SessionCookieName() + for _, cookie := range r.Cookies() { + // Expire any session cookies that are not for the current pod + if strings.HasPrefix(cookie.Name, OpenshiftAccessTokenCookieName) && cookie.Name != currentCookieName { + http.SetCookie(w, &http.Cookie{ + Name: cookie.Name, + Value: "", + Path: cs.clientStore.Options.Path, + MaxAge: -1, + }) + } + } + ls, err := cs.serverStore.AddSession(tokenVerifier, token) if err != nil { return nil, fmt.Errorf("failed to add session to server store: %w", err) @@ -193,10 +208,13 @@ func (cs *CombinedSessionStore) DeleteSession(w http.ResponseWriter, r *http.Req defer cs.sessionLock.Unlock() for _, cookie := range r.Cookies() { - cookie := cookie if strings.HasPrefix(cookie.Name, OpenshiftAccessTokenCookieName) { - cookie.MaxAge = -1 - http.SetCookie(w, cookie) + http.SetCookie(w, &http.Cookie{ + Name: cookie.Name, + Value: "", + Path: cs.clientStore.Options.Path, + MaxAge: -1, + }) } } diff --git a/pkg/auth/sessions/combined_sessions_test.go b/pkg/auth/sessions/combined_sessions_test.go index 9c1ec5f4563..b6e57d06d87 100644 --- a/pkg/auth/sessions/combined_sessions_test.go +++ b/pkg/auth/sessions/combined_sessions_test.go @@ -52,7 +52,7 @@ func TestCombinedSessionStore_AddSession(t *testing.T) { }, testIDToken, ), - origRefreshToken: utilptr.To[string]("orig-refresh-token"), + origRefreshToken: utilptr.To("orig-refresh-token"), wantRefreshToken: "random-token-string", wantRawToken: testIDToken, }, @@ -64,7 +64,7 @@ func TestCombinedSessionStore_AddSession(t *testing.T) { }, testIDToken, ), - origSessionToken: utilptr.To[string]("orig-session-token"), + origSessionToken: utilptr.To("orig-session-token"), wantRefreshToken: "random-token-string", wantRawToken: testIDToken, }, @@ -93,7 +93,7 @@ func TestCombinedSessionStore_AddSession(t *testing.T) { &oauth2.Token{}, testIDToken, ), - origRefreshToken: utilptr.To[string]("random-token-string"), + origRefreshToken: utilptr.To("random-token-string"), wantRawToken: testIDToken, }, } @@ -695,3 +695,51 @@ func indexSessionByRefreshToken(serverStore *SessionStore, refreshToken string, serverStore.byRefreshToken[refreshToken] = session return serverStore } + +func TestCombinedSessionStore_AddSession_CleansUpOldPodCookies(t *testing.T) { + testIDToken := createTestIDToken(`{"sub":"user-id-0"}`) + testVerifier := newTestVerifier(`{"sub":"user-id-0"}`) + + encryptionKey := []byte(randomString(32)) + authnKey := []byte(randomString(64)) + cs := NewSessionStore(authnKey, encryptionKey, true, "/") + + token := addIDToken( + &oauth2.Token{ + RefreshToken: "new-refresh-token", + }, + testIDToken, + ) + + // Simulate request with old session cookies from different pods + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.AddCookie(&http.Cookie{Name: OpenshiftAccessTokenCookieName + "-console-old-pod-1", Value: "old-value-1"}) + req.AddCookie(&http.Cookie{Name: OpenshiftAccessTokenCookieName + "-console-old-pod-2", Value: "old-value-2"}) + req.AddCookie(&http.Cookie{Name: "other-cookie", Value: "other-value"}) + + testWriter := httptest.NewRecorder() + _, err := cs.AddSession(testWriter, req, testVerifier, token) + require.NoError(t, err) + + // Verify old pod cookies were expired + cookies := testWriter.Result().Cookies() + expiredCookies := 0 + newSessionCookie := false + for _, c := range cookies { + if c.Name == OpenshiftAccessTokenCookieName+"-console-old-pod-1" || + c.Name == OpenshiftAccessTokenCookieName+"-console-old-pod-2" { + require.Equal(t, -1, c.MaxAge, "Old pod cookie %s should be expired", c.Name) + expiredCookies++ + } + if c.Name == SessionCookieName() { + require.NotEqual(t, -1, c.MaxAge, "New session cookie should not be expired") + newSessionCookie = true + } + if c.Name == "other-cookie" { + t.Errorf("Non-session cookie 'other-cookie' should not be modified") + } + } + + require.Equal(t, 2, expiredCookies, "Both old pod cookies should be expired") + require.True(t, newSessionCookie, "New session cookie should be created") +}