Avoid returning HttpServerError for things not server errors

With the new django, alerts are raised for everything with status 500,
not juse exceptions. This put a light on a number of places where we
were returning 500 server error code for things that are not actually
server errors. Some should be a regular 200 ok with an error message,
and others should be a permissions error.

diff --git a/pgweb/account/views.py b/pgweb/account/views.py
index f4b2b74d0ffab6bc1e4fbe27c3a72214a32b9a18..6a75791d9f1e4d2fd3b9407ef45f36006682df5b 100644 (file)
--- a/pgweb/account/views.py
+++ b/pgweb/account/views.py
@@ -23,7 +23,7 @@ import itertools
 
 from pgweb.util.contexts import render_pgweb
 from pgweb.util.misc import send_template_mail, generate_random_token, get_client_ip
-from pgweb.util.helpers import HttpServerError
+from pgweb.util.helpers import HttpSimpleResponse
 
 from pgweb.news.models import NewsArticle
 from pgweb.events.models import Event
@@ -147,7 +147,7 @@ def change_email(request):
     if request.user.password == OAUTH_PASSWORD_STORE:
         # Link shouldn't exist in this case, so just throw an unfriendly
         # error message.
-        return HttpServerError(request, "This account cannot change email address as it's connected to a third party login site.")
+        return HttpSimpleResponse(request, "Account error", "This account cannot change email address as it's connected to a third party login site.")
 
     if request.method == 'POST':
         form = ChangeEmailForm(request.user, data=request.POST)
@@ -188,7 +188,7 @@ def confirm_change_email(request, tokenhash):
     if request.user.password == OAUTH_PASSWORD_STORE:
         # Link shouldn't exist in this case, so just throw an unfriendly
         # error message.
-        return HttpServerError(request, "This account cannot change email address as it's connected to a third party login site.")
+        return HttpSimpleResponse(request, "Account error", "This account cannot change email address as it's connected to a third party login site.")
 
     if token:
         # Valid token find, so change the email address
@@ -242,7 +242,7 @@ def logout(request):
 
 def changepwd(request):
     if hasattr(request.user, 'password') and request.user.password == OAUTH_PASSWORD_STORE:
