Server: Validate remote schema queries (#5938)

* [skip ci] use the args while making the fieldParser * modify the execution part of the remote queries * parse union queries deeply * add test for remote schema field validation * add tests for validating remote query arguments Co-authored-by: Auke Booij <auke@hasura.io> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
hasura · Oct 13, 2020 · 3ea611f · 3ea611f
1 parent 19b4f55
commit 3ea611f
Show file tree

Hide file tree

Showing 16 changed files with 458 additions and 108 deletions.
diff --git a/.circleci/server-upgrade-downgrade/run.sh b/.circleci/server-upgrade-downgrade/run.sh
@@ -172,6 +172,7 @@ get_server_upgrade_tests() {
                 --deselect test_graphql_mutations.py::TestGraphqlInsertPermission::test_backend_user_no_admin_secret_fail \
                 --deselect test_graphql_mutations.py::TestGraphqlMutationCustomSchema::test_update_article \
                 --deselect test_graphql_queries.py::TestGraphQLQueryEnums::test_introspect_user_role \
+                --deselect test_schema_stitching.py::TestRemoteSchemaQueriesOverWebsocket::test_remote_query_error \
 		"${args[@]}" 1>/dev/null 2>/dev/null
 	set +x
 	cat "$tmpfile"

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -78,6 +78,7 @@ This release contains the [PDV refactor (#4111)](https://github.com/hasura/graph
 - server: accept only non-negative integers for batch size and refetch interval (close #5653) (#5759)
 - server: limit the length of event trigger names (close #5786)
 **NOTE:** If you have event triggers with names greater than 42 chars, then you should update their names to avoid running into Postgres identifier limit bug (#5786)
+- server: validate remote schema queries (fixes #4143)
 - console: allow user to cascade Postgres dependencies when dropping Postgres objects (close #5109) (#5248)
 - console: mark inconsistent remote schemas in the UI (close #5093) (#5181)
 - cli: add missing global flags for seed command (#5565)

diff --git a/server/src-lib/Hasura/GraphQL/Context.hs b/server/src-lib/Hasura/GraphQL/Context.hs
@@ -96,7 +96,7 @@ data ActionQuery v
   = AQQuery !(RQL.AnnActionExecution v)
   | AQAsync !(RQL.AnnActionAsyncQuery v)
 
-type RemoteField = (RQL.RemoteSchemaInfo, G.Field G.NoFragments Variable)
+type RemoteField = (RQL.RemoteSchemaInfo, G.Field G.NoFragments G.Name)
 
 type QueryRootField v = RootField (QueryDB v) RemoteField (ActionQuery v) J.Value
 

diff --git a/server/src-lib/Hasura/GraphQL/Execute/Remote.hs b/server/src-lib/Hasura/GraphQL/Execute/Remote.hs
@@ -11,17 +11,8 @@ import qualified Data.HashSet                           as Set
 import qualified Hasura.GraphQL.Transport.HTTP.Protocol as GH
 
 import           Hasura.GraphQL.Execute.Prepare
-import           Hasura.GraphQL.Parser
 import           Hasura.RQL.Types
 
-unresolveVariables
-  :: forall fragments
-   . Functor fragments
-  => G.SelectionSet fragments Variable
-  -> G.SelectionSet fragments G.Name
-unresolveVariables =
-  fmap (fmap (getName . vInfo))
-
 collectVariables
   :: forall fragments var
    . (Foldable fragments, Hashable var, Eq var)
@@ -35,12 +26,11 @@ buildExecStepRemote
    . RemoteSchemaInfo
   -> G.OperationType
   -> [G.VariableDefinition]
-  -> G.SelectionSet G.NoFragments Variable
+  -> G.SelectionSet G.NoFragments G.Name
   -> Maybe GH.VariableValues
   -> ExecutionStep db
 buildExecStepRemote remoteSchemaInfo tp varDefs selSet varValsM =
-  let unresolvedSelSet = unresolveVariables selSet
-      requiredVars = collectVariables unresolvedSelSet
+  let requiredVars = collectVariables selSet
       restrictedDefs = filter (\varDef -> G._vdName varDef `Set.member` requiredVars) varDefs
       restrictedValsM = flip Map.intersection (Set.toMap requiredVars) <$> varValsM
-  in ExecStepRemote (remoteSchemaInfo, G.TypedOperationDefinition tp Nothing restrictedDefs [] unresolvedSelSet, restrictedValsM)
+  in ExecStepRemote (remoteSchemaInfo, G.TypedOperationDefinition tp Nothing restrictedDefs [] selSet, restrictedValsM)
diff --git a/server/src-lib/Hasura/GraphQL/Parser.hs b/server/src-lib/Hasura/GraphQL/Parser.hs
@@ -33,8 +33,10 @@ module Hasura.GraphQL.Parser
   , ParsedSelection(..)
   , handleTypename
   , selection
+  , rawSelection
   , selection_
   , subselection
+  , rawSubselection
   , subselection_
 
   , jsonToGraphQL

diff --git a/server/src-lib/Hasura/GraphQL/Parser/Internal/Parser.hs b/server/src-lib/Hasura/GraphQL/Parser/Internal/Parser.hs
@@ -822,18 +822,31 @@ selection
   -> InputFieldsParser m a -- ^ parser for the input arguments
   -> Parser 'Both m b -- ^ type of the result
   -> FieldParser m a
-selection name description argumentsParser resultParser = FieldParser
+selection name description argumentsParser resultParser =
+  rawSelection name description argumentsParser resultParser
+  <&> \(_alias, _args, a) -> a
+
+rawSelection
+  :: forall m a b
+   . MonadParse m
+  => Name
+  -> Maybe Description
+  -> InputFieldsParser m a -- ^ parser for the input arguments
+  -> Parser 'Both m b -- ^ type of the result
+  -> FieldParser m (Maybe Name, HashMap Name (Value Variable), a)
+  -- ^ alias provided (if any), and the arguments
+rawSelection name description argumentsParser resultParser = FieldParser
   { fDefinition = mkDefinition name description $
       FieldInfo (ifDefinitions argumentsParser) (pType resultParser)
-  , fParser = \Field{ _fArguments, _fSelectionSet } -> do
+  , fParser = \Field{ _fAlias, _fArguments, _fSelectionSet } -> do
       unless (null _fSelectionSet) $
         parseError "unexpected subselection set for non-object field"
       -- check for extraneous arguments here, since the InputFieldsParser just
       -- handles parsing the fields it cares about
       for_ (M.keys _fArguments) \argumentName ->
         unless (argumentName `S.member` argumentNames) $
           parseError $ name <<> " has no argument named " <>> argumentName
-      withPath (++[Key "args"]) $ ifParser argumentsParser $ GraphQLValue <$> _fArguments
+      fmap (_fAlias, _fArguments, ) $ withPath (++[Key "args"]) $ ifParser argumentsParser $ GraphQLValue <$> _fArguments
   }
   where
     argumentNames = S.fromList (dName <$> ifDefinitions argumentsParser)
@@ -850,17 +863,29 @@ subselection
   -> InputFieldsParser m a -- ^ parser for the input arguments
   -> Parser 'Output m b -- ^ parser for the subselection set
   -> FieldParser m (a, b)
-subselection name description argumentsParser bodyParser = FieldParser
+subselection name description argumentsParser bodyParser =
+  rawSubselection name description argumentsParser bodyParser
+  <&> \(_alias, _args, a, b) -> (a, b)
+
+rawSubselection
+  :: forall m a b
+   . MonadParse m
+  => Name
+  -> Maybe Description
+  -> InputFieldsParser m a -- ^ parser for the input arguments
+  -> Parser 'Output m b -- ^ parser for the subselection set
+  -> FieldParser m (Maybe Name, HashMap Name (Value Variable), a, b)
+rawSubselection name description argumentsParser bodyParser = FieldParser
   { fDefinition = mkDefinition name description $
       FieldInfo (ifDefinitions argumentsParser) (pType bodyParser)
-  , fParser = \Field{ _fArguments, _fSelectionSet } -> do
+  , fParser = \Field{ _fAlias, _fArguments, _fSelectionSet } -> do
       -- check for extraneous arguments here, since the InputFieldsParser just
       -- handles parsing the fields it cares about
       for_ (M.keys _fArguments) \argumentName ->
         unless (argumentName `S.member` argumentNames) $
           parseError $ name <<> " has no argument named " <>> argumentName
-      (,) <$> withPath (++[Key "args"]) (ifParser argumentsParser $ GraphQLValue <$> _fArguments)
-          <*> pParser bodyParser _fSelectionSet
+      (_fAlias,_fArguments,,) <$> withPath (++[Key "args"]) (ifParser argumentsParser $ GraphQLValue <$> _fArguments)
+        <*> pParser bodyParser _fSelectionSet
   }
   where
     argumentNames = S.fromList (dName <$> ifDefinitions argumentsParser)
@@ -884,7 +909,6 @@ subselection_
 subselection_ name description bodyParser =
   snd <$> subselection name description (pure ()) bodyParser
 
-
 -- -----------------------------------------------------------------------------
 -- helpers
 

diff --git a/server/src-lib/Hasura/GraphQL/Schema.hs b/server/src-lib/Hasura/GraphQL/Schema.hs
@@ -206,7 +206,7 @@ query'
      )
   => HashSet QualifiedTable
   -> [FunctionInfo]
-  -> [P.FieldParser n (RemoteSchemaInfo, G.Field G.NoFragments P.Variable)]
+  -> [P.FieldParser n RemoteField]
   -> [ActionInfo]
   -> NonObjectTypeMap
   -> m [P.FieldParser n (QueryRootField UnpreparedValue)]
@@ -302,7 +302,7 @@ query
   => G.Name
   -> HashSet QualifiedTable
   -> [FunctionInfo]
-  -> [P.FieldParser n (RemoteSchemaInfo, G.Field G.NoFragments P.Variable)]
+  -> [P.FieldParser n RemoteField]
   -> [ActionInfo]
   -> NonObjectTypeMap
   -> m (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue)))
@@ -402,8 +402,8 @@ queryWithIntrospection
      )
   => HashSet QualifiedTable
   -> [FunctionInfo]
-  -> [P.FieldParser n (RemoteSchemaInfo, G.Field G.NoFragments P.Variable)]
-  -> [P.FieldParser n (RemoteSchemaInfo, G.Field G.NoFragments P.Variable)]
+  -> [P.FieldParser n RemoteField]
+  -> [P.FieldParser n RemoteField]
   -> [ActionInfo]
   -> NonObjectTypeMap
   -> m (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue)))
@@ -464,8 +464,8 @@ unauthenticatedQueryWithIntrospection
    . ( MonadSchema n m
      , MonadError QErr m
      )
-  => [P.FieldParser n (RemoteSchemaInfo, G.Field G.NoFragments P.Variable)]
-  -> [P.FieldParser n (RemoteSchemaInfo, G.Field G.NoFragments P.Variable)]
+  => [P.FieldParser n RemoteField]
+  -> [P.FieldParser n RemoteField]
   -> m (Parser 'Output n (OMap.InsOrdHashMap G.Name (QueryRootField UnpreparedValue)))
 unauthenticatedQueryWithIntrospection queryRemotes mutationRemotes = do
   let basicQueryFP = fmap (fmap RFRemote) queryRemotes
@@ -477,7 +477,7 @@ mutation
   :: forall m n r
    . (MonadSchema n m, MonadTableInfo r m, MonadRole r m, Has QueryContext r, Has Scenario r)
   => HashSet QualifiedTable
-  -> [P.FieldParser n (RemoteSchemaInfo, G.Field G.NoFragments P.Variable)]
+  -> [P.FieldParser n RemoteField]
   -> [ActionInfo]
   -> NonObjectTypeMap
   -> m (Maybe (Parser 'Output n (OMap.InsOrdHashMap G.Name (MutationRootField UnpreparedValue))))
@@ -556,7 +556,7 @@ mutation allTables allRemotes allActions nonObjectCustomTypes = do
 unauthenticatedMutation
   :: forall n m
    . (MonadError QErr m, MonadParse n)
-  => [P.FieldParser n (RemoteSchemaInfo, G.Field G.NoFragments P.Variable)]
+  => [P.FieldParser n RemoteField]
   -> m (Maybe (Parser 'Output n (OMap.InsOrdHashMap G.Name (MutationRootField UnpreparedValue))))
 unauthenticatedMutation allRemotes =
   let mutationFieldsParser = fmap (fmap RFRemote) allRemotes