From fa9234bf6d501890313fe2e9b63c91a8513eb041 Mon Sep 17 00:00:00 2001 From: Valentin Deniaud Date: Wed, 15 Apr 2020 15:42:42 +0200 Subject: [PATCH 1/4] views: use one-time token for registration (#41792) --- src/authentic2/urls.py | 2 +- src/authentic2/utils/__init__.py | 9 ++++-- src/authentic2/views.py | 36 ++++++++++----------- tests/test_all.py | 35 ++------------------ tests/test_attribute_kinds.py | 55 +++++++++++++++++++++----------- tests/test_registration.py | 53 ++++++++++++++++++++++++++++++ 6 files changed, 116 insertions(+), 74 deletions(-) diff --git a/src/authentic2/urls.py b/src/authentic2/urls.py index ace607fb..e06250de 100644 --- a/src/authentic2/urls.py +++ b/src/authentic2/urls.py @@ -30,7 +30,7 @@ admin.autodiscover() handler500 = 'authentic2.views.server_error' accounts_urlpatterns = [ - url(r'^activate/(?P[\w: -]+)/$', + url(r'^activate/(?P[A-Za-z0-9_ -]+)/$', views.registration_completion, name='registration_activate'), url(r'^register/$', views.RegistrationView.as_view(), diff --git a/src/authentic2/utils/__init__.py b/src/authentic2/utils/__init__.py index 419c8c90..4f287b75 100644 --- a/src/authentic2/utils/__init__.py +++ b/src/authentic2/utils/__init__.py @@ -687,14 +687,19 @@ def get_registration_url(request, service_slug=None): def build_activation_url(request, email, next_url=None, ou=None, **kwargs): + from authentic2.models import Token + data = kwargs.copy() data['email'] = email if ou: data['ou'] = ou.pk data[REDIRECT_FIELD_NAME] = next_url - registration_token = signing.dumps(data) + lifetime = settings.ACCOUNT_ACTIVATION_DAYS * 3600 * 24 + # invalidate any token associated with this address + Token.objects.filter(kind='registration', content__email=email).delete() + token = Token.create('registration', data, duration=lifetime) activate_url = request.build_absolute_uri( - reverse('registration_activate', kwargs={'registration_token': registration_token})) + reverse('registration_activate', kwargs={'registration_token': token.uuid_b64url})) return activate_url diff --git a/src/authentic2/views.py b/src/authentic2/views.py index b8ce9702..d40c3903 100644 --- a/src/authentic2/views.py +++ b/src/authentic2/views.py @@ -770,21 +770,6 @@ class PasswordResetConfirmView(cbv.RedirectToNextURLViewMixin, FormView): password_reset_confirm = PasswordResetConfirmView.as_view() -def valid_token(method): - def f(request, *args, **kwargs): - try: - request.token = signing.loads(kwargs['registration_token'].replace(' ', ''), - max_age=settings.ACCOUNT_ACTIVATION_DAYS * 3600 * 24) - except signing.SignatureExpired: - messages.warning(request, _('Your activation key is expired')) - return utils.redirect(request, 'registration_register') - except signing.BadSignature: - messages.warning(request, _('Activation failed')) - return utils.redirect(request, 'registration_register') - return method(request, *args, **kwargs) - return f - - class BaseRegistrationView(FormView): form_class = registration_forms.RegistrationForm template_name = 'registration/registration_form.html' @@ -889,9 +874,20 @@ class RegistrationCompletionView(CreateView): return url def dispatch(self, request, *args, **kwargs): - self.token = request.token + registration_token = kwargs['registration_token'].replace(' ', '') + try: + token = models.Token.use('registration', registration_token, delete=False) + except models.Token.DoesNotExist: + messages.warning(request, _('Your activation key is unknown or expired')) + return utils.redirect(request, 'registration_register') + except (TypeError, ValueError): + messages.warning(request, _('Activation failed')) + return utils.redirect(request, 'registration_register') + self.token_obj = token + self.token = token.content + self.authentication_method = self.token.get('authentication_method', 'email') - self.email = request.token['email'] + self.email = self.token['email'] if 'ou' in self.token: self.ou = OU.objects.get(pk=self.token['ou']) else: @@ -1087,6 +1083,7 @@ class RegistrationCompletionView(CreateView): ou=self.ou, next_url=self.get_success_url(), **data) + self.token_obj.delete() self.request.session['registered_email'] = form.cleaned_data['email'] return utils.redirect(self.request, 'registration_complete') super(RegistrationCompletionView, self).form_valid(form) @@ -1095,7 +1092,8 @@ class RegistrationCompletionView(CreateView): def registration_success(self, request, user, form): hooks.call_hooks('event', name='registration', user=user, form=form, view=self, authentication_method=self.authentication_method, - token=request.token, service=self.service) + token=self.token, service=self.service) + self.token_obj.delete() utils.simulate_authentication( request, user, method=self.authentication_method, @@ -1123,7 +1121,7 @@ class RegistrationCompletionView(CreateView): request=self.request) -registration_completion = valid_token(RegistrationCompletionView.as_view()) +registration_completion = RegistrationCompletionView.as_view() class DeleteView(TemplateView): diff --git a/tests/test_all.py b/tests/test_all.py index e2fb19b0..68a2e65f 100644 --- a/tests/test_all.py +++ b/tests/test_all.py @@ -604,22 +604,9 @@ class APITest(TestCase): self.assertEqual(len(mail.outbox), outbox_level + 1) outbox_level = len(mail.outbox) - # Second registration - response2 = client.post(reverse('a2-api-register'), - content_type='application/json', - data=json.dumps(payload)) - self.assertEqual(response2.status_code, status.HTTP_202_ACCEPTED) - self.assertIn('result', response2.data) - self.assertEqual(response2.data['result'], 1) - self.assertIn('token', response2.data) - token2 = response2.data['token'] - self.assertEqual(len(mail.outbox), outbox_level + 1) - - activation_mail1, activation_mail2 = mail.outbox - - # User side - user click on first email + # User side - user click on email client = Client() - activation_url = get_link_from_mail(activation_mail1) + activation_url = get_link_from_mail(mail.outbox[0]) response = client.get(activation_url) self.assertEqual(response.status_code, status.HTTP_200_OK) assert utils.make_url(return_url, params={'token': token}) in force_text(response.content) @@ -632,24 +619,6 @@ class APITest(TestCase): self.assertEqual(last_user.ou.slug, self.ou.slug) self.assertTrue(last_user.check_password(password)) - # User click on second email - client = Client() - activation_url = get_link_from_mail(activation_mail2) - response = client.get(activation_url) - self.assertEqual(response.status_code, status.HTTP_302_FOUND) - self.assertEqual(response['Location'], - utils.make_url(return_url, params={'token': token2})) - self.assertEqual(User.objects.count(), user_count + 1) - response = client.get(reverse('auth_homepage')) - self.assertContains(response, username) - last_user2 = User.objects.order_by('id').last() - self.assertEqual(User.objects.filter(email=payload['email']).count(), 1) - self.assertEqual(last_user.id, last_user2.id) - self.assertEqual(last_user2.username, username) - self.assertEqual(last_user2.email, email) - self.assertEqual(last_user2.ou.slug, self.ou.slug) - self.assertTrue(last_user2.check_password(password)) - # Test email is unique with case change client = test.APIClient() client.credentials(HTTP_AUTHORIZATION='Basic %s' % cred) diff --git a/tests/test_attribute_kinds.py b/tests/test_attribute_kinds.py index 6a63f6ec..5e8a88df 100644 --- a/tests/test_attribute_kinds.py +++ b/tests/test_attribute_kinds.py @@ -79,16 +79,20 @@ def test_string(db, app, admin, mailoutbox): def test_fr_postcode(db, app, admin, mailoutbox): + + def register_john(): + response = app.get('/accounts/register/') + form = response.form + form.set('email', 'john.doe@example.com') + response = form.submit().follow() + assert 'john.doe@example.com' in response + return get_link_from_mail(mailoutbox[-1]) + Attribute.objects.create(name='postcode', label='postcode', kind='fr_postcode', asked_on_registration=True) qs = User.objects.filter(first_name='John') - response = app.get('/accounts/register/') - form = response.form - form.set('email', 'john.doe@example.com') - response = form.submit().follow() - assert 'john.doe@example.com' in response - url = get_link_from_mail(mailoutbox[0]) + url = register_john() response = app.get(url) form = response.form @@ -115,6 +119,7 @@ def test_fr_postcode(db, app, admin, mailoutbox): assert qs.get().attributes.postcode == '12345' qs.delete() + url = register_john() response = app.get(url) form = response.form form.set('first_name', 'John') @@ -126,6 +131,7 @@ def test_fr_postcode(db, app, admin, mailoutbox): assert qs.get().attributes.postcode == '12345' qs.delete() + url = register_john() response = app.get(url) form = response.form form.set('first_name', 'John') @@ -182,17 +188,20 @@ def test_fr_postcode(db, app, admin, mailoutbox): def test_phone_number(db, app, admin, mailoutbox): + + def register_john(): + response = app.get('/accounts/register/') + form = response.form + form.set('email', 'john.doe@example.com') + response = form.submit().follow() + assert 'john.doe@example.com' in response + return get_link_from_mail(mailoutbox[-1]) + Attribute.objects.create(name='phone_number', label='phone', kind='phone_number', asked_on_registration=True) qs = User.objects.filter(first_name='John') - response = app.get('/accounts/register/') - form = response.form - form.set('email', 'john.doe@example.com') - response = form.submit().follow() - assert 'john.doe@example.com' in response - url = get_link_from_mail(mailoutbox[0]) - + url = register_john() response = app.get(url) form = response.form form.set('first_name', 'John') @@ -219,6 +228,7 @@ def test_phone_number(db, app, admin, mailoutbox): assert qs.get().attributes.phone_number == '12345' qs.delete() + url = register_john() response = app.get(url) form = response.form form.set('first_name', 'John') @@ -230,6 +240,7 @@ def test_phone_number(db, app, admin, mailoutbox): assert qs.get().attributes.phone_number == '+12345' qs.delete() + url = register_john() response = app.get(url) form = response.form form.set('first_name', 'John') @@ -241,6 +252,7 @@ def test_phone_number(db, app, admin, mailoutbox): assert qs.get().attributes.phone_number == '' qs.delete() + url = register_john() response = app.get(url) form = response.form form.set('first_name', 'John') @@ -306,17 +318,21 @@ def test_phone_number(db, app, admin, mailoutbox): def test_birthdate(db, app, admin, mailoutbox, freezer): + + def register_john(): + response = app.get('/accounts/register/') + form = response.form + form.set('email', 'john.doe@example.com') + response = form.submit().follow() + assert 'john.doe@example.com' in response + return get_link_from_mail(mailoutbox[-1]) + freezer.move_to('2018-01-01') Attribute.objects.create(name='birthdate', label='birthdate', kind='birthdate', asked_on_registration=True) qs = User.objects.filter(first_name='John') - response = app.get('/accounts/register/') - form = response.form - form.set('email', 'john.doe@example.com') - response = form.submit().follow() - assert 'john.doe@example.com' in response - url = get_link_from_mail(mailoutbox[0]) + url = register_john() response = app.get(url) form = response.form @@ -335,6 +351,7 @@ def test_birthdate(db, app, admin, mailoutbox, freezer): assert qs.get().attributes.birthdate == datetime.date(2017, 12, 31) qs.delete() + url = register_john() response = app.get(url) form = response.form form.set('first_name', 'John') diff --git a/tests/test_registration.py b/tests/test_registration.py index 6a3f25d4..f850592d 100644 --- a/tests/test_registration.py +++ b/tests/test_registration.py @@ -261,6 +261,11 @@ def test_username_is_unique(app, db, settings, mailoutbox): app.session.flush() # try again + response = app.get(reverse('registration_register')) + response.form.set('email', 'testbot@entrouvert.com') + response = response.form.submit() + link = get_link_from_mail(mailoutbox[2]) + response = app.get(link) response = response.click('create') @@ -666,3 +671,51 @@ def test_authentication_method(app, db, rf, hooks): assert hooks.calls['event'][-2]['kwargs']['authentication_method'] == 'another' assert hooks.calls['event'][-1]['kwargs']['name'] == 'login' assert hooks.calls['event'][-1]['kwargs']['how'] == 'another' + + +def test_registration_link_unique_use(app, db, mailoutbox): + models.Attribute.objects.update(disabled=True) + + response = app.get(reverse('registration_register')) + response.form.set('email', 'testbot@entrouvert.com') + response = response.form.submit() + + link = get_link_from_mail(mailoutbox[0]) + + response = app.get(link) + response.form.set('password1', 'T0==toto') + + # accessing multiple times work + response = app.get(link) + response.form.set('password1', 'T0==toto') + response.form.set('password2', 'T0==toto') + response = response.form.submit().follow() + assert 'You have just created an account.' in response.text + + response = app.get(link) + assert urlparse(response['Location']).path == reverse('registration_register') + response = response.follow() + assert 'activation key is unknown or expired' in response.text + + +def test_double_registration_impossible(app, db, mailoutbox): + models.Attribute.objects.update(disabled=True) + + for _ in range(2): + response = app.get(reverse('registration_register')) + response.form.set('email', 'testbot@entrouvert.com') + response = response.form.submit() + assert len(mailoutbox) == 2 + + link1, link2 = get_link_from_mail(mailoutbox[0]), get_link_from_mail(mailoutbox[1]) + + response = app.get(link1) + assert urlparse(response['Location']).path == reverse('registration_register') + response = response.follow() + assert 'activation key is unknown or expired' in response.text + + response = app.get(link2) + response.form.set('password1', 'T0==toto') + response.form.set('password2', 'T0==toto') + response = response.form.submit().follow() + assert 'You have just created an account.' in response.text -- 2.20.1