Merge pull request nokia#12 from skylineos/story/STRY0038018

Story/stry0038018

docs/src/pass_flow.mmd

@@ -0,0 +1,22 @@
+title Kong OIDC Plugin - "pass" functionality
+
+participant User
+participant OIDC Provider
+participant Kong
+participant Upstream API
+
+Note over User,Upstream API: When user isn't authenticated
+User->Kong: GET /<api>
+Kong->Upstream API: GET /<api> without x-userinfo
+Upstream API->User: HTTP response
+
+Note over User,Upstream API: When user isn't authenticated (force_authentication_path)
+User->Kong: GET /force_authentication_path
+Kong->User: Redirect to OIDC Provider for Authorization Grant
+User->+OIDC Provider: Login
+OIDC Provider->-User: Redirect to Kong with Authorization Grant
+note right of User: See "How does Kong OIDC work?" diagram for rest of sequence.
+
+Note over User,Upstream API: When user is authenticated
+User->Kong: GET /<api>
+Kong->Upstream API: GET /<api> with x-userinfo

kong/plugins/oidc/handler.lua

@@ -5,10 +5,10 @@ local filter = require("kong.plugins.oidc.filter")
 local session = require("kong.plugins.oidc.session")
 local cjson = require("cjson")
 local openidc = require("resty.openidc")
+local constants = require("kong.plugins.oidc.util.constants")
 
 OidcHandler.PRIORITY = 1000
 
-
 function OidcHandler:new()
   OidcHandler.super.new(self, "oidc")
 end
@@ -31,6 +31,9 @@ end
 function handle(oidcConfig, oidcSessionConfig)
   local response
 
+  -- clear oidc plugin headers to prevent spoofing of info to upstream api
+  utils.clear_request_headers()
+
   -- get/cache discovery data, mutate oidcConfig.discovery if it is a string (discovery endpoint)
   openidc.get_discovery_doc(oidcConfig)
 
@@ -41,15 +44,15 @@ function handle(oidcConfig, oidcSessionConfig)
     if response then
       local access_token = utils.get_bearer_access_token_from_header(oidcConfig)
       local userinfo, err = get_userinfo(oidcConfig, response)
-      
+
       -- @todo: how can we distinguish between access_token and id_token?
       -- err can occur due to id_token being used for authorization header instead of access_token
       if err or not userinfo then
         ngx.log(ngx.DEBUG, "call to userinfo endpoint failed, attaching decoded token to user")
         -- introspect passed but userinfo failed, set userinfo to decoded token instead of leaving blank
         userinfo = response
       end
-      
+
       response = {
         access_token = access_token,
         user = userinfo
@@ -93,22 +96,29 @@ function make_oidc(oidcConfig, oidcSessionConfig)
     end
   end
 
-  -- grab X-Requested-With Header to see if request was from browser/ajax
-  local unauth_action = nil
   local ngx_headers = ngx.req.get_headers()
-  if ngx_headers then
-    local xhr_value = ngx_headers["X-Requested-With"]
-    -- was the request ajax/async?
-    if xhr_value == "XMLHttpRequest" then
-      -- reference: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1436
-      -- set to deny so resty.openidc returns instead of redirects (ends request)
-      ngx.log(ngx.DEBUG, "OidcHandler ajax/async request detected, setting unauth_action = deny")
-      unauth_action = "deny"
-    end
+
+  -- default value for unauth_action is based on force_authentication_path being set.
+  -- If set, unauth_action is set to "pass", default action is to allow request through to the upstream service.
+  -- If not set, unauth_action is set to nil, default action is to redirect request to idp authentication.
+  local unauth_action = oidcConfig.force_authentication_path and constants.UNAUTH_ACTION.PASS or constants.UNAUTH_ACTION.NIL
+
+  -- If the request is an ajax request, set unauth_action to deny (don't redirect user if authentication fails)
+  if ngx_headers and ngx_headers["X-Requested-With"] == "XMLHttpRequest" then
+    -- reference: https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1436
+    ngx.log(ngx.DEBUG, "OidcHandler ajax/async request detected, setting unauth_action = deny")
+    unauth_action = constants.UNAUTH_ACTION.DENY
+
+  -- If the request is not ajax, and matches the configured authentication path (redirect user if authentication fails)
+  elseif ngx.var.request_uri == oidcConfig.force_authentication_path then
+    ngx.log(ngx.DEBUG, "OidcHandler force_authentication_path matched request, setting unauth_action = nil")
+    unauth_action = constants.UNAUTH_ACTION.NIL
   end
 
+
   local res, err, original_url, session = openidc.authenticate(oidcConfig, nil, unauth_action, oidcSessionConfig)
 
+
   -- @todo: add unit test to check for session:close()
   -- handle and close session, prevent locking
   session:close()
@@ -117,7 +127,7 @@ function make_oidc(oidcConfig, oidcSessionConfig)
   -- code execution has gone this far, so return 401 status code to allow client to respond accordingly
   if err == "unauthorized request" then
     ngx.log(ngx.DEBUG, "OidcHandler unauthorized ajax/async request detected, responding with 401 status code")
-    local message = cjson.encode({ status = ngx.status, request_path = ngx.var.request_uri})
+    local message = cjson.encode({ status = ngx.HTTP_UNAUTHORIZED, request_path = ngx.var.request_uri})
     return utils.exit(ngx.HTTP_UNAUTHORIZED, message, ngx.HTTP_UNAUTHORIZED)
   end
 
@@ -159,7 +169,7 @@ function get_userinfo(oidcConfig, introspect_response)
   -- cache hit
   if userinfo then
     userinfo = cjson.decode(userinfo)
-    
+
     -- check if decoded value is blank
     if userinfo == cjson.null then
       ngx.log(ngx.DEBUG, "userinfo cached value is null returning nil value")
@@ -168,22 +178,22 @@ function get_userinfo(oidcConfig, introspect_response)
 
     return userinfo
   end
-  
+
   ngx.log(ngx.INFO, "userinfo cache miss, calling userinfo endpoint")
   userinfo, err = openidc.call_userinfo_endpoint(oidcConfig, access_token)
-  
+
   if err then
     ngx.log(ngx.ERR, "call to userinfo endpoint failed, ", err)
     return nil, err
   end
 
   -- @see openidc.introspect https://github.com/zmartzone/lua-resty-openidc/blob/master/lib/resty/openidc.lua#L1575
-  -- utilized openidc.introspect caching logic 
+  -- utilized openidc.introspect caching logic
   -- todo: add tests to verify values are respected
   local introspection_cache_ignore = oidcConfig.introspection_cache_ignore or false
   local expiry_claim = oidcConfig.introspection_expiry_claim or "exp"
   local introspection_interval = oidcConfig.introspection_interval or 0
-  
+
   if not introspection_cache_ignore and introspect_response[expiry_claim] then
     local ttl = introspect_response[expiry_claim]
     ngx.log(ngx.INFO, ttl)

kong/plugins/oidc/schema.lua

@@ -61,7 +61,8 @@ return {
                 { end_session_endpoint = { type = "string", required = false  } }
               }
             }
-          }
+          },
+          { force_authentication_path = { type = "string" } }
         }
       }
     }

