You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by mh...@apache.org on 2019/08/08 04:52:55 UTC

[openwhisk-apigateway] branch oauth-improvements created (now 1a9da9f)

This is an automated email from the ASF dual-hosted git repository.

mhamann pushed a change to branch oauth-improvements
in repository https://gitbox.apache.org/repos/asf/openwhisk-apigateway.git.


      at 1a9da9f  OAuth fixes and improvements

This branch includes the following new commits:

     new 1a9da9f  OAuth fixes and improvements

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[openwhisk-apigateway] 01/01: OAuth fixes and improvements

Posted by mh...@apache.org.
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 <mh...@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)