fix: improve caching strategy and config for hydrator (#433)

To enable the hydrator cache you must now use the `cache.enabled` property. Also, the cache key strategy has been improved.

.schema/config.schema.json

@@ -616,7 +616,8 @@
                   "description": "The OAuth 2.0 Scope to be requested during the OAuth 2.0 Client Credentials Grant.",
                   "examples": [
                     [
-                      "foo", "bar"
+                      "foo",
+                      "bar"
                     ]
                   ]
                 }
@@ -710,8 +711,9 @@
               "pattern": "^[0-9]+(ns|us|ms|s|m|h)$",
               "title": "Cache Time to Live",
               "description": "Can override the default behaviour of using the token exp time, and specify a set time to live for the token in the cache.",
-              "examples": ["5s"],
-              "description": "How long to cache hydrate calls"
+              "examples": [
+                "5s"
+              ]
             }
           }
         }
@@ -889,16 +891,17 @@
         },
         "cache": {
           "additionalProperties": false,
-          "required": [
-            "ttl"
-          ],
           "type": "object",
           "properties": {
+            "enabled": {
+              "$ref": "#/definitions/handlerSwitch"
+            },
             "ttl": {
               "type": "string",
               "pattern": "^[0-9]+(ns|us|ms|s|m|h)$",
               "title": "Cache Time to Live",
-              "description": "How long to cache hydrate calls"
+              "description": "How long to cache hydrate calls",
+              "default": "1m"
             }
           }
         }
@@ -1286,9 +1289,11 @@
           "default": [
             "json"
           ],
