From 8c0bbc624160ef22ba18380b935e74e46b4f1cc5 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 5 Sep 2018 11:55:46 +0200 Subject: [PATCH 1/4] t/apicast-policy-3scale-referrer: test that referrers are taken into account in the APIcast auths cache --- t/apicast-policy-3scale-referrer.t | 114 +++++++++++++++++++++++++++++ 1 file changed, 114 insertions(+) diff --git a/t/apicast-policy-3scale-referrer.t b/t/apicast-policy-3scale-referrer.t index b83a12eff..73f0f4c7d 100644 --- a/t/apicast-policy-3scale-referrer.t +++ b/t/apicast-policy-3scale-referrer.t @@ -223,3 +223,117 @@ yay, api backend --- error_code: 200 --- no_error_log [error] + +=== TEST 5: Referrer filters are taken into account in the APIcast auths cache +In this test, we make a request with valid credentials, and a valid referrer +filter. Then, we make a second one with the same credentials, but with an +invalid referrer, and we check that we get and "Authorization denied" error. +If the referrer filters were not taken into account in the auths cache, the +second request would return OK. We need to check that referrer filters are +taken into account. +--- configuration +{ + "services": [ + { + "id": 42, + "backend_version": 1, + "backend_authentication_type": "service_token", + "backend_authentication_value": "token-value", + "proxy": { + "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/", + "proxy_rules": [ + { "pattern": "/", "http_method": "GET", "metric_system_name": "hits", "delta": 2 } + ], + "policy_chain": [ + { + "name": "apicast.policy.3scale_referrer", + "configuration": {} + }, + { "name": "apicast.policy.apicast" } + ] + } + } + ] +} +--- upstream + location / { + content_by_lua_block { + ngx.say('yay, api backend'); + } + } +--- backend + location /transactions/authrep.xml { + content_by_lua_block { + -- Verify just the referrer. Assume the rest of params are correct + if ngx.req.get_uri_args()['referrer'] == '3scale.net' then + ngx.exit(200) + else + ngx.exit(403) + end + } + } +--- request eval +["GET /?user_key=uk", "GET /?user_key=uk", "GET /?user_key=uk"] +--- more_headers eval +["Referer: 3scale.net", "Referer: invalid", "Referer: 3scale.net"] +--- response_body eval +["yay, api backend\x{0a}", "Authentication failed", "yay, api backend\x{0a}"] +--- error_code eval +[200, 403, 200] +--- no_error_log +[error] + +=== TEST 6: Referrer filters taken into account in the auths cache when after APIcast in the chain +Same test as the one above but placing the Referrer policy after the APIcast +one in the chain. +--- configuration +{ + "services": [ + { + "id": 42, + "backend_version": 1, + "backend_authentication_type": "service_token", + "backend_authentication_value": "token-value", + "proxy": { + "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/", + "proxy_rules": [ + { "pattern": "/", "http_method": "GET", "metric_system_name": "hits", "delta": 2 } + ], + "policy_chain": [ + { "name": "apicast.policy.apicast" }, + { + "name": "apicast.policy.3scale_referrer", + "configuration": {} + } + ] + } + } + ] +} +--- upstream + location / { + content_by_lua_block { + ngx.say('yay, api backend'); + } + } +--- backend + location /transactions/authrep.xml { + content_by_lua_block { + -- Verify just the referrer. Assume the rest of params are correct + if ngx.req.get_uri_args()['referrer'] == '3scale.net' then + ngx.exit(200) + else + ngx.exit(403) + end + } + } +--- request eval +["GET /?user_key=uk", "GET /?user_key=uk", "GET /?user_key=uk"] +--- more_headers eval +["Referer: 3scale.net", "Referer: invalid", "Referer: 3scale.net"] +--- response_body eval +["yay, api backend\x{0a}", "Authentication failed", "yay, api backend\x{0a}"] +--- error_code eval +[200, 403, 200] +--- no_error_log +[error] From eb9467cdffdc4f11cb2a38533e8c167cd2397207 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 5 Sep 2018 11:56:41 +0200 Subject: [PATCH 2/4] proxy: take into account extra authrep params when building the cache key --- gateway/src/apicast/proxy.lua | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gateway/src/apicast/proxy.lua b/gateway/src/apicast/proxy.lua index 4cda4e56e..f2d6ff981 100644 --- a/gateway/src/apicast/proxy.lua +++ b/gateway/src/apicast/proxy.lua @@ -132,6 +132,12 @@ function _M:authorize(service, usage, credentials, ttl) -- NYI: return to lower frame local cached_key = ngx.var.cached_key .. ":" .. encoded_usage + + local encoded_extra_params = encode_args(self.extra_params_backend_authrep) + if encoded_extra_params ~= '' then + cached_key = cached_key .. ":" .. encoded_extra_params + end + local cache = self.cache local is_known = cache:get(cached_key) From 3cf00f8c108ba8c4f33a80725b06c71272c40cab Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 5 Sep 2018 14:48:22 +0200 Subject: [PATCH 3/4] spec/proxy: test that referrer filters are taken into account in the auths cache --- spec/proxy_spec.lua | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/spec/proxy_spec.lua b/spec/proxy_spec.lua index b3c888a5b..2fefc5c60 100644 --- a/spec/proxy_spec.lua +++ b/spec/proxy_spec.lua @@ -91,6 +91,40 @@ describe('Proxy', function() assert.spy(proxy.cache_handler).was.called_with( proxy.cache, 'client_id=blah:usage%5Bfoo%5D=0', response, nil) end) + + it('does not use cached auth if creds are the same but extra authrep params are not', function() + proxy.extra_params_backend_authrep = { referrer = '3scale.net' } + + stub(test_backend, 'send', function() return { status = 200 } end) + + local usage = Usage.new() + usage:add('hits', 1) + local cache_key = "uk:usage%5Bhits%5D=1" -- Referrer not here + proxy.cache:set(cache_key, 200) + ngx.var = { cached_key = "uk" } -- authorize() expects creds to be set up + + proxy:authorize(service, usage, { user_key = 'uk' }) + + -- Calls backend because the call is not cached + assert.stub(test_backend.send).was_called() + end) + + it('uses cached auth if creds are the same and authrep params too', function() + proxy.extra_params_backend_authrep = { referrer = '3scale.net' } + + stub(test_backend, 'send', function() return { status = 200 } end) + + local usage = Usage.new() + usage:add('hits', 1) + local cache_key = "uk:usage%5Bhits%5D=1:referrer=3scale.net" -- Referrer here + proxy.cache:set(cache_key, 200) + ngx.var = { cached_key = "uk" } -- authorize() expects creds to be set up + + proxy:authorize(service, usage, { user_key = 'uk' }) + + -- Does not call backend because the call is cached + assert.stub(test_backend.send).was_not_called() + end) end) describe('.handle_backend_response', function() From d7aacb9aaecd4666b3ae446ee4c0adb4d340ffb3 Mon Sep 17 00:00:00 2001 From: David Ortiz Date: Wed, 5 Sep 2018 14:48:47 +0200 Subject: [PATCH 4/4] CHANGELOG: add fix for referrer filters not being taken into account in the cache --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfc52cc2a..5a3d175f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Set default errlog level when `APICAST_LOG_LEVEL` is empty [PR #868](https://github.com/3scale/apicast/pull/868) - Correct JWT validation according to [RFC 7523 Section 3](https://tools.ietf.org/html/rfc7523#section-3). Like not required `nbf` claim. [THREESCALE-583](https://issues.jboss.org/browse/THREESCALE-583) - Mismatch in OIDC issuer when loading configuration through a configuration file [PR #872](https://github.com/3scale/apicast/pull/872) +- When the 3scale referrer filters was enabled, cached requests were not handled correctly [PR #875](https://github.com/3scale/apicast/issues/875) ## [3.3.0-beta2] - 2018-09-03