From 4544ce3255ad6972df8fa9c1b3847d3496985fc9 Mon Sep 17 00:00:00 2001 From: Ruslan Talpa Date: Tue, 8 Dec 2015 11:29:18 +0200 Subject: [PATCH] Fix #399 insert records in tables with no SELECT privileges --- CHANGELOG.md | 1 + src/PostgREST/ApiRequest.hs | 12 ++++++---- src/PostgREST/App.hs | 19 ++++++++-------- src/PostgREST/QueryBuilder.hs | 41 ++++++++++++++++++++++++----------- test/Feature/InsertSpec.hs | 9 ++++++++ test/Feature/StructureSpec.hs | 1 + test/fixtures/privileges.sql | 2 ++ test/fixtures/schema.sql | 9 ++++++++ 8 files changed, 68 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d76dead2d..6a51eb06f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - `pgFmtIdent` always quotes #388 - @calebmer - Default schema, changed from `"1"` to `public` - @calebmer - #414 revert to separate count query +- Fix #399, allow inserting in tables with no select privileges using "Prefer: representation=minimal" - @ruslantalpa ### Added - Allow order by computed columns - @diogob diff --git a/src/PostgREST/ApiRequest.hs b/src/PostgREST/ApiRequest.hs index 622fe37b9..958473d47 100644 --- a/src/PostgREST/ApiRequest.hs +++ b/src/PostgREST/ApiRequest.hs @@ -33,6 +33,8 @@ data Action = ActionCreate | ActionRead data Target = TargetIdent QualifiedIdentifier | TargetRoot | TargetUnknown [T.Text] +-- | How to return the inserted data +data PreferRepresentation = Full | HeadersOnly | None deriving Eq -- | Enumeration of currently supported content types for -- route responses and upload payloads data ContentType = ApplicationJSON | TextCSV deriving Eq @@ -59,7 +61,7 @@ data ApiRequest = ApiRequest { -- | Data sent by client and used for mutation actions , iPayload :: Maybe Payload -- | If client wants created items echoed back - , iPreferRepresentation :: Bool + , iPreferRepresentation :: PreferRepresentation -- | If client wants first row as raw object , iPreferSingular :: Bool -- | Whether the client wants a result count (slower) @@ -119,7 +121,7 @@ userApiRequest schema req reqBody = , iTarget = target , iAccepts = pickContentType $ lookupHeader "accept" , iPayload = relevantPayload - , iPreferRepresentation = hasPrefer "return=representation" + , iPreferRepresentation = representation , iPreferSingular = singular , iPreferCount = not $ hasPrefer "count=none" , iFilters = [ (k, fromJust v) | (k,v) <- qParams, k `notElem` ["select", "order"], isJust v ] @@ -138,8 +140,10 @@ userApiRequest schema req reqBody = lookupHeader = flip lookup hdrs hasPrefer val = any (\(h,v) -> h == "Prefer" && v == val) hdrs singular = hasPrefer "plurality=singular" - - + representation + | hasPrefer "return=representation" = Full + | hasPrefer "return=minimal" = None + | otherwise = HeadersOnly -- PRIVATE --------------------------------------------------------------- diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index a8ed9d0ff..ad21b2456 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -43,6 +43,7 @@ import PostgREST.DbStructure import PostgREST.RangeQuery import PostgREST.ApiRequest (ApiRequest(..), ContentType(..) , Action(..), Target(..) + , PreferRepresentation (..) , userApiRequest) import PostgREST.Types import PostgREST.Auth (tokenJWT) @@ -105,14 +106,14 @@ app dbStructure conf reqBody req = ) ] (fromMaybe "[]" body) - (ActionCreate, TargetIdent (QualifiedIdentifier _ table), + (ActionCreate, TargetIdent qi@(QualifiedIdentifier _ table), Just payload@(PayloadJSON (UniformObjects rows))) -> case mutateSqlParts of Left e -> return $ responseLBS status400 [jsonH] $ cs e Right (sq,mq) -> do let isSingle = (==1) $ V.length rows let pKeys = map pkName $ filter (filterPk schema table) allPrKeys -- would it be ok to move primary key detection in the query itself? - let stm = createWriteStatement sq mq isSingle (iPreferRepresentation apiRequest) pKeys (contentType == TextCSV) payload + let stm = createWriteStatement qi sq mq isSingle (iPreferRepresentation apiRequest) pKeys (contentType == TextCSV) payload row <- H.maybeEx stm let (_, _, location, body) = extractQueryResult row return $ responseLBS status201 @@ -120,28 +121,28 @@ app dbStructure conf reqBody req = contentTypeH, (hLocation, "/" <> cs table <> "?" <> cs (fromMaybe "" location)) ] - $ if iPreferRepresentation apiRequest then fromMaybe "[]" body else "" + $ if iPreferRepresentation apiRequest == Full then fromMaybe "[]" body else "" - (ActionUpdate, TargetIdent _, Just payload@(PayloadJSON _)) -> + (ActionUpdate, TargetIdent qi, Just payload@(PayloadJSON _)) -> case mutateSqlParts of Left e -> return $ responseLBS status400 [jsonH] $ cs e Right (sq,mq) -> do - let stm = createWriteStatement sq mq False (iPreferRepresentation apiRequest) [] (contentType == TextCSV) payload + let stm = createWriteStatement qi sq mq False (iPreferRepresentation apiRequest) [] (contentType == TextCSV) payload row <- H.maybeEx stm let (_, queryTotal, _, body) = extractQueryResult row r = contentRangeH 0 (queryTotal-1) (Just queryTotal) s = case () of _ | queryTotal == 0 -> status404 - | iPreferRepresentation apiRequest -> status200 + | iPreferRepresentation apiRequest == Full -> status200 | otherwise -> status204 return $ responseLBS s [contentTypeH, r] - $ if iPreferRepresentation apiRequest then fromMaybe "[]" body else "" + $ if iPreferRepresentation apiRequest == Full then fromMaybe "[]" body else "" - (ActionDelete, TargetIdent _, Nothing) -> + (ActionDelete, TargetIdent qi, Nothing) -> case mutateSqlParts of Left e -> return $ responseLBS status400 [jsonH] $ cs e Right (sq,mq) -> do let fakeload = PayloadJSON $ UniformObjects V.empty - let stm = createWriteStatement sq mq False False [] (contentType == TextCSV) fakeload + let stm = createWriteStatement qi sq mq False (iPreferRepresentation apiRequest) [] (contentType == TextCSV) fakeload row <- H.maybeEx stm let (_, queryTotal, _, _) = extractQueryResult row return $ if queryTotal == 0 diff --git a/src/PostgREST/QueryBuilder.hs b/src/PostgREST/QueryBuilder.hs index b7e32dd2a..cd3ba5036 100644 --- a/src/PostgREST/QueryBuilder.hs +++ b/src/PostgREST/QueryBuilder.hs @@ -52,6 +52,7 @@ import Data.Scientific ( FPFormat (..) , isInteger ) import Prelude hiding (unwords) +import PostgREST.ApiRequest (PreferRepresentation (..)) type PStmt = H.Stmt P.Postgres instance Monoid PStmt where @@ -79,23 +80,40 @@ createReadStatement selectQuery countQuery range isSingle countTotal asCsv = | isSingle = asJsonSingleF | otherwise = asJsonF -createWriteStatement :: SqlQuery -> SqlQuery -> Bool -> Bool -> +createWriteStatement :: QualifiedIdentifier -> SqlQuery -> SqlQuery -> Bool -> PreferRepresentation -> [Text] -> Bool -> Payload -> B.Stmt P.Postgres -createWriteStatement _ _ _ _ _ _ (PayloadParseError _) = undefined -createWriteStatement selectQuery mutateQuery isSingle echoRequested - pKeys asCsv (PayloadJSON (UniformObjects rows)) = +createWriteStatement _ _ _ _ _ _ _ (PayloadParseError _) = undefined +createWriteStatement _ _ mutateQuery _ None + _ _ (PayloadJSON (UniformObjects rows)) = B.Stmt ( "WITH " <> sourceCTEName <> " AS (" <> mutateQuery <> ") " <> + "SELECT null, 0, null, null" + ) (V.singleton . B.encodeValue . JSON.Array . V.map JSON.Object $ rows) True +createWriteStatement qi _ mutateQuery isSingle HeadersOnly + pKeys _ (PayloadJSON (UniformObjects rows)) = + B.Stmt ( + "WITH " <> sourceCTEName <> " AS (" <> mutateQuery <> " RETURNING " <> fromQi qi <> ".*" <> ") " <> + "SELECT " <> intercalate ", " [ + "null AS total_result_set", + "pg_catalog.count(t) AS page_total", + if isSingle then locationF pKeys else "null", + "null" + ] <> + " FROM (SELECT 1 FROM " <> sourceCTEName <> ") t" + ) (V.singleton . B.encodeValue . JSON.Array . V.map JSON.Object $ rows) True +createWriteStatement qi selectQuery mutateQuery isSingle Full + pKeys asCsv (PayloadJSON (UniformObjects rows)) = + B.Stmt ( + "WITH " <> sourceCTEName <> " AS (" <> mutateQuery <> " RETURNING " <> fromQi qi <> ".*" <> ") " <> "SELECT " <> intercalate ", " [ "null AS total_result_set", -- when updateing it does not make sense "pg_catalog.count(t) AS page_total", - location <> " AS header", - (if echoRequested then bodyF else "null") <> " AS body" + if isSingle then locationF pKeys else "null" <> " AS header", + bodyF <> " AS body" ] <> " FROM ( "<>selectQuery<>") t" ) (V.singleton . B.encodeValue . JSON.Array . V.map JSON.Object $ rows) True where - location = if isSingle then locationF pKeys else "null" bodyF | asCsv = asCsvF | isSingle = asJsonSingleF @@ -270,8 +288,7 @@ requestToQuery schema (DbMutate (Insert mainTbl (PayloadJSON (UniformObjects row "INSERT INTO ", fromQi qi, " (" <> colsString <> ")" <> " SELECT " <> colsString <> - " FROM json_populate_recordset(null::" , fromQi qi, ", ?)", - " RETURNING " <> fromQi qi <> ".*" + " FROM json_populate_recordset(null::" , fromQi qi, ", ?)" ] requestToQuery schema (DbMutate (Update mainTbl (PayloadJSON (UniformObjects rows)) conditions)) = case rows V.!? 0 of @@ -281,8 +298,7 @@ requestToQuery schema (DbMutate (Update mainTbl (PayloadJSON (UniformObjects row unwords [ "UPDATE ", fromQi qi, " SET " <> intercalate "," assignments <> " ", - ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions, - "RETURNING " <> fromQi qi <> ".*" + ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions ] Nothing -> undefined where @@ -294,8 +310,7 @@ requestToQuery schema (DbMutate (Delete mainTbl conditions)) = qi = QualifiedIdentifier schema mainTbl query = unwords [ "DELETE FROM ", fromQi qi, - ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions, - "RETURNING " <> fromQi qi <> ".*" + ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions ] sourceCTEName :: SqlFragment diff --git a/test/Feature/InsertSpec.hs b/test/Feature/InsertSpec.hs index 38a591153..394618f4d 100644 --- a/test/Feature/InsertSpec.hs +++ b/test/Feature/InsertSpec.hs @@ -93,6 +93,15 @@ spec struct pool = beforeAll_ resetDb $ around (withApp cfgDefault struct pool) simpleHeaders p `shouldSatisfy` matchHeader hLocation "/no_pk\\?a=eq.bar&b=eq.baz" simpleStatus p `shouldBe` created201 + it "can insert in tables with no select privileges" $ do + p <- request methodPost "/insertonly" + [("Prefer", "return=minimal")] + [json| { "v":"some value" } |] + liftIO $ do + simpleBody p `shouldBe` "" + simpleStatus p `shouldBe` created201 + + it "can post nulls" $ do p <- request methodPost "/no_pk" [("Prefer", "return=representation")] diff --git a/test/Feature/StructureSpec.hs b/test/Feature/StructureSpec.hs index 034081cb1..d8b06ed7f 100644 --- a/test/Feature/StructureSpec.hs +++ b/test/Feature/StructureSpec.hs @@ -28,6 +28,7 @@ spec struct pool = around (withApp cfgDefault struct pool) $ do , {"schema":"test","name":"has_count_column","insertable":false} , {"schema":"test","name":"has_fk","insertable":true} , {"schema":"test","name":"insertable_view_with_join","insertable":true} + , {"schema":"test","name":"insertonly","insertable":true} , {"schema":"test","name":"items","insertable":true} , {"schema":"test","name":"json","insertable":true} , {"schema":"test","name":"materialized_view","insertable":false} diff --git a/test/fixtures/privileges.sql b/test/fixtures/privileges.sql index 86b7c0c03..be3bc8ca7 100644 --- a/test/fixtures/privileges.sql +++ b/test/fixtures/privileges.sql @@ -34,6 +34,8 @@ GRANT ALL ON TABLE , users_tasks TO postgrest_test_anonymous; +GRANT INSERT ON TABLE insertonly TO postgrest_test_anonymous; + GRANT USAGE ON SEQUENCE auto_incrementing_pk_id_seq , items_id_seq diff --git a/test/fixtures/schema.sql b/test/fixtures/schema.sql index 5587c902a..d2f43c72b 100755 --- a/test/fixtures/schema.sql +++ b/test/fixtures/schema.sql @@ -500,6 +500,15 @@ CREATE TABLE nullable_integer ( ); +-- +-- Name: insertonly; Type: TABLE; Schema: test; Owner: - +-- + +CREATE TABLE insertonly ( + v text NOT NULL +); + + -- -- Name: projects; Type: TABLE; Schema: test; Owner: - --