Skip to content

Commit

Permalink
Add configurable cookie name prefix for the OIDC filter (#3234)
Browse files Browse the repository at this point in the history
Allow specifying cookie name prefix for OIDC filters

Signed-off-by: Carl Zhou <czhou@brex.com>
  • Loading branch information
czhou-brex authored Sep 19, 2024
1 parent 7b74d47 commit 3c2a615
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 15 deletions.
1 change: 1 addition & 0 deletions docs/reference/filters.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
36 changes: 21 additions & 15 deletions filters/auth/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ const (
paramAuthCodeOpts
paramUpstrHeaders
paramSubdomainsToRemove
paramCookieName
)

type OidcOptions struct {
Expand Down Expand Up @@ -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] == "" {
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 17 additions & 0 deletions filters/auth/oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}", "", "")`,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
}
}
})
Expand Down

0 comments on commit 3c2a615

Please sign in to comment.