From bddcc5ddc5a18cd39b4219c21e852cf7712b614f Mon Sep 17 00:00:00 2001 From: Taimoor Zaeem Date: Mon, 21 Apr 2025 11:12:32 +0500 Subject: [PATCH] feat: change JWT cache to limited LRU based cache BREAKING CHANGE Our JWT cache implementation had no upper bound for number of cache entries. This caused OOM errors. Additionally, the purge mechanism for expired entries was quite slow. This changes our implementation to a LRU based cache which limits the amount of cached entries. --- CHANGELOG.md | 2 + docs/postgrest.dict | 3 + docs/references/auth.rst | 4 +- docs/references/configuration.rst | 14 +- nix/tools/loadtest.nix | 2 +- postgrest.cabal | 3 +- src/PostgREST/AppState.hs | 24 +-- src/PostgREST/Auth.hs | 37 +++-- src/PostgREST/Auth/JwtCache.hs | 144 +++++++++--------- src/PostgREST/CLI.hs | 4 +- src/PostgREST/Config.hs | 6 +- src/PostgREST/Config/Database.hs | 2 +- test/io/configs/expected/aliases.config | 2 +- .../configs/expected/boolean-numeric.config | 2 +- .../io/configs/expected/boolean-string.config | 2 +- test/io/configs/expected/defaults.config | 2 +- .../expected/jwt-role-claim-key1.config | 2 +- .../expected/jwt-role-claim-key2.config | 2 +- .../expected/jwt-role-claim-key3.config | 2 +- .../expected/jwt-role-claim-key4.config | 2 +- .../expected/jwt-role-claim-key5.config | 2 +- ...efaults-with-db-other-authenticator.config | 2 +- .../expected/no-defaults-with-db.config | 2 +- test/io/configs/expected/no-defaults.config | 2 +- test/io/configs/expected/types.config | 2 +- test/io/configs/no-defaults-env.yaml | 2 +- test/io/configs/no-defaults.config | 2 +- test/io/db_config.sql | 4 +- test/io/test_io.py | 103 ++++++------- test/spec/Main.hs | 2 +- test/spec/SpecHelper.hs | 2 +- 31 files changed, 193 insertions(+), 193 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bb2cf82a7..3c63ddf461 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). + The selected columns in the embedded resources are aggregated into arrays + Aggregates are not supported - #2967, Add `Proxy-Status` header for better error response - @taimoorzaeem + - #4003, Add config `jwt-cache-max-entries` for maximum number of cached entries - @taimoorzaeem ### Fixed @@ -55,6 +56,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). + Diagnostic error messages instead of exposed internals + Return new `PGRST303` error when jwt claims decoding fails - #3906, Return `PGRST125` and `PGRST126` errors instead of empty json - @taimoorzaeem + - #4003, Remove config `jwt-cache-max-lifetime` and add config `jwt-cache-max-entries` for JWT cache - @taimoorzaeem ## [12.2.10] - 2025-04-18 diff --git a/docs/postgrest.dict b/docs/postgrest.dict index 4aa425591b..3de61dc5ce 100644 --- a/docs/postgrest.dict +++ b/docs/postgrest.dict @@ -23,6 +23,7 @@ Cloudflare config cors CORS +cryptographic cryptographically CSV durations @@ -74,6 +75,7 @@ JSON JSPath JWK JWT +JWTs jwt Keycloak Kubernetes @@ -84,6 +86,7 @@ Logins LIBPQ logins lon +LRU lt lte macOS diff --git a/docs/references/auth.rst b/docs/references/auth.rst index eab4f7b3cc..a7a6e624d8 100644 --- a/docs/references/auth.rst +++ b/docs/references/auth.rst @@ -98,9 +98,9 @@ The ``Bearer`` header value can be used with or without capitalization(``bearer` JWT Caching ----------- -PostgREST validates ``JWTs`` on every request. We can cache ``JWTs`` to avoid this performance overhead. +PostgREST caches ``JWTs`` on every request to avoid performance overhead of parsing and cryptographic operations. -To enable JWT caching, the config :code:`jwt-cache-max-lifetime` is to be set. It is the maximum number of seconds for which the cache stores the JWT validation results. The cache uses the :code:`exp` claim to set the cache entry lifetime. If the JWT does not have an :code:`exp` claim, it uses the config value. See :ref:`jwt-cache-max-lifetime` for more details. +To disable JWT caching, the config :code:`jwt-cache-max-entries` is to be set to ``0``. It is the maximum number of JWTs for which the cache stores their validation results. If the cache reaches its maximum, the `least recently used `_ entry will be removed. The cache honors :code:`exp` claim. If the JWT does not have an :code:`exp` claim, it is cached until it gets removed by the LRU policy. See :ref:`jwt-cache-max-entries` for configuration details. .. note:: diff --git a/docs/references/configuration.rst b/docs/references/configuration.rst index 5c179e82fd..0c88dbc995 100644 --- a/docs/references/configuration.rst +++ b/docs/references/configuration.rst @@ -658,20 +658,20 @@ jwt-secret-is-base64 When this is set to :code:`true`, the value derived from :code:`jwt-secret` will be treated as a base64 encoded secret. -.. _jwt-cache-max-lifetime: +.. _jwt-cache-max-entries: -jwt-cache-max-lifetime ----------------------- +jwt-cache-max-entries +--------------------- =============== ================================= **Type** Int - **Default** 0 + **Default** 1000 **Reloadable** Y - **Environment** PGRST_JWT_CACHE_MAX_LIFETIME - **In-Database** pgrst.jwt_cache_max_lifetime + **Environment** PGRST_JWT_CACHE_MAX_ENTRIES + **In-Database** pgrst.jwt_cache_max_entries =============== ================================= - Maximum number of seconds of lifetime for cached entries. The default :code:`0` disables caching. See :ref:`jwt_caching`. + Maximum number of JWTs that can be cached. Set to :code:`0` to disable caching. See :ref:`jwt_caching`. .. _log-level: diff --git a/nix/tools/loadtest.nix b/nix/tools/loadtest.nix index 7ac9f1cf88..77fc734355 100644 --- a/nix/tools/loadtest.nix +++ b/nix/tools/loadtest.nix @@ -58,7 +58,7 @@ let export PGRST_DB_TX_END="rollback-allow-override" export PGRST_LOG_LEVEL="crit" export PGRST_JWT_SECRET="reallyreallyreallyreallyverysafe" - export PGRST_JWT_CACHE_MAX_LIFETIME="86400" + export PGRST_JWT_CACHE_MAX_ENTRIES="1000" # default mkdir -p "$(dirname "$_arg_output")" abs_output="$(realpath "$_arg_output")" diff --git a/postgrest.cabal b/postgrest.cabal index f06fe783b5..f148c926cd 100644 --- a/postgrest.cabal +++ b/postgrest.cabal @@ -99,10 +99,8 @@ library , auto-update >= 0.1.4 && < 0.2 , base64-bytestring >= 1 && < 1.3 , bytestring >= 0.10.8 && < 0.13 - , cache >= 0.1.3 && < 0.2.0 , case-insensitive >= 1.2 && < 1.3 , cassava >= 0.4.5 && < 0.6 - , clock >= 0.8.3 && < 0.9.0 , configurator-pg >= 0.2 && < 0.3 , containers >= 0.5.7 && < 0.7 , cookie >= 0.4.2 && < 0.5 @@ -122,6 +120,7 @@ library , jose-jwt >= 0.9.6 && < 0.11 , lens >= 4.14 && < 5.3 , lens-aeson >= 1.0.1 && < 1.3 + , lrucache >= 1.2.0.1 && < 1.3 , mtl >= 2.2.2 && < 2.4 , neat-interpolation >= 0.5 && < 0.6 , network >= 2.6 && < 3.2 diff --git a/src/PostgREST/AppState.hs b/src/PostgREST/AppState.hs index 3e2a588f55..ade3e9235b 100644 --- a/src/PostgREST/AppState.hs +++ b/src/PostgREST/AppState.hs @@ -57,7 +57,6 @@ import Data.IORef (IORef, atomicWriteIORef, newIORef, readIORef) import Data.Time.Clock (UTCTime, getCurrentTime) -import PostgREST.Auth.JwtCache (JwtCacheState) import PostgREST.Config (AppConfig (..), addFallbackAppName, readAppConfig) @@ -105,8 +104,8 @@ data AppState = AppState , stateSocketAdmin :: Maybe NS.Socket -- | Observation handler , stateObserver :: ObservationHandler - -- | JWT Cache - , stateJwtCache :: JwtCache.JwtCacheState + -- | JWT Cache, disabled when config jwt-cache-max-entries is set to 0 + , stateJwtCache :: IORef JwtCache.JwtCacheState , stateLogger :: Logger.LoggerState , stateMetrics :: Metrics.MetricsState } @@ -120,14 +119,14 @@ data SchemaCacheStatus type AppSockets = (NS.Socket, Maybe NS.Socket) init :: AppConfig -> IO AppState -init conf@AppConfig{configLogLevel, configDbPoolSize} = do +init conf = do loggerState <- Logger.init - metricsState <- Metrics.init configDbPoolSize - let observer = liftA2 (>>) (Logger.observationLogger loggerState configLogLevel) (Metrics.observationMetrics metricsState) + metricsState <- Metrics.init (configDbPoolSize conf) + let observer = liftA2 (>>) (Logger.observationLogger loggerState (configLogLevel conf)) (Metrics.observationMetrics metricsState) observer $ AppStartObs prettyVersion - jwtCacheState <- JwtCache.init + jwtCacheState <- JwtCache.init (configJwtCacheMaxEntries conf) pool <- initPool conf observer (sock, adminSock) <- initSockets conf state' <- initWithPool (sock, adminSock) pool conf jwtCacheState loggerState metricsState observer @@ -150,7 +149,7 @@ initWithPool (sock, adminSock) pool conf jwtCacheState loggerState metricsState <*> pure sock <*> pure adminSock <*> pure observer - <*> pure jwtCacheState + <*> newIORef jwtCacheState <*> pure loggerState <*> pure metricsState @@ -311,8 +310,8 @@ putConfig = atomicWriteIORef . stateConf getTime :: AppState -> IO UTCTime getTime = stateGetTime -getJwtCacheState :: AppState -> JwtCacheState -getJwtCacheState = stateJwtCache +getJwtCacheState :: AppState -> IO JwtCache.JwtCacheState +getJwtCacheState = readIORef . stateJwtCache getSocketREST :: AppState -> NS.Socket getSocketREST = stateSocketREST @@ -473,8 +472,9 @@ readInDbConfig startingUp appState@AppState{stateObserver=observer} = do -- entries, because they were cached using the old secret if configJwtSecret conf == configJwtSecret newConf then pass - else - JwtCache.emptyCache (getJwtCacheState appState) -- atomic O(1) operation + else do + newJwtCacheState <- JwtCache.init (configJwtCacheMaxEntries newConf) + atomicWriteIORef (stateJwtCache appState) newJwtCacheState if startingUp then pass diff --git a/src/PostgREST/Auth.hs b/src/PostgREST/Auth.hs index cbcef6b5ec..3cb7935a5b 100644 --- a/src/PostgREST/Auth.hs +++ b/src/PostgREST/Auth.hs @@ -160,30 +160,27 @@ middleware appState app req respond = do let token = Wai.extractBearerAuth =<< lookup HTTP.hAuthorization (Wai.requestHeaders req) parseJwt = runExceptT $ parseToken conf token time >>= parseClaims conf - jwtCacheState = getJwtCacheState appState + + jwtCacheState <- getJwtCacheState appState -- If ServerTimingEnabled -> calculate JWT validation time --- If JwtCacheMaxLifetime -> cache JWT validation result - req' <- case (configServerTimingEnabled conf, configJwtCacheMaxLifetime conf) of - (True, 0) -> do - (dur, authResult) <- timeItT parseJwt - return $ req { Wai.vault = Wai.vault req & Vault.insert authResultKey authResult & Vault.insert jwtDurKey dur } - - (True, maxLifetime) -> do - (dur, authResult) <- timeItT $ case token of - Just tkn -> lookupJwtCache jwtCacheState tkn maxLifetime parseJwt time - Nothing -> parseJwt - return $ req { Wai.vault = Wai.vault req & Vault.insert authResultKey authResult & Vault.insert jwtDurKey dur } + req' <- if configServerTimingEnabled conf then do + + (dur, authResult) <- timeItT $ case token of + + Just tkn -> lookupJwtCache jwtCacheState tkn parseJwt time + Nothing -> parseJwt - (False, 0) -> do - authResult <- parseJwt - return $ req { Wai.vault = Wai.vault req & Vault.insert authResultKey authResult } + return $ req { Wai.vault = Wai.vault req & Vault.insert authResultKey authResult & Vault.insert jwtDurKey dur } + + else do + + authResult <- case token of + + Just tkn -> lookupJwtCache jwtCacheState tkn parseJwt time + Nothing -> parseJwt - (False, maxLifetime) -> do - authResult <- case token of - Just tkn -> lookupJwtCache jwtCacheState tkn maxLifetime parseJwt time - Nothing -> parseJwt - return $ req { Wai.vault = Wai.vault req & Vault.insert authResultKey authResult } + return $ req { Wai.vault = Wai.vault req & Vault.insert authResultKey authResult } app req' respond diff --git a/src/PostgREST/Auth/JwtCache.hs b/src/PostgREST/Auth/JwtCache.hs index 79d511201e..db13488729 100644 --- a/src/PostgREST/Auth/JwtCache.hs +++ b/src/PostgREST/Auth/JwtCache.hs @@ -4,96 +4,100 @@ Description : PostgREST Jwt Authentication Result Cache. This module provides functions to deal with the JWT cache -} -{-# LANGUAGE NamedFieldPuns #-} module PostgREST.Auth.JwtCache ( init , JwtCacheState , lookupJwtCache - , emptyCache ) where import qualified Data.Aeson as JSON import qualified Data.Aeson.KeyMap as KM -import qualified Data.Cache as C +import qualified Data.Cache.LRU as C +import qualified Data.IORef as I import qualified Data.Scientific as Sci -import Control.Debounce - import Data.Time.Clock (UTCTime, nominalDiffTimeToSeconds) import Data.Time.Clock.POSIX (utcTimeToPOSIXSeconds) -import System.Clock (TimeSpec (..)) +import GHC.Num (integerFromInt) import PostgREST.Auth.Types (AuthResult (..)) import PostgREST.Error (Error (..)) import Protolude --- | JWT Cache and IO action that triggers purging old entries from the cache -data JwtCacheState = JwtCacheState - { jwtCache :: C.Cache ByteString AuthResult - , purgeCache :: IO () +-- | Jwt Cache State +newtype JwtCacheState = JwtCacheState + { maybeJwtCache :: Maybe (I.IORef (C.LRU ByteString AuthResult)) } -- | Initialize JwtCacheState -init :: IO JwtCacheState -init = do - cache <- C.newCache Nothing -- no default expiration - -- purgeExpired has O(n^2) complexity - -- so we wrap it in debounce to make sure it: - -- 1) is executed asynchronously - -- 2) only a single purge operation is running at a time - debounce <- mkDebounce defaultDebounceSettings - -- debounceFreq is set to default 1 second - { debounceAction = C.purgeExpired cache - , debounceEdge = leadingEdge - } - pure $ JwtCacheState cache debounce +init :: Int -> IO JwtCacheState +init 0 = return $ JwtCacheState Nothing +init maxEntries = do + cache <- I.newIORef $ C.newLRU (Just $ integerFromInt maxEntries) + return $ JwtCacheState $ Just cache + -- | Used to retrieve and insert JWT to JWT Cache -lookupJwtCache :: JwtCacheState -> ByteString -> Int -> IO (Either Error AuthResult) -> UTCTime -> IO (Either Error AuthResult) -lookupJwtCache JwtCacheState{jwtCache, purgeCache} token maxLifetime parseJwt utc = do - checkCache <- C.lookup jwtCache token - authResult <- maybe parseJwt (pure . Right) checkCache - - case (authResult,checkCache) of - -- 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 - - -- insert new cache entry - C.insert' jwtCache (Just timeSpec) token res - - -- Execute IO action to purge the cache - -- It is assumed this action returns immidiately - -- so that request processing is not blocked. - purgeCache - - _ -> pure () - - return authResult - --- Used to extract JWT exp claim and add to JWT Cache -getTimeSpec :: AuthResult -> Int -> UTCTime -> TimeSpec -getTimeSpec res maxLifetime utc = do - let expireJSON = KM.lookup "exp" (authClaims res) - utcToSecs = floor . nominalDiffTimeToSeconds . utcTimeToPOSIXSeconds - sciToInt = fromMaybe 0 . Sci.toBoundedInteger - case expireJSON of - Just (JSON.Number seconds) -> TimeSpec (sciToInt seconds - utcToSecs utc) 0 - _ -> TimeSpec (fromIntegral maxLifetime :: Int64) 0 - --- | Empty the cache (done when the config is reloaded) -emptyCache :: JwtCacheState -> IO () -emptyCache JwtCacheState{jwtCache} = C.purge jwtCache +lookupJwtCache :: JwtCacheState -> ByteString -> IO (Either Error AuthResult) -> UTCTime -> IO (Either Error AuthResult) +lookupJwtCache jwtCacheState token parseJwt utc = do + case maybeJwtCache jwtCacheState of + Nothing -> parseJwt + Just jwtCacheIORef -> do + -- get cache from IORef + jwtCache <- I.readIORef jwtCacheIORef + + -- MAKE SURE WE UPDATE THE CACHE ON ALL PATHS AFTER LOOKUP + -- This is because it is a pure LRU cache, so lookup returns the + -- the cache with new state, hence it should be updated + let (jwtCache', maybeVal) = C.lookup token jwtCache + + case maybeVal of + Nothing -> do -- CACHE MISS + + -- When we get a cache miss, we get the parse result, insert it + -- into the cache. After that, we write the cache IO ref with + -- updated cache + authResult <- parseJwt + + case authResult of + Right result -> do + -- insert token -> update cache -> return token + let jwtCache'' = C.insert token result jwtCache' + I.writeIORef jwtCacheIORef jwtCache'' + return $ Right result + Left e -> do + -- update cache after lookup -> return error + I.writeIORef jwtCacheIORef jwtCache' + return $ Left e + + Just result -> -- CACHE HIT + + -- For cache hit, we get the result from cache, we check the + -- exp claim. If it expired, we delete it from cache and parse + -- the jwt. Otherwise, the hit result is valid, so we return it + + if isExpClaimExpired result utc then do + -- delete token -> update cache -> parse token + let (jwtCache'',_) = C.delete token jwtCache' + I.writeIORef jwtCacheIORef jwtCache'' + parseJwt + else do + -- update cache after lookup -> return result + I.writeIORef jwtCacheIORef jwtCache' + return $ Right result + + +type Expired = Bool + +-- | Check if exp claim is expired when looked up from cache +isExpClaimExpired :: AuthResult -> UTCTime -> Expired +isExpClaimExpired result utc = + case expireJSON of + Nothing -> False -- if exp not present then it is valid + Just (JSON.Number expiredAt) -> (sciToInt expiredAt - now) < 0 + Just _ -> False -- if exp is not a number then valid + where + expireJSON = KM.lookup "exp" (authClaims result) + now = (floor . nominalDiffTimeToSeconds . utcTimeToPOSIXSeconds) utc :: Int + sciToInt = fromMaybe 0 . Sci.toBoundedInteger diff --git a/src/PostgREST/CLI.hs b/src/PostgREST/CLI.hs index e481d4284b..5582f63bff 100644 --- a/src/PostgREST/CLI.hs +++ b/src/PostgREST/CLI.hs @@ -203,8 +203,8 @@ exampleConfigFile = |# jwt-secret = "secret_with_at_least_32_characters" |jwt-secret-is-base64 = false | - |## Enables and set JWT Cache max lifetime, disables caching with 0 - |# jwt-cache-max-lifetime = 0 + |## Maximum number of auth token that can be cached + |# jwt-cache-max-entries = 1000 | |## Logging level, the admitted values are: crit, error, warn, info and debug. |log-level = "error" diff --git a/src/PostgREST/Config.hs b/src/PostgREST/Config.hs index a9cd9a949d..b6d936cc68 100644 --- a/src/PostgREST/Config.hs +++ b/src/PostgREST/Config.hs @@ -97,7 +97,7 @@ data AppConfig = AppConfig , configJwtRoleClaimKey :: JSPath , configJwtSecret :: Maybe BS.ByteString , configJwtSecretIsBase64 :: Bool - , configJwtCacheMaxLifetime :: Int + , configJwtCacheMaxEntries :: Int , configLogLevel :: LogLevel , configLogQuery :: LogQuery , configOpenApiMode :: OpenAPIMode @@ -177,7 +177,7 @@ toText conf = ,("jwt-role-claim-key", q . T.intercalate mempty . fmap dumpJSPath . configJwtRoleClaimKey) ,("jwt-secret", q . T.decodeUtf8 . showJwtSecret) ,("jwt-secret-is-base64", T.toLower . show . configJwtSecretIsBase64) - ,("jwt-cache-max-lifetime", show . configJwtCacheMaxLifetime) + ,("jwt-cache-max-entries", show . configJwtCacheMaxEntries) ,("log-level", q . dumpLogLevel . configLogLevel) ,("log-query", q . dumpLogQuery . configLogQuery) ,("openapi-mode", q . dumpOpenApiMode . configOpenApiMode) @@ -287,7 +287,7 @@ parser optPath env dbSettings roleSettings roleIsolationLvl = <*> (fromMaybe False <$> optWithAlias (optBool "jwt-secret-is-base64") (optBool "secret-is-base64")) - <*> (fromMaybe 0 <$> optInt "jwt-cache-max-lifetime") + <*> (fromMaybe 1000 <$> optInt "jwt-cache-max-entries") <*> parseLogLevel "log-level" <*> parseLogQuery "log-query" <*> parseOpenAPIMode "openapi-mode" diff --git a/src/PostgREST/Config/Database.hs b/src/PostgREST/Config/Database.hs index aff4b5b8a3..6835ea62ed 100644 --- a/src/PostgREST/Config/Database.hs +++ b/src/PostgREST/Config/Database.hs @@ -61,7 +61,7 @@ dbSettingsNames = ,"jwt_role_claim_key" ,"jwt_secret" ,"jwt_secret_is_base64" - ,"jwt_cache_max_lifetime" + ,"jwt_cache_max_entries" ,"openapi_mode" ,"openapi_security_active" ,"openapi_server_proxy_uri" diff --git a/test/io/configs/expected/aliases.config b/test/io/configs/expected/aliases.config index bb161df30e..f7bb9ac6e9 100644 --- a/test/io/configs/expected/aliases.config +++ b/test/io/configs/expected/aliases.config @@ -23,7 +23,7 @@ jwt-aud = "" jwt-role-claim-key = ".\"aliased\"" jwt-secret = "" jwt-secret-is-base64 = true -jwt-cache-max-lifetime = 0 +jwt-cache-max-entries = 1000 log-level = "error" log-query = "disabled" openapi-mode = "follow-privileges" diff --git a/test/io/configs/expected/boolean-numeric.config b/test/io/configs/expected/boolean-numeric.config index 0f52b08e94..a6b6bed23d 100644 --- a/test/io/configs/expected/boolean-numeric.config +++ b/test/io/configs/expected/boolean-numeric.config @@ -23,7 +23,7 @@ jwt-aud = "" jwt-role-claim-key = ".\"role\"" jwt-secret = "" jwt-secret-is-base64 = true -jwt-cache-max-lifetime = 0 +jwt-cache-max-entries = 1000 log-level = "error" log-query = "disabled" openapi-mode = "follow-privileges" diff --git a/test/io/configs/expected/boolean-string.config b/test/io/configs/expected/boolean-string.config index 0f52b08e94..a6b6bed23d 100644 --- a/test/io/configs/expected/boolean-string.config +++ b/test/io/configs/expected/boolean-string.config @@ -23,7 +23,7 @@ jwt-aud = "" jwt-role-claim-key = ".\"role\"" jwt-secret = "" jwt-secret-is-base64 = true -jwt-cache-max-lifetime = 0 +jwt-cache-max-entries = 1000 log-level = "error" log-query = "disabled" openapi-mode = "follow-privileges" diff --git a/test/io/configs/expected/defaults.config b/test/io/configs/expected/defaults.config index 0c7f98788a..86bd6416bb 100644 --- a/test/io/configs/expected/defaults.config +++ b/test/io/configs/expected/defaults.config @@ -23,7 +23,7 @@ jwt-aud = "" jwt-role-claim-key = ".\"role\"" jwt-secret = "" jwt-secret-is-base64 = false -jwt-cache-max-lifetime = 0 +jwt-cache-max-entries = 1000 log-level = "error" log-query = "disabled" openapi-mode = "follow-privileges" diff --git a/test/io/configs/expected/jwt-role-claim-key1.config b/test/io/configs/expected/jwt-role-claim-key1.config index c537df986f..2ffcc6c17d 100644 --- a/test/io/configs/expected/jwt-role-claim-key1.config +++ b/test/io/configs/expected/jwt-role-claim-key1.config @@ -23,7 +23,7 @@ jwt-aud = "" jwt-role-claim-key = ".\"roles\"[?(@ == \"role1\")]" jwt-secret = "" jwt-secret-is-base64 = false -jwt-cache-max-lifetime = 0 +jwt-cache-max-entries = 1000 log-level = "error" log-query = "disabled" openapi-mode = "follow-privileges" diff --git a/test/io/configs/expected/jwt-role-claim-key2.config b/test/io/configs/expected/jwt-role-claim-key2.config index 1fe113fae3..7573004452 100644 --- a/test/io/configs/expected/jwt-role-claim-key2.config +++ b/test/io/configs/expected/jwt-role-claim-key2.config @@ -23,7 +23,7 @@ jwt-aud = "" jwt-role-claim-key = ".\"roles\"[?(@ != \"role1\")]" jwt-secret = "" jwt-secret-is-base64 = false -jwt-cache-max-lifetime = 0 +jwt-cache-max-entries = 1000 log-level = "error" log-query = "disabled" openapi-mode = "follow-privileges" diff --git a/test/io/configs/expected/jwt-role-claim-key3.config b/test/io/configs/expected/jwt-role-claim-key3.config index c18a606a17..7bad741663 100644 --- a/test/io/configs/expected/jwt-role-claim-key3.config +++ b/test/io/configs/expected/jwt-role-claim-key3.config @@ -23,7 +23,7 @@ jwt-aud = "" jwt-role-claim-key = ".\"roles\"[?(@ ^== \"role1\")]" jwt-secret = "" jwt-secret-is-base64 = false -jwt-cache-max-lifetime = 0 +jwt-cache-max-entries = 1000 log-level = "error" log-query = "disabled" openapi-mode = "follow-privileges" diff --git a/test/io/configs/expected/jwt-role-claim-key4.config b/test/io/configs/expected/jwt-role-claim-key4.config index 6763104ade..6002d1f24b 100644 --- a/test/io/configs/expected/jwt-role-claim-key4.config +++ b/test/io/configs/expected/jwt-role-claim-key4.config @@ -23,7 +23,7 @@ jwt-aud = "" jwt-role-claim-key = ".\"roles\"[?(@ ==^ \"role1\")]" jwt-secret = "" jwt-secret-is-base64 = false -jwt-cache-max-lifetime = 0 +jwt-cache-max-entries = 1000 log-level = "error" log-query = "disabled" openapi-mode = "follow-privileges" diff --git a/test/io/configs/expected/jwt-role-claim-key5.config b/test/io/configs/expected/jwt-role-claim-key5.config index 13cead3cae..ae609af1e5 100644 --- a/test/io/configs/expected/jwt-role-claim-key5.config +++ b/test/io/configs/expected/jwt-role-claim-key5.config @@ -23,7 +23,7 @@ jwt-aud = "" jwt-role-claim-key = ".\"roles\"[?(@ *== \"role1\")]" jwt-secret = "" jwt-secret-is-base64 = false -jwt-cache-max-lifetime = 0 +jwt-cache-max-entries = 1000 log-level = "error" log-query = "disabled" openapi-mode = "follow-privileges" diff --git a/test/io/configs/expected/no-defaults-with-db-other-authenticator.config b/test/io/configs/expected/no-defaults-with-db-other-authenticator.config index d33b144c04..59a3ab244d 100644 --- a/test/io/configs/expected/no-defaults-with-db-other-authenticator.config +++ b/test/io/configs/expected/no-defaults-with-db-other-authenticator.config @@ -23,7 +23,7 @@ jwt-aud = "https://otherexample.org" jwt-role-claim-key = ".\"other\".\"pre_config_role\"" jwt-secret = "ODERREALLYREALLYREALLYREALLYVERYSAFE" jwt-secret-is-base64 = false -jwt-cache-max-lifetime = 7200 +jwt-cache-max-entries = 6000 log-level = "info" log-query = "main-query" openapi-mode = "disabled" diff --git a/test/io/configs/expected/no-defaults-with-db.config b/test/io/configs/expected/no-defaults-with-db.config index 5202d7f712..02bd1a2540 100644 --- a/test/io/configs/expected/no-defaults-with-db.config +++ b/test/io/configs/expected/no-defaults-with-db.config @@ -23,7 +23,7 @@ jwt-aud = "https://example.org" jwt-role-claim-key = ".\"a\".\"role\"" jwt-secret = "OVERRIDE=REALLY=REALLY=REALLY=REALLY=VERY=SAFE" jwt-secret-is-base64 = false -jwt-cache-max-lifetime = 3600 +jwt-cache-max-entries = 3000 log-level = "info" log-query = "main-query" openapi-mode = "ignore-privileges" diff --git a/test/io/configs/expected/no-defaults.config b/test/io/configs/expected/no-defaults.config index e5723fbdf9..2eb717bd90 100644 --- a/test/io/configs/expected/no-defaults.config +++ b/test/io/configs/expected/no-defaults.config @@ -23,7 +23,7 @@ jwt-aud = "https://postgrest.org" jwt-role-claim-key = ".\"user\"[0].\"real-role\"" jwt-secret = "c2VjdXJpdHl0aHJvdWdob2JzY3VyaXR5aW5iYXNlNjQ=" jwt-secret-is-base64 = true -jwt-cache-max-lifetime = 86400 +jwt-cache-max-entries = 10000 log-level = "info" log-query = "main-query" openapi-mode = "ignore-privileges" diff --git a/test/io/configs/expected/types.config b/test/io/configs/expected/types.config index 0b3a652cf0..b9ed5bc550 100644 --- a/test/io/configs/expected/types.config +++ b/test/io/configs/expected/types.config @@ -23,7 +23,7 @@ jwt-aud = "" jwt-role-claim-key = ".\"role\"" jwt-secret = "" jwt-secret-is-base64 = false -jwt-cache-max-lifetime = 0 +jwt-cache-max-entries = 1000 log-level = "error" log-query = "disabled" openapi-mode = "follow-privileges" diff --git a/test/io/configs/no-defaults-env.yaml b/test/io/configs/no-defaults-env.yaml index df5bc456ae..673ad65d4e 100644 --- a/test/io/configs/no-defaults-env.yaml +++ b/test/io/configs/no-defaults-env.yaml @@ -26,7 +26,7 @@ PGRST_JWT_AUD: 'https://postgrest.org' PGRST_JWT_ROLE_CLAIM_KEY: '.user[0]."real-role"' PGRST_JWT_SECRET: c2VjdXJpdHl0aHJvdWdob2JzY3VyaXR5aW5iYXNlNjQ= PGRST_JWT_SECRET_IS_BASE64: true -PGRST_JWT_CACHE_MAX_LIFETIME: 86400 +PGRST_JWT_CACHE_MAX_ENTRIES: 10000 PGRST_LOG_LEVEL: info PGRST_LOG_QUERY: 'main-query' PGRST_OPENAPI_MODE: 'ignore-privileges' diff --git a/test/io/configs/no-defaults.config b/test/io/configs/no-defaults.config index f730d1685a..e6fb2f98e9 100644 --- a/test/io/configs/no-defaults.config +++ b/test/io/configs/no-defaults.config @@ -23,7 +23,7 @@ jwt-aud = "https://postgrest.org" jwt-role-claim-key = ".user[0].\"real-role\"" jwt-secret = "c2VjdXJpdHl0aHJvdWdob2JzY3VyaXR5aW5iYXNlNjQ=" jwt-secret-is-base64 = true -jwt-cache-max-lifetime = 86400 +jwt-cache-max-entries = 10000 log-level = "info" log-query = "main-query" openapi-mode = "ignore-privileges" diff --git a/test/io/db_config.sql b/test/io/db_config.sql index 18ac1c2972..5f0ee285ec 100644 --- a/test/io/db_config.sql +++ b/test/io/db_config.sql @@ -14,7 +14,7 @@ ALTER ROLE db_config_authenticator SET pgrst.db_root_spec = 'root'; ALTER ROLE db_config_authenticator SET pgrst.db_schemas = 'test, tenant1, tenant2'; ALTER ROLE db_config_authenticator SET pgrst.db_tx_end = 'commit-allow-override'; ALTER ROLE db_config_authenticator SET pgrst.jwt_aud = 'https://example.org'; -ALTER ROLE db_config_authenticator SET pgrst.jwt_cache_max_lifetime = '3600'; +ALTER ROLE db_config_authenticator SET pgrst.jwt_cache_max_entries = '3000'; ALTER ROLE db_config_authenticator SET pgrst.jwt_role_claim_key = '."a"."role"'; ALTER ROLE db_config_authenticator SET pgrst.jwt_secret = 'REALLY=REALLY=REALLY=REALLY=VERY=SAFE'; ALTER ROLE db_config_authenticator SET pgrst.jwt_secret_is_base64 = 'false'; @@ -68,7 +68,7 @@ ALTER ROLE other_authenticator SET pgrst.db_schemas = 'test, other_tenant1, othe ALTER ROLE other_authenticator SET pgrst.jwt_aud = 'https://otherexample.org'; ALTER ROLE other_authenticator SET pgrst.jwt_secret = 'ODERREALLYREALLYREALLYREALLYVERYSAFE'; ALTER ROLE other_authenticator SET pgrst.jwt_secret_is_base64 = 'false'; -ALTER ROLE other_authenticator SET pgrst.jwt_cache_max_lifetime = '7200'; +ALTER ROLE other_authenticator SET pgrst.jwt_cache_max_entries = '6000'; ALTER ROLE other_authenticator SET pgrst.openapi_mode = 'disabled'; ALTER ROLE other_authenticator SET pgrst.openapi_security_active = 'false'; ALTER ROLE other_authenticator SET pgrst.openapi_server_proxy_uri = 'https://otherexample.org/api'; diff --git a/test/io/test_io.py b/test/io/test_io.py index 02d31349c7..0f1adefc2e 100644 --- a/test/io/test_io.py +++ b/test/io/test_io.py @@ -146,7 +146,7 @@ def test_jwt_errors(defaultenv): env = { **defaultenv, "PGRST_SERVER_TIMING_ENABLED": "true", - "PGRST_JWT_CACHE_MAX_LIFETIME": "86400", + "PGRST_JWT_CACHE_MAX_ENTRIES": "1000", # default "PGRST_JWT_SECRET": SECRET, } @@ -159,7 +159,7 @@ def test_jwt_errors(defaultenv): env = { **defaultenv, "PGRST_SERVER_TIMING_ENABLED": "false", - "PGRST_JWT_CACHE_MAX_LIFETIME": "86400", + "PGRST_JWT_CACHE_MAX_ENTRIES": "1000", # default "PGRST_JWT_SECRET": SECRET, } @@ -1428,7 +1428,7 @@ def test_jwt_cache_server_timing(defaultenv): env = { **defaultenv, "PGRST_SERVER_TIMING_ENABLED": "true", - "PGRST_JWT_CACHE_MAX_LIFETIME": "86400", + "PGRST_JWT_CACHE_MAX_ENTRIES": "1000", # default "PGRST_JWT_SECRET": SECRET, "PGRST_DB_CONFIG": "false", } @@ -1464,7 +1464,7 @@ def test_jwt_cache_without_server_timing(defaultenv): env = { **defaultenv, "PGRST_SERVER_TIMING_ENABLED": "false", - "PGRST_JWT_CACHE_MAX_LIFETIME": "86400", + "PGRST_JWT_CACHE_MAX_ENTRIES": "1000", # default "PGRST_JWT_SECRET": SECRET, "PGRST_DB_CONFIG": "false", } @@ -1485,7 +1485,7 @@ def test_jwt_cache_without_exp_claim(defaultenv): env = { **defaultenv, "PGRST_SERVER_TIMING_ENABLED": "true", - "PGRST_JWT_CACHE_MAX_LIFETIME": "86400", + "PGRST_JWT_CACHE_MAX_ENTRIES": "1000", # default "PGRST_JWT_SECRET": SECRET, "PGRST_DB_CONFIG": "false", } @@ -1737,54 +1737,6 @@ def test_schema_cache_startup_load_with_in_db_config(defaultenv, metapostgrest): 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 - - def test_pgrst_log_503_client_error_to_stderr(defaultenv): "PostgREST should log 503 errors to stderr" @@ -1864,7 +1816,7 @@ def test_invalidate_jwt_cache_when_secret_changes(tmp_path, defaultenv): **defaultenv, "PGRST_JWT_SECRET": f"@{external_secret_file}", "PGRST_DB_CHANNEL_ENABLED": "true", - "PGRST_JWT_CACHE_MAX_LIFETIME": "86400", # enable cache + "PGRST_JWT_CACHE_MAX_ENTRIES": "1000", # default "PGRST_DB_ANON_ROLE": "postgrest_test_anonymous", # required for NOTIFY } @@ -1885,3 +1837,46 @@ def test_invalidate_jwt_cache_when_secret_changes(tmp_path, defaultenv): # now the request should fail because the cached token is removed response = postgrest.session.get("/authors_only", headers=headers) assert response.status_code == 401 + + +def test_jwt_cache_hit_but_entry_expired(defaultenv): + "test jwt cache hit but it is expired" + + relativeSeconds = lambda sec: int( + (datetime.now(timezone.utc) + timedelta(seconds=sec)).timestamp() + ) + + # generate auth header, -31 seconds for clock skew + headers = lambda sec: jwtauthheader( + {"role": "postgrest_test_author", "exp": relativeSeconds(-31 + sec)}, + SECRET, + ) + + env = { + **defaultenv, + "PGRST_JWT_CACHE_MAX_ENTRIES": "1000", # default + "PGRST_JWT_SECRET": SECRET, + "PGRST_DB_CONFIG": "false", + } + + with run(env=env) as postgrest: + + hdrs = headers(5) # 5 second expiry + + response = postgrest.session.get("/authors_only", headers=hdrs) + assert response.status_code == 200 + + # now the jwt is cached + + # wait for 5 seconds for it to expire + + time.sleep(5) + + # make another request with same token + + # it is a cache hit, but the exp claim is expired, so we + # delete the token from cache + + # the request should fail + response = postgrest.session.get("/authors_only", headers=hdrs) + assert response.status_code == 401 diff --git a/test/spec/Main.hs b/test/spec/Main.hs index 16c5f39304..70f7da8bef 100644 --- a/test/spec/Main.hs +++ b/test/spec/Main.hs @@ -85,7 +85,7 @@ main = do -- cached schema cache so most tests run fast baseSchemaCache <- loadSCache pool testCfg sockets <- AppState.initSockets testCfg - jwtCacheState <- JwtCache.init + jwtCacheState <- JwtCache.init (configJwtCacheMaxEntries testCfg) loggerState <- Logger.init metricsState <- Metrics.init (configDbPoolSize testCfg) diff --git a/test/spec/SpecHelper.hs b/test/spec/SpecHelper.hs index d3bcc5fd3a..1e58f4c9ac 100644 --- a/test/spec/SpecHelper.hs +++ b/test/spec/SpecHelper.hs @@ -137,7 +137,7 @@ baseCfg = let secret = encodeUtf8 "reallyreallyreallyreallyverysafe" in , configJwtRoleClaimKey = [JSPKey "role"] , configJwtSecret = Just secret , configJwtSecretIsBase64 = False - , configJwtCacheMaxLifetime = 0 + , configJwtCacheMaxEntries = 0 -- disable cache to allow parallel test runs , configLogLevel = LogCrit , configLogQuery = LogQueryDisabled , configOpenApiMode = OAFollowPriv