This is an automated email from the ASF dual-hosted git repository. mhamann pushed a commit to branch oauth-improvements in repository https://gitbox.apache.org/repos/asf/openwhisk-apigateway.git
commit 1a9da9f1e29dcb0f966a4e27ca1b247cb0c5e190 Author: Matt Hamann <mham...@us.ibm.com> AuthorDate: Thu Aug 8 00:51:41 2019 -0400 OAuth fixes and improvements * Fix App ID caching issue (and add tests to match) * Improve error messages for all OAuth providers --- Dockerfile.test.unit | 13 ++++++- scripts/lua/oauth/app-id.lua | 63 ++++++++++++++++++++------------ scripts/lua/oauth/facebook.lua | 2 +- scripts/lua/oauth/github.lua | 2 +- scripts/lua/oauth/google.lua | 2 +- scripts/lua/policies/security/oauth2.lua | 2 +- tests/install-deps.sh | 2 + tests/scripts/lua/security.lua | 16 +++++--- 8 files changed, 67 insertions(+), 35 deletions(-) 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 @@ RUN wget https://luarocks.org/releases/luarocks-3.1.3.tar.gz && \ 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..931ea9e 100644 --- a/scripts/lua/oauth/app-id.lua +++ b/scripts/lua/oauth/app-id.lua @@ -23,26 +23,39 @@ local _M = {} 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 @@ function _M.process(dataStore, token, securityObj) 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 @@ function exchangeOAuthToken(dataStore, token, facebookAppToken) -- 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 @@ function _M.process(dataStore, token) -- 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 @@ function _M.process (dataStore, token) -- 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 @@ function exchange(dataStore, token, provider, securityObj) 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 sha1 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 @@ describe('OAuth security module', function() 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 @@ describe('OAuth security module', function() 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)