hasura/graphql-engine
1
0
Fork 0
mirror of https://github.com/hasura/graphql-engine.git synced 2024-12-13 19:33:55 +03:00
Code Issues Projects Releases Wiki Activity

server: fix empty remote input objects (fixes #6029, #6703)

Browse Source 
GitOrigin-RevId: 3c474ee85b5d1271abfc8848e29ae1d3be28ff63
This commit is contained in:
Antoine Leblanc 2021-04-21 02:16:08 +01:00 committed by hasura-bot
parent 703928de9f
commit 84ed74aba1
11 changed files with 164 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
CHANGELOG.md
View File

@ -1,7 +1,6 @@
# Hasura GraphQL Engine Changelog
## Next release
(Add entries here in the order of: server, console, cli, docs, others)
### Support comparing columns across related tables in permission's boolean expressions
@ -14,6 +13,9 @@ only when there are enough present in the items inventory.
### Bug fixes and improvements
(Add entries here in the order of: server, console, cli, docs, others)
- server: fix a bug in remote schema permissions that could result in an invalid GraphQL schema (fix #6029, #6703)
- server: support query multiplexing in MSSQL subscriptions
- console: add bigquery support (#1000)
- cli: add support for bigquery in metadata operations

6
scripts/dev.sh
View File

@ -420,8 +420,10 @@ elif [ "$MODE" = "test" ]; then
# Using --metadata-database-url flag to test multiple backends
cabal new-run --project-file=cabal.project.dev-sh -- exe:graphql-engine \
--metadata-database-url="$PG_DB_URL" serve --stringify-numeric-types \
--enable-console --console-assets-dir ../console/static/dist \
--metadata-database-url="$PG_DB_URL" serve \
--stringify-numeric-types \
--enable-console \
--console-assets-dir ../console/static/dist \
&> "$GRAPHQL_ENGINE_TEST_LOG" & GRAPHQL_ENGINE_PID=$!
echo -n "Waiting for graphql-engine"

4
server/src-lib/Hasura/GraphQL/Parser/Internal/Parser.hs
View File

@ -307,6 +307,10 @@ list parser = gcastWith (inputParserInput @k) Parser
where
schemaType = NonNullable $ TList $ pType parser
-- TODO: if we had an optional "strict" mode, we could (and should!) enforce
-- that `fieldName` isn't empty, which sadly can't be done at the type level.
-- This would prevent the creation of an object with no fields, which is against
-- the spec.
object
:: MonadParse m
=> Name

87
server/src-lib/Hasura/GraphQL/Schema/Remote.hs
View File

@ -376,16 +376,51 @@ remoteSchemaUnion schemaDoc defn@(G.UnionTypeDefinition description name _direct
" can only include object types. It cannot include " <> squote objectName
-- | remoteSchemaInputObject returns an input parser for a given 'G.InputObjectTypeDefinition'
--
-- Now, this is tricky! We are faced with two contradicting constraints here. On one hand, the
-- GraphQL spec forbids us from creating empty input objects. This means that if all the arguments
-- have presets, we CANNOT use the parser this function creates, and the caller cannot create a
-- field for this object (and instead should use @pure@ to include the preset values in the result
-- of parsing the fields).
--
-- One way we could fix this would be to change the type of this function to return a `Maybe
-- Parser`, inspect the result of 'argumentsParser', and return @Nothing@ when we realize that there
-- aren't any actual field in it (or at least return a value that propagates the preset values). But
-- this would contradict our second constraint: this function needs to be memoized!
--
-- At time of writing, we can't memoize functions that return arbitrary functors of Parsers; so no
-- memoizing Maybe Parser or Either Presets Parser. Which means that we would need to first call
-- `argumentsParser`, then memoize the "Just" branch that builds the actual Parser. The problem is
-- that the recursive call ro remoteSchemaInputObject is within 'argumentsParser', meaning the call
-- to it MUST be in the memoized branch!
--
-- This is why, in the end, we do the following: we first test whether there is any non-preset
-- field: if yes, we memoize that branch and proceed as normal. Otherwise we can omit the
-- memoization: we know for sure that the preset fields won't generate a recursive call!
remoteSchemaInputObject
:: forall n m
. (MonadSchema n m, MonadError QErr m)
=> RemoteSchemaIntrospection
-> G.InputObjectTypeDefinition RemoteSchemaInputValueDefinition
-> m (Parser 'Input n (Maybe (HashMap G.Name (Value RemoteSchemaVariable))))
-> m ( Either
(InputFieldsParser n (Maybe (HashMap G.Name (Value RemoteSchemaVariable))))
(Parser 'Input n (Maybe (HashMap G.Name (Value RemoteSchemaVariable))))
)
remoteSchemaInputObject schemaDoc defn@(G.InputObjectTypeDefinition desc name _ valueDefns) =
P.memoizeOn 'remoteSchemaInputObject defn do
argsParser <- argumentsParser valueDefns schemaDoc
pure $ P.object name desc $ argsParser
if all (isJust . _rsitdPresetArgument) valueDefns
then
-- All the fields are preset: we can't create a parser, that would result in an invalid type in
-- the schema (an input object with no field). We therefore forward the InputFieldsParser
-- unmodified. No need to memoize this branch: since all arguments are preset, 'argumentsParser'
-- won't be recursively calling this function.
Left <$> argumentsParser valueDefns schemaDoc
else
-- At least one field is not a preset, meaning we have the guarantee that there will be at least
-- one field in the input object. We have to memoize this branch as we might recursively call
-- the same parser.
Right <$> P.memoizeOn 'remoteSchemaInputObject defn do
argsParser <- argumentsParser valueDefns schemaDoc
pure $ P.object name desc $ argsParser
lookupType
:: RemoteSchemaIntrospection
@ -476,7 +511,7 @@ remoteFieldFromName
-> m (FieldParser n (Field NoFragments RemoteSchemaVariable))
remoteFieldFromName sdoc fieldName description fieldTypeName argsDefns =
case lookupType sdoc fieldTypeName of
Nothing -> throw400 RemoteSchemaError $ "Could not find type with name " <>> fieldName
Nothing -> throw400 RemoteSchemaError $ "Could not find type with name " <>> fieldTypeName
Just typeDef -> remoteField sdoc fieldName description argsDefns typeDef
-- | 'inputValueDefinitionParser' accepts a 'G.InputValueDefinition' and will return an
@ -541,7 +576,31 @@ inputValueDefinitionParser schemaDoc (G.InputValueDefinition desc name fieldType
pure $ fieldConstructor' $ doNullability nullability $ remoteFieldEnumParser defn $> Nothing
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
G.TypeDefinitionInputObject defn -> do
fieldConstructor' . doNullability nullability <$> remoteSchemaInputObject schemaDoc defn
potentialObject <- remoteSchemaInputObject schemaDoc defn
pure $ case potentialObject of
Left dummyInputFieldsParser -> do
-- We couln't create a parser, meaning we can't create a field for this
-- object. Instead we must return a "pure" InputFieldsParser that always yields
-- the needed result without containing a field definition.
--
-- !!! WARNING #1 !!!
-- Since we have no input field in the schema for this field, we can't make the
-- distinction between it being actually present at parsing time or not. We
-- therefore choose to behave as if it was always present, and we always
-- include the preset values in the result.
--
-- !!! WARNING #2 !!!
-- We are re-using an 'InputFieldsParser' that was created earlier! Won't that
-- create new fields in the current context? No, it won't, but only because in
-- this case we know that it was created from the preset fields in
-- 'argumentsParser', and therefore contains no field definition.
let dummyInputValue = Just $ GraphQLValue $ G.VObject mempty
dummyInputFieldsParser <&> \presets ->
(dummyInputValue, (Map.singleton name . G.VObject) <$> presets)
Right actualParser -> do
-- We're in the normal case: we do have a parser for the input object, which is
-- therefore valid (non-empty).
fieldConstructor' $ doNullability nullability actualParser
G.TypeDefinitionUnion _ -> throw400 RemoteSchemaError "expected input type, but got output type"
G.TypeDefinitionInterface _ -> throw400 RemoteSchemaError "expected input type, but got output type"
G.TypeList nullability subType ->
@ -593,6 +652,22 @@ argumentsParser
-> RemoteSchemaIntrospection
-> m (InputFieldsParser n (Maybe (HashMap G.Name (Value RemoteSchemaVariable))))
argumentsParser args schemaDoc = do
-- ! DANGER !
--
-- This function is mutually recursive with 'inputValueDefinitionParser': if one of the non-preset
-- arguments is an input object, then recursively we'll end up using 'argumentsParser' to parse
-- its arguments. Note however that if there is no "nonPresetArgs", meaning that all the arguments
-- have a preset value, then this function will not call 'inputValueDefinitionParser', and will
-- simply return without any recursion.
--
-- This is labelled as dangerous because another function in this module,
-- 'remoteSchemaInputObject', EXPLICITLY RELIES ON THIS BEHAVIOUR. Due to limitations of the
-- GraphQL spec and of parser memoization functions, it cannot memoize the case where all
-- arguments are preset, and therefore relies on the assumption that 'argumentsParser' is not
-- recursive in this edge case.
--
-- This assumptions is unlikely to ever be broken; but if you ever modify this function, please
-- nonetheless make sure that it is maintained.
nonPresetArgsParser <- sequenceA <$> for nonPresetArgs (inputValueDefinitionParser schemaDoc)
let nonPresetArgsParser' = (fmap . fmap) snd nonPresetArgsParser
pure $ mkPresets <$> nonPresetArgsParser'

2
server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs
View File

@ -82,7 +82,7 @@ errorToText = \case
InvalidGraphQLName t ->
t <<> " is not a valid GraphQL identifier"
IDTypeJoin typeName ->
"Only ID, Int, uuid or String scalar types can be joined to the ID type, but recieved " <>> typeName
"Only ID, Int, uuid or String scalar types can be joined to the ID type, but received " <>> typeName
-- | Validate a remote relationship given a context.
validateRemoteRelationship

34
server/src-lib/Hasura/RQL/DDL/RemoteSchema/Permission.hs
View File

@ -40,17 +40,17 @@ module Hasura.RQL.DDL.RemoteSchema.Permission (
import Hasura.Prelude
import Control.Monad.Validate
import qualified Data.HashMap.Strict as Map
import qualified Data.HashSet as S
import qualified Data.List.NonEmpty as NE
import Data.List.Extended (duplicates, getDifference)
import qualified Data.Text as T
import qualified Data.HashMap.Strict as Map
import qualified Data.HashSet as S
import Data.List.Extended (duplicates, getDifference)
import qualified Data.List.NonEmpty as NE
import qualified Data.Text as T
import Data.Text.Extended
import qualified Language.GraphQL.Draft.Syntax as G
import qualified Language.GraphQL.Draft.Syntax as G
import Hasura.RQL.Types hiding (GraphQLType, defaultScalars)
import Hasura.Server.Utils (englishList, isSessionVariable)
import Hasura.GraphQL.Schema.Remote
import Hasura.RQL.Types hiding (GraphQLType, defaultScalars)
import Hasura.Server.Utils (englishList, isSessionVariable)
import Hasura.Session
data FieldDefinitionType
@ -260,12 +260,12 @@ showRoleBasedSchemaValidationError :: RoleBasedSchemaValidationError -> Text
showRoleBasedSchemaValidationError = \case
NonMatchingType fldName fldType expectedType providedType ->
"expected type of " <> dquote fldName <> "(" <> dquote fldType <> ")" <>" to be " <>
(G.showGT expectedType) <> " but recieved " <> (G.showGT providedType)
(G.showGT expectedType) <> " but received " <> (G.showGT providedType)
TypeDoesNotExist graphQLType typeName ->
graphQLType <<> ": " <> typeName <<> " does not exist in the upstream remote schema"
NonMatchingDefaultValue inpObjName inpValName expectedVal providedVal ->
"expected default value of input value: " <> inpValName <<> "of input object "
<> inpObjName <<> " to be " <> defaultValueToText expectedVal <> " but recieved "
<> inpObjName <<> " to be " <> defaultValueToText expectedVal <> " but received "
<> defaultValueToText providedVal
NonExistingInputArgument inpObjName inpArgName ->
"input argument " <> inpArgName <<> " does not exist in the input object:" <>> inpObjName
@ -378,13 +378,13 @@ lookupInputType (G.SchemaDocument types) name = go types
case typeDef of
G.TypeDefinitionScalar (G.ScalarTypeDefinition _ scalarName _) ->
if | name == scalarName -> Just $ PresetScalar scalarName
| otherwise -> go tps
| otherwise -> go tps
G.TypeDefinitionEnum (G.EnumTypeDefinition _ enumName _ vals) ->
if | name == enumName -> Just $ PresetEnum enumName $ map G._evdName vals
| otherwise -> go tps
| otherwise -> go tps
G.TypeDefinitionInputObject (G.InputObjectTypeDefinition _ inpObjName _ vals) ->
if | name == inpObjName -> Just $ PresetInputObject vals
| otherwise -> go tps
| otherwise -> go tps
_ -> go tps
go [] = Nothing
@ -430,7 +430,7 @@ parsePresetValue gType varName isStatic value = do
case value of
enumVal@(G.VEnum e) ->
case e `elem` enumVals of
True -> pure $ G.literal enumVal
True -> pure $ G.literal enumVal
False -> refute $ pure $ EnumValueNotFound typeName $ G.unEnumValue e
G.VString t ->
case isSessionVariable t of
@ -460,7 +460,7 @@ parsePresetValue gType varName isStatic value = do
-- a type `[Int]` or `[[Int]]`
s'@(G.VString s) ->
case isSessionVariable s of
True -> refute $ pure $ DisallowSessionVarForListType varName
True -> refute $ pure $ DisallowSessionVarForListType varName
False -> parsePresetValue gType' varName isStatic s'
v -> parsePresetValue gType' varName isStatic v
@ -481,9 +481,9 @@ parsePresetDirective gType parentArgName (G.Directive name args) = do
refute $ pure $ InvalidPresetArgument parentArgName
isStatic <-
case (Map.lookup $$(G.litName "static") args) of
Nothing -> pure False
Nothing -> pure False
(Just (G.VBoolean b)) -> pure b
_ -> refute $ pure $ InvalidStaticValue
_ -> refute $ pure $ InvalidStaticValue
parsePresetValue gType parentArgName isStatic val
-- | validateDirective checks if the arguments of a given directive

11
server/tests-py/queries/remote_schemas/permissions/argument_presets/add_permission_with_static_preset_argument.yaml
View File

@ -37,6 +37,16 @@ args:
eq : String
}
type Photo {
height : Int
width : Int
}
input Dimensions {
height : Int @preset(value: 42)
width : Int @preset(value: 101)
}
type Query {
hello: String
messages(where: MessageWhereInpObj @preset(value: {id: {eq: 1}})): [Message]
@ -44,6 +54,7 @@ args:
users(user_ids: [Int]!): [User]
message(id: Int!) : Message
communications(id: Int): [Communication]
profilePicture(dimensions: Dimensions): Photo
}
schema {

28
server/tests-py/queries/remote_schemas/permissions/argument_presets/execution_with_static_preset_args.yaml
View File

@ -90,3 +90,31 @@
userMessages:
- id: 2
msg: You lose!
- description: "query in which one argument has all fields preset; also check that the type does not exist"
url: /v1/graphql
status: 200
headers:
X-Hasura-Role: user
query:
query: |
{
__type(name: "Dimensions") {
name
fields {
name
type {
name
}
}
}
profilePicture {
width
}
}
response:
data:
__type:
profilePicture:
width: 101

4
server/tests-py/queries/remote_schemas/permissions/role_based_schema_input_object_validation.yaml
View File

@ -31,7 +31,7 @@ response:
path: $.args
error:
"validation for the given role-based schema failed for the following reasons:\n\
\ • expected type of \"eq\"(\"Input object argument\") to be Int but recieved\
\ • expected type of \"eq\"(\"Input object argument\") to be Int but received\
\ Boolean\n • expected type of \"gt\"(\"Input object argument\") to be Int but\
\ recieved Boolean\n"
\ received Boolean\n"
code: validation-failed

2
server/tests-py/queries/remote_schemas/permissions/role_based_schema_object_validation.yaml
View File

@ -26,7 +26,7 @@
path: $.args
error:
"validation for the given role-based schema failed for the following reasons:\n\
\ • expected type of \"user_id\"(\"Object field\") to be Int but recieved Int!\n\
\ • expected type of \"user_id\"(\"Object field\") to be Int but received Int!\n\
\ • field \"created_at\" does not exist in the \"Object\": \"User\"\n"
code: validation-failed

12
server/tests-py/remote_schemas/nodejs/remote_schema_perms.js
View File

@ -61,6 +61,11 @@ const typeDefs = gql`
age: Int
}
input Dimensions {
height: Int
width: Int
}
type Photo {
height: Int
width: Int
@ -84,6 +89,7 @@ const typeDefs = gql`
users(user_ids: [Int]!): [User]
message(id: Int!) : Message
communications(id: Int): [Communication]
profilePicture(dimensions: Dimensions): Photo
}
`;
@ -151,6 +157,9 @@ const resolvers = {
}
},
Photo: {
},
Query: {
hello: () => "world",
message: (_, { id }) => {
@ -218,6 +227,9 @@ const resolvers = {
}
return result;
},
profilePicture: (_, { dimensions }) => {
return dimensions
},
},
Communication: {
__resolveType(communication, context, info){