Merge lp:~matiasb/canonical-identity-provider/prevent-preferring-unvalidated-email into lp:canonical-identity-provider/release
- prevent-preferring-unvalidated-email
- Merge into trunk
Proposed by
Matias Bordese
Status: | Merged |
---|---|
Approved by: | Natalia Bidart |
Approved revision: | no longer in the source branch. |
Merged at revision: | 631 |
Proposed branch: | lp:~matiasb/canonical-identity-provider/prevent-preferring-unvalidated-email |
Merge into: | lp:canonical-identity-provider/release |
Diff against target: |
475 lines (+161/-90) 11 files modified
identityprovider/forms.py (+17/-17) identityprovider/models/account.py (+4/-1) identityprovider/models/emailaddress.py (+3/-0) identityprovider/tests/unit/test_forms.py (+35/-0) identityprovider/tests/unit/test_models_account.py (+9/-0) identityprovider/tests/unit/test_models_emailaddress.py (+10/-0) ubuntu_sso_saml/tests.py (+1/-1) webui/templates/account/edit.html (+15/-6) webui/tests/test_views_account.py (+2/-0) webui/tests/test_views_ui.py (+32/-39) webui/views/ui.py (+33/-26) |
To merge this branch: | bzr merge lp:~matiasb/canonical-identity-provider/prevent-preferring-unvalidated-email |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Natalia Bidart (community) | Approve | ||
Review via email: mp+145032@code.launchpad.net |
Commit message
Updated checks and validation for users and (un)verified email addresses:
- Prevent user changing preferred email address to an unverified email address.
- Prevent password reset for users without a verified email address.
Description of the change
Updated checks and validation for users and (un)verified email addresses:
- Prevent user changing preferred email address to an unverified email address.
- Prevent password reset for users without a verified email address.
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 'identityprovider/forms.py' | |||
2 | --- identityprovider/forms.py 2012-12-18 18:37:03 +0000 | |||
3 | +++ identityprovider/forms.py 2013-01-25 22:16:21 +0000 | |||
4 | @@ -154,12 +154,6 @@ | |||
5 | 154 | displayname = fields.CharField( | 154 | displayname = fields.CharField( |
6 | 155 | error_messages=default_errors, | 155 | error_messages=default_errors, |
7 | 156 | widget=ROAwareTextInput(attrs={'class': 'textType', 'size': '20'})) | 156 | widget=ROAwareTextInput(attrs={'class': 'textType', 'size': '20'})) |
8 | 157 | preferred_email = PreferredEmailField( | ||
9 | 158 | error_messages=email_errors, | ||
10 | 159 | queryset=EmailAddress.objects.none(), | ||
11 | 160 | widget=ROAwareSelect, | ||
12 | 161 | empty_label=None, | ||
13 | 162 | ) | ||
14 | 163 | password = fields.CharField( | 157 | password = fields.CharField( |
15 | 164 | required=False, | 158 | required=False, |
16 | 165 | help_text=PASSWORD_POLICY_HELP_TEXT, | 159 | help_text=PASSWORD_POLICY_HELP_TEXT, |
17 | @@ -193,16 +187,21 @@ | |||
18 | 193 | else: | 187 | else: |
19 | 194 | preferredemail_id = self.instance.preferredemail.id | 188 | preferredemail_id = self.instance.preferredemail.id |
20 | 195 | 189 | ||
31 | 196 | # Override the field to set initial value (current preferred email) | 190 | # check for emails that can be preferred |
32 | 197 | emails = EmailAddress.objects.filter(account=self.instance) | 191 | validated_emails = self.instance.verified_emails() |
33 | 198 | self.fields['preferred_email'] = PreferredEmailField( | 192 | |
34 | 199 | queryset=emails.exclude(status=EmailStatus.NEW).order_by('-status', | 193 | if validated_emails.count() > 0: |
35 | 200 | 'email'), | 194 | # add and display a dropdown with the valid choices |
36 | 201 | initial=preferredemail_id, | 195 | self.fields['preferred_email'] = PreferredEmailField( |
37 | 202 | widget=ROAwareSelect, | 196 | queryset=validated_emails.order_by('email'), |
38 | 203 | error_messages=email_errors, | 197 | initial=preferredemail_id, |
39 | 204 | empty_label=None | 198 | widget=ROAwareSelect, |
40 | 205 | ) | 199 | error_messages=email_errors, |
41 | 200 | empty_label=None, | ||
42 | 201 | help_text=_( | ||
43 | 202 | 'Only verified email addresses are listed. ' | ||
44 | 203 | 'You can add and verify emails through the link below.'), | ||
45 | 204 | ) | ||
46 | 206 | 205 | ||
47 | 207 | class Meta: | 206 | class Meta: |
48 | 208 | fields = ('displayname', 'warn_about_backup_device', | 207 | fields = ('displayname', 'warn_about_backup_device', |
49 | @@ -257,7 +256,8 @@ | |||
50 | 257 | new_password = encrypt_launchpad_password(password) | 256 | new_password = encrypt_launchpad_password(password) |
51 | 258 | self.instance.accountpassword.password = new_password | 257 | self.instance.accountpassword.password = new_password |
52 | 259 | self.instance.accountpassword.save() | 258 | self.instance.accountpassword.save() |
54 | 260 | self.instance.preferredemail = self.cleaned_data['preferred_email'] | 259 | if 'preferred_email' in self.cleaned_data: |
55 | 260 | self.instance.preferredemail = self.cleaned_data['preferred_email'] | ||
56 | 261 | 261 | ||
57 | 262 | super(EditAccountForm, self).save() | 262 | super(EditAccountForm, self).save() |
58 | 263 | 263 | ||
59 | 264 | 264 | ||
60 | === modified file 'identityprovider/models/account.py' | |||
61 | --- identityprovider/models/account.py 2013-01-22 18:28:03 +0000 | |||
62 | +++ identityprovider/models/account.py 2013-01-25 22:16:21 +0000 | |||
63 | @@ -225,6 +225,8 @@ | |||
64 | 225 | return self._preferredemail | 225 | return self._preferredemail |
65 | 226 | 226 | ||
66 | 227 | def _set_preferredemail(self, email): | 227 | def _set_preferredemail(self, email): |
67 | 228 | if not email.is_verified(): | ||
68 | 229 | raise ValidationError('Email must be verified') | ||
69 | 228 | current = self.preferredemail | 230 | current = self.preferredemail |
70 | 229 | if current is not None: | 231 | if current is not None: |
71 | 230 | current.status = EmailStatus.VALIDATED | 232 | current.status = EmailStatus.VALIDATED |
72 | @@ -298,7 +300,8 @@ | |||
73 | 298 | 300 | ||
74 | 299 | @property | 301 | @property |
75 | 300 | def can_reset_password(self): | 302 | def can_reset_password(self): |
77 | 301 | return (self.is_active or self.can_reactivate) | 303 | return ((self.is_active or self.can_reactivate) and |
78 | 304 | self.verified_emails().count() > 0) | ||
79 | 302 | 305 | ||
80 | 303 | @property | 306 | @property |
81 | 304 | def is_active(self): | 307 | def is_active(self): |
82 | 305 | 308 | ||
83 | === modified file 'identityprovider/models/emailaddress.py' | |||
84 | --- identityprovider/models/emailaddress.py 2013-01-22 18:28:03 +0000 | |||
85 | +++ identityprovider/models/emailaddress.py 2013-01-25 22:16:21 +0000 | |||
86 | @@ -39,6 +39,9 @@ | |||
87 | 39 | def is_preferred(self): | 39 | def is_preferred(self): |
88 | 40 | return self.status == EmailStatus.PREFERRED | 40 | return self.status == EmailStatus.PREFERRED |
89 | 41 | 41 | ||
90 | 42 | def is_verified(self): | ||
91 | 43 | return self.status in (EmailStatus.VALIDATED, EmailStatus.PREFERRED) | ||
92 | 44 | |||
93 | 42 | def account_admin_link(self): | 45 | def account_admin_link(self): |
94 | 43 | if self.account is None: | 46 | if self.account is None: |
95 | 44 | return "None" | 47 | return "None" |
96 | 45 | 48 | ||
97 | === modified file 'identityprovider/tests/unit/test_forms.py' | |||
98 | --- identityprovider/tests/unit/test_forms.py 2012-12-17 16:27:17 +0000 | |||
99 | +++ identityprovider/tests/unit/test_forms.py 2013-01-25 22:16:21 +0000 | |||
100 | @@ -9,6 +9,7 @@ | |||
101 | 9 | from openid.extensions.sreg import SRegRequest | 9 | from openid.extensions.sreg import SRegRequest |
102 | 10 | 10 | ||
103 | 11 | from identityprovider.models.account import Account | 11 | from identityprovider.models.account import Account |
104 | 12 | from identityprovider.models.const import EmailStatus | ||
105 | 12 | from identityprovider.forms import ( | 13 | from identityprovider.forms import ( |
106 | 13 | DeviceRenameForm, | 14 | DeviceRenameForm, |
107 | 14 | EditAccountForm, | 15 | EditAccountForm, |
108 | @@ -49,6 +50,40 @@ | |||
109 | 49 | 50 | ||
110 | 50 | self.assertEqual(form.initial.get('displayname', ''), "Sample Person") | 51 | self.assertEqual(form.initial.get('displayname', ''), "Sample Person") |
111 | 51 | 52 | ||
112 | 53 | def test_account_without_validated_emails_no_field(self): | ||
113 | 54 | self.account.emailaddress_set.update(status=EmailStatus.NEW) | ||
114 | 55 | form = EditAccountForm(instance=self.account) | ||
115 | 56 | self.assertNotIn('preferred_email', form.fields) | ||
116 | 57 | |||
117 | 58 | def test_account_with_validated_emails_editable_field(self): | ||
118 | 59 | form = EditAccountForm(instance=self.account) | ||
119 | 60 | self.assertIn('preferred_email', form.fields) | ||
120 | 61 | choices = form.fields['preferred_email'].queryset.all() | ||
121 | 62 | valid_choices = self.account.verified_emails() | ||
122 | 63 | self.assertEqual(choices.count(), valid_choices.count()) | ||
123 | 64 | for email in choices: | ||
124 | 65 | self.assertIn(email, valid_choices) | ||
125 | 66 | |||
126 | 67 | def test_account_without_validated_emails_post_preferred_email(self): | ||
127 | 68 | self.account.emailaddress_set.update(status=EmailStatus.NEW) | ||
128 | 69 | original_value = self.account.preferredemail | ||
129 | 70 | invalid_email = self.account.unverified_emails().exclude( | ||
130 | 71 | email=original_value)[0] | ||
131 | 72 | form = EditAccountForm(instance=self.account, | ||
132 | 73 | data={'displayname': self.account.displayname, | ||
133 | 74 | 'preferred_email': invalid_email.email}) | ||
134 | 75 | self.assertTrue(form.is_valid()) | ||
135 | 76 | form.save() | ||
136 | 77 | account = Account.objects.get(id=self.account.id) | ||
137 | 78 | self.assertEqual(account.preferredemail, original_value) | ||
138 | 79 | |||
139 | 80 | def test_account_with_validated_email_changing_to_unvalidate(self): | ||
140 | 81 | invalid_email = self.account.unverified_emails()[0] | ||
141 | 82 | form = EditAccountForm(instance=self.account, | ||
142 | 83 | data={'displayname': self.account.displayname, | ||
143 | 84 | 'preferred_email': invalid_email.id}) | ||
144 | 85 | self.assertFalse(form.is_valid()) | ||
145 | 86 | |||
146 | 52 | def test_displayname_validation(self): | 87 | def test_displayname_validation(self): |
147 | 53 | form = EditAccountForm(instance=self.account, | 88 | form = EditAccountForm(instance=self.account, |
148 | 54 | data={'displayname': ' '}) | 89 | data={'displayname': ' '}) |
149 | 55 | 90 | ||
150 | === modified file 'identityprovider/tests/unit/test_models_account.py' | |||
151 | --- identityprovider/tests/unit/test_models_account.py 2013-01-14 12:31:37 +0000 | |||
152 | +++ identityprovider/tests/unit/test_models_account.py 2013-01-25 22:16:21 +0000 | |||
153 | @@ -209,6 +209,15 @@ | |||
154 | 209 | account = self.factory.make_account(email_validated=False) | 209 | account = self.factory.make_account(email_validated=False) |
155 | 210 | self.assertIsNone(account.preferredemail) | 210 | self.assertIsNone(account.preferredemail) |
156 | 211 | 211 | ||
157 | 212 | def test_set_preferred_with_unverified_email(self): | ||
158 | 213 | account = self.factory.make_account(email_validated=False) | ||
159 | 214 | with self.assertRaises(ValidationError): | ||
160 | 215 | account.preferredemail = account.emailaddress_set.get() | ||
161 | 216 | |||
162 | 217 | def test_cant_reset_password_without_verified_email(self): | ||
163 | 218 | account = self.factory.make_account(email_validated=False) | ||
164 | 219 | self.assertFalse(account.can_reset_password) | ||
165 | 220 | |||
166 | 212 | def test_validated_email_address_is_preferred_email_by_default(self): | 221 | def test_validated_email_address_is_preferred_email_by_default(self): |
167 | 213 | account = self.factory.make_account(email_validated=False) | 222 | account = self.factory.make_account(email_validated=False) |
168 | 214 | email_address = account.emailaddress_set.create( | 223 | email_address = account.emailaddress_set.create( |
169 | 215 | 224 | ||
170 | === modified file 'identityprovider/tests/unit/test_models_emailaddress.py' | |||
171 | --- identityprovider/tests/unit/test_models_emailaddress.py 2012-10-24 20:04:50 +0000 | |||
172 | +++ identityprovider/tests/unit/test_models_emailaddress.py 2013-01-25 22:16:21 +0000 | |||
173 | @@ -1,6 +1,7 @@ | |||
174 | 1 | from mock import patch | 1 | from mock import patch |
175 | 2 | 2 | ||
176 | 3 | from identityprovider.models import Account, EmailAddress | 3 | from identityprovider.models import Account, EmailAddress |
177 | 4 | from identityprovider.models.const import EmailStatus | ||
178 | 4 | from identityprovider.tests.utils import SSOBaseTestCase | 5 | from identityprovider.tests.utils import SSOBaseTestCase |
179 | 5 | 6 | ||
180 | 6 | 7 | ||
181 | @@ -12,6 +13,15 @@ | |||
182 | 12 | email = EmailAddress.objects.get(email='test@canonical.com') | 13 | email = EmailAddress.objects.get(email='test@canonical.com') |
183 | 13 | self.assertEqual(account, email.account) | 14 | self.assertEqual(account, email.account) |
184 | 14 | 15 | ||
185 | 16 | def test_emailaddress_is_verified(self): | ||
186 | 17 | email = EmailAddress.objects.get(email='test@canonical.com') | ||
187 | 18 | assert email.status == EmailStatus.PREFERRED | ||
188 | 19 | self.assertTrue(email.is_verified()) | ||
189 | 20 | email.status = EmailStatus.VALIDATED | ||
190 | 21 | self.assertTrue(email.is_verified()) | ||
191 | 22 | email.status = EmailStatus.NEW | ||
192 | 23 | self.assertFalse(email.is_verified()) | ||
193 | 24 | |||
194 | 15 | @patch('identityprovider.models.emailaddress.reverse') | 25 | @patch('identityprovider.models.emailaddress.reverse') |
195 | 16 | def test_account_admin_link(self, mock_reverse): | 26 | def test_account_admin_link(self, mock_reverse): |
196 | 17 | mock_reverse.return_value = '/foo' | 27 | mock_reverse.return_value = '/foo' |
197 | 18 | 28 | ||
198 | === modified file 'ubuntu_sso_saml/tests.py' | |||
199 | --- ubuntu_sso_saml/tests.py 2013-01-15 18:20:50 +0000 | |||
200 | +++ ubuntu_sso_saml/tests.py 2013-01-25 22:16:21 +0000 | |||
201 | @@ -176,7 +176,7 @@ | |||
202 | 176 | alternate_email = 'alternate@example.com' | 176 | alternate_email = 'alternate@example.com' |
203 | 177 | alternate, _ = EmailAddress.objects.get_or_create( | 177 | alternate, _ = EmailAddress.objects.get_or_create( |
204 | 178 | email=alternate_email, defaults={'account': account, | 178 | email=alternate_email, defaults={'account': account, |
206 | 179 | 'status': EmailStatus.NEW}) | 179 | 'status': EmailStatus.VALIDATED}) |
207 | 180 | # and make the alternate email the preferred | 180 | # and make the alternate email the preferred |
208 | 181 | account = change_preferred_email(self.login_email, alternate_email) | 181 | account = change_preferred_email(self.login_email, alternate_email) |
209 | 182 | self.assertEqual(account.preferredemail, alternate) | 182 | self.assertEqual(account.preferredemail, alternate) |
210 | 183 | 183 | ||
211 | === modified file 'webui/templates/account/edit.html' | |||
212 | --- webui/templates/account/edit.html 2012-12-04 18:51:42 +0000 | |||
213 | +++ webui/templates/account/edit.html 2013-01-25 22:16:21 +0000 | |||
214 | @@ -95,13 +95,22 @@ | |||
215 | 95 | </span> | 95 | </span> |
216 | 96 | </label> | 96 | </label> |
217 | 97 | <br /> | 97 | <br /> |
223 | 98 | {% if form.preferred_email.errors %} | 98 | {% if form.preferred_email %} |
224 | 99 | <span class="error"> | 99 | {% if form.preferred_email.errors %} |
225 | 100 | {{ form.preferred_email.errors|first }} | 100 | <span class="error"> |
226 | 101 | </span> | 101 | {{ form.preferred_email.errors|first }} |
227 | 102 | <br /> | 102 | </span> |
228 | 103 | <br /> | ||
229 | 104 | {% endif %} | ||
230 | 105 | {{ form.preferred_email }} | ||
231 | 106 | <span class="formHelp">{{ form.preferred_email.help_text }}</span> | ||
232 | 107 | {% else %} | ||
233 | 108 | {# Manually added, it will be ignored by the django form #} | ||
234 | 109 | {% with form.instance.preferredemail as unvalidated_preferred %} | ||
235 | 110 | <input type="text" value="{{ unvalidated_preferred }}" disabled="true" /> | ||
236 | 111 | <span class="formHelp">You still need to <a href="{% url verify_email %}?id={{ unvalidated_preferred.id }}">verify</a> your email address.</span> | ||
237 | 112 | {% endwith %} | ||
238 | 103 | {% endif %} | 113 | {% endif %} |
239 | 104 | {{ form.preferred_email }} | ||
240 | 105 | </p> | 114 | </p> |
241 | 106 | {% if not embedded %} | 115 | {% if not embedded %} |
242 | 107 | <p><a href="/+emails">{% trans "Manage email addresses" %}</a></p> | 116 | <p><a href="/+emails">{% trans "Manage email addresses" %}</a></p> |
243 | 108 | 117 | ||
244 | === modified file 'webui/tests/test_views_account.py' | |||
245 | --- webui/tests/test_views_account.py 2013-01-17 20:27:23 +0000 | |||
246 | +++ webui/tests/test_views_account.py 2013-01-25 22:16:21 +0000 | |||
247 | @@ -414,6 +414,8 @@ | |||
248 | 414 | def test_deactivate_account(self): | 414 | def test_deactivate_account(self): |
249 | 415 | # make sure we have a preferredemail | 415 | # make sure we have a preferredemail |
250 | 416 | email = self.account.emailaddress_set.all()[0] | 416 | email = self.account.emailaddress_set.all()[0] |
251 | 417 | email.status = EmailStatus.VALIDATED | ||
252 | 418 | email.save() | ||
253 | 417 | self.account.preferredemail = email | 419 | self.account.preferredemail = email |
254 | 418 | 420 | ||
255 | 419 | # deactivate account | 421 | # deactivate account |
256 | 420 | 422 | ||
257 | === modified file 'webui/tests/test_views_ui.py' | |||
258 | --- webui/tests/test_views_ui.py 2013-01-22 18:28:03 +0000 | |||
259 | +++ webui/tests/test_views_ui.py 2013-01-25 22:16:21 +0000 | |||
260 | @@ -273,26 +273,17 @@ | |||
261 | 273 | self.assertContains(response, "_gaq.push(['_setAccount', '12345']);") | 273 | self.assertContains(response, "_gaq.push(['_setAccount', '12345']);") |
262 | 274 | 274 | ||
263 | 275 | def test_claim_token_for_password_recovery_no_preferredemail(self): | 275 | def test_claim_token_for_password_recovery_no_preferredemail(self): |
266 | 276 | """ According to bug #524582 this should be handled gracefully by | 276 | """Prevent password reset for unvalidated emails.""" |
265 | 277 | verifying the email address and resetting the password. """ | ||
267 | 278 | account = Account.objects.get_by_email('test@canonical.com') | 277 | account = Account.objects.get_by_email('test@canonical.com') |
268 | 279 | _preferred_email = account.preferredemail | ||
269 | 280 | for email_obj in account.emailaddress_set.all(): | 278 | for email_obj in account.emailaddress_set.all(): |
270 | 281 | email_obj.status = EmailStatus.NEW | 279 | email_obj.status = EmailStatus.NEW |
271 | 282 | email_obj.save() | 280 | email_obj.save() |
272 | 283 | 281 | ||
285 | 284 | r = self.client.post(reverse('forgot_password'), | 282 | self.client.post(reverse('forgot_password'), |
286 | 285 | {'email': 'test@canonical.com'}) | 283 | {'email': 'test@canonical.com'}) |
287 | 286 | 284 | ||
288 | 287 | # reset password | 285 | # since there is verified email address, no email was sent |
289 | 288 | data = {'password': 'Password1', 'passwordconfirm': 'Password1'} | 286 | self.assertEqual(self.mock_send_email.call_count, 0) |
278 | 289 | r = self.client.post(self._token_url(), data) | ||
279 | 290 | self.assertRedirects(r, reverse('account-index')) | ||
280 | 291 | |||
281 | 292 | for email_obj in account.emailaddress_set.all(): | ||
282 | 293 | email_obj.status = EmailStatus.VALIDATED | ||
283 | 294 | email_obj.save() | ||
284 | 295 | account.preferredemail = _preferred_email | ||
290 | 296 | 287 | ||
291 | 297 | 288 | ||
292 | 298 | class EnterTokenTestCase(UIViewsBaseTestCase): | 289 | class EnterTokenTestCase(UIViewsBaseTestCase): |
293 | @@ -638,8 +629,7 @@ | |||
294 | 638 | self.assertRedirects(r, reverse('account-index')) | 629 | self.assertRedirects(r, reverse('account-index')) |
295 | 639 | 630 | ||
296 | 640 | def test_reset_password_when_account_deactivated(self): | 631 | def test_reset_password_when_account_deactivated(self): |
299 | 641 | """ Deactivated accounts can be reactivated using the reset password | 632 | """Prevent password reset for unvalidated emails.""" |
298 | 642 | functionality. Bug #556878 """ | ||
300 | 643 | r = self.client.post(reverse('forgot_password'), | 633 | r = self.client.post(reverse('forgot_password'), |
301 | 644 | {'email': 'test@canonical.com'}) | 634 | {'email': 'test@canonical.com'}) |
302 | 645 | 635 | ||
303 | @@ -656,38 +646,41 @@ | |||
304 | 656 | def test_reset_password_when_account_deactivated_no_preferred_email(self): | 646 | def test_reset_password_when_account_deactivated_no_preferred_email(self): |
305 | 657 | account = Account.objects.get_by_email('test@canonical.com') | 647 | account = Account.objects.get_by_email('test@canonical.com') |
306 | 658 | # make sure the account is deactivated and preferred email is removed | 648 | # make sure the account is deactivated and preferred email is removed |
307 | 659 | _preferred_email = account.preferredemail | ||
308 | 660 | account.status = AccountStatus.DEACTIVATED | 649 | account.status = AccountStatus.DEACTIVATED |
309 | 661 | # Blow out the cached value | 650 | # Blow out the cached value |
310 | 662 | account.save() | 651 | account.save() |
314 | 663 | for email_obj in account.emailaddress_set.all(): | 652 | account.emailaddress_set.update(status=EmailStatus.NEW) |
312 | 664 | email_obj.status = EmailStatus.NEW | ||
313 | 665 | email_obj.save() | ||
315 | 666 | 653 | ||
316 | 667 | account = Account.objects.get_by_email('test@canonical.com') | 654 | account = Account.objects.get_by_email('test@canonical.com') |
317 | 668 | 655 | ||
318 | 669 | if gargoyle.is_active('ALLOW_UNVALIDATED'): | 656 | if gargoyle.is_active('ALLOW_UNVALIDATED'): |
320 | 670 | self.assertEqual(account.preferredemail.email, 'test@canoical.com') | 657 | self.assertEqual(account.preferredemail.email, |
321 | 658 | 'test@canonical.com') | ||
322 | 671 | else: | 659 | else: |
323 | 672 | self.assertEqual(account.preferredemail, None) | 660 | self.assertEqual(account.preferredemail, None) |
324 | 673 | 661 | ||
342 | 674 | r = self.client.post(reverse('forgot_password'), | 662 | self.client.post(reverse('forgot_password'), |
343 | 675 | {'email': 'test@canonical.com'}) | 663 | {'email': 'test@canonical.com'}) |
344 | 676 | 664 | ||
345 | 677 | self.assertEqual(self.mock_send_email.call_count, 1) | 665 | # since there is verified email address, no email was sent |
346 | 678 | 666 | self.assertEqual(self.mock_send_email.call_count, 0) | |
347 | 679 | # reset password | 667 | |
348 | 680 | data = {'password': 'Password1', 'passwordconfirm': 'Password1'} | 668 | def test_reset_password_when_account_no_verified_emails(self): |
349 | 681 | r = self.client.post(self._token_url(), data) | 669 | account = Account.objects.get_by_email('test@canonical.com') |
350 | 682 | self.assertRedirects(r, reverse('account-index')) | 670 | account.emailaddress_set.update(status=EmailStatus.NEW) |
351 | 683 | 671 | self.client.post(reverse('forgot_password'), | |
352 | 684 | account = Account.objects.get_by_email('test@canonical.com') | 672 | {'email': 'test@canonical.com'}) |
353 | 685 | self.assertEqual(account.preferredemail.email, 'test@canonical.com') | 673 | # since there is verified email address, no email was sent |
354 | 686 | 674 | self.assertEqual(self.mock_send_email.call_count, 0) | |
355 | 687 | for email_obj in account.emailaddress_set.all(): | 675 | |
356 | 688 | email_obj.status = EmailStatus.VALIDATED | 676 | def test_reset_password_unverified_email_but_account_verified_email(self): |
357 | 689 | email_obj.save() | 677 | account = Account.objects.get_by_email('test@canonical.com') |
358 | 690 | account.preferredemail = _preferred_email | 678 | verified = account.verified_emails() |
359 | 679 | unverified = account.unverified_emails()[0] | ||
360 | 680 | self.client.post(reverse('forgot_password'), | ||
361 | 681 | {'email': unverified.email}) | ||
362 | 682 | # email sent to verified email addresses instead | ||
363 | 683 | self.assertEqual(self.mock_send_email.call_count, len(verified)) | ||
364 | 691 | 684 | ||
365 | 692 | def test_reset_password_when_account_noaccount(self): | 685 | def test_reset_password_when_account_noaccount(self): |
366 | 693 | r = self.client.post(reverse('forgot_password'), | 686 | r = self.client.post(reverse('forgot_password'), |
367 | 694 | 687 | ||
368 | === modified file 'webui/views/ui.py' | |||
369 | --- webui/views/ui.py 2013-01-22 18:28:03 +0000 | |||
370 | +++ webui/views/ui.py 2013-01-25 22:16:21 +0000 | |||
371 | @@ -768,30 +768,36 @@ | |||
372 | 768 | 768 | ||
373 | 769 | 769 | ||
374 | 770 | def _handle_reset_request(request, email, token): | 770 | def _handle_reset_request(request, email, token): |
375 | 771 | # options for actions to be taken | ||
376 | 772 | NONE, RESET, CREATE = range(3) | ||
377 | 773 | # assume invalid | ||
378 | 774 | send_email = NONE | ||
379 | 775 | account = Account.objects.get_by_email(email) | 771 | account = Account.objects.get_by_email(email) |
380 | 776 | if account is not None: | 772 | if account is not None: |
381 | 773 | verified_emails = account.verified_emails() | ||
382 | 777 | if account.can_reset_password: | 774 | if account.can_reset_password: |
384 | 778 | send_email = RESET | 775 | redirection_url = redirection_url_for_token(token) |
385 | 776 | if not verified_emails.filter(email=email).count() > 0: | ||
386 | 777 | # provided email is not verified | ||
387 | 778 | # send email to verified addresses instead | ||
388 | 779 | for email_obj in verified_emails: | ||
389 | 780 | emailutils.send_password_reset_email(account, | ||
390 | 781 | email_obj.email, | ||
391 | 782 | redirection_url) | ||
392 | 783 | else: | ||
393 | 784 | emailutils.send_password_reset_email(account, email, | ||
394 | 785 | redirection_url) | ||
395 | 786 | set_session_email(request.session, email) | ||
396 | 787 | elif verified_emails.count() == 0: | ||
397 | 788 | # user does not have any verified email address | ||
398 | 789 | # he should contact support | ||
399 | 790 | condition = ("account '%s' has no verified email address" % | ||
400 | 791 | account.displayname) | ||
401 | 792 | logger.debug("In view 'forgot_password' email was not " | ||
402 | 793 | "sent out because %s" % condition) | ||
403 | 779 | else: | 794 | else: |
404 | 780 | send_email = NONE | ||
405 | 781 | # log why email was not sent | 795 | # log why email was not sent |
406 | 782 | condition = ("account '%s' is not active" % | 796 | condition = ("account '%s' is not active" % |
407 | 783 | account.displayname) | 797 | account.displayname) |
408 | 784 | logger.debug("In view 'forgot_password' email was not " | 798 | logger.debug("In view 'forgot_password' email was not " |
409 | 785 | "sent out because %s" % condition) | 799 | "sent out because %s" % condition) |
410 | 786 | else: | 800 | else: |
411 | 787 | send_email = CREATE | ||
412 | 788 | |||
413 | 789 | # we should know now the action we need to take | ||
414 | 790 | if send_email == RESET: | ||
415 | 791 | redirection_url = redirection_url_for_token(token) | ||
416 | 792 | emailutils.send_password_reset_email(account, email, redirection_url) | ||
417 | 793 | set_session_email(request.session, email) | ||
418 | 794 | elif send_email == CREATE: | ||
419 | 795 | # they've tried to reset with an invalid email, so send them an email | 801 | # they've tried to reset with an invalid email, so send them an email |
420 | 796 | # on how to create an account | 802 | # on how to create an account |
421 | 797 | emailutils.send_invitation_after_password_reset(email) | 803 | emailutils.send_invitation_after_password_reset(email) |
422 | @@ -833,18 +839,26 @@ | |||
423 | 833 | msgs = { | 839 | msgs = { |
424 | 834 | 'email_to': email, | 840 | 'email_to': email, |
425 | 835 | 'email_from': settings.NOREPLY_FROM_ADDRESS, | 841 | 'email_from': settings.NOREPLY_FROM_ADDRESS, |
426 | 842 | 'support_form_url': settings.SUPPORT_FORM_URL, | ||
427 | 836 | } | 843 | } |
428 | 844 | reason = _( | ||
429 | 845 | "We've just emailed " | ||
430 | 846 | "%(email_to)s (from %(email_from)s) with " | ||
431 | 847 | "instructions on resetting your password.<br/><br/>" | ||
432 | 848 | "If the email address you provided has not been verified " | ||
433 | 849 | "we'll use your account's verified email address instead. " | ||
434 | 850 | "If you don't have a verified email address please " | ||
435 | 851 | "<a href='%(support_form_url)s'>contact support</a>." | ||
436 | 852 | ) % msgs | ||
437 | 837 | return display_email_sent( | 853 | return display_email_sent( |
438 | 838 | request, | 854 | request, |
439 | 839 | email, | 855 | email, |
440 | 840 | _("Forgotten your password?"), | 856 | _("Forgotten your password?"), |
444 | 841 | _("We’ve just emailed " | 857 | reason, |
442 | 842 | "%(email_to)s (from %(email_from)s) with " | ||
443 | 843 | "instructions on resetting your password.") % msgs, | ||
445 | 844 | _("Check that you’ve actually " | 858 | _("Check that you’ve actually " |
446 | 845 | "entered a subscribed email address."), | 859 | "entered a subscribed email address."), |
447 | 846 | token=token, | 860 | token=token, |
449 | 847 | rpconfig=rpconfig | 861 | rpconfig=rpconfig, |
450 | 848 | ) | 862 | ) |
451 | 849 | else: | 863 | else: |
452 | 850 | # track form errors | 864 | # track form errors |
453 | @@ -864,6 +878,7 @@ | |||
454 | 864 | 878 | ||
455 | 865 | atrequest = verify_token_string(authtoken, email_address) | 879 | atrequest = verify_token_string(authtoken, email_address) |
456 | 866 | account = atrequest and atrequest.requester | 880 | account = atrequest and atrequest.requester |
457 | 881 | |||
458 | 867 | if (atrequest is None or | 882 | if (atrequest is None or |
459 | 868 | atrequest.token_type != TokenType.PASSWORDRECOVERY or | 883 | atrequest.token_type != TokenType.PASSWORDRECOVERY or |
460 | 869 | not account.can_reset_password): | 884 | not account.can_reset_password): |
461 | @@ -894,14 +909,6 @@ | |||
462 | 894 | msg = _("Your account was reactivated, and the preferred " | 909 | msg = _("Your account was reactivated, and the preferred " |
463 | 895 | "email address was set to {email}.") | 910 | "email address was set to {email}.") |
464 | 896 | messages.info(request, msg.format(email=email_obj.email)) | 911 | messages.info(request, msg.format(email=email_obj.email)) |
465 | 897 | elif account.preferredemail is None: | ||
466 | 898 | # handle this case gracefully according to #524582 | ||
467 | 899 | email_obj, created = EmailAddress.objects.get_or_create( | ||
468 | 900 | account=account, | ||
469 | 901 | email=atrequest.requester_email, | ||
470 | 902 | defaults={'status': EmailStatus.PREFERRED}) | ||
471 | 903 | if not created: | ||
472 | 904 | account.preferredemail = email_obj | ||
473 | 905 | email = account.preferredemail.email | 912 | email = account.preferredemail.email |
474 | 906 | account.set_password(password) | 913 | account.set_password(password) |
475 | 907 | user = auth.authenticate(username=email, password=password) | 914 | user = auth.authenticate(username=email, password=password) |
Looks good!