diff --git a/main/Main.hs b/main/Main.hs index 916ab84af74..b1453df7a3b 100644 --- a/main/Main.hs +++ b/main/Main.hs @@ -70,7 +70,7 @@ main = do pathEnvConf <- either panic identity <$> readAppConfig mempty env cliPath Nothing Nothing -- read external files - dbUriFile <- readDbUriFile $ configDbUri pathEnvConf + dbUriFile <- readDbUriFile $ configDbUri pathEnvConf secretFile <- readSecretFile $ configJwtSecret pathEnvConf -- add the external files to AppConfig @@ -111,10 +111,19 @@ main = do -- Config that can change at runtime refConf <- newIORef conf - let dbConfigReader startingUp = readDbConfig startingUp pool gucConfigEnabled env cliPath dbUriFile secretFile refConf - - -- Override the config with config options from the db, only if db-load-guc-config is true - when gucConfigEnabled $ dbConfigReader True + let + -- re-reads config file + db config + dbConfigReReader startingUp = when gucConfigEnabled $ + reReadConfig startingUp pool gucConfigEnabled env cliPath refConf dbUriFile secretFile + -- re-reads jwt-secret external file + config file + db config + fullConfigReReader = + reReadConfig False pool gucConfigEnabled env cliPath refConf + dbUriFile =<< -- db-uri external file could be re-read, but it doesn't make sense as db-uri is not reloadable + readSecretFile (configJwtSecret pathEnvConf) + + -- Override the config with config options from the db + -- TODO: the same operation is repeated on connectionWorker, ideally this would be done only once, but dump CmdDumpConfig needs it for tests. + dbConfigReReader True case cliCommand of CmdDumpConfig -> @@ -133,7 +142,8 @@ main = do -- This is passed to the connectionWorker method so it can kill the main thread if the PostgreSQL's version is not supported. mainTid <- myThreadId - let connWorker = connectionWorker mainTid pool refConf refDbStructure refIsWorkerOn (dbChannelEnabled, mvarConnectionStatus) $ dbConfigReader False + let connWorker = connectionWorker mainTid pool refConf refDbStructure refIsWorkerOn (dbChannelEnabled, mvarConnectionStatus) $ + dbConfigReReader False -- Sets the initial refDbStructure connWorker @@ -156,13 +166,13 @@ main = do -- Re-read the config on SIGUSR2 void $ installHandler sigUSR2 ( - Catch $ dbConfigReader False + Catch fullConfigReReader ) Nothing #endif -- reload schema cache + config on NOTIFY when dbChannelEnabled $ - listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWorker $ dbConfigReader False + listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWorker fullConfigReReader -- ask for the OS time at most once per second getTime <- mkAutoUpdate defaultUpdateSettings {updateAction = getCurrentTime} @@ -219,7 +229,7 @@ connectionWorker -> (Bool, MVar ConnectionStatus) -- ^ For interacting with the LISTEN channel -> IO () -> IO () -connectionWorker mainTid pool refConf refDbStructure refIsWorkerOn (dbChannelEnabled, mvarConnectionStatus) cfRereader = do +connectionWorker mainTid pool refConf refDbStructure refIsWorkerOn (dbChannelEnabled, mvarConnectionStatus) dbCfReader = do isWorkerOn <- readIORef refIsWorkerOn unless isWorkerOn $ do -- Prevents multiple workers to be running at the same time. Could happen on too many SIGUSR1s. atomicWriteIORef refIsWorkerOn True @@ -235,7 +245,7 @@ connectionWorker mainTid pool refConf refDbStructure refIsWorkerOn (dbChannelEna NotConnected -> return () -- Unreachable because connectionStatus will keep trying to connect Connected actualPgVersion -> do -- Procede with initialization putStrLn ("Connection successful" :: Text) - cfRereader + dbCfReader -- this could be fail because the connection drops, but the loadSchemaCache will pick the error and retry again scStatus <- loadSchemaCache pool actualPgVersion refConf refDbStructure case scStatus of SCLoaded -> pure () -- do nothing and proceed if the load was successful @@ -340,9 +350,9 @@ listener dbUri dbChannel pool refConf refDbStructure mvarConnectionStatus connWo errorMessage = "Could not listen for notifications on the " <> dbChannel <> " channel" :: Text retryMessage = "Retrying listening for notifications on the " <> dbChannel <> " channel.." :: Text --- | Reads the config options from the db -readDbConfig :: Bool -> P.Pool -> Bool -> Environment -> Maybe FilePath -> Maybe Text -> Maybe BS.ByteString -> IORef AppConfig -> IO () -readDbConfig startingUp pool gucConfigEnabled env path dbUriFile secretFile refConf = do +-- | Re-reads the config plus config options from the db +reReadConfig :: Bool -> P.Pool -> Bool -> Environment -> Maybe FilePath -> IORef AppConfig -> Maybe Text -> Maybe BS.ByteString -> IO () +reReadConfig startingUp pool gucConfigEnabled env path refConf dbUriFile secretFile = do dbSettings <- if gucConfigEnabled then loadDbSettings else pure [] readAppConfig dbSettings env path dbUriFile secretFile >>= \case Left err -> diff --git a/test/fixtures/roles.sql b/test/fixtures/roles.sql index f0c7537b3e5..5abeaf9f12c 100644 --- a/test/fixtures/roles.sql +++ b/test/fixtures/roles.sql @@ -7,55 +7,55 @@ CREATE ROLE postgrest_test_author; GRANT postgrest_test_anonymous, postgrest_test_default_role, postgrest_test_author TO :USER; -- reloadable config options for io tests -ALTER ROLE postgrest_test_authenticator SET pgrst."jwt_aud" = 'https://example.org'; -ALTER ROLE postgrest_test_authenticator SET pgrst."openapi_server_proxy_uri" = 'https://example.org/api'; -ALTER ROLE postgrest_test_authenticator SET pgrst."raw_media_types" = 'application/vnd.pgrst.db-config'; -ALTER ROLE postgrest_test_authenticator SET pgrst."jwt_secret" = 'REALLYREALLYREALLYREALLYVERYSAFE'; -ALTER ROLE postgrest_test_authenticator SET pgrst."jwt_secret_is_base64" = 'true'; -ALTER ROLE postgrest_test_authenticator SET pgrst."jwt_role_claim_key" = '."a"."role"'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_tx_end" = 'commit-allow-override'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_schemas" = 'test, tenant1, tenant2'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_root_spec" = 'root'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_prepared_statements" = 'false'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_pre_request" = 'test.custom_headers'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_max_rows" = '1000'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_extra_search_path" = 'public, extensions'; +ALTER ROLE postgrest_test_authenticator SET pgrst.jwt_aud = 'https://example.org'; +ALTER ROLE postgrest_test_authenticator SET pgrst.openapi_server_proxy_uri = 'https://example.org/api'; +ALTER ROLE postgrest_test_authenticator SET pgrst.raw_media_types = 'application/vnd.pgrst.db-config'; +ALTER ROLE postgrest_test_authenticator SET pgrst.jwt_secret = 'REALLYREALLYREALLYREALLYVERYSAFE'; +ALTER ROLE postgrest_test_authenticator SET pgrst.jwt_secret_is_base64 = 'true'; +ALTER ROLE postgrest_test_authenticator SET pgrst.jwt_role_claim_key = '."a"."role"'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_tx_end = 'commit-allow-override'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_schemas = 'test, tenant1, tenant2'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_root_spec = 'root'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_prepared_statements = 'false'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_pre_request = 'test.custom_headers'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_max_rows = '1000'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_extra_search_path = 'public, extensions'; -- override with database specific setting -ALTER ROLE postgrest_test_authenticator IN DATABASE :DBNAME SET pgrst."jwt_secret" = 'OVERRIDEREALLYREALLYREALLYREALLYVERYSAFE'; -ALTER ROLE postgrest_test_authenticator IN DATABASE :DBNAME SET pgrst."db_extra_search_path" = 'public, extensions, private'; +ALTER ROLE postgrest_test_authenticator IN DATABASE :DBNAME SET pgrst.jwt_secret = 'OVERRIDEREALLYREALLYREALLYREALLYVERYSAFE'; +ALTER ROLE postgrest_test_authenticator IN DATABASE :DBNAME SET pgrst.db_extra_search_path = 'public, extensions, private'; -- other database settings that should be ignored DROP DATABASE IF EXISTS other; CREATE DATABASE other; -ALTER ROLE postgrest_test_authenticator IN DATABASE other SET pgrst."db_max_rows" = '1111'; +ALTER ROLE postgrest_test_authenticator IN DATABASE other SET pgrst.db_max_rows = '1111'; -- non-reloadable configs for io tests -ALTER ROLE postgrest_test_authenticator SET pgrst."server_host" = 'ignored'; -ALTER ROLE postgrest_test_authenticator SET pgrst."server_port" = 'ignored'; -ALTER ROLE postgrest_test_authenticator SET pgrst."server_unix_socket" = 'ignored'; -ALTER ROLE postgrest_test_authenticator SET pgrst."server_unix_socket_mode" = 'ignored'; -ALTER ROLE postgrest_test_authenticator SET pgrst."log_level" = 'ignored'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_anon_role" = 'ignored'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_uri" = 'postgresql://ignored'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_channel_enabled" = 'ignored'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_channel" = 'ignored'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_pool" = 'ignored'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_pool_timeout" = 'ignored'; -ALTER ROLE postgrest_test_authenticator SET pgrst."db_load_guc_config" = 'ignored'; +ALTER ROLE postgrest_test_authenticator SET pgrst.server_host = 'ignored'; +ALTER ROLE postgrest_test_authenticator SET pgrst.server_port = 'ignored'; +ALTER ROLE postgrest_test_authenticator SET pgrst.server_unix_socket = 'ignored'; +ALTER ROLE postgrest_test_authenticator SET pgrst.server_unix_socket_mode = 'ignored'; +ALTER ROLE postgrest_test_authenticator SET pgrst.log_level = 'ignored'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_anon_role = 'ignored'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_uri = 'postgresql://ignored'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_channel_enabled = 'ignored'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_channel = 'ignored'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_pool = 'ignored'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_pool_timeout = 'ignored'; +ALTER ROLE postgrest_test_authenticator SET pgrst.db_load_guc_config = 'ignored'; -- other authenticator reloadable config options for io tests CREATE ROLE other_authenticator LOGIN NOINHERIT; -ALTER ROLE other_authenticator SET pgrst."jwt_aud" = 'https://otherexample.org'; -ALTER ROLE other_authenticator SET pgrst."openapi_server_proxy_uri" = 'https://otherexample.org/api'; -ALTER ROLE other_authenticator SET pgrst."raw_media_types" = 'application/vnd.pgrst.other-db-config'; -ALTER ROLE other_authenticator SET pgrst."jwt_secret" = 'ODERREALLYREALLYREALLYREALLYVERYSAFE'; -ALTER ROLE other_authenticator SET pgrst."jwt_secret_is_base64" = 'true'; -ALTER ROLE other_authenticator SET pgrst."jwt_role_claim_key" = '."other"."role"'; -ALTER ROLE other_authenticator SET pgrst."db_tx_end" = 'rollback-allow-override'; -ALTER ROLE other_authenticator SET pgrst."db_schemas" = 'test, other_tenant1, other_tenant2'; -ALTER ROLE other_authenticator SET pgrst."db_root_spec" = 'other_root'; -ALTER ROLE other_authenticator SET pgrst."db_prepared_statements" = 'false'; -ALTER ROLE other_authenticator SET pgrst."db_pre_request" = 'test.other_custom_headers'; -ALTER ROLE other_authenticator SET pgrst."db_max_rows" = '100'; -ALTER ROLE other_authenticator SET pgrst."db_extra_search_path" = 'public, extensions, other'; +ALTER ROLE other_authenticator SET pgrst.jwt_aud = 'https://otherexample.org'; +ALTER ROLE other_authenticator SET pgrst.openapi_server_proxy_uri = 'https://otherexample.org/api'; +ALTER ROLE other_authenticator SET pgrst.raw_media_types = 'application/vnd.pgrst.other-db-config'; +ALTER ROLE other_authenticator SET pgrst.jwt_secret = 'ODERREALLYREALLYREALLYREALLYVERYSAFE'; +ALTER ROLE other_authenticator SET pgrst.jwt_secret_is_base64 = 'true'; +ALTER ROLE other_authenticator SET pgrst.jwt_role_claim_key = '."other"."role"'; +ALTER ROLE other_authenticator SET pgrst.db_tx_end = 'rollback-allow-override'; +ALTER ROLE other_authenticator SET pgrst.db_schemas = 'test, other_tenant1, other_tenant2'; +ALTER ROLE other_authenticator SET pgrst.db_root_spec = 'other_root'; +ALTER ROLE other_authenticator SET pgrst.db_prepared_statements = 'false'; +ALTER ROLE other_authenticator SET pgrst.db_pre_request = 'test.other_custom_headers'; +ALTER ROLE other_authenticator SET pgrst.db_max_rows = '100'; +ALTER ROLE other_authenticator SET pgrst.db_extra_search_path = 'public, extensions, other'; diff --git a/test/fixtures/schema.sql b/test/fixtures/schema.sql index 80182382ccb..862b65178ae 100644 --- a/test/fixtures/schema.sql +++ b/test/fixtures/schema.sql @@ -1928,7 +1928,7 @@ select * from pg_catalog.pg_prepared_statements; create or replace function change_max_rows_config(val int, notify bool default false) returns void as $_$ begin execute format($$ - alter role postgrest_test_authenticator set pgrst."db_max_rows" = %L; + alter role postgrest_test_authenticator set pgrst.db_max_rows = %L; $$, val); if notify then perform pg_notify('pgrst', 'reload config'); @@ -1937,13 +1937,13 @@ end $_$ volatile security definer language plpgsql ; create or replace function reset_max_rows_config() returns void as $_$ begin - alter role postgrest_test_authenticator set pgrst."db_max_rows" = '1000'; + alter role postgrest_test_authenticator set pgrst.db_max_rows = '1000'; end $_$ volatile security definer language plpgsql ; create or replace function change_db_schema_and_full_reload(schemas text) returns void as $_$ begin execute format($$ - alter role postgrest_test_authenticator set pgrst."db_schemas" = %L; + alter role postgrest_test_authenticator set pgrst.db_schemas = %L; $$, schemas); perform pg_notify('pgrst', 'reload config'); perform pg_notify('pgrst', 'reload schema'); @@ -1951,19 +1951,25 @@ end $_$ volatile security definer language plpgsql ; create or replace function v1.reset_db_schema_config() returns void as $_$ begin - alter role postgrest_test_authenticator set pgrst."db_schemas" = 'test'; + alter role postgrest_test_authenticator set pgrst.db_schemas = 'test'; 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'; + 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"'; + 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 ; + +create or replace function test.reload_pgrst_config() returns void as $_$ +begin + alter role postgrest_test_authenticator set pgrst.jwt_role_claim_key = '."a"."role"'; + perform pg_notify('pgrst', 'reload config'); +end $_$ language plpgsql ; diff --git a/test/io-tests/configs/sigusr2-settings-external-secret.config b/test/io-tests/configs/sigusr2-settings-external-secret.config new file mode 100644 index 00000000000..a498f5e0a0d --- /dev/null +++ b/test/io-tests/configs/sigusr2-settings-external-secret.config @@ -0,0 +1,5 @@ +db-pool = 1 + +jwt-secret = "$(JWT_SECRET_FILE)" +jwt-secret-is-base64 = false +db-load-guc-config = false diff --git a/test/io-tests/test_io.py b/test/io-tests/test_io.py index 2b1332ce4c1..2e06f8af898 100644 --- a/test/io-tests/test_io.py +++ b/test/io-tests/test_io.py @@ -336,6 +336,7 @@ def test_stable_config(tmp_path, config, defaultenv): "ROLE_CLAIM_KEY": '."https://www.example.com/roles"[0].value', "POSTGREST_TEST_SOCKET": "/tmp/postgrest.sock", "POSTGREST_TEST_PORT": "80", + "JWT_SECRET_FILE": "a_file", } # Some configs expect input from stdin, at least on base64. @@ -508,6 +509,53 @@ def test_jwt_secret_reload(tmp_path, defaultenv): assert response.status_code == 200 +def test_jwt_secret_external_file_reload(tmp_path, defaultenv): + "JWT secret external file should be reloaded when PostgREST is sent a SIGUSR2 or a NOTIFY." + config = CONFIGSDIR / "sigusr2-settings-external-secret.config" + + headers = jwtauthheader({"role": "postgrest_test_author"}, SECRET) + + external_secret_file = tmp_path / "jwt-secret-config" + external_secret_file.write_text("invalid" * 5) + + env = { + **defaultenv, + "JWT_SECRET_FILE": f"@{external_secret_file}", + "PGRST_DB_CHANNEL_ENABLED": "true", + } + + with run(config, env=env) as postgrest: + response = postgrest.session.get("/authors_only", headers=headers) + assert response.status_code == 401 + + # change external file + external_secret_file.write_text(SECRET) + + # SIGUSR1 doesn't reload external files + postgrest.process.send_signal(signal.SIGUSR1) + time.sleep(0.1) + + response = postgrest.session.get("/authors_only", headers=headers) + assert response.status_code == 401 + + # reload config and external file with SIGUSR2 + postgrest.process.send_signal(signal.SIGUSR2) + time.sleep(0.1) + + response = postgrest.session.get("/authors_only", headers=headers) + assert response.status_code == 200 + + # change external file to wrong value again + external_secret_file.write_text("invalid" * 5) + + # reload config and external file with NOTIFY + postgrest.session.get("/rpc/reload_pgrst_config") + time.sleep(0.1) + + response = postgrest.session.get("/authors_only", headers=headers) + assert response.status_code == 401 + + def test_db_schema_reload(tmp_path, defaultenv): "DB schema should be reloaded when PostgREST is sent SIGUSR2." config = (CONFIGSDIR / "sigusr2-settings.config").read_text()