Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add constant cookie name for the OIDC filter #3234

Merged
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
Loading