From f88900017d7347c8aa3851ba8efe62481eaeb8a8 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Wed, 1 Nov 2023 10:30:24 +0800 Subject: [PATCH 1/7] Add support for validation rules --- graphene_django/views.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/graphene_django/views.py b/graphene_django/views.py index 9fc617207..2cb88276a 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -96,6 +96,7 @@ class GraphQLView(View): batch = False subscription_path = None execution_context_class = None + validation_rules = None def __init__( self, @@ -107,6 +108,7 @@ def __init__( batch=False, subscription_path=None, execution_context_class=None, + validation_rules=None, ): if not schema: schema = graphene_settings.SCHEMA @@ -135,6 +137,8 @@ def __init__( ), "A Schema is required to be provided to GraphQLView." assert not all((graphiql, batch)), "Use either graphiql or batch processing" + self.validation_rules = validation_rules + # noinspection PyUnusedLocal def get_root_value(self, request): return self.root_value @@ -332,7 +336,7 @@ def execute_graphql_request( ) ) - validation_errors = validate(schema, document) + validation_errors = validate(schema, document, self.validation_rules) if validation_errors: return ExecutionResult(data=None, errors=validation_errors) From 918b590eea2e817a0b2d470e4161e572fa8084bb Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Wed, 1 Nov 2023 10:31:53 +0800 Subject: [PATCH 2/7] Enable customizing validate max_errors through settings --- docs/settings.rst | 11 +++++++++++ graphene_django/settings.py | 1 + graphene_django/views.py | 7 ++++++- 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/docs/settings.rst b/docs/settings.rst index e5f0faf25..79c52e2e5 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -269,3 +269,14 @@ Default: ``False`` .. _GraphiQLDocs: https://graphiql-test.netlify.app/typedoc/modules/graphiql_react#graphiqlprovider-2 + + +``MAX_VALIDATION_ERRORS`` +------------------------------------ + +In case ``validation_rules`` are provided to ``GraphQLView``, if this is set to a non-negative ``int`` value, +``graphql.validation.validate`` will stop validation after this number of errors has been reached. +If not set or set to ``None``, the maximum number of errors will follow ``graphql.validation.validate`` default +*i.e.* 100. + +Default: ``None`` diff --git a/graphene_django/settings.py b/graphene_django/settings.py index de2c52163..f7e3ee746 100644 --- a/graphene_django/settings.py +++ b/graphene_django/settings.py @@ -43,6 +43,7 @@ "GRAPHIQL_INPUT_VALUE_DEPRECATION": False, "ATOMIC_MUTATIONS": False, "TESTING_ENDPOINT": "/graphql", + "MAX_VALIDATION_ERRORS": None, } if settings.DEBUG: diff --git a/graphene_django/views.py b/graphene_django/views.py index 2cb88276a..fca9194a1 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -336,7 +336,12 @@ def execute_graphql_request( ) ) - validation_errors = validate(schema, document, self.validation_rules) + validation_errors = validate( + schema, + document, + self.validation_rules, + graphene_settings.MAX_VALIDATION_ERRORS, + ) if validation_errors: return ExecutionResult(data=None, errors=validation_errors) From 3ca12446d73c1b8c10f73bd9c6f644ce2152f253 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Fri, 24 Nov 2023 04:30:33 +0800 Subject: [PATCH 3/7] Add tests for validation rules --- graphene_django/tests/test_views.py | 63 ++++++++++++++++++++++++ graphene_django/tests/urls_validation.py | 16 ++++++ 2 files changed, 79 insertions(+) create mode 100644 graphene_django/tests/urls_validation.py diff --git a/graphene_django/tests/test_views.py b/graphene_django/tests/test_views.py index d64a4f02b..ac4c92400 100644 --- a/graphene_django/tests/test_views.py +++ b/graphene_django/tests/test_views.py @@ -827,3 +827,66 @@ def test_query_errors_atomic_request(set_rollback_mock, client): def test_query_errors_non_atomic(set_rollback_mock, client): client.get(url_string(query="force error")) set_rollback_mock.assert_not_called() + + +query_with_two_introspections = """ +query Instrospection { + queryType: __schema { + queryType {name} + } + mutationType: __schema { + mutationType {name} + } +} +""" + +introspection_disallow_error_message = "introspection is disabled" +max_validation_errors_exceeded_message = "too many validation errors" + + +@pytest.mark.urls("graphene_django.tests.urls_validation") +def test_allow_introspection(client): + response = client.post( + url_string("/graphql/", query="{__schema {queryType {name}}}") + ) + assert response.status_code == 200 + + assert response_json(response) == { + "data": {"__schema": {"queryType": {"name": "QueryRoot"}}} + } + + +@pytest.mark.urls("graphene_django.tests.urls_validation") +def test_validation_disallow_introspection(client): + response = client.post( + url_string("/graphql/validation/", query="{__schema {queryType {name}}}") + ) + + assert response.status_code == 400 + assert introspection_disallow_error_message in response.content.decode() + + +@pytest.mark.urls("graphene_django.tests.urls_validation") +@patch("graphene_django.settings.graphene_settings.MAX_VALIDATION_ERRORS", 2) +def test_within_max_validation_errors(client): + response = client.post( + url_string("/graphql/validation/", query=query_with_two_introspections) + ) + + assert response.status_code == 400 + + text_response = response.content.decode().lower() + + assert text_response.count(introspection_disallow_error_message) == 2 + assert max_validation_errors_exceeded_message not in text_response + + +@pytest.mark.urls("graphene_django.tests.urls_validation") +@patch("graphene_django.settings.graphene_settings.MAX_VALIDATION_ERRORS", 1) +def test_exceeds_max_validation_errors(client): + response = client.post( + url_string("/graphql/validation/", query=query_with_two_introspections) + ) + + assert response.status_code == 400 + assert max_validation_errors_exceeded_message in response.content.decode().lower() diff --git a/graphene_django/tests/urls_validation.py b/graphene_django/tests/urls_validation.py new file mode 100644 index 000000000..e8dab36d3 --- /dev/null +++ b/graphene_django/tests/urls_validation.py @@ -0,0 +1,16 @@ +from django.urls import path + +from graphene.validation import DisableIntrospection + +from ..views import GraphQLView +from .schema_view import schema + + +class View(GraphQLView): + schema = schema + + +urlpatterns = [ + path("graphql/", View.as_view()), + path("graphql/validation/", View.as_view(validation_rules=(DisableIntrospection,))), +] From 9a56a2b7514a74bbc2c81f7f693abf937c594416 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Fri, 24 Nov 2023 04:50:14 +0800 Subject: [PATCH 4/7] Add examples for validation rules --- docs/index.rst | 1 + docs/validation.rst | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 docs/validation.rst diff --git a/docs/index.rst b/docs/index.rst index 373969e1d..df97a5707 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -33,5 +33,6 @@ For more advanced use, check out the Relay tutorial. authorization debug introspection + validation testing settings diff --git a/docs/validation.rst b/docs/validation.rst new file mode 100644 index 000000000..71373420e --- /dev/null +++ b/docs/validation.rst @@ -0,0 +1,29 @@ +Query Validation +================ + +Graphene-Django supports query validation by allowing passing a list of validation rules (subclasses of `ValidationRule `_ from graphql-core) to the ``validation_rules`` option in ``GraphQLView``. + +.. code:: python + + from django.urls import path + from graphene.validation import DisableIntrospection + from graphene_django.views import GraphQLView + + urlpatterns = [ + path("graphql", GraphQLView.as_view(validation_rules=(DisableIntrospection,))), + ] + +or + +.. code:: python + + from django.urls import path + from graphene.validation import DisableIntrospection + from graphene_django.views import GraphQLView + + class View(GraphQLView): + validation_rules = (DisableIntrospection,) + + urlpatterns = [ + path("graphql", View.as_view()), + ] From 8eeffc77b5cdf68c090d695c8c2fbe79029727ea Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Fri, 24 Nov 2023 05:09:02 +0800 Subject: [PATCH 5/7] Allow setting validation_rules in class def --- graphene_django/tests/test_views.py | 23 +++++++++++------------ graphene_django/tests/urls_validation.py | 5 +++++ graphene_django/views.py | 2 +- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/graphene_django/tests/test_views.py b/graphene_django/tests/test_views.py index ac4c92400..f718dd331 100644 --- a/graphene_django/tests/test_views.py +++ b/graphene_django/tests/test_views.py @@ -829,6 +829,8 @@ def test_query_errors_non_atomic(set_rollback_mock, client): set_rollback_mock.assert_not_called() +validation_urls = ["/graphql/validation/", "/graphql/validation/alternative/"] + query_with_two_introspections = """ query Instrospection { queryType: __schema { @@ -856,22 +858,20 @@ def test_allow_introspection(client): } +@pytest.mark.parametrize("url", validation_urls) @pytest.mark.urls("graphene_django.tests.urls_validation") -def test_validation_disallow_introspection(client): - response = client.post( - url_string("/graphql/validation/", query="{__schema {queryType {name}}}") - ) +def test_validation_disallow_introspection(client, url): + response = client.post(url_string(url, query="{__schema {queryType {name}}}")) assert response.status_code == 400 assert introspection_disallow_error_message in response.content.decode() +@pytest.mark.parametrize("url", validation_urls) @pytest.mark.urls("graphene_django.tests.urls_validation") @patch("graphene_django.settings.graphene_settings.MAX_VALIDATION_ERRORS", 2) -def test_within_max_validation_errors(client): - response = client.post( - url_string("/graphql/validation/", query=query_with_two_introspections) - ) +def test_within_max_validation_errors(client, url): + response = client.post(url_string(url, query=query_with_two_introspections)) assert response.status_code == 400 @@ -881,12 +881,11 @@ def test_within_max_validation_errors(client): assert max_validation_errors_exceeded_message not in text_response +@pytest.mark.parametrize("url", validation_urls) @pytest.mark.urls("graphene_django.tests.urls_validation") @patch("graphene_django.settings.graphene_settings.MAX_VALIDATION_ERRORS", 1) -def test_exceeds_max_validation_errors(client): - response = client.post( - url_string("/graphql/validation/", query=query_with_two_introspections) - ) +def test_exceeds_max_validation_errors(client, url): + response = client.post(url_string(url, query=query_with_two_introspections)) assert response.status_code == 400 assert max_validation_errors_exceeded_message in response.content.decode().lower() diff --git a/graphene_django/tests/urls_validation.py b/graphene_django/tests/urls_validation.py index e8dab36d3..91eafa194 100644 --- a/graphene_django/tests/urls_validation.py +++ b/graphene_django/tests/urls_validation.py @@ -10,7 +10,12 @@ class View(GraphQLView): schema = schema +class NoIntroSpectionView(View): + validation_rules = (DisableIntrospection,) + + urlpatterns = [ path("graphql/", View.as_view()), path("graphql/validation/", View.as_view(validation_rules=(DisableIntrospection,))), + path("graphql/validation/alternative/", NoIntroSpectionView.as_view()), ] diff --git a/graphene_django/views.py b/graphene_django/views.py index fca9194a1..1ec659881 100644 --- a/graphene_django/views.py +++ b/graphene_django/views.py @@ -137,7 +137,7 @@ def __init__( ), "A Schema is required to be provided to GraphQLView." assert not all((graphiql, batch)), "Use either graphiql or batch processing" - self.validation_rules = validation_rules + self.validation_rules = validation_rules or self.validation_rules # noinspection PyUnusedLocal def get_root_value(self, request): From 9561ec3bdd9e744118235c6e0c18e84b8a0001eb Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Fri, 24 Nov 2023 05:13:52 +0800 Subject: [PATCH 6/7] Add tests for validation_rules inherited from parent class --- graphene_django/tests/test_views.py | 6 +++++- graphene_django/tests/urls_validation.py | 9 +++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/graphene_django/tests/test_views.py b/graphene_django/tests/test_views.py index f718dd331..648261cbb 100644 --- a/graphene_django/tests/test_views.py +++ b/graphene_django/tests/test_views.py @@ -829,7 +829,11 @@ def test_query_errors_non_atomic(set_rollback_mock, client): set_rollback_mock.assert_not_called() -validation_urls = ["/graphql/validation/", "/graphql/validation/alternative/"] +validation_urls = [ + "/graphql/validation/", + "/graphql/validation/alternative/", + "/graphql/validation/inherited/", +] query_with_two_introspections = """ query Instrospection { diff --git a/graphene_django/tests/urls_validation.py b/graphene_django/tests/urls_validation.py index 91eafa194..74f58b20a 100644 --- a/graphene_django/tests/urls_validation.py +++ b/graphene_django/tests/urls_validation.py @@ -10,12 +10,17 @@ class View(GraphQLView): schema = schema -class NoIntroSpectionView(View): +class NoIntrospectionView(View): validation_rules = (DisableIntrospection,) +class NoIntrospectionViewInherited(NoIntrospectionView): + pass + + urlpatterns = [ path("graphql/", View.as_view()), path("graphql/validation/", View.as_view(validation_rules=(DisableIntrospection,))), - path("graphql/validation/alternative/", NoIntroSpectionView.as_view()), + path("graphql/validation/alternative/", NoIntrospectionView.as_view()), + path("graphql/validation/inherited/", NoIntrospectionViewInherited.as_view()), ] From d00654791697288500dca223d0863324383c6cb0 Mon Sep 17 00:00:00 2001 From: Kien Dang Date: Tue, 28 Nov 2023 14:27:38 +0800 Subject: [PATCH 7/7] Make tests for validation rules stricter --- graphene_django/tests/test_views.py | 58 +++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/graphene_django/tests/test_views.py b/graphene_django/tests/test_views.py index 648261cbb..c2b42bce4 100644 --- a/graphene_django/tests/test_views.py +++ b/graphene_django/tests/test_views.py @@ -829,13 +829,13 @@ def test_query_errors_non_atomic(set_rollback_mock, client): set_rollback_mock.assert_not_called() -validation_urls = [ +VALIDATION_URLS = [ "/graphql/validation/", "/graphql/validation/alternative/", "/graphql/validation/inherited/", ] -query_with_two_introspections = """ +QUERY_WITH_TWO_INTROSPECTIONS = """ query Instrospection { queryType: __schema { queryType {name} @@ -846,8 +846,10 @@ def test_query_errors_non_atomic(set_rollback_mock, client): } """ -introspection_disallow_error_message = "introspection is disabled" -max_validation_errors_exceeded_message = "too many validation errors" +N_INTROSPECTIONS = 2 + +INTROSPECTION_DISALLOWED_ERROR_MESSAGE = "introspection is disabled" +MAX_VALIDATION_ERRORS_EXCEEDED_MESSAGE = "too many validation errors" @pytest.mark.urls("graphene_django.tests.urls_validation") @@ -862,34 +864,60 @@ def test_allow_introspection(client): } -@pytest.mark.parametrize("url", validation_urls) +@pytest.mark.parametrize("url", VALIDATION_URLS) @pytest.mark.urls("graphene_django.tests.urls_validation") def test_validation_disallow_introspection(client, url): response = client.post(url_string(url, query="{__schema {queryType {name}}}")) assert response.status_code == 400 - assert introspection_disallow_error_message in response.content.decode() + + json_response = response_json(response) + assert "data" not in json_response + assert "errors" in json_response + assert len(json_response["errors"]) == 1 + + error_message = json_response["errors"][0]["message"] + assert INTROSPECTION_DISALLOWED_ERROR_MESSAGE in error_message -@pytest.mark.parametrize("url", validation_urls) +@pytest.mark.parametrize("url", VALIDATION_URLS) @pytest.mark.urls("graphene_django.tests.urls_validation") -@patch("graphene_django.settings.graphene_settings.MAX_VALIDATION_ERRORS", 2) +@patch( + "graphene_django.settings.graphene_settings.MAX_VALIDATION_ERRORS", N_INTROSPECTIONS +) def test_within_max_validation_errors(client, url): - response = client.post(url_string(url, query=query_with_two_introspections)) + response = client.post(url_string(url, query=QUERY_WITH_TWO_INTROSPECTIONS)) assert response.status_code == 400 - text_response = response.content.decode().lower() + json_response = response_json(response) + assert "data" not in json_response + assert "errors" in json_response + assert len(json_response["errors"]) == N_INTROSPECTIONS + + error_messages = [error["message"].lower() for error in json_response["errors"]] - assert text_response.count(introspection_disallow_error_message) == 2 - assert max_validation_errors_exceeded_message not in text_response + n_introspection_error_messages = sum( + INTROSPECTION_DISALLOWED_ERROR_MESSAGE in msg for msg in error_messages + ) + assert n_introspection_error_messages == N_INTROSPECTIONS + + assert all( + MAX_VALIDATION_ERRORS_EXCEEDED_MESSAGE not in msg for msg in error_messages + ) -@pytest.mark.parametrize("url", validation_urls) +@pytest.mark.parametrize("url", VALIDATION_URLS) @pytest.mark.urls("graphene_django.tests.urls_validation") @patch("graphene_django.settings.graphene_settings.MAX_VALIDATION_ERRORS", 1) def test_exceeds_max_validation_errors(client, url): - response = client.post(url_string(url, query=query_with_two_introspections)) + response = client.post(url_string(url, query=QUERY_WITH_TWO_INTROSPECTIONS)) assert response.status_code == 400 - assert max_validation_errors_exceeded_message in response.content.decode().lower() + + json_response = response_json(response) + assert "data" not in json_response + assert "errors" in json_response + + error_messages = (error["message"].lower() for error in json_response["errors"]) + assert any(MAX_VALIDATION_ERRORS_EXCEEDED_MESSAGE in msg for msg in error_messages)