misc: prevent internal URL leak in browser history (#47302)

This commit is contained in:
Benjamin Dauvergne 2020-10-02 18:59:44 +02:00
parent dcb4b40b39
commit d47bc8e1ad
7 changed files with 55 additions and 38 deletions

View File

@ -26,7 +26,6 @@ from django.utils.deprecation import MiddlewareMixin
from django.utils.functional import SimpleLazyObject
from django.utils.translation import ugettext as _
from django.utils.six.moves.urllib import parse as urlparse
from django.shortcuts import render
from django import http
from . import app_settings, utils, plugins
@ -163,21 +162,9 @@ class DisplayMessageBeforeRedirectMiddleware(MiddlewareMixin):
return response
# Check if there is some messages to show
storage = messages.get_messages(request)
if not storage:
if not len(storage):
return response
only_info = True
some_message = False
for message in storage:
some_message = True
if message.level != messages.INFO:
# If there are warnin or error messages, the intermediate page must not redirect
# automatically but should ask for an user confirmation
only_info = False
storage.used = False
if not some_message:
return response
return render(request, 'authentic2/display_message_and_continue.html',
{'url': url, 'only_info': only_info})
return utils.redirect(request, 'continue', resolve=True, params={'next': url})
class ServiceAccessControlMiddleware(MiddlewareMixin):

View File

@ -114,6 +114,7 @@ urlpatterns = [
url(r'^idp/', include('authentic2.idp.urls')),
url(r'^manage/', include('authentic2.manager.urls')),
url(r'^api/', include('authentic2.api_urls')),
url(r'^continue/$', views.display_message_and_continue, name='continue'),
]
try:

View File

@ -1363,3 +1363,31 @@ def old_view_redirect(request, to, message=None):
if message:
messages.info(request, message)
return utils.redirect(request, to=to)
class DisplayMessageAndContinueView(TemplateView):
template_name = 'authentic2/display_message_and_continue.html'
def get(self, request, *args, **kwargs):
self.url = utils.select_next_url(self.request, reverse('account_management'))
self.only_info = True
storage = messages.get_messages(request)
if not len(storage):
return utils.redirect(request, self.url, resolve=False)
for message in storage:
if message.level != messages.INFO:
# If there are warning or error messages, the intermediate page must not redirect
# automatically but should ask for an user confirmation
self.only_info = False
storage.used = False
return super().get(request, *args, **kwargs)
def get_context_data(self, **kwargs):
ctx = super().get_context_data(**kwargs)
ctx['url'] = self.url
ctx['only_info'] = self.only_info
return ctx
display_message_and_continue = DisplayMessageAndContinueView.as_view()

View File

@ -106,13 +106,11 @@ def test_create(settings, app, franceconnect, hooks):
response.form.set('new_password1', 'ikKL1234')
response.form.set('new_password2', 'ikKL1234')
response = response.form.submit(name='unlink')
assert 'The link with the FranceConnect account has been deleted' in response.text
assert models.FcAccount.objects.count() == 0
continue_url = response.pyquery('a#a2-continue').attr['href']
state = urlparse.parse_qs(urlparse.urlparse(continue_url).query)['state'][0]
assert app.session['fc_states'][state]['next'] == '/accounts/'
response = app.get(reverse('fc-logout') + '?state=' + state)
response = franceconnect.handle_logout(app, response.location)
assert path(response['Location']) == '/accounts/'
response = response.follow()
assert 'The link with the FranceConnect account has been deleted' in response
def test_create_expired(settings, app, franceconnect, hooks):
@ -164,11 +162,11 @@ def test_unlink_after_login_with_fc(app, franceconnect, simple_user):
response.form.set('new_password1', 'ikKL1234')
response.form.set('new_password2', 'ikKL1234')
response = response.form.submit(name='unlink')
assert 'The link with the FranceConnect account has been deleted' in response.text
assert models.FcAccount.objects.count() == 0
continue_url = response.pyquery('a#a2-continue').attr['href']
response = franceconnect.handle_logout(app, continue_url)
response = franceconnect.handle_logout(app, response.location)
assert path(response.location) == '/accounts/'
response = response.follow()
assert 'The link with the FranceConnect account has been deleted' in response
def test_login_email_is_unique_and_already_linked(settings, app, franceconnect, caplog):
@ -268,11 +266,11 @@ def test_registration1(settings, app, franceconnect, caplog, hooks):
response.form.set('new_password1', 'ikKL1234')
response.form.set('new_password2', 'ikKL1234')
response = response.form.submit(name='unlink')
assert 'The link with the FranceConnect account has been deleted' in response.text
assert models.FcAccount.objects.count() == 0
continue_url = response.pyquery('a#a2-continue').attr['href']
response = franceconnect.handle_logout(app, continue_url)
response = franceconnect.handle_logout(app, response.location)
assert path(response.location) == '/accounts/'
response = response.follow()
assert 'The link with the FranceConnect account has been deleted' in response
def test_registration2(settings, app, franceconnect, hooks):
@ -332,9 +330,11 @@ def test_can_change_password(settings, app, franceconnect):
response.form.set('new_password1', 'ikKL1234')
response.form.set('new_password2', 'ikKL1234')
response = response.form.submit(name='unlink')
assert 'The link with the FranceConnect account has been deleted' in response.text
continue_url = response.pyquery('a#a2-continue').attr['href']
response = franceconnect.handle_logout(app, continue_url).follow()
assert models.FcAccount.objects.count() == 0
response = franceconnect.handle_logout(app, response.location)
assert path(response.location) == '/accounts/'
response = response.follow()
assert 'The link with the FranceConnect account has been deleted' in response
assert len(response.pyquery('[href*="password/change"]')) > 0

View File

@ -56,3 +56,4 @@ A2_MAX_EMAILS_PER_IP = None
A2_MAX_EMAILS_FOR_ADDRESS = None
A2_TOKEN_EXISTS_WARNING = False
A2_REDIRECT_WHITELIST = ['http://sp.org/']

View File

@ -497,7 +497,7 @@ class APITest(TestCase):
# User side
client = Client()
activation_url = get_link_from_mail(mail.outbox[-1])
response = client.get(activation_url)
response = client.get(activation_url, follow=True)
self.assertEqual(response.status_code, status.HTTP_200_OK)
assert utils.make_url(return_url, params={'token': token}) in force_text(response.content)
self.assertEqual(User.objects.count(), user_count + 1)
@ -611,7 +611,7 @@ class APITest(TestCase):
# User side - user click on email
client = Client()
activation_url = get_link_from_mail(mail.outbox[0])
response = client.get(activation_url)
response = client.get(activation_url, follow=True)
self.assertEqual(response.status_code, status.HTTP_200_OK)
assert utils.make_url(return_url, params={'token': token}) in force_text(response.content)
self.assertEqual(User.objects.count(), user_count + 1)
@ -717,7 +717,7 @@ class APITest(TestCase):
# User side - user click on first email
client = Client()
activation_url = get_link_from_mail(activation_mail1)
response = client.get(activation_url)
response = client.get(activation_url, follow=True)
self.assertEqual(response.status_code, status.HTTP_200_OK)
assert utils.make_url(return_url, params={'token': token}) in force_text(response.content)
self.assertEqual(User.objects.count(), user_count + 1)

View File

@ -75,13 +75,13 @@ def test_registration_success(app, db, settings, mailoutbox, external_redirect):
# set valid password
response.form.set('password1', 'T0==toto')
response.form.set('password2', 'T0==toto')
response = response.form.submit()
response = response.form.submit().maybe_follow()
if good_next_url:
assert 'You have just created an account.' in response.text
assert next_url in response.text
assert response.request.path == '/continue/'
else:
assert urlparse(response['Location']).path == '/'
response = response.follow()
assert response.request.path == '/'
assert 'You have just created an account.' in response.text
assert User.objects.count() == 1
assert len(mailoutbox) == 2
@ -134,7 +134,7 @@ def test_registration_realm(app, db, settings, mailoutbox):
response.form.set('username', 'toto')
response.form.set('password1', 'T0==toto')
response.form.set('password2', 'T0==toto')
response = response.form.submit()
response = response.form.submit().follow()
assert 'You have just created an account.' in response.text
assert next_url in response.text
assert len(mailoutbox) == 2
@ -572,7 +572,7 @@ def test_registration_redirect(app, db, settings, mailoutbox, external_redirect)
response = app.get(link)
response.form.set('password1', 'T0==toto')
response.form.set('password2', 'T0==toto')
response = response.form.submit()
response = response.form.submit().follow()
assert 'You have just created an account.' in response.text
assert new_next_url in response.text
@ -618,7 +618,7 @@ def test_registration_redirect_tuple(app, db, settings, mailoutbox, external_red
response = app.get(link)
response.form.set('password1', 'T0==toto')
response.form.set('password2', 'T0==toto')
response = response.form.submit()
response = response.form.submit().follow()
assert new_next_url in response.text