From 66d7665ae0687e22f091f5fecd84766f904748a1 Mon Sep 17 00:00:00 2001
From: Eloy Coto <eloy.coto@acalustra.com>
Date: Thu, 6 Aug 2020 15:53:50 +0200
Subject: [PATCH] Policy:3scale_batcher: Fix issues when caching policy is
 active.

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>
---
 .../policy/3scale_batcher/3scale_batcher.lua  | 12 +++
 t/apicast-policy-3scale-batcher.t             | 76 +++++++++++++++++++
 2 files changed, 88 insertions(+)

diff --git a/gateway/src/apicast/policy/3scale_batcher/3scale_batcher.lua b/gateway/src/apicast/policy/3scale_batcher/3scale_batcher.lua
index d60454537..863d9b45b 100644
--- a/gateway/src/apicast/policy/3scale_batcher/3scale_batcher.lua
+++ b/gateway/src/apicast/policy/3scale_batcher/3scale_batcher.lua
@@ -143,6 +143,14 @@ local function handle_backend_denied(self, service, transaction, status, headers
   return error(service, rejection_reason)
 end
 
+local function handle_backend_response_using_cache_handler(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
+
 local function handle_backend_error(self, service, transaction, cache_handler)
   local key = keys_helper.key_for_cached_auth(transaction)
   local cached = cache_handler and self.backend_downtime_cache:get(key)
@@ -206,6 +214,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
@@ -214,6 +223,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
+    handle_backend_response_using_cache_handler(self.backend_downtime_cache, transaction, backend_status, cache_handler)
 
     if backend_status == 200 then
       handle_backend_ok(self, transaction, cache_handler)
diff --git a/t/apicast-policy-3scale-batcher.t b/t/apicast-policy-3scale-batcher.t
index cebe8c555..ff07066f2 100644
--- a/t/apicast-policy-3scale-batcher.t
+++ b/t/apicast-policy-3scale-batcher.t
@@ -615,3 +615,79 @@ my $jwt = encode_jwt(payload => {
 ["Authorization: Bearer $jwt", "Authorization: Bearer $jwt", "" , ""]
 --- no_error_log
 [error]
+
+
+=== TEST 6: 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.
+down.
+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]