diff --git a/main/Main.hs b/main/Main.hs index 44f91a9f0..a66c9c07e 100644 --- a/main/Main.hs +++ b/main/Main.hs @@ -1,4 +1,5 @@ {-# LANGUAGE CPP #-} +{-# LANGUAGE LambdaCase #-} {-# LANGUAGE MultiWayIf #-} {-# LANGUAGE NamedFieldPuns #-} @@ -66,7 +67,7 @@ main = do CLI{cliCommand, cliPath} <- readCLIShowHelp env -- build the 'AppConfig' from the config file path - conf <- readValidateConfig mempty env cliPath + conf <- either panic identity <$> readConfig mempty env cliPath -- These are config values that can't be reloaded at runtime. Reloading some of them would imply restarting the web server. let @@ -103,10 +104,10 @@ main = do -- Config that can change at runtime refConf <- newIORef conf - let configRereader = reReadConfig pool gucConfigEnabled env cliPath refConf + let configRereader startingUp = reReadConfig startingUp pool gucConfigEnabled env cliPath refConf -- re-read and override the config if db-load-guc-config is true - when gucConfigEnabled configRereader + when gucConfigEnabled $ configRereader True case cliCommand of CmdDumpConfig -> @@ -148,13 +149,13 @@ main = do -- Re-read the config on SIGUSR2 void $ installHandler sigUSR2 ( - Catch $ configRereader >> putStrLn ("Config reloaded" :: Text) + Catch $ configRereader False ) Nothing #endif -- reload schema cache + config on NOTIFY when dbChannelEnabled $ - listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWorker configRereader + listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWorker $ configRereader False -- ask for the OS time at most once per second getTime <- mkAutoUpdate defaultUpdateSettings {updateAction = getCurrentTime} @@ -310,7 +311,7 @@ loadSchemaCache pool actualPgVersion refConf refDbStructure = do It uses the connectionWorker in case the LISTEN connection dies. -} listener :: ByteString -> Text -> P.Pool -> IORef AppConfig -> IORef (Maybe DbStructure) -> MVar ConnectionStatus -> IO () -> IO () -> IO () -listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWorker configRereader = start +listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWorker configLoader = start where start = do connStatus <- takeMVar mvarConnectionStatus -- takeMVar makes the thread wait if the MVar is empty(until there's a connection). @@ -321,14 +322,13 @@ listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWo Right db -> do putStrLn $ "Listening for notifications on the " <> dbChannel <> " channel" let channelToListen = N.toPgIdentifier dbChannel - cfLoader = configRereader >> putStrLn ("Config reloaded" :: Text) scLoader = void $ loadSchemaCache pool actualPgVersion refConf refDbStructure -- It's not necessary to check the loadSchemaCache success here. If the connection drops, the thread will die and proceed to recover below. N.listen db channelToListen N.waitForNotifications (\_ msg -> - if | BS.null msg -> scLoader -- reload the schema cache - | msg == "reload schema" -> scLoader -- reload the schema cache - | msg == "reload config" -> cfLoader -- reload the config - | otherwise -> pure () -- Do nothing if anything else than an empty message is sent + if | BS.null msg -> scLoader -- reload the schema cache + | msg == "reload schema" -> scLoader -- reload the schema cache + | msg == "reload config" -> configLoader -- reload the config + | otherwise -> pure () -- Do nothing if anything else than an empty message is sent ) db _ -> die errorMessage) (\_ -> do -- if the thread dies, we try to recover @@ -341,12 +341,19 @@ listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWo retryMessage = "Retrying listening for notifications on the " <> dbChannel <> " channel.." :: Text -- | Re-reads the config at runtime. --- | If it panics(config path was changed, invalid setting), it'll show an error but won't kill the main thread. -reReadConfig :: P.Pool -> Bool -> Environment -> Maybe FilePath -> IORef AppConfig -> IO () -reReadConfig pool gucConfigEnabled env path refConf = do +reReadConfig :: Bool -> P.Pool -> Bool -> Environment -> Maybe FilePath -> IORef AppConfig -> IO () +reReadConfig startingUp pool gucConfigEnabled env path refConf = do dbSettings <- if gucConfigEnabled then loadDbSettings pool else pure [] - conf <- readValidateConfig dbSettings env path - atomicWriteIORef refConf conf + readConfig dbSettings env path >>= \case + Left err -> + if startingUp + then panic err -- die on invalid config if the program is starting up + else hPutStrLn stderr $ "Failed config reload. " <> err + Right conf -> do + atomicWriteIORef refConf conf + if startingUp + then pass + else putStrLn ("Config reloaded" :: Text) -- | Dump DbStructure schema to JSON dumpSchema :: P.Pool -> AppConfig -> IO LBS.ByteString diff --git a/src/PostgREST/Config.hs b/src/PostgREST/Config.hs index ce588a588..6d2a85685 100644 --- a/src/PostgREST/Config.hs +++ b/src/PostgREST/Config.hs @@ -30,7 +30,7 @@ module PostgREST.Config ( prettyVersion , Environment , readCLIShowHelp , readEnvironment - , readValidateConfig + , readConfig ) where @@ -44,6 +44,7 @@ import Control.Lens (preview) import Control.Monad (fail) import Crypto.JWT (JWKSet, StringOrURI, stringOrUri) import Data.Aeson (encode, toJSON) +import Data.Either.Combinators (mapLeft) import Data.List (lookup) import Data.List.NonEmpty (fromList, toList) import Data.Maybe (fromJust) @@ -52,7 +53,6 @@ import Data.Text (dropEnd, dropWhileEnd, filter, intercalate, pack, replace, splitOn, strip, stripPrefix, take, toLower, toTitle, unpack) -import Data.Text.IO (hPutStrLn) import Data.Version (versionBranch) import Development.GitRev (gitHash) import Numeric (readOct, showOct) @@ -330,22 +330,18 @@ instance JustIfMaybe a (Maybe a) where justIfMaybe a = Just a -- | Parse the config file -readAppConfig :: [(Text, Text)] -> Environment -> Maybe FilePath -> IO AppConfig +readAppConfig :: [(Text, Text)] -> Environment -> Maybe FilePath -> IO (Either Text AppConfig) readAppConfig dbSettings env optPath = do -- Now read the actual config file conf <- case optPath of - Just cfgPath -> catches (C.load cfgPath) - [ Handler (\(ex :: IOError) -> exitErr $ "Cannot open config file:\n\t" <> show ex) - , Handler (\(C.ParseError err) -> exitErr $ "Error parsing config file:\n" <> err) + Just cfgPath -> C.load cfgPath `catches` + [ Handler (\(ex :: IOError) -> panic $ "Cannot open config file: " <> show ex) + , Handler (\(C.ParseError err) -> panic $ "Error parsing config file: " <> err) ] -- if no filename provided, start with an empty map to read config from environment Nothing -> return M.empty - case C.runParser parseConfig conf of - Left err -> - exitErr $ "Error in config:\n\t" <> err - Right appConf -> - return appConf + pure $ mapLeft ("Error in config: " <>) $ C.runParser parseConfig conf where parseConfig = @@ -395,7 +391,7 @@ readAppConfig dbSettings env optPath = do parseSocketFileMode :: C.Key -> C.Parser C.Config FileMode parseSocketFileMode k = optString k >>= \case - Nothing -> pure $ 432 -- return default 660 mode if no value was provided + Nothing -> pure 432 -- return default 660 mode if no value was provided Just fileModeText -> case (readOct . unpack) fileModeText of [] -> @@ -518,16 +514,14 @@ readAppConfig dbSettings env optPath = do splitOnCommas (C.String s) = strip <$> splitOn "," s splitOnCommas _ = [] - exitErr :: Text -> IO a - exitErr err = do - hPutStrLn stderr err - exitFailure - --- | Parse the AppConfig and validate it. Overrides the config options from env vars or db settings. Panics on invalid config options. -readValidateConfig :: [(Text, Text)] -> Environment -> Maybe FilePath -> IO AppConfig -readValidateConfig dbSettings env path = do - conf <- loadDbUriFile =<< loadSecretFile =<< readAppConfig dbSettings env path - return $ conf { configJWKS = parseSecret <$> configJwtSecret conf} +-- | Reads the config and overrides its parameters from files, env vars or db settings. +readConfig :: [(Text, Text)] -> Environment -> Maybe FilePath -> IO (Either Text AppConfig) +readConfig dbSettings env path = + readAppConfig dbSettings env path >>= \case + Left err -> pure $ Left err + Right appConf -> do + conf <- loadDbUriFile =<< loadSecretFile appConf + pure $ Right $ conf { configJWKS = parseSecret <$> configJwtSecret conf} type Environment = M.Map [Char] Text diff --git a/test/fixtures/schema.sql b/test/fixtures/schema.sql index 09bf974ed..47fed95a0 100644 --- a/test/fixtures/schema.sql +++ b/test/fixtures/schema.sql @@ -1955,3 +1955,15 @@ begin perform pg_notify('pgrst', 'reload config'); perform pg_notify('pgrst', 'reload schema'); end $_$ volatile security definer language plpgsql ; + +create or replace function test.invalid_role_claim_key_reload() returns void as $_$ +begin + alter role postgrest_test_authenticator set pgrst."jwt-role-claim-key" = 'test'; + perform pg_notify('pgrst', 'reload config'); +end $_$ volatile security definer language plpgsql ; + +create or replace function test.reset_invalid_role_claim_key() returns void as $_$ +begin + alter role postgrest_test_authenticator set pgrst."jwt-role-claim-key" = '."a"."role"'; + perform pg_notify('pgrst', 'reload config'); +end $_$ volatile security definer language plpgsql ; diff --git a/test/io-tests/test_io.py b/test/io-tests/test_io.py index 4d54e3ecd..03c32da21 100644 --- a/test/io-tests/test_io.py +++ b/test/io-tests/test_io.py @@ -154,7 +154,7 @@ def run(configpath=None, stdin=None, env=None, port=None): if configpath: command.append(configpath) - process = subprocess.Popen(command, stdin=subprocess.PIPE, env=env) + process = subprocess.Popen(command, stdin=subprocess.PIPE, stderr=subprocess.PIPE, env=env) try: process.stdin.write(stdin or b"") @@ -590,3 +590,19 @@ def test_max_rows_notify_reload(defaultenv): # reset max-rows config on the db postgrest.session.post("/rpc/reset_max_rows_config") + +def invalid_role_claim_key_notify_reload(defaultenv): + "NOTIFY reload config should show an error if role-claim-key is invalid" + + env = { + **defaultenv, + "PGRST_DB_LOAD_GUC_CONFIG": "true", + "PGRST_DB_CHANNEL_ENABLED": "true", + } + + with run(env=env) as postgrest: + postgrest.session.post("/rpc/invalid_role_claim_key_reload") + + assert "failed to parse role-claim-key value" in str(postgrest.process.stderr.readline()) + + postgrest.session.post("/rpc/reset_invalid_role_claim_key")