OAuth fixes and improvements (#353)
* Fix App ID caching issue (and add tests to match)
* Improve error messages for all OAuth providers
diff --git a/Dockerfile.test.unit b/Dockerfile.test.unit
index 598a038..7038c9f 100644
--- a/Dockerfile.test.unit
+++ b/Dockerfile.test.unit
@@ -24,11 +24,13 @@
FROM alpine:3.9
+ENV CJOSE_VERSION=0.5.1
+
RUN apk update && \
apk add \
gcc tar zlib wget make musl-dev g++ curl \
libtool readline luajit luajit-dev unzip \
- openssl openssl-dev
+ openssl openssl-dev git jansson jansson-dev
WORKDIR /tmp
RUN wget https://luarocks.org/releases/luarocks-3.1.3.tar.gz && \
@@ -38,6 +40,15 @@
make build && \
make install
+RUN echo " ... installing cjose ... " \
+ && mkdir -p /tmp/api-gateway \
+ && curl -L -k https://github.com/cisco/cjose/archive/${CJOSE_VERSION}.tar.gz -o /tmp/api-gateway/cjose-${CJOSE_VERSION}.tar.gz \
+ && tar -xf /tmp/api-gateway/cjose-${CJOSE_VERSION}.tar.gz -C /tmp/api-gateway/ \
+ && cd /tmp/api-gateway/cjose-${CJOSE_VERSION} \
+ && sh configure \
+ && make && make install \
+ && rm -rf /tmp/api-gateway
+
COPY . /etc/api-gateway
WORKDIR /etc/api-gateway/tests
diff --git a/scripts/lua/oauth/app-id.lua b/scripts/lua/oauth/app-id.lua
index 7b5af21..a756f64 100644
--- a/scripts/lua/oauth/app-id.lua
+++ b/scripts/lua/oauth/app-id.lua
@@ -23,26 +23,39 @@
local http = require 'resty.http'
local cjose = require 'resty.cjose'
-function _M.process(dataStore, token, securityObj)
- local result = dataStore:getOAuthToken('appId', token)
- local httpc = http.new()
- local json_resp
- if result ~= ngx.null then
- json_resp = cjson.decode(result)
- ngx.header['X-OIDC-Email'] = json_resp['email']
- ngx.header['X-OIDC-Sub'] = json_resp['sub']
- return json_resp
- end
- local keyUrl = utils.concatStrings({APPID_PKURL, securityObj.tenantId, '/publickeys'})
+local function inject_req_headers(token_obj)
+ ngx.header['X-OIDC-Email'] = token_obj['email']
+ ngx.header['X-OIDC-Sub'] = token_obj['sub']
+end
+
+local function fetchJWKs(tenantId)
+ local keyUrl = utils.concatStrings({APPID_PKURL, tenantId, '/publickeys'})
local request_options = {
headers = {
["Accept"] = "application/json"
},
- ssl_verify = false
+ ssl_verify = true
}
- local res, err = httpc:request_uri(keyUrl, request_options)
- if err then
- request.err(500, 'error getting app id key: ' .. err)
+ return httpc:request_uri(keyUrl, request_options)
+end
+
+function _M.process(dataStore, token, securityObj)
+ local cache_key = 'appid_' .. securityObj.tenantId
+ local result = dataStore:getOAuthToken(cache_key, token)
+ local httpc = http.new()
+ local token_obj
+
+ -- Was the token in the cache?
+ if result ~= ngx.null then
+ token_obj = cjson.decode(result)
+ inject_req_headers(token_obj)
+ return token_obj
+ end
+
+ -- Cache miss. Proceed to validate the token
+ local res, err = fetchJWKs
+ if err or res.status ~= 200 then
+ request.err(500, 'An error occurred while fetching the App ID JWK configuration: ' .. err or res.body)
end
local key
@@ -52,24 +65,26 @@
end
local result = cjose.validateJWS(token, cjson.encode(key))
if not result then
- request.err(401, 'AppId key signature verification failed.')
+ request.err(401, 'The token signature did not match any known JWK.')
return nil
end
- local jwt_obj = cjson.decode(cjose.getJWSInfo(token))
- local expireTime = jwt_obj['exp']
+
+ token_obj = cjson.decode(cjose.getJWSInfo(token))
+ local expireTime = token_obj['exp']
if expireTime < os.time() then
- request.err(401, 'Access token expired.')
+ request.err(401, 'The access token has expired.')
return nil
end
- ngx.header['X-OIDC-Email'] = jwt_obj['email']
- ngx.header['X-OIDC-Sub'] = jwt_obj['sub']
+
+ -- Add token metadata to the request headers
+ inject_req_headers(token_obj)
+
-- keep token in cache until it expires
local ttl = expireTime - os.time()
- dataStore:saveOAuthToken('appId', token, cjson.encode(jwt_obj), ttl)
- return jwt_obj
+ dataStore:saveOAuthToken(cache_key, token, cjson.encode(token_obj), ttl)
+ return token_obj
end
-
return _M
diff --git a/scripts/lua/oauth/facebook.lua b/scripts/lua/oauth/facebook.lua
index f23ddf9..a27b295 100644
--- a/scripts/lua/oauth/facebook.lua
+++ b/scripts/lua/oauth/facebook.lua
@@ -56,7 +56,7 @@
-- convert response
if not res then
ngx.log(ngx.WARN, 'Could not invoke Facebook API. Error=', err)
- request.err(500, 'OAuth provider error.')
+ request.err(500, 'Connection to the OAuth provider failed.')
return
end
local json_resp = cjson.decode(res.body)
diff --git a/scripts/lua/oauth/github.lua b/scripts/lua/oauth/github.lua
index f4f2e9b..51addf1 100644
--- a/scripts/lua/oauth/github.lua
+++ b/scripts/lua/oauth/github.lua
@@ -45,7 +45,7 @@
-- convert response
if not res then
ngx.log(ngx.WARN, utils.concatStrings({"Could not invoke Github API. Error=", err}))
- request.err(500, 'OAuth provider error.')
+ request.err(500, 'Connection to the OAuth provider failed.')
return
end
diff --git a/scripts/lua/oauth/google.lua b/scripts/lua/oauth/google.lua
index b6892b3..bd8e11b 100644
--- a/scripts/lua/oauth/google.lua
+++ b/scripts/lua/oauth/google.lua
@@ -50,7 +50,7 @@
-- convert response
if not res then
ngx.log(ngx.WARN, utils.concatStrings({"Could not invoke Google API. Error=", err}))
- request.err(500, 'OAuth provider error.')
+ request.err(500, 'Connection to the OAuth provider failed.')
return nil
end
local json_resp = cjson.decode(res.body)
diff --git a/scripts/lua/policies/security/oauth2.lua b/scripts/lua/policies/security/oauth2.lua
index 21a8e81..6b58bf9 100644
--- a/scripts/lua/policies/security/oauth2.lua
+++ b/scripts/lua/policies/security/oauth2.lua
@@ -66,7 +66,7 @@
local loaded, impl = pcall(require, utils.concatStrings({'oauth/', provider}))
if not loaded then
request.err(500, 'Error loading OAuth provider authentication module')
- print("error loading provider.")
+ print("error loading provider:", impl)
return nil
end
diff --git a/tests/install-deps.sh b/tests/install-deps.sh
index 74f124c..156cb3f 100755
--- a/tests/install-deps.sh
+++ b/tests/install-deps.sh
@@ -28,3 +28,5 @@
luarocks install --tree=lua_modules md5
luarocks install --tree=lua_modules net-url
luarocks install --tree=lua_modules luafilesystem
+luarocks install --tree=lua_modules lua-resty-http 0.10
+luarocks install --tree=lua_modules https://github.com/mhamann/lua-resty-cjose/raw/master/lua-resty-cjose-0.5-0.rockspec
diff --git a/tests/scripts/lua/security.lua b/tests/scripts/lua/security.lua
index 89533fd..e449ff9 100644
--- a/tests/scripts/lua/security.lua
+++ b/tests/scripts/lua/security.lua
@@ -213,6 +213,7 @@
assert.same(red:exists('oauth:providers:mock:tokens:test'), 1)
assert(result)
end)
+
it('Exchanges a bad token, doesn\'t cache it and returns false', function()
local red = fakeredis.new()
local token = "bad"
@@ -237,31 +238,34 @@
assert.same(red:exists('oauth:providers:mock:tokens:bad'), 0)
assert.falsy(result)
end)
- it('Loads a facebook token from the cache with a valid app id', function()
+
+ it('Has no cross-contamination between App ID caches', function()
local red = fakeredis.new()
local ds = require "lib/dataStore"
local dataStore = ds.initWithDriver(red)
- local token = "test"
+ local token = "test_token"
local appid = "app"
local ngxattrs = [[
{
"http_Authorization":"]] .. token .. [[",
- "http_x_facebook_app_token":"]] .. appid .. [[",
"tenant":"1234",
"gatewayPath":"v1/test"
}
]]
local ngx = fakengx.new()
+ ngx.config = { ngx_lua_version = 'test' }
ngx.var = cjson.decode(ngxattrs)
_G.ngx = ngx
local securityObj = [[
{
"type":"oauth2",
- "provider":"facebook",
- "scope":"resource"
+ "provider":"app-id",
+ "tenantId": "tenant1",
+ "scope":"api"
}
]]
- red:set('oauth:providers:facebook:tokens:testapp', '{"token":"good"}')
+ red:set('oauth:providers:appid_tenant2:tokens:test_token', '{"token":"good"}')
+ red:set('oauth:providers:appid_tenant1:tokens:test_token', '{"token":"good"}')
local result = oauth.process(dataStore, cjson.decode(securityObj))
assert.truthy(result)
end)
