Merge lp:~tartley/canonical-identity-provider/rm-no-verified-address-para into lp:canonical-identity-provider/release

[Jonathan Hartley]

Remove code that cannot be reached.

The containing 'if not account.can_reset_password',
16 lines up from the deletion,
does not check whether there are any validated email addresses.
Instead it just checks that account.status must be either
Suspended or Deleted.

Suspended accounts are handled by an early return,
9 lines up from the deletion.

Deleted accounts also delete their associated email addresses,
so can not be retrieved by the Account.get_by_email call
near the start of this method.

There was a test for this deleted code,
but it monkey patched a fake value to make this code reachable.
I tried replacing the test with one that didn't do monkey patching
(e.g. using a deleted account would be the only way)
but was unable to make the code reachable.

In practice, accounts with no validated email addresses pass
right by this code, and end up reading from account.preferredemail,
14 lines below the deletion,
which falls back to using a new email address.

[Matias Bordese (matiasb)]

Reviewed possible account status values, makes sense to me.
Even more, DEACTIVATED is not possible here (but I just noted you comment and fix on this in your next MP!).

review: Approve

[Matias Bordese (matiasb)]

Reviewed possible account status values, makes sense to me.
Even more, DEACTIVATED is not possible here (but I just noted you comment and fix on this in your next MP!).

review: Approve

=== modified file 'src/api/v20/handlers.py'
--- src/api/v20/handlers.py	2020-01-09 16:39:50 +0000
+++ src/api/v20/handlers.py	2020-01-09 16:39:50 +0000
@@ -300,14 +300,6 @@
                              "not sent out because %s" % condition)
                 return errors.ACCOUNT_DEACTIVATED()
 
-            # user does not have any verified email address
-            # he should contact support
-            condition = ("account '%s' is not allowed to reset password" %
-                         account.displayname)
-            logger.debug("PasswordResetTokenHandler.create: email was not "
-                         "sent out because %s" % condition)
-            return errors.CAN_NOT_RESET_PASSWORD()
-
         # account_email is populated from our database.
         # It is safe to use to generate tokens, send emails, etc.
         account_email = None

=== modified file 'src/api/v20/tests/test_handlers.py'
--- src/api/v20/tests/test_handlers.py	2020-01-09 16:39:50 +0000
+++ src/api/v20/tests/test_handlers.py	2020-01-09 16:39:50 +0000
@@ -1772,20 +1772,6 @@
         self.mock_logger.debug("PasswordResetTokenHandler.create: email was "
                                "not sent out because %s" % condition)
 
-    def test_can_not_reset_password(self):
-        name = 'identityprovider.models.account.Account.can_reset_password'
-        with patch(name, False):
-            content = self.do_post(data=self.data, status_code=403)
-
-        self.assertEqual(content, {
-            'message': 'Can not reset password. Please contact login support',
-            'code': 'CAN_NOT_RESET_PASSWORD',
-            'extra': {}})
-
-        condition = "account '%s' is not active" % self.account.displayname
-        self.mock_logger.debug("PasswordResetTokenHandler.create: email was "
-                               "not sent out because %s" % condition)
-
     def assert_email_used(self, result, expected_email):
         # Then the response contains the user's stored email.
         self.assertEqual(result['email'], expected_email)