3scale_batcher: enforce mapping rule check.

If the 3scale batcher policy is defined, the proxy:authz is not called, so mapping rule is not checked at all, and request got forwarded to the upstream API. Fixes #1207 Fixes THREESCALE-5513 Signed-off-by: Eloy Coto <[email protected]>
3scale · Jul 17, 2020 · c357a19 · c357a19
1 parent c604448
commit c357a19
Show file tree

Hide file tree

Showing 4 changed files with 110 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Fixed issue with IPCheck policy when forwarder-for value contains port [THREESCALE-5258](https://issues.redhat.com/browse/THREESCALE-5258) [PR #1192](https://github.com/3scale/APIcast/pull/1192)
 - Fixed issues with URL encode on routing policy [THREESCALE-5454](https://issues.redhat.com/browse/THREESCALE-5454) [PR #1208](https://github.com/3scale/APIcast/pull/1208)
 
+- Fixed issue with mapping rules and 3scale batcher policy [THREESCALE-5513](https://issues.redhat.com/browse/THREESCALE-5513) [PR #1210](https://github.com/3scale/APIcast/pull/1210)
 
 ### Added
 

diff --git a/gateway/src/apicast/policy/3scale_batcher/3scale_batcher.lua b/gateway/src/apicast/policy/3scale_batcher/3scale_batcher.lua
@@ -180,17 +180,24 @@ end
 -- calls to backend.
 function _M:access(context)
   local backend = backend_client:new(context.service, http_ng_resty)
-  local usage = context.usage
+  local usage = context.usage or {}
+  local service = context.service
+  local service_id = service.id
+  local credentials = context.credentials
+
+ -- Checking that at least one mapping rule match, if not raise no mapping rule
+ -- match error
+  local encoded_usage = usage:encoded_format()
+  if encoded_usage == '' then
+    return errors.no_match(service)
+  end
 
   -- If routing policy changes the upstream and it only belongs to a specified
   -- owner, we need to filter out the usage for APIs that are not used at all.
   if context.route_upstream_usage_cleanup then
     context:route_upstream_usage_cleanup(usage, ngx.ctx.matched_rules)
   end
 
-  local service = context.service
-  local service_id = service.id
-  local credentials = context.credentials
   local transaction = Transaction.new(service_id, credentials, usage)
 
   ensure_timer_task_created(self, service_id, backend)

diff --git a/spec/policy/3scale_batcher/3scale_batcher_spec.lua b/spec/policy/3scale_batcher/3scale_batcher_spec.lua
@@ -83,6 +83,14 @@ describe('3scale batcher policy', function()
       stub(context, 'cache_handler')
     end)
 
+    describe('when mapping rule does not match', function()
+      it('returns 404 back', function()
+        context.usage = Usage.new()
+        batcher_policy:access(context)
+        assert.equals(ngx.status, 404)
+      end)
+    end)
+
     describe('when the request is cached', function()
       it('updates the auths cache counter sending a hit', function()
         batcher_policy.auths_cache:set(transaction, 200)

diff --git a/t/apicast-policy-3scale-batcher-blackbox.t b/t/apicast-policy-3scale-batcher-blackbox.t
@@ -177,3 +177,93 @@ HTTP/1.1 200 OK
 --- error_code: 200
 --- no_error_log
 [error]
+
+
+=== TEST 2: Batching policy returns no mapping rule found correctly
+--- backend
+  location /transactions/authrep.xml {
+    content_by_lua_block {
+      -- request should not be here, is using batcher.
+      ngx.exit(503)
+    }
+  }
+
+  location /transactions/authorize.xml {
+    content_by_lua_block {
+      ngx.say("ok")
+    }
+  }
+
+  location /transactions.xml {
+    content_by_lua_block {
+      ngx.say("ok")
+    }
+  }
+--- upstream
+  location ~* /second/foo/bar {
+    content_by_lua_block {
+      ngx.say('yay, api backend');
+    }
+  }
+--- configuration
+{
+  "services": [
+    {
+      "id": 42,
+      "backend_version": 1,
+      "backend_authentication_type": "service_token",
+      "backend_authentication_value": "token-value",
+      "proxy": {
+        "policy_chain": [
+          {
+            "name": "apicast.policy.routing",
+            "configuration": {
+              "rules": [
+                {
+                  "url": "http://test:$TEST_NGINX_SERVER_PORT/second/",
+                  "owner_id": 4,
+                  "condition": {
+                    "operations": [
+                      {
+                        "match": "path",
+                        "op": "matches",
+                        "value": "/foo/bar"
+                      }
+                    ]
+                  }
+                }
+              ]
+            }
+          },
+          {
+            "name": "apicast.policy.3scale_batcher",
+            "configuration": {
+              "batch_report_seconds" : 1
+            }
+          },
+          {
+            "name": "apicast.policy.apicast"
+          }
+        ],
+        "proxy_rules": [
+          {
+            "pattern": "/foo/bar",
+            "http_method": "GET",
+            "metric_system_name": "hits",
+            "delta": 1,
+            "owner_id": 4,
+            "owner_type": "BackendApi"
+          }
+        ]
+      }
+    }
+  ]
+}
+--- request eval
+["GET /?user_key=value", "GET /foo/bar?user_key=value"]
+--- response_body chomp eval
+["No Mapping Rule matched", "yay, api backend\n"]
+--- error_code eval
+[404, 200]
+--- no_error_log
+[error]