Skip to content

Commit

Permalink
fix: jwt cache is not purged (#3801)
Browse files Browse the repository at this point in the history
  • Loading branch information
taimoorzaeem authored and steve-chavez committed Jan 29, 2025
1 parent c6b4fca commit 89be285
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased

### Fixed

- #3788, Fix jwt cache does not remove expired entries - @taimoorzaeem

## [12.2.5] - 2025-01-20

### Fixed
Expand Down
25 changes: 24 additions & 1 deletion src/PostgREST/Auth.hs
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,33 @@ getJWTFromCache appState token maxLifetime parseJwt utc = do
authResult <- maybe parseJwt (pure . Right) checkCache

case (authResult,checkCache) of
(Right res, Nothing) -> C.insert' (getJwtCache appState) (getTimeSpec res maxLifetime utc) token res
-- From comment:
-- https://github.com/PostgREST/postgrest/pull/3801#discussion_r1857987914
--
-- We purge expired cache entries on a cache miss
-- The reasoning is that:
--
-- 1. We expect it to be rare (otherwise there is no point of the cache)
-- 2. It makes sure the cache is not growing (as inserting new entries
-- does garbage collection)
-- 3. Since this is time expiration based cache there is no real risk of
-- starvation - sooner or later we are going to have a cache miss.

(Right res, Nothing) -> do -- cache miss

let timeSpec = getTimeSpec res maxLifetime utc

-- purge expired cache entries
C.purgeExpired jwtCache

-- insert new cache entry
C.insert' jwtCache timeSpec token res

_ -> pure ()

return authResult
where
jwtCache = getJwtCache appState

-- Used to extract JWT exp claim and add to JWT Cache
getTimeSpec :: AuthResult -> Int -> UTCTime -> Maybe TimeSpec
Expand Down
48 changes: 48 additions & 0 deletions test/io/test_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -1596,3 +1596,51 @@ def test_schema_cache_startup_load_with_in_db_config(defaultenv, metapostgrest):
response = metapostgrest.session.post("/rpc/reset_db_schemas_config")
assert response.text == ""
assert response.status_code == 204


def test_jwt_cache_purges_expired_entries(defaultenv):
"test expired cache entries are purged on cache miss"

# The verification of actual cache size reduction is done manually, see https://github.com/PostgREST/postgrest/pull/3801#issuecomment-2620776041
# This test is written for code coverage of purgeExpired function

relativeSeconds = lambda sec: int(
(datetime.now(timezone.utc) + timedelta(seconds=sec)).timestamp()
)

headers = lambda sec: jwtauthheader(
{"role": "postgrest_test_author", "exp": relativeSeconds(sec)},
SECRET,
)

env = {
**defaultenv,
"PGRST_JWT_CACHE_MAX_LIFETIME": "86400",
"PGRST_JWT_SECRET": SECRET,
"PGRST_DB_CONFIG": "false",
}

with run(env=env) as postgrest:

# Generate two unique JWT tokens
# The 1 second sleep is needed for it generate a unique token
hdrs1 = headers(5)
postgrest.session.get("/authors_only", headers=hdrs1)

time.sleep(1)

hdrs2 = headers(5)
postgrest.session.get("/authors_only", headers=hdrs2)

# Wait 5 seconds for the tokens to expire
time.sleep(5)

hdrs3 = headers(5)

# Make another request which should cause a cache miss and so
# the purgeExpired function will be triggered.
#
# This should remove the 2 expired tokens but adds another to cache
response = postgrest.session.get("/authors_only", headers=hdrs3)

assert response.status_code == 200

0 comments on commit 89be285

Please sign in to comment.