Backend authorization cache handlers

.env

@@ -22,6 +22,9 @@ REDIS_HOST=redis
 # Override backend endpoint in the config globally.
 # BACKEND_ENDPOINT_OVERRIDE=https://backend.example.com
 
+# Set how to handle cached backend responses.
+# APICAST_BACKEND_CACHE_HANDLER=resilient
+
 # Set number of worker processes.
 # APICAST_WORKERS=2
 

CHANGELOG.md

@@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Added
+
+- Ability to configure how to cache backend authorizations [PR #396](https://github.com/3scale/apicast/pull/396)
+
 ## [3.1.0-beta1] - 2017-07-21
 
 ### Fixed

apicast/src/backend/cache_handler.lua

@@ -0,0 +1,55 @@
+local setmetatable = setmetatable
+
+local _M = {
+  handlers = setmetatable({}, { __index = { default = 'strict' } })
+}
+
+local mt = {
+  __index = _M,
+  __call = function(t, ...)
+    local handler = t.handlers[t.handler]
+
+    if handler then
+      return handler(...)
+    else
+      return nil, 'missing handler'
+    end
+  end,
+}
+
+function _M.new(handler)
+  local name = handler or _M.handlers.default
+  ngx.log(ngx.DEBUG, 'backend cache handler: ', name)
+  return setmetatable({ handler = name }, mt)
+end
+
+function _M.handlers.strict(cache, cached_key, response, ttl)
+  if response.status == 200 then
+    -- cached_key is set in post_action and it is in in authorize
+    -- so to not write the cache twice lets write it just in authorize
+    if ngx.var.cached_key ~= cached_key then
+      ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key, ', ttl: ', ttl )
+      cache:set(cached_key, 200, ttl or 0)
+    end
+
+    return true
+  else
+    ngx.log(ngx.NOTICE, 'apicast cache delete key: ', cached_key, ' cause status ', response.status)
+    cache:delete(cached_key)
+    return false, 'not authorized'
+  end
+end
+
+function _M.handlers.resilient(cache, cached_key, response, ttl)
+  local status = response.status
+
+  if status and status < 500 then
+    ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key, ' status: ', status, ', ttl: ', ttl )
+
+    cache:set(cached_key, status, ttl or 0)
+
+    return status == 200
+  end
+end
+
+return _M

apicast/src/proxy.lua

@@ -9,6 +9,9 @@
 local env = require 'resty.env'
 local custom_config = env.get('APICAST_CUSTOM_CONFIG')
 local configuration_store = require 'configuration_store'
+local resty_lrucache = require('resty.lrucache')
+
+local backend_cache_handler = require('backend.cache_handler')
 
 local resty_url = require 'resty.url'
 
@@ -34,9 +37,23 @@ local mt = {
   __index = _M
 }
 
+function _M.shared_cache()
+  return ngx.shared.api_keys or resty_lrucache.new(1)
+end
+
 function _M.new(configuration)
+  local cache = _M.shared_cache() or error('missing cache store')
+
+  if not cache then
+    ngx.log(ngx.WARN, 'apicast cache error missing shared memory zone api_keys')
+  end
+
+  local cache_handler = backend_cache_handler.new(env.get('APICAST_BACKEND_CACHE_HANDLER'))
+
   return setmetatable({
-    configuration = assert(configuration, 'missing proxy configuration')
+    configuration = assert(configuration, 'missing proxy configuration'),
+    cache = cache,
+    cache_handler = cache_handler,
   }, mt)
 end
 
@@ -183,29 +200,23 @@ function _M:authorize(service, usage, credentials, ttl)
   ngx.var.credentials = credentials
   -- NYI: return to lower frame
   local cached_key = ngx.var.cached_key .. ":" .. usage
-  local api_keys = ngx.shared.api_keys
-  local is_known = api_keys and api_keys:get(cached_key)
+  local cache = self.cache
+  local is_known = cache:get(cached_key)
 
   if is_known == 200 then
     ngx.log(ngx.DEBUG, 'apicast cache hit key: ', cached_key)
     ngx.var.cached_key = cached_key
   else
-    ngx.log(ngx.INFO, 'apicast cache miss key: ', cached_key)
-    local res = http.get(internal_location)
+    ngx.log(ngx.INFO, 'apicast cache miss key: ', cached_key, ' value: ', is_known)
 
-    ngx.log(ngx.DEBUG, '[backend] response status: ', res.status, ' body: ', res.body)
+    -- set cached_key to nil to avoid doing the authrep in post_action
+    ngx.var.cached_key = nil
 
-    if res.status == 200 then
-      if api_keys then
-        ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key, ', ttl: ', ttl )
-        api_keys:set(cached_key, 200, ttl or 0)
-      end
-    else -- TODO: proper error handling
-      if api_keys then api_keys:delete(cached_key) end
+    local res = http.get(internal_location)
+
+    if not self:handle_backend_response(cached_key, res, ttl) then
       error_authorization_failed(service)
     end
