From 4603ad914b62437c7601dd2f04a8ced2368da929 Mon Sep 17 00:00:00 2001
From: Frederic BIDON <frederic@oneconcern.com>
Date: Wed, 12 Dec 2018 21:10:12 +0100
Subject: [PATCH] [KEYCLOAK-9074] Optionally enforce token encryption in cookie
* add config option to distinguish when to encrypt token: in header or/and in cookie
* fixes #9704
Signed-off-by: Frederic BIDON <frederic@oneconcern.com>
---
config.go | 2 +-
doc.go | 4 ++-
handlers.go | 10 +++---
middleware.go | 2 +-
middleware_test.go | 85 +++++++++++++++++++++++++++++++++++++++++++++-
session.go | 2 +-
6 files changed, 95 insertions(+), 10 deletions(-)
diff --git a/config.go b/config.go
index d49d4bd..5c4509f 100644
--- a/config.go
+++ b/config.go
@@ -173,7 +173,7 @@ func (r *Config) isValid() error {
return errors.New("the security filter must be switch on for this feature: hostnames")
}
}
- if r.EnableEncryptedToken && r.EncryptionKey == "" {
+ if (r.EnableEncryptedToken || r.ForceEncryptedCookie) && r.EncryptionKey == "" {
return errors.New("you have not specified an encryption key for encoding the access token")
}
if r.EnableRefreshTokens && r.EncryptionKey == "" {
diff --git a/doc.go b/doc.go
index cfa461d..f0686fd 100644
--- a/doc.go
+++ b/doc.go
@@ -211,6 +211,8 @@ type Config struct {
EnableDefaultDeny bool `json:"enable-default-deny" yaml:"enable-default-deny" usage:"enables a default denial on all requests, you have to explicitly say what is permitted (recommended)"`
// 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"`
+ // ForceEncryptedCookie indicates that the access token in the cookie should be encoded, regardless what EnableEncryptedToken says. This way, gatekeeper may receive tokens in header in the clear, whereas tokens in cookies remain encrypted
+ ForceEncryptedCookie bool `json:"force-encrypted-cookie" yaml:"force-encrypted-cookie" usage:"force encryption for the access tokens in cookies"`
// EnableLogging indicates if we should log all the requests
EnableLogging bool `json:"enable-logging" yaml:"enable-logging" usage:"enable http logging of the requests"`
// EnableJSONLogging is the logging format
@@ -227,7 +229,7 @@ type Config struct {
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 indicates we should pass the authorization header to the upstream endpoint
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"`
diff --git a/handlers.go b/handlers.go
index bc7be1b..64a5c55 100644
--- a/handlers.go
+++ b/handlers.go
@@ -142,8 +142,8 @@ func (r *oauthProxy) oauthCallbackHandler(w http.ResponseWriter, req *http.Reque
return
}
- // Flow: once we exchange the authorization code we parse the ID Token; we then check for a access token,
- // if a access token is present and we can decode it, we use that as the session token, otherwise we default
+ // Flow: once we exchange the authorization code we parse the ID Token; we then check for an access token,
+ // if an access token is present and we can decode it, we use that as the session token, otherwise we default
// to the ID Token.
token, identity, err := parseToken(resp.IDToken)
if err != nil {
@@ -168,7 +168,7 @@ func (r *oauthProxy) oauthCallbackHandler(w http.ResponseWriter, req *http.Reque
accessToken := token.Encode()
// step: are we encrypting the access token?
- if r.config.EnableEncryptedToken {
+ if r.config.EnableEncryptedToken || r.config.ForceEncryptedCookie {
if accessToken, err = encodeText(accessToken, r.config.EncryptionKey); err != nil {
r.log.Error("unable to encode the access token", zap.Error(err))
w.WriteHeader(http.StatusInternalServerError)
@@ -181,10 +181,10 @@ func (r *oauthProxy) oauthCallbackHandler(w http.ResponseWriter, req *http.Reque
zap.String("expires", identity.ExpiresAt.Format(time.RFC3339)),
zap.String("duration", time.Until(identity.ExpiresAt).String()))
- // @metric a token has beeb issued
+ // @metric a token has been issued
oauthTokensMetric.WithLabelValues("issued").Inc()
- // step: does the response has a refresh token and we are NOT ignore refresh tokens?
+ // step: does the response have a refresh token and we do NOT ignore refresh tokens?
if r.config.EnableRefreshTokens && resp.RefreshToken != "" {
var encrypted string
encrypted, err = encodeText(resp.RefreshToken, r.config.EncryptionKey)
diff --git a/middleware.go b/middleware.go
index 38c4835..f1ea568 100644
--- a/middleware.go
+++ b/middleware.go
@@ -194,7 +194,7 @@ func (r *oauthProxy) authenticationMiddleware(resource *Resource) func(http.Hand
zap.Duration("expires_in", time.Until(exp)))
accessToken := token.Encode()
- if r.config.EnableEncryptedToken {
+ if r.config.EnableEncryptedToken || r.config.ForceEncryptedCookie {
if accessToken, err = encodeText(accessToken, r.config.EncryptionKey); err != nil {
r.log.Error("unable to encode the access token", zap.Error(err))
w.WriteHeader(http.StatusInternalServerError)
diff --git a/middleware_test.go b/middleware_test.go
index 09fa77a..94e8f9a 100644
--- a/middleware_test.go
+++ b/middleware_test.go
@@ -34,6 +34,10 @@ import (
"gopkg.in/resty.v1"
)
+const (
+ testEncryptionKey = "ZSeCYDUxIlhDrmPpa1Ldc7il384esSF2"
+)
+
type fakeRequest struct {
BasicAuth bool
Cookies []*http.Cookie
@@ -66,6 +70,9 @@ type fakeRequest struct {
ExpectedNoProxyHeaders []string
ExpectedProxy bool
ExpectedProxyHeaders map[string]string
+
+ // advanced test cases
+ ExpectedCookiesValidator map[string]func(string) bool
}
type fakeProxy struct {
@@ -272,6 +279,15 @@ func (f *fakeProxy) RunTests(t *testing.T, requests []fakeRequest) {
assert.Equal(t, cookie.Value, v, "case %d, expected cookie value: %s, got: %s", i, v, cookie.Value)
}
}
+ for k, v := range c.ExpectedCookiesValidator {
+ cookie := findCookie(k, resp.Cookies())
+ if !assert.NotNil(t, cookie, "case %d, expected cookie %s not found", i, k) {
+ continue
+ }
+ if v != nil {
+ assert.True(t, v(cookie.Value), "case %d, invalid cookie value: %s", i, cookie.Value)
+ }
+ }
}
if c.OnResponse != nil {
c.OnResponse(i, request, resp)
@@ -1072,7 +1088,7 @@ func TestCrossSiteHandler(t *testing.T) {
func TestCheckRefreshTokens(t *testing.T) {
cfg := newFakeKeycloakConfig()
cfg.EnableRefreshTokens = true
- cfg.EncryptionKey = "ZSeCYDUxIlhDrmPpa1Ldc7il384esSF2"
+ cfg.EncryptionKey = testEncryptionKey
fn := func(no int, req *resty.Request, resp *resty.Response) {
if no == 0 {
<-time.After(1000 * time.Millisecond)
@@ -1101,6 +1117,73 @@ func TestCheckRefreshTokens(t *testing.T) {
p.RunTests(t, requests)
}
+func TestCheckEncryptedCookie(t *testing.T) {
+ cfg := newFakeKeycloakConfig()
+ cfg.EnableRefreshTokens = true
+ cfg.EnableEncryptedToken = true
+ cfg.Verbose = true
+ cfg.EnableLogging = true
+ cfg.EncryptionKey = testEncryptionKey
+ testEncryptedToken(t, cfg)
+}
+
+func TestCheckForcedEncryptedCookie(t *testing.T) {
+ cfg := newFakeKeycloakConfig()
+ cfg.EnableRefreshTokens = true
+ cfg.EnableEncryptedToken = false
+ cfg.ForceEncryptedCookie = true
+ cfg.Verbose = true
+ cfg.EnableLogging = true
+ cfg.EncryptionKey = testEncryptionKey
+ testEncryptedToken(t, cfg)
+}
+
+func testEncryptedToken(t *testing.T, cfg *Config) {
+ fn := func(no int, req *resty.Request, resp *resty.Response) {
+ if no == 0 {
+ <-time.After(1000 * time.Millisecond)
+ }
+ }
+ val := func(value string) bool {
+ // check the cookie value is an encrypted token
+ accessToken, err := decodeText(value, cfg.EncryptionKey)
+ if err != nil {
+ return false
+ }
+ jwt, err := jose.ParseJWT(accessToken)
+ if err != nil {
+ return false
+ }
+ claims, err := jwt.Claims()
+ if err != nil {
+ return false
+ }
+ return assert.Contains(t, claims, "aud") && assert.Contains(t, claims, "email")
+ }
+ p := newFakeProxy(cfg)
+ p.idp.setTokenExpiration(1000 * time.Millisecond)
+
+ requests := []fakeRequest{
+ {
+ URI: fakeAuthAllURL,
+ HasLogin: true,
+ Redirects: true,
+ OnResponse: fn,
+ ExpectedProxy: true,
+ ExpectedCode: http.StatusOK,
+ },
+ {
+ URI: fakeAuthAllURL,
+ Redirects: false,
+ ExpectedProxy: true,
+ ExpectedCode: http.StatusOK,
+ ExpectedCookies: map[string]string{cfg.CookieAccessName: ""},
+ ExpectedCookiesValidator: map[string]func(string) bool{cfg.CookieAccessName: val},
+ },
+ }
+ p.RunTests(t, requests)
+}
+
func TestCustomHeadersHandler(t *testing.T) {
requests := []struct {
Match []string
diff --git a/session.go b/session.go
index eff7363..76a3070 100644
--- a/session.go
+++ b/session.go
@@ -33,7 +33,7 @@ func (r *oauthProxy) getIdentity(req *http.Request) (*userContext, error) {
if err != nil {
return nil, err
}
- if r.config.EnableEncryptedToken {
+ if r.config.EnableEncryptedToken || r.config.ForceEncryptedCookie && !isBearer {
if access, err = decodeText(access, r.config.EncryptionKey); err != nil {
return nil, ErrDecryption
}
--
GitLab