Skip to content

Commit 4a0d255

Browse files
committed
fix: jwt cache is not purged
1 parent e7cc8fe commit 4a0d255

File tree

3 files changed

+73
-1
lines changed

3 files changed

+73
-1
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
2626
- #3795, Clarify `Accept: vnd.pgrst.object` error message - @steve-chavez
2727
- #3779, Always log the schema cache load time - @steve-chavez
2828
- #3706, Fix insert with `missing=default` uses default value of domain instead of column - @taimoorzaeem
29+
- #3788, Fix jwt cache does not remove expired entries - @taimoorzaeem
2930

3031
### Changed
3132

src/PostgREST/Auth.hs

+24-1
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,33 @@ getJWTFromCache appState token maxLifetime parseJwt utc = do
181181
authResult <- maybe parseJwt (pure . Right) checkCache
182182

183183
case (authResult,checkCache) of
184-
(Right res, Nothing) -> C.insert' (getJwtCache appState) (getTimeSpec res maxLifetime utc) token res
184+
-- From comment:
185+
-- https://github.com/PostgREST/postgrest/pull/3801#discussion_r1857987914
186+
--
187+
-- We purge expired cache entries on a cache miss
188+
-- The reasoning is that:
189+
--
190+
-- 1. We expect it to be rare (otherwise there is no point of the cache)
191+
-- 2. It makes sure the cache is not growing (as inserting new entries
192+
-- does garbage collection)
193+
-- 3. Since this is time expiration based cache there is no real risk of
194+
-- starvation - sooner or later we are going to have a cache miss.
195+
196+
(Right res, Nothing) -> do -- cache miss
197+
198+
let timeSpec = getTimeSpec res maxLifetime utc
199+
200+
-- purge expired cache entries
201+
C.purgeExpired jwtCache
202+
203+
-- insert new cache entry
204+
C.insert' jwtCache timeSpec token res
205+
185206
_ -> pure ()
186207

187208
return authResult
209+
where
210+
jwtCache = getJwtCache appState
188211

189212
-- Used to extract JWT exp claim and add to JWT Cache
190213
getTimeSpec :: AuthResult -> Int -> UTCTime -> Maybe TimeSpec

test/io/test_io.py

+48
Original file line numberDiff line numberDiff line change
@@ -1648,3 +1648,51 @@ def test_schema_cache_startup_load_with_in_db_config(defaultenv, metapostgrest):
16481648
response = metapostgrest.session.post("/rpc/reset_db_schemas_config")
16491649
assert response.text == ""
16501650
assert response.status_code == 204
1651+
1652+
1653+
def test_jwt_cache_purges_expired_entries(defaultenv):
1654+
"test expired cache entries are purged on cache miss"
1655+
1656+
# The verification of actual cache size reduction is done locally
1657+
# This test is written for code coverage of purgeExpired function
1658+
1659+
relativeSeconds = lambda sec: int(
1660+
(datetime.now(timezone.utc) + timedelta(seconds=sec)).timestamp()
1661+
)
1662+
1663+
headers = lambda sec: jwtauthheader(
1664+
{"role": "postgrest_test_author", "exp": relativeSeconds(sec)},
1665+
SECRET,
1666+
)
1667+
1668+
env = {
1669+
**defaultenv,
1670+
"PGRST_JWT_CACHE_MAX_LIFETIME": "86400",
1671+
"PGRST_JWT_SECRET": SECRET,
1672+
"PGRST_DB_CONFIG": "false",
1673+
}
1674+
1675+
with run(env=env) as postgrest:
1676+
1677+
# Generate two unique JWT tokens
1678+
# The 1 second sleep is needed for it generate a unique token
1679+
hdrs1 = headers(5)
1680+
postgrest.session.get("/authors_only", headers=hdrs1)
1681+
1682+
time.sleep(1)
1683+
1684+
hdrs2 = headers(5)
1685+
postgrest.session.get("/authors_only", headers=hdrs2)
1686+
1687+
# Wait 5 seconds for the tokens to expire
1688+
time.sleep(5)
1689+
1690+
hdrs3 = headers(5)
1691+
1692+
# Make another request which should cause a cache miss and so
1693+
# the purgeExpired function will be triggered.
1694+
#
1695+
# This should remove the 2 expired tokens but adds another to cache
1696+
response = postgrest.session.get("/authors_only", headers=hdrs3)
1697+
1698+
assert response.status_code == 200

0 commit comments

Comments
 (0)