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

feature: dropRequestCookie filter #2410

Merged
merged 7 commits into from
Jun 21, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions docs/reference/filters.md
Original file line number Diff line number Diff line change
Expand Up @@ -1696,6 +1696,20 @@ oidcClaimsQuery("/:name%\"*One\"", "/path:groups.#[%\"*-Test-Users\"] groups.#[=
As of now there is no negative/deny rule possible. The first matching path is evaluated against the defined query/queries and if positive, permitted.

## Cookie Handling
### dropRequestCookie
AlexanderYastrebov marked this conversation as resolved.
Show resolved Hide resolved

Deletes given cookie from the request header.

Parameters:

* cookie name (string)

Example:

```
dropRequestCookie("test-session")
```

### requestCookie

Append a cookie to the request header.
Expand Down
1 change: 1 addition & 0 deletions filters/builtin/builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ func Filters() []filters.Spec {
sed.NewRequest(),
sed.NewDelimitedRequest(),
auth.NewBasicAuth(),
cookie.NewDropRequestCookie(),
cookie.NewRequestCookie(),
cookie.NewResponseCookie(),
cookie.NewJSCookie(),
Expand Down
71 changes: 71 additions & 0 deletions filters/cookie/cookie.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ package cookie
import (
"net"
"net/http"
"net/textproto"
"strings"
"time"

Expand Down Expand Up @@ -76,6 +77,76 @@ type filter struct {
changeOnly bool
}

type dropRequestCookie struct {
name string
}

func NewDropRequestCookie() filters.Spec {
return &dropRequestCookie{}
}

func (*dropRequestCookie) Name() string {
return filters.DropRequestCookieName
}
func (*dropRequestCookie) CreateFilter(args []interface{}) (filters.Filter, error) {
if len(args) != 1 {
return nil, filters.ErrInvalidFilterParameters
}

s, ok := args[0].(string)
if !ok {
return nil, filters.ErrInvalidFilterParameters
}

return &dropRequestCookie{
name: s,
}, nil
}
func (d *dropRequestCookie) Request(ctx filters.FilterContext) {
req := ctx.Request()

toDelete, err := req.Cookie(d.name)
if err != nil {
return // not found, done
}

lines := req.Header["Cookie"]
if len(lines) == 0 {
return
}

var result []string

// modified version of readCookies
// https://github.com/golang/go/blob/master/src/net/http/cookie.go#L277
RomanZavodskikh marked this conversation as resolved.
Show resolved Hide resolved
for _, line := range lines {
line = textproto.TrimString(line)
var part string

for len(line) > 0 { // continue since we have rest
part, line, _ = strings.Cut(line, ";")
part = textproto.TrimString(part)
if part == "" {
continue
}
name, _, _ := strings.Cut(part, "=")
if toDelete.Name == textproto.TrimString(name) {
continue
}

result = append(result, part)
}
}

if len(result) == 0 {
delete(req.Header, "Cookie")
} else {
req.Header["Cookie"] = result
AlexanderYastrebov marked this conversation as resolved.
Show resolved Hide resolved
}
}

func (*dropRequestCookie) Response(filters.FilterContext) {}

// Creates a filter spec for appending cookies to requests.
// Name: requestCookie
func NewRequestCookie() filters.Spec {
Expand Down
103 changes: 101 additions & 2 deletions filters/cookie/cookie_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package cookie

import (
"github.com/zalando/skipper/filters"
"github.com/zalando/skipper/filters/filtertest"
"net/http"
"testing"
"time"

"github.com/zalando/skipper/filters"
"github.com/zalando/skipper/filters/filtertest"
)

func TestCreateFilter(t *testing.T) {
Expand Down Expand Up @@ -382,3 +383,101 @@ func TestSetCookie(t *testing.T) {
}
}
}

func TestDropCookie(t *testing.T) {
RomanZavodskikh marked this conversation as resolved.
Show resolved Hide resolved
for _, tt := range []struct {
name string
arg string
cookies http.Header
wantCookies map[string]string
wantErr bool
}{
{
name: "test no cookies",
arg: "no-cookie",
cookies: nil,
wantCookies: nil,
wantErr: false,
},
{
name: "test one cookie not match",
arg: "no-match",
cookies: http.Header{
"Cookie": []string{
"foo=foo1",
},
},
wantCookies: map[string]string{"foo": "foo1"},
wantErr: false,
},
{
name: "test one cookie with match",
arg: "foo",
cookies: http.Header{
"Cookie": []string{
"foo=foo1",
},
},
wantCookies: nil,
wantErr: false,
},
{
name: "test two cookies and one cookie with match",
arg: "foo",
cookies: http.Header{
"Cookie": []string{
"foo=foo1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add more test cases that contain multiple cookies per value

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there could also be several cookies with the same name

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

"bar=baz",
},
},
wantCookies: map[string]string{"bar": "baz"},
wantErr: false,
}} {
t.Run(tt.name, func(t *testing.T) {
spec := NewDropRequestCookie()
f, err := spec.CreateFilter([]interface{}{tt.arg})
if err != nil {
t.Fatalf("Failed to create filter: %v", err)
}

if f == nil {
t.Fatal("Failed to create filter: filter nil")
}

ctx := &filtertest.Context{
FRequest: &http.Request{
Header: tt.cookies,
Host: "foo"},
FStateBag: map[string]interface{}{},
FResponse: &http.Response{Header: http.Header{}},
}

f.Request(ctx)

if c, err := ctx.Request().Cookie(tt.arg); err != http.ErrNoCookie {
t.Fatalf("Failed to delete cookie %s: %q", tt.arg, c)
}
if len(tt.wantCookies) != len(ctx.Request().Cookies()) {
t.Fatalf("Failed to get right len of cookies %d != %d", len(tt.wantCookies), len(ctx.Request().Cookies()))
}

for k, v := range tt.wantCookies {
cookie, err := ctx.Request().Cookie(k)
if err != nil {
t.Fatalf("Failed to get cookie %q: %v", k, err)
}

if cookie.Value != v {
t.Fatalf("Failed to get cookie value %q, got: %q", v, cookie.Value)
}
}

for _, cookie := range ctx.Request().Cookies() {
if _, ok := tt.wantCookies[cookie.Name]; !ok {
t.Fatalf("Failed to delete cookie: %s", cookie.Name)
}
}
})
}

}
3 changes: 2 additions & 1 deletion filters/filters.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,9 @@ const (
OAuthOidcUserInfoName = "oauthOidcUserInfo"
OAuthOidcAnyClaimsName = "oauthOidcAnyClaims"
OAuthOidcAllClaimsName = "oauthOidcAllClaims"
RequestCookieName = "requestCookie"
OidcClaimsQueryName = "oidcClaimsQuery"
DropRequestCookieName = "dropRequestCookie"
RequestCookieName = "requestCookie"
ResponseCookieName = "responseCookie"
JsCookieName = "jsCookie"
ConsecutiveBreakerName = "consecutiveBreaker"
Expand Down