-          "examples": [[
-            "redirect"
-          ]]
+          "examples": [
+            [
+              "redirect"
+            ]
+          ]
         },
         "handlers": {
           "additionalProperties": false,

docs/docs/pipeline/mutator.md

@@ -486,6 +486,18 @@ to the value received from an API.
 Setting `extra` field does not transform the HTTP request, whereas headers set
 in the `header` field will be added to the final request headers.
 
+### Cache
+
+This handler supports caching. If caching is enabled, the `api.url` configuration value
+and the the full `AuthenticationSession` payload.
+
+:::info
+
+Because the cache key is quite complex, the caching handler has a higher chance of cache misses.
+This will be improved in future versions.
+
+:::
+
 ### Configuration
 
 - `api.url` (string - required) - The API URL.

driver/configuration/provider_viper_public_test.go

@@ -63,13 +63,13 @@ func TestPipelineConfig(t *testing.T) {
 		p := setup(t)
 		res := json.RawMessage{}
 		require.Error(t, p.PipelineConfig("mutators", "hydrator", json.RawMessage(`{"not-api":"invalid"}`), &res))
-		assert.JSONEq(t, `{"api":{"url":"https://some-url/","retry":{"give_up_after":"1s","max_delay":"100ms"}},"not-api":"invalid"}`, string(res))
+		assert.JSONEq(t, `{"cache":{"enabled":false,"ttl":"1m"},"api":{"url":"https://some-url/","retry":{"give_up_after":"1s","max_delay":"100ms"}},"not-api":"invalid"}`, string(res))
 
 		require.Error(t, p.PipelineConfig("mutators", "hydrator", json.RawMessage(`{"api":{"this-key-does-not-exist":true}}`), &res))
-		assert.JSONEq(t, `{"api":{"url":"https://some-url/","this-key-does-not-exist":true,"retry":{"give_up_after":"1s","max_delay":"100ms"}}}`, string(res))
+		assert.JSONEq(t, `{"cache":{"enabled":false,"ttl":"1m"},"api":{"url":"https://some-url/","this-key-does-not-exist":true,"retry":{"give_up_after":"1s","max_delay":"100ms"}}}`, string(res))
 
 		require.Error(t, p.PipelineConfig("mutators", "hydrator", json.RawMessage(`{"api":{"url":"not-a-url"}}`), &res))
-		assert.JSONEq(t, `{"api":{"url":"not-a-url","retry":{"give_up_after":"1s","max_delay":"100ms"}}}`, string(res))
+		assert.JSONEq(t, `{"cache":{"enabled":false,"ttl":"1m"},"api":{"url":"not-a-url","retry":{"give_up_after":"1s","max_delay":"100ms"}}}`, string(res))
 	})
 
 	t.Run("case=should pass and override values", func(t *testing.T) {

go.mod

@@ -1,7 +1,6 @@
 module github.com/ory/oathkeeper
 
 require (
-	github.com/Azure/go-autorest/logger v0.1.0 // indirect
 	github.com/Masterminds/goutils v1.1.0 // indirect
 	github.com/Masterminds/sprig v2.20.0+incompatible
 	github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a
@@ -32,6 +31,7 @@ require (
 	github.com/julienschmidt/httprouter v1.2.0
 	github.com/lib/pq v1.3.0
 	github.com/mattn/goveralls v0.0.5
+	github.com/mitchellh/copystructure v1.0.0
 	github.com/ory/analytics-go/v4 v4.0.1
 	github.com/ory/fosite v0.29.2
 	github.com/ory/go-acc v0.2.1

go.sum

@@ -11,9 +11,6 @@ cloud.google.com/go/datastore v1.0.0/go.mod h1:LXYbyblFSglQ5pkeyhO+Qmw7ukd3C+pD7
 cloud.google.com/go/pubsub v1.0.1/go.mod h1:R0Gpsv3s54REJCy4fxDixWD93lHJMoZTyQ2kNxGRt3I=
 github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78 h1:w+iIsaOQNcT7OZ575w+acHgRric5iCyQh+xv+KJ4HB8=
 github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8=
-github.com/Azure/go-autorest v14.0.1+incompatible h1:YhojO9jolWIvvTW7ORhz2ZSNF6Q1TbLqUunKd3jrtyw=
-github.com/Azure/go-autorest/logger v0.1.0 h1:ruG4BSDXONFRrZZJ2GUXDiUyVpayPmb1GnWeHDdaNKY=
-github.com/Azure/go-autorest/logger v0.1.0/go.mod h1:oExouG+K6PryycPJfVSxi/koC6LSNgds39diKLz7Vrc=
 github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
@@ -479,6 +476,7 @@ github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5a
 github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
 github.com/google/go-cmp v0.3.1 h1:Xye71clBPdm5HgqGwUkwhbynsUJZhDbS20FvLhQ2izg=
 github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
+github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
@@ -684,6 +682,8 @@ github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0j
 github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
 github.com/microcosm-cc/bluemonday v1.0.1/go.mod h1:hsXNsILzKxV+sX77C5b8FSuKF00vh2OMYv+xgHpAMF4=
 github.com/microcosm-cc/bluemonday v1.0.2/go.mod h1:iVP4YcDBq+n/5fb23BhYFvIMq/leAFZyRl6bYmGDlGc=
+github.com/mitchellh/copystructure v1.0.0 h1:Laisrj+bAB6b/yJwB5Bt3ITZhGJdqmxquMKeZ+mmkFQ=
+github.com/mitchellh/copystructure v1.0.0/go.mod h1:SNtv71yrdKgLRyLFxmLdkAbkKEFWgYaq1OVrnRcwhnw=
 github.com/mitchellh/go-homedir v1.0.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
 github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
 github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
@@ -692,6 +692,8 @@ github.com/mitchellh/mapstructure v1.1.2 h1:fmNYVwqnSfB9mZU6OS2O6GsXM+wcskZDuKQz
 github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y=
 github.com/mitchellh/mapstructure v1.2.2 h1:dxe5oCinTXiTIcfgmZecdCzPmAJKd46KsCWc35r0TV4=
 github.com/mitchellh/mapstructure v1.2.2/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo=
+github.com/mitchellh/reflectwalk v1.0.0 h1:9D+8oIskB4VJBN5SFlmc27fSlIBZaov1Wpk/IfikLNY=
+github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
 github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
 github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
 github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
@@ -761,16 +763,6 @@ github.com/ory/jsonschema/v3 v3.0.1/go.mod h1:jgLHekkFk0uiGdEWGleC+tOm6JSSP8cbf1
 github.com/ory/ladon v1.1.0 h1:6tgazU2J3Z3odPs1f0qn729kRXCAtlJROliuWUHedV0=
 github.com/ory/ladon v1.1.0/go.mod h1:25bNc/Glx/8xCH7MbItDxjvviAmFQ+aYxb1V1SE5wlg=
 github.com/ory/pagination v0.0.1/go.mod h1:d1ToRROAUleriPhmb2dYbhANhhLwZ8s395m2yJCDFh8=
-github.com/ory/sdk/swagutil v0.0.0-20200202121523-307941feee4b h1:xn3WcZ8Oy285KYiCnoscQxkyRfJZT+KhIbU9LEhPLyw=
-github.com/ory/sdk/swagutil v0.0.0-20200202121523-307941feee4b/go.mod h1:Ufg1eAyz+Zt3+oweSZVThG13ewewWCKwBmoNmK8Z0co=
-github.com/ory/sdk/swagutil v0.0.0-20200403154420-81a368933686 h1:U+74xA6gZn836eJEPyha/Yrix70ytZRrgJhwY0qjOZE=
-github.com/ory/sdk/swagutil v0.0.0-20200403154420-81a368933686/go.mod h1:Ufg1eAyz+Zt3+oweSZVThG13ewewWCKwBmoNmK8Z0co=
-github.com/ory/sdk/swagutil v0.0.0-20200407150153-53df6d772608 h1:FFvuEWPDFcJD3NYcfoUFFP7bUMeqOxgclbCP2vYIzKs=
-github.com/ory/sdk/swagutil v0.0.0-20200407150153-53df6d772608/go.mod h1:Ufg1eAyz+Zt3+oweSZVThG13ewewWCKwBmoNmK8Z0co=
-github.com/ory/sdk/swagutil v0.0.0-20200416162902-684156244f2c h1:w9ABs54jbJxbh3m3PIKC5/ctZA1DZ629ACuHbY5/DgM=
-github.com/ory/sdk/swagutil v0.0.0-20200416162902-684156244f2c/go.mod h1:Ufg1eAyz+Zt3+oweSZVThG13ewewWCKwBmoNmK8Z0co=
-github.com/ory/sdk/swagutil v0.0.0-20200425113349-92ce176f501e h1:X2MrG0850tnYPHyHNOCugvqcs0nfnqfmXHo3HrUFPdc=
-github.com/ory/sdk/swagutil v0.0.0-20200425113349-92ce176f501e/go.mod h1:Ufg1eAyz+Zt3+oweSZVThG13ewewWCKwBmoNmK8Z0co=
 github.com/ory/sdk/swagutil v0.0.0-20200505101021-3f40b808145c h1:xxLzUYgOhUfui6LXZrzNnVc2MKV6D1+Vxj0Mx2eoKs4=
 github.com/ory/sdk/swagutil v0.0.0-20200505101021-3f40b808145c/go.mod h1:Ufg1eAyz+Zt3+oweSZVThG13ewewWCKwBmoNmK8Z0co=
 github.com/ory/viper v1.5.6/go.mod h1:TYmpFpKLxjQwvT4f0QPpkOn4sDXU1kDgAwJpgLYiQ28=

pipeline/authn/authenticator.go

@@ -9,6 +9,8 @@ import (
 
 	"github.com/ory/herodot"
 
+	"github.com/mitchellh/copystructure"
+
 	"github.com/ory/oathkeeper/pipeline"
 )
 
@@ -55,3 +57,7 @@ func (a *AuthenticationSession) SetHeader(key, val string) {
 	}
 	a.Header.Set(key, val)
 }
+
+func (a *AuthenticationSession) Copy() *AuthenticationSession {
+	return copystructure.Must(copystructure.Copy(a)).(*AuthenticationSession)
+}

pipeline/authn/authenticator_oauth2_introspection.go

@@ -138,7 +138,6 @@ func (a *AuthenticatorOAuth2Introspection) Authenticate(r *http.Request, session
 	ss := a.c.ToScopeStrategy(cf.ScopeStrategy, "authenticators.oauth2_introspection.scope_strategy")
 
 	i, ok := a.tokenFromCache(cf, token)
-
 	if !ok {
 		body := url.Values{"token": {token}}
 

pipeline/authn/authenticator_test.go

@@ -2,10 +2,13 @@ package authn_test
 
 import (
 	"fmt"
+	"net/http"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 
+	"github.com/ory/x/urlx"
+
 	"github.com/ory/oathkeeper/pipeline/authn"
 )
 
@@ -15,7 +18,6 @@ const (
 )
 
 func TestSetHeader(t *testing.T) {
-
 	assert := assert.New(t)
 	for k, tc := range []struct {
 		a    *authn.AuthenticationSession
@@ -39,3 +41,29 @@ func TestSetHeader(t *testing.T) {
 		})
 	}
 }
+
+func TestCopy(t *testing.T) {
+	assert := assert.New(t)
+	original := &authn.AuthenticationSession{
+		Subject: "ab",
+		Extra:   map[string]interface{}{"a": "b", "b": map[string]string{"a:": "b"}},
+		Header:  http.Header{"foo": {"bar", "baz"}},
+		MatchContext: authn.MatchContext{
+			RegexpCaptureGroups: []string{"a", "b"},
+			URL:                 urlx.ParseOrPanic("https://foo/bar"),
+		},
+	}
+
+	copied := original.Copy()
+	copied.Subject = "ba"
+	copied.Extra["baz"] = "bar"
+	copied.Header.Add("bazbar", "bar")
+	copied.MatchContext.URL.Host = "asdf"
+	copied.MatchContext.RegexpCaptureGroups[0] = "b"
+
+	assert.NotEqual(original.Subject, copied.Subject)
+	assert.NotEqual(original.Extra, copied.Extra)
+	assert.NotEqual(original.Header, copied.Header)
+	assert.NotEqual(original.MatchContext.URL.Host, copied.MatchContext.URL.Host)
+	assert.NotEqual(original.MatchContext.RegexpCaptureGroups, copied.MatchContext.RegexpCaptureGroups)
+}

pipeline/mutate/mutator_hydrator.go

@@ -22,6 +22,7 @@ package mutate
 
 import (
 	"bytes"
+	"crypto/md5"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -48,8 +49,6 @@ const (
 	ErrNon200ResponseFromAPI            = "The call to an external API returned a non-200 HTTP response"
 	ErrInvalidCredentials               = "Invalid credentials were provided in mutator configuration"
 	ErrNoCredentialsProvided            = "No credentials were provided in mutator configuration"
-	defaultNumberOfRetries              = 3
-	defaultDelayInMilliseconds          = 100
 	contentTypeHeaderKey                = "Content-Type"
 	contentTypeJSONHeaderValue          = "application/json"
 )
@@ -84,7 +83,10 @@ type externalAPIConfig struct {
 }
 
 type cacheConfig struct {
-	TTL string `json:"ttl"`
+	Enabled bool   `json:"enabled"`
+	TTL     string `json:"ttl"`
+
+	ttl time.Duration
 }
 
 type MutatorHydratorConfig struct {
@@ -112,35 +114,30 @@ func (a *MutatorHydrator) GetID() string {
 	return "hydrator"
 }
 
-func (a *MutatorHydrator) cacheKey(config *MutatorHydratorConfig, session *authn.AuthenticationSession) string {
-	return fmt.Sprintf("%s|%s", config.Api.URL, session.Subject)
+func (a *MutatorHydrator) cacheKey(config *MutatorHydratorConfig, session string) string {
+	return fmt.Sprintf("%s|%x", config.Api.URL, md5.Sum([]byte(session)))
 }
 
-func (a *MutatorHydrator) hydrateFromCache(config *MutatorHydratorConfig, session *authn.AuthenticationSession) (*authn.AuthenticationSession, bool) {
-	if a.cacheTTL == nil {
+func (a *MutatorHydrator) hydrateFromCache(config *MutatorHydratorConfig, session string) (*authn.AuthenticationSession, bool) {
+	if !config.Cache.Enabled {
 		return nil, false
 	}
 
-	key := a.cacheKey(config, session)
-
-	item, found := a.hydrateCache.Get(key)
+	item, found := a.hydrateCache.Get(a.cacheKey(config, session))
 	if !found {
 		return nil, false
 	}
 
-	container := item.(*authn.AuthenticationSession)
-	return container, true
+	return item.(*authn.AuthenticationSession).Copy(), true
 }
 
-func (a *MutatorHydrator) hydrateToCache(config *MutatorHydratorConfig, session *authn.AuthenticationSession) {
-	if a.cacheTTL == nil {
+func (a *MutatorHydrator) hydrateToCache(config *MutatorHydratorConfig, key string, session *authn.AuthenticationSession) {
+	if !config.Cache.Enabled {
 		return
 	}
 
-	key := a.cacheKey(config, session)
-	cached := a.hydrateCache.SetWithTTL(key, session, 0, *a.cacheTTL)
-	if !cached {
-		a.d.Logger().Warn("Item not added to cache")
+	if a.hydrateCache.SetWithTTL(a.cacheKey(config, key), session.Copy(), 0, config.Cache.ttl) {
+		a.d.Logger().Debug("Cache reject item")
 	}
 }
 
@@ -155,7 +152,8 @@ func (a *MutatorHydrator) Mutate(r *http.Request, session *authn.AuthenticationS
 		return errors.WithStack(err)
 	}
 
-	if cacheSession, ok := a.hydrateFromCache(cfg, session); ok {
+	encodedSession := b.String()
+	if cacheSession, ok := a.hydrateFromCache(cfg, encodedSession); ok {
 		*session = *cacheSession
 		return nil
 	}
@@ -230,7 +228,7 @@ func (a *MutatorHydrator) Mutate(r *http.Request, session *authn.AuthenticationS
 	}
 	*session = sessionFromUpstream
 
-	a.hydrateToCache(cfg, session)
+	a.hydrateToCache(cfg, encodedSession, session)
 
 	return nil
 }
@@ -250,13 +248,17 @@ func (a *MutatorHydrator) Config(config json.RawMessage) (*MutatorHydratorConfig
 		return nil, NewErrMutatorMisconfigured(a, err)
 	}
 
-	if c.Cache.TTL != "" {
-		cacheTTL, err := time.ParseDuration(c.Cache.TTL)
+	if c.Cache.Enabled {
+		var err error
+		c.Cache.ttl, err = time.ParseDuration(c.Cache.TTL)
 		if err != nil {
-			a.d.Logger().WithError(err).Error("Unable to parse cache ttl in the Hydrator Mutator.")
+			a.d.Logger().WithError(err).WithField("ttl", c.Cache.TTL).Error("Unable to parse cache ttl in the Hydrator Mutator.")
 			return nil, NewErrMutatorMisconfigured(a, err)
 		}
-		a.cacheTTL = &cacheTTL
+
+		if c.Cache.ttl == 0 {
+			c.Cache.ttl = time.Minute
+		}
 	}
 
 	return &c, nil