From d070c5dff2a65ec780789921b5d95b35b86b7107 Mon Sep 17 00:00:00 2001 From: Rohith Date: Fri, 17 Nov 2017 13:43:13 +0000 Subject: [PATCH] Upstream Authorization Cookie - adding an option to stop the proxy from including the authorization cookies in the upstream request --- config.go | 1 + doc.go | 8 +++++--- handlers_test.go | 6 +++--- middleware.go | 8 +++++++- middleware_test.go | 17 +++++++++------- misc.go | 26 ++++++++++++++++++++++++ server_test.go | 50 ++++++++++++++++++++++++++++++++++------------ 7 files changed, 89 insertions(+), 27 deletions(-) diff --git a/config.go b/config.go index 927d87935..d55e318d4 100644 --- a/config.go +++ b/config.go @@ -31,6 +31,7 @@ func newDefaultConfig() *Config { CookieAccessName: "kc-access", CookieRefreshName: "kc-state", EnableAuthorizationHeader: true, + EnableAuthorizationCookies: true, EnableTokenHeader: true, Headers: make(map[string]string), LetsEncryptCacheDir: "./cache/", diff --git a/doc.go b/doc.go index 0b4ddb318..b2e0696da 100644 --- a/doc.go +++ b/doc.go @@ -134,8 +134,6 @@ type Config struct { // Headers permits adding customs headers across the board Headers map[string]string `json:"headers" yaml:"headers" usage:"custom headers to the upstream request, key=value"` - // EnableTokenHeader adds the JWT token to the upstream authentication headers - EnableTokenHeader bool `json:"enable-token-header" yaml:"enable-token-header" usage:"enables the token authentication header X-Auth-Token to upstream"` // EnableEncryptedToken indicates the access token should be encoded EnableEncryptedToken bool `json:"enable-encrypted-token" yaml:"enable-encrypted-token" usage:"enable encryption for the access tokens"` // EnableLogging indicates if we should log all the requests @@ -150,8 +148,12 @@ type Config struct { EnableRefreshTokens bool `json:"enable-refresh-tokens" yaml:"enable-refresh-tokens" usage:"enables the handling of the refresh tokens" env:"ENABLE_REFRESH_TOKEN"` // EnableLoginHandler indicates we want the login handler enabled EnableLoginHandler bool `json:"enable-login-handler" yaml:"enable-login-handler" usage:"enables the handling of the refresh tokens" env:"ENABLE_LOGIN_HANDLER"` + // EnableTokenHeader adds the JWT token to the upstream authentication headers + EnableTokenHeader bool `json:"enable-token-header" yaml:"enable-token-header" usage:"enables the token authentication header X-Auth-Token to upstream"` // EnableAuthorizationHeader indicates we should pass the authorization header - EnableAuthorizationHeader bool `json:"enable-authorization-header" yaml:"enable-authorization-header" usage:"adds the authorization header to the proxy request"` + EnableAuthorizationHeader bool `json:"enable-authorization-header" yaml:"enable-authorization-header" usage:"adds the authorization header to the proxy request" env:"ENABLE_AUTHORIZATION_HEADER"` + // EnableAuthorizationCookies indicates we should pass the authorization cookies to the upstream endpoint + EnableAuthorizationCookies bool `json:"enable-authorization-cookies" yaml:"enable-authorization-cookies" usage:"adds the authorization cookies to the uptream proxy request" env:"ENABLE_AUTHORIZATION_COOKIES"` // EnableHTTPSRedirect indicate we should redirection http -> https EnableHTTPSRedirect bool `json:"enable-https-redirection" yaml:"enable-https-redirection" usage:"enable the http to https redirection on the http service"` // EnableProfiling indicates if profiles is switched on diff --git a/handlers_test.go b/handlers_test.go index 80f6a47c2..9455fc9c3 100644 --- a/handlers_test.go +++ b/handlers_test.go @@ -288,19 +288,19 @@ func TestCallbackURL(t *testing.T) { }, { URI: oauthURL + callbackURL + "?code=fake", - ExpectedCookies: []string{cfg.CookieAccessName}, + ExpectedCookies: map[string]string{cfg.CookieAccessName: ""}, ExpectedLocation: "/", ExpectedCode: http.StatusTemporaryRedirect, }, { URI: oauthURL + callbackURL + "?code=fake&state=/admin", - ExpectedCookies: []string{cfg.CookieAccessName}, + ExpectedCookies: map[string]string{cfg.CookieAccessName: ""}, ExpectedLocation: "/", ExpectedCode: http.StatusTemporaryRedirect, }, { URI: oauthURL + callbackURL + "?code=fake&state=L2FkbWlu", - ExpectedCookies: []string{cfg.CookieAccessName}, + ExpectedCookies: map[string]string{cfg.CookieAccessName: ""}, ExpectedLocation: "/admin", ExpectedCode: http.StatusTemporaryRedirect, }, diff --git a/middleware.go b/middleware.go index d7bc79108..58438e7b5 100644 --- a/middleware.go +++ b/middleware.go @@ -317,6 +317,8 @@ func (r *oauthProxy) headersMiddleware(custom []string) func(http.Handler) http. customClaims[x] = fmt.Sprintf("X-Auth-%s", toHeader(x)) } + cookieFilter := []string{r.config.CookieAccessName, r.config.CookieRefreshName} + return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { scope := req.Context().Value(contextScopeName).(*RequestScope) @@ -328,6 +330,7 @@ func (r *oauthProxy) headersMiddleware(custom []string) func(http.Handler) http. req.Header.Set("X-Auth-Subject", user.id) req.Header.Set("X-Auth-Userid", user.name) req.Header.Set("X-Auth-Username", user.name) + // should we add the token header? if r.config.EnableTokenHeader { req.Header.Set("X-Auth-Token", user.token.Encode()) @@ -336,7 +339,10 @@ func (r *oauthProxy) headersMiddleware(custom []string) func(http.Handler) http. if r.config.EnableAuthorizationHeader { req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", user.token.Encode())) } - + // are we filtering out the cookies + if !r.config.EnableAuthorizationCookies { + filterCookies(req, cookieFilter) + } // inject any custom claims for claim, header := range customClaims { if claim, found := user.claims[claim]; found { diff --git a/middleware_test.go b/middleware_test.go index fb151be42..2021b3a14 100644 --- a/middleware_test.go +++ b/middleware_test.go @@ -57,7 +57,7 @@ type fakeRequest struct { ExpectedCode int ExpectedContent string ExpectedContentContains string - ExpectedCookies []string + ExpectedCookies map[string]string ExpectedHeaders map[string]string ExpectedProxyHeaders map[string]string ExpectedLocation string @@ -238,11 +238,14 @@ func (f *fakeProxy) RunTests(t *testing.T, requests []fakeRequest) { assert.Contains(t, e, c.ExpectedContentContains, "case %d, expected content: %s, got: %s", i, c.ExpectedContentContains, e) } if len(c.ExpectedCookies) > 0 { - l := len(c.ExpectedCookies) - g := len(resp.Cookies()) - assert.Equal(t, l, g, "case %d, expected %d cookies, got: %d", i, l, g) - for _, x := range c.ExpectedCookies { - assert.NotNil(t, findCookie(x, resp.Cookies()), "case %d, expected cookie %s not found", i, x) + for k, v := range c.ExpectedCookies { + cookie := findCookie(k, resp.Cookies()) + if !assert.NotNil(t, cookie, "case %d, expected cookie %s not found", i, k) { + continue + } + if v != "" { + assert.Equal(t, cookie.Value, v, "case %d, expected cookie value: %s, got: %s", i, v, cookie.Value) + } } } if c.OnResponse != nil { @@ -883,7 +886,7 @@ func TestCheckRefreshTokens(t *testing.T) { Redirects: false, ExpectedProxy: true, ExpectedCode: http.StatusOK, - ExpectedCookies: []string{cfg.CookieAccessName}, + ExpectedCookies: map[string]string{cfg.CookieAccessName: ""}, }, } p.RunTests(t, requests) diff --git a/misc.go b/misc.go index 41cfc1f87..eab4fd85d 100644 --- a/misc.go +++ b/misc.go @@ -27,6 +27,32 @@ import ( "go.uber.org/zap" ) +// filterCookies is responsible for censoring any cookies we don't want sent +func filterCookies(req *http.Request, filter []string) error { + // @NOTE: there doesn't appear to be a way of removing a cookie from the http.Request as + // AddCookie() just append + cookies := req.Cookies() + // @step: empty the current cookies + req.Header.Set("Cookie", "") + // @step: iterate the cookies and filter out anything we + for _, x := range cookies { + var found bool + // @step: does this cookie match our filter? + for _, n := range filter { + if x.Name == n { + req.AddCookie(&http.Cookie{Name: x.Name, Value: "censored"}) + found = true + break + } + } + if !found { + req.AddCookie(x) + } + } + + return nil +} + // revokeProxy is responsible to stopping the middleware from proxying the request func (r *oauthProxy) revokeProxy(w http.ResponseWriter, req *http.Request) context.Context { var scope *RequestScope diff --git a/server_test.go b/server_test.go index 5b8e0ef15..501bde7cf 100644 --- a/server_test.go +++ b/server_test.go @@ -327,6 +327,29 @@ func TestAuthTokenHeaderDisabled(t *testing.T) { p.RunTests(t, requests) } +func TestDisableAuthorizationCookie(t *testing.T) { + c := newFakeKeycloakConfig() + c.EnableAuthorizationCookies = false + p := newFakeProxy(c) + token := newTestToken(p.idp.getLocation()) + signed, _ := p.idp.signToken(token.claims) + + requests := []fakeRequest{ + { + URI: "/auth_all/test", + Cookies: []*http.Cookie{ + {Name: c.CookieAccessName, Value: signed.Encode()}, + {Name: "mycookie", Value: "myvalue"}, + }, + HasToken: true, + ExpectedContentContains: "kc-access=censored; mycookie=myvalue", + ExpectedCode: http.StatusOK, + ExpectedProxy: true, + }, + } + p.RunTests(t, requests) +} + func newTestService() string { _, _, u := newTestProxyService(nil) return u @@ -375,19 +398,20 @@ func newFakeHTTPRequest(method, path string) *http.Request { func newFakeKeycloakConfig() *Config { return &Config{ - ClientID: fakeClientID, - ClientSecret: fakeSecret, - CookieAccessName: "kc-access", - CookieRefreshName: "kc-state", - DisableAllLogging: true, - DiscoveryURL: "127.0.0.1:0", - EnableAuthorizationHeader: true, - EnableLogging: false, - EnableLoginHandler: true, - EnableTokenHeader: true, - Listen: "127.0.0.1:0", - Scopes: []string{}, - Verbose: true, + ClientID: fakeClientID, + ClientSecret: fakeSecret, + CookieAccessName: "kc-access", + CookieRefreshName: "kc-state", + DisableAllLogging: true, + DiscoveryURL: "127.0.0.1:0", + EnableAuthorizationHeader: true, + EnableAuthorizationCookies: true, + EnableLogging: false, + EnableLoginHandler: true, + EnableTokenHeader: true, + Listen: "127.0.0.1:0", + Scopes: []string{}, + Verbose: true, Resources: []*Resource{ { URL: fakeAdminRoleURL,