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.
This commit is contained in:
Magnus Hagander
2020-04-04 14:43:27 +02:00
parent 454ea7a7be
commit b405302d97
6 changed files with 34 additions and 17 deletions

View File

@ -23,7 +23,7 @@ import itertools
from pgweb.util.contexts import render_pgweb from pgweb.util.contexts import render_pgweb
from pgweb.util.misc import send_template_mail, generate_random_token, get_client_ip 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.news.models import NewsArticle
from pgweb.events.models import Event from pgweb.events.models import Event
@ -147,7 +147,7 @@ def change_email(request):
if request.user.password == OAUTH_PASSWORD_STORE: if request.user.password == OAUTH_PASSWORD_STORE:
# Link shouldn't exist in this case, so just throw an unfriendly # Link shouldn't exist in this case, so just throw an unfriendly
# error message. # 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': if request.method == 'POST':
form = ChangeEmailForm(request.user, data=request.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: if request.user.password == OAUTH_PASSWORD_STORE:
# Link shouldn't exist in this case, so just throw an unfriendly # Link shouldn't exist in this case, so just throw an unfriendly
# error message. # 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: if token:
# Valid token find, so change the email address # Valid token find, so change the email address
@ -242,7 +242,7 @@ def logout(request):
def changepwd(request): def changepwd(request):
if hasattr(request.user, 'password') and request.user.password == OAUTH_PASSWORD_STORE: 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))) log.info("Initiating password change from {0}".format(get_client_ip(request)))
return authviews.PasswordChangeView.as_view(template_name='account/password_change.html', return authviews.PasswordChangeView.as_view(template_name='account/password_change.html',
@ -257,7 +257,7 @@ def resetpwd(request):
try: try:
u = User.objects.get(email__iexact=request.POST['email']) u = User.objects.get(email__iexact=request.POST['email'])
if u.password == OAUTH_PASSWORD_STORE: 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: except User.DoesNotExist:
log.info("Attempting to reset password of {0}, user not found".format(request.POST['email'])) log.info("Attempting to reset password of {0}, user not found".format(request.POST['email']))
return HttpResponseRedirect('/account/reset/done/') return HttpResponseRedirect('/account/reset/done/')
@ -313,7 +313,7 @@ def reset_complete(request):
@frame_sources('https://www.google.com/') @frame_sources('https://www.google.com/')
def signup(request): def signup(request):
if request.user.is_authenticated: 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': if request.method == 'POST':
# Attempt to create user then, eh? # Attempt to create user then, eh?
@ -376,7 +376,7 @@ def signup_oauth(request):
if 'oauth_email' not in request.session \ if 'oauth_email' not in request.session \
or 'oauth_firstname' not in request.session \ or 'oauth_firstname' not in request.session \
or 'oauth_lastname' 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': if request.method == 'POST':
# Second stage, so create the account. But verify that the # Second stage, so create the account. But verify that the

View File

@ -1,6 +1,7 @@
from django.shortcuts import render, get_object_or_404 from django.shortcuts import render, get_object_or_404
from django.http import HttpResponse, Http404, HttpResponseRedirect from django.http import HttpResponse, Http404, HttpResponseRedirect
from django.http import HttpResponseNotModified from django.http import HttpResponseNotModified
from django.core.exceptions import PermissionDenied
from django.template import TemplateDoesNotExist, loader from django.template import TemplateDoesNotExist, loader
from django.contrib.auth.decorators import user_passes_test from django.contrib.auth.decorators import user_passes_test
from pgweb.util.decorators import login_required from pgweb.util.decorators import login_required
@ -19,7 +20,7 @@ import urllib.parse
from pgweb.util.decorators import cache, nocache from pgweb.util.decorators import cache, nocache
from pgweb.util.contexts import render_pgweb, get_nav_menu, PGWebContextProcessor 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.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.misc import get_client_ip, varnish_purge, varnish_purge_expr, varnish_purge_xkey
from pgweb.util.sitestruct import get_all_pages_struct from pgweb.util.sitestruct import get_all_pages_struct
@ -328,9 +329,9 @@ def admin_purge(request):
@csrf_exempt @csrf_exempt
def api_varnish_purge(request): def api_varnish_purge(request):
if not request.META['REMOTE_ADDR'] in settings.VARNISH_PURGERS: 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': if request.method != 'POST':
return HttpServerError(request, "Can't use this way") raise PermissionDenied("Can't use this way")
n = int(request.POST['n']) n = int(request.POST['n'])
curs = connection.cursor() curs = connection.cursor()
for i in range(0, n): for i in range(0, n):

View File

@ -1,5 +1,6 @@
from django.shortcuts import render, get_object_or_404 from django.shortcuts import render, get_object_or_404
from django.http import HttpResponse, Http404, HttpResponseRedirect from django.http import HttpResponse, Http404, HttpResponseRedirect
from django.core.exceptions import PermissionDenied
from pgweb.util.decorators import login_required from pgweb.util.decorators import login_required
from django.views.decorators.csrf import csrf_exempt from django.views.decorators.csrf import csrf_exempt
from django.conf import settings from django.conf import settings
@ -127,9 +128,9 @@ def ftpbrowser(request, subpath):
@csrf_exempt @csrf_exempt
def uploadftp(request): def uploadftp(request):
if request.method != 'PUT': if request.method != 'PUT':
return HttpServerError(request, "Invalid method") raise PermissionDenied("Invalid method")
if not request.META['REMOTE_ADDR'] in settings.FTP_MASTERS: 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 # We have the data in request.body. Attempt to load it as
# a pickle to make sure it's properly formatted # a pickle to make sure it's properly formatted
pickle.loads(request.body) pickle.loads(request.body)
@ -158,9 +159,9 @@ def uploadftp(request):
@csrf_exempt @csrf_exempt
def uploadyum(request): def uploadyum(request):
if request.method != 'PUT': if request.method != 'PUT':
return HttpServerError(request, "Invalid method") raise PermissionDenied("Invalid method")
if not request.META['REMOTE_ADDR'] in settings.FTP_MASTERS: 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 # We have the data in request.body. Attempt to load it as
# json to ensure correct format. # json to ensure correct format.
json.loads(request.body.decode('utf8')) json.loads(request.body.decode('utf8'))

View File

@ -6,7 +6,7 @@ from django.views.decorators.csrf import csrf_exempt
from pgweb.util.contexts import render_pgweb from pgweb.util.contexts import render_pgweb
from pgweb.util.misc import get_client_ip, varnish_purge 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 from .models import Survey, SurveyAnswer, SurveyLock
@ -30,7 +30,7 @@ def vote(request, surveyid):
try: try:
ansnum = int(request.POST['answer']) ansnum = int(request.POST['answer'])
if ansnum < 1 or ansnum > 8: if ansnum < 1 or ansnum > 8:
return HttpServerError(request, "Invalid answer") return HttpSimpleResponse(request, "Response error", "Invalid answer")
except Exception as e: except Exception as e:
# When no answer is given, redirect to results instead # When no answer is given, redirect to results instead
return HttpResponseRedirect("/community/survey/%s-%s" % (surv.id, slugify(surv.question))) return HttpResponseRedirect("/community/survey/%s-%s" % (surv.id, slugify(surv.question)))
@ -46,7 +46,7 @@ def vote(request, surveyid):
# Check if we are locked # Check if we are locked
lock = SurveyLock.objects.filter(ipaddr=addr) lock = SurveyLock.objects.filter(ipaddr=addr)
if len(lock) > 0: 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 # Generate a new lock item, and store it
lock = SurveyLock(ipaddr=addr) lock = SurveyLock(ipaddr=addr)

View File

@ -175,6 +175,13 @@ def HttpServerError(request, msg):
return r return r
def HttpSimpleResponse(request, title, msg):
return render(request, 'simple.html', {
'title': title,
'message': msg,
})
class PgXmlHelper(django.utils.xmlutils.SimplerXMLGenerator): class PgXmlHelper(django.utils.xmlutils.SimplerXMLGenerator):
def __init__(self, outstream, skipempty=False): def __init__(self, outstream, skipempty=False):
django.utils.xmlutils.SimplerXMLGenerator.__init__(self, outstream, 'utf-8') django.utils.xmlutils.SimplerXMLGenerator.__init__(self, outstream, 'utf-8')

8
templates/simple.html Normal file
View File

@ -0,0 +1,8 @@
{%extends "base/page.html"%}
{%block title%}{{title}}{%endblock%}
{%block contents%}
<h1>{{title}}</h1>
<p>
{{message}}
</p>
{%endblock%}