Skip to content

Commit

Permalink
Policy:3scale_batcher: Fix issues when caching policy is active.
Browse files Browse the repository at this point in the history
A user reported that when the caching mode is enabled, in allow mode, and 3scale-batcher is active, the handler for allowing all calls is no working as expected, and users got 403 response.

This commit allows users to get a correct authz when allow mode is enabled.

Fix THREESCALE-5753

Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>
  • Loading branch information
eloycoto committed Aug 21, 2020
1 parent 3c77126 commit 4511ed3
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Support Proxy Protocol [PR #1211](https://github.com/3scale/APIcast/pull/1211) [THREESCALE-5366](https://issues.redhat.com/browse/THREESCALE-5366)


### Fixed

- Fixed issues with allow caching mode and 3scale batcher [PR #1216](https://github.com/3scale/APIcast/pull/1216) [THREESCALE-5753](https://issues.redhat.com/browse/THREESCALE-5753)

## [3.9.0] 2020-08-17

No issues found on beta1,so becames final release.
Expand Down
7 changes: 7 additions & 0 deletions gateway/src/apicast/policy/3scale_batcher/3scale_batcher.lua
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,9 @@ local function error(service, rejection_reason)
end

local function update_downtime_cache(cache, transaction, backend_status, cache_handler)
if not cache_handler then
return
end
local key = keys_helper.key_for_cached_auth(transaction)
cache_handler(cache, key, { status = backend_status })
end
Expand Down Expand Up @@ -206,6 +209,7 @@ function _M:access(context)
local auth_is_cached = (cached_auth and true) or false
metrics.update_cache_counters(auth_is_cached)


if cached_auth then
handle_cached_auth(self, cached_auth, service, transaction)
else
Expand All @@ -214,6 +218,9 @@ function _M:access(context)
context:publish_backend_auth(backend_res)
local backend_status = backend_res.status
local cache_handler = context.cache_handler -- Set by Caching policy
-- this is needed, because in allow mode, the status maybe is always 200, so
-- Request need to go to the Upstream API
update_downtime_cache(self.backend_downtime_cache, transaction, backend_status, cache_handler)

if backend_status == 200 then
handle_backend_ok(self, transaction, cache_handler)
Expand Down
75 changes: 75 additions & 0 deletions t/apicast-policy-3scale-batcher.t
Original file line number Diff line number Diff line change
Expand Up @@ -615,3 +615,78 @@ my $jwt = encode_jwt(payload => {
["Authorization: Bearer $jwt", "Authorization: Bearer $jwt", "" , ""]
--- no_error_log
[error]
=== TEST 7: with caching policy (allow mode)
The purpose of this test is to test that the 3scale batcher policy works
correctly when combined with the caching one.
In this case, the caching policy is configured as "allow". We define a
backend that returns 500
The caching policy will allow all request to the Upstream API.
To make sure that nothing is cached in the 3scale batcher policy, we flush its
auth cache on every request (see rewrite_by_lua_block).
--- http_config
include $TEST_NGINX_UPSTREAM_CONFIG;
lua_shared_dict api_keys 10m;
lua_shared_dict cached_auths 1m;
lua_shared_dict batched_reports 1m;
lua_shared_dict batched_reports_locks 1m;
lua_package_path "$TEST_NGINX_LUA_PATH";
init_by_lua_block {
require('apicast.configuration_loader').mock({
services = {
{
id = 42,
backend_version = 1,
backend_authentication_type = 'service_token',
backend_authentication_value = 'token-value',
proxy = {
backend = { endpoint = "http://127.0.0.1:$TEST_NGINX_SERVER_PORT" },
api_backend = "http://127.0.0.1:$TEST_NGINX_SERVER_PORT/api-backend/",
proxy_rules = {
{ pattern = '/', http_method = 'GET', metric_system_name = 'hits', delta = 2 }
},
policy_chain = {
{
name = 'apicast.policy.3scale_batcher',
configuration = { }
},
{
name = 'apicast.policy.apicast'
},
{
name = 'apicast.policy.caching',
configuration = { caching_type = 'allow' }
}
}
}
}
}
})
}
rewrite_by_lua_block {
require('resty.ctx').apply()
ngx.shared.cached_auths:flush_all()
}
--- config
include $TEST_NGINX_APICAST_CONFIG;
location /transactions/authorize.xml {
content_by_lua_block {
ngx.exit(500)
}
}
location /api-backend {
echo 'yay, api backend';
}
--- request eval
["GET /test?user_key=foo", "GET /foo?user_key=foo", "GET /?user_key=foo"]
--- response_body eval
["yay, api backend\x{0a}", "yay, api backend\x{0a}", "yay, api backend\x{0a}"]
--- error_code eval
[ 200, 200, 200 ]
--- no_error_log
[error]

0 comments on commit 4511ed3

Please sign in to comment.