kong/plugins/oidc/util/constants.lua

@@ -0,0 +1,22 @@
+--------------------------------------------------
+--               Declare Contants               --
+--------------------------------------------------
+local constants = {
+
+  -- Request Headers
+  REQUEST_HEADERS = {
+    X_ACCESS_TOKEN = "X-Access-Token",
+    X_ID_TOKEN = "X-ID-Token",
+    X_USERINFO = "X-Userinfo",
+  },
+
+  -- unauth_action values
+  UNAUTH_ACTION = {
+    PASS = "pass",
+    DENY = "deny",
+    NIL = nil,
+  }
+}
+
+
+return constants

kong/plugins/oidc/utils.lua

@@ -1,4 +1,5 @@
 local cjson = require("cjson")
+local constants = require("kong.plugins.oidc.util.constants")
 
 local M = {}
 
@@ -42,6 +43,7 @@ function M.get_options(config, ngx)
     filters = parseFilters(config.filters),
     logout_path = config.logout_path,
     redirect_after_logout_uri = config.redirect_after_logout_uri,
+    force_authentication_path = config.force_authentication_path
   }, config.session
 end
 
@@ -52,12 +54,12 @@ function M.exit(httpStatusCode, message, ngxCode)
 end
 
 function M.injectAccessToken(accessToken)
-  ngx.req.set_header("X-Access-Token", accessToken)
+  ngx.req.set_header(constants.REQUEST_HEADERS.X_ACCESS_TOKEN, accessToken)
 end
 
 function M.injectIDToken(idToken)
   local tokenStr = cjson.encode(idToken)
-  ngx.req.set_header("X-ID-Token", ngx.encode_base64(tokenStr))
+  ngx.req.set_header(constants.REQUEST_HEADERS.X_ID_TOKEN, ngx.encode_base64(tokenStr))
 end
 
 function M.injectUser(user)
@@ -66,7 +68,7 @@ function M.injectUser(user)
   tmp_user.username = user.preferred_username
   ngx.ctx.authenticated_credential = tmp_user
   local userinfo = cjson.encode(user)
-  ngx.req.set_header("X-Userinfo", ngx.encode_base64(userinfo))
+  ngx.req.set_header(constants.REQUEST_HEADERS.X_USERINFO, ngx.encode_base64(userinfo))
 end
 
 function M.has_bearer_access_token()
@@ -132,4 +134,10 @@ function M.cache_get(type, key)
   return value
 end
 
+function M.clear_request_headers()
+  ngx.req.clear_header(constants.REQUEST_HEADERS.X_ACCESS_TOKEN)
+  ngx.req.clear_header(constants.REQUEST_HEADERS.X_ID_TOKEN)
+  ngx.req.clear_header(constants.REQUEST_HEADERS.X_USERINFO)
+end
+
 return M

test/unit/mockable_case.lua

@@ -5,6 +5,7 @@ local MockableCase = BaseCase:extend()
 function MockableCase:setUp()
   MockableCase.super:setUp()
   self.logs = {}
+
   self.mocked_ngx = {
     DEBUG = "debug",
     ERR = "error",
@@ -16,7 +17,8 @@ function MockableCase:setUp()
     req = {
       get_uri_args = function(...) end,
       set_header = function(...) end,
-      get_headers = function(...) end
+      get_headers = function(...) end,
+      clear_header = function(...) end
     },
     log = function(...)
       self.logs[#self.logs+1] = table.concat({...}, " ")