From 9176508e92478513c28d0c52f0b62cfab958def1 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 9 Sep 2026 00:08:13 +0530 Subject: [PATCH 01/14] fix(access): set the http timeouts in the right order --- kong/plugins/frontier/access.lua | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kong/plugins/frontier/access.lua b/kong/plugins/frontier/access.lua index fc20821..271ea00 100644 --- a/kong/plugins/frontier/access.lua +++ b/kong/plugins/frontier/access.lua @@ -26,7 +26,8 @@ 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) + -- set_timeouts takes connect, send, read in that order + client:set_timeouts(conf.http_connect_timeout, conf.http_send_timeout, conf.http_read_timeout) return client end From 0fb1f0094103565f285a5255015dcf257581ba08 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 9 Sep 2026 00:08:13 +0530 Subject: [PATCH 02/14] fix(jwt): stop a malformed token from raising in the decoder --- kong/plugins/frontier/jwt_decoder.lua | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/kong/plugins/frontier/jwt_decoder.lua b/kong/plugins/frontier/jwt_decoder.lua index 82004b3..ec338c2 100644 --- a/kong/plugins/frontier/jwt_decoder.lua +++ b/kong/plugins/frontier/jwt_decoder.lua @@ -4,7 +4,16 @@ 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) + -- pcall'd because jwt_parser reads the decoded header without checking its + -- type, so a token whose header segment is valid json but not an object + -- raises. A cached token can come from anywhere with write access to the + -- cache, so nothing here can assume the token is well formed. + local ok, jwt, err = pcall(jwt_decoder.new, jwt_decoder, token) + + if not ok then + ngx.log(ngx.STDERR, jwt) + return nil, "could not decode token" + end if err then ngx.log(ngx.STDERR, err) @@ -14,4 +23,4 @@ function _M.decode_token(token) return jwt, nil end -return _M \ No newline at end of file +return _M From 61383efdaf584274905fd5236b4e15b182790187 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 9 Sep 2026 00:08:13 +0530 Subject: [PATCH 03/14] fix(access): refuse a token the plugin cannot read claims from --- kong/plugins/frontier/access.lua | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/kong/plugins/frontier/access.lua b/kong/plugins/frontier/access.lua index 271ea00..6c7d1c3 100644 --- a/kong/plugins/frontier/access.lua +++ b/kong/plugins/frontier/access.lua @@ -187,6 +187,14 @@ local function append_claims_as_headers(conf, user_token) local claims = jwt.claims + -- A payload only has to be valid json, so it can decode to a string. In lua + -- that still indexes: `claims.sub` would hand back string.sub, and setting + -- a header to a function fails the request with a 500. + if type(claims) ~= "table" 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] @@ -209,12 +217,16 @@ 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 + -- a claim we cannot read is one we cannot verify against, so the header + -- gets dropped rather than raising in gmatch 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 From 3dced358e02d6735d57e5e037461f0348ede9e28 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 9 Sep 2026 00:08:13 +0530 Subject: [PATCH 04/14] feat(cache): cache the frontier authn token call in redis --- kong-plugin-frontier-0.1.1-1.rockspec | 2 + kong/plugins/frontier/access.lua | 53 ++++++- kong/plugins/frontier/cache.lua | 131 +++++++++++++++++ kong/plugins/frontier/redis.lua | 193 ++++++++++++++++++++++++++ kong/plugins/frontier/schema.lua | 118 +++++++++++++++- kong/plugins/frontier/utils.lua | 39 ++++++ 6 files changed, 527 insertions(+), 9 deletions(-) create mode 100644 kong/plugins/frontier/cache.lua create mode 100644 kong/plugins/frontier/redis.lua 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 6c7d1c3..94c86fa 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" @@ -31,8 +32,10 @@ local function get_http_client(conf) 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) +-- Sends a request to the auth server and gets a user token back for the +-- cookies. Failures come back as `nil, err, upstream_status`, so the caller +-- decides how to end the request instead of this doing it. +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) @@ -60,13 +63,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) @@ -90,6 +91,46 @@ 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 + +-- verifies user identity, using the cache when it is turned on +local function check_request_identity(conf, cookies, bearer) + -- set by the fetch below, read only when there is no token to return + local upstream_status + + local function fetch() + local token, err, status = fetch_identity_token(conf, cookies, bearer) + upstream_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 upstream_status then + return kong.response.exit(ngx.HTTP_UNAUTHORIZED, unauthorized_response, { + ["x-upstream-status"] = upstream_status + }) + end + + return fail_auth() + end + return token end diff --git a/kong/plugins/frontier/cache.lua b/kong/plugins/frontier/cache.lua new file mode 100644 index 0000000..16dc1a2 --- /dev/null +++ b/kong/plugins/frontier/cache.lua @@ -0,0 +1,131 @@ +local _M = {} + +local jwt_decoder = require "kong.plugins.frontier.jwt_decoder" +local redis = require "kong.plugins.frontier.redis" +local utils = require "kong.plugins.frontier.utils" + +local kong = kong +local ngx = ngx +local pcall = pcall +local concat = table.concat +local ipairs = ipairs +local sort = table.sort +local type = type +local tonumber = tonumber +local hash = utils.hash + +-- Builds the key for the credential being exchanged. Only the cookies named in +-- conf.cache_cookie_names go in; the rest change too often to key on. Returns +-- nil when there is no credential, so anonymous requests never share an entry. +function _M.build_key(conf, cookies, bearer) + local jar = utils.parse_cookies(cookies) + + -- sorted, so the order the names are listed in does not matter + local names = {} + for _, name in ipairs(conf.cache_cookie_names or {}) do + names[#names + 1] = name + end + sort(names) + + local has_credential = false + + -- everything that changes what the entry means + local parts = { + conf.authn_url or "", + conf.http_method or "", + conf.header_name or "", + conf.token_response_field or "", + tostring(conf.cache_ttl), + tostring(conf.cache_exp_skew) + } + + for _, name in ipairs(names) do + local values = jar[name] + + -- Every occurrence goes in. Frontier acts on the last `sid` that + -- decodes, so taking one would let two users hash to the same key. + if values then + for _, value in ipairs(values) do + if value ~= "" then + has_credential = true + end + parts[#parts + 1] = name .. "=" .. value + end + else + parts[#parts + 1] = name .. "=" + end + end + + if bearer and bearer ~= "" then + has_credential = true + parts[#parts + 1] = bearer + else + parts[#parts + 1] = "" + end + + if not has_credential then + return nil + end + + -- hashed, so no session sits in redis as a plaintext key + return hash(concat(parts, "\0")) +end + +-- How long the entry may live, in seconds. Zero or less means do not store it. +-- Clamped to the token's own expiry minus cache_exp_skew, and nothing else sets +-- the expiry, so a token still in redis has at least the skew left on it. +function _M.ttl_for(conf, token) + local ttl = conf.cache_ttl + + local jwt = jwt_decoder.decode_token(token) + local claims = jwt and jwt.claims + local exp = type(claims) == "table" and tonumber(claims.exp) or nil + + if exp then + -- ngx.now(), not ngx.time(): whole seconds round down, which would let + -- an entry outlive its token when cache_exp_skew is 0 + local remaining = exp - ngx.now() - conf.cache_exp_skew + if remaining < ttl then + ttl = remaining + end + end + + return ttl +end + +-- Resolves the token: redis first, then the auth server through `fetch`. Redis +-- is a cache and not an authority, so any problem with it falls through too. +function _M.get(conf, key, fetch) + -- no credential to key on, or no redis to key it in + if not key or not redis.enabled(conf) then + return fetch() + end + + -- pcall'd so redis cannot fail a request even by raising + local ok, cached = pcall(redis.get, conf, key) + + if not ok then + kong.log.warn("redis lookup raised, ignoring it: ", cached) + elseif cached then + kong.log.debug("token served from redis") + return cached + end + + local token, err = fetch() + if not token then + return nil, err + end + + local ttl = _M.ttl_for(conf, token) + + if ttl > 0 then + local set_ok, set_err = pcall(redis.set, conf, key, token, ttl) + if not set_ok then + kong.log.warn("redis write raised, ignoring it: ", set_err) + end + end + + return token +end + +return _M diff --git a/kong/plugins/frontier/redis.lua b/kong/plugins/frontier/redis.lua new file mode 100644 index 0000000..1c21713 --- /dev/null +++ b/kong/plugins/frontier/redis.lua @@ -0,0 +1,193 @@ +local _M = {} + +local resty_redis = require "resty.redis" +local utils = require "kong.plugins.frontier.utils" + +local kong = kong +local ngx = ngx +local fmt = string.format +local math_floor = math.floor +local tonumber = tonumber + +-- Redis is a cache here, not an authority, so nothing in this file fails a +-- request. Every problem returns nil and the caller falls through. +-- +-- A worker whose command fails stops trying that instance for +-- redis_breaker_seconds, so an outage cannot make every request pay the +-- timeout. Keyed per instance. A rejected password or database does not trip +-- it, being a config mistake rather than a fault. +local breaker_until = {} + +-- memoised per conf table, so the password is hashed once and not per command +local ids = setmetatable({}, { __mode = "k" }) + +-- Identifies one instance, and is also the pool name. A pooled connection skips +-- authentication, so anything that changes what a connection means belongs in +-- here. The password is hashed so it cannot reach a log. +local function instance_id(conf) + local id = ids[conf] + + if not id then + local secret = "" + if conf.redis_password and conf.redis_password ~= "" then + secret = utils.hash(conf.redis_password) + end + + id = fmt("frontier:%s:%d:%d:%s:%s:%s:%s:%s", + conf.redis_host, + conf.redis_port, + conf.redis_database, + conf.redis_username or "", + secret, + conf.redis_ssl and "s" or "p", + conf.redis_ssl_verify and "v" or "n", + conf.redis_server_name or "") + + ids[conf] = id + end + + return id +end + +local function connection_options(conf) + return { + ssl = conf.redis_ssl, + ssl_verify = conf.redis_ssl_verify, + server_name = conf.redis_server_name, + pool = instance_id(conf) + } +end + +local function breaker_is_open(conf) + local until_when = breaker_until[instance_id(conf)] + return until_when ~= nil and ngx.now() < until_when +end + +local function trip_breaker(conf, action, err) + breaker_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 get_connection(conf) + local red = resty_redis:new() + red:set_timeouts(conf.redis_timeout, conf.redis_timeout, conf.redis_timeout) + + local ok, err = red:connect(conf.redis_host, conf.redis_port, connection_options(conf)) + if not ok then + trip_breaker(conf, "connect", err) + return nil + end + + -- a pooled connection has already authenticated and selected its database + local reused, reuse_err = red:get_reused_times() + if reuse_err then + trip_breaker(conf, "get_reused_times", reuse_err) + red:close() + return nil + end + + -- Redis answering and refusing is a config problem, not an unreachable + -- instance, so neither of these trips the breaker. + if reused == 0 then + if conf.redis_password and conf.redis_password ~= "" then + local auth_ok, auth_err + if conf.redis_username and conf.redis_username ~= "" then + auth_ok, auth_err = red:auth(conf.redis_username, conf.redis_password) + else + auth_ok, auth_err = red:auth(conf.redis_password) + end + if not auth_ok then + kong.log.warn("redis refused the credentials for ", + conf.redis_host, ":", conf.redis_port, ": ", auth_err) + red:close() + return nil + end + end + + if conf.redis_database ~= 0 then + local sel_ok, sel_err = red:select(conf.redis_database) + if not sel_ok then + kong.log.warn("redis rejected database ", conf.redis_database, + " on ", conf.redis_host, ":", conf.redis_port, ": ", sel_err) + red:close() + return nil + end + end + end + + return red +end + +local function release(conf, red) + local ok, err = red:set_keepalive(conf.redis_keepalive_ms, conf.redis_pool_size) + if not ok then + kong.log.debug("failed to return redis connection to the pool: ", err) + red:close() + end +end + +-- Returns the stored value, or nil for a miss, a failure or a tripped breaker. +function _M.get(conf, key) + if breaker_is_open(conf) then + return nil + end + + local red = get_connection(conf) + if not red then + return nil + end + + local value, err = red:get(conf.redis_key_prefix .. key) + + if not value then + trip_breaker(conf, "get", err) + red:close() + return nil + end + + release(conf, red) + + -- ngx.null is redis saying the key is not there + if value == ngx.null or value == "" then + return nil + end + + return value +end + +-- Stores the value with an expiry. A failure is logged and ignored, because the +-- token in hand is still good to use. +function _M.set(conf, key, value, ttl) + if breaker_is_open(conf) then + return + end + + -- milliseconds, so a fractional cache_ttl survives. SETEX takes whole + -- seconds and redis rejects a fractional argument. + local px = math_floor((tonumber(ttl) or 0) * 1000) + + if px <= 0 then + return + end + + local red = get_connection(conf) + if not red then + return + end + + local ok, err = red:set(conf.redis_key_prefix .. key, value, "PX", px) + if not ok then + kong.log.warn("redis rejected the write: ", err) + red:close() + return + end + + release(conf, red) +end + +return _M diff --git a/kong/plugins/frontier/schema.lua b/kong/plugins/frontier/schema.lua index 37a53f7..7e933cf 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,111 @@ 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. + cache_ttl = { + type = "number", + default = 5, + between = { 0, 3600 } + } + }, { + -- 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" + } + } + }, { + -- seconds of clock skew allowed when clamping the cache ttl to + -- the token expiry + cache_exp_skew = { + type = "number", + default = 2, + between = { 0, 300 } + } + }, { + -- 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 } + } + }, { + redis_keepalive_ms = { + type = "number", + default = 60000, + between = { 0, 3600000 } + } + }, { + redis_pool_size = { + type = "number", + default = 30, + between = { 1, 1000 } + } }, { rule = { type = "record", diff --git a/kong/plugins/frontier/utils.lua b/kong/plugins/frontier/utils.lua index 59ae6b7..c7b1fe8 100644 --- a/kong/plugins/frontier/utils.lua +++ b/kong/plugins/frontier/utils.lua @@ -1,5 +1,19 @@ local _M = {} +local resty_sha256 = require "resty.sha256" + +local encode_base64 = ngx.encode_base64 + +-- resty.sha256 rather than kong.tools.sha256, which does not exist before 3.6. +-- One instance per worker, safe because nothing yields between reset and final. +local sha256 = resty_sha256:new() + +function _M.hash(input) + sha256:reset() + sha256:update(input) + return (encode_base64(sha256:final(), true):gsub("+", "-"):gsub("/", "_")) +end + -- splits a string s using a delimiter and returns a table -- containing the resulting substrings function _M.split(s, delimiter) @@ -15,4 +29,29 @@ function _M.ltrim(s) return s:match'^%s*(.*)' end +-- Parses a cookie header into a table of name to list of values, in order. A +-- name can legitimately appear more than once, so every value is kept: callers +-- cannot guess which one the auth server will act on. +function _M.parse_cookies(cookie_header) + local jar = {} + + if not cookie_header then + return jar + end + + for pair in cookie_header:gmatch("[^;]+") do + local name, value = pair:match("^%s*([^=%s]+)%s*=%s*(.-)%s*$") + if name then + local values = jar[name] + if values then + values[#values + 1] = value + else + jar[name] = { value } + end + end + end + + return jar +end + return _M From 18796364fdbc227a8fdf9d7c12c72e08c7578c6c Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 9 Sep 2026 00:08:13 +0530 Subject: [PATCH 05/14] test: cover the cache and the token handling --- .../spec/frontier-test/01-schema_spec.lua | 28 ++ .../spec/frontier-test/04-cache_spec.lua | 420 ++++++++++++++++++ .../spec/frontier-test/05-access_spec.lua | 226 ++++++++++ 3 files changed, 674 insertions(+) create mode 100644 kong/plugins/spec/frontier-test/04-cache_spec.lua create mode 100644 kong/plugins/spec/frontier-test/05-access_spec.lua diff --git a/kong/plugins/spec/frontier-test/01-schema_spec.lua b/kong/plugins/spec/frontier-test/01-schema_spec.lua index 209980f..1f62f2c 100644 --- a/kong/plugins/spec/frontier-test/01-schema_spec.lua +++ b/kong/plugins/spec/frontier-test/01-schema_spec.lua @@ -9,4 +9,32 @@ 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.equal(2, ok.config.cache_exp_skew) + 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..d32a74a --- /dev/null +++ b/kong/plugins/spec/frontier-test/04-cache_spec.lua @@ -0,0 +1,420 @@ +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 jwt_parser = require "kong.plugins.jwt.jwt_parser" +local pkey = require "resty.openssl.pkey" + +local signing_key = assert(pkey.new({ type = "RSA", bits = 2048 })) + +local function token_expiring_in(seconds) + return assert(jwt_parser.encode({ + sub = "u1", + exp = ngx.time() + seconds + }, signing_key:to_PEM("private"), "RS256")) +end + +local function b64(input) + return (ngx.encode_base64(input, true):gsub("%+", "-"):gsub("/", "_")) +end + +local function conf(overrides) + local c = { + authn_url = "http://frontier/v1beta1/auth/token", + cache_ttl = 5, + cache_cookie_names = { "sid" }, + cache_exp_skew = 2, + 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, + redis_keepalive_ms = 60000, + redis_pool_size = 30 + } + 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 = 3600 }), "sid=abc", nil) + assert.not_equal(a, b) + assert.not_equal(a, c) + assert.not_equal(b, c) + end) + + it("a different cache_exp_skew is a different entry", function() + assert.not_equal( + cache.build_key(conf({ cache_exp_skew = 2 }), "sid=abc", nil), + cache.build_key(conf({ cache_exp_skew = 30 }), "sid=abc", nil)) + 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("does not store a token with nothing left after the skew", function() + reset() + local c = conf() + -- exp is cache_exp_skew away, so the clamp leaves nothing + local token = token_expiring_in(c.cache_exp_skew) + assert.is_true(cache.ttl_for(c, token) <= 0) + + local fetch, fetched = auth_server(token) + local key = cache.build_key(c, "sid=nearly-dead", nil) + + assert.equal(token, cache.get(c, key, fetch)) + assert.equal(token, cache.get(c, key, fetch)) + + assert.equal(2, fetched()) + assert.equal(0, calls.set) + end) + + it("does not store an already expired token", function() + reset() + local c = conf() + local fetch = auth_server(token_expiring_in(-60)) + + cache.get(c, cache.build_key(c, "sid=expired", nil), fetch) + assert.equal(0, calls.set) + end) + + it("stores with the expiry clamped to the token", function() + reset() + local c = conf() + local fetch = auth_server(token_expiring_in(4)) + local key = cache.build_key(c, "sid=short-lived", nil) + + cache.get(c, key, fetch) + -- about 4 - 2 = 2, to the fraction + assert.is_true(store[key].ttl > 1 and store[key].ttl <= 2) + 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) + + describe("ttl_for", function() + it("uses the configured ttl for an opaque token", function() + assert.equal(5, cache.ttl_for(conf(), "not-a-jwt")) + end) + + it("keeps the configured ttl when the token outlives it", function() + assert.equal(5, cache.ttl_for(conf(), token_expiring_in(10))) + end) + + it("clamps to the token expiry, minus the skew", function() + -- sub second, because the clamp works to the fraction so an entry + -- cannot outlive the token it holds + local ttl = cache.ttl_for(conf(), token_expiring_in(4)) + assert.is_true(ttl > 1 and ttl <= 2) + end) + + it("is not positive for a token that is already gone", function() + assert.is_true(cache.ttl_for(conf(), token_expiring_in(-60)) <= 0) + end) + end) + + describe("hostile input", function() + -- anything with write access to the cache can put a value in it, so a + -- malformed entry must not raise. Under the old node cache a raise came + -- back as a cache error and turned into a 401 for a valid credential. + local function jwt_with(header, payload) + return b64(header) .. "." .. b64(payload) .. ".signature" + end + + it("a payload that is not an object does not raise", function() + for _, payload in ipairs({ "1", "null", '"a string"', "[1,2]", "{}", "not json" }) do + local ok, ttl = pcall(cache.ttl_for, conf(), jwt_with('{"alg":"RS256"}', payload)) + assert.is_true(ok) + assert.is_true(type(ttl) == "number") + end + end) + + it("a header that is not an object does not raise", function() + -- jwt_parser reads header.alg without checking the header's type, + -- so these raise inside it + for _, header in ipairs({ "1", "null", "true", '"a string"', "[1]", "{}" }) do + local ok, ttl = pcall(cache.ttl_for, conf(), jwt_with(header, '{"sub":"u1"}')) + assert.is_true(ok) + assert.is_true(type(ttl) == "number") + end + end) + + it("a token that cannot be read is still usable, just not trusted for its expiry", function() + assert.equal(5, cache.ttl_for(conf(), jwt_with("1", '{"exp":1}'))) + assert.equal(5, cache.ttl_for(conf(), "not-a-jwt")) + 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) From c037ab65f67dfa95cd6dd42c848e3735cc7c74b4 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 9 Sep 2026 00:08:13 +0530 Subject: [PATCH 06/14] docs: document the token cache --- README.md | 150 +++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 149 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 6a49d80..18bbfab 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,155 @@ 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 | +| `cache_cookie_names` | `["sid"]` | Only these cookies go into the cache key | +| `cache_exp_skew` | `2` | Clock skew allowed when clamping the ttl to the token expiry | +| `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 | +| `redis_keepalive_ms` | `60000` | How long a pooled connection is kept | +| `redis_pool_size` | `30` | Connections kept per worker | + +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`, + `cache_ttl` and `cache_exp_skew`. 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 expiry is clamped to the token's own `exp`, minus `cache_exp_skew`. + That is the only thing that sets the expiry, so a token still in redis always + has at least the skew left on it and an expired token is never handed to the + upstream. +- 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 From 9dc108054e590c905be14fb5e2fb249aab85e513 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Fri, 11 Sep 2026 20:01:44 +0530 Subject: [PATCH 07/14] refactor(redis): name the connection pool without hashing the password --- kong/plugins/frontier/redis.lua | 67 ++++++++++++--------------------- 1 file changed, 24 insertions(+), 43 deletions(-) diff --git a/kong/plugins/frontier/redis.lua b/kong/plugins/frontier/redis.lua index 1c21713..49aa927 100644 --- a/kong/plugins/frontier/redis.lua +++ b/kong/plugins/frontier/redis.lua @@ -1,7 +1,6 @@ local _M = {} local resty_redis = require "resty.redis" -local utils = require "kong.plugins.frontier.utils" local kong = kong local ngx = ngx @@ -9,6 +8,11 @@ local fmt = string.format local math_floor = math.floor local tonumber = tonumber +-- how long an idle connection is kept, and how many per worker. The bundled +-- rate limiting plugin hardcodes the same shape of numbers. +local KEEPALIVE_MS = 60000 +local POOL_SIZE = 30 + -- Redis is a cache here, not an authority, so nothing in this file fails a -- request. Every problem returns nil and the caller falls through. -- @@ -18,44 +22,16 @@ local tonumber = tonumber -- it, being a config mistake rather than a fault. local breaker_until = {} --- memoised per conf table, so the password is hashed once and not per command -local ids = setmetatable({}, { __mode = "k" }) - --- Identifies one instance, and is also the pool name. A pooled connection skips --- authentication, so anything that changes what a connection means belongs in --- here. The password is hashed so it cannot reach a log. +-- Names the pool openresty keeps the connection in, and identifies the instance +-- for the breaker. A pooled connection has already authenticated and selected +-- its database, so anything that changes what a connection means belongs here. local function instance_id(conf) - local id = ids[conf] - - if not id then - local secret = "" - if conf.redis_password and conf.redis_password ~= "" then - secret = utils.hash(conf.redis_password) - end - - id = fmt("frontier:%s:%d:%d:%s:%s:%s:%s:%s", - conf.redis_host, - conf.redis_port, - conf.redis_database, - conf.redis_username or "", - secret, - conf.redis_ssl and "s" or "p", - conf.redis_ssl_verify and "v" or "n", - conf.redis_server_name or "") - - ids[conf] = id - end - - return id -end - -local function connection_options(conf) - return { - ssl = conf.redis_ssl, - ssl_verify = conf.redis_ssl_verify, - server_name = conf.redis_server_name, - pool = 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 breaker_is_open(conf) @@ -77,7 +53,12 @@ local function get_connection(conf) local red = resty_redis:new() red:set_timeouts(conf.redis_timeout, conf.redis_timeout, conf.redis_timeout) - local ok, err = red:connect(conf.redis_host, conf.redis_port, connection_options(conf)) + local ok, 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 ok then trip_breaker(conf, "connect", err) return nil @@ -123,8 +104,8 @@ local function get_connection(conf) return red end -local function release(conf, red) - local ok, err = red:set_keepalive(conf.redis_keepalive_ms, conf.redis_pool_size) +local function release(red) + local ok, err = red:set_keepalive(KEEPALIVE_MS, POOL_SIZE) if not ok then kong.log.debug("failed to return redis connection to the pool: ", err) red:close() @@ -150,7 +131,7 @@ function _M.get(conf, key) return nil end - release(conf, red) + release(red) -- ngx.null is redis saying the key is not there if value == ngx.null or value == "" then @@ -187,7 +168,7 @@ function _M.set(conf, key, value, ttl) return end - release(conf, red) + release(red) end return _M From ee3345add81321f1723cc0f22366ecf31f75efa1 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Fri, 11 Sep 2026 20:01:44 +0530 Subject: [PATCH 08/14] refactor(schema): drop the redis keepalive and pool size knobs --- kong/plugins/frontier/schema.lua | 12 ------------ kong/plugins/spec/frontier-test/04-cache_spec.lua | 4 +--- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/kong/plugins/frontier/schema.lua b/kong/plugins/frontier/schema.lua index 7e933cf..9d54a9d 100644 --- a/kong/plugins/frontier/schema.lua +++ b/kong/plugins/frontier/schema.lua @@ -194,18 +194,6 @@ local schema = { default = 10, between = { 0, 600 } } - }, { - redis_keepalive_ms = { - type = "number", - default = 60000, - between = { 0, 3600000 } - } - }, { - redis_pool_size = { - type = "number", - default = 30, - between = { 1, 1000 } - } }, { rule = { type = "record", diff --git a/kong/plugins/spec/frontier-test/04-cache_spec.lua b/kong/plugins/spec/frontier-test/04-cache_spec.lua index d32a74a..fcd0a3f 100644 --- a/kong/plugins/spec/frontier-test/04-cache_spec.lua +++ b/kong/plugins/spec/frontier-test/04-cache_spec.lua @@ -83,9 +83,7 @@ local function conf(overrides) redis_timeout = 100, redis_database = 0, redis_key_prefix = "frontier:authn:test:", - redis_breaker_seconds = 10, - redis_keepalive_ms = 60000, - redis_pool_size = 30 + redis_breaker_seconds = 10 } for k, val in pairs(overrides or {}) do c[k] = val From fb791961f2d35e3c706b9fe6292e0c5228dc1b55 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Fri, 11 Sep 2026 20:01:44 +0530 Subject: [PATCH 09/14] docs: explain how redis connections are pooled --- README.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 18bbfab..50599dc 100644 --- a/README.md +++ b/README.md @@ -109,8 +109,11 @@ plugins: | `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 | -| `redis_keepalive_ms` | `60000` | How long a pooled connection is kept | -| `redis_pool_size` | `30` | Connections kept per worker | + +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 From 550c5d55722fc92dcbf49bb7dd65d7f244c7ba75 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Fri, 11 Sep 2026 21:16:41 +0530 Subject: [PATCH 10/14] refactor(cache): store for the configured ttl without parsing the token --- kong/plugins/frontier/cache.lua | 37 +++++--------------------------- kong/plugins/frontier/schema.lua | 16 ++++++-------- 2 files changed, 12 insertions(+), 41 deletions(-) diff --git a/kong/plugins/frontier/cache.lua b/kong/plugins/frontier/cache.lua index 16dc1a2..46c734c 100644 --- a/kong/plugins/frontier/cache.lua +++ b/kong/plugins/frontier/cache.lua @@ -1,17 +1,13 @@ local _M = {} -local jwt_decoder = require "kong.plugins.frontier.jwt_decoder" local redis = require "kong.plugins.frontier.redis" local utils = require "kong.plugins.frontier.utils" local kong = kong -local ngx = ngx local pcall = pcall local concat = table.concat local ipairs = ipairs local sort = table.sort -local type = type -local tonumber = tonumber local hash = utils.hash -- Builds the key for the credential being exchanged. Only the cookies named in @@ -35,8 +31,7 @@ function _M.build_key(conf, cookies, bearer) conf.http_method or "", conf.header_name or "", conf.token_response_field or "", - tostring(conf.cache_ttl), - tostring(conf.cache_exp_skew) + tostring(conf.cache_ttl) } for _, name in ipairs(names) do @@ -71,28 +66,6 @@ function _M.build_key(conf, cookies, bearer) return hash(concat(parts, "\0")) end --- How long the entry may live, in seconds. Zero or less means do not store it. --- Clamped to the token's own expiry minus cache_exp_skew, and nothing else sets --- the expiry, so a token still in redis has at least the skew left on it. -function _M.ttl_for(conf, token) - local ttl = conf.cache_ttl - - local jwt = jwt_decoder.decode_token(token) - local claims = jwt and jwt.claims - local exp = type(claims) == "table" and tonumber(claims.exp) or nil - - if exp then - -- ngx.now(), not ngx.time(): whole seconds round down, which would let - -- an entry outlive its token when cache_exp_skew is 0 - local remaining = exp - ngx.now() - conf.cache_exp_skew - if remaining < ttl then - ttl = remaining - end - end - - return ttl -end - -- Resolves the token: redis first, then the auth server through `fetch`. Redis -- is a cache and not an authority, so any problem with it falls through too. function _M.get(conf, key, fetch) @@ -116,10 +89,10 @@ function _M.get(conf, key, fetch) return nil, err end - local ttl = _M.ttl_for(conf, token) - - if ttl > 0 then - local set_ok, set_err = pcall(redis.set, conf, key, token, ttl) + -- cache_ttl as configured. The token is not read for its expiry, so + -- cache_ttl must stay well under the auth server's token lifetime. + if conf.cache_ttl > 0 then + local set_ok, set_err = pcall(redis.set, conf, key, token, conf.cache_ttl) if not set_ok then kong.log.warn("redis write raised, ignoring it: ", set_err) end diff --git a/kong/plugins/frontier/schema.lua b/kong/plugins/frontier/schema.lua index 9d54a9d..240b64d 100644 --- a/kong/plugins/frontier/schema.lua +++ b/kong/plugins/frontier/schema.lua @@ -107,10 +107,15 @@ local schema = { -- 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, 3600 } + between = { 0, 300 } } }, { -- only these cookies go into the cache key. Browsers send many @@ -124,14 +129,7 @@ local schema = { type = "string" } } - }, { - -- seconds of clock skew allowed when clamping the cache ttl to - -- the token expiry - cache_exp_skew = { - type = "number", - default = 2, - between = { 0, 300 } - } + }, { -- setting a host turns caching on. Leave it unset and every -- request goes to the auth server. From 0535ec83ec9a493ae7bab2f57581533f3d3853c7 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Fri, 11 Sep 2026 21:16:41 +0530 Subject: [PATCH 11/14] test: drop the ttl clamp cases the cache no longer has --- .../spec/frontier-test/01-schema_spec.lua | 1 - .../spec/frontier-test/04-cache_spec.lua | 117 +++--------------- 2 files changed, 14 insertions(+), 104 deletions(-) diff --git a/kong/plugins/spec/frontier-test/01-schema_spec.lua b/kong/plugins/spec/frontier-test/01-schema_spec.lua index 1f62f2c..3835d94 100644 --- a/kong/plugins/spec/frontier-test/01-schema_spec.lua +++ b/kong/plugins/spec/frontier-test/01-schema_spec.lua @@ -16,7 +16,6 @@ describe("Plugin: " .. PLUGIN_NAME .. " (schema), ", function() }, schema_def)) assert.equal(5, ok.config.cache_ttl) - assert.equal(2, ok.config.cache_exp_skew) assert.same({ "sid" }, ok.config.cache_cookie_names) end) diff --git a/kong/plugins/spec/frontier-test/04-cache_spec.lua b/kong/plugins/spec/frontier-test/04-cache_spec.lua index fcd0a3f..8654945 100644 --- a/kong/plugins/spec/frontier-test/04-cache_spec.lua +++ b/kong/plugins/spec/frontier-test/04-cache_spec.lua @@ -56,28 +56,12 @@ package.loaded["kong.plugins." .. PLUGIN_NAME .. ".redis"] = { local cache = require("kong.plugins."..PLUGIN_NAME..".cache") local utils = require("kong.plugins."..PLUGIN_NAME..".utils") -local jwt_parser = require "kong.plugins.jwt.jwt_parser" -local pkey = require "resty.openssl.pkey" - -local signing_key = assert(pkey.new({ type = "RSA", bits = 2048 })) - -local function token_expiring_in(seconds) - return assert(jwt_parser.encode({ - sub = "u1", - exp = ngx.time() + seconds - }, signing_key:to_PEM("private"), "RS256")) -end - -local function b64(input) - return (ngx.encode_base64(input, true):gsub("%+", "-"):gsub("/", "_")) -end local function conf(overrides) local c = { authn_url = "http://frontier/v1beta1/auth/token", cache_ttl = 5, cache_cookie_names = { "sid" }, - cache_exp_skew = 2, redis_host = "127.0.0.1", redis_port = 6379, redis_timeout = 100, @@ -199,18 +183,12 @@ describe("Plugin: " .. PLUGIN_NAME .. " (cache), ", function() -- 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 = 3600 }), "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("a different cache_exp_skew is a different entry", function() - assert.not_equal( - cache.build_key(conf({ cache_exp_skew = 2 }), "sid=abc", nil), - cache.build_key(conf({ cache_exp_skew = 30 }), "sid=abc", nil)) - 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), @@ -306,41 +284,27 @@ describe("Plugin: " .. PLUGIN_NAME .. " (cache), ", function() assert.equal(0, calls.get) end) - it("does not store a token with nothing left after the skew", function() + it("stores for exactly the configured ttl", function() reset() local c = conf() - -- exp is cache_exp_skew away, so the clamp leaves nothing - local token = token_expiring_in(c.cache_exp_skew) - assert.is_true(cache.ttl_for(c, token) <= 0) - - local fetch, fetched = auth_server(token) - local key = cache.build_key(c, "sid=nearly-dead", nil) - - assert.equal(token, cache.get(c, key, fetch)) - assert.equal(token, cache.get(c, key, fetch)) - - assert.equal(2, fetched()) - assert.equal(0, calls.set) - end) - - it("does not store an already expired token", function() - reset() - local c = conf() - local fetch = auth_server(token_expiring_in(-60)) + local fetch = auth_server("tok") + local key = cache.build_key(c, "sid=plain-ttl", nil) - cache.get(c, cache.build_key(c, "sid=expired", nil), fetch) - assert.equal(0, calls.set) + cache.get(c, key, fetch) + assert.equal(c.cache_ttl, store[key].ttl) end) - it("stores with the expiry clamped to the token", function() + 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 = auth_server(token_expiring_in(4)) - local key = cache.build_key(c, "sid=short-lived", nil) + local fetch, fetched = auth_server("not-a-jwt") + local key = cache.build_key(c, "sid=opaque", nil) - cache.get(c, key, fetch) - -- about 4 - 2 = 2, to the fraction - assert.is_true(store[key].ttl > 1 and store[key].ttl <= 2) + 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() @@ -362,57 +326,4 @@ describe("Plugin: " .. PLUGIN_NAME .. " (cache), ", function() assert.equal("tok", cache.get(c, cache.build_key(c, "sid=user-f", nil), fetch)) end) end) - - describe("ttl_for", function() - it("uses the configured ttl for an opaque token", function() - assert.equal(5, cache.ttl_for(conf(), "not-a-jwt")) - end) - - it("keeps the configured ttl when the token outlives it", function() - assert.equal(5, cache.ttl_for(conf(), token_expiring_in(10))) - end) - - it("clamps to the token expiry, minus the skew", function() - -- sub second, because the clamp works to the fraction so an entry - -- cannot outlive the token it holds - local ttl = cache.ttl_for(conf(), token_expiring_in(4)) - assert.is_true(ttl > 1 and ttl <= 2) - end) - - it("is not positive for a token that is already gone", function() - assert.is_true(cache.ttl_for(conf(), token_expiring_in(-60)) <= 0) - end) - end) - - describe("hostile input", function() - -- anything with write access to the cache can put a value in it, so a - -- malformed entry must not raise. Under the old node cache a raise came - -- back as a cache error and turned into a 401 for a valid credential. - local function jwt_with(header, payload) - return b64(header) .. "." .. b64(payload) .. ".signature" - end - - it("a payload that is not an object does not raise", function() - for _, payload in ipairs({ "1", "null", '"a string"', "[1,2]", "{}", "not json" }) do - local ok, ttl = pcall(cache.ttl_for, conf(), jwt_with('{"alg":"RS256"}', payload)) - assert.is_true(ok) - assert.is_true(type(ttl) == "number") - end - end) - - it("a header that is not an object does not raise", function() - -- jwt_parser reads header.alg without checking the header's type, - -- so these raise inside it - for _, header in ipairs({ "1", "null", "true", '"a string"', "[1]", "{}" }) do - local ok, ttl = pcall(cache.ttl_for, conf(), jwt_with(header, '{"sub":"u1"}')) - assert.is_true(ok) - assert.is_true(type(ttl) == "number") - end - end) - - it("a token that cannot be read is still usable, just not trusted for its expiry", function() - assert.equal(5, cache.ttl_for(conf(), jwt_with("1", '{"exp":1}'))) - assert.equal(5, cache.ttl_for(conf(), "not-a-jwt")) - end) - end) end) From 5482418ff8a97a85c9e1360cd6c6ef10f54957bd Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Fri, 11 Sep 2026 21:16:41 +0530 Subject: [PATCH 12/14] docs: state that cache_ttl must stay under the token lifetime --- README.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 50599dc..34f9551 100644 --- a/README.md +++ b/README.md @@ -95,9 +95,8 @@ plugins: | Field | Default | What it does | |---|---|---| -| `cache_ttl` | `5` | Seconds a token is reused for. `0` turns caching off | +| `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 | -| `cache_exp_skew` | `2` | Clock skew allowed when clamping the ttl to the token expiry | | `redis_host` | unset | Setting it turns caching on | | `redis_port` | `6379` | | | `redis_timeout` | `100` | Milliseconds, for connect, send and read | @@ -148,10 +147,9 @@ it. - 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`, - `cache_ttl` and `cache_exp_skew`. The session value is never stored in plain - text, and two routes that would resolve a credential differently cannot share - an entry. + `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. @@ -159,10 +157,12 @@ it. 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 expiry is clamped to the token's own `exp`, minus `cache_exp_skew`. - That is the only thing that sets the expiry, so a token still in redis always - has at least the skew left on it and an expired token is never handed to the - upstream. +- 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 From d58daeba65fc777140628dfcca0f333b6cb0cb15 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Fri, 11 Sep 2026 21:29:59 +0530 Subject: [PATCH 13/14] refactor: name things so the cache and redis code reads without comments --- kong/plugins/frontier/cache.lua | 96 +++++++++++---------- kong/plugins/frontier/redis.lua | 145 ++++++++++++++++---------------- 2 files changed, 122 insertions(+), 119 deletions(-) diff --git a/kong/plugins/frontier/cache.lua b/kong/plugins/frontier/cache.lua index 46c734c..4f228a2 100644 --- a/kong/plugins/frontier/cache.lua +++ b/kong/plugins/frontier/cache.lua @@ -8,42 +8,42 @@ local pcall = pcall local concat = table.concat local ipairs = ipairs local sort = table.sort +local tostring = tostring local hash = utils.hash --- Builds the key for the credential being exchanged. Only the cookies named in --- conf.cache_cookie_names go in; the rest change too often to key on. Returns --- nil when there is no credential, so anonymous requests never share an entry. -function _M.build_key(conf, cookies, bearer) - local jar = utils.parse_cookies(cookies) - - -- sorted, so the order the names are listed in does not matter +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) - local has_credential = false + return names +end - -- everything that changes what the entry means - local parts = { +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 - for _, name in ipairs(names) do - local values = jar[name] +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] - -- Every occurrence goes in. Frontier acts on the last `sid` that - -- decodes, so taking one would let two users hash to the same key. - if values then - for _, value in ipairs(values) do - if value ~= "" then - has_credential = true - end + 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 @@ -51,51 +51,57 @@ function _M.build_key(conf, cookies, bearer) end end - if bearer and bearer ~= "" then - has_credential = true - parts[#parts + 1] = bearer - else - parts[#parts + 1] = "" - end + found_a_credential = found_a_credential or (bearer ~= nil and bearer ~= "") + parts[#parts + 1] = bearer or "" - if not has_credential then + if not found_a_credential then return nil end - -- hashed, so no session sits in redis as a plaintext key return hash(concat(parts, "\0")) end --- Resolves the token: redis first, then the auth server through `fetch`. Redis --- is a cache and not an authority, so any problem with it falls through too. -function _M.get(conf, key, fetch) - -- no credential to key on, or no redis to key it in - if not key or not redis.enabled(conf) then - return fetch() +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 - -- pcall'd so redis cannot fail a request even by raising - local ok, cached = pcall(redis.get, conf, key) + 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 - if not ok then - kong.log.warn("redis lookup raised, ignoring it: ", cached) - elseif cached then +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() + local token, err = fetch_from_auth_server() + if not token then return nil, err end - -- cache_ttl as configured. The token is not read for its expiry, so - -- cache_ttl must stay well under the auth server's token lifetime. if conf.cache_ttl > 0 then - local set_ok, set_err = pcall(redis.set, conf, key, token, conf.cache_ttl) - if not set_ok then - kong.log.warn("redis write raised, ignoring it: ", set_err) - end + remember_token_in_redis(conf, key, token) end return token diff --git a/kong/plugins/frontier/redis.lua b/kong/plugins/frontier/redis.lua index 49aa927..74dfc63 100644 --- a/kong/plugins/frontier/redis.lua +++ b/kong/plugins/frontier/redis.lua @@ -8,23 +8,13 @@ local fmt = string.format local math_floor = math.floor local tonumber = tonumber --- how long an idle connection is kept, and how many per worker. The bundled --- rate limiting plugin hardcodes the same shape of numbers. local KEEPALIVE_MS = 60000 local POOL_SIZE = 30 +local KEY_NOT_FOUND = ngx.null +local NEVER_USED_BEFORE = 0 + +local skip_instance_until = {} --- Redis is a cache here, not an authority, so nothing in this file fails a --- request. Every problem returns nil and the caller falls through. --- --- A worker whose command fails stops trying that instance for --- redis_breaker_seconds, so an outage cannot make every request pay the --- timeout. Keyed per instance. A rejected password or database does not trip --- it, being a config mistake rather than a fault. -local breaker_until = {} - --- Names the pool openresty keeps the connection in, and identifies the instance --- for the breaker. A pooled connection has already authenticated and selected --- its database, so anything that changes what a connection means belongs here. local function instance_id(conf) return fmt("frontier:%s:%d:%d:%s:%s", conf.redis_host, @@ -34,13 +24,15 @@ local function instance_id(conf) conf.redis_ssl and "s" or "p") end -local function breaker_is_open(conf) - local until_when = breaker_until[instance_id(conf)] +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 trip_breaker(conf, action, err) - breaker_until[instance_id(conf)] = ngx.now() + conf.redis_breaker_seconds +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 @@ -49,76 +41,84 @@ function _M.enabled(conf) return conf.redis_host ~= nil and conf.redis_host ~= "" end -local function get_connection(conf) +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 ok, err = red:connect(conf.redis_host, conf.redis_port, { + 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 ok then - trip_breaker(conf, "connect", err) + + if not connected then + skip_instance_for_a_while(conf, "connect", connect_err) return nil end - -- a pooled connection has already authenticated and selected its database - local reused, reuse_err = red:get_reused_times() + local times_used_before, reuse_err = red:get_reused_times() + if reuse_err then - trip_breaker(conf, "get_reused_times", reuse_err) + skip_instance_for_a_while(conf, "get_reused_times", reuse_err) red:close() return nil end - -- Redis answering and refusing is a config problem, not an unreachable - -- instance, so neither of these trips the breaker. - if reused == 0 then - if conf.redis_password and conf.redis_password ~= "" then - local auth_ok, auth_err - if conf.redis_username and conf.redis_username ~= "" then - auth_ok, auth_err = red:auth(conf.redis_username, conf.redis_password) - else - auth_ok, auth_err = red:auth(conf.redis_password) - end - if not auth_ok then - kong.log.warn("redis refused the credentials for ", - conf.redis_host, ":", conf.redis_port, ": ", auth_err) - red:close() - return nil - end - end - - if conf.redis_database ~= 0 then - local sel_ok, sel_err = red:select(conf.redis_database) - if not sel_ok then - kong.log.warn("redis rejected database ", conf.redis_database, - " on ", conf.redis_host, ":", conf.redis_port, ": ", sel_err) - red:close() - return nil - end - 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 release(red) - local ok, err = red:set_keepalive(KEEPALIVE_MS, POOL_SIZE) - if not ok then +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 --- Returns the stored value, or nil for a miss, a failure or a tripped breaker. function _M.get(conf, key) - if breaker_is_open(conf) then + if instance_is_being_skipped(conf) then return nil end - local red = get_connection(conf) + local red = borrow_connection(conf) + if not red then return nil end @@ -126,49 +126,46 @@ function _M.get(conf, key) local value, err = red:get(conf.redis_key_prefix .. key) if not value then - trip_breaker(conf, "get", err) + skip_instance_for_a_while(conf, "get", err) red:close() return nil end - release(red) + return_connection(red) - -- ngx.null is redis saying the key is not there - if value == ngx.null or value == "" then + if value == KEY_NOT_FOUND or value == "" then return nil end return value end --- Stores the value with an expiry. A failure is logged and ignored, because the --- token in hand is still good to use. -function _M.set(conf, key, value, ttl) - if breaker_is_open(conf) then +function _M.set(conf, key, value, ttl_seconds) + if instance_is_being_skipped(conf) then return end - -- milliseconds, so a fractional cache_ttl survives. SETEX takes whole - -- seconds and redis rejects a fractional argument. - local px = math_floor((tonumber(ttl) or 0) * 1000) + local expires_in_ms = math_floor((tonumber(ttl_seconds) or 0) * 1000) - if px <= 0 then + if expires_in_ms <= 0 then return end - local red = get_connection(conf) + local red = borrow_connection(conf) + if not red then return end - local ok, err = red:set(conf.redis_key_prefix .. key, value, "PX", px) - if not ok then + 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 - release(red) + return_connection(red) end return _M From b635a5b2cb99d73086f37dd342e4e6662b984f1f Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Fri, 11 Sep 2026 21:29:59 +0530 Subject: [PATCH 14/14] refactor: name things so the access path reads without comments --- kong/plugins/frontier/access.lua | 28 +++++++++++---------------- kong/plugins/frontier/jwt_decoder.lua | 12 ++++-------- kong/plugins/frontier/utils.lua | 19 +++++++----------- 3 files changed, 22 insertions(+), 37 deletions(-) diff --git a/kong/plugins/frontier/access.lua b/kong/plugins/frontier/access.lua index 94c86fa..97823de 100644 --- a/kong/plugins/frontier/access.lua +++ b/kong/plugins/frontier/access.lua @@ -27,14 +27,12 @@ end local function get_http_client(conf) local client = http.new() - -- set_timeouts takes connect, send, read in that order - client:set_timeouts(conf.http_connect_timeout, conf.http_send_timeout, conf.http_read_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 --- Sends a request to the auth server and gets a user token back for the --- cookies. Failures come back as `nil, err, upstream_status`, so the caller --- decides how to end the request instead of this doing it. 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) @@ -99,14 +97,12 @@ local function fetch_identity_token(conf, cookies, bearer) return token, nil, nil end --- verifies user identity, using the cache when it is turned on local function check_request_identity(conf, cookies, bearer) - -- set by the fetch below, read only when there is no token to return - local upstream_status + local auth_server_status local function fetch() local token, err, status = fetch_identity_token(conf, cookies, bearer) - upstream_status = status + auth_server_status = status return token, err end @@ -122,9 +118,9 @@ local function check_request_identity(conf, cookies, bearer) if not token then kong.log.warn("failed to resolve user token: ", err) - if upstream_status then + if auth_server_status then return kong.response.exit(ngx.HTTP_UNAUTHORIZED, unauthorized_response, { - ["x-upstream-status"] = upstream_status + ["x-upstream-status"] = auth_server_status }) end @@ -228,10 +224,9 @@ local function append_claims_as_headers(conf, user_token) local claims = jwt.claims - -- A payload only has to be valid json, so it can decode to a string. In lua - -- that still indexes: `claims.sub` would hand back string.sub, and setting - -- a header to a function fails the request with a 500. - if type(claims) ~= "table" then + 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 @@ -260,9 +255,8 @@ local function verify_organization_id_header(conf, user_token) local claims = jwt.claims local org_ids = type(claims) == "table" and claims[frontier_org_ids_claim_key] or nil - -- a claim we cannot read is one we cannot verify against, so the header - -- gets dropped rather than raising in gmatch local org_id_header_verified = false + if type(org_ids) == "string" then for word in string.gmatch(org_ids, '([^,]+)') do if word == request_organization_id then diff --git a/kong/plugins/frontier/jwt_decoder.lua b/kong/plugins/frontier/jwt_decoder.lua index ec338c2..da4e414 100644 --- a/kong/plugins/frontier/jwt_decoder.lua +++ b/kong/plugins/frontier/jwt_decoder.lua @@ -2,16 +2,12 @@ local _M = {} local jwt_decoder = require "kong.plugins.jwt.jwt_parser" --- Return type: [metatable, error] function _M.decode_token(token) - -- pcall'd because jwt_parser reads the decoded header without checking its - -- type, so a token whose header segment is valid json but not an object - -- raises. A cached token can come from anywhere with write access to the - -- cache, so nothing here can assume the token is well formed. - local ok, jwt, err = pcall(jwt_decoder.new, jwt_decoder, token) + local parsed_without_raising, jwt, err = pcall(jwt_decoder.new, jwt_decoder, token) - if not ok then - ngx.log(ngx.STDERR, jwt) + if not parsed_without_raising then + local raised = jwt + ngx.log(ngx.STDERR, raised) return nil, "could not decode token" end diff --git a/kong/plugins/frontier/utils.lua b/kong/plugins/frontier/utils.lua index c7b1fe8..c16f4ab 100644 --- a/kong/plugins/frontier/utils.lua +++ b/kong/plugins/frontier/utils.lua @@ -4,8 +4,6 @@ local resty_sha256 = require "resty.sha256" local encode_base64 = ngx.encode_base64 --- resty.sha256 rather than kong.tools.sha256, which does not exist before 3.6. --- One instance per worker, safe because nothing yields between reset and final. local sha256 = resty_sha256:new() function _M.hash(input) @@ -14,8 +12,6 @@ function _M.hash(input) return (encode_base64(sha256:final(), true):gsub("+", "-"):gsub("/", "_")) end --- splits a string s using a delimiter and returns a table --- containing the resulting substrings function _M.split(s, delimiter) local result = {} for match in (s .. delimiter):gmatch("(.-)" .. delimiter) do @@ -29,29 +25,28 @@ function _M.ltrim(s) return s:match'^%s*(.*)' end --- Parses a cookie header into a table of name to list of values, in order. A --- name can legitimately appear more than once, so every value is kept: callers --- cannot guess which one the auth server will act on. function _M.parse_cookies(cookie_header) - local jar = {} + local name_to_every_value_sent = {} if not cookie_header then - return jar + 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 = jar[name] + local values = name_to_every_value_sent[name] + if values then values[#values + 1] = value else - jar[name] = { value } + name_to_every_value_sent[name] = { value } end end end - return jar + return name_to_every_value_sent end return _M