fix: GQL syntax strings are incorrectly validated as enums (#1134)

core/src/main/scala/caliban/GraphQL.scala

@@ -7,7 +7,7 @@ import caliban.introspection.Introspector
 import caliban.introspection.adt._
 import caliban.parsing.adt.Definition.TypeSystemDefinition.SchemaDefinition
 import caliban.parsing.adt.{ Document, OperationType }
-import caliban.parsing.{ Parser, SourceMapper }
+import caliban.parsing.{ Parser, SourceMapper, VariablesUpdater }
 import caliban.schema._
 import caliban.validation.Validator
 import caliban.wrappers.Wrapper
@@ -94,21 +94,23 @@ trait GraphQL[-R] { self =>
             case (overallWrappers, parsingWrappers, validationWrappers, executionWrappers, fieldWrappers, _) =>
               wrap((request: GraphQLRequest) =>
                 (for {
-                  doc              <- wrap(Parser.parseQuery)(parsingWrappers, request.query.getOrElse(""))
-                  intro             = Introspector.isIntrospection(doc)
-                  _                <- IO.when(intro && !enableIntrospection) {
-                                        IO.fail(CalibanError.ValidationError("Introspection is disabled", ""))
-                                      }
-                  typeToValidate    = if (intro) introspectionRootType else rootType
-                  schemaToExecute   = if (intro) introspectionRootSchema else schema
+                  doc            <- wrap(Parser.parseQuery)(parsingWrappers, request.query.getOrElse(""))
+                  intro           = Introspector.isIntrospection(doc)
+                  _              <- IO.when(intro && !enableIntrospection) {
+                                      IO.fail(CalibanError.ValidationError("Introspection is disabled", ""))
+                                    }
+                  typeToValidate  = if (intro) introspectionRootType else rootType
+                  schemaToExecute = if (intro) introspectionRootSchema else schema
+                  updatedRequest  = VariablesUpdater.updateVariables(request, doc, typeToValidate)
+
                   validate          = (doc: Document) =>
                                         Validator
                                           .prepare(
                                             doc,
                                             typeToValidate,
                                             schemaToExecute,
-                                            request.operationName,
-                                            request.variables.getOrElse(Map()),
+                                            updatedRequest.operationName,
+                                            updatedRequest.variables.getOrElse(Map.empty),
                                             skipValidation
                                           )
                   executionRequest <- wrap(validate)(validationWrappers, doc)

core/src/main/scala/caliban/execution/Field.scala

@@ -109,40 +109,12 @@ object Field {
             definition  <- variableDefinitions.find(_.name == name)
             defaultValue = definition.defaultValue getOrElse NullValue
             value        = variableValues.getOrElse(name, defaultValue)
-          } yield resolveEnumValues(value, definition, rootType)) getOrElse NullValue
+          } yield value) getOrElse NullValue
         case value: Value                   => value
       }
     arguments.map { case (k, v) => k -> resolveVariable(v) }
   }
 
-  // Since we cannot separate a String from an Enum when variables
-  // are parsed, we need to translate from strings to enums here
-  // if we have a valid enum field.
-  private def resolveEnumValues(
-    value: InputValue,
-    definition: VariableDefinition,
-    rootType: RootType
-  ): InputValue = {
-    val t = Type
-      .innerType(definition.variableType)
-
-    rootType.types
-      .get(t)
-      .map(_.kind)
-      .flatMap { kind =>
-        (kind, value) match {
-          case (__TypeKind.ENUM, InputValue.ListValue(v)) =>
-            Some(
-              InputValue.ListValue(v.map(resolveEnumValues(_, definition, rootType)))
-            )
-          case (__TypeKind.ENUM, Value.StringValue(v))    =>
-            Some(Value.EnumValue(v))
-          case _                                          => None
-        }
-      }
-      .getOrElse(value)
-  }
-
   private def subtypeNames(typeName: String, rootType: RootType): Option[List[String]] =
     rootType.types
       .get(typeName)

core/src/main/scala/caliban/parsing/VariablesUpdater.scala

@@ -0,0 +1,53 @@
+package caliban.parsing
+
+import caliban.GraphQLRequest
+import caliban.introspection.adt._
+import caliban.parsing.adt.Definition.ExecutableDefinition.OperationDefinition
+import caliban.parsing.adt._
+import caliban.schema.RootType
+import caliban.{ InputValue, Value }
+import zio.{ IO, UIO }
+
+object VariablesUpdater {
+  def updateVariables(
+    req: GraphQLRequest,
+    doc: Document,
+    rootType: RootType
+  ): GraphQLRequest = {
+    val variableDefinitions = doc.operationDefinitions.flatMap(_.variableDefinitions)
+    val updated             = req.variables.getOrElse(Map.empty).map { case (key, value) =>
+      val v = variableDefinitions.find(_.name == key).map(resolveEnumValues(value, _, rootType)).getOrElse(value)
+      key -> v
+    }
+
+    req.copy(variables = Some(updated))
+  }
+
+  // Since we cannot separate a String from an Enum when variables
+  // are parsed, we need to translate from strings to enums here
+  // if we have a valid enum field.
+  private def resolveEnumValues(
+    value: InputValue,
+    definition: VariableDefinition,
+    rootType: RootType
+  ): InputValue = {
+    val t = Type
+      .innerType(definition.variableType)
+
+    rootType.types
+      .get(t)
+      .map(_.kind)
+      .flatMap { kind =>
+        (kind, value) match {
+          case (__TypeKind.ENUM, InputValue.ListValue(v)) =>
+            Some(
+              InputValue.ListValue(v.map(resolveEnumValues(_, definition, rootType)))
+            )
+          case (__TypeKind.ENUM, Value.StringValue(v))    =>
+            Some(Value.EnumValue(v))
+          case _                                          => None
+        }
+      }
+      .getOrElse(value)
+  }
+}

core/src/main/scala/caliban/validation/ValueValidator.scala

@@ -81,11 +81,9 @@ object ValueValidator {
             }
           case ENUM         =>
             argValue match {
-              case EnumValue(value)   =>
+              case EnumValue(value) =>
                 validateEnum(value, inputType, errorContext)
-              case StringValue(value) =>
-                validateEnum(value, inputType, errorContext)
-              case _                  =>
+              case _                =>
                 failValidation(
                   s"$errorContext has invalid type: $argValue",
                   "Input field was supposed to be an enum value."

core/src/test/scala/caliban/execution/FieldArgsSpec.scala

@@ -6,6 +6,7 @@ import caliban.Value.EnumValue
 import zio._
 import zio.test._
 import zio.test.environment.TestEnvironment
+import zio.test.Assertion._
 
 object FieldArgsSpec extends DefaultRunnableSpec {
   sealed trait COLOR
@@ -155,6 +156,27 @@ object FieldArgsSpec extends DefaultRunnableSpec {
             )
           )
       )
+    },
+    testM("it doesn't allow strings as enums in GQL syntax") {
+      case class QueryInput(color: COLOR)
+      case class Query(query: QueryInput => UIO[String])
+      val query =
+        """query {
+          |  query(color: "BLUE")
+          |}""".stripMargin
+
+      val api = graphQL(
+        RootResolver(
+          Query(
+            query = i => ZIO.succeed(i.toString)
+          )
+        )
+      )
+
+      for {
+        interpreter <- api.interpreter
+        res         <- interpreter.execute(query)
+      } yield assert(res.errors.headOption)(isSome(anything))
     }
   )
 }