Skip to content

feat: cache the frontier authn token call in redis - #23

Open
rohilsurana wants to merge 23 commits into
mainfrom
feat/cache-authn-token
Open

rohilsurana wants to merge 23 commits into
mainfrom
feat/cache-authn-token

Conversation

@rohilsurana

@rohilsurana rohilsurana commented Sep 2, 2026

Copy link
Copy Markdown
Member

The problem

Every request through this plugin makes an HTTP call to Frontier to exchange the
cookie or bearer for a user token. Two hundred requests from one browser tab
means two hundred identical calls.

What this does

It caches that exchange in Redis. The lookup is Redis first, then Frontier on a
miss. Redis is shared by every pod, so a token is fetched once for the fleet
rather than once per pod.

The default TTL is 5 seconds, and it is deliberately short. A cached token means
a change to someone's access is not noticed until the entry expires.

Caching needs Redis. Set redis_host and it works. Leave it unset and
cache_ttl does nothing on its own, so every request goes to Frontier exactly
as it does today. There is no deployment step beyond pointing it at a Redis.

The cache is two files: cache.lua is the whole chain at 110 lines, and
redis.lua is the driver and its breaker. jwt_decoder.lua gains a guard
against a malformed token, and access.lua changes only in where it asks for a
token and how it ends a failed request.

Config

Field Default What it does
redis_host unset Setting it turns caching on
cache_ttl 5 Seconds a token is reused for. 0 turns caching off. Max 300
cache_cookie_names ["sid"] Only these cookies go into the cache key
redis_timeout 100 Milliseconds. A healthy Redis answers in well under one
redis_key_prefix frontier:authn: Prefix on every key
redis_breaker_seconds 10 How long a worker stops trying Redis after a command fails

Plus redis_port, redis_username, redis_password, redis_database,
redis_ssl, redis_ssl_verify and redis_server_name. Full table in the
README. Connection pooling is OpenResty's own and is not configurable, the same
way it is not in the bundled rate limiting plugin.

An entry costs about 1KB in Redis, so size it as users active within the TTL
window, times 1KB.

What is cached, and what is not

  • The key is a sha256 of the named cookies, the authorization header, and
    the config that decides what an entry means. Sessions are never stored as
    plaintext keys, 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.
  • Cookie values are read exactly the way Frontier reads them. Frontier uses
    Go's net/http, which keeps a space after the =, strips a surrounding pair
    of quotes, and drops a value holding a byte it does not allow. A differential
    test runs the same 25 headers through net/http and through this plugin and
    asserts they agree, because a disagreement would let two different sessions
    share one entry.
  • A request with no credential is never cached, so anonymous requests cannot
    share an entry.
  • A failed exchange is never cached. Someone who just got access is not
    locked out for the length of the TTL.
  • The entry lives for exactly cache_ttl. The token is never parsed, so
    cache_ttl has to stay well under the auth server's token lifetime. 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_url permission check still runs
    on every request.
  • There is no lock. Several requests arriving together with the same new
    credential will each fetch a token. They all write an equivalent entry, and
    everything after that is served from Redis.
  • The cache never parses the token. It is stored and served as an opaque
    string. It is only read where it was already being read before this PR, for
    the claims that become headers, and a token that is valid JSON but not shaped
    like a JWT is refused there rather than half applied.

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, gets logged and the plugin carries
on to Frontier. When anything fails, that worker stops trying that instance for
redis_breaker_seconds, so an outage cannot make every request pay the timeout.
The pause is per instance, so one bad Redis does not stop the worker using
another. A refused password or database index starts the pause too, otherwise a
mistyped password would cost a fresh connection, a login round trip and a
warning line on every request, and a TLS handshake as well when redis_ssl is
on. The warning names which it was, so the pause hides nothing.

Guard write access to this Redis. The plugin does not check the token
signature, with or without Redis. It trusts what Frontier hands back. So
anything that can write these keys can put a token of its choosing in front of
your 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.

One behaviour change

If Frontier answers 200 but the plugin cannot find a token in the response, it
now returns 401. It used to pass nil into set_header and fail with a 500:

invalid header value for "x-user-token": got nil, expected array of string

