From 91a36f4af93f54c418e73419039a5930240e3b26 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 5 Mar 2024 18:15:59 +0100 Subject: [PATCH 1/8] Validate enum values in input variables --- apollo-router/src/error.rs | 2 +- ...pergraph__tests__invalid_input_enum-2.snap | 15 ++++ ...supergraph__tests__invalid_input_enum.snap | 20 +++++ .../src/services/supergraph/tests.rs | 81 +++++++++++++++++++ apollo-router/src/spec/field_type.rs | 9 +-- 5 files changed, 121 insertions(+), 6 deletions(-) create mode 100644 apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum-2.snap create mode 100644 apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum.snap diff --git a/apollo-router/src/error.rs b/apollo-router/src/error.rs index 6ffba0fdc3..e7659cc6cd 100644 --- a/apollo-router/src/error.rs +++ b/apollo-router/src/error.rs @@ -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", FetchError::ValidationPlanningError { .. } => "VALIDATION_PLANNING_ERROR", FetchError::SubrequestMalformedResponse { .. } => "SUBREQUEST_MALFORMED_RESPONSE", FetchError::SubrequestUnexpectedPatchResponse { .. } => { diff --git a/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum-2.snap b/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum-2.snap new file mode 100644 index 0000000000..423386bb22 --- /dev/null +++ b/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum-2.snap @@ -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" + } + } + ] +} diff --git a/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum.snap b/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum.snap new file mode 100644 index 0000000000..15ed4468d4 --- /dev/null +++ b/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum.snap @@ -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" + } + } + ] +} diff --git a/apollo-router/src/services/supergraph/tests.rs b/apollo-router/src/services/supergraph/tests.rs index c89bc0ef5e..16b4221b77 100644 --- a/apollo-router/src/services/supergraph/tests.rs +++ b/apollo-router/src/services/supergraph/tests.rs @@ -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());*/ + + 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); +} diff --git a/apollo-router/src/spec/field_type.rs b/apollo-router/src/spec/field_type.rs index 771a6bfd25..1c2034a75b 100644 --- a/apollo-router/src/spec/field_type.rs +++ b/apollo-router/src/spec/field_type.rs @@ -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`? From ee43138d36d97ed4d271db0004c6e41e145f7a58 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 5 Mar 2024 18:22:52 +0100 Subject: [PATCH 2/8] fix tests --- ...er__services__supergraph__tests__query_reconstruction.snap | 4 ++-- apollo-router/tests/integration_tests.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__query_reconstruction.snap b/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__query_reconstruction.snap index ff64cc1b5e..2863f93fdd 100644 --- a/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__query_reconstruction.snap +++ b/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__query_reconstruction.snap @@ -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() --- { @@ -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" } } ] diff --git a/apollo-router/tests/integration_tests.rs b/apollo-router/tests/integration_tests.rs index 98a97b3492..4a752d592b 100644 --- a/apollo-router/tests/integration_tests.rs +++ b/apollo-router/tests/integration_tests.rs @@ -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(), ]; From 1fed9b3cf7182c5311f00226a82ed61702d3eec1 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 8 Mar 2024 10:57:25 +0100 Subject: [PATCH 3/8] changeset --- .changesets/fix_geal_input_enum_validation.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changesets/fix_geal_input_enum_validation.md diff --git a/.changesets/fix_geal_input_enum_validation.md b/.changesets/fix_geal_input_enum_validation.md new file mode 100644 index 0000000000..e2d62418fb --- /dev/null +++ b/.changesets/fix_geal_input_enum_validation.md @@ -0,0 +1,5 @@ +### Validate enum values in input variables ([PR #4753](https://github.com/apollographql/router/pull/4753)) + +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 \ No newline at end of file From 80cc6a93df409247ad0f147f7fe5acebfc72de0b Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Fri, 8 Mar 2024 10:59:24 +0100 Subject: [PATCH 4/8] Update .changesets/fix_geal_input_enum_validation.md --- .changesets/fix_geal_input_enum_validation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changesets/fix_geal_input_enum_validation.md b/.changesets/fix_geal_input_enum_validation.md index e2d62418fb..18ab66e76b 100644 --- a/.changesets/fix_geal_input_enum_validation.md +++ b/.changesets/fix_geal_input_enum_validation.md @@ -1,4 +1,4 @@ -### Validate enum values in input variables ([PR #4753](https://github.com/apollographql/router/pull/4753)) +### 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`. From 1b4fb0506af9e3322531832b97fedaac37b58299 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Wed, 13 Mar 2024 10:33:30 +0100 Subject: [PATCH 5/8] change back to VALIDATION_INVALID_TYPE_VARIABLE --- apollo-router/src/error.rs | 2 +- ...er__services__supergraph__tests__invalid_input_enum-2.snap | 2 +- ...er__services__supergraph__tests__query_reconstruction.snap | 2 +- apollo-router/tests/integration_tests.rs | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/apollo-router/src/error.rs b/apollo-router/src/error.rs index e7659cc6cd..6ffba0fdc3 100644 --- a/apollo-router/src/error.rs +++ b/apollo-router/src/error.rs @@ -160,7 +160,7 @@ impl FetchError { impl ErrorExtension for FetchError { fn extension_code(&self) -> String { match self { - FetchError::ValidationInvalidTypeVariable { .. } => "GRAPHQL_VALIDATION_FAILED", + FetchError::ValidationInvalidTypeVariable { .. } => "VALIDATION_INVALID_TYPE_VARIABLE", FetchError::ValidationPlanningError { .. } => "VALIDATION_PLANNING_ERROR", FetchError::SubrequestMalformedResponse { .. } => "SUBREQUEST_MALFORMED_RESPONSE", FetchError::SubrequestUnexpectedPatchResponse { .. } => { diff --git a/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum-2.snap b/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum-2.snap index 423386bb22..40b8fa4908 100644 --- a/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum-2.snap +++ b/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum-2.snap @@ -8,7 +8,7 @@ expression: response "message": "invalid type for variable: 'input'", "extensions": { "name": "input", - "code": "GRAPHQL_VALIDATION_FAILED" + "code": "VALIDATION_INVALID_TYPE_VARIABLE" } } ] diff --git a/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__query_reconstruction.snap b/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__query_reconstruction.snap index 2863f93fdd..34f783cd01 100644 --- a/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__query_reconstruction.snap +++ b/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__query_reconstruction.snap @@ -8,7 +8,7 @@ expression: stream.next_response().await.unwrap() "message": "invalid type for variable: 'userId'", "extensions": { "name": "userId", - "code": "GRAPHQL_VALIDATION_FAILED" + "code": "VALIDATION_INVALID_TYPE_VARIABLE" } } ] diff --git a/apollo-router/tests/integration_tests.rs b/apollo-router/tests/integration_tests.rs index 4a752d592b..98a97b3492 100644 --- a/apollo-router/tests/integration_tests.rs +++ b/apollo-router/tests/integration_tests.rs @@ -663,12 +663,12 @@ async fn missing_variables() { let mut expected = vec![ graphql::Error::builder() .message("invalid type for variable: 'missingVariable'") - .extension_code("GRAPHQL_VALIDATION_FAILED") + .extension_code("VALIDATION_INVALID_TYPE_VARIABLE") .extension("name", "missingVariable") .build(), graphql::Error::builder() .message("invalid type for variable: 'yetAnotherMissingVariable'") - .extension_code("GRAPHQL_VALIDATION_FAILED") + .extension_code("VALIDATION_INVALID_TYPE_VARIABLE") .extension("name", "yetAnotherMissingVariable") .build(), ]; From c27edfdf7081523df2772d60b43fca1da4d13314 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Wed, 13 Mar 2024 10:34:13 +0100 Subject: [PATCH 6/8] update changeset --- .changesets/fix_geal_input_enum_validation.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changesets/fix_geal_input_enum_validation.md b/.changesets/fix_geal_input_enum_validation.md index 18ab66e76b..ebaf0beba1 100644 --- a/.changesets/fix_geal_input_enum_validation.md +++ b/.changesets/fix_geal_input_enum_validation.md @@ -1,5 +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`. +The Router will now validate enum values provided in JSON variables. By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/4753 \ No newline at end of file From d96a63c84bf549c1cb029ca70c42fcce453527ad Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Wed, 13 Mar 2024 10:35:03 +0100 Subject: [PATCH 7/8] Update apollo-router/src/services/supergraph/tests.rs --- apollo-router/src/services/supergraph/tests.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/apollo-router/src/services/supergraph/tests.rs b/apollo-router/src/services/supergraph/tests.rs index 16b4221b77..df22c291a4 100644 --- a/apollo-router/src/services/supergraph/tests.rs +++ b/apollo-router/src/services/supergraph/tests.rs @@ -3579,14 +3579,6 @@ const ENUM_SCHEMA: &str = r#"schema #[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());*/ - let service = TestHarness::builder() .configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true } })) .unwrap() From 31a149c7b30e7535cb7566000bc92545f3315274 Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Thu, 2 May 2024 12:06:03 +0200 Subject: [PATCH 8/8] update error message --- ...router__services__supergraph__tests__invalid_input_enum.snap | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum.snap b/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum.snap index 15ed4468d4..0e2dd5d122 100644 --- a/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum.snap +++ b/apollo-router/src/services/supergraph/snapshots/apollo_router__services__supergraph__tests__invalid_input_enum.snap @@ -5,7 +5,7 @@ expression: response { "errors": [ { - "message": "value `C` does not exist on `InputEnum`", + "message": "Value \"C does not exist in \"InputEnum\" enum.", "locations": [ { "line": 1,