From 8b6fd469b4d5ca7a22a66c11fde05a0cbe77e24a Mon Sep 17 00:00:00 2001
From: Michal Cichra <michal@cichra.cz>
Date: Wed, 30 Aug 2017 10:17:25 +0200
Subject: [PATCH] [proxy] handle partial credentials

- part of the credentials can be missing
- return proper error message
---
 CHANGELOG.md                          |  4 ++++
 apicast/src/configuration/service.lua | 14 +++++++++---
 apicast/src/proxy.lua                 | 33 +++++++++++++++------------
 spec/proxy_spec.lua                   | 15 ++++++++++++
 t/003-apicast.t                       | 26 +++++++++++++++++++++
 5 files changed, 75 insertions(+), 17 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ae774c803..2aac2b8c9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Crash on empty OIDC Issuer endpoint [PR #408](https://github.com/3scale/apicast/pull/408)
 
+### Fixes
+
+- Handle partial credentials [PR #409](https://github.com/3scale/apicast/pull/409)
+
 ### Changed
 
 - `THREESCALE\_DEPLOYMENT\_ENV` defaults to `production` [PR #406](https://github.com/3scale/apicast/pull/406)
diff --git a/apicast/src/configuration/service.lua b/apicast/src/configuration/service.lua
index f730400f0..252b79790 100644
--- a/apicast/src/configuration/service.lua
+++ b/apicast/src/configuration/service.lua
@@ -50,6 +50,14 @@ local function read_http_header(name)
   return ngx.var['http_' .. normalized]
 end
 
+local function tuple_mt(size)
+  return { __len = function() return size end }
+end
+
+local credentials_v2_mt = tuple_mt(2)
+local credentials_v1_mt = tuple_mt(1)
+local credentials_oauth_mt = tuple_mt(1)
+
 local backend_version_credentials = { }
 
 function backend_version_credentials.version_1(config)
@@ -74,7 +82,7 @@ function backend_version_credentials.version_1(config)
   -- @field 1 User Key
   -- @field user_key User Key
   -- @table credentials_v1
-  return { user_key, user_key = user_key }
+  return setmetatable({ user_key, user_key = user_key }, credentials_v1_mt)
 end
 
 function backend_version_credentials.version_2(config)
@@ -113,7 +121,7 @@ function backend_version_credentials.version_2(config)
   -- @field app_id App ID
   -- @field app_key App Key
   -- @table credentials_v2
-  return { app_id, app_key, app_id = app_id, app_key = app_key }
+  return setmetatable({ app_id, app_key, app_id = app_id, app_key = app_key }, credentials_v2_mt)
 end
 
 function backend_version_credentials.version_oauth(config)
@@ -143,7 +151,7 @@ function backend_version_credentials.version_oauth(config)
   -- @field 1 Access Token
   -- @field access_token Access Token
   -- @table credentials_oauth
-  return { access_token, access_token = access_token }
+  return setmetatable({ access_token, access_token = access_token }, credentials_oauth_mt)
 end
 
 -- This table can be used with `table.concat` to serialize
diff --git a/apicast/src/proxy.lua b/apicast/src/proxy.lua
index 4588585cf..e6fdf3bf9 100644
--- a/apicast/src/proxy.lua
+++ b/apicast/src/proxy.lua
@@ -19,7 +19,6 @@ local assert = assert
 local type = type
 local next = next
 local insert = table.insert
-
 local concat = table.concat
 local gsub = string.gsub
 local tonumber = tonumber
@@ -64,7 +63,7 @@ local function error_no_credentials(service)
   ngx.status = service.auth_missing_status
   ngx.header.content_type = service.auth_missing_headers
   ngx.print(service.error_auth_missing)
-  ngx.exit(ngx.HTTP_OK)
+  return ngx.exit(ngx.HTTP_OK)
 end
 
 local function error_authorization_failed(service)
@@ -73,7 +72,7 @@ local function error_authorization_failed(service)
   ngx.status = service.auth_failed_status
   ngx.header.content_type = service.auth_failed_headers
   ngx.print(service.error_auth_failed)
-  ngx.exit(ngx.HTTP_OK)
+  return ngx.exit(ngx.HTTP_OK)
 end
 
 local function error_no_match(service)
@@ -83,14 +82,14 @@ local function error_no_match(service)
   ngx.status = service.no_match_status
   ngx.header.content_type = service.no_match_headers
   ngx.print(service.error_no_match)
-  ngx.exit(ngx.HTTP_OK)
+  return ngx.exit(ngx.HTTP_OK)
 end
 
 local function error_service_not_found(host)
   ngx.status = 404
   ngx.print('')
   ngx.log(ngx.WARN, 'could not find service for host: ', host)
-  ngx.exit(ngx.status)
+  return ngx.exit(ngx.status)
 end
 -- End Error Codes
 
@@ -320,32 +319,38 @@ function _M:access(service)
 
   local credentials, err = service:extract_credentials()
 
-  if not credentials or #credentials == 0 then
-    if err then
-      ngx.log(ngx.WARN, "cannot get credentials: ", err)
-    end
+  if not credentials then
+    ngx.log(ngx.WARN, "cannot get credentials: ", err or 'unknown error')
     return error_no_credentials(service)
   end
 
-  insert(credentials, 1, service.id)
-
   local _, matched_patterns, usage_params = service:extract_usage(request)
-
-  ngx.var.cached_key = concat(credentials, ':')
+  local cached_key = { service.id }
 
   -- remove integer keys for serialization
   -- as ngx.encode_args can't serialize integer keys
+  -- and verify all the keys exist
   for i=1,#credentials do
-    credentials[i] = nil
+    local val = credentials[i]
+    if not val then
+      return error_no_credentials(service)
+    else
+      credentials[i] = nil
+    end
+
+    insert(cached_key, val)
   end
 
   local ctx = ngx.ctx
+  local var = ngx.var
 
   -- save those tables in context so they can be used in the backend client
   ctx.usage = usage_params
   ctx.credentials = credentials
   ctx.matched_patterns = matched_patterns
 
+  var.cached_key = concat(cached_key, ':')
+
   local ttl
 
   if self.oauth then
diff --git a/spec/proxy_spec.lua b/spec/proxy_spec.lua
index 48a56c110..79e7f674c 100644
--- a/spec/proxy_spec.lua
+++ b/spec/proxy_spec.lua
@@ -46,6 +46,21 @@ describe('Proxy', function()
     end)
   end)
 
+  describe(':access', function()
+    local service
+    before_each(function()
+      ngx.var = { backend_endpoint = 'http://localhost:1853' }
+      service = Service.new({ extract_usage = function() end })
+    end)
+
+    it('works with part of the credentials', function()
+      service.credentials = { location = 'headers' }
+      service.backend_version = 2
+      ngx.var.http_app_key = 'key'
+      assert.falsy(proxy:access(service))
+    end)
+  end)
+
   it('has post_action function', function()
     assert.truthy(proxy.post_action)
     assert.same('function', type(proxy.post_action))
diff --git a/t/003-apicast.t b/t/003-apicast.t
index 1d27d6090..567379650 100644
--- a/t/003-apicast.t
+++ b/t/003-apicast.t
@@ -44,6 +44,32 @@ GET /
 credentials missing!
 --- error_code: 401
 
+=== TEST 2: authentication (part of) credentials missing
+The message is configurable as well as the status.
+--- http_config
+  lua_package_path "$TEST_NGINX_LUA_PATH";
+  init_by_lua_block {
+    require('configuration_loader').mock({
+      services = {
+        {
+          backend_version = 2,
+          proxy = {
+            error_auth_missing = 'credentials missing!',
+            error_status_auth_missing = 401
+          }
+        }
+      }
+    })
+  }
+--- config
+include $TEST_NGINX_BACKEND_CONFIG;
+include $TEST_NGINX_APICAST_CONFIG;
+--- request
+GET /?app_key=42
+--- response_body chomp
+credentials missing!
+--- error_code: 401
+
 === TEST 2: no mapping rules matched
 The message is configurable and status also.
 --- http_config