-        return HttpServerError(request, "This account cannot change password as it's connected to a third party login site.")
+        return HttpSimpleResponse(request, "Account error", "This account cannot change password as it's connected to a third party login site.")
 
     log.info("Initiating password change from {0}".format(get_client_ip(request)))
     return authviews.PasswordChangeView.as_view(template_name='account/password_change.html',
@@ -257,7 +257,7 @@ def resetpwd(request):
         try:
             u = User.objects.get(email__iexact=request.POST['email'])
             if u.password == OAUTH_PASSWORD_STORE:
-                return HttpServerError(request, "This account cannot change password as it's connected to a third party login site.")
+                return HttpSimpleResponse(request, "Account error", "This account cannot change password as it's connected to a third party login site.")
         except User.DoesNotExist:
             log.info("Attempting to reset password of {0}, user not found".format(request.POST['email']))
             return HttpResponseRedirect('/account/reset/done/')
@@ -313,7 +313,7 @@ def reset_complete(request):
 @frame_sources('https://www.google.com/')
 def signup(request):
     if request.user.is_authenticated:
-        return HttpServerError(request, "You must log out before you can sign up for a new account")
+        return HttpSimpleResponse(request, "Account error", "You must log out before you can sign up for a new account")
 
     if request.method == 'POST':
         # Attempt to create user then, eh?
@@ -376,7 +376,7 @@ def signup_oauth(request):
     if 'oauth_email' not in request.session \
        or 'oauth_firstname' not in request.session \
        or 'oauth_lastname' not in request.session:
-        return HttpServerError(request, 'Invalid redirect received')
+        return HttpSimpleResponse(request, "OAuth error", 'Invalid redirect received')
 
     if request.method == 'POST':
         # Second stage, so create the account. But verify that the

diff --git a/pgweb/core/views.py b/pgweb/core/views.py
index 24a0beecefb7612f70ec5a815bbeda76b084ed01..a7b4483f1918c66462bbf60c0d35b950a4f60f9a 100644 (file)
--- a/pgweb/core/views.py
+++ b/pgweb/core/views.py
@@ -1,6 +1,7 @@
 from django.shortcuts import render, get_object_or_404
 from django.http import HttpResponse, Http404, HttpResponseRedirect
 from django.http import HttpResponseNotModified
+from django.core.exceptions import PermissionDenied
 from django.template import TemplateDoesNotExist, loader
 from django.contrib.auth.decorators import user_passes_test
 from pgweb.util.decorators import login_required
@@ -19,7 +20,7 @@ import urllib.parse
 
 from pgweb.util.decorators import cache, nocache
 from pgweb.util.contexts import render_pgweb, get_nav_menu, PGWebContextProcessor
-from pgweb.util.helpers import simple_form, PgXmlHelper, HttpServerError
+from pgweb.util.helpers import simple_form, PgXmlHelper
 from pgweb.util.moderation import get_all_pending_moderations
 from pgweb.util.misc import get_client_ip, varnish_purge, varnish_purge_expr, varnish_purge_xkey
 from pgweb.util.sitestruct import get_all_pages_struct
@@ -328,9 +329,9 @@ def admin_purge(request):
 @csrf_exempt
 def api_varnish_purge(request):
     if not request.META['REMOTE_ADDR'] in settings.VARNISH_PURGERS:
-        return HttpServerError(request, "Invalid client address")
+        raise PermissionDenied("Invalid client address")
     if request.method != 'POST':
-        return HttpServerError(request, "Can't use this way")
+        raise PermissionDenied("Can't use this way")
     n = int(request.POST['n'])
     curs = connection.cursor()
     for i in range(0, n):

diff --git a/pgweb/downloads/views.py b/pgweb/downloads/views.py
index 48d908af24ff9b1c0fe638960a7698c3a5f798df..3d507658578053d2cf7f3c32c9ebc9f2e0ec05a8 100644 (file)
--- a/pgweb/downloads/views.py
+++ b/pgweb/downloads/views.py
@@ -1,5 +1,6 @@
 from django.shortcuts import render, get_object_or_404
 from django.http import HttpResponse, Http404, HttpResponseRedirect
+from django.core.exceptions import PermissionDenied
 from pgweb.util.decorators import login_required
 from django.views.decorators.csrf import csrf_exempt
 from django.conf import settings
@@ -127,9 +128,9 @@ def ftpbrowser(request, subpath):
 @csrf_exempt
 def uploadftp(request):
     if request.method != 'PUT':
-        return HttpServerError(request, "Invalid method")
+        raise PermissionDenied("Invalid method")
     if not request.META['REMOTE_ADDR'] in settings.FTP_MASTERS:
-        return HttpServerError(request, "Invalid client address")
+        raise PermissionDenied("Invalid client address")
     # We have the data in request.body. Attempt to load it as
     # a pickle to make sure it's properly formatted
     pickle.loads(request.body)
@@ -158,9 +159,9 @@ def uploadftp(request):
 @csrf_exempt
 def uploadyum(request):
     if request.method != 'PUT':
-        return HttpServerError(request, "Invalid method")
+        raise PermissionDenied("Invalid method")
     if not request.META['REMOTE_ADDR'] in settings.FTP_MASTERS:
-        return HttpServerError(request, "Invalid client address")
+        raise PermissionDenied("Invalid client address")
     # We have the data in request.body. Attempt to load it as
     # json to ensure correct format.
     json.loads(request.body.decode('utf8'))

diff --git a/pgweb/survey/views.py b/pgweb/survey/views.py
index 07077cd70c52ed69b39878256e0021b85efb44bc..1cd6739de44b8563e8eda167c5f631b5a3adf8a3 100644 (file)
--- a/pgweb/survey/views.py
+++ b/pgweb/survey/views.py
@@ -6,7 +6,7 @@ from django.views.decorators.csrf import csrf_exempt
 
 from pgweb.util.contexts import render_pgweb
 from pgweb.util.misc import get_client_ip, varnish_purge
-from pgweb.util.helpers import HttpServerError
+from pgweb.util.helpers import HttpSimpleResponse
 
 from .models import Survey, SurveyAnswer, SurveyLock
 
@@ -30,7 +30,7 @@ def vote(request, surveyid):
     try:
         ansnum = int(request.POST['answer'])
         if ansnum < 1 or ansnum > 8:
-            return HttpServerError(request, "Invalid answer")
+            return HttpSimpleResponse(request, "Response error", "Invalid answer")
     except Exception as e:
         # When no answer is given, redirect to results instead
         return HttpResponseRedirect("/community/survey/%s-%s" % (surv.id, slugify(surv.question)))
@@ -46,7 +46,7 @@ def vote(request, surveyid):
     # Check if we are locked
     lock = SurveyLock.objects.filter(ipaddr=addr)
     if len(lock) > 0:
-        return HttpServerError(request, "Too many requests from your IP in the past 15 minutes")
+        return HttpSimpleResponse(request, "Rate limited", "Too many requests from your IP in the past 15 minutes")
 
     # Generate a new lock item, and store it
     lock = SurveyLock(ipaddr=addr)

diff --git a/pgweb/util/helpers.py b/pgweb/util/helpers.py
index 1b0e0ab47f4162760f03398c9337073d6f7ad21b..ae2ea6bcad592fb164200e4d920ca0d94a8c8941 100644 (file)
--- a/pgweb/util/helpers.py
+++ b/pgweb/util/helpers.py
@@ -175,6 +175,13 @@ def HttpServerError(request, msg):
     return r
 
 
+def HttpSimpleResponse(request, title, msg):
+    return render(request, 'simple.html', {
+        'title': title,
+        'message': msg,
+    })
+
+
 class PgXmlHelper(django.utils.xmlutils.SimplerXMLGenerator):
     def __init__(self, outstream, skipempty=False):
         django.utils.xmlutils.SimplerXMLGenerator.__init__(self, outstream, 'utf-8')

diff --git a/templates/simple.html b/templates/simple.html
new file mode 100644 (file)
index 0000000..2cc7472
--- /dev/null
+++ b/templates/simple.html
@@ -0,0 +1,8 @@
+{%extends "base/page.html"%}
+{%block title%}{{title}}{%endblock%}
+{%block contents%}
+<h1>{{title}}</h1>
+<p>
+{{message}}
+</p>
+{%endblock%}