Skip to content

Commit 1c7e275

Browse files
authored
Drop skipperOauthOidc cookies in oauthOidc filter (#3465)
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 #3459 Signed-off-by: Andreas Skorczyk <[email protected]>
1 parent aeb46b5 commit 1c7e275

File tree

2 files changed

+31
-1
lines changed

2 files changed

+31
-1
lines changed

Diff for: 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)

Diff for: 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)