Skip to content
This repository has been archived by the owner on Dec 7, 2020. It is now read-only.

Commit

Permalink
Upstream Authorization Cookie (#287)
Browse files Browse the repository at this point in the history
- adding an option to stop the proxy from including the authorization cookies in the upstream request
  • Loading branch information
gambol99 authored Nov 23, 2017
1 parent 7179eac commit 81c7892
Show file tree
Hide file tree
Showing 7 changed files with 89 additions and 27 deletions.
1 change: 1 addition & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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/",
Expand Down
8 changes: 5 additions & 3 deletions doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
8 changes: 7 additions & 1 deletion middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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())
Expand All @@ -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 {
Expand Down
17 changes: 10 additions & 7 deletions middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
26 changes: 26 additions & 0 deletions misc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

This comment has been minimized.

Copy link
@jangaraj

jangaraj Nov 24, 2017

Contributor

See #278 -> we can have also cookie names r.config.CookieAccessName, r.config.CookieRefreshName with suffix "-". Probably better condition:

if x.Name == n || strings.HasSuffix(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
Expand Down
50 changes: 37 additions & 13 deletions server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 81c7892

Please sign in to comment.