From 9176508e92478513c28d0c52f0b62cfab958def1 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 9 Sep 2026 00:08:13 +0530 Subject: [PATCH 01/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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/26] 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 From 46b8128a8d9925bd9089999102956b6e91f9f416 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 15 Sep 2026 13:28:04 +0530 Subject: [PATCH 15/26] fix(cache): read cookie values exactly as the auth server does --- ...c => kong-plugin-frontier-0.2.0-1.rockspec | 0 kong/plugins/frontier/utils.lua | 17 +++++++-- .../spec/frontier-test/04-cache_spec.lua | 36 +++++++++++++++++-- 3 files changed, 49 insertions(+), 4 deletions(-) rename kong-plugin-frontier-0.1.1-1.rockspec => kong-plugin-frontier-0.2.0-1.rockspec (100%) diff --git a/kong-plugin-frontier-0.1.1-1.rockspec b/kong-plugin-frontier-0.2.0-1.rockspec similarity index 100% rename from kong-plugin-frontier-0.1.1-1.rockspec rename to kong-plugin-frontier-0.2.0-1.rockspec diff --git a/kong/plugins/frontier/utils.lua b/kong/plugins/frontier/utils.lua index c16f4ab..c33beba 100644 --- a/kong/plugins/frontier/utils.lua +++ b/kong/plugins/frontier/utils.lua @@ -25,6 +25,18 @@ function _M.ltrim(s) return s:match'^%s*(.*)' end +local function has_a_byte_frontier_would_reject(value) + return value:find("[^\32-\126]") ~= nil or value:find('["\\;]') ~= nil +end + +local function unquoted(value) + if #value > 1 and value:sub(1, 1) == '"' and value:sub(-1) == '"' then + return value:sub(2, -2) + end + + return value +end + function _M.parse_cookies(cookie_header) local name_to_every_value_sent = {} @@ -33,9 +45,10 @@ function _M.parse_cookies(cookie_header) end for pair in cookie_header:gmatch("[^;]+") do - local name, value = pair:match("^%s*([^=%s]+)%s*=%s*(.-)%s*$") + local name, raw_value = pair:match("^%s*([^=%s]+)%s*=(.-)%s*$") + local value = raw_value and unquoted(raw_value) - if name then + if value and not has_a_byte_frontier_would_reject(value) then local values = name_to_every_value_sent[name] if values then diff --git a/kong/plugins/spec/frontier-test/04-cache_spec.lua b/kong/plugins/spec/frontier-test/04-cache_spec.lua index 8654945..56c8b48 100644 --- a/kong/plugins/spec/frontier-test/04-cache_spec.lua +++ b/kong/plugins/spec/frontier-test/04-cache_spec.lua @@ -113,8 +113,40 @@ describe("Plugin: " .. PLUGIN_NAME .. " (cache), ", function() assert.same({ "x" }, jar.other) end) - it("trims surrounding spaces", function() - assert.same({ "abc" }, utils.parse_cookies(" sid = abc ").sid) + it("parses a value exactly the way frontier does", function() + -- go's net/http trims the pair and the name but keeps whatever + -- follows the `=`, so a value with a leading space is a different + -- credential and has to stay a different cache key + assert.same({ "abc" }, utils.parse_cookies(" sid =abc ").sid) + assert.same({ " abc" }, utils.parse_cookies(" sid = abc ").sid) + assert.same({ " abc" }, utils.parse_cookies("sid= abc").sid) + assert.same({ " abc" }, utils.parse_cookies("sid= abc ").sid) + assert.same({ "a b c" }, utils.parse_cookies("sid=a b c").sid) + assert.same({ "" }, utils.parse_cookies("sid=").sid) + assert.same({ "a=b" }, utils.parse_cookies("sid=a=b").sid) + end) + + it("strips surrounding quotes, as go does", function() + assert.same({ "quoted" }, utils.parse_cookies('sid="quoted"').sid) + assert.same({ "" }, utils.parse_cookies('sid=""').sid) + assert.same({ " abc " }, utils.parse_cookies('sid=" abc "').sid) + end) + + it("drops a value holding a byte go would reject", function() + -- go drops the whole cookie, so keeping it would make our key + -- disagree with the session frontier actually sees + assert.is_nil(utils.parse_cookies('sid="').sid) + assert.is_nil(utils.parse_cookies('sid="a"b"').sid) + assert.is_nil(utils.parse_cookies("sid=a\\b").sid) + assert.is_nil(utils.parse_cookies("sid=ab\tcd").sid) + assert.is_nil(utils.parse_cookies("sid=caf\xc3\xa9").sid) + end) + + it("a leading space makes a different cache key", function() + local plain = cache.build_key(conf(), "sid=abc", nil) + local spaced = cache.build_key(conf(), "sid= abc", nil) + + assert.not_equal(plain, spaced) end) end) From f813aadf00926f5335b2daf17fbd67e66a476214 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 15 Sep 2026 13:28:04 +0530 Subject: [PATCH 16/26] fix: do not let a malformed auth server body raise --- kong/plugins/frontier/access.lua | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/kong/plugins/frontier/access.lua b/kong/plugins/frontier/access.lua index 97823de..6a96039 100644 --- a/kong/plugins/frontier/access.lua +++ b/kong/plugins/frontier/access.lua @@ -77,14 +77,13 @@ local function fetch_identity_token(conf, cookies, bearer) -- fallback to response body if header token is not found if not token and res.body then - kong.log.debug("check_request_identity: Attempting to extract token from response body") - local bodyJson, err = json.decode(res.body) - if not err and bodyJson and bodyJson[conf.token_response_field] then - token = bodyJson[conf.token_response_field] - kong.log.debug("check_request_identity: Token found in response body field '", conf.token_response_field, "'") + local decoded_ok, body = pcall(json.decode, res.body) + local field = decoded_ok and type(body) == "table" and body[conf.token_response_field] + + if type(field) == "string" then + token = field else - kong.log.debug("check_request_identity: Failed to extract token from response body - err: ", err, - ", field present: ", bodyJson and bodyJson[conf.token_response_field] and "yes" or "no") + kong.log.debug("no ", conf.token_response_field, " in the auth server's body") end end @@ -109,7 +108,7 @@ local function check_request_identity(conf, cookies, bearer) local token, err - if conf.cache_ttl > 0 then + if cache.enabled(conf) then token, err = cache.get(conf, cache.build_key(conf, cookies, bearer), fetch) else token, err = fetch() @@ -184,14 +183,14 @@ local function check_request_permission(conf, cookies, bearer) }) end - local bodyJson, err = json.decode(res.body) - if err or not bodyJson then - kong.log.warn("failed to parse response body: ", err) + local decoded_ok, body = pcall(json.decode, res.body) + if not decoded_ok or type(body) ~= "table" then + kong.log.warn("could not read the authz response body") return fail_auth() end - if bodyJson["status"] ~= true then - kong.log.warn("status value not true: ", bodyJson["status"]) + if body["status"] ~= true then + kong.log.warn("status value not true: ", tostring(body["status"])) return fail_auth() end end From e766d489fde75b2c3f1e6ddddb0f158be61153ab Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 15 Sep 2026 13:28:04 +0530 Subject: [PATCH 17/26] perf(cache): build the key only when there is a redis to use --- kong/plugins/frontier/cache.lua | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kong/plugins/frontier/cache.lua b/kong/plugins/frontier/cache.lua index 4f228a2..2b221f3 100644 --- a/kong/plugins/frontier/cache.lua +++ b/kong/plugins/frontier/cache.lua @@ -80,10 +80,12 @@ local function remember_token_in_redis(conf, key, token) 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) +function _M.enabled(conf) + return conf.cache_ttl > 0 and redis.enabled(conf) +end - if nothing_to_cache_or_nowhere_to_cache_it then +function _M.get(conf, key, fetch_from_auth_server) + if key == nil or not _M.enabled(conf) then return fetch_from_auth_server() end @@ -100,9 +102,7 @@ function _M.get(conf, key, fetch_from_auth_server) return nil, err end - if conf.cache_ttl > 0 then - remember_token_in_redis(conf, key, token) - end + remember_token_in_redis(conf, key, token) return token end From 898b2d36c7539272e767518bd468f9e657bda499 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 15 Sep 2026 13:28:04 +0530 Subject: [PATCH 18/26] fix(redis): pause the instance when credentials are refused --- kong/plugins/frontier/redis.lua | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/kong/plugins/frontier/redis.lua b/kong/plugins/frontier/redis.lua index 74dfc63..2c04c9b 100644 --- a/kong/plugins/frontier/redis.lua +++ b/kong/plugins/frontier/redis.lua @@ -52,8 +52,7 @@ local function authenticate_and_select_database(red, conf) end if not accepted then - kong.log.warn("redis refused the credentials for ", - conf.redis_host, ":", conf.redis_port, ": ", err) + skip_instance_for_a_while(conf, "auth", err) return false end end @@ -62,8 +61,7 @@ local function authenticate_and_select_database(red, conf) 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) + skip_instance_for_a_while(conf, "select", err) return false end end From 4c7af142aa53a483dd235749c9dff4828a0e8001 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 15 Sep 2026 13:28:04 +0530 Subject: [PATCH 19/26] test: cover redis.lua against a stubbed client --- .../spec/frontier-test/06-redis_spec.lua | 384 ++++++++++++++++++ 1 file changed, 384 insertions(+) create mode 100644 kong/plugins/spec/frontier-test/06-redis_spec.lua diff --git a/kong/plugins/spec/frontier-test/06-redis_spec.lua b/kong/plugins/spec/frontier-test/06-redis_spec.lua new file mode 100644 index 0000000..b410bb1 --- /dev/null +++ b/kong/plugins/spec/frontier-test/06-redis_spec.lua @@ -0,0 +1,384 @@ +local PLUGIN_NAME = "frontier" + +-- redis.lua talks to a real socket, so resty.redis stands in as a table here. +-- The stub records every command and can be told to fail any of them, which is +-- what lets the pause, the login and the not-found reply be driven directly. + +-- redis.lua binds `kong` at require time, so this table has to be in place +-- before it is required and must not be swapped out afterwards +local warnings = 0 + +_G.kong = { + log = { + debug = function() end, + info = function() end, + err = function() end, + warn = function() warnings = warnings + 1 end + } +} + +local server = {} + +local function reset_server() + server.store = {} + server.calls = {} + server.fail = {} + server.reused_times = 0 + server.closed = 0 + server.kept_alive = 0 + warnings = 0 +end + +local function record(name, ...) + server.calls[#server.calls + 1] = name + return ... +end + +local function calls_named(name) + local n = 0 + for _, call in ipairs(server.calls) do + if call == name then + n = n + 1 + end + end + return n +end + +package.loaded["resty.redis"] = { + new = function() + return { + set_timeouts = function() end, + + connect = function(_, host, port) + record("connect") + if server.fail.connect then + return nil, "connection refused" + end + server.last_host, server.last_port = host, port + return true + end, + + get_reused_times = function() + record("get_reused_times") + if server.fail.get_reused_times then + return nil, "broken pool" + end + return server.reused_times + end, + + auth = function(_, a, b) + record("auth") + server.auth_args = { a, b } + if server.fail.auth then + return nil, "WRONGPASS" + end + return true + end, + + select = function(_, db) + record("select") + server.selected = db + if server.fail.select then + return nil, "DB index out of range" + end + return true + end, + + get = function(_, key) + record("get") + if server.fail.get then + return nil, "read timeout" + end + local value = server.store[key] + if value == nil then + return ngx.null + end + return value + end, + + set = function(_, key, value, unit, amount) + record("set") + if server.fail.set then + return nil, "OOM" + end + server.store[key] = value + server.last_expiry = { unit = unit, amount = amount } + return true + end, + + set_keepalive = function() + record("set_keepalive") + server.kept_alive = server.kept_alive + 1 + return true + end, + + close = function() + record("close") + server.closed = server.closed + 1 + return true + end + } + end +} + +-- 04-cache_spec swaps this module for a table of its own, so it is cleared +-- here to be sure these tests drive the real one +package.loaded["kong.plugins." .. PLUGIN_NAME .. ".redis"] = nil + +local redis = require("kong.plugins." .. PLUGIN_NAME .. ".redis") + +local function conf(overrides) + local c = { + redis_host = "10.0.0.1", + redis_port = 6379, + redis_timeout = 100, + redis_database = 0, + redis_key_prefix = "frontier:authn:", + redis_breaker_seconds = 10, + redis_ssl = false, + redis_ssl_verify = false + } + for k, v in pairs(overrides or {}) do + c[k] = v + end + return c +end + +-- each test uses its own host so the pause from one cannot reach another +local next_host = 0 +local function fresh_conf(overrides) + next_host = next_host + 1 + local c = conf(overrides) + c.redis_host = "10.0.0." .. next_host + return c +end + + +describe("Plugin: " .. PLUGIN_NAME .. " (redis), ", function() + describe("enabled", function() + it("needs a host", function() + assert.is_false(redis.enabled(conf({ redis_host = "" }))) + assert.is_true(redis.enabled(conf())) + end) + end) + + describe("reading", function() + it("returns the stored value", function() + reset_server() + local c = fresh_conf() + server.store["frontier:authn:k"] = "a-token" + + assert.equal("a-token", redis.get(c, "k")) + end) + + it("reads a not-found reply as a miss", function() + reset_server() + local c = fresh_conf() + + assert.is_nil(redis.get(c, "missing")) + assert.equal(1, calls_named("get")) + end) + + it("reads an empty value as a miss", function() + reset_server() + local c = fresh_conf() + server.store["frontier:authn:k"] = "" + + assert.is_nil(redis.get(c, "k")) + end) + + it("prefixes the key", function() + reset_server() + local c = fresh_conf({ redis_key_prefix = "other:" }) + server.store["other:k"] = "v" + + assert.equal("v", redis.get(c, "k")) + end) + + it("returns the connection to the pool on the way out", function() + reset_server() + local c = fresh_conf() + server.store["frontier:authn:k"] = "v" + + redis.get(c, "k") + assert.equal(1, server.kept_alive) + assert.equal(0, server.closed) + end) + end) + + describe("writing", function() + it("stores with a millisecond expiry", function() + reset_server() + local c = fresh_conf() + + redis.set(c, "k", "v", 5) + assert.equal("v", server.store["frontier:authn:k"]) + assert.same({ unit = "PX", amount = 5000 }, server.last_expiry) + end) + + it("keeps a fractional ttl", function() + reset_server() + local c = fresh_conf() + + redis.set(c, "k", "v", 0.5) + assert.same({ unit = "PX", amount = 500 }, server.last_expiry) + end) + + it("writes nothing for a ttl of zero or less", function() + reset_server() + local c = fresh_conf() + + redis.set(c, "k", "v", 0) + redis.set(c, "k", "v", -1) + assert.equal(0, calls_named("set")) + assert.equal(0, calls_named("connect")) + end) + + it("a rejected write is swallowed", function() + reset_server() + local c = fresh_conf() + server.fail.set = true + + redis.set(c, "k", "v", 5) + assert.equal(1, server.closed) + end) + end) + + describe("logging in", function() + it("logs in and selects on a connection never used before", function() + reset_server() + local c = fresh_conf({ redis_password = "s3cret", redis_database = 3 }) + server.reused_times = 0 + + redis.get(c, "k") + assert.equal(1, calls_named("auth")) + assert.equal(1, calls_named("select")) + assert.equal(3, server.selected) + end) + + it("does neither on a connection from the pool", function() + reset_server() + local c = fresh_conf({ redis_password = "s3cret", redis_database = 3 }) + server.reused_times = 4 + + redis.get(c, "k") + assert.equal(0, calls_named("auth")) + assert.equal(0, calls_named("select")) + end) + + it("sends the user name when there is one", function() + reset_server() + local c = fresh_conf({ redis_username = "alice", redis_password = "s3cret" }) + + redis.get(c, "k") + assert.same({ "alice", "s3cret" }, server.auth_args) + end) + + it("sends only the password when there is no user name", function() + reset_server() + local c = fresh_conf({ redis_password = "s3cret" }) + + redis.get(c, "k") + assert.same({ "s3cret" }, server.auth_args) + end) + + it("skips select on database zero", function() + reset_server() + local c = fresh_conf({ redis_database = 0 }) + + redis.get(c, "k") + assert.equal(0, calls_named("select")) + end) + end) + + describe("the pause after a failure", function() + it("starts on a failed get and skips the next command entirely", function() + reset_server() + local c = fresh_conf() + server.fail.get = true + + assert.is_nil(redis.get(c, "k")) + assert.equal(1, calls_named("connect")) + assert.equal(1, warnings) + + -- the second call must not reach the socket at all + assert.is_nil(redis.get(c, "k")) + assert.equal(1, calls_named("connect")) + assert.equal(1, warnings) + end) + + it("skips writes too, not just reads", function() + reset_server() + local c = fresh_conf() + server.fail.connect = true + + redis.get(c, "k") + assert.equal(1, calls_named("connect")) + + redis.set(c, "k", "v", 5) + assert.equal(1, calls_named("connect")) + end) + + it("starts on a refused password", function() + reset_server() + local c = fresh_conf({ redis_password = "wrong" }) + server.fail.auth = true + + assert.is_nil(redis.get(c, "k")) + assert.equal(1, warnings) + + -- a mistyped password must not cost a dial and a login per request + assert.is_nil(redis.get(c, "k")) + assert.equal(1, calls_named("connect")) + assert.equal(1, calls_named("auth")) + assert.equal(1, warnings) + end) + + it("starts on a rejected database", function() + reset_server() + local c = fresh_conf({ redis_database = 9 }) + server.fail.select = true + + assert.is_nil(redis.get(c, "k")) + assert.is_nil(redis.get(c, "k")) + assert.equal(1, calls_named("connect")) + assert.equal(1, warnings) + end) + + it("is kept per instance, so one bad redis does not stop a good one", function() + reset_server() + local bad = fresh_conf() + local good = fresh_conf() + server.fail.connect = true + + redis.get(bad, "k") + server.fail.connect = false + server.store["frontier:authn:k"] = "v" + + assert.equal("v", redis.get(good, "k")) + end) + + it("treats a different database as a different instance", function() + reset_server() + local c = fresh_conf() + local other_db = conf({ redis_host = c.redis_host, redis_database = 7 }) + server.fail.connect = true + + redis.get(c, "k") + server.fail.connect = false + server.store["frontier:authn:k"] = "v" + + assert.equal("v", redis.get(other_db, "k")) + end) + + it("closes the connection when the pool check fails", function() + reset_server() + local c = fresh_conf() + server.fail.get_reused_times = true + + assert.is_nil(redis.get(c, "k")) + assert.equal(1, server.closed) + assert.equal(0, server.kept_alive) + end) + end) +end) From 162ec08bb7ff00bcde5291e2b52fd44d5dcbef05 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 15 Sep 2026 13:28:04 +0530 Subject: [PATCH 20/26] test: drive every exit of the auth server call --- .../spec/frontier-test/05-access_spec.lua | 90 ++++++++++++++++--- 1 file changed, 80 insertions(+), 10 deletions(-) diff --git a/kong/plugins/spec/frontier-test/05-access_spec.lua b/kong/plugins/spec/frontier-test/05-access_spec.lua index f68a362..cec43e4 100644 --- a/kong/plugins/spec/frontier-test/05-access_spec.lua +++ b/kong/plugins/spec/frontier-test/05-access_spec.lua @@ -37,21 +37,29 @@ local function base_conf() } 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 } +-- runs the plugin against an auth server and reports what reached the upstream. +-- `answer` is either a token the server hands back in a 200, or a table +-- describing the raw reply so the failure paths can be driven. +local function run_plugin(conf, answer, request_headers) + local result = { set = {}, cleared = {}, status = nil, exit_headers = nil } + + local reply, reply_err + if type(answer) == "table" then + reply, reply_err = answer.response, answer.err + else + reply = { + status = 200, + headers = {}, + body = '{"' .. conf.token_response_field .. '":"' .. answer .. '"}' + } + end 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 + return reply, reply_err end } end @@ -94,8 +102,9 @@ local function run_plugin(conf, token, request_headers) } }, response = { - exit = function(status) + exit = function(status, _, headers) result.status = status + result.exit_headers = headers -- kong ends the request here, so nothing after it runs error(exited) end @@ -185,6 +194,67 @@ describe("Plugin: " .. PLUGIN_NAME .. " (access), ", function() end) end) + describe("what the auth server answers", function() + it("a 200 with no token in it gives a 401, not a 500", function() + -- the one behaviour change in this work. it used to pass nil into + -- set_header and fail with `invalid header value ... got nil` + local out = run_plugin(base_conf(), { response = { status = 200, headers = {}, body = "{}" } }, {}) + + assert.is_nil(out.raised) + assert.equal(401, out.status) + assert.is_nil(out.set["x-user-token"]) + end) + + it("a 200 with an empty body gives a 401", function() + local out = run_plugin(base_conf(), { response = { status = 200, headers = {}, body = "" } }, {}) + + assert.is_nil(out.raised) + assert.equal(401, out.status) + end) + + it("a 200 whose body names a different field gives a 401", function() + local out = run_plugin(base_conf(), + { response = { status = 200, headers = {}, body = '{"some_other_field":"tok"}' } }, {}) + + assert.is_nil(out.raised) + assert.equal(401, out.status) + end) + + it("a 401 is passed on as a 401 carrying the upstream status", function() + local out = run_plugin(base_conf(), { response = { status = 401, headers = {}, body = "" } }, {}) + + assert.is_nil(out.raised) + assert.equal(401, out.status) + assert.equal(401, out.exit_headers["x-upstream-status"]) + end) + + it("a 500 from the auth server becomes a 401 carrying the upstream status", function() + local out = run_plugin(base_conf(), { response = { status = 500, headers = {}, body = "" } }, {}) + + assert.is_nil(out.raised) + assert.equal(401, out.status) + assert.equal(500, out.exit_headers["x-upstream-status"]) + end) + + it("an unreachable auth server gives a 401 with no upstream status", function() + local out = run_plugin(base_conf(), { response = nil, err = "connection refused" }, {}) + + assert.is_nil(out.raised) + assert.equal(401, out.status) + assert.is_nil(out.exit_headers) + end) + + it("the token can come back in a header instead of the body", function() + local token = token_with_payload('{"sub":"u1"}') + local out = run_plugin(base_conf(), + { response = { status = 200, headers = { ["x-user-token"] = token }, body = "{}" } }, {}) + + assert.is_nil(out.raised) + assert.is_nil(out.status) + assert.equal(token, out.set["x-user-token"]) + end) + end) + describe("organization id header", function() it("keeps a header the token's org_ids claim allows", function() local conf = base_conf() From ddf13419f12ebb19841d7d845ec5e33025311377 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 15 Sep 2026 13:28:04 +0530 Subject: [PATCH 21/26] test: assert the cache ttl ceiling and the removed fields --- .../spec/frontier-test/01-schema_spec.lua | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/kong/plugins/spec/frontier-test/01-schema_spec.lua b/kong/plugins/spec/frontier-test/01-schema_spec.lua index 3835d94..ae99a5d 100644 --- a/kong/plugins/spec/frontier-test/01-schema_spec.lua +++ b/kong/plugins/spec/frontier-test/01-schema_spec.lua @@ -36,4 +36,29 @@ describe("Plugin: " .. PLUGIN_NAME .. " (schema), ", function() assert.same({ "sid", "other_session" }, ok.config.cache_cookie_names) end) + + it("the cache ttl ceiling is enforced", function() + -- nothing else keeps the cache under the auth server's token lifetime, + -- so this bound has to hold + assert(v({ authn_url = "my_auth_url", cache_ttl = 300 }, schema_def)) + + local ok, err = v({ authn_url = "my_auth_url", cache_ttl = 301 }, schema_def) + assert.is_nil(ok) + assert.not_nil(err) + end) + + it("a zero redis timeout is rejected", function() + assert(v({ authn_url = "my_auth_url", redis_timeout = 1 }, schema_def)) + + local ok, err = v({ authn_url = "my_auth_url", redis_timeout = 0 }, schema_def) + assert.is_nil(ok) + assert.not_nil(err) + end) + + it("the fields the cache no longer has are gone", function() + for _, field in ipairs({ "cache_exp_skew", "redis_keepalive_ms", "redis_pool_size" }) do + local ok = v({ authn_url = "my_auth_url", [field] = 1 }, schema_def) + assert.is_nil(ok, field .. " should not be accepted") + end + end) end) \ No newline at end of file From 2fb822018d7761adce2d0893e4f827cfb568f0ff Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 15 Sep 2026 13:28:04 +0530 Subject: [PATCH 22/26] chore: bump the plugin version to 0.2.0 --- README.md | 15 +++++++++++---- kong-plugin-frontier-0.2.0-1.rockspec | 2 +- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 34f9551..0158068 100644 --- a/README.md +++ b/README.md @@ -127,12 +127,14 @@ worker stops trying redis for `redis_breaker_seconds`. 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 +When anything 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. +worker talking to another. A refused password or database index starts the pause +too. Without that, a mistyped password would cost a fresh connection, a login +round trip and a warning line on every single request, and a full TLS handshake +as well when `redis_ssl` is on. The warning says which it was, so the pause +hides nothing. **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 @@ -153,6 +155,11 @@ it. - 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. +- Cookie values are read exactly the way the auth server reads them. Frontier + uses Go's `net/http`, which keeps a space that follows the `=`, strips a + surrounding pair of double quotes, and drops a cookie whose value holds a + byte it does not allow. Getting any of that wrong would let two different + sessions share one entry. - 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 diff --git a/kong-plugin-frontier-0.2.0-1.rockspec b/kong-plugin-frontier-0.2.0-1.rockspec index e179f9d..ab2447a 100644 --- a/kong-plugin-frontier-0.2.0-1.rockspec +++ b/kong-plugin-frontier-0.2.0-1.rockspec @@ -1,6 +1,6 @@ local plugin_name = "frontier" local package_name = "kong-plugin-" .. plugin_name -local package_version = "0.1.1" +local package_version = "0.2.0" local rockspec_revision = "1" local github_account_name = "raystack" From cdb5525d1b65ee7116dacbcd91989e252e54e3cc Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Tue, 15 Sep 2026 20:05:46 +0530 Subject: [PATCH 23/26] feat(cache): version the entry format so a rollout cannot mix them --- README.md | 5 +++++ kong/plugins/frontier/cache.lua | 5 +++++ .../spec/frontier-test/04-cache_spec.lua | 17 +++++++++++++++++ 3 files changed, 27 insertions(+) diff --git a/README.md b/README.md index 0158068..126ebf3 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,11 @@ it. `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. +- The key also carries an entry format version. Every pod shares these keys, so + during a rollout pods on two plugin versions read the same entries. Bumping + `ENTRY_FORMAT_VERSION` in `cache.lua` whenever the stored value or the key + recipe changes keeps the two apart. A spec pins the recipe so a change that + forgets the bump fails. - 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. diff --git a/kong/plugins/frontier/cache.lua b/kong/plugins/frontier/cache.lua index 2b221f3..bbdab30 100644 --- a/kong/plugins/frontier/cache.lua +++ b/kong/plugins/frontier/cache.lua @@ -11,6 +11,10 @@ local sort = table.sort local tostring = tostring local hash = utils.hash +-- bump when the stored value's shape or the recipe below changes, so pods on +-- two plugin versions never read each other's entries during a rollout +local ENTRY_FORMAT_VERSION = "1" + local function cookie_names_in_a_stable_order(conf) local names = {} @@ -25,6 +29,7 @@ end local function settings_that_change_what_an_entry_means(conf) return { + ENTRY_FORMAT_VERSION, conf.authn_url or "", conf.http_method or "", conf.header_name or "", diff --git a/kong/plugins/spec/frontier-test/04-cache_spec.lua b/kong/plugins/spec/frontier-test/04-cache_spec.lua index 56c8b48..065aba9 100644 --- a/kong/plugins/spec/frontier-test/04-cache_spec.lua +++ b/kong/plugins/spec/frontier-test/04-cache_spec.lua @@ -142,6 +142,23 @@ describe("Plugin: " .. PLUGIN_NAME .. " (cache), ", function() assert.is_nil(utils.parse_cookies("sid=caf\xc3\xa9").sid) end) + it("the recipe is pinned, so changing it forces a version bump", function() + -- every pod shares these keys, so during a rollout two plugin + -- versions read each other's entries. If this value changes, bump + -- ENTRY_FORMAT_VERSION in cache.lua and update it here + local pinned = { + authn_url = "http://auth.test/AuthToken", + http_method = "POST", + header_name = "x-user-token", + token_response_field = "access_token", + cache_ttl = 5, + cache_cookie_names = { "sid" } + } + + assert.equal("m0rQX7ob88P9M0tTpourWk93bnvOoio9E1qp-UvHqo4", + cache.build_key(pinned, "sid=abc", nil)) + end) + it("a leading space makes a different cache key", function() local plain = cache.build_key(conf(), "sid=abc", nil) local spaced = cache.build_key(conf(), "sid= abc", nil) From c6f7d59cf172bb2a1326c57de1215b674f251366 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 16 Sep 2026 13:14:00 +0530 Subject: [PATCH 24/26] fix(cache): trim only the whitespace the auth server trims --- kong/plugins/frontier/utils.lua | 2 +- kong/plugins/spec/frontier-test/04-cache_spec.lua | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/kong/plugins/frontier/utils.lua b/kong/plugins/frontier/utils.lua index c33beba..85bf29c 100644 --- a/kong/plugins/frontier/utils.lua +++ b/kong/plugins/frontier/utils.lua @@ -45,7 +45,7 @@ function _M.parse_cookies(cookie_header) end for pair in cookie_header:gmatch("[^;]+") do - local name, raw_value = pair:match("^%s*([^=%s]+)%s*=(.-)%s*$") + local name, raw_value = pair:match("^[ \t\r\n]*([^=%s]+)[ \t\r\n]*=(.-)[ \t\r\n]*$") local value = raw_value and unquoted(raw_value) if value and not has_a_byte_frontier_would_reject(value) then diff --git a/kong/plugins/spec/frontier-test/04-cache_spec.lua b/kong/plugins/spec/frontier-test/04-cache_spec.lua index 065aba9..c42b19a 100644 --- a/kong/plugins/spec/frontier-test/04-cache_spec.lua +++ b/kong/plugins/spec/frontier-test/04-cache_spec.lua @@ -132,6 +132,15 @@ describe("Plugin: " .. PLUGIN_NAME .. " (cache), ", function() assert.same({ " abc " }, utils.parse_cookies('sid=" abc "').sid) end) + it("trims only the whitespace go trims", function() + -- go's isASCIISpace is space, tab, CR and LF. lua's %s also covers + -- \v and \f, which go keeps and then drops as control bytes + assert.is_nil(utils.parse_cookies("sid=abc\v").sid) + assert.is_nil(utils.parse_cookies("sid=abc\f").sid) + assert.is_nil(utils.parse_cookies("\vsid=abc").sid) + assert.same({ "abc" }, utils.parse_cookies("\tsid=abc\r").sid) + end) + it("drops a value holding a byte go would reject", function() -- go drops the whole cookie, so keeping it would make our key -- disagree with the session frontier actually sees From 31967b29693b10dd9039fe1748a3da34aa59ce9e Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 16 Sep 2026 13:14:00 +0530 Subject: [PATCH 25/26] fix(redis): key the pool and the pause on the password too --- README.md | 8 +++-- kong/plugins/frontier/redis.lua | 30 +++++++++++++++---- .../spec/frontier-test/06-redis_spec.lua | 16 ++++++++++ 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 126ebf3..bbea993 100644 --- a/README.md +++ b/README.md @@ -110,9 +110,11 @@ plugins: | `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. +port, database, user, hashed password and whether SSL is on. Two plugin configs +that mean the same thing share a pool; two that differ do not. The same key +separates the pause, so a config with the wrong password cannot pause one with +the right password. 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 diff --git a/kong/plugins/frontier/redis.lua b/kong/plugins/frontier/redis.lua index 2c04c9b..7ecfe4f 100644 --- a/kong/plugins/frontier/redis.lua +++ b/kong/plugins/frontier/redis.lua @@ -1,6 +1,7 @@ local _M = {} local resty_redis = require "resty.redis" +local utils = require "kong.plugins.frontier.utils" local kong = kong local ngx = ngx @@ -15,13 +16,30 @@ local NEVER_USED_BEFORE = 0 local skip_instance_until = {} +local instance_ids_by_conf = setmetatable({}, { __mode = "k" }) + 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") + local id = instance_ids_by_conf[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", + conf.redis_host, + conf.redis_port, + conf.redis_database, + conf.redis_username or "", + secret, + conf.redis_ssl and "s" or "p") + + instance_ids_by_conf[conf] = id + end + + return id end local function instance_is_being_skipped(conf) diff --git a/kong/plugins/spec/frontier-test/06-redis_spec.lua b/kong/plugins/spec/frontier-test/06-redis_spec.lua index b410bb1..2a7f351 100644 --- a/kong/plugins/spec/frontier-test/06-redis_spec.lua +++ b/kong/plugins/spec/frontier-test/06-redis_spec.lua @@ -371,6 +371,22 @@ describe("Plugin: " .. PLUGIN_NAME .. " (redis), ", function() assert.equal("v", redis.get(other_db, "k")) end) + it("treats a different password as a different instance", function() + -- the pool name carries the hashed password, so a config with the + -- wrong one cannot pause the config with the right one + reset_server() + local right = fresh_conf({ redis_password = "right" }) + local wrong = conf({ redis_host = right.redis_host, redis_password = "wrong" }) + server.fail.auth = true + + assert.is_nil(redis.get(wrong, "k")) + + server.fail.auth = false + server.store["frontier:authn:k"] = "v" + + assert.equal("v", redis.get(right, "k")) + end) + it("closes the connection when the pool check fails", function() reset_server() local c = fresh_conf() From 5b0433a680fea0b6716aa065992dbc3e9dc87389 Mon Sep 17 00:00:00 2001 From: Rohil Surana Date: Wed, 16 Sep 2026 13:14:00 +0530 Subject: [PATCH 26/26] chore: report 0.2.0 from the handler as well --- kong/plugins/frontier/handler.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kong/plugins/frontier/handler.lua b/kong/plugins/frontier/handler.lua index 62ab440..5d5d0ec 100644 --- a/kong/plugins/frontier/handler.lua +++ b/kong/plugins/frontier/handler.lua @@ -1,5 +1,5 @@ local plugin = { - VERSION = "0.1.0", + VERSION = "0.2.0", PRIORITY = 900 } local access = require "kong.plugins.frontier.access"