From 501edc718db6c10ab9104b81eec64076f7c1d6ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steve=20Ch=C3=A1vez?= Date: Sat, 8 Dec 2018 11:39:31 -0500 Subject: [PATCH] Add db-extra-search-path config (#1218) For adding schemas to the search_path, solves issues related to extensions created in the public schema. --- CHANGELOG.md | 4 +++- postgrest.cabal | 1 + src/PostgREST/Config.hs | 13 ++++++++-- src/PostgREST/Middleware.hs | 10 ++++---- src/PostgREST/QueryBuilder.hs | 7 +++++- test/Feature/ExtraSearchPathSpec.hs | 37 +++++++++++++++++++++++++++++ test/Feature/QuerySpec.hs | 3 +++ test/Main.hs | 27 ++++++++++++--------- test/SpecHelper.hs | 5 ++++ test/fixtures/data.sql | 10 ++++++++ test/fixtures/database.sql | 2 +- test/fixtures/privileges.sql | 3 +++ test/fixtures/schema.sql | 29 ++++++++++++++++++---- 13 files changed, 126 insertions(+), 25 deletions(-) create mode 100644 test/Feature/ExtraSearchPathSpec.hs diff --git a/CHANGELOG.md b/CHANGELOG.md index e7e19e4eb..8dd289677 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,10 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased ### Added -- #1205, Add support for parsing JSON Web Key Sets -@russelldavies + +- #1205, Add support for parsing JSON Web Key Sets - @russelldavies - #1203, Add support for reading db-uri from a separate file - @zhoufeng1989 +- #1200, Add db-extra-search-path config for adding schemas to the search_path, solves issues related to extensions created on the public schema - @steve-chavez ### Fixed diff --git a/postgrest.cabal b/postgrest.cabal index a10cb6c9b..e1980ed0f 100644 --- a/postgrest.cabal +++ b/postgrest.cabal @@ -124,6 +124,7 @@ Test-Suite spec , Feature.ConcurrentSpec , Feature.CorsSpec , Feature.DeleteSpec + , Feature.ExtraSearchPathSpec , Feature.InsertSpec , Feature.JsonOperatorSpec , Feature.NoJwtSpec diff --git a/src/PostgREST/Config.hs b/src/PostgREST/Config.hs index 03681435d..44f83aa34 100644 --- a/src/PostgREST/Config.hs +++ b/src/PostgREST/Config.hs @@ -39,7 +39,7 @@ import Data.Scientific (floatingOrInteger) import Data.String (String) import Data.Text (dropAround, intercalate, lines, - strip, take) + strip, take, splitOn) import Data.Text.Encoding (encodeUtf8) import Data.Text.IO (hPutStrLn) import Data.Version (versionBranch) @@ -78,6 +78,7 @@ data AppConfig = AppConfig { , configQuiet :: Bool , configSettings :: [(Text, Text)] , configRoleClaimKey :: Either ApiRequestError JSPath + , configExtraSearchPath :: [Text] } defaultCorsPolicy :: CorsResourcePolicy @@ -140,6 +141,7 @@ readOptions = do <*> pure False <*> (fmap (fmap coerceText) <$> C.subassocs "app.settings") <*> (maybe (Right [JSPKey "role"]) parseRoleClaimKey <$> C.key "role-claim-key") + <*> (maybe ["public"] splitExtraSearchPath <$> C.key "db-extra-search-path") case mAppConf of Nothing -> do @@ -176,6 +178,10 @@ readOptions = do parseRoleClaimKey (String s) = pRoleClaimKey s parseRoleClaimKey v = pRoleClaimKey $ show v + splitExtraSearchPath :: Value -> [Text] + splitExtraSearchPath (String s) = strip <$> splitOn "," s + splitExtraSearchPath _ = [] + opts = info (helper <*> pathParser) $ fullDesc <> progDesc ( @@ -199,7 +205,7 @@ readOptions = do exampleCfg :: Doc exampleCfg = vsep . map (text . toS) . lines $ [str|db-uri = "postgres://user:pass@localhost:5432/dbname" - |db-schema = "public" + |db-schema = "public" # this schema gets added to the search_path of every request |db-anon-role = "postgres" |db-pool = 10 | @@ -223,6 +229,9 @@ readOptions = do | |## jspath to the role claim key |# role-claim-key = ".role" + | + |## extra schemas to add to the search_path of every request + |# db-extra-search-path = "extensions, util" |] pathParser :: Parser FilePath diff --git a/src/PostgREST/Middleware.hs b/src/PostgREST/Middleware.hs index 3aedd3fad..76cbe2db3 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, pgFmtSetLocal) +import PostgREST.QueryBuilder (unquoted, pgFmtSetLocal, pgFmtSetLocalSearchPath) import Protolude @@ -32,7 +32,7 @@ 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 ++ appSettingsSql + H.sql $ toS . mconcat $ setSearchPathSql : setRoleSql ++ claimsSql ++ headersSql ++ cookiesSql ++ appSettingsSql mapM_ H.sql customReqCheck app req where @@ -40,9 +40,9 @@ runWithClaims conf eClaims app 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 local schema " <> pgFmtLit (configSchema conf) <> ";"] :: [Text] + setRoleSql = maybeToList $ (\x -> + pgFmtSetLocal mempty ("role", unquoted x)) <$> M.lookup "role" claimsWithRole + setSearchPathSql = pgFmtSetLocalSearchPath $ configSchema conf : configExtraSearchPath conf -- role claim defaults to anon if not specified in jwt claimsWithRole = M.union claims (M.singleton "role" anon) anon = JSON.String . toS $ configAnonRole conf diff --git a/src/PostgREST/QueryBuilder.hs b/src/PostgREST/QueryBuilder.hs index 72e78f32b..95085af4a 100644 --- a/src/PostgREST/QueryBuilder.hs +++ b/src/PostgREST/QueryBuilder.hs @@ -24,6 +24,7 @@ module PostgREST.QueryBuilder ( , unquoted , ResultsWithCount , pgFmtSetLocal + , pgFmtSetLocalSearchPath ) where import qualified Hasql.Statement as H @@ -470,7 +471,11 @@ pgFmtAs _ _ (Just alias) = " AS " <> pgFmtIdent alias pgFmtSetLocal :: Text -> (Text, Text) -> SqlFragment pgFmtSetLocal prefix (k, v) = - "set local " <> pgFmtIdent (prefix <> k) <> " = " <> pgFmtLit v <> ";" + "SET LOCAL " <> pgFmtIdent (prefix <> k) <> " = " <> pgFmtLit v <> ";" + +pgFmtSetLocalSearchPath :: [Text] -> SqlFragment +pgFmtSetLocalSearchPath vals = + "SET LOCAL search_path = " <> intercalate ", " (pgFmtLit <$> vals) <> ";" trimNullChars :: Text -> Text trimNullChars = T.takeWhile (/= '\x0') diff --git a/test/Feature/ExtraSearchPathSpec.hs b/test/Feature/ExtraSearchPathSpec.hs new file mode 100644 index 000000000..d45acaee9 --- /dev/null +++ b/test/Feature/ExtraSearchPathSpec.hs @@ -0,0 +1,37 @@ +module Feature.ExtraSearchPathSpec where + +import Test.Hspec +import Test.Hspec.Wai +import Test.Hspec.Wai.JSON +import Network.HTTP.Types + +import SpecHelper +import Network.Wai (Application) + +import Protolude + +spec :: SpecWith Application +spec = describe "extra search path" $ do + + it "finds the ltree <@ operator on the public schema" $ + request methodGet "/ltree_sample?path=cd.Top.Science.Astronomy" [] "" + `shouldRespondWith` [json|[ + {"path":"Top.Science.Astronomy"}, + {"path":"Top.Science.Astronomy.Astrophysics"}, + {"path":"Top.Science.Astronomy.Cosmology"}]|] + { matchHeaders = [matchContentTypeJson] } + + it "finds the ltree nlevel function on the public schema, used through a computed column" $ + request methodGet "/ltree_sample?select=number_of_labels&path=eq.Top.Science" [] "" + `shouldRespondWith` [json|[{"number_of_labels":2}]|] + { matchHeaders = [matchContentTypeJson] } + + it "finds the isn = operator on the extensions schema" $ + request methodGet "/isn_sample?id=eq.978-0-393-04002-9&select=name" [] "" + `shouldRespondWith` [json|[{"name":"Mathematics: From the Birth of Numbers"}]|] + { matchHeaders = [matchContentTypeJson] } + + it "finds the isn is_valid function on the extensions schema" $ + request methodGet "/rpc/is_valid_isbn?input=978-0-393-04002-9" [] "" + `shouldRespondWith` [json|true|] + { matchHeaders = [matchContentTypeJson] } diff --git a/test/Feature/QuerySpec.hs b/test/Feature/QuerySpec.hs index 6160e409e..9b51ad6e8 100644 --- a/test/Feature/QuerySpec.hs +++ b/test/Feature/QuerySpec.hs @@ -993,3 +993,6 @@ spec = do get "/projects_dump?select=id,name&order=id.desc&limit=3" `shouldRespondWith` [json| [{"id":5,"name":"Orphan"}, {"id":4,"name":"OSX"}, {"id":3,"name":"IOS"}] |] { matchHeaders = [matchContentTypeJson] } + + it "cannot use ltree(in public schema) extension operators if no extra search path added" $ + get "/ltree_sample?path=cd.Top.Science.Astronomy" `shouldRespondWith` 400 diff --git a/test/Main.hs b/test/Main.hs index 5f4ae276b..62798439b 100644 --- a/test/Main.hs +++ b/test/Main.hs @@ -21,6 +21,7 @@ import qualified Feature.AudienceJwtSecretSpec import qualified Feature.ConcurrentSpec import qualified Feature.CorsSpec import qualified Feature.DeleteSpec +import qualified Feature.ExtraSearchPathSpec import qualified Feature.InsertSpec import qualified Feature.JsonOperatorSpec import qualified Feature.NoJwtSpec @@ -57,16 +58,17 @@ main = do refDbStructure <- newIORef $ Just dbStructure - let withApp = return $ postgrest (testCfg testDbConn) refDbStructure pool getTime $ pure () - ltdApp = return $ postgrest (testLtdRowsCfg testDbConn) refDbStructure pool getTime $ pure () - unicodeApp = return $ postgrest (testUnicodeCfg testDbConn) refDbStructure pool getTime $ pure () - proxyApp = return $ postgrest (testProxyCfg testDbConn) refDbStructure pool getTime $ pure () - noJwtApp = return $ postgrest (testCfgNoJWT testDbConn) refDbStructure pool getTime $ pure () - binaryJwtApp = return $ postgrest (testCfgBinaryJWT testDbConn) refDbStructure pool getTime $ pure () - audJwtApp = return $ postgrest (testCfgAudienceJWT testDbConn) refDbStructure pool getTime $ pure () - asymJwkApp = return $ postgrest (testCfgAsymJWK testDbConn) refDbStructure pool getTime $ pure () - asymJwkSetApp = return $ postgrest (testCfgAsymJWKSet testDbConn) refDbStructure pool getTime $ pure () - nonexistentSchemaApp = return $ postgrest (testNonexistentSchemaCfg testDbConn) refDbStructure pool getTime $ pure () + let withApp = return $ postgrest (testCfg testDbConn) refDbStructure pool getTime $ pure () + ltdApp = return $ postgrest (testLtdRowsCfg testDbConn) refDbStructure pool getTime $ pure () + unicodeApp = return $ postgrest (testUnicodeCfg testDbConn) refDbStructure pool getTime $ pure () + proxyApp = return $ postgrest (testProxyCfg testDbConn) refDbStructure pool getTime $ pure () + noJwtApp = return $ postgrest (testCfgNoJWT testDbConn) refDbStructure pool getTime $ pure () + binaryJwtApp = return $ postgrest (testCfgBinaryJWT testDbConn) refDbStructure pool getTime $ pure () + audJwtApp = return $ postgrest (testCfgAudienceJWT testDbConn) refDbStructure pool getTime $ pure () + asymJwkApp = return $ postgrest (testCfgAsymJWK testDbConn) refDbStructure pool getTime $ pure () + asymJwkSetApp = return $ postgrest (testCfgAsymJWKSet testDbConn) refDbStructure pool getTime $ pure () + nonexistentSchemaApp = return $ postgrest (testNonexistentSchemaCfg testDbConn) refDbStructure pool getTime $ pure () + extraSearchPathApp = return $ postgrest (testCfgExtraSearchPath testDbConn) refDbStructure pool getTime $ pure () let reset :: IO () reset = resetDb testDbConn @@ -90,7 +92,6 @@ main = do , ("Feature.SingularSpec" , Feature.SingularSpec.spec) , ("Feature.StructureSpec" , Feature.StructureSpec.spec) , ("Feature.AndOrParamsSpec" , Feature.AndOrParamsSpec.spec) - , ("Feature.NonexistentSchemaSpec" , Feature.NonexistentSchemaSpec.spec) ] ++ extraSpecs hspec $ do @@ -131,3 +132,7 @@ main = do -- this test runs with a nonexistent db-schema beforeAll_ reset . before nonexistentSchemaApp $ describe "Feature.NonexistentSchemaSpec" Feature.NonexistentSchemaSpec.spec + + -- this test runs with an extra search path + beforeAll_ reset . before extraSearchPathApp $ + describe "Feature.ExtraSearchPathSpec" Feature.ExtraSearchPathSpec.spec diff --git a/test/SpecHelper.hs b/test/SpecHelper.hs index 53ac02a58..403b5104d 100644 --- a/test/SpecHelper.hs +++ b/test/SpecHelper.hs @@ -78,6 +78,8 @@ _baseCfg = -- Connection Settings ] -- Default role claim key (Right [JSPKey "role"]) + -- Empty db-extra-search-path + [] testCfg :: Text -> AppConfig testCfg testDbConn = _baseCfg { configDatabase = testDbConn } @@ -122,6 +124,9 @@ testCfgAsymJWKSet testDbConn = (testCfg testDbConn) { testNonexistentSchemaCfg :: Text -> AppConfig testNonexistentSchemaCfg testDbConn = (testCfg testDbConn) { configSchema = "nonexistent" } +testCfgExtraSearchPath :: Text -> AppConfig +testCfgExtraSearchPath testDbConn = (testCfg testDbConn) { configExtraSearchPath = ["public", "extensions"] } + setupDb :: Text -> IO () setupDb dbConn = do loadFixture dbConn "database" diff --git a/test/fixtures/data.sql b/test/fixtures/data.sql index db44ffd4e..a1f7fe800 100644 --- a/test/fixtures/data.sql +++ b/test/fixtures/data.sql @@ -463,3 +463,13 @@ select 'last_name_' || generate_series, '2018-10-11' from generate_series(1, 6); + +TRUNCATE TABLE ltree_sample CASCADE; +INSERT INTO ltree_sample VALUES ('Top'); +INSERT INTO ltree_sample VALUES ('Top.Science'); +INSERT INTO ltree_sample VALUES ('Top.Science.Astronomy'); +INSERT INTO ltree_sample VALUES ('Top.Science.Astronomy.Astrophysics'); +INSERT INTO ltree_sample VALUES ('Top.Science.Astronomy.Cosmology'); + +TRUNCATE TABLE isn_sample CASCADE; +INSERT INTO isn_sample VALUES ('978-0-393-04002-9', 'Mathematics: From the Birth of Numbers'); diff --git a/test/fixtures/database.sql b/test/fixtures/database.sql index 409d98b90..52e6aad94 100644 --- a/test/fixtures/database.sql +++ b/test/fixtures/database.sql @@ -1,3 +1,3 @@ set client_min_messages to warning; -DROP SCHEMA IF EXISTS test, private, postgrest, jwt, public, تست CASCADE; +DROP SCHEMA IF EXISTS test, private, postgrest, jwt, public, تست, extensions CASCADE; DROP TYPE IF EXISTS jwt_token CASCADE; diff --git a/test/fixtures/privileges.sql b/test/fixtures/privileges.sql index d2cd5d3cd..e1123004e 100644 --- a/test/fixtures/privileges.sql +++ b/test/fixtures/privileges.sql @@ -5,6 +5,7 @@ GRANT USAGE ON SCHEMA , jwt , public , "تست" + , extensions TO postgrest_test_anonymous; -- Schema test objects @@ -92,6 +93,8 @@ GRANT ALL ON TABLE , contract , player_view , contract_view + , ltree_sample + , isn_sample TO postgrest_test_anonymous; GRANT INSERT ON TABLE insertonly TO postgrest_test_anonymous; diff --git a/test/fixtures/schema.sql b/test/fixtures/schema.sql index 7de705a47..647e96f0a 100755 --- a/test/fixtures/schema.sql +++ b/test/fixtures/schema.sql @@ -17,7 +17,7 @@ CREATE SCHEMA postgrest; CREATE SCHEMA private; CREATE SCHEMA test; CREATE SCHEMA تست; - +CREATE SCHEMA extensions; -- -- Name: plpgsql; Type: EXTENSION; Schema: -; Owner: - @@ -1605,6 +1605,27 @@ create view test.contract_view as select * from test.contract; create type public.my_type AS enum ('something'); -CREATE FUNCTION test.test_arg(my_arg public.my_type) RETURNS text AS $$ - SELECT 'foobar'::text; -$$ LANGUAGE sql; +create function test.test_arg(my_arg public.my_type) returns text as $$ + select 'foobar'::text; +$$ language sql; + +create extension if not exists ltree with schema public; + +create table test.ltree_sample ( + path public.ltree +); + +CREATE FUNCTION test.number_of_labels(test.ltree_sample) RETURNS integer AS $$ + SELECT nlevel($1.path) +$$ language sql; + +create extension if not exists isn with schema extensions; + +create table test.isn_sample ( + id extensions.isbn, + name text +); + +create function test.is_valid_isbn(input text) returns boolean as $$ + select is_valid(input::isbn); +$$ language sql;