Canonical SSO provider

Merge lp:~maxiberta/canonical-identity-provider/prevent-forgot-password-user-enumeration into lp:canonical-identity-provider/release

  1. prevent-forgot-password-user-enumeration
  2. Merge into trunk
Proposed by Maximiliano Bertacchini
Status: Merged
Approved by: Maximiliano Bertacchini
Approved revision: no longer in the source branch.
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: lp:~maxiberta/canonical-identity-provider/prevent-forgot-password-user-enumeration
Merge into: lp:canonical-identity-provider/release
Diff against target: 284 lines (+59/-49)
5 files modified
src/api/v20/handlers.py (+3/-4)
src/api/v20/tests/test_handlers.py (+4/-3)
src/identityprovider/tests/openid_server/per_version/test_sso_workflow_reset_password.py (+2/-7)
src/webui/tests/test_loginservice.py (+24/-22)
src/webui/tests/test_views_registration.py (+26/-13)
To merge this branch: bzr merge lp:~maxiberta/canonical-identity-provider/prevent-forgot-password-user-enumeration
Reviewer Review Type Date Requested Status
Tom Wardill (community) Approve
Review via email: mp+361166@code.launchpad.net

Commit message

Redirect forgot password valid POST to email-sent view regardless of whether the account exists or not.

To post a comment you must log in.
Revision history for this message
Maximiliano Bertacchini (maxiberta) :
Revision history for this message
Tom Wardill (twom) :
review: Approve
Revision history for this message
Otto Co-Pilot (otto-copilot) wrote :