This had to change, because nil cannot pass through a cache cleanly. 401 is also
the right answer. Frontier returns a proper error status for every real auth
failure, so this state only means the plugin is pointed at the wrong endpoint,
or token_response_field does not match what the server returns. The 401 cannot
hide a successful auth.

Performance

Measured against a real Frontier, a real Redis and Kong in DB-less mode.
Absolute numbers come from Docker on macOS, 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 Frontier 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, and latency
with the plain and cached routes interleaved:

Kong CPU per request Median latency
No plugin 0.224 ms 6.8 ms
Plugin, Redis hit 0.330 ms 7.4 ms
Plugin, caching off 1.731 ms 39.2 ms

So the Redis hop adds about 0.6ms and saves about 32ms.

Testing

Suite Result
Unit, Kong 3.4 66 passed
Unit, Kong 3.9 66 passed
End to end, 3 pods and a real Redis 57 passed

The unit tests cover the key builder, the cache behaviour, the token handling,
every exit of the auth server call, and redis.lua itself through a stubbed
client that drives the pause, the login, and a not-found reply. They need no
Redis and no network. The end to end suite runs against a real Frontier, with postgres and
SpiceDB, using a real session cookie from the mailotp flow, and counts actual
AuthToken calls from Frontier's own metrics.

The end to end suite adds a full outage of each dependency:

  • Frontier stopped entirely. A cached session still proxies through both
    pods, and a session the cache has never seen gets a 401 rather than a 500.
    Everything recovers when it comes back.
  • Redis stopped mid flight. Requests keep succeeding through Frontier, and
    Redis is picked up again once the pause expires.

Two more things only a real gateway shows:

  • No shared memory zone is configured on any pod, and the suite asserts none
    of them logs a warning about a missing one.
  • A Kong 3.4 pod and two 3.9 pods share one Redis correctly, both
    directions, which matters if the fleet is ever mid-upgrade.

Works on Kong 3.4 and later, using only modules that ship with Kong and
OpenResty.

Why not a node cache as well

An earlier version of this kept a shared memory cache on each pod in front of
Redis, so the common path would pay no network at all. It is not here because
it did not earn its keep. It needed a lua_shared_dict added to kong.conf on
every gateway, it brought mlcache and a single-flight lock, and it needed
extra machinery to stop a token going stale twice over on its way through two
caches. Three adversarial reviews found sixteen defects on this branch, and by
the end every open one lived in that layer and none lived in Redis.

The measurements say it was not buying much. The Redis round trip it would have
saved costs 0.6ms of latency and no measurable CPU: 0.330ms per request against
0.34ms for the node memory hit it replaced.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 7970e409-1035-4e31-bd20-3b67e06e1ca8


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@rohilsurana rohilsurana changed the title feat: cache the frontier authn token call feat: cache the frontier authn token call, with an optional shared redis layer Sep 3, 2026
@rohilsurana rohilsurana changed the title feat: cache the frontier authn token call, with an optional shared redis layer feat: cache the frontier authn token call in redis Sep 8, 2026
@rohilsurana
rohilsurana force-pushed the feat/cache-authn-token branch from 1cf7c34 to c037ab6 Compare September 8, 2026 18:38
@rohilsurana
rohilsurana marked this pull request as ready for review September 8, 2026 19:03
Comment thread kong/plugins/frontier/utils.lua Outdated
end

for pair in cookie_header:gmatch("[^;]+") do
local name, value = pair:match("^%s*([^=%s]+)%s*=%s*(.-)%s*$")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frontier parses cookies with Go's net/http, which keeps a space after = as part of the
value, and the session decoder then rejects that value. This pattern trims that space, so
sid= X and sid=X share one key while Frontier treats them differently. Can we drop the
%s* after the = so the value is kept exactly as Frontier sees it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, and worse than it looked. Fixed in 46b8128 by dropping the %s* after the =.

While checking your claim against Go's source I found a second divergence: parseCookieValue strips a surrounding pair of double quotes, so sid="abc" and sid=abc are one session to Frontier but were two keys here. And validCookieValueByte rejects control bytes, DEL, high bytes, a bare " and a backslash, in which case Go drops the cookie entirely while we kept it.