-    -- set cached_key to nil to avoid doing the authrep in post_action
-    ngx.var.cached_key = nil
   end
 end
 
@@ -381,23 +392,20 @@ function _M:post_action()
     local auth_uri = service.backend_version == 'oauth' and 'threescale_oauth_authrep' or 'threescale_authrep'
     local res = http.get("/".. auth_uri .."?log=" .. response_codes_encoded_data())
 
-    if res.status ~= 200 then
-      local api_keys = ngx.shared.api_keys
-
-      if api_keys then
-        ngx.log(ngx.NOTICE, 'apicast cache delete key: ', cached_key, ' cause status ', res.status)
-        api_keys:delete(cached_key)
-      else
-        ngx.log(ngx.ALERT, 'apicast cache error missing shared memory zone api_keys')
-      end
-    end
+    self:handle_backend_response(cached_key, res)
   else
     ngx.log(ngx.INFO, '[async] skipping after action, no cached key')
   end
 
   exit(ngx.HTTP_OK)
 end
 
+function _M:handle_backend_response(cached_key, response, ttl)
+  ngx.log(ngx.DEBUG, '[backend] response status: ', response.status, ' body: ', response.body)
+
+  return self.cache_handler(self.cache, cached_key, response, ttl)
+end
+
 if custom_config then
   local path = package.path
   local module = gsub(custom_config, '%.lua$', '') -- strip .lua from end of the file

doc/parameters.md

@@ -32,6 +32,15 @@ Defines how to load the configuration.
 Boot will require configuration when the gateway starts.
 Lazy will load it on demand on incoming request.
 
+### `APICAST_BACKEND_CACHE_HANDLER`
+
+**Values:** strict | resilient
+**Default:** strict
+
+Defines how the authorization cache behaves when backend is unavailable.
+Strict will remove cached application when backend is unavailable.
+Resilient will do so only on getting authorization denied from backend.
+
 ### `APICAST_MODULE`
 
 **Default:** "apicast"

openshift/apicast-template.yml

@@ -40,6 +40,8 @@ objects:
             value: "${SERVICES_LIST}"
           - name: APICAST_CONFIGURATION_LOADER
             value: "${CONFIGURATION_LOADER}"
+          - name: APICAST_BACKEND_CACHE_HANDLER
+            value: "${BACKEND_CACHE_HANDLER}"
           - name: APICAST_LOG_LEVEL
             value: "${LOG_LEVEL}"
           - name: APICAST_PATH_ROUTING_ENABLED
@@ -128,6 +130,10 @@ parameters:
   description: "When to load configuration. If on gateway start or incoming request. Allowed values are: lazy, boot."
   value: boot
   required: false
+- name: BACKEND_CACHE_HANDLER
+  description: "How to handle and cache backend authorization responses. Can be one of: strict, resilient."
+  value: strict
+  required: false
 - description: "Log level. One of the following: debug, info, notice, warn, error, crit, alert, or emerg."
   name: LOG_LEVEL
   required: false

spec/backend/cache_handler_spec.lua

@@ -0,0 +1,110 @@
+
+local _M = require('backend.cache_handler')
+local lrucache = require('resty.lrucache')
+
+
+describe('Cache Handler', function()
+
+  describe('.new', function()
+    it('has a default handler', function()
+      local handler = _M.new()
+      assert.equal(assert(_M.handlers.default), handler.handler)
+    end)
+
+    it('sets a handler', function()
+      local handler = _M.new('strict')
+
+      assert.equal('strict', handler.handler)
+    end)
+  end)
+
+  describe('call', function()
+    it('calls handler on runtime', function()
+      local handler = _M.new('test-fake')
+
+      stub(_M.handlers, 'test-fake', function() return 'return value' end)
+
+      assert.equal('return value', handler())
+    end)
+  end)
+
+  describe('strict', function()
+    local handler = _M.handlers.strict
+
+    it('caches successful response', function()
+      local cache = lrucache.new(1)
+      ngx.var = { cached_key = nil }
+
+      assert.truthy(handler(cache, 'foobar', { status = 200 }))
+
+      assert.equal(200, cache:get('foobar'))
+    end)
+
+    it('not caches in post action', function()
+      local cache = lrucache.new(1)
+      ngx.var = { cached_key = 'foobar' } -- means it is performed in post_action
+
+      assert.truthy(handler(cache, 'foobar', { status = 200 }))
+
+      assert.falsy(cache:get('foobar'))
+    end)
+
+    it('deletes cache on forbidden response', function()
+      local cache = lrucache.new(1)
+      cache:set('foobar', 200)
+
+      assert.falsy(handler(cache, 'foobar', { status = 403 }))
+
+      assert.falsy(cache:get('foobar'))
+    end)
+
+    it('deletes cache on server errors', function()
+      local cache = lrucache.new(1)
+      cache:set('foobar', 200)
+
+      assert.falsy(handler(cache, 'foobar', { status = 503 }))
+
+      assert.falsy(cache:get('foobar'))
+    end)
+  end)
+
+
+  describe('resilient', function()
+    local handler = _M.handlers.resilient
+
+    it('caches successful response', function()
+      local cache = lrucache.new(1)
+
+      assert.truthy(handler(cache, 'foobar', { status = 200 }))
+
+      assert.equal(200, cache:get('foobar'))
+    end)
+
+    it('caches forbidden response', function()
+      local cache = lrucache.new(1)
+
+      assert.falsy(handler(cache, 'foobar', { status = 403 }))
+
+      assert.equal(403, cache:get('foobar'))
+    end)
+
+    it('not caches server errors', function()
+      local cache = lrucache.new(1)
+
+      assert.falsy(handler(cache, 'foobar', { status = 503 }))
+
+      assert.falsy(cache:get('foobar'))
+    end)
+
+    it('not overrides cache on server errors', function()
+      local cache = lrucache.new(1)
+
+      cache:set('foobar', 200)
+
+      assert.falsy(handler(cache, 'foobar', { status = 503 }))
+
+      assert.equal(200, cache:get('foobar'))
+    end)
+  end)
+
+end)

