entrouvert
/
authentic
16
0
Fork 0
Code Issues Pull Requests 32 Releases Activity

empêcher le contrôle des redirections sur les mails d'enregistrement et de réinitialisation de mot de passe (#76835) #171

Open
bdauvergne wants to merge 2 commits from wip/76835-open-redirection into main
pull from: wip/76835-open-redirection
merge into: entrouvert:main
Conversation 1 Commits 2 Files Changed 10 +111 -119
10 changed files with 111 additions and 119 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
src/authentic2/cbv.py
View File

@ -14,12 +14,13 @@
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
from django.conf import settings
from django.contrib.auth import REDIRECT_FIELD_NAME
import functools
from django.forms import Form
from django.utils.decorators import method_decorator
from django.views.decorators.csrf import csrf_exempt, ensure_csrf_cookie
from .forms.utils import NextUrlFormMixin
from .utils import hooks
from .utils import misc as utils_misc
from .utils.views import csrf_token_check
@ -47,39 +48,35 @@ class ValidateCSRFMixin:
return super().form_valid(*args, **kwargs)
class RedirectToNextURLViewMixin:
@functools.cache
def make_next_url_form_class(form_class):
if issubclass(form_class, NextUrlFormMixin):
return form_class
return type(f'NextURL{form_class.__name__}', (form_class, NextUrlFormMixin), {})
class NextUrlViewMixin:
'''Mixin to FormView of forms using NextUrlFormMixin'''
next_url = None
def setup(self, request, *args, **kwargs):
super().setup(request, *args, **kwargs)
self.next_url = utils_misc.select_next_url(request, self.next_url, include_post=True)
def get_form_class(self):
return make_next_url_form_class(super().get_form_class())
def get_form_kwargs(self):
kwargs = super().get_form_kwargs()
kwargs.setdefault('initial', {})['next_url'] = self.next_url
kwargs['request'] = self.request
return kwargs
class SuccessUrlViewMixin(NextUrlViewMixin):
def get_success_url(self):
if REDIRECT_FIELD_NAME in self.request.GET:
return self.request.GET[REDIRECT_FIELD_NAME]
return settings.LOGIN_REDIRECT_URL
class NextURLViewMixin(RedirectToNextURLViewMixin):
"""Make a view handle a next parameter, if it's not present it is
automatically generated from the Referrer or from the value
returned by the method get_next_url_default().
"""
next_url_default = '..'
def get_next_url_default(self):
return self.next_url_default
def dispatch(self, request, *args, **kwargs):
if REDIRECT_FIELD_NAME in request.GET:
pass
else:
next_url = request.headers.get('Referer') or self.next_url_default
return utils_misc.redirect(
request,
request.path,
keep_params=True,
params={
REDIRECT_FIELD_NAME: next_url,
},
status=303,
)
return super().dispatch(request, *args, **kwargs)
return self.next_url or super().get_success_url()
class TemplateNamesMixin:

8
src/authentic2/forms/passwords.py
View File

