Make sanitise_redirect_url map the empty string to LOGIN_REDIRECT_URL. Fixes bug #510866.

django_openid_auth/__init__.py

@@ -2,7 +2,6 @@
 #
 # Copyright (C) 2007 Simon Willison
 # Copyright (C) 2008-2009 Canonical Ltd.
-# Copyright (C) 2010 Dave Walker
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions
@@ -28,25 +27,3 @@
 # ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 # POSSIBILITY OF SUCH DAMAGE.
 
-""" Support for allowing openid authentication for /admin (django.contrib.admin) """
-
-from django.conf import settings
-
-if getattr(settings, 'OPENID_USE_AS_ADMIN_LOGIN', False):
-    from django.http import HttpResponseRedirect
-    from django.contrib.admin import sites
-    from django_openid_auth import views
-
-    def _openid_login(self, request, error_message='', extra_context=None):
-        if request.user.is_authenticated():
-            if not request.user.is_staff:
-                return views.render_failure(request, "User %s does not have admin access." 
-                    % request.user.username)
-            return views.render_failure(request, "Unknown Error: %s" % error_message)
-        else:
-            # Redirect to openid login path,
-            return HttpResponseRedirect(settings.LOGIN_URL+"?next="+request.get_full_path())
-        
-    # Overide the standard admin login form. 
-    sites.AdminSite.display_login_form = _openid_login
-

django_openid_auth/admin.py

 # django-openid-auth -  OpenID integration for django.contrib.auth
 #
 # Copyright (C) 2008-2009 Canonical Ltd.
+# Copyright (C) 2010 Dave Walker
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions
@@ -26,6 +27,7 @@
 # ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 # POSSIBILITY OF SUCH DAMAGE.
 
+from django.conf import settings
 from django.contrib import admin
 from django_openid_auth.models import Nonce, Association, UserOpenID
 from django_openid_auth.store import DjangoOpenIDStore
@@ -64,3 +66,25 @@ class UserOpenIDAdmin(admin.ModelAdmin):
     search_fields = ('claimed_id',)
 
 admin.site.register(UserOpenID, UserOpenIDAdmin)
+
+
+# Support for allowing openid authentication for /admin (django.contrib.admin)
+if getattr(settings, 'OPENID_USE_AS_ADMIN_LOGIN', False):
+    from django.http import HttpResponseRedirect
+    from django_openid_auth import views
+
+    def _openid_login(self, request, error_message='', extra_context=None):
+        if request.user.is_authenticated():
+            if not request.user.is_staff:
+                return views.render_failure(
+                    request, "User %s does not have admin access."
+                    % request.user.username)
+            return views.render_failure(
+                request, "Unknown Error: %s" % error_message)
+        else:
+            # Redirect to openid login path,
+            return HttpResponseRedirect(
+                settings.LOGIN_URL + "?next=" + request.get_full_path())
+
+    # Overide the standard admin login form.
+    admin.sites.AdminSite.display_login_form = _openid_login

django_openid_auth/forms.py

@@ -27,7 +27,6 @@
 # ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 # POSSIBILITY OF SUCH DAMAGE.
 
-import settings
 from django import forms
 from django.contrib.auth.admin import UserAdmin
 from django.contrib.auth.forms import UserChangeForm

django_openid_auth/tests/test_views.py

@@ -126,6 +126,7 @@ class RelyingPartyTests(TestCase):
         self.provider = StubOpenIDProvider('http://example.com/')
         setDefaultFetcher(self.provider, wrap_exceptions=False)
 
+        self.old_login_redirect_url = getattr(settings, 'LOGIN_REDIRECT_URL', '/accounts/profile/')
         self.old_create_users = getattr(settings, 'OPENID_CREATE_USERS', False)
         self.old_update_details = getattr(settings, 'OPENID_UPDATE_DETAILS_FROM_SREG', False)
         self.old_sso_server_url = getattr(settings, 'OPENID_SSO_SERVER_URL')
@@ -139,6 +140,7 @@ class RelyingPartyTests(TestCase):
         settings.OPENID_USE_AS_ADMIN_LOGIN = False
 
     def tearDown(self):
+        settings.LOGIN_REDIRECT_URL = self.old_login_redirect_url
         settings.OPENID_CREATE_USERS = self.old_create_users
         settings.OPENID_UPDATE_DETAILS_FROM_SREG = self.old_update_details
         settings.OPENID_SSO_SERVER_URL = self.old_sso_server_url
@@ -190,6 +192,31 @@ class RelyingPartyTests(TestCase):
         response = self.client.get('/getuser/')
         self.assertEquals(response.content, 'someuser')
 
+    def test_login_no_next(self):
+        """Logins with no next parameter redirect to LOGIN_REDIRECT_URL."""
+        user = User.objects.create_user('someuser', 'someone@example.com')
+        useropenid = UserOpenID(
+            user=user,
+            claimed_id='http://example.com/identity',
+            display_id='http://example.com/identity')
+        useropenid.save()
+
+        settings.LOGIN_REDIRECT_URL = '/getuser/'
+        response = self.client.post('/openid/login/',
+            {'openid_identifier': 'http://example.com/identity'})
+        self.assertContains(response, 'OpenID transaction in progress')
+
+        openid_request = self.provider.parseFormPost(response.content)
+        self.assertEquals(openid_request.mode, 'checkid_setup')
+        self.assertTrue(openid_request.return_to.startswith(
+                'http://testserver/openid/complete/'))
+
+        # Complete the request.  The user is redirected to the next URL.
+        openid_response = openid_request.answer(True)
+        response = self.complete(openid_response)
+        self.assertRedirects(
+            response, 'http://testserver' + settings.LOGIN_REDIRECT_URL)
+
     def test_login_sso(self):
         settings.OPENID_SSO_SERVER_URL = 'http://example.com/identity'
         user = User.objects.create_user('someuser', 'someone@example.com')
@@ -381,6 +408,9 @@ class HelperFunctionsTest(TestCase):
             ("http://example.net/foo/bar?baz=quux", False),
             ("/somewhere/local", True),
             ("/somewhere/local?url=http://fail.com/bar", True),
+            # An empty path, as seen when no "next" parameter is passed.
+            ("", False),
+            ("/path with spaces", False),
         ]
         for url, returns_self in urls:
             sanitised = sanitise_redirect_url(url)

django_openid_auth/views.py

@@ -64,7 +64,10 @@ def is_valid_next_url(next):
 def sanitise_redirect_url(redirect_to):
     """Sanitise the redirection URL."""
     # Light security check -- make sure redirect_to isn't garbage.
-    if not redirect_to or '//' in redirect_to or ' ' in redirect_to:
+    is_valid = True
+    if not redirect_to or ' ' in redirect_to:
+        is_valid = False
+    elif '//' in redirect_to:
         # Allow the redirect URL to be external if it's a permitted domain
         allowed_domains = getattr(settings, 
             "ALLOWED_EXTERNAL_OPENID_REDIRECT_DOMAINS", [])
@@ -75,11 +78,12 @@ def sanitise_redirect_url(redirect_to):
             if netloc.find(":") != -1:
                 netloc, _ = netloc.split(":", 1)
             if netloc not in allowed_domains:
-                redirect_to = settings.LOGIN_REDIRECT_URL
-        else:
-            # netloc is blank, so it's a local URL (possibly with another URL
-            # passed in the querystring. Allow it.)
-            pass
+                is_valid = False
+
+    # If the return_to URL is not valid, use the default.
+    if not is_valid:
+        redirect_to = settings.LOGIN_REDIRECT_URL
+
     return redirect_to