From 81ba27c149c06744e7daa5bd09a996776ccb4524 Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Wed, 5 Jul 2023 17:12:56 +0300 Subject: [PATCH] Feat: Match allowed cookies with optional character (#71047) * Match allowed cookies with optional character * Use strings package --- pkg/services/datasources/models_test.go | 60 +++++++++++++++++++++++++ pkg/util/proxyutil/proxyutil.go | 18 +++++++- pkg/util/proxyutil/proxyutil_test.go | 56 +++++++++++++++++++++++ 3 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 pkg/services/datasources/models_test.go diff --git a/pkg/services/datasources/models_test.go b/pkg/services/datasources/models_test.go new file mode 100644 index 00000000000..5fc04ad0d8a --- /dev/null +++ b/pkg/services/datasources/models_test.go @@ -0,0 +1,60 @@ +package datasources + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/components/simplejson" +) + +func TestAllowedCookies(t *testing.T) { + testCases := []struct { + desc string + given map[string]interface{} + want []string + }{ + { + desc: "Usual json data with keepCookies", + given: map[string]interface{}{ + "keepCookies": []string{"cookie2"}, + }, + want: []string{"cookie2"}, + }, + { + desc: "Usual json data without kepCookies", + given: map[string]interface{}{ + "something": "somethingelse", + }, + want: []string(nil), + }, + { + desc: "Usual json data that has multiple values in keepCookies", + given: map[string]interface{}{ + "keepCookies": []string{"cookie1", "cookie2", "special[]"}, + }, + want: []string{"cookie1", "cookie2", "special[]"}, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + jsonDataBytes, err := json.Marshal(&test.given) + require.NoError(t, err) + jsonData, err := simplejson.NewJson(jsonDataBytes) + require.NoError(t, err) + + ds := DataSource{ + ID: 1235, + JsonData: jsonData, + UID: "test", + } + + actual := ds.AllowedCookies() + assert.Equal(t, test.want, actual) + assert.EqualValues(t, test.want, actual) + }) + } +} diff --git a/pkg/util/proxyutil/proxyutil.go b/pkg/util/proxyutil/proxyutil.go index 6cccf55e37d..01875643ceb 100644 --- a/pkg/util/proxyutil/proxyutil.go +++ b/pkg/util/proxyutil/proxyutil.go @@ -5,6 +5,7 @@ import ( "net" "net/http" "sort" + "strings" "github.com/grafana/grafana/pkg/services/user" ) @@ -46,8 +47,23 @@ func ClearCookieHeader(req *http.Request, keepCookiesNames []string, skipCookies keepCookies := map[string]*http.Cookie{} for _, c := range req.Cookies() { for _, v := range keepCookiesNames { - if c.Name == v { + // match all + if v == "[]" { keepCookies[c.Name] = c + continue + } + + if strings.HasSuffix(v, "[]") { + // match prefix + pattern := strings.TrimSuffix(v, "[]") + if strings.HasPrefix(c.Name, pattern) { + keepCookies[c.Name] = c + } + } else { + // exact match + if c.Name == v { + keepCookies[c.Name] = c + } } } } diff --git a/pkg/util/proxyutil/proxyutil_test.go b/pkg/util/proxyutil/proxyutil_test.go index 24a5d157781..4ea59ee61ea 100644 --- a/pkg/util/proxyutil/proxyutil_test.go +++ b/pkg/util/proxyutil/proxyutil_test.go @@ -110,6 +110,62 @@ func TestClearCookieHeader(t *testing.T) { require.Contains(t, req.Header, "Cookie") require.Equal(t, "cookie1=", req.Header.Get("Cookie")) }) + + t.Run("Clear cookie header with cookies to keep should clear Cookie header and keep cookies with optional matching", func(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) + req.AddCookie(&http.Cookie{Name: "cookie1"}) + req.AddCookie(&http.Cookie{Name: "cookie3"}) + + ClearCookieHeader(req, []string{"cookie[]"}, nil) + require.Contains(t, req.Header, "Cookie") + require.Equal(t, "cookie1=; cookie3=", req.Header.Get("Cookie")) + }) + + t.Run("Clear cookie header with cookies to keep should clear Cookie header and keep cookies with matching pattern but with empty matching option", func(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) + req.AddCookie(&http.Cookie{Name: "cookie1"}) + req.AddCookie(&http.Cookie{Name: "cookie2"}) + req.AddCookie(&http.Cookie{Name: "cookie3"}) + + ClearCookieHeader(req, []string{"cookie[]"}, []string{"cookie2"}) + require.Contains(t, req.Header, "Cookie") + require.Equal(t, "cookie1=; cookie3=", req.Header.Get("Cookie")) + }) + + t.Run("Clear cookie header with cookie match pattern to keep and skip should clear Cookie header and keep cookies", func(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) + req.AddCookie(&http.Cookie{Name: "cook1"}) + req.AddCookie(&http.Cookie{Name: "special23"}) + req.AddCookie(&http.Cookie{Name: "special_1asd987dsf9a"}) + req.AddCookie(&http.Cookie{Name: "c00k1e"}) + + ClearCookieHeader(req, []string{"special_[]"}, nil) + require.Contains(t, req.Header, "Cookie") + require.Equal(t, "special_1asd987dsf9a=", req.Header.Get("Cookie")) + }) + + t.Run("Clear cookie header with cookie should not match BAD pattern and return no cookies", func(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) + req.AddCookie(&http.Cookie{Name: "cookie1"}) + req.AddCookie(&http.Cookie{Name: "special23"}) + + ClearCookieHeader(req, []string{"[]cookie"}, nil) + require.NotContains(t, req.Header, "Cookie") + }) + + t.Run("Clear cookie header with cookie should match all cookies when keepCookies is *", func(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, "/", nil) + require.NoError(t, err) + req.AddCookie(&http.Cookie{Name: "cookie1"}) + req.AddCookie(&http.Cookie{Name: "special23"}) + + ClearCookieHeader(req, []string{"[]"}, nil) + require.Equal(t, "cookie1=; special23=", req.Header.Get("Cookie")) + }) } func TestApplyUserHeader(t *testing.T) {