Rather than patch the three cases by hand I built a differential test: the same headers through real net/http and through parse_cookies, compared. 25 cases, all matching now. The interesting ones are in 04-cache_spec.

On severity, for the record: exploiting the original needs you to already know a valid sid, and if you know it you can just send it, so it is not an escalation. What it broke is the invariant the key exists for, that the cache never turns a request the auth server would reject into one it accepts.

end

-- hashed, so no session sits in redis as a plaintext key
return hash(concat(parts, "\0"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every pod shares these keys, and during a rollout old and new plugin versions read each
other's entries. The value is the raw token today, so a later change to the stored shape
or to the key parts would break the older pods for the length of the ttl. Can we add a
version constant to the key, bumped whenever the value shape or the key recipe changes,
so two versions never read each other's entries?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed in principle, and it is cheap. Holding it for the next round along with the double decode, so this one stays reviewable. Tracking both.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in cdb5525, kept in this PR after all.

ENTRY_FORMAT_VERSION is now the first part of the key. Since a constant only helps if someone remembers to bump it, there is also a test pinning the recipe to one exact hash, so any change to the key parts, the separator or the hash fails and forces a deliberate call on the version.

end
end

if #conf.token_claims_to_append_as_headers > 0 then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The token is decoded at line 219 for the claim headers and again at line 249 for the
org id check, so a request with both on decodes the same string twice. Can we decode
once in run(), after check_request_identity returns, and pass the claims table to
both header steps?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed, access.lua:219 and access.lua:249 decode the same string. Holding it for the next round with the key version.

Slight qualifier for when we do it: verify_request_organization_id_header defaults to false, so the second decode only happens when an operator turns that on. Still worth doing, just less often than it reads.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed, but in #24 rather than here so this PR stays reviewable. It is based on this branch, so it needs this one to go in first.

run() now decodes once and passes the claims to both steps. Only the decode moved: each function keeps its own handling of an unreadable payload, since merging those into one guard would have turned the org check's dropped header into a 401.

The counting test fails on the old code with expected 1 got 2, and only in the case where both steps run and the client actually sent the org header, which matches the qualifier above.

Comment on lines +49 to +54
request_uri = function()
return {
status = 200,
headers = {},
body = '{"' .. conf.token_response_field .. '":"' .. token .. '"}'
}, nil

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stub here always answers 200 with a token, so the four failure exits in
check_request_identity are not driven by any spec, including the 200-with-no-token
path the description calls the one behaviour change. Can we add cases where the stub
answers 200 with an empty body and a plain 401, and assert the 401 and the
x-upstream-status header?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair hit, and it found a real bug. Fixed in 162ec08, seven cases covering all four exits.

The stub now takes either a token or a raw reply, so the failure paths can be driven. Writing the empty body case turned up a 500: cjson.decode raises on malformed input and returns only one value on success, so local bodyJson, err = json.decode(res.body) leaves err permanently nil and an empty 200 body took down the request. It is on main too, at both the authn and the authz call sites. Both are now pcalled in f813aad.

Comment thread kong/plugins/frontier/access.lua Outdated

local token, err

if conf.cache_ttl > 0 then

@AmanGIT07 AmanGIT07 Sep 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cache_ttl defaults to 5, so a deployment without redis_host still parses the cookies and hashes a key on every request, then drops it at cache.lua:84. Can we check redis.enabled(conf) here alongside the ttl, so the key is only built when there is somewhere to store it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, and it is the default path, since cache_ttl is 5 and redis_host is unset out of the box. Fixed in e766d48.

Added cache.enabled(conf), which is cache_ttl > 0 and redis.enabled(conf), and access.lua now checks that before building the key. cache.get reuses the same function as its own guard so the rule lives in one place.

@@ -0,0 +1,171 @@
local _M = {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file has no spec of its own, and 04-cache_spec swaps the whole module for a table,
so the pause starting on a failed GET, the pause skipping both commands, the not-found
reply read as a miss, and login running only on a fresh connection are covered only by
the end to end suite that is not in the repo. Can we add a spec with a stubbed
resty.redis client that drives those four paths?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and I had flagged the same gap to myself. I had a driver suite for exactly those four paths but it only ran against a real Redis in Docker, which is no use to you or to CI. Replaced with 4c7af14, a spec that stubs resty.redis, so it needs no Redis and no network.

Twenty two cases: the pause starting on a failed GET and on a refused password, the pause skipping both commands, the pause staying per instance, a not-found reply and an empty value read as misses, login and SELECT happening only when get_reused_times is 0, and the millisecond expiry.

One thing worth knowing: 04-cache_spec replaces this whole module in package.loaded, so the new spec clears that entry before requiring the real one. Without it the new tests silently exercise the cache spec's table.

Comment thread kong-plugin-frontier-0.1.1-1.rockspec Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets bump the version?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 2fb8220, 0.1.1 to 0.2.0, with the file renamed to match.

@AmanGIT07

Copy link
Copy Markdown
Contributor

There is no CI workflow, so were 3.4 and 3.9 spec runs in the description done by hand? Can we add a workflow that runs pongo against both versions on every push?

accepted, err = red:auth(conf.redis_password)
end

if not accepted then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A refused password or database index is logged and skipped on every request, so a
mistyped password costs a fresh dial, a login round trip and a warning line per
request, at traffic rate, and a full TLS handshake each time when redis_ssl is on.
The warning text already tells a refusal apart from an outage, so pausing hides
nothing. Can we start the pause on a refusal too, so a bad config costs one attempt
and one warning per worker per redis_breaker_seconds?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right and your reasoning beats mine. Changed in 898b2d3, both the auth and the SELECT refusal now start the pause.

My original thinking was that a config mistake should not be hidden, and as you say the pause hides nothing because the warning still names which failure it was. Meanwhile the cost you describe is real: a fresh dial, a login round trip and a warning per request at traffic rate, plus a TLS handshake each time with redis_ssl on.

One consequence to be aware of. Since the pool key is host, port, database, user and ssl, and deliberately not the password, two configs pointing at the same instance as the same user with different passwords now share a breaker, so the wrong one pauses the right one for redis_breaker_seconds. That needs a fairly specific misconfiguration and the log says exactly what happened, so I left it. Happy to put the password back in the key if you would rather not have that.

}, schema_def))

