From f715c9371b1b61cc3d62c94f87da78a15b69a7ad Mon Sep 17 00:00:00 2001 From: einfachchr Date: Fri, 15 Mar 2019 08:18:37 +0100 Subject: [PATCH] Fixes deletion of splitted cookies - Issue #69 (#70) * fixes deletion of splitted cookies * three minor adjustments to improve the tests * changed cookie name matching to regex * Update oauthproxy.go Co-Authored-By: einfachchr * removed unused variable * Changelog --- CHANGELOG.md | 1 + oauthproxy.go | 15 ++++++++++++--- oauthproxy_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68954d0..9de55c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Changes since v3.1.0 +- [#70](https://github.com/pusher/oauth2_proxy/pull/70) Fix handling of splitted cookies (@einfachchr) - [#92](https://github.com/pusher/oauth2_proxy/pull/92) Merge websocket proxy feature from openshift/oauth-proxy (@butzist) - [#57](https://github.com/pusher/oauth2_proxy/pull/57) Fall back to using OIDC Subject instead of Email (@aigarius) - [#85](https://github.com/pusher/oauth2_proxy/pull/85) Use non-root user in docker images (@kskewes) diff --git a/oauthproxy.go b/oauthproxy.go index 6d261da..1dbb063 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -452,9 +452,18 @@ func (p *OAuthProxy) SetCSRFCookie(rw http.ResponseWriter, req *http.Request, va // ClearSessionCookie creates a cookie to unset the user's authentication cookie // stored in the user's session func (p *OAuthProxy) ClearSessionCookie(rw http.ResponseWriter, req *http.Request) { - cookies := p.MakeSessionCookie(req, "", time.Hour*-1, time.Now()) - for _, clr := range cookies { - http.SetCookie(rw, clr) + var cookies []*http.Cookie + + // matches CookieName, CookieName_ + var cookieNameRegex = regexp.MustCompile(fmt.Sprintf("^%s(_\\d+)?$", p.CookieName)) + + for _, c := range req.Cookies() { + if cookieNameRegex.MatchString(c.Name) { + clearCookie := p.makeCookie(req, c.Name, "", time.Hour*-1, time.Now()) + + http.SetCookie(rw, clearCookie) + cookies = append(cookies, clearCookie) + } } // ugly hack because default domain changed diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 757180a..45bacd7 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -1064,3 +1064,47 @@ func TestAjaxForbiddendRequest(t *testing.T) { mime := rh.Get("Content-Type") assert.NotEqual(t, applicationJSON, mime) } + +func TestClearSplitCookie(t *testing.T) { + p := OAuthProxy{CookieName: "oauth2", CookieDomain: "abc"} + var rw = httptest.NewRecorder() + req := httptest.NewRequest("get", "/", nil) + + req.AddCookie(&http.Cookie{ + Name: "test1", + Value: "test1", + }) + req.AddCookie(&http.Cookie{ + Name: "oauth2_0", + Value: "oauth2_0", + }) + req.AddCookie(&http.Cookie{ + Name: "oauth2_1", + Value: "oauth2_1", + }) + + p.ClearSessionCookie(rw, req) + header := rw.Header() + + assert.Equal(t, 2, len(header["Set-Cookie"]), "should have 3 set-cookie header entries") +} + +func TestClearSingleCookie(t *testing.T) { + p := OAuthProxy{CookieName: "oauth2", CookieDomain: "abc"} + var rw = httptest.NewRecorder() + req := httptest.NewRequest("get", "/", nil) + + req.AddCookie(&http.Cookie{ + Name: "test1", + Value: "test1", + }) + req.AddCookie(&http.Cookie{ + Name: "oauth2", + Value: "oauth2", + }) + + p.ClearSessionCookie(rw, req) + header := rw.Header() + + assert.Equal(t, 1, len(header["Set-Cookie"]), "should have 1 set-cookie header entries") +}