@ -42,9 +42,7 @@ from .utils import NextUrlFormMixin
logger = logging.getLogger(__name__)
class PasswordResetForm(HoneypotForm):
next_url = forms.CharField(widget=forms.HiddenInput, required=False)
class PasswordResetForm(NextUrlFormMixin, HoneypotForm):
email = ValidatedEmailField(label=_('Email'), required=False)
phone = PhoneField(
@ -273,9 +271,7 @@ class SetPasswordForm(NotifyOfPasswordChange, PasswordResetMixin, auth_forms.Set
return new_password1
class PasswordChangeForm(
NotifyOfPasswordChange, NextUrlFormMixin, PasswordResetMixin, auth_forms.PasswordChangeForm
):
class PasswordChangeForm(NotifyOfPasswordChange, PasswordResetMixin, auth_forms.PasswordChangeForm):
old_password = PasswordField(label=_('Old password'))
new_password1 = NewPasswordField(label=_('New password'))
new_password2 = CheckPasswordField(label=_('New password confirmation'))

26
src/authentic2/forms/utils.py
View File

@ -15,19 +15,23 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
from django import forms
from django.contrib.auth import REDIRECT_FIELD_NAME
from ..middleware import StoreRequestMiddleware
from authentic2.utils.misc import good_next_url
class NextUrlFormMixin(forms.Form):
class FormNeedsRequest:
def __init__(self, *args, **kwargs):
self.request = kwargs.pop('request')
super().__init__(*args, **kwargs)
class NextUrlFormMixin(FormNeedsRequest, forms.Form):
'''Use with authentic.cbv.NextUrlViewMixin.'''
next_url = forms.CharField(widget=forms.HiddenInput(), required=False)
def __init__(self, *args, **kwargs):
next_url = kwargs.pop('next_url', None)
request = StoreRequestMiddleware.get_request()
if not next_url and request:
next_url = request.GET.get(REDIRECT_FIELD_NAME)
super().__init__(*args, **kwargs)
if next_url:
self.fields['next_url'].initial = next_url
def clean_next_url(self):
next_url = self.cleaned_data.get('next_url')
if not good_next_url(self.request, next_url):
return ''
return next_url

10
src/authentic2/templates/registration/password_change_done.html
View File

@ -1,10 +0,0 @@
{% extends "authentic2/base-page.html" %}
{% load i18n %}
{% block title %}
{{ view.title }}
{% endblock %}
{% block content %}
<p>{% trans "Password changed" %}</p>
{% endblock %}

6
src/authentic2/urls.py
View File

@ -16,7 +16,6 @@
from django.conf import settings
from django.contrib import admin
from django.contrib.auth import views as dj_auth_views
from django.contrib.auth.decorators import login_required
from django.contrib.staticfiles.views import serve
from django.urls import include, path, re_path
@ -75,11 +74,6 @@ accounts_urlpatterns = [
path('', views.profile, name='account_management'),
# Password change
path('password/change/', views.password_change, name='password_change'),
path(
'password/change/done/',
dj_auth_views.PasswordChangeDoneView.as_view(),
name='password_change_done',
),
# permament redirections for views moved to root
path('register/', RedirectView.as_view(permanent=True, pattern_name='registration_register')),
path('register/complete/', RedirectView.as_view(permanent=True, pattern_name='registration_complete')),

75
src/authentic2/views.py
View File

@ -87,10 +87,11 @@ class HomeURLMixin:
return super().dispatch(request, *args, **kwargs)
class EditProfile(HomeURLMixin, cbv.HookMixin, cbv.TemplateNamesMixin, UpdateView):
class EditProfile(HomeURLMixin, cbv.SuccessUrlViewMixin, cbv.HookMixin, cbv.TemplateNamesMixin, UpdateView):
model = User
template_names = ['profiles/edit_profile.html', 'authentic2/accounts_edit.html']
title = _('Edit account data')
success_url = reverse_lazy('account_management')
def get_template_names(self):
template_names = []
@ -133,7 +134,8 @@ class EditProfile(HomeURLMixin, cbv.HookMixin, cbv.TemplateNamesMixin, UpdateVie
fields = [field for field in fields if field in default_fields]
return fields, labels
def get_form_class(self):
@property
def form_class(self):
if 'scope' in self.kwargs:
scopes = [self.kwargs['scope']]
else:
@ -152,19 +154,9 @@ class EditProfile(HomeURLMixin, cbv.HookMixin, cbv.TemplateNamesMixin, UpdateVie
def get_object(self):
return self.request.user
def get_form_kwargs(self, **kwargs):
kwargs = super().get_form_kwargs(**kwargs)
kwargs['next_url'] = utils_misc.select_next_url(self.request, reverse('account_management'))
return kwargs
def get_success_url(self):
return utils_misc.select_next_url(
self.request, default=reverse('account_management'), field_name='next_url', include_post=True
)
def post(self, request, *args, **kwargs):
if 'cancel' in request.POST:
return utils_misc.redirect(request, self.get_success_url())
return utils_misc.redirect(request, self.next_url or str(self.success_url))
return super().post(request, *args, **kwargs)
def form_valid(self, form):
@ -1105,16 +1097,16 @@ def csrf_failure_view(request, reason=''):
return HttpResponseRedirect(request.get_full_path())
class PasswordResetView(FormView):
class PasswordResetView(cbv.NextUrlViewMixin, FormView):
'''Ask for an email and send a password reset link by mail'''
form_class = passwords_forms.PasswordResetForm
title = _('Password Reset')
code = None
def dispatch(self, *args, **kwargs):
def setup(self, request, *args, **kwargs):
super().setup(request, *args, **kwargs)
self.authenticator = utils_misc.get_password_authenticator()
return super().dispatch(*args, **kwargs)
def get_success_url(self):
if (
@ -1136,9 +1128,6 @@ class PasswordResetView(FormView):
def get_form_kwargs(self, **kwargs):
kwargs = super().get_form_kwargs(**kwargs)
kwargs['password_authenticator'] = self.authenticator
initial = kwargs.setdefault('initial', {})
if next_url := utils_misc.select_next_url(self.request, ''):
initial['next_url'] = next_url
return kwargs
def get_context_data(self, **kwargs):
@ -1325,11 +1314,12 @@ class TokenLoginView(RedirectView):
token_login = TokenLoginView.as_view()
class PasswordResetConfirmView(cbv.RedirectToNextURLViewMixin, FormView):
class PasswordResetConfirmView(cbv.SuccessUrlViewMixin, FormView):
"""Validate password reset link, show a set password form and login
the user.
"""
success_url = reverse_lazy('auth_homepage')
form_class = passwords_forms.SetPasswordForm
title = _('Password Reset')
@ -1400,20 +1390,15 @@ class PasswordResetConfirmView(cbv.RedirectToNextURLViewMixin, FormView):
hooks.call_hooks('event', name='password-reset-confirm', user=form.user, token=self.token, form=form)
logger.info('password reset for user %s with token %r', self.user, self.token.uuid)
self.token.delete()
return self.finish()
def finish(self):
response = utils_misc.simulate_authentication(
self.request, self.user, 'email', next_url=self.next_url
)
utils_misc.simulate_authentication(self.request, self.user, 'email')
self.request.journal.record('user.password.reset')
return response
return super().form_valid(form)
password_reset_confirm = PasswordResetConfirmView.as_view()
class BaseRegistrationView(HomeURLMixin, FormView):
class BaseRegistrationView(HomeURLMixin, cbv.NextUrlViewMixin, FormView):
form_class = registration_forms.RegistrationForm
template_name = 'registration/registration_form.html'
title = _('Registration')
@ -1445,12 +1430,7 @@ class BaseRegistrationView(HomeURLMixin, FormView):
return HttpResponseBadRequest('invalid token', content_type='text/plain')
if 'ou' in self.token:
self.ou = OU.objects.get(pk=self.token['ou'])
if self.token.get(REDIRECT_FIELD_NAME):
self.next_url = self.token.pop(REDIRECT_FIELD_NAME)
elif (next_url := request.GET.get(REDIRECT_FIELD_NAME)) and utils_misc.good_next_url(
request, next_url
):
self.next_url = next_url
self.next_url = self.token.pop(REDIRECT_FIELD_NAME, self.next_url)
set_home_url(request, self.next_url)
return super().dispatch(request, *args, **kwargs)
@ -1597,7 +1577,8 @@ class BaseRegistrationView(HomeURLMixin, FormView):
return self.form_invalid(form)
for field in form.cleaned_data:
self.token[field] = form.cleaned_data[field]
if field in app_settings.A2_PRE_REGISTRATION_FIELDS:
self.token[field] = form.cleaned_data[field]
self.token.pop(REDIRECT_FIELD_NAME, None)
self.token.pop('email', None)
@ -1621,7 +1602,7 @@ class BaseRegistrationView(HomeURLMixin, FormView):
return context
class InputSMSCodeView(cbv.ValidateCSRFMixin, FormView):
class InputSMSCodeView(cbv.ValidateCSRFMixin, cbv.NextUrlViewMixin, FormView):
template_name = 'registration/sms_input_code.html'
form_class = registration_forms.InputSMSCodeForm
success_url = '/accounts/'
@ -2272,23 +2253,21 @@ registration_complete = RegistrationCompleteView.as_view()
class PasswordChangeView(HomeURLMixin, DjPasswordChangeView):
title = _('Password Change')
do_not_call_in_templates = True
success_url = reverse_lazy('account_management')
def redirect(self):
return HttpResponseRedirect(self.get_success_url())
def dispatch(self, request, *args, **kwargs):
if 'next_url' in request.POST and request.POST['next_url']:
self.post_change_redirect = request.POST['next_url']
elif REDIRECT_FIELD_NAME in request.GET:
self.post_change_redirect = request.GET[REDIRECT_FIELD_NAME]
else:
self.post_change_redirect = reverse('account_management')
if not utils_misc.user_can_change_password(request=request):
messages.warning(request, _('Password change is forbidden'))
return utils_misc.redirect(request, self.post_change_redirect)
return self.redirect()
hooks.call_hooks('password_change_view', request=self.request)
return super().dispatch(request, *args, **kwargs)
def post(self, request, *args, **kwargs):
if 'cancel' in request.POST:
return utils_misc.redirect(request, self.post_change_redirect)
return self.redirect()
return super().post(request, *args, **kwargs)
def form_valid(self, form):
@ -2303,17 +2282,13 @@ class PasswordChangeView(HomeURLMixin, DjPasswordChangeView):
self.request.journal.record('user.password.change', session=self.request.session)
return response
def get_form_class(self):
@property
def form_class(self):
if self.request.user.has_usable_password():
return passwords_forms.PasswordChangeForm
else:
return passwords_forms.SetPasswordForm
def get_context_data(self, **kwargs):
ctx = super().get_context_data(**kwargs)
ctx[REDIRECT_FIELD_NAME] = self.post_change_redirect
return ctx
password_change = decorators.setting_enabled('A2_REGISTRATION_CAN_CHANGE_PASSWORD')(
PasswordChangeView.as_view()

2
tests/test_all.py
View File

@ -235,7 +235,7 @@ class UserProfileTests(TestCase):
kwargs = {'%s-%s' % (form.prefix, k): v for k, v in kwargs.items()}
response = self.client.post(reverse('profile_edit'), kwargs)
new = {'custom': 'random data', 'next_url': '', 'national_number': 'xx20153566342yy'}
new = {'custom': 'random data', 'national_number': 'xx20153566342yy'}
assert_event('user.profile.edit', user=self.user, session=self.client.session, old={}, new=new)
self.assertEqual(response.status_code, 302)

18
tests/test_password_reset.py
View File

@ -25,6 +25,7 @@ from django.urls import reverse
from authentic2.apps.authenticators.models import LoginPasswordAuthenticator
from authentic2.models import Attribute, SMSCode, Token
from authentic2.utils.misc import send_password_reset_mail
from authentic2.views import PasswordResetView
from . import utils
@ -493,3 +494,20 @@ def test_ou_policies(app, db, settings, user_ou1, ou1, user_ou2, ou2, mailoutbox
url = reverse('password_reset')
resp = app.get(url, status=404) # globally deactivated, page not found
def test_open_redirection(db, rf, app):
BAD_URL = 'https://bad.url.com/'
request = rf.get(f'/password/reset/?next={BAD_URL}')
password_reset = PasswordResetView()
password_reset.setup(request)
assert password_reset.get_form_kwargs()['initial'].get('next_url') != BAD_URL
request = rf.post('/password/reset/', {'next_url': BAD_URL, 'email': 'john.doe@example.com'})
password_reset = PasswordResetView()
password_reset.setup(request)
form = password_reset.get_form()
assert form.is_valid()
assert form.cleaned_data['next_url'] == ''

18
tests/test_registration.py
View File

@ -33,6 +33,7 @@ from authentic2.forms.registration import RegistrationCompletionForm
from authentic2.models import Attribute, AttributeValue, SMSCode, Token
from authentic2.utils import misc as utils_misc
from authentic2.validators import EmailValidator
from authentic2.views import RegistrationView
from .utils import assert_event, get_link_from_mail, login
@ -1407,3 +1408,20 @@ def test_already_logged(db, app, simple_user):
# then we can register.
resp = resp.follow()
assert resp.form['email']
def test_open_redirection(app, rf, db):
BAD_URL = 'https://bad.url.com/'
request = rf.get(f'/register/?next={BAD_URL}')
register = RegistrationView()
register.setup(request)
assert register.get_form_kwargs()['initial'].get('next_url') != BAD_URL
request = rf.post('/register/', {'next_url': BAD_URL, 'email': 'john.doe@example.com'})
register = RegistrationView()
register.setup(request)
form = register.get_form()
assert form.is_valid()
assert form.cleaned_data['next_url'] == ''

2
tests/test_views.py
View File

@ -85,7 +85,7 @@ def test_password_change(app, simple_user):
resp.form['new_password2'] = 'hopAbcde1'
resp = resp.form.submit()
assert resp.location == '/accounts/password/change/done/'
assert resp.location == '/accounts/'
new_session_key = app.session.session_key
assert old_session_key != new_session_key, 'session\'s key has not been cycled'