Skip to content

Commit 8efd40f

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) Closes zalando#3459 Signed-off-by: Andreas Skorczyk <[email protected]>
1 parent 7e2ca3b commit 8efd40f

File tree

2 files changed

+31
-1
lines changed

2 files changed

+31
-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

+25
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"
@@ -62,6 +63,19 @@ func newInsecureCookieJar() *insecureCookieJar {
6263
}
6364

6465
func (jar *insecureCookieJar) SetCookies(u *url.URL, cookies []*http.Cookie) {
66+
cookieMap := make(map[string]*http.Cookie)
67+
for _, c := range jar.store[u.Hostname()] {
68+
cookieMap[c.Name] = c
69+
}
70+
for _, c := range cookies {
71+
cookieMap[c.Name] = c
72+
}
73+
74+
cookies = make([]*http.Cookie, 0, len(cookieMap))
75+
for _, c := range cookieMap {
76+
cookies = append(cookies, c)
77+
}
78+
6579
jar.store[u.Hostname()] = cookies
6680
}
6781
func (jar *insecureCookieJar) Cookies(u *url.URL) []*http.Cookie {
@@ -705,6 +719,7 @@ func TestOIDCSetup(t *testing.T) {
705719
hostname string
706720
filter string
707721
queries []string
722+
cookies []*http.Cookie
708723
expected int
709724
expectRequest string
710725
expectNoCookies bool
@@ -859,11 +874,20 @@ func TestOIDCSetup(t *testing.T) {
859874
filter: `oauthOidcUserInfo("{{ .OIDCServerURL }}", "valid-client", "mysec", "{{ .RedirectURL }}", "", "")`,
860875
expected: 200,
861876
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",
862885
}} {
863886
t.Run(tc.msg, func(t *testing.T) {
864887
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
865888
requestDump, _ := httputil.DumpRequest(r, false)
866889
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")
867891
w.Write([]byte("OK"))
868892
}))
869893
defer backend.Close()
@@ -949,6 +973,7 @@ func TestOIDCSetup(t *testing.T) {
949973
Timeout: 1 * time.Second,
950974
Jar: newInsecureCookieJar(),
951975
}
976+
client.Jar.SetCookies(reqURL, tc.cookies)
952977

953978
// trigger OpenID Connect Authorization Code Flow
954979
resp, err := client.Do(req)

0 commit comments

Comments
 (0)