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>
2024-09-20 15:09:02 +03:00 · 2020-10-13 14:03:11 +05:30 · 2020-10-13 14:03:11 +05:30 · 3ea611f9fd
commit 3ea611f9fd
parent 19b4f55ca1
16 changed files with 458 additions and 108 deletions
--- 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"
--- 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)
--- 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

--- 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)
--- 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
--- a/server/src-lib/Hasura/GraphQL/Parser/Internal/Parser.hs
+++ b/server/src-lib/Hasura/GraphQL/Parser/Internal/Parser.hs
@ -822,10 +822,23 @@ 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
@ -833,7 +846,7 @@ selection name description argumentsParser resultParser = FieldParser
      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

--- 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
--- a/server/src-lib/Hasura/GraphQL/Schema/Remote.hs
+++ b/server/src-lib/Hasura/GraphQL/Schema/Remote.hs
@ -1,6 +1,5 @@
 module Hasura.GraphQL.Schema.Remote
  ( buildRemoteParser
-  , remoteFieldFullSchema
  , inputValueDefinitionParser
  , lookupObject
  , lookupType
@ -11,77 +10,61 @@ import           Hasura.Prelude
 import           Hasura.RQL.Types
 import           Hasura.SQL.Types

-import           Language.GraphQL.Draft.Syntax       as G
-import qualified Data.List.NonEmpty                  as NE
+import           Language.GraphQL.Draft.Syntax         as G
+import qualified Data.List.NonEmpty                    as NE
 import           Data.Type.Equality
-import           Data.Foldable                       (sequenceA_)
+import qualified Data.HashMap.Strict                   as Map
+import qualified Data.HashMap.Strict.Extended          as Map
+import qualified Data.HashMap.Strict.InsOrd            as OMap
+import           Data.Foldable                         (sequenceA_)

-import           Hasura.GraphQL.Parser               as P
+import           Hasura.GraphQL.Parser                 as P
 import qualified Hasura.GraphQL.Parser.Internal.Parser as P
+import           Hasura.GraphQL.Context                (RemoteField)

 buildRemoteParser
  :: forall m n
   . (MonadSchema n m, MonadError QErr m)
  => IntrospectionResult
  -> RemoteSchemaInfo
-  -> m ( [P.FieldParser n (RemoteSchemaInfo, Field NoFragments Variable)]
-       , Maybe [P.FieldParser n (RemoteSchemaInfo, Field NoFragments Variable)]
-       , Maybe [P.FieldParser n (RemoteSchemaInfo, Field NoFragments Variable)])
+  -> m ( [P.FieldParser n RemoteField]
+       , Maybe [P.FieldParser n RemoteField]
+       , Maybe [P.FieldParser n RemoteField])
 buildRemoteParser (IntrospectionResult sdoc query_root mutation_root subscription_root) info = do
  queryT <- makeParsers query_root
  mutationT <- traverse makeParsers mutation_root
  subscriptionT <- traverse makeParsers subscription_root
  return (queryT, mutationT, subscriptionT)
  where
-    makeFieldParser :: G.FieldDefinition -> m (P.FieldParser n (RemoteSchemaInfo, Field NoFragments Variable))
+    makeFieldParser :: G.FieldDefinition -> m (P.FieldParser n RemoteField)
    makeFieldParser fieldDef = do
-      fp <- remoteField' sdoc fieldDef
-      return $ do
-        raw <- P.unsafeRawField (P.fDefinition fp)
-        return (info, raw)
-    makeParsers :: G.Name -> m [P.FieldParser n (RemoteSchemaInfo, Field NoFragments Variable)]
+      fldParser <- remoteField' sdoc fieldDef
+      pure $ (info, ) <$> fldParser
+    makeParsers :: G.Name -> m [P.FieldParser n RemoteField]
    makeParsers rootName =
      case lookupType sdoc rootName of
        Just (G.TypeDefinitionObject o) ->
          traverse makeFieldParser $ _otdFieldsDefinition o
        _ -> throw400 Unexpected $ rootName <<> " has to be an object type"

-- | 'remoteFieldFullSchema' takes the 'SchemaIntrospection' and a 'G.Name' and will
--   return a 'SelectionSet' parser if the 'G.Name' is found and is a 'TypeDefinitionObject',
--   otherwise, an error will be thrown.
-remoteFieldFullSchema
-  :: forall n m
-   . (MonadSchema n m, MonadError QErr m)
-  => SchemaIntrospection
-  -> G.Name
-  -> m (Parser 'Output n (G.SelectionSet NoFragments Variable))
-remoteFieldFullSchema sdoc name =
-  P.memoizeOn 'remoteFieldFullSchema name do
-  fieldObjectType <-
-    case lookupType sdoc name of
-      Just (G.TypeDefinitionObject o) -> pure o
-      _ -> throw400 RemoteSchemaError $ "object with " <> G.unName name <> " not found"
-  fieldParser <- remoteSchemaObject sdoc fieldObjectType
-  pure $ P.unsafeRawParser (P.pType fieldParser)
-
 remoteField'
  :: forall n m
  . (MonadSchema n m, MonadError QErr m)
  => SchemaIntrospection
  -> G.FieldDefinition
-  -> m (FieldParser n ())
+  -> m (FieldParser n (Field NoFragments G.Name))
 remoteField' schemaDoc (G.FieldDefinition description name argsDefinition gType _) =
  let
-    addNullableList :: FieldParser n () -> FieldParser n ()
+    addNullableList :: FieldParser n (Field NoFragments G.Name) -> FieldParser n (Field NoFragments G.Name)
    addNullableList (P.FieldParser (Definition name' un desc (FieldInfo args typ)) parser)
      = P.FieldParser (Definition name' un desc (FieldInfo args (Nullable (TList typ)))) parser

-    addNonNullableList :: FieldParser n () -> FieldParser n ()
+    addNonNullableList :: FieldParser n (Field NoFragments G.Name) -> FieldParser n (Field NoFragments G.Name)
    addNonNullableList (P.FieldParser (Definition name' un desc (FieldInfo args typ)) parser)
      = P.FieldParser (Definition name' un desc (FieldInfo args (NonNullable (TList typ)))) parser

    -- TODO add directives, deprecation
-    convertType :: G.GType -> m (FieldParser n ())
+    convertType :: G.GType -> m (FieldParser n (Field NoFragments G.Name))
    convertType gType' = do
        case gType' of
          G.TypeNamed (Nullability True) fieldTypeName ->
@ -100,7 +83,7 @@ remoteSchemaObject
  . (MonadSchema n m, MonadError QErr m)
  => SchemaIntrospection
  -> G.ObjectTypeDefinition
-  -> m (Parser 'Output n ())
+  -> m (Parser 'Output n [Field NoFragments Name])
 remoteSchemaObject schemaDoc defn@(G.ObjectTypeDefinition description name interfaces _directives subFields) =
  P.memoizeOn 'remoteSchemaObject defn do
  subFieldParsers <- traverse (remoteField' schemaDoc) subFields
@ -108,7 +91,12 @@ remoteSchemaObject schemaDoc defn@(G.ObjectTypeDefinition description name inter
  implements <- traverse (remoteSchemaInterface schemaDoc) interfaceDefs
  -- TODO: also check sub-interfaces, when these are supported in a future graphql spec
  traverse_ validateImplementsFields interfaceDefs
-  pure $ void $ P.selectionSetObject name description subFieldParsers implements
+  pure $ P.selectionSetObject name description subFieldParsers implements <&>
+    toList . (OMap.mapWithKey $ \alias -> \case
+        P.SelectField fld  -> fld
+        P.SelectTypename _ ->
+          G.Field (Just alias) $$(G.litName "__typename") mempty mempty mempty
+    )
  where
    getInterface :: G.Name -> m (G.InterfaceTypeDefinition [G.Name])
    getInterface interfaceName =
@ -181,18 +169,83 @@ remoteSchemaObject schemaDoc defn@(G.ObjectTypeDefinition description name inter
      = True -- TODO write appropriate check (may require saving 'possibleTypes' in Syntax.hs)
    validateSubTypeDefinition _ _ = False

+-- | helper function to get a parser of an object with it's name
+--   This function is called from 'remoteSchemaInterface' and
+--   'remoteSchemaObject' functions. Both of these have a slightly
+--   different implementation of 'getObject', which is the
+--   reason 'getObject' is an argument to this function
+getObjectParser
+  :: forall n m
+  . (MonadSchema n m, MonadError QErr m)
+  => SchemaIntrospection
+  -> (G.Name -> m G.ObjectTypeDefinition)
+  -> G.Name
+  -> m (Parser 'Output n (Name, [Field NoFragments G.Name]))
+getObjectParser schemaDoc getObject objName = do
+  obj <- remoteSchemaObject schemaDoc =<< getObject objName
+  return $ (objName,) <$> obj
+
+{- Note [Querying remote schema interfaces]
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+When querying Remote schema interfaces, we need to re-construct
+the incoming query to be compliant with the upstream remote.
+We need to do this because the `SelectionSet`(s) that are
+inputted to this function have the fragments (if any) flattened.
+(Check `flattenSelectionSet` in 'Hasura.GraphQL.Parser.Collect' module)
+The `constructInterfaceSelectionSet` function makes a valid interface query by:
+1. Getting the common interface fields in all the selection sets
+2. Remove the common fields obtained in #1 from the selection sets
+3. Construct a selection field for every common interface field
+4. Construct inline fragments for non-common interface fields
+   using the result of #2 for every object
+5. Construct the final selection set by combining #3 and #4
+
+Example: Suppose an interface 'Character' is defined in the upstream
+and two objects 'Human' and 'Droid' implement the 'Character' Interface.
+
+Suppose, a field 'hero' returns 'Character'.
+
+{
+   hero {
+     id
+     name
+     ... on Droid {
+       primaryFunction
+     }
+     ... on Human {
+       homePlanet
+     }
+   }
+}
+
+When we parse the selection set of the `hero` field, we parse the selection set
+twice: once for the `Droid` object type, which would be passed a selection set
+containing the field(s) defined in the `Droid` object type and similarly once
+for the 'Human' object type. The result of the interface selection set parsing
+would then be the results of the parsing of the object types when passed their
+corresponding flattened selection sets and the results of the parsing of the
+interface fields.
+
+After we parse the above GraphQL query, we get a selection set containing
+the interface fields and the selection sets of the objects that were queried
+in the GraphQL query. Since, we have the selection sets of the objects that
+were being queried, we can convert them into inline fragments resembling
+the original query and then query the remote schema with the newly
+constructed query.
+-}
+
 -- | 'remoteSchemaInterface' returns a output parser for a given 'InterfaceTypeDefinition'.
+--   Also check Note [Querying remote schema interfaces]
 remoteSchemaInterface
  :: forall n m
  . (MonadSchema n m, MonadError QErr m)
  => SchemaIntrospection
  -> G.InterfaceTypeDefinition [G.Name]
-  -> m (Parser 'Output n ())
+  -> m (Parser 'Output n (G.SelectionSet NoFragments G.Name))
 remoteSchemaInterface schemaDoc defn@(G.InterfaceTypeDefinition description name _directives fields possibleTypes) =
  P.memoizeOn 'remoteSchemaObject defn do
  subFieldParsers <- traverse (remoteField' schemaDoc) fields
-  objs :: [Parser 'Output n ()] <-
-    traverse (getObject >=> remoteSchemaObject schemaDoc) possibleTypes
+  objs <- traverse (getObjectParser schemaDoc getObject) possibleTypes
  -- In the Draft GraphQL spec (> June 2018), interfaces can themselves
  -- implement superinterfaces.  In the future, we may need to support this
  -- here.
@ -201,8 +254,8 @@ remoteSchemaInterface schemaDoc defn@(G.InterfaceTypeDefinition description name
  -- TODO: another way to obtain 'possibleTypes' is to lookup all the object
  -- types in the schema document that claim to implement this interface.  We
  -- should have a check that expresses that that collection of objects is equal
-  -- to 'possibelTypes'.
-  pure $ void $ P.selectionSetInterface name description subFieldParsers objs
+  -- to 'possibleTypes'.
+  pure $ P.selectionSetInterface name description subFieldParsers objs <&> constructInterfaceSelectionSet
  where
    getObject :: G.Name -> m G.ObjectTypeDefinition
    getObject objectName =
@ -213,20 +266,70 @@ remoteSchemaInterface schemaDoc defn@(G.InterfaceTypeDefinition description name
          Just _  -> throw400 RemoteSchemaError $ "Interface type " <> squote name <>
            " can only include object types. It cannot include " <> squote objectName

+    -- 'constructInterfaceQuery' constructs a remote interface query.
+    constructInterfaceSelectionSet
+      :: [(G.Name, [Field NoFragments G.Name])]
+      -> SelectionSet NoFragments G.Name
+    constructInterfaceSelectionSet objNameAndFields =
+      let -- common interface fields that exist in every
+          -- selection set provided
+          -- #1 of Note [Querying remote schema Interfaces]
+          commonInterfaceFields =
+            Map.elems $
+            Map.mapMaybe allTheSame $
+            Map.groupOn G._fName $
+            concatMap (filter ((`elem` interfaceFieldNames) . G._fName) . snd) $
+            objNameAndFields
+
+          interfaceFieldNames = map G._fldName fields
+
+          allTheSame (x:xs) | all (== x) xs = Just x
+          allTheSame _ = Nothing
+
+          -- #2 of Note [Querying remote schema interface fields]
+          nonCommonInterfaceFields =
+            catMaybes $ flip map objNameAndFields $ \(objName, objFields) ->
+              let nonCommonFields = filter (not . flip elem commonInterfaceFields) objFields
+              in mkObjInlineFragment (objName, map G.SelectionField nonCommonFields)
+
+          -- helper function for #4 of Note [Querying remote schema interface fields]
+          mkObjInlineFragment (_, []) = Nothing
+          mkObjInlineFragment (objName, selSet) =
+            Just $ G.SelectionInlineFragment $
+              G.InlineFragment (Just objName) mempty selSet
+
+      -- #5 of Note [Querying remote schema interface fields]
+      in (fmap G.SelectionField commonInterfaceFields) <> nonCommonInterfaceFields
+
 -- | 'remoteSchemaUnion' returns a output parser for a given 'UnionTypeDefinition'.
 remoteSchemaUnion
  :: forall n m
  . (MonadSchema n m, MonadError QErr m)
  => SchemaIntrospection
  -> G.UnionTypeDefinition
-  -> m (Parser 'Output n ())
+  -> m (Parser 'Output n (SelectionSet NoFragments G.Name))
 remoteSchemaUnion schemaDoc defn@(G.UnionTypeDefinition description name _directives objectNames) =
  P.memoizeOn 'remoteSchemaObject defn do
-  objDefs <- traverse getObject objectNames
-  objs :: [Parser 'Output n ()] <- traverse (remoteSchemaObject schemaDoc) objDefs
+  objs <- traverse (getObjectParser schemaDoc getObject) objectNames
  when (null objs) $
    throw400 RemoteSchemaError $ "List of member types cannot be empty for union type " <> squote name
-  pure $ void $ P.selectionSetUnion name description objs
+  pure $ P.selectionSetUnion name description objs <&>
+    (\objNameAndFields ->
+       catMaybes $ objNameAndFields <&> \(objName, fields) ->
+        case fields of
+          -- The return value obtained from the parsing of a union selection set
+          -- specifies, for each object in the union type, a fragment-free
+          -- selection set for that object type. In particular, if, for a given
+          -- object type, the selection set passed to the union type did not
+          -- specify any fields for that object type (i.e. if no inline fragment
+          -- applied to that object), the selection set resulting from the parsing
+          -- through that object type would be empty, i.e. []. We exclude such
+          -- object types from the reconstructed selection set for the union
+          -- type, as selection sets cannot be empty.
+          [] -> Nothing
+          _  ->
+            Just (G.SelectionInlineFragment
+                    $ G.InlineFragment (Just objName) mempty $ fmap G.SelectionField fields))
  where
    getObject :: G.Name -> m G.ObjectTypeDefinition
    getObject objectName =
@ -301,10 +404,10 @@ remoteFieldFromName
  -> Maybe G.Description
  -> G.Name
  -> G.ArgumentsDefinition
-  -> m (FieldParser n ())
+  -> m (FieldParser n (Field NoFragments G.Name))
 remoteFieldFromName sdoc fieldName description fieldTypeName argsDefns =
  case lookupType sdoc fieldTypeName of
-    Nothing -> throw400 RemoteSchemaError $ "Could not find type with name " <> G.unName fieldName
+    Nothing -> throw400 RemoteSchemaError $ "Could not find type with name " <>> fieldName
    Just typeDef -> remoteField sdoc fieldName description argsDefns typeDef

 -- | 'inputValuefinitionParser' accepts a 'G.InputValueDefinition' and will return an
@ -341,11 +444,11 @@ inputValueDefinitionParser schemaDoc (G.InputValueDefinition desc name fieldType
      buildField fieldType' fieldConstructor' = case fieldType' of
       G.TypeNamed nullability typeName ->
         case lookupType schemaDoc typeName of
-           Nothing -> throw400 RemoteSchemaError $ "Could not find type with name " <> G.unName typeName
+           Nothing -> throw400 RemoteSchemaError $ "Could not find type with name " <>> typeName
           Just typeDef ->
             case typeDef of
-               G.TypeDefinitionScalar (G.ScalarTypeDefinition scalarDesc name' _) ->
-                 pure $ fieldConstructor' $ doNullability nullability $ remoteFieldScalarParser name' scalarDesc
+               G.TypeDefinitionScalar scalarTypeDefn ->
+                 pure $ fieldConstructor' $ doNullability nullability $ remoteFieldScalarParser scalarTypeDefn
               G.TypeDefinitionEnum defn ->
                 pure $ fieldConstructor' $ doNullability nullability $ remoteFieldEnumParser defn
               G.TypeDefinitionObject _ -> throw400 RemoteSchemaError "expected input type, but got output type" -- couldn't find the equivalent error in Validate/Types.hs, so using a new error message
@ -362,8 +465,8 @@ argumentsParser
  => G.ArgumentsDefinition
  -> G.SchemaIntrospection
  -> m (InputFieldsParser n ())
-argumentsParser args schemaDoc =
-  sequenceA_ <$> traverse (inputValueDefinitionParser schemaDoc) args
+argumentsParser args schemaDoc = do
+  sequenceA_ <$> for args (inputValueDefinitionParser schemaDoc)

 -- | 'remoteField' accepts a 'G.TypeDefinition' and will returns a 'FieldParser' for it.
 --   Note that the 'G.TypeDefinition' should be of the GraphQL 'Output' kind, when an
@ -376,32 +479,57 @@ remoteField
  -> Maybe G.Description
  -> G.ArgumentsDefinition
  -> G.TypeDefinition [G.Name]
-  -> m (FieldParser n ()) -- TODO return something useful, maybe?
+  -> m (FieldParser n (Field NoFragments G.Name))
 remoteField sdoc fieldName description argsDefn typeDefn = do
  -- TODO add directives
  argsParser <- argumentsParser argsDefn sdoc
  case typeDefn of
    G.TypeDefinitionObject objTypeDefn -> do
-      remoteSchemaObj <- remoteSchemaObject sdoc objTypeDefn
-      pure $ void $ P.subselection fieldName description argsParser remoteSchemaObj
-    G.TypeDefinitionScalar (G.ScalarTypeDefinition desc name' _) ->
-      pure $ P.selection fieldName description argsParser $ remoteFieldScalarParser name' desc
+      remoteSchemaObjFields <- remoteSchemaObject sdoc objTypeDefn
+      -- converting [Field NoFragments Name] to (SelectionSet NoFragments G.Name)
+      let remoteSchemaObjSelSet = fmap G.SelectionField <$> remoteSchemaObjFields
+      pure remoteSchemaObjSelSet
+        <&> mkFieldParserWithSelectionSet argsParser
+    G.TypeDefinitionScalar scalarTypeDefn ->
+      pure $ mkFieldParserWithoutSelectionSet argsParser
+             $ remoteFieldScalarParser scalarTypeDefn
    G.TypeDefinitionEnum enumTypeDefn ->
-      pure $ P.selection fieldName description argsParser $ remoteFieldEnumParser enumTypeDefn
-    G.TypeDefinitionInterface ifaceTypeDefn -> do
-      remoteSchemaObj <- remoteSchemaInterface sdoc ifaceTypeDefn
-      pure $ void $ P.subselection fieldName description argsParser remoteSchemaObj
-    G.TypeDefinitionUnion unionTypeDefn -> do
-      remoteSchemaObj <- remoteSchemaUnion sdoc unionTypeDefn
-      pure $ void $ P.subselection fieldName description argsParser remoteSchemaObj
+      pure $ mkFieldParserWithoutSelectionSet argsParser
+             $ remoteFieldEnumParser enumTypeDefn
+    G.TypeDefinitionInterface ifaceTypeDefn ->
+      remoteSchemaInterface sdoc ifaceTypeDefn <&>
+        mkFieldParserWithSelectionSet argsParser
+    G.TypeDefinitionUnion unionTypeDefn ->
+      remoteSchemaUnion sdoc unionTypeDefn <&>
+        mkFieldParserWithSelectionSet argsParser
    _ -> throw400 RemoteSchemaError "expected output type, but got input type"
+  where
+    mkFieldParserWithoutSelectionSet
+      :: InputFieldsParser n ()
+      -> Parser 'Both n ()
+      -> FieldParser n (Field NoFragments G.Name)
+    mkFieldParserWithoutSelectionSet argsParser outputParser =
+      -- 'rawSelection' is used here to get the alias and args data
+      -- specified to be able to construct the `Field NoFragments G.Name`
+      P.rawSelection fieldName description argsParser outputParser
+      <&> (\(alias, args, _) -> (G.Field alias fieldName (fmap getName <$> args) mempty []))
+
+    mkFieldParserWithSelectionSet
+      :: InputFieldsParser n ()
+      -> Parser 'Output n (SelectionSet NoFragments G.Name)
+      -> FieldParser n (Field NoFragments G.Name)
+    mkFieldParserWithSelectionSet argsParser outputParser =
+      -- 'rawSubselection' is used here to get the alias and args data
+      -- specified to be able to construct the `Field NoFragments G.Name`
+      P.rawSubselection fieldName description argsParser outputParser
+      <&> (\(alias, args, _, selSet) ->
+             (G.Field alias fieldName (fmap getName <$> args) mempty selSet))

 remoteFieldScalarParser
  :: MonadParse n
-  => G.Name
-  -> Maybe G.Description
+  => G.ScalarTypeDefinition
  -> Parser 'Both n ()
-remoteFieldScalarParser name description =
+remoteFieldScalarParser (G.ScalarTypeDefinition description name _directives) =
  case G.unName name of
    "Boolean" -> P.boolean $> ()
    "Int" -> P.int $> ()
@ -414,7 +542,7 @@ remoteFieldEnumParser
  :: MonadParse n
  => G.EnumTypeDefinition
  -> Parser 'Both n ()
-remoteFieldEnumParser (G.EnumTypeDefinition desc name _ valueDefns) =
+remoteFieldEnumParser (G.EnumTypeDefinition desc name _directives valueDefns) =
  let enumValDefns = valueDefns <&> \(G.EnumValueDefinition enumDesc enumName _) ->
        (mkDefinition (G.unEnumValue enumName) enumDesc P.EnumValueInfo,())
  in P.enum name desc $ NE.fromList enumValDefns
--- a/server/src-lib/Hasura/RQL/Types/SchemaCache.hs
+++ b/server/src-lib/Hasura/RQL/Types/SchemaCache.hs
@ -116,7 +116,7 @@ module Hasura.RQL.Types.SchemaCache
  ) where

 import           Hasura.Db
-import           Hasura.GraphQL.Context            (GQLContext, RoleContext)
+import           Hasura.GraphQL.Context            (GQLContext, RoleContext, RemoteField)
 import qualified Hasura.GraphQL.Parser             as P
 import           Hasura.Incremental                (Dependency, MonadDepend (..), selectKeyD)
 import           Hasura.Prelude
@ -178,9 +178,9 @@ data IntrospectionResult

 data ParsedIntrospection
  = ParsedIntrospection
-  { piQuery        :: [P.FieldParser (P.ParseT Identity) (RemoteSchemaInfo, G.Field G.NoFragments P.Variable)]
-  , piMutation     :: Maybe [P.FieldParser (P.ParseT Identity) (RemoteSchemaInfo, G.Field G.NoFragments P.Variable)]
-  , piSubscription :: Maybe [P.FieldParser (P.ParseT Identity) (RemoteSchemaInfo, G.Field G.NoFragments P.Variable)]
+  { piQuery        :: [P.FieldParser (P.ParseT Identity) RemoteField]
+  , piMutation     :: Maybe [P.FieldParser (P.ParseT Identity) RemoteField]
+  , piSubscription :: Maybe [P.FieldParser (P.ParseT Identity) RemoteField]
  }

 data RemoteSchemaCtx
--- a/server/tests-py/graphql_server.py
+++ b/server/tests-py/graphql_server.py
@ -12,6 +12,8 @@ from enum import Enum

 import time

+from graphql import GraphQLError
+
 HGE_URLS=[]

 def mkJSONResp(graphql_result):
@ -53,6 +55,8 @@ class HelloGraphQL(RequestHandler):
 class User(graphene.ObjectType):
    id = graphene.Int()
    username = graphene.String()
+    generateError = graphene.String()
+
    def __init__(self, id, username):
        self.id = id
        self.username = username
@ -63,6 +67,9 @@ class User(graphene.ObjectType):
    def resolve_username(self, info):
        return self.username

+    def resolve_generateError(self, info):
+        return GraphQLError ('Cannot query field "generateError" on type "User".')
+
    @staticmethod
    def get_by_id(_id):
        xs = list(filter(lambda u: u.id == _id, all_users))
@ -76,6 +83,25 @@ all_users = [
    User(3, 'joe'),
 ]

+class UserDetailsInput(graphene.InputObjectType):
+    id = graphene.Int(required=True)
+    username = graphene.String(required=True)
+
+class CreateUserInputObject(graphene.Mutation):
+    class Arguments:
+        user_data = UserDetailsInput(required=True)
+
+    ok = graphene.Boolean()
+    user = graphene.Field(lambda: User)
+
+    def mutate(self, info, user_data=None):
+        user = User(
+            id = user_data.id,
+            username = user_data.username
+        )
+        all_users.append(user)
+        return CreateUserInputObject(ok=True, user = user)
+
 class CreateUser(graphene.Mutation):
    class Arguments:
        id = graphene.Int(required=True)
@ -101,6 +127,8 @@ class UserQuery(graphene.ObjectType):

 class UserMutation(graphene.ObjectType):
    createUser = CreateUser.Field()
+    createUserInputObj = CreateUserInputObject.Field()
+
 user_schema = graphene.Schema(query=UserQuery, mutation=UserMutation)

 class UserGraphQL(RequestHandler):
--- a/server/tests-py/queries/heterogeneous/basic.yaml
+++ b/server/tests-py/queries/heterogeneous/basic.yaml
@ -78,13 +78,16 @@
          name
        }
        user_alias : user(id: 2) {
-          NonExistingField
+          generateError
        }
      }
  response:
    data:
    errors:
-    - message: Cannot query field "NonExistingField" on type "User".
+    - message: Cannot query field "generateError" on type "User".
+      path:
+        - user_alias
+        - generateError
      locations:
      - line: 1
        column: 36
--- a/server/tests-py/queries/remote_schemas/validation/argument_validation.yaml
+++ b/server/tests-py/queries/remote_schemas/validation/argument_validation.yaml
@ -0,0 +1,55 @@
+- description: query the remote with a non-existing input argument 'foo'
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+      {
+        user(foo:1) {
+          id
+          username
+        }
+      }
+  response:
+    errors:
+      - extensions:
+          path: $.selectionSet.user
+          code: validation-failed
+        message: '"user" has no argument named "foo"'
+
+- description: query the remote with a non-existing input argument 'foo'
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+      {
+        user(id:"1") {
+          id
+          username
+        }
+      }
+  response:
+    errors:
+      - extensions:
+          path: $.selectionSet.user.args.id
+          code: validation-failed
+        message: "expected a 32-bit integer for type \"Int\", but found a string"
+
+- description: query the remote with a non-existing input object field 'foo'
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+       mutation {
+         createUserInputObj(userData:{id:5,username:"somethin",foo:"baz"}) {
+           user {
+             id
+             username
+           }
+         }
+       }
+  response:
+    errors:
+      - extensions:
+          path: $.selectionSet.createUserInputObj.args.userData.foo
+          code: validation-failed
+        message: "field \"foo\" not found in type: 'UserDetailsInput'"
--- a/server/tests-py/queries/remote_schemas/validation/field_validation.yaml
+++ b/server/tests-py/queries/remote_schemas/validation/field_validation.yaml
@ -0,0 +1,68 @@
+- description: query the remote with a non-existing field 'non_existing_field', which should fail to validate
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+      query {
+        user (id: 1) {
+          id
+          username
+          non_existing_field
+        }
+      }
+  response:
+    errors:
+      - extensions:
+          path: $.selectionSet.user.selectionSet.non_existing_field
+          code: validation-failed
+        message: "field \"non_existing_field\" not found in type: 'User'"
+
+- description: query the remote with a non-existing field in an interface type
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+      {
+        hero(episode: 4) {
+          id
+          name
+          ... on Droid {
+            id
+            name
+            primaryFunction
+            non_existing_field
+          }
+        }
+      }
+  response:
+    errors:
+      - extensions:
+          path: $.selectionSet.hero.selectionSet.non_existing_field
+          code: validation-failed
+        message: "field \"non_existing_field\" not found in type: 'Droid'"
+
+- description: query the remote with a non-existing field in an union type
+  url: /v1/graphql
+  status: 200
+  query:
+    query: |
+      {
+         search(episode: 2) {
+           __typename
+           ... on Droid {
+             id
+             name
+           }
+           ... on Human {
+             id
+             name
+             non_existing_field
+           }
+         }
+      }
+  response:
+    errors:
+      - extensions:
+          path: $.selectionSet.search.selectionSet.non_existing_field
+          code: validation-failed
+        message: "field \"non_existing_field\" not found in type: 'Human'"
--- a/server/tests-py/queries/remote_schemas/validation/setup.yaml
+++ b/server/tests-py/queries/remote_schemas/validation/setup.yaml
@ -0,0 +1,19 @@
+type: bulk
+args:
+- type: add_remote_schema
+  args:
+    name: user
+    definition:
+      url: http://localhost:5000/user-graphql
+
+- type: add_remote_schema
+  args:
+    name: character-iface
+    definition:
+      url: http://localhost:5000/character-iface-graphql
+
+- type: add_remote_schema
+  args:
+    name: union
+    definition:
+      url: http://localhost:5000/union-graphql
--- a/server/tests-py/queries/remote_schemas/validation/teardown.yaml
+++ b/server/tests-py/queries/remote_schemas/validation/teardown.yaml
@ -0,0 +1,13 @@
+type: bulk
+args:
+- type: remove_remote_schema
+  args:
+    name: user
+
+- type: remove_remote_schema
+  args:
+    name: character-iface
+
+- type: remove_remote_schema
+  args:
+    name: union
--- a/server/tests-py/test_schema_stitching.py
+++ b/server/tests-py/test_schema_stitching.py
@ -12,6 +12,7 @@ import time
 import pytest

 from validate import check_query_f, check_query
+from graphql import GraphQLError

 def mk_add_remote_q(name, url, headers=None, client_hdrs=False, timeout=None):
    return {
@ -354,7 +355,7 @@ class TestRemoteSchemaQueriesOverWebsocket:
        query = """
        query {
          user(id: 2) {
-            blah
+            generateError
            username
          }
        }
@ -368,7 +369,7 @@ class TestRemoteSchemaQueriesOverWebsocket:
            assert ev['type'] == 'data' and ev['id'] == query_id, ev
            assert 'errors' in ev['payload']
            assert ev['payload']['errors'][0]['message'] == \
-                'Cannot query field "blah" on type "User".'
+                'Cannot query field "generateError" on type "User".'
        finally:
            ws_client.stop(query_id)

@ -598,3 +599,20 @@ class TestRemoteSchemaReload:
        # Delete remote schema
        st_code, resp = hge_ctx.v1q(mk_delete_remote_q('simple 1'))
        assert st_code == 200, resp
+
+@pytest.mark.usefixtures('per_class_tests_db_state')
+class TestValidateRemoteSchemaQuery:
+
+    @classmethod
+    def dir(cls):
+        return "queries/remote_schemas/validation/"
+
+    def test_remote_schema_argument_validation(self, hge_ctx):
+        """ test to check that the graphql-engine throws an validation error
+            when an remote object is queried with an unknown argument  """
+        check_query_f(hge_ctx, self.dir() + '/argument_validation.yaml')
+
+    def test_remote_schema_field_validation(self, hge_ctx):
+        """ test to check that the graphql-engine throws an validation error
+            when an remote object is queried with an unknown field  """
+        check_query_f(hge_ctx, self.dir() + '/field_validation.yaml')