mozreview: update login error handler to use shortcuts.render (bug 1253552) r?smacleod draft
authorbyron jones <glob@mozilla.com>
Thu, 11 Aug 2016 15:26:51 +0800
changeset 9678 6b6cea446b9fe003fbee1fc4581feaae3e32fcd3
parent 9677 184c2ff0e97366ca60ed86e906259595ef218291
child 9679 1336761190e70b2e258242757baff2c18912e998
push id1268
push userbjones@mozilla.com
push dateMon, 10 Oct 2016 14:39:40 +0000
reviewerssmacleod
bugs1253552
mozreview: update login error handler to use shortcuts.render (bug 1253552) r?smacleod Simplify login error renderer, and rename for clarity. MozReview-Commit-ID: 6Nigs0W6YcS
pylib/mozreview/mozreview/views.py
--- a/pylib/mozreview/mozreview/views.py
+++ b/pylib/mozreview/mozreview/views.py
@@ -11,18 +11,17 @@ import uuid
 
 from django.contrib.auth import login
 from django.core.urlresolvers import reverse
 from django.http import (
     HttpResponse,
     HttpResponseRedirect,
     HttpResponseNotAllowed,
 )
-from django.shortcuts import render_to_response
-from django.template.context import RequestContext
+from django.shortcuts import render
 from django.utils import timezone
 
 from djblets.siteconfig.models import SiteConfiguration
 
 from mozreview.bugzilla.client import Bugzilla
 from mozreview.bugzilla.errors import BugzillaError
 from mozreview.ldap import (
     associate_employee_ldap,
@@ -33,22 +32,20 @@ from mozreview.models import (
     set_bugzilla_api_key,
     UnverifiedBugzillaApiKey,
 )
 
 
 logger = logging.getLogger(__name__)
 
 
-def show_error_page(request):
-    return render_to_response(
-        'mozreview/login-error.html', RequestContext(request, {
-            'login_error': 'An error occurred when trying to log you in.',
-        })
-    )
+def render_login_error(request):
+    return render(request, 'mozreview/login-error.html', {
+        'login_error': 'An error occurred when trying to log you in.',
+    })
 
 
 def bmo_auth_callback(request):
     """Callback from Bugzilla for receiving API keys."""
     if request.method == 'GET':
         return get_bmo_auth_callback(request)
     elif request.method == 'POST':
         return post_bmo_auth_callback(request)
@@ -144,17 +141,17 @@ def get_bmo_auth_callback(request):
     bmo_username = request.GET.get('client_api_login', None)
     callback_result = request.GET.get('callback_result', None)
     redirect = request.GET.get('redirect', None)
     secret = request.GET.get('secret', None)
 
     if not (bmo_username and callback_result):
         logger.error('Bugzilla auth callback called without required '
                      'parameters.')
-        return show_error_page(request)
+        return render_login_error(request)
 
     # Delete expired unverified keys (5 minute lifetime).
     UnverifiedBugzillaApiKey.objects.filter(
         timestamp__lte=timezone.now() - timedelta(minutes=5)).delete()
 
     parsed = None if not redirect else urlparse(redirect)
 
     # Enforce relative redirects; we don't want people crafting auth links
@@ -165,75 +162,75 @@ def get_bmo_auth_callback(request):
         redirect = '/'
 
     unverified_keys = UnverifiedBugzillaApiKey.objects.filter(
         bmo_username=bmo_username).order_by('timestamp')
 
     if not unverified_keys:
         logger.error('No unverified keys found for BMO user %s.' %
                      bmo_username)
-        return show_error_page(request)
+        return render_login_error(request)
 
     unverified_key = unverified_keys.last()
 
     if len(unverified_keys) > 1:
         logger.warning('Multiple unverified keys on file for BMO user %s. '
                        'Using most recent, from %s.' %
                        (bmo_username, unverified_key.timestamp))
 
     if callback_result != unverified_key.callback_result:
         logger.error('Callback result does not match for BMO user %s.' %
                      bmo_username)
-        return show_error_page(request)
+        return render_login_error(request)
 
     if secret is None or request.COOKIES['bmo_auth_secret'] != secret:
         logger.error('Callback secret does not match cookie for user %s.' %
                      bmo_username)
-        return show_error_page(request)
+        return render_login_error(request)
 
     bmo_api_key = unverified_key.api_key
     unverified_key.delete()
 
     b = Bugzilla()
 
     try:
         if not b.valid_api_key(bmo_username, bmo_api_key):
             logger.error('Invalid API key for %s.' % bmo_username)
-            return show_error_page(request)
+            return render_login_error(request)
     except BugzillaError as e:
         logger.error('Error validating API key: %s' % e.msg)
-        return show_error_page(request)
+        return render_login_error(request)
 
     b.api_key = bmo_api_key
 
     try:
         user_data = b.get_user(bmo_username)
     except BugzillaError as e:
         logger.error('Error getting user data: %s' % e.msg)
-        return show_error_page(request)
+        return render_login_error(request)
 
     if not user_data:
         logger.warning('Could not retrieve user info for %s after '
                        'validating API key.' % bmo_username)
-        return show_error_page(request)
+        return render_login_error(request)
 
     users = get_or_create_bugzilla_users(user_data)
 
     if not users:
         logger.warning('Failed to create user %s after validating API key.' %
                        bmo_username)
-        return show_error_page(request)
+        return render_login_error(request)
 
     user = users[0]
     assert user.email == bmo_username
 
     if not user.is_active:
         logger.warning('Validated API key but user %s is inactive.' %
                        bmo_username)
-        return show_error_page(request)
+        return render_login_error(request)
 
     set_bugzilla_api_key(user, bmo_api_key)
 
     try:
         associate_employee_ldap(user)
     except LDAPAssociationException as e:
         logger.info('LDAP association failed: %s' % str(e))
     except Exception: