CVE-2019-13594

@@ -8,6 +8,7 @@ All notable, unreleased changes to this project will be documented in this file.
 - Fix GATEWAYS_ENUM to always contain all implemented payment gateways - #4108 by @koradon
 - Fix translation discard button - #4109 by @benekex2
 - Change input style and improve Storybook stories - #4115 by @dominik-zeglen
+- Separated the legacy middleware from the GQL API middleware - #4102 by @NyanKiyoshi
 
 ## 2.6.0
 

@@ -1,2 +1,2 @@
 [settings]
-known_third_party = PIL,babel,bleach,bootstrap4,braintree,captcha,celery,dj_database_url,dj_email_url,django,django_babel,django_cache_url,django_countries,django_elasticsearch_dsl,django_filters,django_measurement,django_prices,django_prices_openexchangerates,django_prices_vatlayer,elasticsearch_dsl,faker,freezegun,geolite2,google_measurement_protocol,graphene,graphene_django,graphene_django_optimizer,graphql,graphql_jwt,graphql_relay,html5lib,i18naddress,impersonate,markdown,measurement,mptt,phonenumber_field,phonenumbers,prices,promise,pytest,razorpay,six,social_core,storages,stripe,templated_email,text_unidecode,versatileimagefield
+known_third_party = PIL,babel,bleach,bootstrap4,braintree,captcha,celery,dj_database_url,dj_email_url,django,django_babel,django_cache_url,django_countries,django_elasticsearch_dsl,django_filters,django_measurement,django_prices,django_prices_openexchangerates,django_prices_vatlayer,elasticsearch_dsl,faker,freezegun,geolite2,google_measurement_protocol,graphene,graphene_django,graphene_django_optimizer,graphql,graphql_jwt,graphql_relay,html5lib,i18naddress,impersonate,markdown,measurement,mptt,phonenumber_field,phonenumbers,prices,promise,pytest,razorpay,six,social_core,social_django,storages,stripe,templated_email,text_unidecode,versatileimagefield

@@ -1,8 +1,22 @@
 import logging
 from datetime import date
-
+from functools import wraps
+from typing import Callable
+
+import django.contrib.auth.middleware
+import django.contrib.messages.middleware
+import django.contrib.sessions.middleware
+import django.middleware.common
+import django.middleware.csrf
+import django.middleware.locale
+import django.middleware.security
+import django_babel.middleware
+import impersonate.middleware
+import social_django.middleware
 from django.conf import settings
 from django.contrib.sites.models import Site
+from django.core.exceptions import MiddlewareNotUsed
+from django.urls import reverse
 from django.utils.functional import SimpleLazyObject
 from django.utils.translation import get_language
 from django_countries.fields import Country
@@ -15,9 +29,66 @@
 logger = logging.getLogger(__name__)
 
 
+def django_only_request_handler(get_response: Callable, handler: Callable):
+    api_path = reverse("api")
+
+    @wraps(handler)
+    def handle_request(request):
+        if request.path == api_path:
+            return get_response(request)
+        return handler(request)
+
+    return handle_request
+
+
+def django_only_middleware(middleware):
+    @wraps(middleware)
+    def wrapped(get_response):
+        handler = middleware(get_response)
+        return django_only_request_handler(get_response, handler)
+
+    return wrapped
+
+
+social_auth_exception_middleware = django_only_middleware(
+    social_django.middleware.SocialAuthExceptionMiddleware
+)
+impersonate_middleware = django_only_middleware(
+    impersonate.middleware.ImpersonateMiddleware
+)
+babel_locale_middleware = django_only_middleware(
+    django_babel.middleware.LocaleMiddleware
+)
+django_locale_middleware = django_only_middleware(
+    django.middleware.locale.LocaleMiddleware
+)
+django_messages_middleware = django_only_middleware(
+    django.contrib.messages.middleware.MessageMiddleware
+)
+django_auth_middleware = django_only_middleware(
+    django.contrib.auth.middleware.AuthenticationMiddleware
+)
+django_csrf_view_middleware = django_only_middleware(
+    django.middleware.csrf.CsrfViewMiddleware
+)
+django_common_middleware = django_only_middleware(
+    django.middleware.common.CommonMiddleware
+)
+django_security_middleware = django_only_middleware(
+    django.middleware.security.SecurityMiddleware
+)
+django_session_middleware = django_only_middleware(
+    django.contrib.sessions.middleware.SessionMiddleware
+)
+
+
+@django_only_middleware
 def google_analytics(get_response):
     """Report a page view to Google Analytics."""
 
+    if not settings.GOOGLE_ANALYTICS_TRACKING_ID:
+        raise MiddlewareNotUsed()
+
     def middleware(request):
         client_id = analytics.get_client_id(request)
         path = request.path
@@ -38,10 +109,9 @@ def discounts(get_response):
     """Assign active discounts to `request.discounts`."""
 
     def middleware(request):
-        discounts = Sale.objects.active(date.today()).prefetch_related(
+        request.discounts = Sale.objects.active(date.today()).prefetch_related(
             "products", "categories", "collections"
         )
-        request.discounts = discounts
         return get_response(request)
 
     return middleware
@@ -83,9 +153,12 @@ def site(get_response):
     the cache. Using this middleware solves this problem.
     """
 
-    def middleware(request):
+    def _get_site():
         Site.objects.clear_cache()
-        request.site = Site.objects.get_current()
+        return Site.objects.get_current()
+
+    def middleware(request):
+        request.site = SimpleLazyObject(_get_site)
         return get_response(request)
 
     return middleware

@@ -1,9 +1,33 @@
+from functools import wraps
+from typing import Callable
+
 from django.contrib.auth.models import AnonymousUser
-from django.shortcuts import reverse
+from django.urls import reverse
 from graphene_django.settings import graphene_settings
 from graphql_jwt.middleware import JSONWebTokenMiddleware
 
 
+def api_only_request_handler(get_response: Callable, handler: Callable):
+    @wraps(handler)
+    def handle_request(request):
+        api_path = reverse("api")
+        if request.path != api_path:
+            return get_response(request)
+        return handler(request)
+
+    return handle_request
+
+
+def api_only_middleware(middleware):
+    @wraps(middleware)
+    def wrapped(get_response):
+        handler = middleware(get_response)
+        return api_only_request_handler(get_response, handler)
+
+    return wrapped
+
+
+@api_only_middleware
 def jwt_middleware(get_response):
     """Authenticate user using JSONWebTokenMiddleware
     ignoring the session-based authentication.
@@ -18,13 +42,12 @@ def jwt_middleware(get_response):
     graphene_settings.MIDDLEWARE.remove(JSONWebTokenMiddleware)
 
     def middleware(request):
-        if request.path == reverse("api"):
-            # clear user authenticated by AuthenticationMiddleware
-            request._cached_user = AnonymousUser()
-            request.user = AnonymousUser()
+        # clear user authenticated by AuthenticationMiddleware
+        request._cached_user = AnonymousUser()
+        request.user = AnonymousUser()
 
-            # authenticate using JWT middleware
-            jwt_middleware_inst.process_request(request)
+        # authenticate using JWT middleware
+        jwt_middleware_inst.process_request(request)
         return get_response(request)
 
     return middleware

@@ -195,22 +195,22 @@ def get_bool_from_env(name, default_value):
 SECRET_KEY = os.environ.get("SECRET_KEY")
 
 MIDDLEWARE = [
-    "django.contrib.sessions.middleware.SessionMiddleware",
-    "django.middleware.security.SecurityMiddleware",
-    "django.middleware.common.CommonMiddleware",
-    "django.middleware.csrf.CsrfViewMiddleware",
-    "django.contrib.auth.middleware.AuthenticationMiddleware",
-    "django.contrib.messages.middleware.MessageMiddleware",
-    "django.middleware.locale.LocaleMiddleware",
-    "django_babel.middleware.LocaleMiddleware",
+    "saleor.core.middleware.django_session_middleware",
+    "saleor.core.middleware.django_security_middleware",
+    "saleor.core.middleware.django_common_middleware",
+    "saleor.core.middleware.django_csrf_view_middleware",
+    "saleor.core.middleware.django_auth_middleware",
+    "saleor.core.middleware.django_messages_middleware",
+    "saleor.core.middleware.django_locale_middleware",
+    "saleor.core.middleware.babel_locale_middleware",
     "saleor.core.middleware.discounts",
     "saleor.core.middleware.google_analytics",
     "saleor.core.middleware.country",
     "saleor.core.middleware.currency",
     "saleor.core.middleware.site",
     "saleor.core.middleware.taxes",
-    "social_django.middleware.SocialAuthExceptionMiddleware",
-    "impersonate.middleware.ImpersonateMiddleware",
+    "saleor.core.middleware.social_auth_exception_middleware",
+    "saleor.core.middleware.impersonate_middleware",
     "saleor.graphql.middleware.jwt_middleware",
 ]
 

@@ -20,6 +20,20 @@
 from tests.api.utils import get_graphql_content
 
 
+def test_middleware_dont_generate_sql_requests(
+    client, settings, django_assert_num_queries
+):
+    """When requesting on the GraphQL API endpoint, no SQL request should happen
+    indirectly. This test ensures that."""
+
+    # Enables the Graphql playground
+    settings.DEBUG = True
+
+    with django_assert_num_queries(0):
+        response = client.get(reverse("api"))
+        assert response.status_code == 200
+
+
 def test_jwt_middleware(admin_user):
     def get_response(request):
         return HttpResponse()