spacewander commented on a change in pull request #3308:
URL: https://github.com/apache/apisix/pull/3308#discussion_r563283871



##########
File path: apisix/plugins/authz-keycloak.lua
##########
@@ -69,225 +109,566 @@ local _M = {
 
 
 function _M.check_schema(conf)
+    -- Check for deprecated audience attribute and emit warnings if used.
+    if conf.audience then
+        log.warn("Plugin attribute `audience` is deprecated, use `client_id` 
instead.")
+        if conf.client_id then
+            log.warn("Ignoring `audience` attribute in favor of `client_id`.")
+        end
+    end
     return core.schema.check(schema, conf)
 end
 
 
+-- Return the configured client ID parameter.
+local function authz_keycloak_get_client_id(conf)
+    if conf.client_id then
+        -- Prefer client_id, if given.
+        return conf.client_id
+    end
+
+    return conf.audience
+end
+
+
 -- Some auxiliary functions below heavily inspired by the excellent
 -- lua-resty-openidc module; see https://github.com/zmartzone/lua-resty-openidc
 
 
 -- Retrieve value from server-wide cache, if available.
 local function authz_keycloak_cache_get(type, key)
-  local dict = ngx.shared[type]
-  local value
-  if dict then
-    value = dict:get(key)
-    if value then log.debug("cache hit: type=", type, " key=", key) end
-  end
-  return value
+    local dict = ngx.shared[type]
+    local value
+    if dict then
+        value = dict:get(key)
+        if value then log.debug("cache hit: type=", type, " key=", key) end
+    end
+    return value
 end
 
 
 -- Set value in server-wide cache, if available.
 local function authz_keycloak_cache_set(type, key, value, exp)
-  local dict = ngx.shared[type]
-  if dict and (exp > 0) then
-    local success, err, forcible = dict:set(key, value, exp)
-    if err then
-        log.error("cache set: success=", success, " err=", err, " forcible=", 
forcible)
+    local dict = ngx.shared[type]
+    if dict and (exp > 0) then
+        local success, err, forcible = dict:set(key, value, exp)
+        if err then
+            log.error("cache set: success=", success, " err=", err, " 
forcible=", forcible)
+        else
+            log.debug("cache set: success=", success, " err=", err, " 
forcible=", forcible)
+        end
+    end
+end
+
+
+-- Configure request parameters.
+local function authz_keycloak_configure_params(params, conf)
+    -- Keepalive options.
+    if conf.keepalive then
+        params.keepalive_timeout = conf.keepalive_timeout
+        params.keepalive_pool = conf.keepalive_pool
     else
-        log.debug("cache set: success=", success, " err=", err, " forcible=", 
forcible)
+        params.keepalive = conf.keepalive
     end
-  end
+
+    -- TLS verification.
+    params.ssl_verify = conf.ssl_verify
+
+    -- Decorate parameters, maybe, and return.
+    return conf.http_request_decorator and conf.http_request_decorator(params) 
or params
 end
 
 
 -- Configure timeouts.
 local function authz_keycloak_configure_timeouts(httpc, timeout)
-  if timeout then
-    if type(timeout) == "table" then
-      httpc:set_timeouts(timeout.connect or 0, timeout.send or 0, timeout.read 
or 0)
-    else
-      httpc:set_timeout(timeout)
+    if timeout then
+        if type(timeout) == "table" then
+            httpc:set_timeouts(timeout.connect or 0, timeout.send or 0, 
timeout.read or 0)
+        else
+            httpc:set_timeout(timeout)
+        end
     end
-  end
 end
 
 
 -- Set outgoing proxy options.
 local function authz_keycloak_configure_proxy(httpc, proxy_opts)
-  if httpc and proxy_opts and type(proxy_opts) == "table" then
-    log.debug("authz_keycloak_configure_proxy : use http proxy")
-    httpc:set_proxy_options(proxy_opts)
-  else
-    log.debug("authz_keycloak_configure_proxy : don't use http proxy")
-  end
+    if httpc and proxy_opts and type(proxy_opts) == "table" then
+        log.debug("authz_keycloak_configure_proxy : use http proxy")
+        httpc:set_proxy_options(proxy_opts)
+    else
+        log.debug("authz_keycloak_configure_proxy : don't use http proxy")
+    end
+end
+
+
+-- Get and configure HTTP client.
+local function authz_keycloak_get_http_client(conf)
+    local httpc = http.new()
+    authz_keycloak_configure_timeouts(httpc, conf.timeout)
+    authz_keycloak_configure_proxy(httpc, conf.proxy_opts)
+    return httpc
 end
 
 
 -- Parse the JSON result from a call to the OP.
 local function authz_keycloak_parse_json_response(response)
-  local err
-  local res
+    local err
+    local res
 
-  -- Check the response from the OP.
-  if response.status ~= 200 then
-    err = "response indicates failure, status=" .. response.status .. ", 
body=" .. response.body
-  else
-    -- Decode the response and extract the JSON object.
-    res, err = core.json.decode(response.body)
+    -- Check the response from the OP.
+    if response.status ~= 200 then
+        err = "response indicates failure, status=" .. response.status .. ", 
body=" .. response.body
+    else
+        -- Decode the response and extract the JSON object.
+        res, err = core.json.decode(response.body)
 
-    if not res then
-      err = "JSON decoding failed: " .. err
+        if not res then
+            err = "JSON decoding failed: " .. err
+        end
     end
-  end
 
-  return res, err
-end
-
-
-local function decorate_request(http_request_decorator, req)
-  return http_request_decorator and http_request_decorator(req) or req
+    return res, err
 end
 
 
 -- Get the Discovery metadata from the specified URL.
-local function authz_keycloak_discover(url, ssl_verify, keepalive, timeout,
-                                       exptime, proxy_opts, 
http_request_decorator)
-  log.debug("authz_keycloak_discover: URL is: " .. url)
-
-  local json, err
-  local v = authz_keycloak_cache_get("discovery", url)
-  if not v then
-
-    log.debug("Discovery data not in cache, making call to discovery 
endpoint.")
-    -- Make the call to the discovery endpoint.
-    local httpc = http.new()
-    authz_keycloak_configure_timeouts(httpc, timeout)
-    authz_keycloak_configure_proxy(httpc, proxy_opts)
-    local res, error = httpc:request_uri(url, 
decorate_request(http_request_decorator, {
-      ssl_verify = (ssl_verify ~= "no"),
-      keepalive = (keepalive ~= "no")
-    }))
-    if not res then
-      err = "accessing discovery url (" .. url .. ") failed: " .. error
-      log.error(err)
+local function authz_keycloak_discover(conf)
+    log.debug("authz_keycloak_discover: URL is: " .. conf.discovery)
+
+    local json, err
+    local v = authz_keycloak_cache_get("discovery", conf.discovery)
+
+    if not v then
+        log.debug("Discovery data not in cache, making call to discovery 
endpoint.")
+
+        -- Make the call to the discovery endpoint.
+        local httpc = authz_keycloak_get_http_client(conf)
+
+        local params = authz_keycloak_configure_params({}, conf)
+
+        local res, error = httpc:request_uri(conf.discovery, params)
+
+        if not res then
+            err = "Accessing discovery URL (" .. conf.discovery .. ") failed: 
" .. error
+            log.error(err)
+        else
+            log.debug("Response data: " .. res.body)
+            json, err = authz_keycloak_parse_json_response(res)
+            if json then
+                authz_keycloak_cache_set("discovery", conf.discovery, 
core.json.encode(json),
+                                         conf.cache_ttl_seconds)
+            else
+                err = "could not decode JSON from Discovery data" .. (err and 
(": " .. err) or '')
+                log.error(err)
+            end
+        end
     else
-      log.debug("response data: " .. res.body)
-      json, err = authz_keycloak_parse_json_response(res)
-      if json then
-        authz_keycloak_cache_set("discovery", url, core.json.encode(json), 
exptime or 24 * 60 * 60)
-      else
-        err = "could not decode JSON from Discovery data" .. (err and (": " .. 
err) or '')
-        log.error(err)
-      end
+        json = core.json.decode(v)
     end
 
-  else
-    json = core.json.decode(v)
-  end
-
-  return json, err
+    return json, err
 end
 
 
--- Turn a discovery url set in the opts dictionary into the discovered 
information.
-local function authz_keycloak_ensure_discovered_data(opts)
-  local err
-  if type(opts.discovery) == "string" then
-    local discovery
-    discovery, err = authz_keycloak_discover(opts.discovery, opts.ssl_verify, 
opts.keepalive,
-                                             opts.timeout, 
opts.jwk_expires_in, opts.proxy_opts,
-                                             opts.http_request_decorator)
-    if not err then
-      opts.discovery = discovery
+-- Turn a discovery url set in the conf dictionary into the discovered 
information.
+local function authz_keycloak_ensure_discovered_data(conf)
+    local err
+    if type(conf.discovery) == "string" then
+        local discovery
+        discovery, err = authz_keycloak_discover(conf)
+        if not err then
+            conf.discovery = discovery
+        end
     end
-  end
-  return err
+    return err
 end
 
 
+-- Get an endpoint from the configuration.
 local function authz_keycloak_get_endpoint(conf, endpoint)
     if conf and conf[endpoint] then
+        -- Use explicit entry.
         return conf[endpoint]
     elseif conf and conf.discovery and type(conf.discovery) == "table" then
+        -- Use discovery data.
         return conf.discovery[endpoint]
     end
 
+    -- Unable to obtain endpoint.
     return nil
 end
 
 
+-- Return the token endpoint from the configuration.
 local function authz_keycloak_get_token_endpoint(conf)
     return authz_keycloak_get_endpoint(conf, "token_endpoint")
 end
 
 
-local function is_path_protected(conf)
-    -- TODO if permissions are empty lazy load paths from Keycloak
-    if conf.permissions == nil then
-        return false
+-- Return the resource registration endpoint from the configuration.
+local function authz_keycloak_get_resource_registration_endpoint(conf)
+    return authz_keycloak_get_endpoint(conf, "resource_registration_endpoint")
+end
+
+
+-- Return access_token expires_in value (in seconds).
+local function authz_keycloak_access_token_expires_in(conf, expires_in)
+    return (expires_in or conf.access_token_expires_in or 300)
+           - 1 - (conf.access_token_expires_leeway or 0)
+end
+
+
+-- Return refresh_token expires_in value (in seconds).
+local function authz_keycloak_refresh_token_expires_in(conf, expires_in)
+    return (expires_in or conf.refresh_token_expires_in or 3600)
+           - 1 - (conf.refresh_token_expires_leeway or 0)

Review comment:
       Ditto




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to