From 0337a95fc6bb4c2798e555b28ae61500b30a6b9b Mon Sep 17 00:00:00 2001 From: Jan Larwig Date: Mon, 13 Apr 2026 18:17:50 +0200 Subject: [PATCH] Merge commit from fork * fix: clear session cookie at beginning of signinpage handler Co-authored-by: Christopher Schrewing Signed-off-by: Michael Bella Signed-off-by: Jan Larwig * test: clear session cookie at beginning of signinpage handler Signed-off-by: Jan Larwig * doc: changelog entry for GHSA-f24x-5g9q-753f Signed-off-by: Jan Larwig --------- Signed-off-by: Michael Bella Signed-off-by: Jan Larwig Co-authored-by: Christopher Schrewing --- CHANGELOG.md | 1 + oauthproxy.go | 8 ++++---- oauthproxy_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8f2d777..2dccaaf1 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/oauthproxy.go b/oauthproxy.go index 3efe66fd..0685bbbb 100644 --- a/oauthproxy.go +++ b/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) } diff --git a/oauthproxy_test.go b/oauthproxy_test.go index e06f50e9..46e39a90 100644 --- a/oauthproxy_test.go +++ b/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 {