From 4ac47df5287a5d89eb9c9a8f7f1c91b6d6cf7b72 Mon Sep 17 00:00:00 2001 From: Laurence Isla Date: Wed, 10 Aug 2022 11:50:57 -0500 Subject: [PATCH] tests: Add tests for the pg-safeupdate library --- default.nix | 12 ++-- postgrest.cabal | 1 + test/spec/Feature/Query/PgSafeUpdateSpec.hs | 76 +++++++++++++++++++++ test/spec/Main.hs | 43 +++++++----- test/spec/SpecHelper.hs | 3 + test/spec/fixtures/data.sql | 9 +++ test/spec/fixtures/privileges.sql | 4 ++ test/spec/fixtures/schema.sql | 27 ++++++++ 8 files changed, 151 insertions(+), 24 deletions(-) create mode 100644 test/spec/Feature/Query/PgSafeUpdateSpec.hs diff --git a/default.nix b/default.nix index 56de4c707..d5615c317 100644 --- a/default.nix +++ b/default.nix @@ -46,12 +46,12 @@ let postgresqlVersions = [ - { name = "postgresql-14"; postgresql = pkgs.postgresql_14.withPackages (p: [ p.postgis ]); } - { name = "postgresql-13"; postgresql = pkgs.postgresql_13.withPackages (p: [ p.postgis ]); } - { name = "postgresql-12"; postgresql = pkgs.postgresql_12.withPackages (p: [ p.postgis ]); } - { name = "postgresql-11"; postgresql = pkgs.postgresql_11.withPackages (p: [ p.postgis ]); } - { name = "postgresql-10"; postgresql = pkgs.postgresql_10.withPackages (p: [ p.postgis ]); } - { name = "postgresql-9.6"; postgresql = pkgs.postgresql_9_6.withPackages (p: [ p.postgis ]); } + { name = "postgresql-14"; postgresql = pkgs.postgresql_14.withPackages (p: [ p.postgis p.pg_safeupdate ]); } + { name = "postgresql-13"; postgresql = pkgs.postgresql_13.withPackages (p: [ p.postgis p.pg_safeupdate ]); } + { name = "postgresql-12"; postgresql = pkgs.postgresql_12.withPackages (p: [ p.postgis p.pg_safeupdate ]); } + { name = "postgresql-11"; postgresql = pkgs.postgresql_11.withPackages (p: [ p.postgis p.pg_safeupdate ]); } + { name = "postgresql-10"; postgresql = pkgs.postgresql_10.withPackages (p: [ p.postgis p.pg_safeupdate ]); } + { name = "postgresql-9.6"; postgresql = pkgs.postgresql_9_6.withPackages (p: [ p.postgis p.pg_safeupdate ]); } ]; patches = diff --git a/postgrest.cabal b/postgrest.cabal index fbd017ba0..837d07d44 100644 --- a/postgrest.cabal +++ b/postgrest.cabal @@ -201,6 +201,7 @@ test-suite spec Feature.Query.JsonOperatorSpec Feature.Query.MultipleSchemaSpec Feature.Query.ErrorSpec + Feature.Query.PgSafeUpdateSpec Feature.Query.PostGISSpec Feature.Query.QueryLimitedSpec Feature.Query.QuerySpec diff --git a/test/spec/Feature/Query/PgSafeUpdateSpec.hs b/test/spec/Feature/Query/PgSafeUpdateSpec.hs new file mode 100644 index 000000000..e53412a85 --- /dev/null +++ b/test/spec/Feature/Query/PgSafeUpdateSpec.hs @@ -0,0 +1,76 @@ +module Feature.Query.PgSafeUpdateSpec where + +import Network.Wai (Application) + +import Network.HTTP.Types +import Test.Hspec hiding (pendingWith) +import Test.Hspec.Wai +import Test.Hspec.Wai.JSON + +import Protolude hiding (get, put) +import SpecHelper + +spec :: SpecWith ((), Application) +spec = + describe "Enabling pg-safeupdate" $ do + context "Full table update" $ + it "does not update and throws no error if no condition is present" $ + request methodPatch "/safe_update" + [("Prefer", "count=exact")] + [json| {"name": "New name"} |] + `shouldRespondWith` + "" + { matchStatus = 404 + , matchHeaders = [ matchHeaderAbsent hContentType + , "Content-Range" <:> "*/0" ] + } + + context "Full table delete" $ do + it "does not delete and throws error if no condition is present" $ + request methodDelete "/safe_delete" [] mempty + `shouldRespondWith` + [json|{ + "code": "21000", + "details": null, + "hint": null, + "message": "DELETE requires a WHERE clause" + }|] + { matchStatus = 400 } + + it "allows full table delete if a filter is present" $ + request methodDelete "/safe_delete?id=gt.0" + [("Prefer", "count=exact")] + mempty + `shouldRespondWith` + "" + { matchStatus = 204 + , matchHeaders = [ matchHeaderAbsent hContentType + , "Content-Range" <:> "*/3" ] + } + +disabledSpec :: SpecWith ((), Application) +disabledSpec = + describe "Disabling pg-safeupdate" $ do + context "Full table update" $ + it "does not update and does not throw error if no condition is present" $ + request methodPatch "/unsafe_update" + [("Prefer", "count=exact")] + [json| {"name": "New name"} |] + `shouldRespondWith` + "" + { matchStatus = 404 + , matchHeaders = [ matchHeaderAbsent hContentType + , "Content-Range" <:> "*/0" ] + } + + context "Full table delete" $ + it "deletes and does not throw error if no condition is present" $ do + request methodDelete "/unsafe_delete" + [("Prefer", "count=exact")] + mempty + `shouldRespondWith` + "" + { matchStatus = 204 + , matchHeaders = [ matchHeaderAbsent hContentType + , "Content-Range" <:> "*/3" ] + } diff --git a/test/spec/Main.hs b/test/spec/Main.hs index f02b72e47..2ab5b4b91 100644 --- a/test/spec/Main.hs +++ b/test/spec/Main.hs @@ -45,6 +45,7 @@ import qualified Feature.Query.HtmlRawOutputSpec import qualified Feature.Query.InsertSpec import qualified Feature.Query.JsonOperatorSpec import qualified Feature.Query.MultipleSchemaSpec +import qualified Feature.Query.PgSafeUpdateSpec import qualified Feature.Query.PlanSpec import qualified Feature.Query.PostGISSpec import qualified Feature.Query.QueryLimitedSpec @@ -112,6 +113,7 @@ main = do forceRollbackApp = app testCfgForceRollback testCfgLegacyGucsApp = app testCfgLegacyGucs planEnabledApp = app testPlanEnabledCfg + pgSafeUpdateApp = app testPgSafeUpdateEnabledCfg extraSearchPathApp = appDbs testCfgExtraSearchPath unicodeApp = appDbs testUnicodeCfg @@ -126,24 +128,25 @@ main = do analyzeTable "child_entities" specs = uncurry describe <$> [ - ("Feature.Query.AndOrParamsSpec" , Feature.Query.AndOrParamsSpec.spec actualPgVersion) - , ("Feature.Auth.AuthSpec" , Feature.Auth.AuthSpec.spec actualPgVersion) - , ("Feature.ConcurrentSpec" , Feature.ConcurrentSpec.spec) - , ("Feature.CorsSpec" , Feature.CorsSpec.spec) - , ("Feature.Query.DeleteSpec" , Feature.Query.DeleteSpec.spec) - , ("Feature.Query.EmbedDisambiguationSpec" , Feature.Query.EmbedDisambiguationSpec.spec) - , ("Feature.Query.EmbedInnerJoinSpec" , Feature.Query.EmbedInnerJoinSpec.spec) - , ("Feature.Query.InsertSpec" , Feature.Query.InsertSpec.spec actualPgVersion) - , ("Feature.Query.JsonOperatorSpec" , Feature.Query.JsonOperatorSpec.spec actualPgVersion) - , ("Feature.OpenApi.OpenApiSpec" , Feature.OpenApi.OpenApiSpec.spec actualPgVersion) - , ("Feature.OptionsSpec" , Feature.OptionsSpec.spec actualPgVersion) - , ("Feature.Query.PlanSpec.disabledSpec" , Feature.Query.PlanSpec.disabledSpec) - , ("Feature.Query.QuerySpec" , Feature.Query.QuerySpec.spec actualPgVersion) - , ("Feature.Query.RawOutputTypesSpec" , Feature.Query.RawOutputTypesSpec.spec) - , ("Feature.Query.RpcSpec" , Feature.Query.RpcSpec.spec actualPgVersion) - , ("Feature.Query.SingularSpec" , Feature.Query.SingularSpec.spec) - , ("Feature.Query.UpdateSpec" , Feature.Query.UpdateSpec.spec) - , ("Feature.Query.UpsertSpec" , Feature.Query.UpsertSpec.spec actualPgVersion) + ("Feature.Query.AndOrParamsSpec" , Feature.Query.AndOrParamsSpec.spec actualPgVersion) + , ("Feature.Auth.AuthSpec" , Feature.Auth.AuthSpec.spec actualPgVersion) + , ("Feature.ConcurrentSpec" , Feature.ConcurrentSpec.spec) + , ("Feature.CorsSpec" , Feature.CorsSpec.spec) + , ("Feature.Query.DeleteSpec" , Feature.Query.DeleteSpec.spec) + , ("Feature.Query.EmbedDisambiguationSpec" , Feature.Query.EmbedDisambiguationSpec.spec) + , ("Feature.Query.EmbedInnerJoinSpec" , Feature.Query.EmbedInnerJoinSpec.spec) + , ("Feature.Query.InsertSpec" , Feature.Query.InsertSpec.spec actualPgVersion) + , ("Feature.Query.JsonOperatorSpec" , Feature.Query.JsonOperatorSpec.spec actualPgVersion) + , ("Feature.OpenApi.OpenApiSpec" , Feature.OpenApi.OpenApiSpec.spec actualPgVersion) + , ("Feature.OptionsSpec" , Feature.OptionsSpec.spec actualPgVersion) + , ("Feature.Query.PgSafeUpdateSpec.disabledSpec" , Feature.Query.PgSafeUpdateSpec.disabledSpec) + , ("Feature.Query.PlanSpec.disabledSpec" , Feature.Query.PlanSpec.disabledSpec) + , ("Feature.Query.QuerySpec" , Feature.Query.QuerySpec.spec actualPgVersion) + , ("Feature.Query.RawOutputTypesSpec" , Feature.Query.RawOutputTypesSpec.spec) + , ("Feature.Query.RpcSpec" , Feature.Query.RpcSpec.spec actualPgVersion) + , ("Feature.Query.SingularSpec" , Feature.Query.SingularSpec.spec) + , ("Feature.Query.UpdateSpec" , Feature.Query.UpdateSpec.spec) + , ("Feature.Query.UpsertSpec" , Feature.Query.UpsertSpec.spec actualPgVersion) ] hspec $ do @@ -234,6 +237,10 @@ main = do parallel $ before planEnabledApp $ describe "Feature.Query.PlanSpec.spec" $ Feature.Query.PlanSpec.spec actualPgVersion + -- this test runs with a pre request to enable the pg-safeupdate library per-session + parallel $ before pgSafeUpdateApp $ + describe "Feature.Query.PgSafeUpdateSpec.spec" Feature.Query.PgSafeUpdateSpec.spec + -- Note: the rollback tests can not run in parallel, because they test persistance and -- this results in race conditions diff --git a/test/spec/SpecHelper.hs b/test/spec/SpecHelper.hs index 8f8ad72a8..40e39c0cf 100644 --- a/test/spec/SpecHelper.hs +++ b/test/spec/SpecHelper.hs @@ -196,6 +196,9 @@ testMultipleSchemaCfg = baseCfg { configDbSchemas = fromList ["v1", "v2", "SPECI testCfgLegacyGucs :: AppConfig testCfgLegacyGucs = baseCfg { configDbUseLegacyGucs = False } +testPgSafeUpdateEnabledCfg :: AppConfig +testPgSafeUpdateEnabledCfg = baseCfg { configDbPreRequest = Just $ QualifiedIdentifier "test" "load_safeupdate" } + analyzeTable :: Text -> IO () analyzeTable tableName = void $ readProcess "psql" ["--set", "ON_ERROR_STOP=1", "-a", "-c", toS $ "ANALYZE test.\"" <> tableName <> "\""] [] diff --git a/test/spec/fixtures/data.sql b/test/spec/fixtures/data.sql index ef2d0bd78..108213374 100644 --- a/test/spec/fixtures/data.sql +++ b/test/spec/fixtures/data.sql @@ -803,3 +803,12 @@ INSERT INTO "SPECIAL ""@/\#~_-".names (id, name) VALUES (1, 'John'), (2, 'Mary') TRUNCATE TABLE do$llar$s CASCADE; INSERT INTO do$llar$s (a$num$) VALUES (100), (200), (300); + +TRUNCATE TABLE safe_update CASCADE; +INSERT INTO safe_update(id, name) VALUES (1, 'First'), (2, 'Second'), (3, 'Third'); +TRUNCATE TABLE safe_delete CASCADE; +INSERT INTO safe_delete(id, name) VALUES (1, 'First'), (2, 'Second'), (3, 'Third'); +TRUNCATE TABLE unsafe_update CASCADE; +INSERT INTO unsafe_update(id, name) VALUES (1, 'First'), (2, 'Second'), (3, 'Third'); +TRUNCATE TABLE unsafe_delete CASCADE; +INSERT INTO unsafe_delete(id, name) VALUES (1, 'First'), (2, 'Second'), (3, 'Third'); diff --git a/test/spec/fixtures/privileges.sql b/test/spec/fixtures/privileges.sql index ca6d1e52e..9a0bc95a8 100644 --- a/test/spec/fixtures/privileges.sql +++ b/test/spec/fixtures/privileges.sql @@ -195,6 +195,10 @@ GRANT ALL ON TABLE , shop_bles , "SPECIAL ""@/\#~_-".names , do$llar$s + , safe_update + , safe_delete + , unsafe_update + , unsafe_delete TO postgrest_test_anonymous; GRANT INSERT ON TABLE insertonly TO postgrest_test_anonymous; diff --git a/test/spec/fixtures/schema.sql b/test/spec/fixtures/schema.sql index 70068b87b..b2e13ab19 100644 --- a/test/spec/fixtures/schema.sql +++ b/test/spec/fixtures/schema.sql @@ -2689,3 +2689,30 @@ $$ LANGUAGE sql; CREATE TABLE do$llar$s ( a$num$ numeric ); + +-- Tables and functions to test the pg-safeupdate library + +CREATE TABLE test.safe_update( + id INT PRIMARY KEY, + name TEXT +); + +CREATE TABLE test.safe_delete( + id INT PRIMARY KEY, + name TEXT +); + +CREATE TABLE test.unsafe_update( + id INT PRIMARY KEY, + name TEXT +); + +CREATE TABLE test.unsafe_delete( + id INT PRIMARY KEY, + name TEXT +); + +CREATE OR REPLACE FUNCTION test.load_safeupdate() RETURNS VOID AS $$ +BEGIN + LOAD 'safeupdate'; +END; $$ LANGUAGE plpgsql SECURITY DEFINER;