[proxy] split proxy:call() into rewrite and access phase

CHANGELOG.md

@@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - `post_action` phase now shares `ngx.ctx` with the main request [PR #539](https://github.com/3scale/apicast/pull/539)
 - Decrease nginx timer resolution to improve performance and enable PCRE JIT [PR #543](https://github.com/3scale/apicast/pull/543)
 - Moved `proxy_pass` into new internal location `@upstream` [PR #535](https://github.com/3scale/apicast/pull/535)
+- Split 3scale authorization to rewrite and access phase [PR #556](https://github.com/3scale/apicast/pull/556)
 
 ## [3.2.0-alpha2] - 2017-11-30
 

gateway/src/apicast/policy/apicast/policy.lua

@@ -50,7 +50,17 @@ function _M:rewrite(context)
     p.cache_handler = context.cache_handler
   end
 
-  p.set_upstream(context.service)
+  local service = context.service
+
+  if service then
+    ngx.ctx.service = service
+
+    -- it is possible that proxy:rewrite will terminate the request
+    p:rewrite(service)
+  end
+
+  p.set_upstream(service)
+
   ngx.ctx.proxy = p
 end
 
@@ -66,20 +76,12 @@ function _M:post_action(context)
 end
 
 function _M:access(context)
-  local p = context and context.proxy or ngx.ctx.proxy or self.proxy
-  ngx.ctx.service = context.service
-
-  local access, handler = p:call(context.service) -- proxy:access() or oauth handler
-
-  local ok, err
+  local ctx = ngx.ctx
+  local p = context and context.proxy or ctx.proxy or self.proxy
 
-  if access then
-    ok, err = access()
-  elseif handler then
-    ok, err = handler()
+  if p then
+    return p:access(context.service, context.usage, context.credentials, context.ttl)
   end
-
-  return ok, err
 end
 
 _M.content = function()

gateway/src/apicast/proxy.lua

@@ -126,6 +126,8 @@ local function output_debug_headers(service, usage, credentials)
 end
 
 function _M:authorize(service, usage, credentials, ttl)
+  if not usage or not credentials then return nil, 'missing usage or credentials' end
+
   local encoded_usage = encode_args(usage)
   if encoded_usage == '' then
     return error_no_match(service)
@@ -206,36 +208,31 @@ function _M.set_upstream(service)
   ngx.req.set_header('Host', upstream.host or ngx.var.host)
 end
 
------
--- call the proxy and return a handler function
--- that will perform an action based on the path and backend version
--- @tparam service service service object
--- @treturn nil|function access function (when the request needs to be authenticated with this)
--- @treturn nil|function handler function (when the request is not authenticated and has some own action)
-function _M:call(service)
-  service = _M.set_service(service or ngx.ctx.service)
-
-  self.oauth = service:oauth()
+local function handle_oauth(service)
+  local oauth = service:oauth()
 
-  ngx.log(ngx.DEBUG, 'using OAuth: ', self.oauth)
+  if oauth then
+    ngx.log(ngx.DEBUG, 'using OAuth: ', oauth)
+  end
 
-  -- means that OAuth integration has own router
-  if self.oauth and self.oauth.call then
-    local f, params = self.oauth:call(service)
+  if oauth and oauth.call then
+    local f, params = oauth:call(service)
 
     if f then
-      ngx.log(ngx.DEBUG, 'apicast oauth flow')
-      return nil, function() return f(params) end
+      ngx.log(ngx.DEBUG, 'OAuth matched route')
+      return f(params) -- not really about the return value but showing something will call ngx.exit
     end
   end
 
-  return function()
-    -- call access phase
-    return self:access(service)
-  end
+  return oauth
 end
 
-function _M:access(service)
+function _M:rewrite(service)
+  service = _M.set_service(service or ngx.ctx.service)
+
+  -- handle_oauth can terminate the request
+  self.oauth = handle_oauth(service)
+
   ngx.var.secret_token = service.secret_token
 
   local credentials, err = service:extract_credentials()
@@ -285,9 +282,14 @@ function _M:access(service)
       return error_authorization_failed(service)
     end
     ctx.credentials = credentials
+    ctx.ttl = ttl
   end
+end
+
+function _M:access(service, usage, credentials, ttl)
+  local ctx = ngx.ctx
 
-  return self:authorize(service, usage_params, credentials, ttl)
+  return self:authorize(service, usage or ctx.usage, credentials or ctx.credentials, ttl or ctx.ttl)
 end
 
 local function response_codes_data()

spec/policy/apicast/policy_spec.lua

@@ -9,49 +9,4 @@ describe('APIcast module', function()
   it('has a version', function()
     assert.truthy(_M._VERSION)
   end)
-
-  describe(':access', function()
-
-    local apicast
-
-    before_each(function()
-      apicast = _M.new()
-      ngx.ctx.proxy = {}
-    end)
-
-    it('triggers post action when access phase succeeds', function()
-      ngx.var = { original_request_id = 'foobar' }
-
-      stub(ngx.ctx.proxy, 'call', function()
-        return function() return 'ok' end
-      end)
-
-      stub(ngx.ctx.proxy, 'post_action', function()
-        return 'post_ok'
-      end)
-
-      local ok, err = apicast:access({})
-
-      assert.same('ok', ok)
-      assert.falsy(err)
-
-      assert.same('post_ok', apicast:post_action())
-    end)
-
-    it('skips post action when access phase not executed', function()
-      stub(ngx.ctx.proxy, 'call', function()
-        -- return nothing for example
-      end)
-
-      local ok, err = apicast:access({})
-
-      assert.falsy(ok)
-      assert.falsy(err)
-
-      ngx.ctx.proxy = nil -- in post_action the ctx is not shared
-      ngx.var = { original_request_id = 'foobar' }
-
-      assert.equal(nil, apicast:post_action())
-    end)
-  end)
 end)

spec/proxy_spec.lua

@@ -17,39 +17,7 @@ describe('Proxy', function()
     assert.same('function', type(proxy.access))
   end)
 
-  describe(':call', function()
-    local service
-    before_each(function()
-      ngx.var = { backend_endpoint = 'http://localhost:1853' }
-      service = Service.new({ id = 42, hosts = { 'localhost' }})
-    end)
-
-    it('has authorize function after call', function()
-      proxy:call(service)
-
-      assert.truthy(proxy.authorize)
-      assert.same('function', type(proxy.authorize))
-    end)
-
-    it('returns access function', function()
-      local access = proxy:call(service)
-
-      assert.same('function', type(access))
-    end)
-
-    it('returns oauth handler when matches oauth route', function()
-      service.backend_version = 'oauth'
-      stub(ngx.req, 'get_method', function() return 'GET' end)
-      ngx.var.uri = '/authorize'
-
-      local access, handler = proxy:call(service)
-
-      assert.equal(nil, access)
-      assert.same('function', type(handler))
-    end)
-  end)
-
-  describe(':access', function()
+  describe(':rewrite', function()
     local service
     before_each(function()
       ngx.var = { backend_endpoint = 'http://localhost:1853', uri = '/a/uri' }
@@ -61,7 +29,7 @@ describe('Proxy', function()
       service.credentials = { location = 'headers' }
       service.backend_version = 2
       ngx.var.http_app_key = 'key'
-      assert.falsy(proxy:access(service))
+      assert.falsy(proxy:rewrite(service))
     end)
   end)