fix: invalid JWTs after jwt-secret is changed in a config reload (#4015)
This commit is contained in:
@@ -35,6 +35,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
|
|||||||
- #3600, #3926, Improve JWT errors - @taimoorzaeem
|
- #3600, #3926, Improve JWT errors - @taimoorzaeem
|
||||||
- #3013, Fix `order=` with POST, PATCH, PUT and DELETE requests - @taimoorzaeem
|
- #3013, Fix `order=` with POST, PATCH, PUT and DELETE requests - @taimoorzaeem
|
||||||
- #3498, Fix incorrect parsing of the `for` parameter of the `application/vnd.pgrst.plan` media type - @taimoorzaeem
|
- #3498, Fix incorrect parsing of the `for` parameter of the `application/vnd.pgrst.plan` media type - @taimoorzaeem
|
||||||
|
- #4014, Fix JWT cache allows old tokens after the jwt-secret is changed in a config reload - @taimoorzaeem
|
||||||
|
|
||||||
### Changed
|
### Changed
|
||||||
|
|
||||||
|
|||||||
@@ -438,11 +438,11 @@ retryingSchemaCacheLoad appState@AppState{stateObserver=observer, stateMainThrea
|
|||||||
-- | We don't retry reading the in-db config after it fails immediately, because it could have user errors. We just report the error and continue.
|
-- | We don't retry reading the in-db config after it fails immediately, because it could have user errors. We just report the error and continue.
|
||||||
readInDbConfig :: Bool -> AppState -> IO ()
|
readInDbConfig :: Bool -> AppState -> IO ()
|
||||||
readInDbConfig startingUp appState@AppState{stateObserver=observer} = do
|
readInDbConfig startingUp appState@AppState{stateObserver=observer} = do
|
||||||
AppConfig{..} <- getConfig appState
|
conf <- getConfig appState
|
||||||
pgVer <- getPgVersion appState
|
pgVer <- getPgVersion appState
|
||||||
dbSettings <-
|
dbSettings <-
|
||||||
if configDbConfig then do
|
if configDbConfig conf then do
|
||||||
qDbSettings <- usePool appState (queryDbSettings (dumpQi <$> configDbPreConfig) configDbPreparedStatements)
|
qDbSettings <- usePool appState (queryDbSettings (dumpQi <$> configDbPreConfig conf) (configDbPreparedStatements conf))
|
||||||
case qDbSettings of
|
case qDbSettings of
|
||||||
Left e -> do
|
Left e -> do
|
||||||
observer $ ConfigReadErrorObs e
|
observer $ ConfigReadErrorObs e
|
||||||
@@ -451,8 +451,8 @@ readInDbConfig startingUp appState@AppState{stateObserver=observer} = do
|
|||||||
else
|
else
|
||||||
pure mempty
|
pure mempty
|
||||||
(roleSettings, roleIsolationLvl) <-
|
(roleSettings, roleIsolationLvl) <-
|
||||||
if configDbConfig then do
|
if configDbConfig conf then do
|
||||||
rSettings <- usePool appState (queryRoleSettings pgVer configDbPreparedStatements)
|
rSettings <- usePool appState (queryRoleSettings pgVer (configDbPreparedStatements conf))
|
||||||
case rSettings of
|
case rSettings of
|
||||||
Left e -> do
|
Left e -> do
|
||||||
observer $ QueryRoleSettingsErrorObs e
|
observer $ QueryRoleSettingsErrorObs e
|
||||||
@@ -460,7 +460,7 @@ readInDbConfig startingUp appState@AppState{stateObserver=observer} = do
|
|||||||
Right x -> pure x
|
Right x -> pure x
|
||||||
else
|
else
|
||||||
pure mempty
|
pure mempty
|
||||||
readAppConfig dbSettings configFilePath (Just configDbUri) roleSettings roleIsolationLvl >>= \case
|
readAppConfig dbSettings (configFilePath conf) (Just $ configDbUri conf) roleSettings roleIsolationLvl >>= \case
|
||||||
Left err ->
|
Left err ->
|
||||||
if startingUp then
|
if startingUp then
|
||||||
panic err -- die on invalid config if the program is starting up
|
panic err -- die on invalid config if the program is starting up
|
||||||
@@ -468,6 +468,14 @@ readInDbConfig startingUp appState@AppState{stateObserver=observer} = do
|
|||||||
observer $ ConfigInvalidObs err
|
observer $ ConfigInvalidObs err
|
||||||
Right newConf -> do
|
Right newConf -> do
|
||||||
putConfig appState newConf
|
putConfig appState newConf
|
||||||
|
-- After the config has reloaded, jwt-secret might have changed, so
|
||||||
|
-- if it has changed, it is important to invalidate the jwt cache
|
||||||
|
-- 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
|
||||||
|
|
||||||
if startingUp then
|
if startingUp then
|
||||||
pass
|
pass
|
||||||
else
|
else
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ module PostgREST.Auth.JwtCache
|
|||||||
( init
|
( init
|
||||||
, JwtCacheState
|
, JwtCacheState
|
||||||
, lookupJwtCache
|
, lookupJwtCache
|
||||||
|
, emptyCache
|
||||||
) where
|
) where
|
||||||
|
|
||||||
import qualified Data.Aeson as JSON
|
import qualified Data.Aeson as JSON
|
||||||
@@ -77,3 +78,7 @@ getTimeSpec res maxLifetime utc = do
|
|||||||
case expireJSON of
|
case expireJSON of
|
||||||
Just (JSON.Number seconds) -> TimeSpec (sciToInt seconds - utcToSecs utc) 0
|
Just (JSON.Number seconds) -> TimeSpec (sciToInt seconds - utcToSecs utc) 0
|
||||||
_ -> TimeSpec (fromIntegral maxLifetime :: Int64) 0
|
_ -> TimeSpec (fromIntegral maxLifetime :: Int64) 0
|
||||||
|
|
||||||
|
-- | Empty the cache (done when the config is reloaded)
|
||||||
|
emptyCache :: JwtCacheState -> IO ()
|
||||||
|
emptyCache JwtCacheState{jwtCache} = C.purge jwtCache
|
||||||
|
|||||||
@@ -1850,3 +1850,38 @@ def test_proxy_status_header(defaultenv, metapostgrest):
|
|||||||
assert response.headers["Proxy-Status"] == "PostgREST; error=57014"
|
assert response.headers["Proxy-Status"] == "PostgREST; error=57014"
|
||||||
data = response.json()
|
data = response.json()
|
||||||
assert data["message"] == "canceling statement due to statement timeout"
|
assert data["message"] == "canceling statement due to statement timeout"
|
||||||
|
|
||||||
|
|
||||||
|
def test_invalidate_jwt_cache_when_secret_changes(tmp_path, defaultenv):
|
||||||
|
"JWT cache should be emptied after jwt-secret is changed in a config reload"
|
||||||
|
|
||||||
|
headers = jwtauthheader({"role": "postgrest_test_author"}, SECRET)
|
||||||
|
|
||||||
|
external_secret_file = tmp_path / "jwt-secret-config"
|
||||||
|
external_secret_file.write_text(SECRET)
|
||||||
|
|
||||||
|
env = {
|
||||||
|
**defaultenv,
|
||||||
|
"PGRST_JWT_SECRET": f"@{external_secret_file}",
|
||||||
|
"PGRST_DB_CHANNEL_ENABLED": "true",
|
||||||
|
"PGRST_JWT_CACHE_MAX_LIFETIME": "86400", # enable cache
|
||||||
|
"PGRST_DB_ANON_ROLE": "postgrest_test_anonymous", # required for NOTIFY
|
||||||
|
}
|
||||||
|
|
||||||
|
with run(env=env) as postgrest:
|
||||||
|
response = postgrest.session.get("/authors_only", headers=headers)
|
||||||
|
assert response.status_code == 200 # jwt gets cached
|
||||||
|
|
||||||
|
# change external file
|
||||||
|
external_secret_file.write_text("invalid" * 5)
|
||||||
|
|
||||||
|
# reload config and external file with NOTIFY
|
||||||
|
# jwt-cache should get empty
|
||||||
|
response = postgrest.session.post("/rpc/reload_pgrst_config")
|
||||||
|
assert response.text == ""
|
||||||
|
assert response.status_code == 204
|
||||||
|
sleep_until_postgrest_config_reload()
|
||||||
|
|
||||||
|
# now the request should fail because the cached token is removed
|
||||||
|
response = postgrest.session.get("/authors_only", headers=headers)
|
||||||
|
assert response.status_code == 401
|
||||||
|
|||||||
Reference in New Issue
Block a user