Minor improvements proposal

django/contrib/auth/views.py

@@ -6,6 +6,7 @@
     from urlparse import urlparse, urlunparse
 
 from django.conf import settings
+from django.core.exceptions import ImproperlyConfigured, ValidationError
 from django.core.urlresolvers import reverse_lazy
 from django.http import HttpResponseRedirect, QueryDict
 from django.utils.decorators import method_decorator
@@ -53,12 +54,25 @@ def get_context_data(self, **kwargs):
         return context
 
 
-def is_valid_redirect(url, request, allow_empty=False): # XXX: Name?
-    """"Validate that the given URL is on the same host as the given request."""
+def validate_redirect_url(url, request, allow_empty=False):
+    """"Validate that ``url`` is a valid URL for use in redirections.
+
+    Raise ValidationError if:
+
+    * ``allow_empty`` is False and ``url`` is empty string.
+    * URL is not on the same host as the given request.
+
+    """
     if not url:
-        return allow_empty
-    netloc = urlparse(url)[1]
-    return not netloc or netloc == request.get_host()
+        if not allow_empty:
+            raise ValidationError("URL can't be empty.")
+    else:
+        netloc = urlparse(url)[1]
+        request_host = request.get_host()
+        if netloc and netloc != request_host:
+            raise ValidationError("URL must belong to '%s' host."
+                                  % request_host)
+    return url
 
 
 class LoginView(CurrentAppMixin, CurrentSiteMixin, generic.FormView):
@@ -72,7 +86,7 @@ class LoginView(CurrentAppMixin, CurrentSiteMixin, generic.FormView):
     @method_decorator(csrf_protect)
     @method_decorator(never_cache)
     def dispatch(self, request, *args, **kwargs):
-        request.session.set_test_cookie()
+        self.set_test_cookie(request)
         return super(LoginView, self).dispatch(request, *args, **kwargs)
 
     def get_form_kwargs(self):
@@ -85,25 +99,43 @@ def get_context_data(self, **kwargs):
         context[self.redirect_field_name] = self.get_success_url()
         return context
 
-    def form_valid(self, form):
-        """Log the user in and redirect."""
-        auth_login(self.request, form.get_user())
+    def login(self, user):
+        """Log user in."""
+        auth_login(self.request, user)
+
+    def set_test_cookie(self, request):
+        """Set test cookie to request."""
+        request.session.set_test_cookie()
 
+    def unset_test_cookie(self):
+        """Cleanup test cookie if necessary."""
         if self.request.session.test_cookie_worked():
             self.request.session.delete_test_cookie()
 
+    def form_valid(self, form):
+        """Log the user in and redirect."""
+        self.login(form.get_user())
+        self.unset_test_cookie()
         # Redirect
         return super(LoginView, self).form_valid(form)
-    
+
     def get_success_url(self):
         """
         Look for a redirect URL in the request parameters.
         If none is found, or if it's not valid, use settings.LOGIN_REDIRECT_URL.
 
         """
         redir = self.request.REQUEST.get(self.redirect_field_name)
-        if not is_valid_redirect(redir, self.request, allow_empty=False):
-            redir = settings.LOGIN_REDIRECT_URL
+        try:
+            redir = validate_redirect_url(redir, self.request,
+                                          allow_empty=False)
+        except ValidationError:
+            # Silently fallback to view's ``success_url``.
+            try:
+                redir = super(LoginView, self).get_success_url()
+            except ImproperlyConfigured:
+                # Silently fallback to settings.
+                redir = settings.LOGIN_REDIRECT_URL
         return redir
 
 
@@ -141,12 +173,21 @@ def get_success_url(self):
 
         """
         redir = self.request.REQUEST.get(self.redirect_field_name)
-        if is_valid_redirect(redir, self.request, allow_empty=False):
-            return redir
-        elif self.success_url is not None:
-            return self.success_url or self.request.path
-        else:
-            return None
+        try:
+            return validate_redirect_url(redir, self.request,
+                                         allow_empty=False)
+        except ValidationError:
+            # Silently fallback to view's ``success_url``.
+            try:
+                return super(LogoutView, self).get_success_url()
+            except ImproperlyConfigured:
+                # Silently fallback to current request.path if success_url has
+                # been set to empty value (except None).
+                if self.success_url is not None:
+                    return self.request.path
+                else:
+                    # Silently fallback to None.
+                    return None
 
 
 class LogoutThenLoginView(LogoutView):