fix: Fix regression in openapi output with mode follow-privileges

This was introduced in d5b92a4. Before this change, the OpenApi output would have <pk/> annotations for views, too. After this change, they got lost for mode follow-privileges, because the pks are refined in haskell code, but the request only fetches all the tables again, but not the view dependencies. This fix changes follow-privileges to only fetch a list of accessible tables, which is then used to filter the tables in the schema cache. Fixes PostgREST#2356 Signed-off-by: Wolfgang Walther <[email protected]>
wolfgangwalther · Oct 26, 2022 · 3bee6b1 · 3bee6b1
1 parent 6fe1617
commit 3bee6b1
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
  - #2455, Fix embedding the same table multiple times - @steve-chavez
  - #2518, Fix a regression when embedding views where base tables have a different column order for FK columns - @wolfgangwalther
  - #2458, Fix a regression with the location header when inserting into views with PKs from multiple tables - @wolfgangwalther
+ - #2356, Fix a regression in openapi output with mode follow-privileges - @wolfgangwalther
 
 ### Deprecated
 

diff --git a/src/PostgREST/Query.hs b/src/PostgREST/Query.hs
@@ -18,6 +18,7 @@ import qualified Data.Aeson.Key                    as K
 import qualified Data.Aeson.KeyMap                 as KM
 import qualified Data.ByteString.Lazy.Char8        as LBS
 import qualified Data.HashMap.Strict               as HM
+import qualified Data.Set                          as S
 import qualified Data.Text.Encoding                as T
 import qualified Hasql.Decoders                    as HD
 import qualified Hasql.DynamicStatements.Snippet   as SQL (Snippet)
@@ -177,11 +178,12 @@ invokeQuery proc CallReadPlan{crReadPlan, crCallPlan} apiReq@ApiRequest{..} conf
 openApiQuery :: SchemaCache -> PgVersion -> AppConfig -> Schema -> DbHandler (Maybe (TablesMap, ProcsMap, Maybe Text))
 openApiQuery sCache pgVer AppConfig{..} tSchema =
   lift $ case configOpenApiMode of
-    OAFollowPriv ->
+    OAFollowPriv -> do
+      tableAccess <- SQL.statement [tSchema] (SchemaCache.accessibleTables pgVer configDbPreparedStatements)
       Just <$> ((,,)
-         <$> SQL.statement [tSchema] (SchemaCache.accessibleTables pgVer configDbPreparedStatements)
-         <*> SQL.statement tSchema (SchemaCache.accessibleProcs pgVer configDbPreparedStatements)
-         <*> SQL.statement tSchema (SchemaCache.schemaDescription configDbPreparedStatements))
+            (HM.filterWithKey (\qi _ -> S.member qi tableAccess) $ SchemaCache.dbTables sCache)
+        <$> SQL.statement tSchema (SchemaCache.accessibleProcs pgVer configDbPreparedStatements)
+        <*> SQL.statement tSchema (SchemaCache.schemaDescription configDbPreparedStatements))
     OAIgnorePriv ->
       Just <$> ((,,)
             (HM.filterWithKey (\(QualifiedIdentifier sch _) _ ->  sch == tSchema) $ SchemaCache.dbTables sCache)

diff --git a/src/PostgREST/SchemaCache.hs b/src/PostgREST/SchemaCache.hs
@@ -40,7 +40,7 @@ import Text.InterpolatedString.Perl6 (q)
 import PostgREST.Config.Database          (pgVersionStatement)
 import PostgREST.Config.PgVersion         (PgVersion, pgVersion100,
                                            pgVersion110)
-import PostgREST.SchemaCache.Identifiers  (FieldName,
+import PostgREST.SchemaCache.Identifiers  (AccessSet, FieldName,
                                            QualifiedIdentifier (..),
                                            Schema)
 import PostgREST.SchemaCache.Proc         (PgType (..),
@@ -136,6 +136,14 @@ removeInternal schemas dbStruct =
       M2M Junction{junTable} -> qiSchema junTable `notElem` schemas
       _                      -> False
 
+decodeAccessibleIdentifiers :: HD.Result AccessSet
+decodeAccessibleIdentifiers =
+ S.fromList <$> HD.rowList row
+ where
+  row = QualifiedIdentifier
+    <$> column HD.text
+    <*> column HD.text
+
 decodeTables :: HD.Result TablesMap
 decodeTables =
  HM.fromList . map (\tbl@Table{tableSchema, tableName} -> (QualifiedIdentifier tableSchema tableName, tbl)) <$> HD.rowList tblRow
@@ -331,11 +339,27 @@ schemaDescription =
       where
         n.nspname = $1 |]
 
-accessibleTables :: PgVersion -> Bool -> SQL.Statement [Schema] TablesMap
+accessibleTables :: PgVersion -> Bool -> SQL.Statement [Schema] AccessSet
 accessibleTables pgVer =
-  SQL.Statement sql (arrayParam HE.text) decodeTables
+  SQL.Statement sql (arrayParam HE.text) decodeAccessibleIdentifiers
  where
-  sql = tablesSqlQuery False pgVer
+  sql = [q|
+    SELECT
+      n.nspname AS table_schema,
+      c.relname AS table_name
+    FROM pg_class c
+    JOIN pg_namespace n ON n.oid = c.relnamespace
+    WHERE c.relkind IN ('v','r','m','f','p')
+    AND n.nspname NOT IN ('pg_catalog', 'information_schema')
+    AND n.nspname = ANY($1)
+    AND (
+      pg_has_role(c.relowner, 'USAGE')
+      or has_table_privilege(c.oid, 'SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER')
+      or has_any_column_privilege(c.oid, 'SELECT, INSERT, UPDATE, REFERENCES')
+    ) |] <>
+    relIsPartition <>
+    "ORDER BY table_schema, table_name"
+  relIsPartition = if pgVer >= pgVersion100 then " AND not c.relispartition " else mempty
 
 {-
 Adds M2O and O2O relationships for views to tables, tables to views, and views to views. The example below is taken from the test fixtures, but the views names/colnames were modified.
@@ -435,11 +459,11 @@ allTables :: PgVersion -> Bool -> SQL.Statement [Schema] TablesMap
 allTables pgVer =
   SQL.Statement sql (arrayParam HE.text) decodeTables
   where
-    sql = tablesSqlQuery True pgVer
+    sql = tablesSqlQuery pgVer
 
 -- | Gets tables with their PK cols
-tablesSqlQuery :: Bool -> PgVersion -> SqlQuery
-tablesSqlQuery getAll pgVer =
+tablesSqlQuery :: PgVersion -> SqlQuery
+tablesSqlQuery pgVer =
   -- the tbl_constraints/key_col_usage CTEs are based on the standard "information_schema.table_constraints"/"information_schema.key_column_usage" views,
   -- we cannot use those directly as they include the following privilege filter:
   -- (pg_has_role(ss.relowner, 'USAGE'::text) OR has_column_privilege(ss.roid, a.attnum, 'SELECT, INSERT, UPDATE, REFERENCES'::text));
@@ -630,16 +654,8 @@ tablesSqlQuery getAll pgVer =
   WHERE c.relkind IN ('v','r','m','f','p')
   AND n.nspname NOT IN ('pg_catalog', 'information_schema') |] <>
   relIsPartition <>
-  fltTables <>
   "ORDER BY table_schema, table_name"
   where
-    fltTables = if getAll then mempty else [q|
-      AND n.nspname = ANY($1)
-      AND (
-        pg_has_role(c.relowner, 'USAGE')
-        or has_table_privilege(c.oid, 'SELECT, INSERT, UPDATE, DELETE, TRUNCATE, REFERENCES, TRIGGER')
-        or has_any_column_privilege(c.oid, 'SELECT, INSERT, UPDATE, REFERENCES')
-      )|]
     relIsPartition = if pgVer >= pgVersion100 then " AND not c.relispartition " else mempty
 
 

diff --git a/src/PostgREST/SchemaCache/Identifiers.hs b/src/PostgREST/SchemaCache/Identifiers.hs
@@ -6,11 +6,13 @@ module PostgREST.SchemaCache.Identifiers
   , Schema
   , TableName
   , FieldName
+  , AccessSet
   , dumpQi
   , toQi
   ) where
 
 import qualified Data.Aeson as JSON
+import qualified Data.Set   as S
 import qualified Data.Text  as T
 
 import Protolude
@@ -40,3 +42,5 @@ toQi txt = case T.drop 1 <$> T.breakOn "." txt of
 type Schema = Text
 type TableName = Text
 type FieldName = Text
+
+type AccessSet = S.Set QualifiedIdentifier
diff --git a/test/spec/Feature/OpenApi/OpenApiSpec.hs b/test/spec/Feature/OpenApi/OpenApiSpec.hs
@@ -157,6 +157,38 @@ spec actualPgVersion = describe "OpenAPI" $ do
             }
           |]
 
