Merge commit from fork

* fix: clear session cookie at beginning of signinpage handler

Co-authored-by: Christopher Schrewing <christopher.schrewing@weidmueller.com>
Signed-off-by: Michael Bella <michael.bella@weidmueller.com>
Signed-off-by: Jan Larwig <jan@larwig.com>

* test: clear session cookie at beginning of signinpage handler

Signed-off-by: Jan Larwig <jan@larwig.com>

* doc: changelog entry for GHSA-f24x-5g9q-753f

Signed-off-by: Jan Larwig <jan@larwig.com>

---------

Signed-off-by: Michael Bella <michael.bella@weidmueller.com>
Signed-off-by: Jan Larwig <jan@larwig.com>
Co-authored-by: Christopher Schrewing <christopher.schrewing@weidmueller.com>

CHANGELOG.md

@@ -10,6 +10,7 @@
 
 - [#3411](https://github.com/oauth2-proxy/oauth2-proxy/pull/3411) chore(deps): update gomod dependencies (@tuunit)
 - [#3333](https://github.com/oauth2-proxy/oauth2-proxy/pull/3333) fix: invalidate session on fatal OAuth2 refresh errors (@frhack)
+- [GHSA-f24x-5g9q-753f](https://github.com/oauth2-proxy/oauth2-proxy/security/advisories/GHSA-f24x-5g9q-753f) fix: clear session cookie at beginning of signinpage handler (@fnoehWM / @bella-WI / @tuunit)
 
 # V7.15.1
 

oauthproxy.go

@@ -634,6 +634,10 @@ func (p *OAuthProxy) isTrustedIP(req *http.Request) bool {
 // SignInPage writes the sign in template to the response
 func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code int) {
 	prepareNoCache(rw)
+
+	if err := p.ClearSessionCookie(rw, req); err != nil {
+		logger.Printf("Error clearing session cookie: %v", err)
+	}
 	rw.WriteHeader(code)
 
 	redirectURL, err := p.appDirector.GetRedirect(req)
@@ -647,10 +651,6 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code
 		redirectURL = "/"
 	}
 
-	if err := p.ClearSessionCookie(rw, req); err != nil {
-		logger.Printf("Error clearing session cookie: %v", err)
-	}
-
 	p.pageWriter.WriteSignInPage(rw, req, redirectURL, code)
 }
 

oauthproxy_test.go

@@ -713,6 +713,50 @@ func TestManualSignInCorrectCredentials(t *testing.T) {
 	assert.Equal(t, http.StatusFound, statusCode)
 }
 
+func TestSignInPageClearsExistingSessionCookie(t *testing.T) {
+	opts := baseTestOptions()
+	err := validation.Validate(opts)
+	require.NoError(t, err)
+
+	proxy, err := NewOAuthProxy(opts, func(string) bool {
+		return true
+	})
+	require.NoError(t, err)
+
+	// Create a real session cookie using the actual session store.
+	saveRW := httptest.NewRecorder()
+	saveReq := httptest.NewRequest(http.MethodGet, "/", nil)
+	err = proxy.sessionStore.Save(saveRW, saveReq, &sessions.SessionState{
+		Email: "john.doe@example.com",
+	})
+	require.NoError(t, err)
+
+	cookies := saveRW.Result().Cookies()
+	require.NotEmpty(t, cookies)
+
+	// Send that cookie to the sign-in page.
+	req := httptest.NewRequest(http.MethodGet, "/oauth2/sign_in", nil)
+	for _, c := range cookies {
+		req.AddCookie(c)
+	}
+
+	rw := httptest.NewRecorder()
+	proxy.ServeHTTP(rw, req)
+
+	assert.Equal(t, http.StatusOK, rw.Code)
+
+	cleared := false
+	for _, c := range rw.Result().Cookies() {
+		if c.Name == proxy.CookieOptions.Name {
+			cleared = true
+			assert.Equal(t, "", c.Value)
+			assert.Less(t, c.MaxAge, 0)
+		}
+	}
+
+	assert.True(t, cleared, "expected sign-in page to clear existing session cookie")
+}
+
 func TestSignInPageIncludesTargetRedirect(t *testing.T) {
 	sipTest, err := NewSignInPageTest(false)
 	if err != nil {