From 3c2a615fb98ed7b73bef2147c04dc34d1ce52f16 Mon Sep 17 00:00:00 2001 From: czhou-brex Date: Thu, 19 Sep 2024 09:21:34 -0700 Subject: [PATCH] Add configurable cookie name prefix for the OIDC filter (#3234) Allow specifying cookie name prefix for OIDC filters Signed-off-by: Carl Zhou --- docs/reference/filters.md | 1 + filters/auth/oidc.go | 36 +++++++++++++++++++++--------------- filters/auth/oidc_test.go | 17 +++++++++++++++++ 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/docs/reference/filters.md b/docs/reference/filters.md index 584333ba68..9bd4af47e3 100644 --- a/docs/reference/filters.md +++ b/docs/reference/filters.md @@ -1844,6 +1844,7 @@ The filter needs the following parameters: * **Auth Code Options** (optional) Passes key/value parameters to a provider's authorization endpoint. The value can be dynamically set by a query parameter with the same key name if the placeholder `skipper-request-query` is used. * **Upstream Headers** (optional) The upstream endpoint will receive these headers which values are parsed from the OIDC information. The header definition can be one or more header-query pairs, space delimited. The query syntax is [GJSON](https://github.com/tidwall/gjson/blob/master/SYNTAX.md). * **SubdomainsToRemove** (optional, default "1") Configures number of subdomains to remove from the request hostname to derive OIDC cookie domain. By default one subdomain is removed, e.g. for the www.example.com request hostname the OIDC cookie domain will be example.com (to support SSO for all subdomains of the example.com). Configure "0" to use the same hostname. Note that value is a string. +* **Custom Cookie Name** (optional) Defines a constant cookie name generated by the OIDC filter. By default the cookie name is SkipperOauthOidc{hash}, where {hash} is a generated value. #### oauthOidcAnyClaims diff --git a/filters/auth/oidc.go b/filters/auth/oidc.go index 6d4e502b7e..88c952c52c 100644 --- a/filters/auth/oidc.go +++ b/filters/auth/oidc.go @@ -85,6 +85,7 @@ const ( paramAuthCodeOpts paramUpstrHeaders paramSubdomainsToRemove + paramCookieName ) type OidcOptions struct { @@ -201,22 +202,27 @@ func (s *tokenOidcSpec) CreateFilter(args []interface{}) (filters.Filter, error) return nil, filters.ErrInvalidFilterParameters } - h := sha256.New() - for i, s := range sargs { - // CallbackURL not taken into account for cookie hashing for additional sub path ingresses - if i == paramCallbackURL { - continue - } - // SubdomainsToRemove not taken into account for cookie hashing for additional sub-domain ingresses - if i == paramSubdomainsToRemove { - continue + var cookieName string + if len(sargs) > paramCookieName && sargs[paramCookieName] != "" { + cookieName = sargs[paramCookieName] + } else { + h := sha256.New() + for i, s := range sargs { + // CallbackURL not taken into account for cookie hashing for additional sub path ingresses + if i == paramCallbackURL { + continue + } + // SubdomainsToRemove not taken into account for cookie hashing for additional sub-domain ingresses + if i == paramSubdomainsToRemove { + continue + } + h.Write([]byte(s)) } - h.Write([]byte(s)) + byteSlice := h.Sum(nil) + sargsHash := fmt.Sprintf("%x", byteSlice)[:8] + cookieName = oauthOidcCookieName + sargsHash + "-" } - byteSlice := h.Sum(nil) - sargsHash := fmt.Sprintf("%x", byteSlice)[:8] - generatedCookieName := oauthOidcCookieName + sargsHash + "-" - log.Debugf("Generated Cookie Name: %s", generatedCookieName) + log.Debugf("Cookie Name: %s", cookieName) redirectURL, err := url.Parse(sargs[paramCallbackURL]) if err != nil || sargs[paramCallbackURL] == "" { @@ -269,7 +275,7 @@ func (s *tokenOidcSpec) CreateFilter(args []interface{}) (filters.Filter, error) ClientID: oidcClientId, }), validity: validity, - cookiename: generatedCookieName, + cookiename: cookieName, encrypter: encrypter, compressor: newDeflatePoolCompressor(flate.BestCompression), subdomainsToRemove: subdomainsToRemove, diff --git a/filters/auth/oidc_test.go b/filters/auth/oidc_test.go index 7198bd223d..771fde01fe 100644 --- a/filters/auth/oidc_test.go +++ b/filters/auth/oidc_test.go @@ -711,6 +711,7 @@ func TestOIDCSetup(t *testing.T) { expectCookieDomain string filterCookies []string extraClaims jwt.MapClaims + expectCookieName string }{{ msg: "wrong provider", filter: `oauthOidcAnyClaims("no url", "", "", "{{ .RedirectURL }}", "", "")`, @@ -848,6 +849,16 @@ func TestOIDCSetup(t *testing.T) { expected: 200, expectCookieDomain: "bar.foo.skipper.test", filterCookies: []string{"badheader", "malformed"}, + }, { + msg: "custom cookie name", + filter: `oauthOidcUserInfo("{{ .OIDCServerURL }}", "valid-client", "mysec", "{{ .RedirectURL }}", "", "", "", "", "", "custom-cookie")`, + expected: 200, + expectCookieName: "custom-cookie", + }, { + msg: "default cookie name when not specified", + filter: `oauthOidcUserInfo("{{ .OIDCServerURL }}", "valid-client", "mysec", "{{ .RedirectURL }}", "", "")`, + expected: 200, + expectCookieName: "skipperOauthOidc", }} { t.Run(tc.msg, func(t *testing.T) { backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -976,6 +987,12 @@ func TestOIDCSetup(t *testing.T) { assert.True(t, c.Value == "") } } + + // Check for custom cookie name + if tc.expectCookieName != "" { + assert.True(t, strings.HasPrefix(c.Name, tc.expectCookieName), + "Cookie name should start with %s, but got %s", tc.expectCookieName, c.Name) + } } } })