assert.same({ "sid", "other_session" }, ok.config.cache_cookie_names)
end)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 300 ceiling on cache_ttl is now the only thing keeping the cache under the token
lifetime, and no case here proves Kong enforces it. Can we add a case that a
cache_ttl of 301 is rejected, and one that a redis_timeout of 0 is rejected?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in ddf1341. A cache_ttl of 300 passes and 301 is rejected, redis_timeout of 1 passes and 0 is rejected, plus a case that cache_exp_skew, redis_keepalive_ms and redis_pool_size are no longer accepted at all.

Worth saying plainly: this spec needs spec.helpers, so I cannot run it without pongo. I verified every one of those assertions against a real gateway with kong config parse instead:

cache_ttl: 300        parse successful
cache_ttl: 301        value should be between 0 and 300
redis_timeout: 1      parse successful
redis_timeout: 0      value should be between 1 and 10000
cache_exp_skew        unknown field
redis_keepalive_ms    unknown field
redis_pool_size       unknown field

Which is a decent argument for your CI point.

@rohilsurana

Copy link
Copy Markdown
Member Author

On the CI question: yes, by hand. I ran them in Docker against kong:3.4 and kong:3.9 with a busted shim I wrote, not pongo. The numbers were real but nobody could reproduce them from the repo, which is a fair criticism.

Worth knowing before we do it: the specs have never run under real pongo, only under that shim. I already know 03-jwt_decoder_spec uses assert.is.truthy, which my shim does not implement, and 01-schema_spec needs spec.helpers, which I cannot load at all locally. Standing pongo up will likely surface a few more of those, so it is real work rather than a config file.

I would rather do it as its own PR than grow this one further. Shout if you would prefer it here.

The rest of your comments are addressed and pushed. The two I am holding for the next round are the cache key version and the double decode, both replied to inline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants