From 0724c08e402d0bffb0eb53192c4363dac1311fe3 Mon Sep 17 00:00:00 2001
From: Magnus Hagander <magnus@hagander.net>
Date: Mon, 22 Feb 2021 10:43:59 +0100
Subject: [PATCH] Require explicit tagging on views taking query parameters

Require each view to declare which query parameters it wants, and filter
out any other parameters.

We have very few views that actually take query parameters, and random
additional query patterns will have no effect on the view. However, they
will break frontend caching (in making them look like different pages).

This will be extended into an implementation in the caching frontends as
well, btu it's needed in the backend to ensure that local testing will
have tbe same effect as the caches.
---
 pgweb/account/views.py   |  6 +++++-
 pgweb/search/views.py    |  3 ++-
 pgweb/util/decorators.py | 11 +++++++++++
 pgweb/util/middleware.py | 27 +++++++++++++++++++++++++++
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/pgweb/account/views.py b/pgweb/account/views.py
index 1f723bc4..ae42851c 100644
--- a/pgweb/account/views.py
+++ b/pgweb/account/views.py
@@ -4,7 +4,7 @@ import django.contrib.auth.views as authviews
 from django.http import HttpResponseRedirect, Http404, HttpResponse
 from django.core.exceptions import PermissionDenied
 from django.shortcuts import get_object_or_404
-from pgweb.util.decorators import login_required, script_sources, frame_sources, content_sources
+from pgweb.util.decorators import login_required, script_sources, frame_sources, content_sources, queryparams
 from django.views.decorators.csrf import csrf_exempt
 from django.utils.encoding import force_bytes
 from django.utils.http import urlsafe_base64_encode
@@ -535,6 +535,7 @@ def signup_complete(request):
 @script_sources('https://www.gstatic.com/recaptcha/')
 @frame_sources('https://www.google.com/')
 @transaction.atomic
+@queryparams('do_abort')
 def signup_oauth(request):
     if 'oauth_email' not in request.session \
        or 'oauth_firstname' not in request.session \
@@ -618,6 +619,7 @@ def signup_oauth(request):
 ####
 # Community authentication endpoint
 ####
+@queryparams('d', 'su')
 def communityauth(request, siteid):
     # Get whatever site the user is trying to log in to.
     site = get_object_or_404(CommunityAuthSite, pk=siteid)
@@ -742,6 +744,7 @@ def communityauth_logout(request, siteid):
 
 
 @login_required
+@queryparams('next')
 def communityauth_consent(request, siteid):
     org = get_object_or_404(CommunityAuthSite, id=siteid).org
     if request.method == 'POST':
@@ -776,6 +779,7 @@ def _encrypt_site_response(site, s):
     )
 
 
+@queryparams('s', 'e', 'n', 'u')
 def communityauth_search(request, siteid):
     # Perform a search for users. The response will be encrypted with the site
     # key to prevent abuse, therefor we need the site.
diff --git a/pgweb/search/views.py b/pgweb/search/views.py
index 049916fd..cfed3651 100644
--- a/pgweb/search/views.py
+++ b/pgweb/search/views.py
@@ -3,7 +3,7 @@ from django.http import HttpResponseRedirect, Http404
 from django.views.decorators.csrf import csrf_exempt
 from django.conf import settings
 
-from pgweb.util.decorators import cache
+from pgweb.util.decorators import cache, queryparams
 
 import urllib.parse
 import requests
@@ -47,6 +47,7 @@ def generate_pagelinks(pagenum, totalpages, querystring):
 
 
 @csrf_exempt
+@queryparams('a', 'd', 'l', 'ln', 'm', 'p', 'q', 's', 'u')
 @cache(minutes=30)
 def search(request):
     # Perform a general web search
diff --git a/pgweb/util/decorators.py b/pgweb/util/decorators.py
index 9333d456..c893973c 100644
--- a/pgweb/util/decorators.py
+++ b/pgweb/util/decorators.py
@@ -24,6 +24,17 @@ def cache(days=0, hours=0, minutes=0, seconds=0):
     return _cache
 
 
+def queryparams(*args):
+    """
+    Allow specified query parameters when calling function.
+    NOTE! Must be the "outermost" decorator!!!
+    """
+    def _queryparams(fn):
+        fn.queryparams = args
+        return fn
+    return _queryparams
+
+
 def allow_frames(fn):
     def _allow_frames(request, *_args, **_kwargs):
         resp = fn(request, *_args, **_kwargs)
diff --git a/pgweb/util/middleware.py b/pgweb/util/middleware.py
index 1609e001..6a6e3dbf 100644
--- a/pgweb/util/middleware.py
+++ b/pgweb/util/middleware.py
@@ -1,4 +1,5 @@
 from django.conf import settings
+from django.http import QueryDict
 
 from pgweb.util.templateloader import initialize_template_collection, get_all_templates
 
@@ -76,3 +77,29 @@ class PgMiddleware(object):
 
         response['X-XSS-Protection'] = "1; mode=block"
         return response
+
+    def process_view(self, request, view_func, view_args, view_kwargs):
+        # Filter out any query parameters that are not explicitly allowed. We do the same thing in Varnish,
+        # and it's better to also do it in django if they show up here, so issues because of it are caught
+        # in local testing where there is no Varnish.
+        if not request.GET:
+            # If there are no parameters, just skip this whole process
+            return None
+
+        if request.path.startswith('/admin/'):
+            # django-admin uses it a lot and it's not for us to change
+            return None
+
+        allowed = getattr(view_func, 'queryparams', None)
+
+        if allowed:
+            # Filter the QueryDict for only the allowed parameters
+            result = request.GET.copy()
+            for k in request.GET.keys():
+                if k not in allowed:
+                    del result[k]
+            result.mutable = False
+            request.GET = result
+        else:
+            request.GET = QueryDict()
+        return None
-- 
2.39.5