Add Redis-backed caching for AuthN/AuthZ - #39
Open
ianmuchyri wants to merge 13 commits into
Open
Conversation
Introduces a race-safe caching layer for authentication/authorization decision inputs: a Redis hash per key with version/dirty/payload fields and three atomic Lua primitives (begin/end/try_populate) that prevent a reader from repopulating stale data around a concurrent mutation. Caching is off by default and fully optional - disabled, this is a no-op passthrough to Postgres. This commit adds the client, config, health/metrics scaffolding, and AppState/main wiring; nothing consumes it yet.
Routes JWT and API-key authentication through the cache client added previously, guarded by the version/dirty barrier so a revoked session, deactivated entity/tenant, or revoked credential is denied immediately even when caching is enabled - never served stale for the cache TTL. Wires invalidation into every mutation that can affect these: session revoke/refresh, entity and tenant status changes and soft-delete/ restore, and credential revoke/rotate/verifier-upgrade, including the session and credential fallout of tenant/entity soft-delete that a naive "invalidate only the status field" approach would miss once the tenant/entity is later restored.
Caches each subject's effective grant expansion, invalidated whenever a direct policy, role assignment, role's linked permission blocks, or group membership/hierarchy/status changes it. For a group or role subject, enumerating "which subjects are affected" and invalidating their cache entries is done under the same Postgres row lock add_group_member/remove_group_member take, closing a race where a membership change concurrent with the enumeration could leave a new member's stale grant cached until TTL. That lock is acquired in a fixed, consistent order (role before group) everywhere it's needed, to rule out a lock-order deadlock between concurrent admin operations.
Covers direct policies, role assignments, group membership/hierarchy, sessions, credentials, tenant/entity soft-delete and restore, API-key tenant context after an entity moves tenants, and the group/role lock ordering - each warms the cache, mutates through the real GraphQL path, and asserts the effect is visible on the very next read with no wait, never relying on a short TTL happening to expire.
deleteEntity, deleteTenant, restoreTenant, and password reset enumerated affected session/credential cache keys via a plain pool query before any transaction or lock was taken. A session or credential created concurrently in that window was never covered by the cache barrier and could keep serving as a stale hit indefinitely, even past a later restore. Fixed by moving enumeration inside the same transaction as the row-locking mutation that starts each of these flows, mirroring the existing group-membership invalidation fix. Adds a shared begin/end helper for establishing a multi-category cache barrier around an already-open transaction, and concurrency tests proving the lock now closes the window.
The cache-gated tests (src/cache/mod.rs's unit tests and tests/m25_cache_invalidation.rs) are #[ignore]d so they're skippable without Redis, but CI runs the full suite with --include-ignored and had no Redis service or ATOM_TEST_REDIS_URL configured — they panicked on missing config, which aborted the test script before the per-binary loop even reached the integration test files.
Condenses the repeated "body, minus opening/committing its own transaction" template across the _in_tx twin functions in authz/repo.rs and identity/repo.rs into a short pointer plus the caller-specific detail. Also removes resolver-level comments that fully restated their own repo function's doc comment (delete_entity, delete_tenant, restore_tenant, reset_password).
arvindh123
reviewed
Jul 30, 2026
arvindh123
left a comment
arvindh123
left a comment
Contributor
There was a problem hiding this comment.
@codex review and explain how the Redis cache helps and how the cache is invalidate on change of new data
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
From the code i see credentials are cached |
Cross-tenant tenant_status poisoning: the JWT miss path built the tenant_status key from the token's `tid` claim but populated it with the payload of the entity's *current* tenant, since the miss loader joins tenants through entities.tenant_id. A token outliving a tenant move wrote one tenant's status under another tenant's key, and because the populate ran before check_session_entity_tenant, even the request rejected for the tid mismatch poisoned it — marking a frozen tenant active for all its members until the TTL elapsed. Now populates only when the observed version belongs to the key the payload describes. The API-key path had the same shape in its credential-hit/entity-miss branch, keying entity_status off the cached credential's entity_id while taking the payload from the fresh row. REST handlers were left behind when the GraphQL resolvers were wired to the cache: delete_entity invalidated only entity_status, so a later restoreEntity resurrected sessions and access tokens that Postgres had revoked and restore deliberately never reinstates; add/remove_group_member did no grants invalidation at all, leaving a removed member exercising the group's roles until the grants TTL. Entity deletion now has one entry point, identity::service::delete_entity, shared by both callers. Drop the deleted_at fields from the entity/tenant cache entries. Both miss loaders filter deleted_at IS NULL, so the cached copy was always None by construction and no check ever read it — a dead copy of a security predicate that implied a tombstone check existed where none did. Guardrail validators now take &mut PgConnection and run on the caller's transaction. The *_in_tx refactor left them reading through the pool while the resolver held an open transaction with FOR UPDATE locks: the reads could not see the locked state they were meant to validate against, and acquiring a second connection per in-flight mutation deadlocks the pool under concurrency. Also: the barrier end script re-applies PEXPIRE, since HINCRBY recreates an already-expired key and left it immortal; barrier_ttl saturates and ATOM_CACHE_TTL_* is bounded at startup rather than panicking Duration multiplication on the mutation path; remove_group_member takes the group row lock its sibling takes, which the closure-locking helper's doc already claimed as an invariant. CI flushes Redis with the database recreate between test binaries. m25 caches under keys derived from the fixed seeded admin id, so the admin's grant expansion outlived the database it described and authorized against a tenant graph that no longer existed.
Signed-off-by: Arvindh <arvindh91@gmail.com>
Add cache::invalidate::guarded_tx_mutation and route all twelve
locked-transaction invalidation sites through it. Each had its own hand-
written copy of the same sequence — open transaction, lock and enumerate,
begin barrier, mutate, commit, end barrier — so each independently
re-derived the ordering that makes the barrier correct, and each was its
own opportunity to drop the end call on an error path. Stable Rust has no
async closures, so the helper takes closures returning a boxed future
borrowing the transaction; call sites spell that as
|tx| Box::pin(async move { .. }).
Fix a call site missed by the previous commit: create_role_assignment_in_tx
still called the pool variant of validate_role_assignment while holding
lock_live_subject and lock_role on its own transaction. That is the same
defect the previous commit fixed elsewhere — the validation could not see
the state it was locking, and the second connection risked exhausting the
pool. With it routed through the transaction, the pool-based
validate_role_assignment and validate_subject_boundary are unreachable and
are removed.
Record post-miss cache writes as atom_cache_populate_total, labelled
applied|stale|error. try_populate was the one cache operation with no
metric, and it discarded the script's return value, so a hit rate stuck at
zero could not be told apart from every write being rejected by a stuck
barrier.
Read the auth path's independent keys in one pipelined round trip.
lookup_many issues them on a single pooled connection and decode parses
each into its own type; a full-hit JWT authentication was three pool
acquisitions and three serial round trips, each bounded by op_timeout,
before any request work started. begin and end likewise hold one
connection across all their chunks instead of re-acquiring per 500 keys
while already holding Postgres row locks.
Stop cloning the signing key on every refreshSession. LoadedKey holds the
raw PKCS8 private-key PEM, and the clone existed only to escape the
ActiveKeys read guard; JwtSigner carries the parsed key that signing needs
anyway, so the guard is released without duplicating key material.
Remove guarded_multi_mutation, which was never called: both multi-category
sites hold an open transaction and use begin_all/end_all instead.
Signed-off-by: Arvindh <arvindh91@gmail.com>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
atom-docs | 339ddfb | Commit Preview URL Branch Preview URL |
Jul 31 2026, 11:24 AM |
An enabled cache that cannot reach Redis at startup no longer degrades to `None`. `None` means "caching is not configured", and every mutation guard becomes a pass-through on that basis — so a replica that booted during a Redis outage went on mutating grants, sessions and credentials without invalidating entries other replicas were still serving, leaving a revoke here authorized there. Unreachable Redis is a runtime condition, not a configuration one: `CacheClient::build` now separates pool construction (fatal on a bad URL) from `probe` (retryable), and startup keeps the client either way. The client's own behavior already covers the outage — reads fall through to Postgres as misses, and `begin` fails, so security-sensitive mutations are refused until Redis returns. ATOM_CACHE_FAIL_FAST_ON_STARTUP still decides whether to abort instead of booting into that refusing state. Discarding a corrupt payload no longer deletes the whole hash. `DEL` took `v` and `dirty` with it, so a cleanup landing after a concurrent `begin` destroyed that mutation's barrier: the next reader saw an absent key, observed version 0, loaded pre-commit state, and populated it successfully. The new discard script clears only `p`, guarded by the same version/dirty check `try_populate` uses. `end` also clears `p` defensively, which is what its contract already claimed. createTenant invalidates the creator's grants. It bootstraps a tenant-admin role, assignment and membership for the creator in the same transaction, growing their grant set, while the capability gate immediately above warms that exact key — so a creator without platform-wide manage could not administer the tenant they had just created until the grants TTL lapsed.
Signed-off-by: dusan <borovcanindusan1@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an optional Redis cache in front of Atom's per-request authentication and authorization checks (session/JWT validation, API-key verification, entity/tenant status, and effective grant expansion), which today hit Postgres on every request. Caching is off by default and behaves as a pure no-op when disabled.
The cache uses a version/dirty barrier around every security-sensitive mutation rather than plain TTL expiry, so a revoked session, deactivated entity/tenant, revoked or rotated credential, or changed policy/role/group membership is denied on the very next request - never served stale for the cache's TTL, including under concurrent read/write races. Group- and role-subject invalidation additionally locks the affected rows (in a fixed, consistent order) to close a race where a concurrent membership change could leave a new member's grant stale, and to rule out a lock-order deadlock between concurrent admin operations.
Test plan
cargo fmt,cargo clippy(default and--no-default-features) cleanResolves #36