diff --git a/CHANGELOG.md b/CHANGELOG.md index 13b8b2c3a..cf68b750c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #1152, Fix RPC failing when having arguments with reserved or uppercase keywords - @mdr1384 - #905, Fix intermittent empty replies - @steve-chavez - #1139, Fix JWTIssuedAtFuture failure for valid iat claim - @steve-chavez +- #1141, Fix app.settings resetting on pool timeout - @steve-chavez ### Changed diff --git a/main/Main.hs b/main/Main.hs index 6dec58984..ff1c08c16 100644 --- a/main/Main.hs +++ b/main/Main.hs @@ -7,8 +7,7 @@ import PostgREST.App (postgrest) import PostgREST.Config (AppConfig (..), minimumPgVersion, prettyVersion, readOptions) -import PostgREST.DbStructure (getDbStructure, getPgVersion, - fillSessionWithSettings) +import PostgREST.DbStructure (getDbStructure, getPgVersion) import PostgREST.Error (encodeError) import PostgREST.OpenAPI (isMalformedProxyUri) import PostgREST.Types (DbStructure, Schema, PgVersion(..)) @@ -63,11 +62,10 @@ connectionWorker :: ThreadId -- ^ This thread is killed if pg version is unsupported -> P.Pool -- ^ The PostgreSQL connection pool -> Schema -- ^ Schema PostgREST is serving up - -> [(Text, Text)] -- ^ Settings or Environment passed in through the config -> IORef (Maybe DbStructure) -- ^ mutable reference to 'DbStructure' -> IORef Bool -- ^ Used as a binary Semaphore -> IO () -connectionWorker mainTid pool schema settings refDbStructure refIsWorkerOn = do +connectionWorker mainTid pool schema refDbStructure refIsWorkerOn = do isWorkerOn <- readIORef refIsWorkerOn unless isWorkerOn $ do atomicWriteIORef refIsWorkerOn True @@ -85,7 +83,6 @@ connectionWorker mainTid pool schema settings refDbStructure refIsWorkerOn = do ("Cannot run in this PostgreSQL version, PostgREST needs at least " <> pgvName minimumPgVersion) killThread mainTid - fillSessionWithSettings settings dbStructure <- getDbStructure schema actualPgVersion liftIO $ atomicWriteIORef refDbStructure $ Just dbStructure case result of @@ -184,7 +181,6 @@ main = do mainTid pool (configSchema conf) - (configSettings conf) refDbStructure refIsWorkerOn -- @@ -208,7 +204,6 @@ main = do mainTid pool (configSchema conf) - (configSettings conf) refDbStructure refIsWorkerOn ) Nothing @@ -229,7 +224,6 @@ main = do mainTid pool (configSchema conf) - (configSettings conf) refDbStructure refIsWorkerOn) diff --git a/src/PostgREST/Config.hs b/src/PostgREST/Config.hs index dc9c49ade..f5add77ad 100644 --- a/src/PostgREST/Config.hs +++ b/src/PostgREST/Config.hs @@ -141,7 +141,7 @@ readOptions = do <*> (join . fmap coerceInt <$> C.key "max-rows") <*> (mfilter (/= "") <$> C.key "pre-request") <*> pure False - <*> (fmap parsedPairToTextPair <$> C.subassocs "app.settings") + <*> (fmap (fmap coerceText) <$> C.subassocs "app.settings") <*> (maybe (Right [JSPKey "role"]) parseRoleClaimKey <$> C.key "role-claim-key") case mAppConf of @@ -152,13 +152,6 @@ readOptions = do return appConf where - parsedPairToTextPair :: (Name, Value) -> (Text, Text) - parsedPairToTextPair (k, v) = (k, newValue) - where - newValue = case v of - String textVal -> textVal - _ -> show v - parseJwtAudience :: Name -> C.ConfigParserM (Maybe StringOrURI) parseJwtAudience k = C.key k >>= \case @@ -168,6 +161,10 @@ readOptions = do (Just "") -> pure Nothing aud' -> pure aud' + coerceText :: Value -> Text + coerceText (String s) = s + coerceText v = show v + coerceInt :: (Read i, Integral i) => Value -> Maybe i coerceInt (Number x) = rightToMaybe $ floatingOrInteger x coerceInt (String x) = readMaybe $ toS x diff --git a/src/PostgREST/DbStructure.hs b/src/PostgREST/DbStructure.hs index 37521b3ec..99e792ec6 100644 --- a/src/PostgREST/DbStructure.hs +++ b/src/PostgREST/DbStructure.hs @@ -10,7 +10,6 @@ module PostgREST.DbStructure ( , accessibleProcs , schemaDescription , getPgVersion -, fillSessionWithSettings ) where import qualified Hasql.Decoders as HD @@ -33,9 +32,6 @@ import GHC.Exts (groupWith) import Protolude import Unsafe (unsafeHead) -import Data.Functor.Contravariant (contramap) -import Contravariant.Extras (contrazip2) - getDbStructure :: Schema -> PgVersion -> H.Session DbStructure getDbStructure schema pgVer = do tabs <- H.query () allTables @@ -764,17 +760,3 @@ getPgVersion = H.query () $ H.statement sql HE.unit versionRow False where sql = "SELECT current_setting('server_version_num')::integer, current_setting('server_version')" versionRow = HD.singleRow $ PgVersion <$> HD.value HD.int4 <*> HD.value HD.text - -fillSessionWithSettings :: [(Text, Text)] -> H.Session () -fillSessionWithSettings settings = - -- Send all of the config settings to the set_config function, using pgsql's `unnest` to transform arrays of values - H.query settings $ H.statement "SELECT set_config(k, v, false) FROM unnest($1, $2) AS f1(k, v)" encoder HD.unit False - - where - -- Take a list of (key, value) pairs and encode each as an array to later bind to the query - -- see Insert Many section at https://hackage.haskell.org/package/hasql-1.1.1/docs/Hasql-Encoders.html - encoder = contramap L.unzip $ contrazip2 (vector HE.text) (vector HE.text) - where - vector value = - HE.value $ HE.array $ HE.arrayDimension foldl' $ HE.arrayValue value - diff --git a/src/PostgREST/Middleware.hs b/src/PostgREST/Middleware.hs index 5db16d9e0..6037eb847 100644 --- a/src/PostgREST/Middleware.hs +++ b/src/PostgREST/Middleware.hs @@ -19,7 +19,7 @@ import PostgREST.ApiRequest (ApiRequest(..)) import PostgREST.Auth (JWTAttempt(..)) import PostgREST.Config (AppConfig (..), corsPolicy) import PostgREST.Error (simpleError) -import PostgREST.QueryBuilder (pgFmtLit, unquoted, pgFmtEnvVar) +import PostgREST.QueryBuilder (pgFmtLit, unquoted, pgFmtSetLocal) import Protolude hiding (concat, null) @@ -32,13 +32,14 @@ runWithClaims conf eClaims app req = JWTInvalid e -> return $ unauthed $ show e JWTMissingSecret -> return $ simpleError status500 [] "Server lacks JWT secret" JWTClaims claims -> do - H.sql $ toS.mconcat $ setSchemaSql ++ setRoleSql ++ claimsSql ++ headersSql ++ cookiesSql + H.sql $ toS.mconcat $ setSchemaSql ++ setRoleSql ++ claimsSql ++ headersSql ++ cookiesSql ++ appSettingsSql mapM_ H.sql customReqCheck app req where - headersSql = map (pgFmtEnvVar "request.header.") $ iHeaders req - cookiesSql = map (pgFmtEnvVar "request.cookie.") $ iCookies req - claimsSql = map (pgFmtEnvVar "request.jwt.claim.") [(c,unquoted v) | (c,v) <- M.toList claimsWithRole] + headersSql = pgFmtSetLocal "request.header." <$> iHeaders req + cookiesSql = pgFmtSetLocal "request.cookie." <$> iCookies req + claimsSql = pgFmtSetLocal "request.jwt.claim." <$> [(c,unquoted v) | (c,v) <- M.toList claimsWithRole] + appSettingsSql = pgFmtSetLocal mempty <$> configSettings conf setRoleSql = maybeToList $ (\r -> "set local role " <> r <> ";") . toS . pgFmtLit . unquoted <$> M.lookup "role" claimsWithRole setSchemaSql = ["set schema " <> pgFmtLit (configSchema conf) <> ";"] :: [Text] diff --git a/src/PostgREST/QueryBuilder.hs b/src/PostgREST/QueryBuilder.hs index 302251c6b..e29ce6308 100644 --- a/src/PostgREST/QueryBuilder.hs +++ b/src/PostgREST/QueryBuilder.hs @@ -23,7 +23,7 @@ module PostgREST.QueryBuilder ( , requestToCountQuery , unquoted , ResultsWithCount - , pgFmtEnvVar + , pgFmtSetLocal ) where import qualified Hasql.Query as H @@ -469,8 +469,8 @@ pgFmtAs fName jp Nothing = case jOp <$> lastMay jp of Nothing -> "" pgFmtAs _ _ (Just alias) = " AS " <> pgFmtIdent alias -pgFmtEnvVar :: Text -> (Text, Text) -> SqlFragment -pgFmtEnvVar prefix (k, v) = +pgFmtSetLocal :: Text -> (Text, Text) -> SqlFragment +pgFmtSetLocal prefix (k, v) = "set local " <> pgFmtIdent (prefix <> k) <> " = " <> pgFmtLit v <> ";" trimNullChars :: Text -> Text diff --git a/test/Main.hs b/test/Main.hs index d52dd683c..dcd53fe3d 100644 --- a/test/Main.hs +++ b/test/Main.hs @@ -6,8 +6,8 @@ import SpecHelper import qualified Hasql.Pool as P import PostgREST.App (postgrest) -import PostgREST.Config (pgVersion95, pgVersion96, configSettings) -import PostgREST.DbStructure (getDbStructure, getPgVersion, fillSessionWithSettings) +import PostgREST.Config (pgVersion95, pgVersion96) +import PostgREST.DbStructure (getDbStructure, getPgVersion) import PostgREST.Types (DbStructure(..)) import Control.AutoUpdate (defaultUpdateSettings, mkAutoUpdate, updateAction) import Data.Function (id) @@ -66,7 +66,7 @@ main = do nonexistentSchemaApp = return $ postgrest (testNonexistentSchemaCfg testDbConn) refDbStructure pool getTime $ pure () let reset :: IO () - reset = P.use pool (fillSessionWithSettings (configSettings $ testCfg testDbConn)) >> resetDb testDbConn + reset = resetDb testDbConn actualPgVersion = pgVersion dbStructure extraSpecs = diff --git a/test/io-tests.sh b/test/io-tests.sh index b11f13a16..ef5f7cac5 100755 --- a/test/io-tests.sh +++ b/test/io-tests.sh @@ -143,6 +143,26 @@ ensureIatClaimWorks(){ pgrStop } +# ensure app settings don't reset on pool timeout of 10 seconds, see https://github.com/PostgREST/postgrest/issues/1141 +ensureAppSettings(){ + pgrStart "./configs/app-settings.config" + while pgrStarted && test "$( rootStatus )" -ne 200 + do + # wait for the server to start + sleep 0.1 \ + || sleep 1 # fallback: subsecond sleep is not standard and may fail + done + sleep 11 + response=$(curl -s "http://localhost:$pgrPort/rpc/get_guc_value?name=app.settings.external_api_secret") + if test "$response" = "\"0123456789abcdef\"" + then + ok "GET /rpc/get_guc_value response is $response" + else + ko "GET /rpc/get_guc_value response was $response" + fi + pgrStop +} + # PRE: curl must be available test -n "$(command -v curl)" || bailOut 'curl is not available' @@ -181,6 +201,7 @@ invalidRoleClaimKey '' invalidRoleClaimKey 1234 ensureIatClaimWorks +ensureAppSettings cleanUp diff --git a/test/io-tests/configs/app-settings.config b/test/io-tests/configs/app-settings.config new file mode 100644 index 000000000..4d7ab96a3 --- /dev/null +++ b/test/io-tests/configs/app-settings.config @@ -0,0 +1,8 @@ +db-uri = "postgres:///postgrest_test" +db-schema = "test" +db-anon-role = "postgrest_test_anonymous" +db-pool = 1 +server-host = "127.0.0.1" +server-port = 49421 + +app.settings.external_api_secret = "0123456789abcdef"