Fix #746 (#769)

also fix broken text env

After the last couple of commits the tests ran correctly only on a
fresh db, in addition, the roles within the db were not created/dropped
on each request and we need that since their privileges differ and we
need to to have an absolute clean db on each execution

CHANGELOG.md

@@ -27,6 +27,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Allow using nulls order without explicit order direction - @steve-chavez
 - Fatal error on postgres unsupported version, format supported version in error message - @steve-chavez
 - Prevent database memory cosumption by prepared statements caches - @ruslantalpa
+- Use specific columns in the RETURNING section - @ruslantalpa
 
 ### Changed
 - Use HTTP 400 for raise\_exception - @begriffs

src/PostgREST/App.hs

@@ -62,6 +62,7 @@ import           PostgREST.QueryBuilder ( callProc
                                         , createReadStatement
                                         , createWriteStatement
                                         , ResultsWithCount
+                                        , returningF
                                         )
 import           PostgREST.Types
 import           PostgREST.OpenAPI
@@ -262,8 +263,9 @@ app dbStructure conf apiRequest =
       mapSnd f (a, b) = (a, f b)
       readDbRequest = DbRead <$> readRequest (configMaxRows conf) (dbRelations dbStructure) (map (mapSnd pdReturnType) $ dbProcs dbStructure) apiRequest
       mutateDbRequest = DbMutate <$> mutateRequest apiRequest
-      selectQuery = requestToQuery schema False <$> readDbRequest
+      returningSql = returningF (iTarget apiRequest) (iPreferRepresentation apiRequest) <$> readDbRequest
-      mutateQuery = requestToQuery schema False <$> mutateDbRequest
+      selectQuery = requestToQuery schema False "" <$> readDbRequest
+      mutateQuery = requestToQuery schema False <$> returningSql <*> mutateDbRequest
       countQuery = requestToCountQuery schema <$> readDbRequest
       readSqlParts = (,) <$> selectQuery <*> countQuery
       mutateSqlParts = (,) <$> selectQuery <*> mutateQuery
@@ -288,6 +290,7 @@ responseContentTypeOrError accepts action = serves contentTypesForRequest accept
             "None of these Content-Types are available: " <> failed
         Just ct -> Right ct
 
+
 splitKeyValue :: BS.ByteString -> (BS.ByteString, BS.ByteString)
 splitKeyValue kv = (k, BS.tail v)
   where (k, v) = BS.break (== '=') kv

src/PostgREST/QueryBuilder.hs