Merging failed
https://jenkins.ols.canonical.com/online-services/job/canonical-identity-provider/216/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/api/v20/handlers.py'
--- src/api/v20/handlers.py 2018-12-20 20:25:26 +0000
+++ src/api/v20/handlers.py 2019-01-15 17:33:10 +0000
@@ -15,7 +15,6 @@
15)15)
16from django.core.validators import validate_email16from django.core.validators import validate_email
17from django.http import HttpResponse17from django.http import HttpResponse
18from django.utils.translation import ugettext_lazy as _
19from django_statsd.clients import statsd18from django_statsd.clients import statsd
20from gargoyle import gargoyle19from gargoyle import gargoyle
21from piston.handler import AnonymousBaseHandler, BaseHandler20from piston.handler import AnonymousBaseHandler, BaseHandler
@@ -221,9 +220,9 @@
221 # but there was some with it invalidated220 # but there was some with it invalidated
222 return errors.EMAIL_INVALIDATED()221 return errors.EMAIL_INVALIDATED()
223 else:222 else:
224 msg = _("No account associated with {email}")223 response = rc.CREATED
225 # there is no account, neither with this email invalidated224 response.content = dict(email=email)
226 return errors.INVALID_DATA(email=[msg.format(email=email)])225 return response
227226
228 if not account.can_reset_password:227 if not account.can_reset_password:
229 if account.status == AccountStatus.SUSPENDED:228 if account.status == AccountStatus.SUSPENDED:
230229
=== modified file 'src/api/v20/tests/test_handlers.py'
--- src/api/v20/tests/test_handlers.py 2018-12-20 20:25:26 +0000
+++ src/api/v20/tests/test_handlers.py 2019-01-15 17:33:10 +0000
@@ -1600,12 +1600,13 @@
1600 def test_email_missing(self):1600 def test_email_missing(self):
1601 self.assert_field_required(data={}, field='email')1601 self.assert_field_required(data={}, field='email')
16021602
1603 def test_invalid_email(self):1603 def test_nonexistent_account_email(self):
1604 mock_send_invitation = self.patch(1604 mock_send_invitation = self.patch(
1605 'api.v20.handlers.emailutils.send_invitation_after_password_reset')1605 'api.v20.handlers.emailutils.send_invitation_after_password_reset')
1606 email = 'invalid@foo.com'
1607 body = self.do_post({'email': email}, status_code=201)
16061608
1607 extra = {'email': ['No account associated with invalid@foo.com']}1609 self.assertEqual(body['email'], email)
1608 self.assert_bad_request(data={'email': 'invalid@foo.com'}, extra=extra)
1609 mock_send_invitation.assert_called_once_with(1610 mock_send_invitation.assert_called_once_with(
1610 self.sso_root_url, 'invalid@foo.com')1611 self.sso_root_url, 'invalid@foo.com')
16111612
16121613
=== modified file 'src/identityprovider/tests/openid_server/per_version/test_sso_workflow_reset_password.py'
--- src/identityprovider/tests/openid_server/per_version/test_sso_workflow_reset_password.py 2019-01-09 18:37:18 +0000
+++ src/identityprovider/tests/openid_server/per_version/test_sso_workflow_reset_password.py 2019-01-15 17:33:10 +0000
@@ -26,10 +26,7 @@
26 data = dict(email='no-account@example.com')26 data = dict(email='no-account@example.com')
27 response = self.client.post(link, data=data, follow=True)27 response = self.client.post(link, data=data, follow=True)
2828
29 self.assertContains(response, "Reset password")29 self.assertContains(response, "Step 2 of 3: Check your email")
30 self.assertFormError(
31 response, 'form', 'email',
32 'No account associated with no-account@example.com')
3330
34 def test_email_for_team(self):31 def test_email_for_team(self):
35 response = self.do_openid_dance()32 response = self.do_openid_dance()
@@ -40,9 +37,7 @@
40 data = dict(email='support@ubuntu.com')37 data = dict(email='support@ubuntu.com')
41 response = self.client.post(link, data=data, follow=True)38 response = self.client.post(link, data=data, follow=True)
4239
43 self.assertContains(response, "Reset password")40 self.assertContains(response, "Step 2 of 3: Check your email")
44 self.assertFormError(response, 'form', 'email',
45 'No account associated with support@ubuntu.com')
4641
47 def test_valid_email(self):42 def test_valid_email(self):
48 # Finally, lets try and recover the password for a test@canonical.com:43 # Finally, lets try and recover the password for a test@canonical.com:
4944
=== modified file 'src/webui/tests/test_loginservice.py'
--- src/webui/tests/test_loginservice.py 2016-05-10 14:42:33 +0000
+++ src/webui/tests/test_loginservice.py 2019-01-15 17:33:10 +0000
@@ -46,23 +46,23 @@
4646
47class NewAccountTest(SSOBaseTestCase):47class NewAccountTest(SSOBaseTestCase):
4848
49 fixtures = ["test"]
50
49 def test_newaccount_existing(self):51 def test_newaccount_existing(self):
50 query = {52 query = {
51 'email': 'nobody@debian.org',53 'displayname': 'Tester',
54 'email': 'test@canonical.com',
55 'password': 'Testing123',
56 'passwordconfirm': 'Testing123',
57 'accept_tos': True,
52 }58 }
53 self.preseed_cookie_check()59 self.preseed_cookie_check()
54 response = self.client.post('/+forgot_password', query)60 response = self.client.post('/+new_account', query)
55 self.assertEqual(response.status_code, 200)61 self.assertContains(response, "test@canonical.com")
56 self.assertIn("Reset password", response.content)
57 self.assertIn("nobody@debian.org", response.content)
58 self.assertEqual(len(mail.outbox), 1)62 self.assertEqual(len(mail.outbox), 1)
59 self.assertEqual(63 self.assertEqual(mail.outbox[0].subject,
60 mail.outbox[0].subject,64 u"%s: Warning" % settings.NOREPLY_FROM_NAME)
61 u"%s: Password reset request" % settings.NOREPLY_FROM_NAME)65 mail.outbox = []
62 self.assertIn('nobody@debian.org', mail.outbox[0].body)
63 self.assertIn('+new_account', mail.outbox[0].body)
64 self.assertFormError(response, 'form', 'email',
65 'No account associated with nobody@debian.org')
6666
67 def test_newaccount_no_tos_accept(self):67 def test_newaccount_no_tos_accept(self):
68 self.preseed_cookie_check()68 self.preseed_cookie_check()
@@ -85,19 +85,20 @@
8585
86 def test_forgottenpass_nonexisting(self):86 def test_forgottenpass_nonexisting(self):
87 query = {87 query = {
88 'displayname': 'Tester',88 'email': 'nobody@debian.org',
89 'email': 'test@canonical.com',
90 'password': 'Testing123',
91 'passwordconfirm': 'Testing123',
92 'accept_tos': True,
93 }89 }
94 self.preseed_cookie_check()90 self.preseed_cookie_check()
95 response = self.client.post('/+new_account', query)91 response = self.client.post('/+forgot_password', query)
96 self.assertContains(response, "test@canonical.com")92 self.assertEqual(response.status_code, 200)
93 self.assertIn("Reset password", response.content)
94 self.assertIn("nobody@debian.org", response.content)
97 self.assertEqual(len(mail.outbox), 1)95 self.assertEqual(len(mail.outbox), 1)
98 self.assertEqual(mail.outbox[0].subject,96 self.assertEqual(
99 u"%s: Warning" % settings.NOREPLY_FROM_NAME)97 mail.outbox[0].subject,
100 mail.outbox = []98 u"%s: Password reset request" % settings.NOREPLY_FROM_NAME)
99 self.assertIn('nobody@debian.org', mail.outbox[0].body)
100 self.assertIn('+new_account', mail.outbox[0].body)
101 self.assertTemplateUsed(response, 'registration/email_sent.html')
101102
102 def test_resetform_success(self):103 def test_resetform_success(self):
103 query = {104 query = {
@@ -112,5 +113,6 @@
112 mail.outbox[0].subject,113 mail.outbox[0].subject,
113 u"%s: Forgotten Password" % settings.NOREPLY_FROM_NAME)114 u"%s: Forgotten Password" % settings.NOREPLY_FROM_NAME)
114 mail.outbox = []115 mail.outbox = []
116 self.assertTemplateUsed(response, 'registration/email_sent.html')
115117
116 AuthToken.objects.all().delete()118 AuthToken.objects.all().delete()
117119
=== modified file 'src/webui/tests/test_views_registration.py'
--- src/webui/tests/test_views_registration.py 2019-01-09 18:37:18 +0000
+++ src/webui/tests/test_views_registration.py 2019-01-15 17:33:10 +0000
@@ -490,11 +490,13 @@
490 self.assertEqual(ctx['rpconfig'], None)490 self.assertEqual(ctx['rpconfig'], None)
491 self.assert_form_displayed(response)491 self.assert_form_displayed(response)
492 self.assert_stat_calls(['requested'])492 self.assert_stat_calls(['requested'])
493 self.assertTemplateUsed(response, 'registration/forgot_password.html')
493494
494 def test_get_with_email(self):495 def test_get_with_email(self):
495 response = self.get(data=dict(email='test@test.com'))496 response = self.get(data=dict(email='test@test.com'))
496 ctx = response.context_data497 ctx = response.context_data
497 self.assertEqual(ctx['form']['email'].value(), 'test@test.com')498 self.assertEqual(ctx['form']['email'].value(), 'test@test.com')
499 self.assertTemplateUsed(response, 'registration/forgot_password.html')
498500
499 def test_post_with_initial_data(self):501 def test_post_with_initial_data(self):
500 data = dict(email='test@test.com', forgot_password='')502 data = dict(email='test@test.com', forgot_password='')
@@ -514,6 +516,7 @@
514 response = self.post()516 response = self.post()
515 self.assert_form_displayed(response, email=FIELD_REQUIRED)517 self.assert_form_displayed(response, email=FIELD_REQUIRED)
516 self.assert_stat_calls(['error.form'])518 self.assert_stat_calls(['error.form'])
519 self.assertTemplateUsed(response, 'registration/forgot_password.html')
517520
518 def test_post_invalid_data(self):521 def test_post_invalid_data(self):
519 response = self.post(data=dict(email="111"))522 response = self.post(data=dict(email="111"))
@@ -521,6 +524,7 @@
521 self.assert_form_displayed(response, email=INVALID_EMAIL)524 self.assert_form_displayed(response, email=INVALID_EMAIL)
522 self.assert_form_displayed(response)525 self.assert_form_displayed(response)
523 self.assert_stat_calls(['error.form'])526 self.assert_stat_calls(['error.form'])
527 self.assertTemplateUsed(response, 'registration/forgot_password.html')
524528
525 def test_post_invalid_data_with_token(self):529 def test_post_invalid_data_with_token(self):
526 token = '12345678' * 2530 token = '12345678' * 2
@@ -531,16 +535,19 @@
531535
532 self.assert_form_displayed(response)536 self.assert_form_displayed(response)
533 self.assert_stat_calls(['error.form'])537 self.assert_stat_calls(['error.form'])
538 self.assertTemplateUsed(response, 'registration/forgot_password.html')
534539
535 def test_post_success(self):540 def test_post_success(self):
536 response = self.post(data=self.data)541 response = self.post(data=self.data)
537542
538 self.mock_api_request_password_reset.assert_called_once_with(543 self.mock_api_request_password_reset.assert_called_once_with(
539 email=self.data['email'], token=None)544 email=self.data['email'], token=None)
540
541 self.assertEqual(response.status_code, 200)545 self.assertEqual(response.status_code, 200)
542 self.assertEqual(546 self.assertEqual(
543 self.client.session['token_email'], self.data['email'])547 self.client.session['token_email'], self.data['email'])
548 self.assertEqual(response.context_data['email_heading'],
549 'Reset password')
550 self.assertTemplateUsed(response, 'registration/email_sent.html')
544551
545 def test_post_success_with_token(self):552 def test_post_success_with_token(self):
546 token = '12345678' * 2553 token = '12345678' * 2
@@ -551,6 +558,19 @@
551 self.assertEqual(response.status_code, 200)558 self.assertEqual(response.status_code, 200)
552 self.assertEqual(559 self.assertEqual(
553 self.client.session['token_email'], self.data['email'])560 self.client.session['token_email'], self.data['email'])
561 self.assertTemplateUsed(response, 'registration/email_sent.html')
562
563 def test_post_with_unknown_email(self):
564 response = self.post(data=self.data)
565
566 self.mock_api_request_password_reset.assert_called_once_with(
567 email=self.data['email'], token=None)
568 self.assertEqual(response.status_code, 200)
569 self.assertEqual(
570 self.client.session['token_email'], self.data['email'])
571 self.assertEqual(response.context_data['email_heading'],
572 'Reset password')
573 self.assertTemplateUsed(response, 'registration/email_sent.html')
554574
555 def test_post_email_invalidated(self):575 def test_post_email_invalidated(self):
556 exc = api_errors.EmailInvalidated(Mock())576 exc = api_errors.EmailInvalidated(Mock())
@@ -558,6 +578,7 @@
558 response = self.post(data=self.data)578 response = self.post(data=self.data)
559 self.assertEqual(response.status_code, 200)579 self.assertEqual(response.status_code, 200)
560 self.assert_stat_calls(['error.email_invalidated'])580 self.assert_stat_calls(['error.email_invalidated'])
581 self.assertTemplateUsed(response, 'registration/forgot_password.html')
561582
562 def test_post_account_suspended(self):583 def test_post_account_suspended(self):
563 exc = api_errors.AccountSuspended(Mock())584 exc = api_errors.AccountSuspended(Mock())
@@ -565,6 +586,7 @@
565 response = self.post(data=self.data)586 response = self.post(data=self.data)
566 self.assertEqual(response.status_code, 200)587 self.assertEqual(response.status_code, 200)
567 self.assert_stat_calls(['error.account_suspended'])588 self.assert_stat_calls(['error.account_suspended'])
589 self.assertTemplateUsed(response, 'registration/forgot_password.html')
568590
569 def test_post_account_deactivated(self):591 def test_post_account_deactivated(self):
570 exc = api_errors.AccountDeactivated(Mock())592 exc = api_errors.AccountDeactivated(Mock())
@@ -572,6 +594,7 @@
572 response = self.post(data=self.data)594 response = self.post(data=self.data)
573 self.assertEqual(response.status_code, 200)595 self.assertEqual(response.status_code, 200)
574 self.assert_stat_calls(['error.account_deactivated'])596 self.assert_stat_calls(['error.account_deactivated'])
597 self.assertTemplateUsed(response, 'registration/forgot_password.html')
575598
576 def test_post_can_not_reset_password(self):599 def test_post_can_not_reset_password(self):
577 exc = api_errors.CanNotResetPassword(Mock())600 exc = api_errors.CanNotResetPassword(Mock())
@@ -579,6 +602,7 @@
579 response = self.post(data=self.data)602 response = self.post(data=self.data)
580 self.assertEqual(response.status_code, 200)603 self.assertEqual(response.status_code, 200)
581 self.assert_stat_calls(['error.can_not_reset_password'])604 self.assert_stat_calls(['error.can_not_reset_password'])
605 self.assertTemplateUsed(response, 'registration/forgot_password.html')
582606
583 def test_post_too_many_tokens(self):607 def test_post_too_many_tokens(self):
584 exc = api_errors.TooManyTokens(Mock())608 exc = api_errors.TooManyTokens(Mock())
@@ -586,6 +610,7 @@
586 response = self.post(data=self.data)610 response = self.post(data=self.data)
587 self.assertEqual(response.status_code, 200)611 self.assertEqual(response.status_code, 200)
588 self.assert_stat_calls(['error.too_many_tokens'])612 self.assert_stat_calls(['error.too_many_tokens'])
613 self.assertTemplateUsed(response, 'registration/forgot_password.html')
589614
590 def test_post_too_many_requests(self):615 def test_post_too_many_requests(self):
591 exc = api_errors.TooManyRequests(Mock())616 exc = api_errors.TooManyRequests(Mock())
@@ -595,18 +620,6 @@
595 self.assert_form_displayed(response, __all__=ERROR_TOO_MANY_REQUESTS)620 self.assert_form_displayed(response, __all__=ERROR_TOO_MANY_REQUESTS)
596 self.assert_stat_calls(['error.too_many_requests'])621 self.assert_stat_calls(['error.too_many_requests'])
597622
598 def test_reset_password(self):
599 response = self.post(data=self.data)
600
601 self.mock_api_request_password_reset.assert_called_once_with(
602 email=self.data['email'], token=None)
603
604 self.assertEqual(response.status_code, 200)
605 self.assertEqual(
606 self.client.session['token_email'], self.data['email'])
607 self.assertEqual(response.context_data['email_heading'],
608 'Reset password')
609
610 def test_method_not_allowed(self):623 def test_method_not_allowed(self):
611 response = self.put(data=self.data)624 response = self.put(data=self.data)
612 self.assertEqual(response.status_code, 405)625 self.assertEqual(response.status_code, 405)