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

Proposed by Maximiliano Bertacchini on 2018-12-19
Status: Merged
Approved by: Maximiliano Bertacchini on 2019-01-15
Approved revision: 1673
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
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 2018-12-19 Approve on 2019-01-14
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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/api/v20/handlers.py'
2--- src/api/v20/handlers.py 2018-12-20 20:25:26 +0000
3+++ src/api/v20/handlers.py 2019-01-15 17:33:10 +0000
4@@ -15,7 +15,6 @@
5 )
6 from django.core.validators import validate_email
7 from django.http import HttpResponse
8-from django.utils.translation import ugettext_lazy as _
9 from django_statsd.clients import statsd
10 from gargoyle import gargoyle
11 from piston.handler import AnonymousBaseHandler, BaseHandler
12@@ -221,9 +220,9 @@
13 # but there was some with it invalidated
14 return errors.EMAIL_INVALIDATED()
15 else:
16- msg = _("No account associated with {email}")
17- # there is no account, neither with this email invalidated
18- return errors.INVALID_DATA(email=[msg.format(email=email)])
19+ response = rc.CREATED
20+ response.content = dict(email=email)
21+ return response
22
23 if not account.can_reset_password:
24 if account.status == AccountStatus.SUSPENDED:
25
26=== modified file 'src/api/v20/tests/test_handlers.py'
27--- src/api/v20/tests/test_handlers.py 2018-12-20 20:25:26 +0000
28+++ src/api/v20/tests/test_handlers.py 2019-01-15 17:33:10 +0000
29@@ -1600,12 +1600,13 @@
30 def test_email_missing(self):
31 self.assert_field_required(data={}, field='email')
32
33- def test_invalid_email(self):
34+ def test_nonexistent_account_email(self):
35 mock_send_invitation = self.patch(
36 'api.v20.handlers.emailutils.send_invitation_after_password_reset')
37+ email = 'invalid@foo.com'
38+ body = self.do_post({'email': email}, status_code=201)
39
40- extra = {'email': ['No account associated with invalid@foo.com']}
41- self.assert_bad_request(data={'email': 'invalid@foo.com'}, extra=extra)
42+ self.assertEqual(body['email'], email)
43 mock_send_invitation.assert_called_once_with(
44 self.sso_root_url, 'invalid@foo.com')
45
46
47=== modified file 'src/identityprovider/tests/openid_server/per_version/test_sso_workflow_reset_password.py'
48--- src/identityprovider/tests/openid_server/per_version/test_sso_workflow_reset_password.py 2019-01-09 18:37:18 +0000
49+++ src/identityprovider/tests/openid_server/per_version/test_sso_workflow_reset_password.py 2019-01-15 17:33:10 +0000
50@@ -26,10 +26,7 @@
51 data = dict(email='no-account@example.com')
52 response = self.client.post(link, data=data, follow=True)
53
54- self.assertContains(response, "Reset password")
55- self.assertFormError(
56- response, 'form', 'email',
57- 'No account associated with no-account@example.com')
58+ self.assertContains(response, "Step 2 of 3: Check your email")
59
60 def test_email_for_team(self):
61 response = self.do_openid_dance()
62@@ -40,9 +37,7 @@
63 data = dict(email='support@ubuntu.com')
64 response = self.client.post(link, data=data, follow=True)
65
66- self.assertContains(response, "Reset password")
67- self.assertFormError(response, 'form', 'email',
68- 'No account associated with support@ubuntu.com')
69+ self.assertContains(response, "Step 2 of 3: Check your email")
70
71 def test_valid_email(self):
72 # Finally, lets try and recover the password for a test@canonical.com:
73
74=== modified file 'src/webui/tests/test_loginservice.py'
75--- src/webui/tests/test_loginservice.py 2016-05-10 14:42:33 +0000
76+++ src/webui/tests/test_loginservice.py 2019-01-15 17:33:10 +0000
77@@ -46,23 +46,23 @@
78
79 class NewAccountTest(SSOBaseTestCase):
80
81+ fixtures = ["test"]
82+
83 def test_newaccount_existing(self):
84 query = {
85- 'email': 'nobody@debian.org',
86+ 'displayname': 'Tester',
87+ 'email': 'test@canonical.com',
88+ 'password': 'Testing123',
89+ 'passwordconfirm': 'Testing123',
90+ 'accept_tos': True,
91 }
92 self.preseed_cookie_check()
93- response = self.client.post('/+forgot_password', query)
94- self.assertEqual(response.status_code, 200)
95- self.assertIn("Reset password", response.content)
96- self.assertIn("nobody@debian.org", response.content)
97+ response = self.client.post('/+new_account', query)
98+ self.assertContains(response, "test@canonical.com")
99 self.assertEqual(len(mail.outbox), 1)
100- self.assertEqual(
101- mail.outbox[0].subject,
102- u"%s: Password reset request" % settings.NOREPLY_FROM_NAME)
103- self.assertIn('nobody@debian.org', mail.outbox[0].body)
104- self.assertIn('+new_account', mail.outbox[0].body)
105- self.assertFormError(response, 'form', 'email',
106- 'No account associated with nobody@debian.org')
107+ self.assertEqual(mail.outbox[0].subject,
108+ u"%s: Warning" % settings.NOREPLY_FROM_NAME)
109+ mail.outbox = []
110
111 def test_newaccount_no_tos_accept(self):
112 self.preseed_cookie_check()
113@@ -85,19 +85,20 @@
114
115 def test_forgottenpass_nonexisting(self):
116 query = {
117- 'displayname': 'Tester',
118- 'email': 'test@canonical.com',
119- 'password': 'Testing123',
120- 'passwordconfirm': 'Testing123',
121- 'accept_tos': True,
122+ 'email': 'nobody@debian.org',
123 }
124 self.preseed_cookie_check()
125- response = self.client.post('/+new_account', query)
126- self.assertContains(response, "test@canonical.com")
127+ response = self.client.post('/+forgot_password', query)
128+ self.assertEqual(response.status_code, 200)
129+ self.assertIn("Reset password", response.content)
130+ self.assertIn("nobody@debian.org", response.content)
131 self.assertEqual(len(mail.outbox), 1)
132- self.assertEqual(mail.outbox[0].subject,
133- u"%s: Warning" % settings.NOREPLY_FROM_NAME)
134- mail.outbox = []
135+ self.assertEqual(
136+ mail.outbox[0].subject,
137+ u"%s: Password reset request" % settings.NOREPLY_FROM_NAME)
138+ self.assertIn('nobody@debian.org', mail.outbox[0].body)
139+ self.assertIn('+new_account', mail.outbox[0].body)
140+ self.assertTemplateUsed(response, 'registration/email_sent.html')
141
142 def test_resetform_success(self):
143 query = {
144@@ -112,5 +113,6 @@
145 mail.outbox[0].subject,
146 u"%s: Forgotten Password" % settings.NOREPLY_FROM_NAME)
147 mail.outbox = []
148+ self.assertTemplateUsed(response, 'registration/email_sent.html')
149
150 AuthToken.objects.all().delete()
151
152=== modified file 'src/webui/tests/test_views_registration.py'
153--- src/webui/tests/test_views_registration.py 2019-01-09 18:37:18 +0000
154+++ src/webui/tests/test_views_registration.py 2019-01-15 17:33:10 +0000
155@@ -490,11 +490,13 @@
156 self.assertEqual(ctx['rpconfig'], None)
157 self.assert_form_displayed(response)
158 self.assert_stat_calls(['requested'])
159+ self.assertTemplateUsed(response, 'registration/forgot_password.html')
160
161 def test_get_with_email(self):
162 response = self.get(data=dict(email='test@test.com'))
163 ctx = response.context_data
164 self.assertEqual(ctx['form']['email'].value(), 'test@test.com')
165+ self.assertTemplateUsed(response, 'registration/forgot_password.html')
166
167 def test_post_with_initial_data(self):
168 data = dict(email='test@test.com', forgot_password='')
169@@ -514,6 +516,7 @@
170 response = self.post()
171 self.assert_form_displayed(response, email=FIELD_REQUIRED)
172 self.assert_stat_calls(['error.form'])
173+ self.assertTemplateUsed(response, 'registration/forgot_password.html')
174
175 def test_post_invalid_data(self):
176 response = self.post(data=dict(email="111"))
177@@ -521,6 +524,7 @@
178 self.assert_form_displayed(response, email=INVALID_EMAIL)
179 self.assert_form_displayed(response)
180 self.assert_stat_calls(['error.form'])
181+ self.assertTemplateUsed(response, 'registration/forgot_password.html')
182
183 def test_post_invalid_data_with_token(self):
184 token = '12345678' * 2
185@@ -531,16 +535,19 @@
186
187 self.assert_form_displayed(response)
188 self.assert_stat_calls(['error.form'])
189+ self.assertTemplateUsed(response, 'registration/forgot_password.html')
190
191 def test_post_success(self):
192 response = self.post(data=self.data)
193
194 self.mock_api_request_password_reset.assert_called_once_with(
195 email=self.data['email'], token=None)
196-
197 self.assertEqual(response.status_code, 200)
198 self.assertEqual(
199 self.client.session['token_email'], self.data['email'])
200+ self.assertEqual(response.context_data['email_heading'],
201+ 'Reset password')
202+ self.assertTemplateUsed(response, 'registration/email_sent.html')
203
204 def test_post_success_with_token(self):
205 token = '12345678' * 2
206@@ -551,6 +558,19 @@
207 self.assertEqual(response.status_code, 200)
208 self.assertEqual(
209 self.client.session['token_email'], self.data['email'])
210+ self.assertTemplateUsed(response, 'registration/email_sent.html')
211+
212+ def test_post_with_unknown_email(self):
213+ response = self.post(data=self.data)
214+
215+ self.mock_api_request_password_reset.assert_called_once_with(
216+ email=self.data['email'], token=None)
217+ self.assertEqual(response.status_code, 200)
218+ self.assertEqual(
219+ self.client.session['token_email'], self.data['email'])
220+ self.assertEqual(response.context_data['email_heading'],
221+ 'Reset password')
222+ self.assertTemplateUsed(response, 'registration/email_sent.html')
223
224 def test_post_email_invalidated(self):
225 exc = api_errors.EmailInvalidated(Mock())
226@@ -558,6 +578,7 @@
227 response = self.post(data=self.data)
228 self.assertEqual(response.status_code, 200)
229 self.assert_stat_calls(['error.email_invalidated'])
230+ self.assertTemplateUsed(response, 'registration/forgot_password.html')
231
232 def test_post_account_suspended(self):
233 exc = api_errors.AccountSuspended(Mock())
234@@ -565,6 +586,7 @@
235 response = self.post(data=self.data)
236 self.assertEqual(response.status_code, 200)
237 self.assert_stat_calls(['error.account_suspended'])
238+ self.assertTemplateUsed(response, 'registration/forgot_password.html')
239
240 def test_post_account_deactivated(self):
241 exc = api_errors.AccountDeactivated(Mock())
242@@ -572,6 +594,7 @@
243 response = self.post(data=self.data)
244 self.assertEqual(response.status_code, 200)
245 self.assert_stat_calls(['error.account_deactivated'])
246+ self.assertTemplateUsed(response, 'registration/forgot_password.html')
247
248 def test_post_can_not_reset_password(self):
249 exc = api_errors.CanNotResetPassword(Mock())
250@@ -579,6 +602,7 @@
251 response = self.post(data=self.data)
252 self.assertEqual(response.status_code, 200)
253 self.assert_stat_calls(['error.can_not_reset_password'])
254+ self.assertTemplateUsed(response, 'registration/forgot_password.html')
255
256 def test_post_too_many_tokens(self):
257 exc = api_errors.TooManyTokens(Mock())
258@@ -586,6 +610,7 @@
259 response = self.post(data=self.data)
260 self.assertEqual(response.status_code, 200)
261 self.assert_stat_calls(['error.too_many_tokens'])
262+ self.assertTemplateUsed(response, 'registration/forgot_password.html')
263
264 def test_post_too_many_requests(self):
265 exc = api_errors.TooManyRequests(Mock())
266@@ -595,18 +620,6 @@
267 self.assert_form_displayed(response, __all__=ERROR_TOO_MANY_REQUESTS)
268 self.assert_stat_calls(['error.too_many_requests'])
269
270- def test_reset_password(self):
271- response = self.post(data=self.data)
272-
273- self.mock_api_request_password_reset.assert_called_once_with(
274- email=self.data['email'], token=None)
275-
276- self.assertEqual(response.status_code, 200)
277- self.assertEqual(
278- self.client.session['token_email'], self.data['email'])
279- self.assertEqual(response.context_data['email_heading'],
280- 'Reset password')
281-
282 def test_method_not_allowed(self):
283 response = self.put(data=self.data)
284 self.assertEqual(response.status_code, 405)