diff --git a/README.md b/README.md index 6a49d80..34f9551 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,6 @@ Kong plugin to use with [frontier](https://github.com/raystack/frontier/) auth s ### TODO - Add test cases - https://github.com/lunarmodules/luacheck -- cache frontier response - https://docs.konghq.com/gateway/latest/plugin-development/entities-cache/#cache-custom-entities ### Notes - Add plugin configuration in kong.yml file where url is a required field @@ -65,6 +64,158 @@ disabled = { default = false } ``` +### Token caching + +Works on Kong 3.4 and later. It only uses modules that ship with Kong and +OpenResty, so there is nothing extra to install. + +The plugin exchanges the incoming cookie or bearer for a user token on every +request. Setting `redis_host` caches that exchange in redis, so the same +credential is not exchanged again for a few seconds. The lookup is redis first, +then the auth server on a miss. + +Redis is shared by every pod, so a token is fetched once for the whole fleet +rather than once per pod. + +The default ttl is 5 seconds. It is deliberately short: a cached token means a +change to someone's access is not picked up until the entry expires. + +**Caching needs redis.** Without `redis_host` there is nowhere to keep a token, +so `cache_ttl` does nothing on its own and every request goes to the auth +server, exactly as it did before this existed. + +```yaml +plugins: +- name: frontier + config: + authn_url: ... + redis_host: redis.internal + redis_port: 6379 +``` + +| Field | Default | What it does | +|---|---|---| +| `cache_ttl` | `5` | Seconds a token is reused for. `0` turns caching off. Max `300` | +| `cache_cookie_names` | `["sid"]` | Only these cookies go into the cache key | +| `redis_host` | unset | Setting it turns caching on | +| `redis_port` | `6379` | | +| `redis_timeout` | `100` | Milliseconds, for connect, send and read | +| `redis_username` | unset | Redis 6 ACL user, if you use one | +| `redis_password` | unset | | +| `redis_database` | `0` | | +| `redis_ssl` | `false` | | +| `redis_ssl_verify` | `false` | Needs `lua_ssl_trusted_certificate` set on the gateway | +| `redis_server_name` | unset | SNI, when using SSL | +| `redis_key_prefix` | `frontier:authn:` | Prefix on every key | +| `redis_breaker_seconds` | `10` | How long a worker stops trying after a failure | + +Connections are reused through OpenResty's own connection pool, keyed by host, +port, database, user and whether SSL is on. Two plugin configs that mean the +same thing share a pool; two that differ do not. The pool is not configurable, +the same way it is not in the bundled rate limiting plugin. + +The timeout default is 100ms, much lower than the bundled rate limiting +plugin's 2000ms. A healthy redis answers in well under a millisecond, so 100ms +is already a hundred times the expected latency. This sits in the auth path, so +a redis slower than that should be given up on rather than held onto. The cost +of being wrong is small: the request goes to the auth server instead, and the +worker stops trying redis for `redis_breaker_seconds`. + +#### How redis behaves + +**It never fails a request.** Redis is a cache, not an authority. A connect +error, a timeout, a bad reply, even a raise, is logged and the plugin carries on +to the auth server. + +When a command to an instance fails, the worker stops trying that instance for +`redis_breaker_seconds`, so an outage cannot make every request pay the timeout +first. The pause is per instance, so a fault on one redis does not stop the +worker talking to another. A wrong password or a bad database index is a config +mistake rather than a broken instance, so those are logged without starting the +pause. + +**Treat write access to this redis as equal to being any user.** The plugin +never checks the token signature, with or without redis. It trusts whatever the +auth server hands back. So anything that can write these keys can put a token of +its choosing in front of the upstream. The entries also hold live user tokens, +which is more sensitive than something like rate limit counters. Turn on auth +and SSL if the instance is shared or reachable from outside the cluster, and +keep `redis_key_prefix` set so the keys cannot collide with anything else using +it. + +#### What it does and does not cache + +- The cache key is a sha256 of the cookies named in `cache_cookie_names`, the + authorization header, and the config that decides what an entry means: + `authn_url`, `http_method`, `header_name`, `token_response_field` and + `cache_ttl`. The session value is never stored in plain text, and two routes + that would resolve a credential differently cannot share an entry. +- Only the named cookies go into the key. Browsers send analytics and consent + cookies that change constantly, so keying on the whole cookie header would + miss on nearly every request. +- A request with none of those credentials is never cached, so anonymous + requests cannot share an entry. +- A failed exchange is never cached. A user who has just been given access is + not locked out for the length of the ttl. +- The entry lives for exactly `cache_ttl`. The token is never parsed, so + **`cache_ttl` has to stay well under your auth server's token lifetime**, or + the cache will hand out tokens that have already expired. Frontier mints a + fresh token on every call and its `token.validity` defaults to an hour, so + the default of 5 seconds leaves a very wide margin. The ceiling of 300 is + there so a careless value cannot get close. +- Only the authn call is cached. The authz check in `authz_url` still runs on + every request. +- There is no lock, so several requests arriving together with the same new + credential will each fetch a token. They all write an equivalent entry, and + every request after that is served from redis. +- A token is read for its `exp` when it is stored, and for the claims that + become headers when it is used. Nothing else about it is assumed, so a token + that is valid JSON but is not shaped like a JWT is refused rather than half + applied. + +#### What it costs and saves + +Measured against a real Frontier and a real redis, with Kong in DB-less mode. +Absolute numbers come from docker on macOS, where container networking is slow, +so read the gaps rather than the values. + +One session making requests as fast as it can for 20 seconds, `cache_ttl` at 5: + +| | Requests served | Auth server calls | +|---|---|---| +| Caching on | 455 | 4 | +| Caching off | 262 | 262 | + +Four calls in a 20 second window is what a 5 second ttl should give. The same +client also got through 1.7 times as many requests, because it was not waiting +on an auth call every time. + +Kong's own CPU per request, from the container's cgroup accounting, 500 requests +per run over one reused connection, median of 5 runs: + +| | Kong CPU per request | requests/sec | +|---|---|---| +| No plugin | 0.224 ms | 293 | +| Plugin, redis hit | 0.330 ms | 264 | +| Plugin, caching off | 1.731 ms | 26 | + +A redis hit costs 0.11ms more CPU than plain proxying, for the cookie parse, the +hash and the redis round trip. The auth call it replaces costs 1.5ms, about +fourteen times more. + +Latency, with the plain route and the cached route interleaved to cancel drift: + +| Path | Median | p95 | +|---|---|---| +| No plugin | 6.8 ms | 13.8 ms | +| Redis hit | 7.4 ms | 16.1 ms | +| Auth server fetch | 39.2 ms | 52.0 ms | + +So the redis hop adds about 0.6ms and saves about 32ms. + +An entry costs about 1KB in redis, for a token of roughly 950 bytes, so size it +as `users active within the ttl window x 1KB`. + - For local development linting ``` brew install wget diff --git a/kong-plugin-frontier-0.1.1-1.rockspec b/kong-plugin-frontier-0.1.1-1.rockspec index 3d9309e..e179f9d 100644 --- a/kong-plugin-frontier-0.1.1-1.rockspec +++ b/kong-plugin-frontier-0.1.1-1.rockspec @@ -32,6 +32,8 @@ build = { ["kong.plugins."..plugin_name..".jwt_decoder"] = "kong/plugins/"..plugin_name.."/jwt_decoder.lua", ["kong.plugins."..plugin_name..".schema"] = "kong/plugins/"..plugin_name.."/schema.lua", ["kong.plugins."..plugin_name..".access"] = "kong/plugins/"..plugin_name.."/access.lua", + ["kong.plugins."..plugin_name..".cache"] = "kong/plugins/"..plugin_name.."/cache.lua", + ["kong.plugins."..plugin_name..".redis"] = "kong/plugins/"..plugin_name.."/redis.lua", ["kong.plugins."..plugin_name..".utils"] = "kong/plugins/"..plugin_name.."/utils.lua", } } \ No newline at end of file diff --git a/kong/plugins/frontier/access.lua b/kong/plugins/frontier/access.lua index fc20821..97823de 100644 --- a/kong/plugins/frontier/access.lua +++ b/kong/plugins/frontier/access.lua @@ -3,6 +3,7 @@ local _M = {} local http = require "resty.http" local json = require('cjson') local jwt_decoder = require "kong.plugins.frontier.jwt_decoder" +local cache = require "kong.plugins.frontier.cache" local kong = kong local ngx = ngx local utils = require "kong.plugins.frontier.utils" @@ -26,12 +27,13 @@ end local function get_http_client(conf) local client = http.new() - client:set_timeouts(conf.http_connect_timeout, conf.http_read_timeout, conf.http_send_timeout) + local connect_timeout, send_timeout, read_timeout = + conf.http_connect_timeout, conf.http_send_timeout, conf.http_read_timeout + client:set_timeouts(connect_timeout, send_timeout, read_timeout) return client end --- send a request to auth server and fetch user token in exchange of cookies -local function check_request_identity(conf, cookies, bearer) +local function fetch_identity_token(conf, cookies, bearer) local client = get_http_client(conf) local correlation_id = kong.request.get_header(conf.correlation_header_name) @@ -59,13 +61,11 @@ local function check_request_identity(conf, cookies, bearer) local res, err = client:request_uri(conf.authn_url, request_options) if not res or err then kong.log.warn("failed to check request identity: ", err) - return fail_auth() + return nil, err or "no response from auth server" end if not err and res and res.status ~= 200 then kong.log.warn("received non 200 response status: ", res.status) - return kong.response.exit(ngx.HTTP_UNAUTHORIZED, unauthorized_response, { - ["x-upstream-status"] = res.status - }) + return nil, "non 200 response status", res.status end kong.log.debug("check_request_identity: Received successful response with status: ", res.status) @@ -89,6 +89,44 @@ local function check_request_identity(conf, cookies, bearer) end kong.log.debug("check_request_identity: Returning token: ", token and "found" or "not found") + + if not token then + return nil, "no token in auth server response" + end + + return token, nil, nil +end + +local function check_request_identity(conf, cookies, bearer) + local auth_server_status + + local function fetch() + local token, err, status = fetch_identity_token(conf, cookies, bearer) + auth_server_status = status + + return token, err + end + + local token, err + + if conf.cache_ttl > 0 then + token, err = cache.get(conf, cache.build_key(conf, cookies, bearer), fetch) + else + token, err = fetch() + end + + if not token then + kong.log.warn("failed to resolve user token: ", err) + + if auth_server_status then + return kong.response.exit(ngx.HTTP_UNAUTHORIZED, unauthorized_response, { + ["x-upstream-status"] = auth_server_status + }) + end + + return fail_auth() + end + return token end @@ -186,6 +224,13 @@ local function append_claims_as_headers(conf, user_token) local claims = jwt.claims + local claims_are_readable = type(claims) == "table" + + if not claims_are_readable then + kong.log.warn("token payload is not an object, cannot read claims") + return fail_auth() + end + for _, header_name in pairs(conf.token_claims_to_append_as_headers) do local new_header = conf.frontier_header_prefix .. header_name local val = claims[header_name] @@ -208,12 +253,15 @@ local function verify_organization_id_header(conf, user_token) end local claims = jwt.claims - local org_ids = claims[frontier_org_ids_claim_key] + local org_ids = type(claims) == "table" and claims[frontier_org_ids_claim_key] or nil local org_id_header_verified = false - for word in string.gmatch(org_ids, '([^,]+)') do - if word == request_organization_id then - org_id_header_verified = true + + if type(org_ids) == "string" then + for word in string.gmatch(org_ids, '([^,]+)') do + if word == request_organization_id then + org_id_header_verified = true + end end end diff --git a/kong/plugins/frontier/cache.lua b/kong/plugins/frontier/cache.lua new file mode 100644 index 0000000..4f228a2 --- /dev/null +++ b/kong/plugins/frontier/cache.lua @@ -0,0 +1,110 @@ +local _M = {} + +local redis = require "kong.plugins.frontier.redis" +local utils = require "kong.plugins.frontier.utils" + +local kong = kong +local pcall = pcall +local concat = table.concat +local ipairs = ipairs +local sort = table.sort +local tostring = tostring +local hash = utils.hash + +local function cookie_names_in_a_stable_order(conf) + local names = {} + + for _, name in ipairs(conf.cache_cookie_names or {}) do + names[#names + 1] = name + end + + sort(names) + + return names +end + +local function settings_that_change_what_an_entry_means(conf) + return { + conf.authn_url or "", + conf.http_method or "", + conf.header_name or "", + conf.token_response_field or "", + tostring(conf.cache_ttl) + } +end + +function _M.build_key(conf, cookies, bearer) + local jar = utils.parse_cookies(cookies) + local parts = settings_that_change_what_an_entry_means(conf) + local found_a_credential = false + + for _, name in ipairs(cookie_names_in_a_stable_order(conf)) do + local every_value_sent_under_this_name = jar[name] + + if every_value_sent_under_this_name then + for _, value in ipairs(every_value_sent_under_this_name) do + found_a_credential = found_a_credential or value ~= "" + parts[#parts + 1] = name .. "=" .. value + end + else + parts[#parts + 1] = name .. "=" + end + end + + found_a_credential = found_a_credential or (bearer ~= nil and bearer ~= "") + parts[#parts + 1] = bearer or "" + + if not found_a_credential then + return nil + end + + return hash(concat(parts, "\0")) +end + +local function token_in_redis(conf, key) + local reached_redis, token = pcall(redis.get, conf, key) + + if not reached_redis then + kong.log.warn("redis lookup raised, ignoring it: ", token) + return nil + end + + return token +end + +local function remember_token_in_redis(conf, key, token) + local reached_redis, err = pcall(redis.set, conf, key, token, conf.cache_ttl) + + if not reached_redis then + kong.log.warn("redis write raised, ignoring it: ", err) + end +end + +function _M.get(conf, key, fetch_from_auth_server) + local nothing_to_cache_or_nowhere_to_cache_it = key == nil or not redis.enabled(conf) + + if nothing_to_cache_or_nowhere_to_cache_it then + return fetch_from_auth_server() + end + + local cached = token_in_redis(conf, key) + + if cached then + kong.log.debug("token served from redis") + return cached + end + + local token, err = fetch_from_auth_server() + + if not token then + return nil, err + end + + if conf.cache_ttl > 0 then + remember_token_in_redis(conf, key, token) + end + + return token +end + +return _M diff --git a/kong/plugins/frontier/jwt_decoder.lua b/kong/plugins/frontier/jwt_decoder.lua index 82004b3..da4e414 100644 --- a/kong/plugins/frontier/jwt_decoder.lua +++ b/kong/plugins/frontier/jwt_decoder.lua @@ -2,9 +2,14 @@ local _M = {} local jwt_decoder = require "kong.plugins.jwt.jwt_parser" --- Return type: [metatable, error] function _M.decode_token(token) - local jwt, err = jwt_decoder:new(token) + local parsed_without_raising, jwt, err = pcall(jwt_decoder.new, jwt_decoder, token) + + if not parsed_without_raising then + local raised = jwt + ngx.log(ngx.STDERR, raised) + return nil, "could not decode token" + end if err then ngx.log(ngx.STDERR, err) @@ -14,4 +19,4 @@ function _M.decode_token(token) return jwt, nil end -return _M \ No newline at end of file +return _M diff --git a/kong/plugins/frontier/redis.lua b/kong/plugins/frontier/redis.lua new file mode 100644 index 0000000..74dfc63 --- /dev/null +++ b/kong/plugins/frontier/redis.lua @@ -0,0 +1,171 @@ +local _M = {} + +local resty_redis = require "resty.redis" + +local kong = kong +local ngx = ngx +local fmt = string.format +local math_floor = math.floor +local tonumber = tonumber + +local KEEPALIVE_MS = 60000 +local POOL_SIZE = 30 +local KEY_NOT_FOUND = ngx.null +local NEVER_USED_BEFORE = 0 + +local skip_instance_until = {} + +local function instance_id(conf) + return fmt("frontier:%s:%d:%d:%s:%s", + conf.redis_host, + conf.redis_port, + conf.redis_database, + conf.redis_username or "", + conf.redis_ssl and "s" or "p") +end + +local function instance_is_being_skipped(conf) + local until_when = skip_instance_until[instance_id(conf)] + + return until_when ~= nil and ngx.now() < until_when +end + +local function skip_instance_for_a_while(conf, action, err) + skip_instance_until[instance_id(conf)] = ngx.now() + conf.redis_breaker_seconds + + kong.log.warn("redis at ", conf.redis_host, ":", conf.redis_port, " failed (", + action, ": ", err, "), skipping it for ", conf.redis_breaker_seconds, "s") +end + +function _M.enabled(conf) + return conf.redis_host ~= nil and conf.redis_host ~= "" +end + +local function authenticate_and_select_database(red, conf) + if conf.redis_password and conf.redis_password ~= "" then + local accepted, err + + if conf.redis_username and conf.redis_username ~= "" then + accepted, err = red:auth(conf.redis_username, conf.redis_password) + else + accepted, err = red:auth(conf.redis_password) + end + + if not accepted then + kong.log.warn("redis refused the credentials for ", + conf.redis_host, ":", conf.redis_port, ": ", err) + return false + end + end + + if conf.redis_database ~= 0 then + local selected, err = red:select(conf.redis_database) + + if not selected then + kong.log.warn("redis rejected database ", conf.redis_database, + " on ", conf.redis_host, ":", conf.redis_port, ": ", err) + return false + end + end + + return true +end + +local function borrow_connection(conf) + local red = resty_redis:new() + red:set_timeouts(conf.redis_timeout, conf.redis_timeout, conf.redis_timeout) + + local connected, connect_err = red:connect(conf.redis_host, conf.redis_port, { + ssl = conf.redis_ssl, + ssl_verify = conf.redis_ssl_verify, + server_name = conf.redis_server_name, + pool = instance_id(conf) + }) + + if not connected then + skip_instance_for_a_while(conf, "connect", connect_err) + return nil + end + + local times_used_before, reuse_err = red:get_reused_times() + + if reuse_err then + skip_instance_for_a_while(conf, "get_reused_times", reuse_err) + red:close() + return nil + end + + if times_used_before == NEVER_USED_BEFORE and not authenticate_and_select_database(red, conf) then + red:close() + return nil + end + + return red +end + +local function return_connection(red) + local returned, err = red:set_keepalive(KEEPALIVE_MS, POOL_SIZE) + + if not returned then + kong.log.debug("failed to return redis connection to the pool: ", err) + red:close() + end +end + +function _M.get(conf, key) + if instance_is_being_skipped(conf) then + return nil + end + + local red = borrow_connection(conf) + + if not red then + return nil + end + + local value, err = red:get(conf.redis_key_prefix .. key) + + if not value then + skip_instance_for_a_while(conf, "get", err) + red:close() + return nil + end + + return_connection(red) + + if value == KEY_NOT_FOUND or value == "" then + return nil + end + + return value +end + +function _M.set(conf, key, value, ttl_seconds) + if instance_is_being_skipped(conf) then + return + end + + local expires_in_ms = math_floor((tonumber(ttl_seconds) or 0) * 1000) + + if expires_in_ms <= 0 then + return + end + + local red = borrow_connection(conf) + + if not red then + return + end + + local stored, err = red:set(conf.redis_key_prefix .. key, value, "PX", expires_in_ms) + + if not stored then + kong.log.warn("redis rejected the write: ", err) + red:close() + return + end + + return_connection(red) +end + +return _M diff --git a/kong/plugins/frontier/schema.lua b/kong/plugins/frontier/schema.lua index 37a53f7..240b64d 100644 --- a/kong/plugins/frontier/schema.lua +++ b/kong/plugins/frontier/schema.lua @@ -9,6 +9,10 @@ local DEFAULT_TOKEN_HEADERS = { "user_id" } +local DEFAULT_CACHE_COOKIE_NAMES = { + "sid" +} + -- https://github.com/Kong/kong-plugin/blob/master/kong/plugins/myplugin/schema.lua local schema = { name = PLUGIN_NAME, @@ -20,17 +24,20 @@ local schema = { fields = {{ http_connect_timeout = { type = "number", - default = 2000 + default = 2000, + between = { 1, 60000 } } }, { http_send_timeout = { type = "number", - default = 2000 + default = 2000, + between = { 1, 60000 } } }, { http_read_timeout = { type = "number", - default = 2000 + default = 2000, + between = { 1, 60000 } } }, { header_name = { @@ -94,6 +101,97 @@ local schema = { type = "string", default = "X-Request-Id" } + }, { + -- how long a fetched user token is reused for, in seconds. + -- Kept short so that an access change is picked up quickly. + -- Set to 0 to turn caching off. Caching needs redis_host set; + -- without it there is nowhere to keep a token and every request + -- goes to the auth server. + -- + -- The token is never read for its own expiry, so this has to + -- stay well under the auth server's token lifetime. Frontier + -- mints a fresh token per call and defaults to an hour, so the + -- 300 ceiling leaves a wide margin. + cache_ttl = { + type = "number", + default = 5, + between = { 0, 300 } + } + }, { + -- only these cookies go into the cache key. Browsers send many + -- other cookies that change often, and keying on all of them + -- would miss on nearly every request. `sid` is the cookie + -- frontier uses for the session. + cache_cookie_names = { + type = "array", + default = DEFAULT_CACHE_COOKIE_NAMES, + elements = { + type = "string" + } + } + + }, { + -- setting a host turns caching on. Leave it unset and every + -- request goes to the auth server. + redis_host = typedefs.host + }, { + redis_port = typedefs.port({ + default = 6379 + }) + }, { + -- deliberately much lower than the bundled rate limiting + -- plugin's 2000ms. A healthy redis answers in well under a + -- millisecond, and this sits in the auth path, so a slow one + -- should be given up on quickly in favour of the auth server. + redis_timeout = { + type = "number", + default = 100, + between = { 1, 10000 } + } + }, { + redis_username = { + type = "string", + referenceable = true + } + }, { + redis_password = { + type = "string", + len_min = 0, + referenceable = true + } + }, { + redis_database = { + type = "integer", + default = 0, + between = { 0, 15 } + } + }, { + redis_ssl = { + type = "boolean", + default = false + } + }, { + redis_ssl_verify = { + type = "boolean", + default = false + } + }, { + redis_server_name = typedefs.sni + }, { + -- prefix on every key, so this cannot collide with anything + -- else sharing the same redis + redis_key_prefix = { + type = "string", + default = "frontier:authn:" + } + }, { + -- after a redis failure a worker stops trying for this long, so + -- a redis outage cannot make every request pay the timeout + redis_breaker_seconds = { + type = "number", + default = 10, + between = { 0, 600 } + } }, { rule = { type = "record", diff --git a/kong/plugins/frontier/utils.lua b/kong/plugins/frontier/utils.lua index 59ae6b7..c16f4ab 100644 --- a/kong/plugins/frontier/utils.lua +++ b/kong/plugins/frontier/utils.lua @@ -1,7 +1,17 @@ local _M = {} --- splits a string s using a delimiter and returns a table --- containing the resulting substrings +local resty_sha256 = require "resty.sha256" + +local encode_base64 = ngx.encode_base64 + +local sha256 = resty_sha256:new() + +function _M.hash(input) + sha256:reset() + sha256:update(input) + return (encode_base64(sha256:final(), true):gsub("+", "-"):gsub("/", "_")) +end + function _M.split(s, delimiter) local result = {} for match in (s .. delimiter):gmatch("(.-)" .. delimiter) do @@ -15,4 +25,28 @@ function _M.ltrim(s) return s:match'^%s*(.*)' end +function _M.parse_cookies(cookie_header) + local name_to_every_value_sent = {} + + if not cookie_header then + return name_to_every_value_sent + end + + for pair in cookie_header:gmatch("[^;]+") do + local name, value = pair:match("^%s*([^=%s]+)%s*=%s*(.-)%s*$") + + if name then + local values = name_to_every_value_sent[name] + + if values then + values[#values + 1] = value + else + name_to_every_value_sent[name] = { value } + end + end + end + + return name_to_every_value_sent +end + return _M diff --git a/kong/plugins/spec/frontier-test/01-schema_spec.lua b/kong/plugins/spec/frontier-test/01-schema_spec.lua index 209980f..3835d94 100644 --- a/kong/plugins/spec/frontier-test/01-schema_spec.lua +++ b/kong/plugins/spec/frontier-test/01-schema_spec.lua @@ -9,4 +9,31 @@ describe("Plugin: " .. PLUGIN_NAME .. " (schema), ", function() authn_url = "my_auth_url" }, schema_def)) end) + + it("caching defaults are applied", function() + local ok = assert(v({ + authn_url = "my_auth_url" + }, schema_def)) + + assert.equal(5, ok.config.cache_ttl) + assert.same({ "sid" }, ok.config.cache_cookie_names) + end) + + it("caching can be turned off with a zero ttl", function() + local ok = assert(v({ + authn_url = "my_auth_url", + cache_ttl = 0 + }, schema_def)) + + assert.equal(0, ok.config.cache_ttl) + end) + + it("cache cookie names can be overridden", function() + local ok = assert(v({ + authn_url = "my_auth_url", + cache_cookie_names = { "sid", "other_session" } + }, schema_def)) + + assert.same({ "sid", "other_session" }, ok.config.cache_cookie_names) + end) end) \ No newline at end of file diff --git a/kong/plugins/spec/frontier-test/04-cache_spec.lua b/kong/plugins/spec/frontier-test/04-cache_spec.lua new file mode 100644 index 0000000..8654945 --- /dev/null +++ b/kong/plugins/spec/frontier-test/04-cache_spec.lua @@ -0,0 +1,329 @@ +local PLUGIN_NAME = "frontier" + +-- cache.lua logs through kong and takes its reference to both kong and the +-- redis module at load time, so both are set up before it is required. +_G.kong = _G.kong or { + log = { + debug = function() end, + info = function() end, + warn = function() end, + err = function() end + } +} + +-- Reaching a real redis needs a cosocket, which only works inside a request, so +-- a table stands in for it. +local store = {} +local calls = { get = 0, set = 0 } +local raise_on = {} + +package.loaded["kong.plugins." .. PLUGIN_NAME .. ".redis"] = { + enabled = function(conf) + return conf.redis_host ~= nil and conf.redis_host ~= "" + end, + + get = function(_, key) + calls.get = calls.get + 1 + + if raise_on.get then + error("redis blew up") + end + + local entry = store[key] + if not entry then + return nil + end + + -- expire the way redis would + if entry.expires_at <= ngx.now() then + store[key] = nil + return nil + end + + return entry.value + end, + + set = function(_, key, value, ttl) + calls.set = calls.set + 1 + + if raise_on.set then + error("redis blew up") + end + + store[key] = { value = value, ttl = ttl, expires_at = ngx.now() + ttl } + end +} + +local cache = require("kong.plugins."..PLUGIN_NAME..".cache") +local utils = require("kong.plugins."..PLUGIN_NAME..".utils") + +local function conf(overrides) + local c = { + authn_url = "http://frontier/v1beta1/auth/token", + cache_ttl = 5, + cache_cookie_names = { "sid" }, + redis_host = "127.0.0.1", + redis_port = 6379, + redis_timeout = 100, + redis_database = 0, + redis_key_prefix = "frontier:authn:test:", + redis_breaker_seconds = 10 + } + for k, val in pairs(overrides or {}) do + c[k] = val + end + return c +end + +local function reset() + store = {} + calls.get, calls.set = 0, 0 + raise_on.get, raise_on.set = nil, nil +end + +-- an auth server that hands back `token` and counts how often it was asked +local function auth_server(token, err) + local count = 0 + return function() + count = count + 1 + return token, err + end, function() + return count + end +end + + +describe("Plugin: " .. PLUGIN_NAME .. " (cache), ", function() + + describe("parse_cookies", function() + it("reads a name to values table", function() + local jar = utils.parse_cookies("sid=abc; _ga=GA1.2.3; consent=yes") + assert.same({ "abc" }, jar.sid) + assert.same({ "GA1.2.3" }, jar._ga) + assert.same({ "yes" }, jar.consent) + end) + + it("returns an empty table when there is no header", function() + assert.same({}, utils.parse_cookies(nil)) + end) + + it("keeps every occurrence of a repeated name, in order", function() + local jar = utils.parse_cookies("sid=first; other=x; sid=second") + assert.same({ "first", "second" }, jar.sid) + assert.same({ "x" }, jar.other) + end) + + it("trims surrounding spaces", function() + assert.same({ "abc" }, utils.parse_cookies(" sid = abc ").sid) + end) + end) + + describe("build_key", function() + it("ignores cookies that are not in cache_cookie_names", function() + local a = cache.build_key(conf(), "sid=abc; _ga=1", nil) + local b = cache.build_key(conf(), "sid=abc; _ga=999; theme=dark", nil) + assert.equal(a, b) + end) + + it("changes when the session changes", function() + local a = cache.build_key(conf(), "sid=abc", nil) + local b = cache.build_key(conf(), "sid=xyz", nil) + assert.not_equal(a, b) + end) + + it("separates two routes pointing at different auth servers", function() + local a = cache.build_key(conf(), "sid=abc", nil) + local b = cache.build_key(conf({ authn_url = "http://other/token" }), "sid=abc", nil) + assert.not_equal(a, b) + end) + + it("keys on the authorization header too", function() + local a = cache.build_key(conf(), nil, "Bearer one") + local b = cache.build_key(conf(), nil, "Bearer two") + assert.not_equal(a, b) + assert.not_nil(a) + end) + + it("returns nil when there is no credential, so anonymous requests never share an entry", function() + assert.is_nil(cache.build_key(conf(), "_ga=1; theme=dark", nil)) + assert.is_nil(cache.build_key(conf(), nil, nil)) + assert.is_nil(cache.build_key(conf(), "sid=", "")) + end) + + it("does not leak the session value into the key", function() + local key = cache.build_key(conf(), "sid=supersecretsession", nil) + assert.is_nil(key:find("supersecretsession", 1, true)) + end) + + it("two headers that authenticate differently cannot share a key", function() + -- frontier walks every cookie it is sent and acts on the last `sid` + -- that decodes. A stale undecodable cookie shared by everyone must + -- not collapse two users onto one entry, so every occurrence is in + -- the key. + local a = cache.build_key(conf(), "sid=USER_A; sid=STALE", nil) + local b = cache.build_key(conf(), "sid=USER_B; sid=STALE", nil) + assert.not_equal(a, b) + end) + + it("order of repeated cookies changes the key", function() + local a = cache.build_key(conf(), "sid=ONE; sid=TWO", nil) + local b = cache.build_key(conf(), "sid=TWO; sid=ONE", nil) + assert.not_equal(a, b) + end) + + it("includes the fields that decide which value is read out", function() + local base = cache.build_key(conf(), "sid=abc", nil) + assert.not_equal(base, cache.build_key(conf({ token_response_field = "accessToken" }), "sid=abc", nil)) + assert.not_equal(base, cache.build_key(conf({ header_name = "x-other" }), "sid=abc", nil)) + assert.not_equal(base, cache.build_key(conf({ http_method = "GET" }), "sid=abc", nil)) + end) + + it("a different cache_ttl is a different entry", function() + -- otherwise a route with a short window can be handed an entry a + -- neighbouring route cached for much longer + local a = cache.build_key(conf({ cache_ttl = 5 }), "sid=abc", nil) + local b = cache.build_key(conf({ cache_ttl = 2.5 }), "sid=abc", nil) + local c = cache.build_key(conf({ cache_ttl = 300 }), "sid=abc", nil) + assert.not_equal(a, b) + assert.not_equal(a, c) + assert.not_equal(b, c) + end) + + it("the order cookie names are listed in does not matter", function() + assert.equal( + cache.build_key(conf({ cache_cookie_names = { "sid", "other" } }), "sid=abc; other=1", nil), + cache.build_key(conf({ cache_cookie_names = { "other", "sid" } }), "sid=abc; other=1", nil)) + end) + end) + + describe("get", function() + it("asks the auth server once, then serves from redis", function() + reset() + local c = conf() + local fetch, fetched = auth_server("tok") + local key = cache.build_key(c, "sid=user-a", nil) + + assert.equal("tok", cache.get(c, key, fetch)) + for _ = 1, 5 do + assert.equal("tok", cache.get(c, key, fetch)) + end + + assert.equal(1, fetched()) + assert.equal(1, calls.set) + end) + + it("stores the token with the configured ttl", function() + reset() + local c = conf() + local fetch = auth_server("tok") + local key = cache.build_key(c, "sid=user-b", nil) + + cache.get(c, key, fetch) + assert.equal("tok", store[key].value) + assert.equal(5, store[key].ttl) + end) + + it("asks again once the entry has expired", function() + reset() + local c = conf() + local fetch, fetched = auth_server("tok") + local key = cache.build_key(c, "sid=user-c", nil) + + cache.get(c, key, fetch) + cache.get(c, key, fetch) + assert.equal(1, fetched()) + + -- let the entry age out the way redis would drop it + store[key].expires_at = ngx.now() - 1 + + cache.get(c, key, fetch) + assert.equal(2, fetched()) + end) + + it("does not store a failure", function() + reset() + local c = conf() + local fetch, fetched = auth_server(nil, "no dice") + local key = cache.build_key(c, "sid=rejected", nil) + + local token, err = cache.get(c, key, fetch) + assert.is_nil(token) + assert.equal("no dice", err) + + -- a second request asks again, so somebody who has just been + -- granted access is not locked out for the window + assert.is_nil(cache.get(c, key, fetch)) + assert.equal(2, fetched()) + assert.equal(0, calls.set) + end) + + it("does not touch redis when there is no credential to key on", function() + reset() + local c = conf() + local fetch, fetched = auth_server("tok") + + assert.equal("tok", cache.get(c, nil, fetch)) + assert.equal("tok", cache.get(c, nil, fetch)) + + assert.equal(2, fetched()) + assert.equal(0, calls.get) + assert.equal(0, calls.set) + end) + + it("goes straight to the auth server when redis is not configured", function() + reset() + local c = conf() + c.redis_host = nil + local fetch, fetched = auth_server("tok") + local key = cache.build_key(c, "sid=user-d", nil) + + assert.equal("tok", cache.get(c, key, fetch)) + assert.equal("tok", cache.get(c, key, fetch)) + + assert.equal(2, fetched()) + assert.equal(0, calls.get) + end) + + it("stores for exactly the configured ttl", function() + reset() + local c = conf() + local fetch = auth_server("tok") + local key = cache.build_key(c, "sid=plain-ttl", nil) + + cache.get(c, key, fetch) + assert.equal(c.cache_ttl, store[key].ttl) + end) + + it("stores an opaque token the same way", function() + -- the token is never parsed here, so one that is not a jwt at all + -- is stored and served like any other + reset() + local c = conf() + local fetch, fetched = auth_server("not-a-jwt") + local key = cache.build_key(c, "sid=opaque", nil) + + assert.equal("not-a-jwt", cache.get(c, key, fetch)) + assert.equal("not-a-jwt", cache.get(c, key, fetch)) + assert.equal(1, fetched()) + end) + + it("a redis read that raises falls through to the auth server", function() + reset() + raise_on.get = true + local c = conf() + local fetch, fetched = auth_server("tok") + + assert.equal("tok", cache.get(c, cache.build_key(c, "sid=user-e", nil), fetch)) + assert.equal(1, fetched()) + end) + + it("a redis write that raises does not fail the request", function() + reset() + raise_on.set = true + local c = conf() + local fetch = auth_server("tok") + + assert.equal("tok", cache.get(c, cache.build_key(c, "sid=user-f", nil), fetch)) + end) + end) +end) diff --git a/kong/plugins/spec/frontier-test/05-access_spec.lua b/kong/plugins/spec/frontier-test/05-access_spec.lua new file mode 100644 index 0000000..f68a362 --- /dev/null +++ b/kong/plugins/spec/frontier-test/05-access_spec.lua @@ -0,0 +1,226 @@ +local PLUGIN_NAME = "frontier" + +-- access.lua makes an http call and talks to kong, so both are stubbed here. +-- These tests cover what the plugin does with the token it gets back, which +-- matters more now that a token can come out of a shared redis. + +local function b64url(input) + return (ngx.encode_base64(input, true):gsub("%+", "-"):gsub("/", "_")) +end + +local function token_with_payload(payload) + return b64url('{"alg":"RS256","typ":"JWT"}') .. "." .. b64url(payload) .. ".sig" +end + +local function token_with_header(header) + return b64url(header) .. "." .. b64url('{"sub":"u1"}') .. ".sig" +end + +local function base_conf() + return { + disabled = false, + http_connect_timeout = 2000, + http_send_timeout = 2000, + http_read_timeout = 2000, + header_name = "x-user-token", + authn_url = "http://auth.test/AuthToken", + http_method = "POST", + token_response_field = "accessToken", + correlation_header_name = "X-Request-Id", + override_authz_header = false, + token_claims_to_append_as_headers = { "sub", "org_ids", "user_id" }, + frontier_header_prefix = "X-Frontier-", + request_organization_id_header = "X-Organization-Id", + verify_request_organization_id_header = false, + -- caching off, so these tests exercise the token handling only + cache_ttl = 0 + } +end + +-- runs the plugin against an auth server that hands back `token`, and reports +-- what reached the upstream. +local function run_plugin(conf, token, request_headers) + local result = { set = {}, cleared = {}, status = nil } + + package.loaded["resty.http"] = { + new = function() + return { + set_timeouts = function() end, + request_uri = function() + return { + status = 200, + headers = {}, + body = '{"' .. conf.token_response_field .. '":"' .. token .. '"}' + }, nil + end + } + end + } + + local exited = {} + + _G.kong = { + log = { + debug = function() end, + info = function() end, + warn = function() end, + err = function() end + }, + request = { + get_header = function(name) + return request_headers[string.lower(name)] + end, + get_headers = function() + return request_headers + end, + get_method = function() + return "GET" + end + }, + service = { + request = { + set_header = function(name, value) + -- kong itself rejects anything else, and rejecting it here + -- too is what makes a function value show up as a failure + local t = type(value) + if t ~= "string" and t ~= "number" and t ~= "boolean" then + error("invalid header value for " .. name .. ": got " .. t) + end + result.set[name] = value + end, + clear_header = function(name) + result.cleared[#result.cleared + 1] = name + end + } + }, + response = { + exit = function(status) + result.status = status + -- kong ends the request here, so nothing after it runs + error(exited) + end + } + } + + for _, mod in ipairs({ "access", "cache", "utils", "jwt_decoder" }) do + package.loaded["kong.plugins." .. PLUGIN_NAME .. "." .. mod] = nil + end + + local access = require("kong.plugins." .. PLUGIN_NAME .. ".access") + + local ok, err = pcall(access.run, conf) + if not ok and err ~= exited then + result.raised = err + end + + return result +end + + +describe("Plugin: " .. PLUGIN_NAME .. " (access), ", function() + describe("claims to headers", function() + it("appends the claims the token has", function() + local token = token_with_payload('{"sub":"u1","org_ids":"o1,o2"}') + local out = run_plugin(base_conf(), token, {}) + + assert.is_nil(out.raised) + assert.equal("u1", out.set["X-Frontier-sub"]) + assert.equal("o1,o2", out.set["X-Frontier-org_ids"]) + -- the token has no user_id, so no header is invented for it + assert.is_nil(out.set["X-Frontier-user_id"]) + assert.equal(token, out.set["x-user-token"]) + end) + + it("refuses a token whose payload is a json string", function() + -- indexing a lua string does not fail, it hands back the matching + -- function from the string library. `sub` is one of them and is in + -- the default claim list, so this used to set a header to a + -- function value and fail the request with a 500 + local out = run_plugin(base_conf(), token_with_payload('"just a string"'), {}) + + assert.is_nil(out.raised) + assert.equal(401, out.status) + assert.is_nil(out.set["X-Frontier-sub"]) + end) + + it("refuses a token whose payload is a number", function() + local out = run_plugin(base_conf(), token_with_payload("1"), {}) + + assert.is_nil(out.raised) + assert.equal(401, out.status) + end) + + it("passes a json array payload through with no claim headers", function() + -- an array is a table, so it cannot be told apart from an object + -- that simply has none of the configured claims, which is a real + -- case. It is forwarded with no identity headers rather than + -- refused, and the upstream sees a request that claims nothing + local out = run_plugin(base_conf(), token_with_payload("[1,2]"), {}) + + assert.is_nil(out.raised) + assert.is_nil(out.status) + assert.is_nil(out.set["X-Frontier-sub"]) + assert.is_nil(out.set["X-Frontier-org_ids"]) + end) + + it("refuses a token whose header is not an object", function() + -- jwt_parser reads header.alg without checking the header's type, + -- so this raised inside the decoder. A cached token can come from + -- anywhere with write access to the cache, so a raise here would + -- have been a 500 on a request carrying a valid credential. + for _, header in ipairs({ "1", "null", "true" }) do + local out = run_plugin(base_conf(), token_with_header(header), {}) + + assert.is_nil(out.raised) + assert.equal(401, out.status) + assert.is_nil(out.set["X-Frontier-sub"]) + end + end) + + it("refuses a token that does not decode at all", function() + local out = run_plugin(base_conf(), "not-a-jwt", {}) + + assert.is_nil(out.raised) + assert.equal(401, out.status) + end) + end) + + describe("organization id header", function() + it("keeps a header the token's org_ids claim allows", function() + local conf = base_conf() + conf.verify_request_organization_id_header = true + + local out = run_plugin(conf, token_with_payload('{"sub":"u1","org_ids":"o1,o2"}'), + { ["x-organization-id"] = "o2" }) + + assert.is_nil(out.raised) + assert.is_nil(out.status) + assert.same({}, out.cleared) + end) + + it("drops a header the token's org_ids claim does not allow", function() + local conf = base_conf() + conf.verify_request_organization_id_header = true + + local out = run_plugin(conf, token_with_payload('{"sub":"u1","org_ids":"o1"}'), + { ["x-organization-id"] = "other" }) + + assert.is_nil(out.raised) + assert.same({ "X-Organization-Id" }, out.cleared) + end) + + it("drops the header when the token has no org_ids claim", function() + -- a missing claim used to reach string.gmatch as nil and fail the + -- request with a 500. A claim we cannot read is one we cannot + -- verify against, so the header goes + local conf = base_conf() + conf.verify_request_organization_id_header = true + + local out = run_plugin(conf, token_with_payload('{"sub":"u1"}'), + { ["x-organization-id"] = "o1" }) + + assert.is_nil(out.raised) + assert.same({ "X-Organization-Id" }, out.cleared) + end) + end) +end)