From 3ea611f9fd8f8b4425221f9ae299b51dce4bccec Mon Sep 17 00:00:00 2001 From: Karthikeyan Chinnakonda Date: Tue, 13 Oct 2020 14:03:11 +0530 Subject: [PATCH] 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 Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .circleci/server-upgrade-downgrade/run.sh | 1 + CHANGELOG.md | 1 + server/src-lib/Hasura/GraphQL/Context.hs | 2 +- .../src-lib/Hasura/GraphQL/Execute/Remote.hs | 16 +- server/src-lib/Hasura/GraphQL/Parser.hs | 2 + .../Hasura/GraphQL/Parser/Internal/Parser.hs | 40 ++- server/src-lib/Hasura/GraphQL/Schema.hs | 16 +- .../src-lib/Hasura/GraphQL/Schema/Remote.hs | 268 +++++++++++++----- .../src-lib/Hasura/RQL/Types/SchemaCache.hs | 8 +- server/tests-py/graphql_server.py | 28 ++ .../tests-py/queries/heterogeneous/basic.yaml | 7 +- .../validation/argument_validation.yaml | 55 ++++ .../validation/field_validation.yaml | 68 +++++ .../remote_schemas/validation/setup.yaml | 19 ++ .../remote_schemas/validation/teardown.yaml | 13 + server/tests-py/test_schema_stitching.py | 22 +- 16 files changed, 458 insertions(+), 108 deletions(-) create mode 100644 server/tests-py/queries/remote_schemas/validation/argument_validation.yaml create mode 100644 server/tests-py/queries/remote_schemas/validation/field_validation.yaml create mode 100644 server/tests-py/queries/remote_schemas/validation/setup.yaml create mode 100644 server/tests-py/queries/remote_schemas/validation/teardown.yaml diff --git a/.circleci/server-upgrade-downgrade/run.sh b/.circleci/server-upgrade-downgrade/run.sh index 4a37477a3fbc8..2ed26697b43fd 100755 --- 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 index 7d4c4d17ffe80..6af112632c0b3 100644 --- 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 index f5ba13b392717..3e12b679fe72c 100644 --- 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 index e412e1dddcee8..852d8f4ea7140 100644 --- 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 index 009972f48572b..0c78f41b80eb9 100644 --- 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 index 86d41a15b64df..e5947a1017a37 100644 --- 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 diff --git a/server/src-lib/Hasura/GraphQL/Schema.hs b/server/src-lib/Hasura/GraphQL/Schema.hs index 5ad02f0352cac..a2f468ef80085 100644 --- 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 diff --git a/server/src-lib/Hasura/GraphQL/Schema/Remote.hs b/server/src-lib/Hasura/GraphQL/Schema/Remote.hs index 6dbe227d0dcf8..b957f937cf81e 100644 --- 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 diff --git a/server/src-lib/Hasura/RQL/Types/SchemaCache.hs b/server/src-lib/Hasura/RQL/Types/SchemaCache.hs index 2996e8145cc0e..944705f949bbf 100644 --- 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 diff --git a/server/tests-py/graphql_server.py b/server/tests-py/graphql_server.py index 40098f89248e5..ef2f9afbd83d6 100644 --- a/server/tests-py/graphql_server.py +++ b/server/tests-py/graphql_server.py @@ -12,6 +12,8 @@ import time +from graphql import GraphQLError + HGE_URLS=[] def mkJSONResp(graphql_result): @@ -53,6 +55,8 @@ def post(self, request): 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 @@ def resolve_id(self, info): 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 @@ def get_by_id(_id): 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 @@ def resolve_allUsers(self, info): class UserMutation(graphene.ObjectType): createUser = CreateUser.Field() + createUserInputObj = CreateUserInputObject.Field() + user_schema = graphene.Schema(query=UserQuery, mutation=UserMutation) class UserGraphQL(RequestHandler): diff --git a/server/tests-py/queries/heterogeneous/basic.yaml b/server/tests-py/queries/heterogeneous/basic.yaml index 6a97b3d6ae3ab..6ab3d560bab84 100644 --- 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 diff --git a/server/tests-py/queries/remote_schemas/validation/argument_validation.yaml b/server/tests-py/queries/remote_schemas/validation/argument_validation.yaml new file mode 100644 index 0000000000000..da5c1ff00081f --- /dev/null +++ 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'" diff --git a/server/tests-py/queries/remote_schemas/validation/field_validation.yaml b/server/tests-py/queries/remote_schemas/validation/field_validation.yaml new file mode 100644 index 0000000000000..48ce474dee412 --- /dev/null +++ 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'" diff --git a/server/tests-py/queries/remote_schemas/validation/setup.yaml b/server/tests-py/queries/remote_schemas/validation/setup.yaml new file mode 100644 index 0000000000000..4b7acbf9af3c2 --- /dev/null +++ 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 diff --git a/server/tests-py/queries/remote_schemas/validation/teardown.yaml b/server/tests-py/queries/remote_schemas/validation/teardown.yaml new file mode 100644 index 0000000000000..4bb57fda0de67 --- /dev/null +++ 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 diff --git a/server/tests-py/test_schema_stitching.py b/server/tests-py/test_schema_stitching.py index eaad62dddbdec..0201ddd97abad 100644 --- a/server/tests-py/test_schema_stitching.py +++ b/server/tests-py/test_schema_stitching.py @@ -12,6 +12,7 @@ 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 @@ def test_remote_query_error(self, ws_client): query = """ query { user(id: 2) { - blah + generateError username } } @@ -368,7 +369,7 @@ def test_remote_query_error(self, ws_client): 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 @@ def test_inconsistent_remote_schema_reload_metadata(self, gql_server, hge_ctx): # 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')