git.iko.soy/typeable/oauth2-proxy
on master help sign in
Fix joined cookie name for those containing underline in the suffix (#970)

* properly handle splitted cookies with names ending with _ * test update * provide cookieName into joinCookies instead of processing the suffix * changelog update * test update

commit597ffeb1217058afd71182e8cec1de670f8b6998 authorIlia Pertsev <peppered@users.noreply.github.com> dateTue 5 Jan 2021, 04:21 (6 years ago) parent1d74a51
3 files changed+54−5
CHANGELOG.md+1 pkg/sessions/cookie/session_store.go+3−4 pkg/sessions/cookie/session_store_test.go+50−1
CHANGELOG.md+1
@@ -59,6 +59,7 @@
5959
6060## Changes since v6.1.1
6161
62+- [#970](https://github.com/oauth2-proxy/oauth2-proxy/pull/970) Fix joined cookie name for those containing underline in the suffix (@peppered)
6263- [#953](https://github.com/oauth2-proxy/oauth2-proxy/pull/953) Migrate Keycloak to EnrichSession & support multiple groups for authorization (@NickMeves)
6364- [#957](https://github.com/oauth2-proxy/oauth2-proxy/pull/957) Use X-Forwarded-{Proto,Host,Uri} on redirect as last resort (@linuxgemini)
6465- [#630](https://github.com/oauth2-proxy/oauth2-proxy/pull/630) Add support for Gitlab project based authentication (@factorysh)
pkg/sessions/cookie/session_store.go+3−4
@@ -5,7 +5,6 @@ import (
55 "fmt"
66 "net/http"
77 "regexp"
8− "strings"
98 "time"
109
1110 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
@@ -220,12 +219,12 @@ func loadCookie(req *http.Request, cookieName string) (*http.Cookie, error) {
220219 if len(cookies) == 0 {
221220 return nil, fmt.Errorf("could not find cookie %s", cookieName)
222221 }
223− return joinCookies(cookies)
222+ return joinCookies(cookies, cookieName)
224223}
225224
226225// joinCookies takes a slice of cookies from the request and reconstructs the
227226// full session cookie
228−func joinCookies(cookies []*http.Cookie) (*http.Cookie, error) {
227+func joinCookies(cookies []*http.Cookie, cookieName string) (*http.Cookie, error) {
229228 if len(cookies) == 0 {
230229 return nil, fmt.Errorf("list of cookies must be > 0")
231230 }
@@ -236,7 +235,7 @@ func joinCookies(cookies []*http.Cookie) (*http.Cookie, error) {
236235 for i := 1; i < len(cookies); i++ {
237236 c.Value += cookies[i].Value
238237 }
239− c.Name = strings.TrimRight(c.Name, "_0")
238+ c.Name = cookieName
240239 return c, nil
241240}
242241
pkg/sessions/cookie/session_store_test.go+50−1
@@ -154,9 +154,58 @@ func Test_splitCookie_joinCookies(t *testing.T) {
154154 Value: value,
155155 }
156156 splitCookies := splitCookie(cookie)
157− joinedCookie, err := joinCookies(splitCookies)
157+ joinedCookie, err := joinCookies(splitCookies, cookie.Name)
158158 assert.NoError(t, err)
159159 assert.Equal(t, *cookie, *joinedCookie)
160160 })
161161 }
162162}
163+
164+func Test_joinCookies_withUnderlineSuffix(t *testing.T) {
165+ testCases := map[string]struct {
166+ CookieName string
167+ SplitOrder []int
168+ }{
169+ "Ascending order split with \"_\" suffix": {
170+ CookieName: "_cookie_name_",
171+ SplitOrder: []int{0, 1, 2, 3, 4},
172+ },
173+ "Descending order split with \"_\" suffix": {
174+ CookieName: "_cookie_name_",
175+ SplitOrder: []int{4, 3, 2, 1, 0},
176+ },
177+ "Arbitrary order split with \"_\" suffix": {
178+ CookieName: "_cookie_name_",
179+ SplitOrder: []int{3, 1, 2, 0, 4},
180+ },
181+ "Arbitrary order split with \"_0\" suffix": {
182+ CookieName: "_cookie_name_0",
183+ SplitOrder: []int{1, 3, 0, 2, 4},
184+ },
185+ "Arbitrary order split with \"_1\" suffix": {
186+ CookieName: "_cookie_name_1",
187+ SplitOrder: []int{4, 1, 3, 0, 2},
188+ },
189+ "Arbitrary order split with \"__\" suffix": {
190+ CookieName: "_cookie_name__",
191+ SplitOrder: []int{1, 0, 4, 3, 2},
192+ },
193+ }
194+
195+ for testName, testCase := range testCases {
196+ t.Run(testName, func(t *testing.T) {
197+ cookieName := testCase.CookieName
198+ var splitCookies []*http.Cookie
199+ for _, splitSuffix := range testCase.SplitOrder {
200+ cookie := &http.Cookie{
201+ Name: splitCookieName(cookieName, splitSuffix),
202+ Value: strings.Repeat("v", 1000),
203+ }
204+ splitCookies = append(splitCookies, cookie)
205+ }
206+ joinedCookie, err := joinCookies(splitCookies, cookieName)
207+ assert.NoError(t, err)
208+ assert.Equal(t, cookieName, joinedCookie.Name)
209+ })
210+ }
211+}