fix: jwt cache is not purged (#3801)
This commit is contained in:
@@ -27,6 +27,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
|
|||||||
- #3795, Clarify `Accept: vnd.pgrst.object` error message - @steve-chavez
|
- #3795, Clarify `Accept: vnd.pgrst.object` error message - @steve-chavez
|
||||||
- #3779, Always log the schema cache load time - @steve-chavez
|
- #3779, Always log the schema cache load time - @steve-chavez
|
||||||
- #3706, Fix insert with `missing=default` uses default value of domain instead of column - @taimoorzaeem
|
- #3706, Fix insert with `missing=default` uses default value of domain instead of column - @taimoorzaeem
|
||||||
|
- #3788, Fix jwt cache does not remove expired entries - @taimoorzaeem
|
||||||
|
|
||||||
### Changed
|
### Changed
|
||||||
|
|
||||||
|
|||||||
+24
-1
@@ -181,10 +181,33 @@ getJWTFromCache appState token maxLifetime parseJwt utc = do
|
|||||||
authResult <- maybe parseJwt (pure . Right) checkCache
|
authResult <- maybe parseJwt (pure . Right) checkCache
|
||||||
|
|
||||||
case (authResult,checkCache) of
|
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 ()
|
_ -> pure ()
|
||||||
|
|
||||||
return authResult
|
return authResult
|
||||||
|
where
|
||||||
|
jwtCache = getJwtCache appState
|
||||||
|
|
||||||
-- Used to extract JWT exp claim and add to JWT Cache
|
-- Used to extract JWT exp claim and add to JWT Cache
|
||||||
getTimeSpec :: AuthResult -> Int -> UTCTime -> Maybe TimeSpec
|
getTimeSpec :: AuthResult -> Int -> UTCTime -> Maybe TimeSpec
|
||||||
|
|||||||
@@ -1648,3 +1648,51 @@ def test_schema_cache_startup_load_with_in_db_config(defaultenv, metapostgrest):
|
|||||||
response = metapostgrest.session.post("/rpc/reset_db_schemas_config")
|
response = metapostgrest.session.post("/rpc/reset_db_schemas_config")
|
||||||
assert response.text == ""
|
assert response.text == ""
|
||||||
assert response.status_code == 204
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user