Merge lp:~canonical-isd-hackers/canonical-identity-provider/bug-816622 into lp:canonical-identity-provider/release

[Łukasz Czyżykowski]

Fix bug #816622: Users can login with unverified e-mail addresses

Overview
========
This branch fixes Bug: 816622, which was allowing users to log in to
SSO with an email account which was not yet verified.

Details
=======
The reason of this bug showing up is that there is a discrepancy
between API and Web. In the API it's possible to log in to an account
without verifying the email first. It was never the case for the
web. When working on limiting the number of queries and consolidating
codebase, one method is being used for both of those scenarios and the
change have made the behaviour consistent.

The change explicitly prevents logging in with email with status
'NEW' in the ``LoginForm``.

To test it follow instructions in ``README`` and then run ``$ fab test``.

[Ricardo Kirkner (ricardokirkner)]

LGTM, except that the original code verified email status against VALIDATED, OLD and PREFERRED instead of not NEW.
I think since this is a regression, we should keep the same condition check.

review: Needs Fixing

=== modified file 'identityprovider/forms.py'
--- identityprovider/forms.py	2011-08-05 09:42:18 +0000
+++ identityprovider/forms.py	2011-08-25 13:30:25 +0000
@@ -62,6 +62,9 @@
             email = self.cleaned_data['email']
             password = self.cleaned_data['password']
 
+            password_error = forms.ValidationError(
+                mark_safe(_("Password didn't match.")))
+
             account = auth.authenticate(username=email, password=password)
             if account is None:
                 account = Account.objects.get_by_email(email)
@@ -69,8 +72,14 @@
                     raise forms.ValidationError(_("Your account has been "
                                                   "deactivated"))
                 else:
-                    raise forms.ValidationError(
-                        mark_safe(_("Password didn't match.")))
+                    raise password_error
+
+            # Check that the email used for logging in is not 'NEW'. It's
+            # possible to use it to log in to the SSO with API, but not through
+            # web.
+            email_obj = account.emailaddress_set.get(email=email)
+            if email_obj.status == EmailStatus.NEW:
+                raise password_error
 
             # Pass account to the view, as it's already authenticated and
             # there's no good reason to do exactly the same call there.

=== modified file 'identityprovider/tests/test_views_ui.py'
--- identityprovider/tests/test_views_ui.py	2011-08-10 19:31:53 +0000
+++ identityprovider/tests/test_views_ui.py	2011-08-25 13:30:25 +0000
@@ -176,6 +176,15 @@
                                          'password': 'test'})
         self.assertRedirects(r, '/')
 
+    def test_login_with_not_validated_email(self):
+        """Preventing regression for bug 816622"""
+        account = Account.objects.get_by_email("[e-mail]")
+        account.emailaddress_set.create(
+            email="[e-mail]", status=EmailStatus.NEW)
+        r = self.client.post('/+login', {'email': '[e-mail]',
+                                         'password': 'test'})
+        self.assertFormError(r, 'form', None, "Password didn't match.")
+
     def test_logout(self):
         self.authenticate()
         r = self.client.get('/+logout')