From 3bee6b1e27b704c6cbd802a6365d5d567cc33043 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Tue, 5 Jul 2022 10:49:56 +0200 Subject: [PATCH] fix: Fix regression in openapi output with mode follow-privileges This was introduced in d5b92a433a7b9f9d6bfd31be979ea96f7f7ef3a9. Before this change, the OpenApi output would have 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 #2356 Signed-off-by: Wolfgang Walther --- CHANGELOG.md | 1 + src/PostgREST/Query.hs | 10 +++--- src/PostgREST/SchemaCache.hs | 46 ++++++++++++++++-------- src/PostgREST/SchemaCache/Identifiers.hs | 4 +++ test/spec/Feature/OpenApi/OpenApiSpec.hs | 34 +++++++++++++++++- test/spec/fixtures/schema.sql | 6 ++++ 6 files changed, 81 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa672f0e9a..ecc181feb5 100644 --- 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 index 2e4b1dabf0..022a3af3d7 100644 --- 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 index e67f1d6e22..3b576fe792 100644 --- 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 index 1992b42b4e..a1eee39ac5 100644 --- 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 index 0336da3675..04f9198ee6 100644 --- 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.", + "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`.", + "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 index 1a47915c7b..87350b22e1 100644 --- 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