From b3e88a37d3430addc4e633dfe10280dbe0138aa0 Mon Sep 17 00:00:00 2001 From: Ruslan Talpa Date: Mon, 2 Nov 2015 23:31:35 +0200 Subject: [PATCH 1/5] Refacttoring --- src/PostgREST/App.hs | 36 ++++++++++++++++-------------------- src/PostgREST/Parsers.hs | 11 ++++------- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index 2361a7355..11767b2ab 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -57,7 +57,11 @@ import Prelude app :: DbStructure -> AppConfig -> BL.ByteString -> Request -> H.Tx P.Postgres s Response app dbstructure conf reqBody req = case (path, verb) of - + -- ([table], v) -> + -- case request of + -- Left e -> return $ responseLBS status400 [jsonH] $ cs e + -- Right (selectQuery, mutateQuery, isSingle) -> + ([table], "GET") -> if range == Just emptyRange then return $ responseLBS status416 [] "HTTP Range error" @@ -83,13 +87,10 @@ app dbstructure conf reqBody req = if Prelude.null canonical then "" else "?" <> cs canonical ) ] (fromMaybe "[]" body) - where frm = fromMaybe 0 $ rangeOffset <$> range - request = parseRequest schema allRels table req reqBody - ([table], "POST") -> do - let echoRequested = hasPrefer "return=representation" + ([table], "POST") -> case request of Left e -> return $ responseLBS status400 [jsonH] $ cs e Right (selectQuery, mutateQuery, isSingle) -> do @@ -103,12 +104,8 @@ app dbstructure conf reqBody req = (hLocation, "/" <> cs table <> "?" <> cs (fromMaybe "" location)) ] $ if echoRequested then fromMaybe "[]" body else "" - where - request = parseRequest schema (fakeSourceRelations ++ allRels) table req reqBody - fakeSourceRelations = mapMaybe (toSourceRelation table) allRels - ([table], "PATCH") -> do - let echoRequested = hasPrefer "return=representation" + ([_], "PATCH") -> case request of Left e -> return $ responseLBS status400 [jsonH] $ cs e Right (selectQuery, mutateQuery, _) -> do @@ -122,11 +119,7 @@ app dbstructure conf reqBody req = return $ responseLBS s [contentTypeH, r] $ if echoRequested then fromMaybe "[]" body else "" - where - request = parseRequest schema (fakeSourceRelations ++ allRels) table req reqBody - fakeSourceRelations = mapMaybe (toSourceRelation table) allRels - - ([table], "DELETE") -> + ([_], "DELETE") -> case request of Left e -> return $ responseLBS status400 [jsonH] $ cs e Right (selectQuery, mutateQuery, _) -> do @@ -137,10 +130,6 @@ app dbstructure conf reqBody req = then responseLBS status404 [] "" else responseLBS status204 [("Content-Range", "*/"<> cs (show queryTotal))] "" - - where - request = parseRequest schema allRels table req reqBody - (["rpc", proc], "POST") -> do let qi = QualifiedIdentifier schema (cs proc) exists <- doesProcExist schema proc @@ -201,6 +190,8 @@ app dbstructure conf reqBody req = contentType = fromMaybe "application/json" $ contentTypeForAccept accept isCsv = contentType == csvMT contentTypeH = (hContentType, contentType) + echoRequested = hasPrefer "return=representation" + request = parseRequest schema allRels (head path) req reqBody --TODO! is head safe? rangeStatus :: Int -> Int -> Maybe Int -> Status rangeStatus _ _ Nothing = status200 @@ -390,7 +381,12 @@ parseRequest schema allRels rootTableName httpRequest reqBody = allFilters = whereFilters qParams mutateFilters = filter (not . ( '.' `elem` ) . fst) allFilters -- update/delete filters can be only on the root table cond = first formatParserError $ map snd <$> mapM pRequestFilter mutateFilters - selectApiRequest = augumentRequestWithJoin schema allRels + fakeSourceRelations = mapMaybe (toSourceRelation rootTableName) allRels + rels = case method of + "POST" -> fakeSourceRelations ++ allRels + "PATCH" -> fakeSourceRelations ++ allRels + _ -> allRels + selectApiRequest = augumentRequestWithJoin schema rels =<< buildSelectApiRequest rootName sel filters (orderStr qParams) where sel = if method == "DELETE" diff --git a/src/PostgREST/Parsers.hs b/src/PostgREST/Parsers.hs index e4d56e1ba..7069a049c 100644 --- a/src/PostgREST/Parsers.hs +++ b/src/PostgREST/Parsers.hs @@ -34,7 +34,6 @@ pRequestFilter (k, v) = (,) <$> path <*> (Filter <$> fld <*> op <*> val) op = fst <$> opVal val = snd <$> opVal - ws :: Parser Text ws = cs <$> many (oneOf " \t") @@ -45,23 +44,21 @@ pTreePath :: Parser (Path,Field) pTreePath = do p <- pFieldName `sepBy1` pDelimiter jp <- optionMaybe ( string "->" >> pJsonPath) - let pp = map cs p - jpp = map cs <$> jp - return (init pp, (last pp, jpp)) + return (init p, (last p, jp)) pFieldForest :: Parser [Tree SelectItem] pFieldForest = pFieldTree `sepBy1` lexeme (char ',') pFieldTree :: Parser (Tree SelectItem) -pFieldTree = try (Node <$> pSelect <*> ( char '(' *> pFieldForest <* char ')')) - <|> Node <$> pSelect <*> pure [] +pFieldTree = try (Node <$> pSelect <*> between (char '(') (char ')') pFieldForest) + <|> Node <$> pSelect <*> pure [] pStar :: Parser Text pStar = cs <$> (string "*" *> pure ("*"::String)) pFieldName :: Parser Text pFieldName = cs <$> (many1 (letter <|> digit <|> oneOf "_") - "field name (* or [a..z0..9_])") + "field name (* or [a..z0..9_])") pJsonPathDelimiter :: Parser Text pJsonPathDelimiter = cs <$> (try (string "->>") <|> string "->") From 14e806759db28bd6e115dc5dace89966ae18fb04 Mon Sep 17 00:00:00 2001 From: Ruslan Talpa Date: Tue, 3 Nov 2015 09:31:56 +0200 Subject: [PATCH 2/5] Revert to the old way of displaying the list of tables --- src/PostgREST/App.hs | 15 +----- src/PostgREST/Main.hs | 8 ++-- src/PostgREST/PgStructure.hs | 89 +++++++++++++++++++++++------------- src/PostgREST/Types.hs | 4 +- test/SpecHelper.hs | 8 ++-- 5 files changed, 66 insertions(+), 58 deletions(-) diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index 11767b2ab..2f1bf4b67 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -57,11 +57,7 @@ import Prelude app :: DbStructure -> AppConfig -> BL.ByteString -> Request -> H.Tx P.Postgres s Response app dbstructure conf reqBody req = case (path, verb) of - -- ([table], v) -> - -- case request of - -- Left e -> return $ responseLBS status400 [jsonH] $ cs e - -- Right (selectQuery, mutateQuery, isSingle) -> - + ([table], "GET") -> if range == Just emptyRange then return $ responseLBS status416 [] "HTTP Range error" @@ -151,8 +147,7 @@ app dbstructure conf reqBody req = -- select * from public.proc(a := "foo"::undefined) where whereT limit limitT ([], _) -> do - Identity (dbrole :: Text) <- H.singleEx $ [H.stmt|SELECT current_user|] - let body = encode $ filter (filterTableAcl dbrole) $ filter ((cs schema==).tableSchema) allTabs + body <- encode <$> tables (cs schema) return $ responseLBS status200 [jsonH] $ cs body ([table], "OPTIONS") -> do @@ -165,20 +160,14 @@ app dbstructure conf reqBody req = return $ responseLBS status404 [] "" where - allTabs = tables dbstructure allRels = relations dbstructure allCols = columns dbstructure allPrKeys = primaryKeys dbstructure filterCol sc table (Column{colSchema=s, colTable=t}) = s==sc && table==t filterCol _ _ _ = False filterPk sc table pk = sc == pkSchema pk && table == pkTable pk - - filterTableAcl :: Text -> Table -> Bool - filterTableAcl r (Table{tableAcl=a}) = r `elem` a path = pathInfo req verb = requestMethod req - --qq = queryString req - --qualify = QualifiedIdentifier schema hdrs = requestHeaders req lookupHeader = flip lookup hdrs hasPrefer val = any (\(h,v) -> h == "Prefer" && v == val) hdrs diff --git a/src/PostgREST/Main.hs b/src/PostgREST/Main.hs index af6f395fb..7dc0540e0 100644 --- a/src/PostgREST/Main.hs +++ b/src/PostgREST/Main.hs @@ -81,19 +81,17 @@ main = do let txSettings = Just (H.ReadCommitted, Just True) metadata <- H.session pool $ H.tx txSettings $ do - tabs <- allTables rels <- allRelations cols <- allColumns rels keys <- allPrimaryKeys - return (tabs, rels, cols, keys) + return (rels, cols, keys) dbstructure <- either hasqlError - (\(tabs, rels, cols, keys) -> + (\(rels, cols, keys) -> return DbStructure { - tables=tabs - , columns=cols + columns=cols , relations=rels , primaryKeys=keys } diff --git a/src/PostgREST/PgStructure.hs b/src/PostgREST/PgStructure.hs index d7e1710db..b9db312ae 100644 --- a/src/PostgREST/PgStructure.hs +++ b/src/PostgREST/PgStructure.hs @@ -48,11 +48,8 @@ doesProcReturnJWT = doesProc [H.stmt| AND pg_catalog.pg_get_function_result(p.oid) = 'jwt_claims' |] -tableFromRow :: (Text, Text, Bool, Maybe Text) -> Table -tableFromRow (s, n, i, a) = Table s n i (parseAcl a) - where - parseAcl :: Maybe Text -> [Text] - parseAcl str = fromMaybe [] $ split (==',') <$> str +tableFromRow :: (Text, Text, Bool) -> Table +tableFromRow (s, n, i) = Table s n i columnFromRow :: (Text, Text, Text, Int, Bool, Text, @@ -77,34 +74,62 @@ pkFromRow (s, t, n) = PrimaryKey s t n addParentRelation :: Relation -> [Relation] -> [Relation] addParentRelation rel@(Relation s t c ft fc _ _ _ _) rels = Relation s ft fc t c Parent Nothing Nothing Nothing:rel:rels -allTables :: H.Tx P.Postgres s [Table] -allTables = do - rows <- H.listEx $ [H.stmt| - SELECT - n.nspname AS table_schema, - c.relname AS table_name, - c.relkind = 'r' OR (c.relkind IN ('v','f')) - AND (pg_relation_is_updatable(c.oid::regclass, FALSE) & 8) = 8 - OR (EXISTS - ( SELECT 1 - FROM pg_trigger - WHERE pg_trigger.tgrelid = c.oid - AND (pg_trigger.tgtype::integer & 69) = 69) ) AS insertable, - array_to_string(array_agg(r.rolname), ',') AS acl - FROM pg_class c - CROSS JOIN pg_roles r - JOIN pg_namespace n ON n.oid = c.relnamespace - WHERE c.relkind IN ('v','r','m') - AND n.nspname NOT IN ('pg_catalog', 'information_schema') - AND ( - pg_has_role(r.rolname, c.relowner, 'USAGE'::text) OR - has_table_privilege(r.rolname, c.oid, 'SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER'::text) OR - has_any_column_privilege(r.rolname, c.oid, 'SELECT, INSERT, UPDATE, REFERENCES'::text) ) +-- allTables :: H.Tx P.Postgres s [Table] +-- allTables = do +-- rows <- H.listEx $ [H.stmt| +-- SELECT +-- n.nspname AS table_schema, +-- c.relname AS table_name, +-- c.relkind = 'r' OR (c.relkind IN ('v','f')) +-- AND (pg_relation_is_updatable(c.oid::regclass, FALSE) & 8) = 8 +-- OR (EXISTS +-- ( SELECT 1 +-- FROM pg_trigger +-- WHERE pg_trigger.tgrelid = c.oid +-- AND (pg_trigger.tgtype::integer & 69) = 69) ) AS insertable, +-- array_to_string(array_agg(r.rolname), ',') AS acl +-- FROM pg_class c +-- CROSS JOIN pg_roles r +-- JOIN pg_namespace n ON n.oid = c.relnamespace +-- WHERE c.relkind IN ('v','r','m') +-- AND n.nspname NOT IN ('pg_catalog', 'information_schema') +-- AND ( +-- pg_has_role(r.rolname, c.relowner, 'USAGE'::text) OR +-- has_table_privilege(r.rolname, c.oid, 'SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER'::text) OR +-- has_any_column_privilege(r.rolname, c.oid, 'SELECT, INSERT, UPDATE, REFERENCES'::text) ) +-- +-- GROUP BY table_schema, table_name, insertable +-- ORDER BY table_schema, table_name +-- |] +-- return $ map tableFromRow rows - GROUP BY table_schema, table_name, insertable - ORDER BY table_schema, table_name - |] - return $ map tableFromRow rows +tables :: Text -> H.Tx P.Postgres s [Table] +tables schema = do + rows <- H.listEx $ + [H.stmt| + select + n.nspname as table_schema, + relname as table_name, + c.relkind = 'r' or (c.relkind IN ('v', 'f')) and (pg_relation_is_updatable(c.oid::regclass, false) & 8) = 8 + or (exists ( + select 1 + from pg_trigger + where pg_trigger.tgrelid = c.oid and (pg_trigger.tgtype::integer & 69) = 69) + ) as insertable + from + pg_class c + join pg_namespace n on n.oid = c.relnamespace + where + c.relkind in ('v', 'r', 'm') + and n.nspname = ? + and ( + pg_has_role(c.relowner, 'USAGE'::text) + or has_table_privilege(c.oid, 'SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER'::text) + or has_any_column_privilege(c.oid, 'SELECT, INSERT, UPDATE, REFERENCES'::text) + ) + order by relname + |] schema + return $ map tableFromRow rows allRelations :: H.Tx P.Postgres s [Relation] allRelations = do diff --git a/src/PostgREST/Types.hs b/src/PostgREST/Types.hs index 8eeac7604..691efad99 100644 --- a/src/PostgREST/Types.hs +++ b/src/PostgREST/Types.hs @@ -6,8 +6,7 @@ import Data.Aeson import Data.Map data DbStructure = DbStructure { - tables :: [Table] -, columns :: [Column] + columns :: [Column] , relations :: [Relation] , primaryKeys :: [PrimaryKey] } @@ -17,7 +16,6 @@ data Table = Table { tableSchema :: Text , tableName :: Text , tableInsertable :: Bool -, tableAcl :: [Text] } deriving (Show) data ForeignKey = ForeignKey { diff --git a/test/SpecHelper.hs b/test/SpecHelper.hs index 1a0dae557..6dd276057 100644 --- a/test/SpecHelper.hs +++ b/test/SpecHelper.hs @@ -56,18 +56,16 @@ withApp perform = do let txSettings = Just (H.ReadCommitted, Just True) metadata <- H.session pool $ H.tx txSettings $ do - tabs <- allTables rels <- allRelations cols <- allColumns rels keys <- allPrimaryKeys - return (tabs, rels, cols, keys) + return (rels, cols, keys) dbstructure <- case metadata of Left e -> fail $ show e - Right (tabs, rels, cols, keys) -> + Right (rels, cols, keys) -> return DbStructure { - tables=tabs - , columns=cols + columns=cols , relations=rels , primaryKeys=keys } From 28b7b80bbb29f5fa4c7058f1f823a80fd806d47d Mon Sep 17 00:00:00 2001 From: Ruslan Talpa Date: Wed, 4 Nov 2015 16:22:13 +0200 Subject: [PATCH 3/5] remove PUT from cors --- src/PostgREST/Config.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PostgREST/Config.hs b/src/PostgREST/Config.hs index efb090fa5..54d9dcb0e 100644 --- a/src/PostgREST/Config.hs +++ b/src/PostgREST/Config.hs @@ -56,7 +56,7 @@ argParser = AppConfig defaultCorsPolicy :: CorsResourcePolicy defaultCorsPolicy = CorsResourcePolicy Nothing - ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"] ["Authorization"] Nothing + ["GET", "POST", "PATCH", "DELETE", "OPTIONS"] ["Authorization"] Nothing (Just $ 60*60*24) False False True -- | CORS policy to be used in by Wai Cors middleware From 37b1d7d69279dfcc293ad78bba563553f7f9f0f6 Mon Sep 17 00:00:00 2001 From: Ruslan Talpa Date: Wed, 4 Nov 2015 16:23:02 +0200 Subject: [PATCH 4/5] fix debian scripts to match current parameters --- debian/postgrest.default | 19 ++++++++++---- debian/postgrest.init.d | 55 +++++++++++++++++++++++++++------------- 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/debian/postgrest.default b/debian/postgrest.default index 33dab96cc..fe0141969 100644 --- a/debian/postgrest.default +++ b/debian/postgrest.default @@ -7,17 +7,26 @@ # database host #POSTGREST_DBHOST=localhost +# database host +#POSTGREST_DBPORT=5432 + # database to use -#POSTGREST_DBNAME= +#POSTGREST_DBNAME=app # database user -#POSTGREST_DBUSER=postgres +#POSTGREST_DBUSER=authenticator # database password #POSTGREST_DBPASS= # database pool -#POSTGREST_DBPOOL=10 +#POSTGREST_POOL=10 -# additional options -#POSTGREST_OPTS= +# jwt secret +#POSTGREST_JWT_SECRET=secret + +# default schema +#POSTGREST_SCHEMA=public + +# secure (use 1 to enable, empty string to disable) +#POSTGREST_SECURE= diff --git a/debian/postgrest.init.d b/debian/postgrest.init.d index 2227d8c07..a9091740e 100755 --- a/debian/postgrest.init.d +++ b/debian/postgrest.init.d @@ -13,31 +13,52 @@ if test -f /etc/default/postgrest; then . /etc/default/postgrest fi POSTGREST=/usr/local/bin/postgrest +CONNECTION_STRING="postgres://" +POSTGREST_OPTS="" POSTGREST_USER=${POSTGREST_USER:-postgrest} -POSTGREST_DBNAME=${POSTGREST_DBNAME:-postgres} -POSTGREST_DBUSER=${POSTGREST_DBUSER:-postgres} -if [ -n "$POSTGREST_DBHOST" ]; then - POSTGREST_OPTS="$POSTGREST_OPTS --db-host $POSTGREST_DBHOST" -fi -if [ -n "$POSTGREST_DBNAME" ]; then - POSTGREST_OPTS="$POSTGREST_OPTS --db-name $POSTGREST_DBNAME" -fi -if [ -n "$POSTGREST_DBUSER" ]; then - POSTGREST_OPTS="$POSTGREST_OPTS --db-user $POSTGREST_DBUSER" - POSTGREST_OPTS="$POSTGREST_OPTS --anonymous $POSTGREST_DBUSER" -fi +POSTGREST_PORT=${POSTGREST_PORT:-3000} +POSTGREST_DBUSER=${POSTGREST_DBUSER:-authenticator} +#POSTGREST_DBPASS=${POSTGREST_DBPASS:-authenticator} +POSTGREST_DBHOST=${POSTGREST_DBHOST:-localhost} +POSTGREST_DBPORT=${POSTGREST_DBPORT:-5432} +POSTGREST_DBNAME=${POSTGREST_DBNAME:-app} +POSTGREST_DBPOOL=${POSTGREST_DBPOOL:-10} +POSTGREST_ANON=${POSTGREST_ANON:-anonymous} +POSTGREST_JWT_SECRET=${POSTGREST_JWT_SECRET:-secret} +POSTGREST_SCHEMA=${POSTGREST_SCHEMA:-public} + +CONNECTION_STRING="$CONNECTION_STRING$POSTGREST_DBUSER" if [ -n "$POSTGREST_DBPASS" ]; then - POSTGREST_OPTS="$POSTGREST_OPTS --db-pass $POSTGREST_DBPASS" + CONNECTION_STRING="$CONNECTION_STRING:$POSTGREST_DBPASS" fi -if [ -n "$POSTGREST_DBPOOL" ]; then - POSTGREST_OPTS="$POSTGREST_OPTS --db-pool $POSTGREST_DBPOOL" +CONNECTION_STRING="$CONNECTION_STRING@$POSTGREST_DBHOST:$POSTGREST_DBPORT/$POSTGREST_DBNAME" + +if [ -n "$POSTGREST_PORT" ]; then + POSTGREST_OPTS="$POSTGREST_OPTS --port $POSTGREST_PORT" fi -POSTGREST_OPTS="$POSTGREST_OPTS --schema public" + +if [ -n "$POSTGREST_POOL" ]; then + POSTGREST_OPTS="$POSTGREST_OPTS --pool $POSTGREST_POOL" +fi +if [ -n "$POSTGREST_JWT_SECRET" ]; then + #export POSTGREST_JWT_SECRET="$POSTGREST_JWT_SECRET" + POSTGREST_OPTS="$POSTGREST_OPTS --jwt-secret $POSTGREST_JWT_SECRET" +fi +if [ -n "$POSTGREST_SCHEMA" ]; then + POSTGREST_OPTS="$POSTGREST_OPTS --schema $POSTGREST_SCHEMA" +fi +if [ -n "$POSTGREST_ANON" ]; then + POSTGREST_OPTS="$POSTGREST_OPTS --anonymous $POSTGREST_ANON" +fi + +#export CONNECTION_STRING="$CONNECTION_STRING" + +START_PARAMS="$CONNECTION_STRING $POSTGREST_OPTS" start() { log_daemon_msg "Starting PostgreSQL REST API daemon" "postgrest" || true - if start-stop-daemon --start --quiet --oknodo --chuid ${POSTGREST_USER} --startas /usr/local/bin/postgrest-wrapper --exec $POSTGREST -- $POSTGREST_OPTS; then + if start-stop-daemon --start --quiet --oknodo --chuid ${POSTGREST_USER} --startas /usr/local/bin/postgrest-wrapper --exec $POSTGREST -- $START_PARAMS; then log_end_msg 0 || true else log_end_msg 1 || true From cfad68f5cb05f09708ae6a1cafcc906f609838ed Mon Sep 17 00:00:00 2001 From: Ruslan Talpa Date: Wed, 4 Nov 2015 16:28:22 +0200 Subject: [PATCH 5/5] Fix test --- test/Feature/CorsSpec.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Feature/CorsSpec.hs b/test/Feature/CorsSpec.hs index fa005dc22..35ff69982 100644 --- a/test/Feature/CorsSpec.hs +++ b/test/Feature/CorsSpec.hs @@ -41,7 +41,7 @@ spec = around withApp $ describe "CORS" $ do "true" respHeaders `shouldSatisfy` matchHeader "Access-Control-Allow-Methods" - "GET, POST, PUT, PATCH, DELETE, OPTIONS, HEAD" + "GET, POST, PATCH, DELETE, OPTIONS, HEAD" respHeaders `shouldSatisfy` matchHeader "Access-Control-Allow-Headers" "Authentication, Foo, Bar, Accept, Accept-Language, Content-Language"