Skip to content

Commit 8f591e8

Browse files
committed
Drop skipperOauthOidc cookies in oauthOidc filter
The `skipperOauthOidc` cookies are only intended for Skipper, so they don't need to be forwarded to the application. With this PR we simply drop those cookies before forwarding the request. This will reduce the overall header size of requests. The implementation is inspired by the [`oauthGrant` filter](https://github.com/zalando/skipper/blob/6b448b1fe90cc113e365be8fba7cd6d122ad7a6d/filters/auth/grantcookie.go#L93-L100) Signed-off-by: Andreas Skorczyk <[email protected]>
1 parent b109e9f commit 8f591e8

File tree

2 files changed

+21
-1
lines changed

2 files changed

+21
-1
lines changed

filters/auth/oidc.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -788,9 +788,14 @@ func (f *tokenOidcFilter) Request(ctx filters.FilterContext) {
788788
)
789789
r := ctx.Request()
790790

791-
for _, cookie := range r.Cookies() {
791+
// Retrieve skipperOauthOidc cookie for processing and remove it from downstream request
792+
rCookies := r.Cookies()
793+
r.Header.Del("Cookie")
794+
for _, cookie := range rCookies {
792795
if strings.HasPrefix(cookie.Name, f.cookiename) {
793796
cookies = append(cookies, cookie)
797+
} else {
798+
r.AddCookie(cookie)
794799
}
795800
}
796801
sessionCookie := mergerCookies(cookies)

filters/auth/oidc_test.go

+15
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package auth
22

33
import (
4+
"cmp"
45
"compress/flate"
56
"crypto/rsa"
67
"crypto/x509"
@@ -718,6 +719,7 @@ func TestOIDCSetup(t *testing.T) {
718719
hostname string
719720
filter string
720721
queries []string
722+
cookies []*http.Cookie
721723
expected int
722724
expectRequest string
723725
expectNoCookies bool
@@ -872,11 +874,20 @@ func TestOIDCSetup(t *testing.T) {
872874
filter: `oauthOidcUserInfo("{{ .OIDCServerURL }}", "valid-client", "mysec", "{{ .RedirectURL }}", "", "")`,
873875
expected: 200,
874876
expectCookieName: "skipperOauthOidc",
877+
}, {
878+
msg: "cookies should be forwarded",
879+
hostname: "skipper.test",
880+
filter: `oauthOidcUserInfo("{{ .OIDCServerURL }}", "valid-client", "mysec", "{{ .RedirectURL }}", "", "")`,
881+
cookies: []*http.Cookie{{Name: "please-forward", Value: "me", Domain: "skipper.test", MaxAge: 7200}},
882+
expected: 200,
883+
expectRequest: "please-forward=me",
884+
expectCookieDomain: "skipper.test",
875885
}} {
876886
t.Run(tc.msg, func(t *testing.T) {
877887
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
878888
requestDump, _ := httputil.DumpRequest(r, false)
879889
assert.Contains(t, string(requestDump), tc.expectRequest, "expected request not fulfilled")
890+
assert.NotContains(t, string(requestDump), cmp.Or(tc.expectCookieName, oauthOidcCookieName), "oidc cookie should be dropped")
880891
w.Write([]byte("OK"))
881892
}))
882893
defer backend.Close()
@@ -963,6 +974,10 @@ func TestOIDCSetup(t *testing.T) {
963974
Jar: newInsecureCookieJar(),
964975
}
965976

977+
for _, c := range tc.cookies {
978+
client.Jar.SetCookies(reqURL, []*http.Cookie{c})
979+
}
980+
966981
// trigger OpenID Connect Authorization Code Flow
967982
resp, err := client.Do(req)
968983
if err != nil {

0 commit comments

Comments
 (0)