+    it "includes definitions to views" $ do
+      r <- simpleBody <$> get "/"
+
+      let def = r ^? key "definitions" . key "child_entities_view"
+
+      liftIO $
+
+        def `shouldBe` Just
+          [aesonQQ|
+            {
+              "type": "object",
+              "description": "child_entities_view comment",
+              "properties": {
+                "id": {
+                  "description": "child_entities_view id comment\n\nNote:\nThis is a Primary Key.<pk/>",
+                  "format": "integer",
+                  "type": "integer"
+                },
+                "name": {
+                  "description": "child_entities_view name comment. Can be longer than sixty-three characters long",
+                  "format": "text",
+                  "type": "string"
+                },
+                "parent_id": {
+                  "description": "Note:\nThis is a Foreign Key to `entities.id`.<fk table='entities' column='id'/>",
+                  "format": "integer",
+                  "type": "integer"
+                }
+              }
+            }
+          |]
+
     it "doesn't include privileged table for anonymous" $ do
       r <- simpleBody <$> get "/"
       let tablePath = r ^? key "paths" . key "/authors_only"
@@ -523,7 +555,7 @@ spec actualPgVersion = describe "OpenAPI" $ do
 
         types `shouldBe` Just [aesonQQ|
             {
-              "format": "enum_menagerie_type",
+              "format": "test.enum_menagerie_type",
               "type": "string",
               "enum": [
                 "foo",

diff --git a/test/spec/fixtures/schema.sql b/test/spec/fixtures/schema.sql
@@ -1153,6 +1153,8 @@ create table child_entities (
   parent_id integer references entities(id)
 );
 
+create view child_entities_view as table child_entities;
+
 create table grandchild_entities (
   id integer primary key,
   name text,
@@ -1176,6 +1178,10 @@ comment on table child_entities is 'child_entities comment';
 comment on column child_entities.id is 'child_entities id comment';
 comment on column child_entities.name is 'child_entities name comment. Can be longer than sixty-three characters long';
 
+comment on view child_entities_view is 'child_entities_view comment';
+comment on column child_entities_view.id is 'child_entities_view id comment';
+comment on column child_entities_view.name is 'child_entities_view name comment. Can be longer than sixty-three characters long';
+
 comment on table grandchild_entities is
 $$grandchild_entities summary