From 925b668161a7f69fedf02158965966dddc07aec2 Mon Sep 17 00:00:00 2001 From: Rikin Kachhia <54616969+rikinsk@users.noreply.github.com> Date: Tue, 4 Aug 2020 16:30:38 +0530 Subject: [PATCH 1/6] console: separate timeout statement from user statement in raw sql page (#5516) --- .../components/Services/Data/RawSQL/Actions.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/console/src/components/Services/Data/RawSQL/Actions.js b/console/src/components/Services/Data/RawSQL/Actions.js index 436d2ee98de..8a7a13265c1 100644 --- a/console/src/components/Services/Data/RawSQL/Actions.js +++ b/console/src/components/Services/Data/RawSQL/Actions.js @@ -40,7 +40,7 @@ const executeSQL = (isMigration, migrationName, statementTimeout) => ( dispatch({ type: MAKING_REQUEST }); dispatch(showSuccessNotification('Executing the Query...')); - const { isTableTrackChecked, isCascadeChecked } = getState().rawSQL; + const { isTableTrackChecked, isCascadeChecked, sql } = getState().rawSQL; const { migrationMode, readOnlyMode } = getState().main; const migrateUrl = returnMigrateUrl(migrationMode); @@ -49,13 +49,17 @@ const executeSQL = (isMigration, migrationName, statementTimeout) => ( const schemaChangesUp = []; - const sql = - statementTimeout && !isMigration - ? `${getStatementTimeoutSql(statementTimeout)} ${getState().rawSQL.sql}` - : getState().rawSQL.sql; + if (statementTimeout && !isMigration) { + schemaChangesUp.push( + getRunSqlQuery( + getStatementTimeoutSql(statementTimeout), + false, + readOnlyMode + ) + ); + } schemaChangesUp.push(getRunSqlQuery(sql, isCascadeChecked, readOnlyMode)); - // check if track view enabled if (isTableTrackChecked) { const objects = parseCreateSQL(sql); From eb80d97b0af8d526593d5eaa92048c1fd8f591a7 Mon Sep 17 00:00:00 2001 From: Tirumarai Selvan Date: Wed, 5 Aug 2020 15:53:14 +0530 Subject: [PATCH 2/6] change log kind from db_migrate to catalog_migrate (#5531) --- CHANGELOG.md | 3 ++- server/src-lib/Hasura/App.hs | 2 +- server/src-lib/Hasura/Server/Migrate.hs | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96884e474b2..2507626702d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ - server: bugfix to allow HASURA_GRAPHQL_QUERY_PLAN_CACHE_SIZE of 0 (#5363) - server: support only a bounded plan cache, with a default size of 4000 (closes #5363) - server: add logs for action handlers -- server: add request/response sizes in event triggers (and scheduled trigger) logs +- server: add request/response sizes in event triggers (and scheduled trigger) logs (#5463) +- server: change startup log kind `db_migrate` to `catalog_migrate` (#5531) - console: handle nested fragments in allowed queries (close #5137) (#5252) - console: update sidebar icons for different action and trigger types (#5445) - console: make add column UX consistent with others (#5486) diff --git a/server/src-lib/Hasura/App.hs b/server/src-lib/Hasura/App.hs index 18f537801ed..50cbc0775d5 100644 --- a/server/src-lib/Hasura/App.hs +++ b/server/src-lib/Hasura/App.hs @@ -242,7 +242,7 @@ migrateCatalogSchema env logger pool httpManager sqlGenCtx = do initialiseResult `onLeft` \err -> do unLogger logger StartupLog { slLogLevel = LevelError - , slKind = "db_migrate" + , slKind = "catalog_migrate" , slInfo = A.toJSON err } liftIO (printErrExit DatabaseMigrationError (BLC.unpack $ A.encode err)) diff --git a/server/src-lib/Hasura/Server/Migrate.hs b/server/src-lib/Hasura/Server/Migrate.hs index f28fbb4e0f7..32fae9d3168 100644 --- a/server/src-lib/Hasura/Server/Migrate.hs +++ b/server/src-lib/Hasura/Server/Migrate.hs @@ -64,7 +64,7 @@ data MigrationResult instance ToEngineLog MigrationResult Hasura where toEngineLog result = toEngineLog $ StartupLog { slLogLevel = LevelInfo - , slKind = "db_migrate" + , slKind = "catalog_migrate" , slInfo = A.toJSON $ case result of MRNothingToDo -> "Already at the latest catalog version (" <> latestCatalogVersionString From 01a4ceeca9cc8cca27481395517a90a38d0622d1 Mon Sep 17 00:00:00 2001 From: Phil Freeman Date: Wed, 5 Aug 2020 03:24:43 -0700 Subject: [PATCH 3/6] Show method and complete URI in traced HTTP calls (#5525) Co-authored-by: Vamshi Surabhi <0x777@users.noreply.github.com> --- server/src-lib/Hasura/Tracing.hs | 34 ++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/server/src-lib/Hasura/Tracing.hs b/server/src-lib/Hasura/Tracing.hs index 9f1fea6074c..1e25bbae0dd 100644 --- a/server/src-lib/Hasura/Tracing.hs +++ b/server/src-lib/Hasura/Tracing.hs @@ -26,6 +26,7 @@ import Control.Monad.Trans.Control import Control.Monad.Morph import Control.Monad.Unique import Data.String (fromString) +import Network.URI (URI) import qualified Data.Aeson as J import qualified Data.Aeson.Lens as JL @@ -237,22 +238,25 @@ extractEventContext e = do -- | Perform HTTP request which supports Trace headers tracedHttpRequest :: MonadTrace m - => HTTP.Request + => HTTP.Request -- ^ http request that needs to be made -> (HTTP.Request -> m a) -- ^ a function that takes the traced request and executes it -> m a -tracedHttpRequest req f = trace (bsToTxt (HTTP.path req)) do - let reqBytes = case HTTP.requestBody req of - HTTP.RequestBodyBS bs -> Just (fromIntegral (BS.length bs)) - HTTP.RequestBodyLBS bs -> Just (BL.length bs) - HTTP.RequestBodyBuilder len _ -> Just len - HTTP.RequestBodyStream len _ -> Just len - _ -> Nothing - for_ reqBytes \b -> - attachMetadata [("request_body_bytes", fromString (show b))] - ctx <- currentContext - let req' = req { HTTP.requestHeaders = - injectHttpContext ctx <> HTTP.requestHeaders req - } - f req' +tracedHttpRequest req f = do + let method = bsToTxt (HTTP.method req) + uri = show @URI (HTTP.getUri req) + trace (method <> " " <> fromString uri) do + let reqBytes = case HTTP.requestBody req of + HTTP.RequestBodyBS bs -> Just (fromIntegral (BS.length bs)) + HTTP.RequestBodyLBS bs -> Just (BL.length bs) + HTTP.RequestBodyBuilder len _ -> Just len + HTTP.RequestBodyStream len _ -> Just len + _ -> Nothing + for_ reqBytes \b -> + attachMetadata [("request_body_bytes", fromString (show b))] + ctx <- currentContext + let req' = req { HTTP.requestHeaders = + injectHttpContext ctx <> HTTP.requestHeaders req + } + f req' From d4b4459c98e9d7bece73be0f31fa06530d85dad2 Mon Sep 17 00:00:00 2001 From: Varun Choudhary <68095256+Varun-Choudhary@users.noreply.github.com> Date: Wed, 5 Aug 2020 17:16:57 +0530 Subject: [PATCH 4/6] console: fix inconsistency in permissions builder (close #4119) (#5524) --- .../TablePermissions/PermissionBuilder/PermissionBuilder.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/console/src/components/Services/Data/TablePermissions/PermissionBuilder/PermissionBuilder.js b/console/src/components/Services/Data/TablePermissions/PermissionBuilder/PermissionBuilder.js index fef1b4a100d..8482890bf03 100644 --- a/console/src/components/Services/Data/TablePermissions/PermissionBuilder/PermissionBuilder.js +++ b/console/src/components/Services/Data/TablePermissions/PermissionBuilder/PermissionBuilder.js @@ -774,7 +774,7 @@ class PermissionBuilder extends React.Component { const _tableExp = [ { key: 'schema', value: schemaSelect }, - { key: 'table', value: tableSelect }, + { key: 'name', value: tableSelect }, ]; return ; From cfd7944849e6dc653758146bdc1cf7e817a41b01 Mon Sep 17 00:00:00 2001 From: Rakesh Emmadi <12475069+rakeshkky@users.noreply.github.com> Date: Wed, 5 Aug 2020 18:44:53 +0530 Subject: [PATCH 5/6] restrict env variables start with HASURA_GRAPHQL_ for headers configuration in actions, event triggers & remote schemas (#5519) * restrict env variables start with HASURA_GRAPHQL_ for headers definition in actions & event triggers * update CHANGELOG.md * Apply suggestions from code review Co-authored-by: Vamshi Surabhi <0x777@users.noreply.github.com> --- CHANGELOG.md | 11 +++++++++- server/src-lib/Hasura/RQL/DDL/Headers.hs | 7 +++++-- .../actions/metadata/create_with_headers.yaml | 21 +++++++++++++++++++ server/tests-py/test_actions.py | 3 +++ 4 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 server/tests-py/queries/actions/metadata/create_with_headers.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 2507626702d..5abb6e3689e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,19 @@ ## Next release +### Breaking change + +Headers from environment variables starting with `HASURA_GRAPHQL_` are not allowed +in event triggers, actions & remote schemas. + +If you do have such headers configured, then you must update the header configuration before upgrading. + ### Bug fixes and improvements (Add entries here in the order of: server, console, cli, docs, others) +- server: disallow headers from env variables starting with `HASURA_GRAPHQL_` in actions, event triggers & remote schemas (#5519) +**WARNING**: This might break certain deployments. See `Breaking change` section above. - server: bugfix to allow HASURA_GRAPHQL_QUERY_PLAN_CACHE_SIZE of 0 (#5363) - server: support only a bounded plan cache, with a default size of 4000 (closes #5363) - server: add logs for action handlers @@ -39,7 +48,7 @@ - server: have haskell runtime release blocks of memory back to the OS eagerly (related to #3388) - server: unlock locked scheduled events on graceful shutdown (#4928) - server: disable prepared statements for mutations as we end up with single-use objects which result in excessive memory consumption for mutation heavy workloads (#5255) -- server: include scheduled event metadata (`created_at`,`scheduled_time`,`id`, etc) along with the configured payload in the request body to the webhook. +- server: include scheduled event metadata (`created_at`,`scheduled_time`,`id`, etc) along with the configured payload in the request body to the webhook. **WARNING:** This is breaking for beta versions as the payload is now inside a key called `payload`. - console: allow configuring statement timeout on console RawSQL page (close #4998) (#5045) - console: support tracking partitioned tables (close #5071) (#5258) diff --git a/server/src-lib/Hasura/RQL/DDL/Headers.hs b/server/src-lib/Hasura/RQL/DDL/Headers.hs index 4bbf89a19d6..c2231c49678 100644 --- a/server/src-lib/Hasura/RQL/DDL/Headers.hs +++ b/server/src-lib/Hasura/RQL/DDL/Headers.hs @@ -8,8 +8,8 @@ import Hasura.RQL.Types.Error import Language.Haskell.TH.Syntax (Lift) import qualified Data.CaseInsensitive as CI -import qualified Data.Text as T import qualified Data.Environment as Env +import qualified Data.Text as T import qualified Network.HTTP.Types as HTTP @@ -35,7 +35,10 @@ instance FromJSON HeaderConf where case (value, valueFromEnv ) of (Nothing, Nothing) -> fail "expecting value or value_from_env keys" (Just val, Nothing) -> return $ HeaderConf name (HVValue val) - (Nothing, Just val) -> return $ HeaderConf name (HVEnv val) + (Nothing, Just val) -> do + when (T.isPrefixOf "HASURA_GRAPHQL_" val) $ + fail $ "env variables starting with \"HASURA_GRAPHQL_\" are not allowed in value_from_env: " <> T.unpack val + return $ HeaderConf name (HVEnv val) (Just _, Just _) -> fail "expecting only one of value or value_from_env keys" parseJSON _ = fail "expecting object for headers" diff --git a/server/tests-py/queries/actions/metadata/create_with_headers.yaml b/server/tests-py/queries/actions/metadata/create_with_headers.yaml new file mode 100644 index 00000000000..0a10a459546 --- /dev/null +++ b/server/tests-py/queries/actions/metadata/create_with_headers.yaml @@ -0,0 +1,21 @@ +description: Define an action with headers configuration +url: /v1/query +status: 400 +query: + type: create_action + args: + name: create_user_1 + definition: + kind: synchronous + arguments: + - name: name + type: String! + output_type: User! + handler: http://127.0.0.1:5593/create-user + headers: + - name: x-client-id + value_from_env: HASURA_GRAPHQL_CLIENT_NAME +response: + path: $.definition.headers[0] + error: 'env variables starting with "HASURA_GRAPHQL_" are not allowed in value_from_env: HASURA_GRAPHQL_CLIENT_NAME' + code: parse-failed diff --git a/server/tests-py/test_actions.py b/server/tests-py/test_actions.py index b4a7c5a57c1..849a532bcb1 100644 --- a/server/tests-py/test_actions.py +++ b/server/tests-py/test_actions.py @@ -455,6 +455,9 @@ class TestActionsMetadata: def test_recreate_permission(self, hge_ctx): check_query_f(hge_ctx, self.dir() + '/recreate_permission.yaml') + def test_create_with_headers(self, hge_ctx): + check_query_f(hge_ctx, self.dir() + '/create_with_headers.yaml') + # Test case for bug reported at https://github.com/hasura/graphql-engine/issues/5166 @pytest.mark.usefixtures('per_class_tests_db_state') class TestActionIntrospection: From 34288e1eb5f2c5dad9e6d1e05453dd52397dc970 Mon Sep 17 00:00:00 2001 From: Rakesh Emmadi <12475069+rakeshkky@users.noreply.github.com> Date: Wed, 5 Aug 2020 19:38:36 +0530 Subject: [PATCH 6/6] fix introspection query if any enum column present in primary key (fix #5200) (#5522) --- CHANGELOG.md | 1 + server/src-lib/Hasura/GraphQL/Schema.hs | 1 + .../enums/introspect_user_role.yaml | 48 +++++++++++++++++++ .../queries/graphql_query/enums/setup.yaml | 31 ++++++++++++ .../queries/graphql_query/enums/teardown.yaml | 2 + server/tests-py/test_graphql_queries.py | 3 ++ 6 files changed, 86 insertions(+) create mode 100644 server/tests-py/queries/graphql_query/enums/introspect_user_role.yaml diff --git a/CHANGELOG.md b/CHANGELOG.md index 5abb6e3689e..cbfc26797d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ If you do have such headers configured, then you must update the header configur (Add entries here in the order of: server, console, cli, docs, others) +- server: fix failing introspection query when an enum column is part of a primary key (fixes #5200) - server: disallow headers from env variables starting with `HASURA_GRAPHQL_` in actions, event triggers & remote schemas (#5519) **WARNING**: This might break certain deployments. See `Breaking change` section above. - server: bugfix to allow HASURA_GRAPHQL_QUERY_PLAN_CACHE_SIZE of 0 (#5363) diff --git a/server/src-lib/Hasura/GraphQL/Schema.hs b/server/src-lib/Hasura/GraphQL/Schema.hs index db4ad5945da..819bba7fa06 100644 --- a/server/src-lib/Hasura/GraphQL/Schema.hs +++ b/server/src-lib/Hasura/GraphQL/Schema.hs @@ -214,6 +214,7 @@ mkMutationTypesAndFieldsRole tn insPermM selFldsM updColsM delPermM pkeyCols con (selFldsM ^.. _Just.traverse._SFPGColumn) <> (insPermM ^. _Just._1) <> (updColsM ^. _Just) + <> (pkeyCols ^. _Just.pkColumns.to toList) allEnumReferences = allColumnInfos ^.. traverse.to pgiType._PGColumnEnumReference in flip map allEnumReferences $ \enumReference@(EnumReference referencedTableName _) -> let typeName = mkTableEnumType referencedTableName diff --git a/server/tests-py/queries/graphql_query/enums/introspect_user_role.yaml b/server/tests-py/queries/graphql_query/enums/introspect_user_role.yaml new file mode 100644 index 00000000000..78323d63bf3 --- /dev/null +++ b/server/tests-py/queries/graphql_query/enums/introspect_user_role.yaml @@ -0,0 +1,48 @@ +# https://github.com/hasura/graphql-engine/issues/5200 +description: Test introspecting enum types as user role +url: /v1/graphql +status: 200 +headers: + X-Hasura-Role: user +response: + data: + country: + kind: ENUM + name: country_enum + enumValues: + - name: India + description: Republic of India + - name: USA + description: United States of America + zones: + fields: + - name: code + type: + ofType: + name: String + - name: id + type: + ofType: + name: Int +query: + query: | + { + country: __type(name: "country_enum") { + name + kind + enumValues { + name + description + } + } + zones: __type(name: "zones") { + fields { + name + type { + ofType { + name + } + } + } + } + } diff --git a/server/tests-py/queries/graphql_query/enums/setup.yaml b/server/tests-py/queries/graphql_query/enums/setup.yaml index 310e836cd00..dd0b7ddd598 100644 --- a/server/tests-py/queries/graphql_query/enums/setup.yaml +++ b/server/tests-py/queries/graphql_query/enums/setup.yaml @@ -23,12 +23,34 @@ args: ('Alyssa', 'red'), ('Ben', 'blue'); + CREATE TABLE country + ( value text PRIMARY KEY + , comment text); + INSERT INTO country (value, comment) VALUES + ('India', 'Republic of India'), + ('USA', 'United States of America'); + + CREATE TABLE zones + ( id SERIAL + , code text NOT NULL + , country text NOT NULL REFERENCES country + , PRIMARY KEY (code, country) ); + INSERT INTO zones (code, country) VALUES + ('67432', 'USA'), + ('600036', 'India'); + - type: track_table args: table: colors is_enum: true - type: track_table args: users +- type: track_table + args: + table: country + is_enum: true +- type: track_table + args: zones # Anonymous users can query users, but not colors - type: create_select_permission @@ -38,3 +60,12 @@ args: permission: columns: [id, name, favorite_color] filter: {} + +# A user can query only code but not country +- type: create_select_permission + args: + table: zones + role: user + permission: + columns: [id, code] + filter: {} diff --git a/server/tests-py/queries/graphql_query/enums/teardown.yaml b/server/tests-py/queries/graphql_query/enums/teardown.yaml index e72c029f262..fb340d04b54 100644 --- a/server/tests-py/queries/graphql_query/enums/teardown.yaml +++ b/server/tests-py/queries/graphql_query/enums/teardown.yaml @@ -5,4 +5,6 @@ args: sql: | DROP TABLE users; DROP TABLE colors; + DROP TABLE zones; + DROP TABLE country; cascade: true diff --git a/server/tests-py/test_graphql_queries.py b/server/tests-py/test_graphql_queries.py index 8d026b0a914..5f434318b39 100644 --- a/server/tests-py/test_graphql_queries.py +++ b/server/tests-py/test_graphql_queries.py @@ -561,6 +561,9 @@ class TestGraphQLQueryEnums: def test_introspect(self, hge_ctx, transport): check_query_f(hge_ctx, self.dir() + '/introspect.yaml', transport) + def test_introspect_user_role(self, hge_ctx, transport): + check_query_f(hge_ctx, self.dir() + '/introspect_user_role.yaml', transport) + def test_select_enum_field(self, hge_ctx, transport): check_query_f(hge_ctx, self.dir() + '/select_enum_field.yaml', transport)