Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate enum values in input variables #4753

Merged
merged 13 commits into from
May 2, 2024
5 changes: 5 additions & 0 deletions .changesets/fix_geal_input_enum_validation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
### Validate enum values in input variables ([Issue #4633](https://github.com/apollographql/router/issues/4633))

The Router will now validate enum values provided in JSON variables. The error message fomr input variables validation is changed from `VALIDATION_INVALID_TYPE_VARIABLE` to `GRAPHQL_VALIDATION_FAILED`.

By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/4753
2 changes: 1 addition & 1 deletion apollo-router/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ impl FetchError {
impl ErrorExtension for FetchError {
fn extension_code(&self) -> String {
match self {
FetchError::ValidationInvalidTypeVariable { .. } => "VALIDATION_INVALID_TYPE_VARIABLE",
FetchError::ValidationInvalidTypeVariable { .. } => "GRAPHQL_VALIDATION_FAILED",
Geal marked this conversation as resolved.
Show resolved Hide resolved
FetchError::ValidationPlanningError { .. } => "VALIDATION_PLANNING_ERROR",
FetchError::SubrequestMalformedResponse { .. } => "SUBREQUEST_MALFORMED_RESPONSE",
FetchError::SubrequestUnexpectedPatchResponse { .. } => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
source: apollo-router/src/services/supergraph/tests.rs
expression: response
---
{
"errors": [
{
"message": "invalid type for variable: 'input'",
"extensions": {
"name": "input",
"code": "GRAPHQL_VALIDATION_FAILED"
}
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
source: apollo-router/src/services/supergraph/tests.rs
expression: response
---
{
"errors": [
{
"message": "value `C` does not exist on `InputEnum`",
"locations": [
{
"line": 1,
"column": 21
}
],
"extensions": {
"code": "GRAPHQL_VALIDATION_FAILED"
}
}
]
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
source: apollo-router/src/services/supergraph_service.rs
source: apollo-router/src/services/supergraph/tests.rs
expression: stream.next_response().await.unwrap()
---
{
Expand All @@ -8,7 +8,7 @@ expression: stream.next_response().await.unwrap()
"message": "invalid type for variable: 'userId'",
"extensions": {
"name": "userId",
"code": "VALIDATION_INVALID_TYPE_VARIABLE"
"code": "GRAPHQL_VALIDATION_FAILED"
}
}
]
Expand Down
81 changes: 81 additions & 0 deletions apollo-router/src/services/supergraph/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3549,3 +3549,84 @@ async fn abstract_types_in_requires() {
let mut stream = service.oneshot(request).await.unwrap();
insta::assert_json_snapshot!(stream.next_response().await.unwrap());
}

const ENUM_SCHEMA: &str = r#"schema
@core(feature: "https://specs.apollo.dev/core/v0.1")
@core(feature: "https://specs.apollo.dev/join/v0.1")
@core(feature: "https://specs.apollo.dev/inaccessible/v0.1")
{
query: Query
}
directive @core(feature: String!) repeatable on SCHEMA
directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION
directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE
directive @join__owner(graph: join__Graph!) on OBJECT | INTERFACE
directive @join__graph(name: String!, url: String!) on ENUM_VALUE
directive @inaccessible on OBJECT | FIELD_DEFINITION | INTERFACE | UNION
scalar join__FieldSet
enum join__Graph {
USER @join__graph(name: "user", url: "http://localhost:4001/graphql")
ORGA @join__graph(name: "orga", url: "http://localhost:4002/graphql")
}
type Query {
test(input: InputEnum): String @join__field(graph: USER)
}

enum InputEnum {
A
B
}"#;

#[tokio::test]
async fn invalid_input_enum() {
/*let subgraphs = MockedSubgraphs([
("user", MockSubgraph::builder().with_json(
serde_json::json!{{"query":"{currentUser{activeOrganization{__typename id}}}"}},
serde_json::json!{{"data": {"currentUser": { "activeOrganization": null }}}}
).build()),
("orga", MockSubgraph::default())
].into_iter().collect());*/

Geal marked this conversation as resolved.
Show resolved Hide resolved
let service = TestHarness::builder()
.configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true } }))
.unwrap()
.schema(ENUM_SCHEMA)
//.extra_plugin(subgraphs)
.build_supergraph()
.await
.unwrap();

let request = supergraph::Request::fake_builder()
.query("query { test(input: C) }")
.context(defer_context())
// Request building here
.build()
.unwrap();
let response = service
.clone()
.oneshot(request)
.await
.unwrap()
.next_response()
.await
.unwrap();

insta::assert_json_snapshot!(response);

let request = supergraph::Request::fake_builder()
.query("query($input: InputEnum) { test(input: $input) }")
.variable("input", "INVALID")
.context(defer_context())
// Request building here
.build()
.unwrap();
let response = service
.oneshot(request)
.await
.unwrap()
.next_response()
.await
.unwrap();

insta::assert_json_snapshot!(response);
}
9 changes: 4 additions & 5 deletions apollo-router/src/spec/field_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,10 @@ fn validate_input_value(
// Custom scalar: accept any JSON value
(schema::ExtendedType::Scalar(_), _) => Ok(()),

// TODO: check enum value?
// (schema::ExtendedType::Enum(def), Value::String(s)) => {
// from_bool(def.values.contains_key(s))
// },
(schema::ExtendedType::Enum(_), _) => Ok(()),
(schema::ExtendedType::Enum(def), Value::String(s)) => {
from_bool(def.values.contains_key(s.as_str()))
}
(schema::ExtendedType::Enum(_), _) => Err(InvalidValue),

(schema::ExtendedType::InputObject(def), Value::Object(obj)) => {
// TODO: check keys in `obj` but not in `def.fields`?
Expand Down
4 changes: 2 additions & 2 deletions apollo-router/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,12 +663,12 @@ async fn missing_variables() {
let mut expected = vec![
graphql::Error::builder()
.message("invalid type for variable: 'missingVariable'")
.extension_code("VALIDATION_INVALID_TYPE_VARIABLE")
.extension_code("GRAPHQL_VALIDATION_FAILED")
.extension("name", "missingVariable")
.build(),
graphql::Error::builder()
.message("invalid type for variable: 'yetAnotherMissingVariable'")
.extension_code("VALIDATION_INVALID_TYPE_VARIABLE")
.extension_code("GRAPHQL_VALIDATION_FAILED")
.extension("name", "yetAnotherMissingVariable")
.build(),
];
Expand Down
Loading