Merge lp:~salgado/launchpad/bug-452525 into lp:launchpad

[Guilherme Salgado]

[Guilherme Salgado (salgado)]

= Summary =

Today, if you try to reset the password of an account in the NOACCOUNT
state, the password will be changed but the account will be left
unusable.

== Proposed fix ==

Just like we do when resetting the password of a DEACTIVATED account,
activate the account (in the NOACCOUNT state) for which the password is
being reset.

The actual bug was triggered through the login service, but a test for
that would have to go into the canonical-identity-provider, whereas the
fix (as you can see) is in launchpad. What I'd like to do is land this
fix with the LP-specific test and open an extra task on bug 452525 for
canonical-identity-provider to have a similar test there. The test
there would be really trivial as it'd just have to inherit from
TestPasswordReset (added here).

== Tests ==

./bin/test -vvt test_password_reset

== Demo and Q/A ==

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/browser/tests/test_password_reset.py
  lib/devscripts/ec2test/instance.py
  lib/canonical/launchpad/browser/logintoken.py

== Pylint notices ==

lib/devscripts/ec2test/instance.py
    409: [W0703, EC2Instance.set_up_and_run] Catch "Exception"

[Aaron Bentley (abentley)]

As discussed on IRC, it would be nice to change the brower test to a view test, but I think this is landable as-is.

review: Approve (code)

=== modified file 'lib/canonical/launchpad/browser/logintoken.py'
--- lib/canonical/launchpad/browser/logintoken.py	2009-09-18 01:09:10 +0000
+++ lib/canonical/launchpad/browser/logintoken.py	2009-10-26 15:06:13 +0000
@@ -221,7 +221,8 @@
 
         naked_account = removeSecurityProxy(account)
         # Reset password can be used to reactivate a deactivated account.
-        if account.status == AccountStatus.DEACTIVATED:
+        inactive_states = [AccountStatus.DEACTIVATED, AccountStatus.NOACCOUNT]
+        if account.status in inactive_states:
             self.reactivate(data)
             self.request.response.addInfoNotification(
                 _('Welcome back to Launchpad.'))

=== added file 'lib/canonical/launchpad/browser/tests/test_password_reset.py'
--- lib/canonical/launchpad/browser/tests/test_password_reset.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/browser/tests/test_password_reset.py	2009-10-26 15:06:13 +0000
@@ -0,0 +1,55 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+import unittest
+
+from zope.component import getUtility
+
+import transaction
+from canonical.launchpad.browser.logintoken import ResetPasswordView
+from canonical.launchpad.ftests import LaunchpadFormHarness
+from canonical.launchpad.interfaces.account import AccountStatus
+from canonical.launchpad.interfaces.authtoken import LoginTokenType
+from canonical.launchpad.interfaces.emailaddress import EmailAddressStatus
+from canonical.launchpad.interfaces.logintoken import ILoginTokenSet
+from lp.testing import TestCaseWithFactory
+from canonical.testing import DatabaseFunctionalLayer
+
+
+class TestPasswordReset(TestCaseWithFactory):
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        TestCaseWithFactory.setUp(self)
+        self.email = '[e-mail]'
+        self.person = self.factory.makePerson(
+            email=self.email, email_address_status=EmailAddressStatus.NEW)
+        self.account = self.person.account
+
+    def test_inactive_accounts_are_activated(self):
+        # Resetting the password of an account in the NOACCOUNT state will
+        # activate it.
+        self.assertEquals(self.account.status, AccountStatus.NOACCOUNT)
+        token = getUtility(ILoginTokenSet).new(
+            self.person, self.email, self.email,
+            LoginTokenType.PASSWORDRECOVERY)
+        self._completePasswordReset(token)
+        self.assertEquals(self.account.status, AccountStatus.ACTIVE)
+        self.assertEquals(
+            self.account.preferredemail.email, '[e-mail]')
+
+    def _completePasswordReset(self, token):
+        harness = LaunchpadFormHarness(token, ResetPasswordView)
+        form = {'field.email': self.email,
+                'field.password': 'test',
+                'field.password_dupe': 'test'}
+        harness.submit('continue', form)
+        self.assertFalse(harness.hasErrors())
+        # Need to manually commit because we're interested in testing the
+        # changes done by the view on the token's requester (i.e.
+        # self.person).
+        transaction.commit()
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/devscripts/ec2test/instance.py'
--- lib/devscripts/ec2test/instance.py	2009-10-05 19:20:37 +0000
+++ lib/devscripts/ec2test/instance.py	2009-10-26 15:06:13 +0000
@@ -169,7 +169,7 @@
 EC2Instance is available as `instance`.
 Also try these:
   http://%(dns)s/current_test.log
-  ssh -A %(dns)s
+  ssh -A ec2test@%(dns)s
 """