@@ -23,6 +23,7 @@ module PostgREST.QueryBuilder (
   , pgFmtLit
   , requestToQuery
   , requestToCountQuery
+  , returningF
   , sourceCTEName
   , unquoted
   , ResultsWithCount
@@ -52,8 +53,8 @@ import           Data.Scientific         ( FPFormat (..)
                                          , isInteger
                                          )
 import           Protolude hiding        (from, intercalate, ord, cast)
+import           PostgREST.ApiRequest    (PreferRepresentation (..), Target (..))
 import           Unsafe                  (unsafeHead)
-import           PostgREST.ApiRequest    (PreferRepresentation (..))
 
 {-| The generic query result format used by API responses. The location header
     is represented as a list of strings containing variable bindings like
@@ -121,12 +122,12 @@ createWriteStatement _ _ mutateQuery _ None
       WITH {sourceCTEName} AS ({mutateQuery})
       SELECT '', 0, {noLocationF}, '' |]
 
-createWriteStatement qi _ mutateQuery isSingle HeadersOnly
+createWriteStatement _ _ mutateQuery isSingle HeadersOnly
                      pKeys _ (PayloadJSON _) =
   unicodeStatement sql encodeUniformObjs decodeStandardMay True
  where
   sql = [qc|
-      WITH {sourceCTEName} AS ({mutateQuery} RETURNING {fromQi qi}.*)
+      WITH {sourceCTEName} AS ({mutateQuery})
       SELECT {cols}
       FROM (SELECT 1 FROM {sourceCTEName}) _postgrest_t |]
   cols = intercalate ", " [
@@ -136,12 +137,12 @@ createWriteStatement qi _ mutateQuery isSingle HeadersOnly
       "''"
     ]
 
-createWriteStatement qi selectQuery mutateQuery isSingle Full
+createWriteStatement _ selectQuery mutateQuery isSingle Full
                      pKeys asCsv (PayloadJSON _) =
   unicodeStatement sql encodeUniformObjs decodeStandardMay True
  where
   sql = [qc|
-      WITH {sourceCTEName} AS ({mutateQuery} RETURNING {fromQi qi}.*)
+      WITH {sourceCTEName} AS ({mutateQuery})
       SELECT {cols}
       FROM ({selectQuery}) _postgrest_t |]
   cols = intercalate ", " [
@@ -320,8 +321,8 @@ requestToCountQuery schema (DbRead (Node (Select _ _ conditions _ _, (mainTbl, _
    fn Filter{value=VForeignKey _ _} = False
    localConditions = filter fn conditions
 
-requestToQuery :: Schema -> Bool -> DbRequest -> SqlQuery
+requestToQuery :: Schema -> Bool -> SqlFragment -> DbRequest -> SqlQuery
-requestToQuery schema isParent (DbRead (Node (Select colSelects tbls conditions ord range, (nodeName, maybeRelation, _)) forest)) =
+requestToQuery schema isParent _ (DbRead (Node (Select colSelects tbls conditions ord range, (nodeName, maybeRelation, _)) forest)) =
   query
   where
     -- TODO! the following helper functions are just to remove the "schema" part when the table is "source" which is the name
@@ -358,8 +359,7 @@ requestToQuery schema isParent (DbRead (Node (Select colSelects tbls conditions
            <> "SELECT array_to_json(array_agg(row_to_json("<>pgFmtIdent table<>"))) "
            <> "FROM (" <> subquery <> ") " <> pgFmtIdent table
            <> "), '[]') AS " <> pgFmtIdent (fromMaybe name alias)
-           where subquery = requestToQuery schema False (DbRead (Node n forst))
+           where subquery = requestToQuery schema False "" (DbRead (Node n forst))
-
     getQueryParts (Node n@(_, (name, Just r@Relation{relType=Parent,relTable=Table{tableName=table}}, alias)) forst) (j,s) = (joi:j,sel:s)
       where
         node_name = fromMaybe name alias
@@ -369,31 +369,30 @@ requestToQuery schema isParent (DbRead (Node (Select colSelects tbls conditions
         sel = "row_to_json(" <> pgFmtIdent local_table_name <> ".*) AS " <> pgFmtIdent node_name
         joi = " LEFT OUTER JOIN ( " <> subquery <> " ) AS " <> pgFmtIdent local_table_name  <>
               " ON " <> intercalate " AND " ( map (pgFmtCondition qi . replaceTableName local_table_name) (getJoinConditions r) )
-          where subquery = requestToQuery schema True (DbRead (Node n forst))
+          where subquery = requestToQuery schema True "" (DbRead (Node n forst))
     getQueryParts (Node n@(_, (name, Just Relation{relType=Many,relTable=Table{tableName=table}}, alias)) forst) (j,s) = (j,sel:s)
       where
         sel = "COALESCE (("
            <> "SELECT array_to_json(array_agg(row_to_json("<>pgFmtIdent table<>"))) "
            <> "FROM (" <> subquery <> ") " <> pgFmtIdent table
            <> "), '[]') AS " <> pgFmtIdent (fromMaybe name alias)
-           where subquery = requestToQuery schema False (DbRead (Node n forst))
+           where subquery = requestToQuery schema False "" (DbRead (Node n forst))
     --the following is just to remove the warning
     --getQueryParts is not total but requestToQuery is called only after addJoinConditions which ensures the only
     --posible relations are Child Parent Many
-    getQueryParts _ _ = undefined --error "undefined getQueryParts"
+    getQueryParts _ _ = undefined
-requestToQuery schema _ (DbMutate (Insert mainTbl (PayloadJSON rows))) =
+requestToQuery schema _ returningSql (DbMutate (Insert mainTbl (PayloadJSON rows))) =
-  let qi = QualifiedIdentifier schema mainTbl
+  insInto <> vals <> returningSql
-      cols = map pgFmtIdent $ fromMaybe [] (HM.keys <$> (rows V.!? 0))
+  where qi = QualifiedIdentifier schema mainTbl
-      colsString = intercalate ", " cols
+        cols = map pgFmtIdent $ fromMaybe [] (HM.keys <$> (rows V.!? 0))
-      insInto = unwords [ "INSERT INTO" , fromQi qi,
+        colsString = intercalate ", " cols
-          if T.null colsString then "" else "(" <> colsString <> ")"
+        insInto = unwords [ "INSERT INTO" , fromQi qi,
-        ]
+            if T.null colsString then "" else "(" <> colsString <> ")"
-      vals = unwords $ if T.null colsString
+          ]
-                then ["DEFAULT VALUES"]
+        vals = unwords $ if T.null colsString
-                else ["SELECT", colsString, "FROM json_populate_recordset(null::" , fromQi qi, ", $1)"] in
+                  then ["DEFAULT VALUES"]
-  insInto <> vals
+                  else ["SELECT", colsString, "FROM json_populate_recordset(null::" , fromQi qi, ", $1)"]
-
+requestToQuery schema _ returningSql (DbMutate (Update mainTbl (PayloadJSON rows) conditions)) =
-requestToQuery schema _ (DbMutate (Update mainTbl (PayloadJSON rows) conditions)) =
   case rows V.!? 0 of
     Just obj ->
       let assignments = map
@@ -401,20 +400,33 @@ requestToQuery schema _ (DbMutate (Update mainTbl (PayloadJSON rows) conditions)
       unwords [
         "UPDATE ", fromQi qi,
         " SET " <> intercalate "," assignments <> " ",
-        ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions
+        ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions,
+        returningSql
         ]
     Nothing -> undefined
   where
     qi = QualifiedIdentifier schema mainTbl
-requestToQuery schema _ (DbMutate (Delete mainTbl conditions)) =
+requestToQuery schema _ returningSql (DbMutate (Delete mainTbl conditions)) =
   query
   where
     qi = QualifiedIdentifier schema mainTbl
     query = unwords [
       "DELETE FROM ", fromQi qi,
-      ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions
+      ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions,
+      returningSql
       ]
 
+returningF :: Target -> PreferRepresentation -> DbRequest -> SqlFragment
+returningF _ None _ = ""
+returningF (TargetIdent qi) _ (DbRead (Node (Select colSelects _ _ _ _, (_, _, _)) forest)) =
+  " RETURNING " <>
+  intercalate ", " ( map (pgFmtSelectItem qi) colSelects ++ map (pgFmtColumn qi . colName) fks)
+  where
+    fks = concatMap (fromMaybe [] . f) forest
+    f (Node (_, (_, Just Relation{relFColumns=cols, relType=Parent}, _)) _) = Just cols
+    f _ = Nothing
+returningF _ _ _ = ""
+
 sourceCTEName :: SqlFragment
 sourceCTEName = "pg_source"
 

test/Feature/InsertSpec.hs

@@ -201,6 +201,28 @@ spec = do
           , matchStatus  = 201
           , matchHeaders = []
           }
+    context "table with limited privileges" $ do
+      it "succeeds if correct select is applied" $
+        request methodPost "/limited_article_stars?select=article_id,user_id" [("Prefer", "return=representation")]
+          [json| {"article_id": 2, "user_id": 1} |] `shouldRespondWith` ResponseMatcher {
+            matchBody    = Just [str|{"article_id":2,"user_id":1}|]
+          , matchStatus  = 201
+          , matchHeaders = []
+          }
+      it "fails if more columns are selected" $
+        request methodPost "/limited_article_stars?select=article_id,user_id,created_at" [("Prefer", "return=representation")]
+          [json| {"article_id": 2, "user_id": 2} |] `shouldRespondWith` ResponseMatcher {
+            matchBody    = Just [str|{"hint":null,"details":null,"code":"42501","message":"permission denied for relation limited_article_stars"}|]
+          , matchStatus  = 401
+          , matchHeaders = []
+          }
+      it "fails if select is not specified" $
+        request methodPost "/limited_article_stars" [("Prefer", "return=representation")]
+          [json| {"article_id": 3, "user_id": 1} |] `shouldRespondWith` ResponseMatcher {
+            matchBody    = Just [str|{"hint":null,"details":null,"code":"42501","message":"permission denied for relation limited_article_stars"}|]
+          , matchStatus  = 401
+          , matchHeaders = []
+          }
 
   describe "CSV insert" $ do
 

test/Main.hs

@@ -31,6 +31,7 @@ import Protolude
 main :: IO ()
 main = do
   testDbConn <- getEnvVarWithDefault "POSTGREST_TEST_CONNECTION" "postgres://postgrest_test@localhost/postgrest_test"
+  setupDb testDbConn
 
   pool <- P.acquire (3, 10, toS testDbConn)
   -- ask for the OS time at most once per second

test/SpecHelper.hs

@@ -84,6 +84,15 @@ testCfgBinaryJWT testDbConn = (testCfg testDbConn) { configJwtSecret = Just secr
   where secretBs = B64.decodeLenient "h2CGB1FoBd51aQooCS2g+UmRgYQfTPQ6v3+9ALbaqM4="
 
 
+setupDb :: Text -> IO ()
+setupDb dbConn = do
+  loadFixture dbConn "database"
+  loadFixture dbConn "roles"
+  loadFixture dbConn "schema"
+  loadFixture dbConn "jwt"
+  loadFixture dbConn "privileges"
+  resetDb dbConn
+
 resetDb :: Text -> IO ()
 resetDb dbConn = loadFixture dbConn "data"
 

test/create_test_db

@@ -23,10 +23,10 @@ URI=$(echo $1 | cut -d'/' -f1-3)
 HOST_PORT=$(echo $URI | cut -d'/' -f3 | cut -d'@' -f2 )
 DB=$2
 # Specify the username of choice, or let the script create a random unique user by appending the database name
-TEST_USER_NAME=${3:-postgrest_test_$DB}
+TEST_USER_NAME=postgrest_test_authenticator
 # New password will get assigned only if the user does not already exist
 # Otherwise make sure to provide the correct password for the existing user
-TEST_USER_PASS=${4:-$(cat /dev/urandom | env LC_CTYPE=C tr -dc 'a-zA-Z0-9' | fold -w 16 | head -n 1)a}
+TEST_USER_PASS=$(cat /dev/urandom | env LC_CTYPE=C tr -dc 'a-zA-Z0-9' | fold -w 16 | head -n 1)
 
 PGOPTIONS='-c client_min_messages=WARNING' psql "$URI" -Xq >/dev/null -c 'select rolcreatedb from pg_authid where rolname = current_user;' 2>/dev/null
 if [ $? -ne 0 ]; then
@@ -41,53 +41,16 @@ FROM pg_stat_activity
 WHERE pg_stat_activity.datname = '$DB'
   AND pid <> pg_backend_pid();
 
-drop database if exists "$DB";
+DROP DATABASE IF EXISTS $DB;
-
+DROP ROLE IF EXISTS $TEST_USER_NAME;
-create database "$DB" encoding = 'UTF8';
+CREATE USER $TEST_USER_NAME WITH LOGIN NOINHERIT PASSWORD '$TEST_USER_PASS' CREATEROLE;
-
+CREATE DATABASE $DB OWNER $TEST_USER_NAME;
-DO \$\$
-  BEGIN
-    IF NOT EXISTS (SELECT * FROM pg_catalog.pg_roles WHERE rolname = '$TEST_USER_NAME')
-      THEN CREATE ROLE $TEST_USER_NAME WITH LOGIN NOINHERIT PASSWORD '$TEST_USER_PASS';
-    END IF;
-  END \$\$;
-DO \$\$
-  BEGIN
-    IF NOT EXISTS (SELECT * FROM pg_catalog.pg_roles WHERE rolname = 'postgrest_test_anonymous')
-      THEN CREATE ROLE postgrest_test_anonymous;
-    END IF;
-  END \$\$;
-DO \$\$
-  BEGIN
-    IF NOT EXISTS (SELECT * FROM pg_catalog.pg_roles WHERE rolname = 'postgrest_test_default_role')
-      THEN CREATE ROLE postgrest_test_default_role;
-    END IF;
-  END \$\$;
-DO \$\$
-  BEGIN
-    IF NOT EXISTS (SELECT * FROM pg_catalog.pg_roles WHERE rolname = 'postgrest_test_author')
-      THEN CREATE ROLE postgrest_test_author;
-    END IF;
-  END \$\$;
-
-DO \$\$
-  BEGIN
-  IF NOT EXISTS (select * from pg_roles where rolname = 'postgrest_test_anonymous' AND pg_has_role('$TEST_USER_NAME', oid, 'member'))
-    THEN GRANT postgrest_test_anonymous TO $TEST_USER_NAME;
-  END IF;
-  IF NOT EXISTS (select * from pg_roles where rolname = 'postgrest_test_author' AND pg_has_role('$TEST_USER_NAME', oid, 'member'))
-    THEN GRANT postgrest_test_author TO $TEST_USER_NAME;
-  END IF;
-  IF NOT EXISTS (select * from pg_roles where rolname = 'postgrest_test_default_role' AND pg_has_role('$TEST_USER_NAME', oid, 'member'))
-    THEN GRANT postgrest_test_default_role TO $TEST_USER_NAME;
-  END IF;
-END \$\$;
 EOF
 
-PGDATABASE=$DB PGOPTIONS='-c client_min_messages=WARNING' psql "$URI" --set=db=$DB -Xqf $BASEPATH/fixtures/database.sql
+PGDATABASE=$DB PGOPTIONS='-c client_min_messages=WARNING' psql "$URI" --set=db=$DB -Xq <<EOF
-PGDATABASE=$DB PGOPTIONS='-c client_min_messages=WARNING' psql "$URI" -Xqf $BASEPATH/fixtures/schema.sql
+  CREATE EXTENSION IF NOT EXISTS pgcrypto;
-PGDATABASE=$DB PGOPTIONS='-c client_min_messages=WARNING' psql "$URI" -Xqf $BASEPATH/fixtures/jwt.sql
+  ALTER DATABASE ${DB} SET request.jwt.claim.id = '-1';
-PGDATABASE=$DB PGOPTIONS='-c client_min_messages=WARNING' psql "$URI" --set=test_user_name="$TEST_USER_NAME" -Xqf $BASEPATH/fixtures/privileges.sql
+EOF
 
 # Create a new connection string to use with the test runner
 echo 'postgres://'${TEST_USER_NAME}':'$TEST_USER_PASS'@'$HOST_PORT'/'$DB

test/fixtures/database.sql

@@ -1,3 +1,3 @@
-CREATE EXTENSION IF NOT EXISTS pgcrypto;
+set client_min_messages to warning;
-
+DROP SCHEMA IF EXISTS test, private, postgrest, jwt, تست CASCADE;
-ALTER DATABASE :db SET request.jwt.claim.id = '-1';
+DROP TYPE IF EXISTS jwt_token CASCADE;

test/fixtures/jwt.sql

@@ -1,5 +1,6 @@
 -- From michelp/pgjwt commit c02bbd3
 BEGIN;
+set client_min_messages to warning;
 DROP SCHEMA IF EXISTS jwt CASCADE;
 CREATE SCHEMA jwt;
 

test/fixtures/privileges.sql

@@ -58,7 +58,6 @@ TO postgrest_test_anonymous;
 GRANT USAGE ON SCHEMA test TO postgrest_test_author;
 GRANT ALL ON TABLE authors_only TO postgrest_test_author;
 
-GRANT USAGE ON SCHEMA postgrest,private,test to :test_user_name;
+GRANT SELECT (article_id, user_id) ON TABLE limited_article_stars TO postgrest_test_anonymous;
-GRANT ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA postgrest,private,test TO :test_user_name;
+GRANT INSERT (article_id, user_id) ON TABLE limited_article_stars TO postgrest_test_anonymous;
-GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA postgrest,private,test TO :test_user_name;
+GRANT UPDATE (article_id, user_id) ON TABLE limited_article_stars TO postgrest_test_anonymous;
-

test/fixtures/roles.sql

@@ -0,0 +1,7 @@
+\set AUTHENTICATOR current_user
+DROP ROLE IF EXISTS postgrest_test_anonymous, postgrest_test_default_role, postgrest_test_author;
+CREATE ROLE postgrest_test_anonymous;
+CREATE ROLE postgrest_test_default_role;
+CREATE ROLE postgrest_test_author;
+
+GRANT postgrest_test_anonymous, postgrest_test_default_role, postgrest_test_author TO :USER;

test/fixtures/schema.sql

@@ -382,6 +382,10 @@ CREATE TABLE articles (
 
 SET search_path = test, pg_catalog;
 
+CREATE VIEW limited_article_stars AS
+  SELECT article_id, user_id, created_at FROM private.article_stars;
+
+
 --
 -- Name: articleStars; Type: VIEW; Schema: test; Owner: -
 --