Commit 6e748b5d authored by Jacob Kaplan-Moss's avatar Jacob Kaplan-Moss
Browse files

Fixed #11457: tightened the security check for "next" redirects after logins. 

The new behavior still disallows redirects to off-site URLs, but now allows
redirects of the form `/some/other/view?foo=http://...`.

Thanks to brutasse.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@12635 bcc190cf-cafb-0310-a4f2-bffc1f526a37
parent c8015052

django/contrib/auth/tests/views.py

+42 −1
Original line number Diff line number Diff line 
import os

import re

import urllib



from django.conf import settings

from django.contrib.auth import SESSION_KEY

from django.contrib.auth import SESSION_KEY, REDIRECT_FIELD_NAME

from django.contrib.auth.forms import AuthenticationForm

from django.contrib.sites.models import Site, RequestSite

from django.contrib.auth.models import User
@@ -184,6 +185,46 @@ class LoginTest(AuthViewsTestCase): 
        self.assert_(isinstance(response.context['form'], AuthenticationForm), 

                     'Login form is not an AuthenticationForm')



    def test_security_check(self, password='password'):

        login_url = reverse('django.contrib.auth.views.login')



        # Those URLs should not pass the security check

        for bad_url in ('http://example.com',

                        'https://example.com',

                        'ftp://exampel.com',

                        '//example.com'):



            nasty_url = '%(url)s?%(next)s=%(bad_url)s' % {

                'url': login_url,

                'next': REDIRECT_FIELD_NAME,

                'bad_url': urllib.quote(bad_url)

            }

            response = self.client.post(nasty_url, {

                'username': 'testclient',

                'password': password,

                }

            )

            self.assertEquals(response.status_code, 302)

            self.assertFalse(bad_url in response['Location'], "%s should be blocked" % bad_url)



        # Now, these URLs have an other URL as a GET parameter and therefore

        # should be allowed

        for url_ in ('http://example.com', 'https://example.com',

                    'ftp://exampel.com',  '//example.com'):

            safe_url = '%(url)s?%(next)s=/view/?param=%(safe_param)s' % {

                'url': login_url,

                'next': REDIRECT_FIELD_NAME,

                'safe_param': urllib.quote(url_)

            }

            response = self.client.post(safe_url, {

                    'username': 'testclient',

                    'password': password,

                }

            )

            self.assertEquals(response.status_code, 302)

            self.assertTrue('/view/?param=%s' % url_ in response['Location'], "/view/?param=%s should be allowed" % url_)



        

class LogoutTest(AuthViewsTestCase):

    urls = 'django.contrib.auth.tests.urls'

django/contrib/auth/views.py

+23 −4
Original line number Diff line number Diff line 
import re

from django.conf import settings

from django.contrib.auth import REDIRECT_FIELD_NAME

# Avoid shadowing the login() view below.

from django.contrib.auth import login as auth_login

from django.contrib.auth.decorators import login_required

from django.contrib.auth.forms import AuthenticationForm

from django.contrib.auth.forms import PasswordResetForm, SetPasswordForm, PasswordChangeForm
@@ -20,26 +23,42 @@ from django.views.decorators.cache import never_cache 
def login(request, template_name='registration/login.html',

          redirect_field_name=REDIRECT_FIELD_NAME,

          authentication_form=AuthenticationForm):

    "Displays the login form and handles the login action."

    """Displays the login form and handles the login action."""



    redirect_to = request.REQUEST.get(redirect_field_name, '')

    

    if request.method == "POST":

        form = authentication_form(data=request.POST)

        if form.is_valid():

            # Light security check -- make sure redirect_to isn't garbage.

            if not redirect_to or '//' in redirect_to or ' ' in redirect_to:

            if not redirect_to or ' ' in redirect_to:

                redirect_to = settings.LOGIN_REDIRECT_URL

            

            # Heavier security check -- redirects to http://example.com should 

            # not be allowed, but things like /view/?param=http://example.com 

            # should be allowed. This regex checks if there is a '//' *before* a

            # question mark.

            elif '//' in redirect_to and re.match(r'[^\?]*//', redirect_to):

                    redirect_to = settings.LOGIN_REDIRECT_URL

            from django.contrib.auth import login

            login(request, form.get_user())

            

            # Okay, security checks complete. Log the user in.

            auth_login(request, form.get_user())



            if request.session.test_cookie_worked():

                request.session.delete_test_cookie()



            return HttpResponseRedirect(redirect_to)



    else:

        form = authentication_form(request)

    

    request.session.set_test_cookie()

    

    if Site._meta.installed:

        current_site = Site.objects.get_current()

    else:

        current_site = RequestSite(request)

    

    return render_to_response(template_name, {

        'form': form,

        redirect_field_name: redirect_to,