spec/proxy_spec.lua

@@ -84,38 +84,37 @@ describe('Proxy', function()
   end)
 
   describe('.authorize', function()
-    local authorize
     local service = { backend_authentication = { value = 'not_baz' } }
     local usage = 'foo'
     local credentials = 'client_id=blah'
 
-    before_each(function()
-      authorize = proxy.authorize
-    end)
 
     it('takes ttl value if sent', function()
       local ttl = 80
       ngx.var = { cached_key = credentials, usage=usage, credentials=credentials, http_x_3scale_debug='baz', real_url='blah' }
       ngx.ctx = { backend_upstream = ''}
-      ngx.shared = { api_keys = { cached_key = 'client_id=blah:foo', get = function () return {} end } }
 
-      stub(ngx.shared.api_keys, 'set')
-      stub(ngx.location, 'capture', function() return { status = 200 } end)
+      local response = { status = 200 }
+      stub(ngx.location, 'capture', function() return response end)
+
+      stub(proxy, 'cache_handler').returns(true)
 
-      authorize(proxy, service, usage, credentials, ttl)
-      assert.spy(ngx.shared.api_keys.set).was.called_with(ngx.shared.api_keys, 'client_id=blah:foo', 200, 80)
+      proxy:authorize(service, usage, credentials, ttl)
+
+      assert.spy(proxy.cache_handler).was.called_with(proxy.cache, 'client_id=blah:foo', response, ttl)
     end)
 
     it('works with no ttl', function()
       ngx.var = { cached_key = "client_id=blah", usage=usage, credentials=credentials, http_x_3scale_debug='baz', real_url='blah' }
       ngx.ctx = { backend_upstream = ''}
-      ngx.shared = { api_keys = { cached_key = 'client_id=blah:foo', get = function () return {} end } }
 
-      stub(ngx.shared.api_keys, 'set')
-      stub(ngx.location, 'capture', function() return { status = 200 } end)
+      local response = { status = 200 }
+      stub(ngx.location, 'capture', function() return response end)
+      stub(proxy, 'cache_handler').returns(true)
+
+      proxy:authorize(service, usage, credentials)
 
-      authorize(proxy, service, usage, credentials)
-      assert.spy(ngx.shared.api_keys.set).was.called_with(ngx.shared.api_keys, 'client_id=blah:foo', 200, 0)
+      assert.spy(proxy.cache_handler).was.called_with(proxy.cache, 'client_id=blah:foo', response, nil)
     end)
   end)
 end)

t/001-management.t

@@ -7,7 +7,7 @@ my $apicast = $ENV{TEST_NGINX_APICAST_PATH} || "$pwd/apicast";
 $ENV{TEST_NGINX_LUA_PATH} = "$apicast/src/?.lua;;";
 $ENV{TEST_NGINX_MANAGEMENT_CONFIG} = "$apicast/conf.d/management.conf";
 
-require("t/dns.pl");
+require("$pwd/t/dns.pl");
 
 log_level('debug');
 repeat_each(2);

t/003-apicast.t

@@ -9,7 +9,7 @@ $ENV{TEST_NGINX_UPSTREAM_CONFIG} = "$apicast/http.d/upstream.conf";
 $ENV{TEST_NGINX_BACKEND_CONFIG} = "$apicast/conf.d/backend.conf";
 $ENV{TEST_NGINX_APICAST_CONFIG} = "$apicast/conf.d/apicast.conf";
 
-require("t/dns.pl");
+require("$pwd/t/dns.pl");
 
 log_level('debug');
 repeat_each(2);