Merge lp:~deryck/launchpad/reauth-gpg-363916 into lp:launchpad

[Deryck Hodge]

Requrie a fresh login from users editing their gpg keys.

= Summary =

This branch is the final in a series of branches to fix bug 363916. This will ensure that all authentication info for users cannot be edited with an old session. The user will need to refresh his or her login to edit gpg keys after this lands. The work that has gone before added this fresh login requirement for email and ssh keys.

== Proposed fix ==

The fix is pretty simple -- just use require_fresh_login -- which was added in earlier branches.

== Pre-implementation notes ==

I didn't discuss this particular branch with anyone, but I had several conversations with flacoste about the earlier work and branches.

== LOC Rationale ==

I've got LOC credit from earlier work to refactor doctests. This is the final branch to add code, and I believe I'll be -50 lines all told once this lands.

== Implementation details ==

* Add a test to ensure the view redirects if the login is stale
* Add an initialize method which calls require_fresh_login

== Tests ==

./bin/test -cvvt test_edit_pgp_keys_login_redirect

(There will be a few doctests that break, I expect, but I'm running this through ec2 while I get it reviewed. I'll fix up any tests that I find and do another ec2 run to land it.)

== Demo and Q/A ==

* Login to Launchpad
* Get coffee to wait 2 minutes
* Visit a user's profile page
* Click the edit icon next to OpenPGP Keys

At this point, you should be redirect to the login server.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/tests/test_gpgkey.py
  lib/lp/registry/browser/person.py

[Benji York (benji)]

Looks good.

review: Approve (code)

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2012-08-20 13:28:42 +0000
+++ lib/lp/registry/browser/person.py	2012-08-21 14:43:20 +0000
@@ -2495,6 +2495,10 @@
     error_message = None
     info_message = None
 
+    def initialize(self):
+        require_fresh_login(self.request, self.context, '+editpgpkeys')
+        super(PersonGPGView, self).initialize()
+
     @property
     def cancel_url(self):
         return canonical_url(self.context, view_name="+edit")

=== modified file 'lib/lp/registry/browser/tests/test_gpgkey.py'
--- lib/lp/registry/browser/tests/test_gpgkey.py	2012-01-01 02:58:52 +0000
+++ lib/lp/registry/browser/tests/test_gpgkey.py	2012-08-21 14:43:20 +0000
@@ -6,8 +6,12 @@
 __metaclass__ = type
 
 from lp.services.webapp import canonical_url
-from lp.testing import TestCaseWithFactory
+from lp.testing import (
+    login_person,
+    TestCaseWithFactory,
+    )
 from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.views import create_initialized_view
 
 
 class TestCanonicalUrl(TestCaseWithFactory):
@@ -22,3 +26,19 @@
             '%s/+gpg-keys/%s' % (
                 canonical_url(person, rootsite='api'), gpgkey.keyid),
             canonical_url(gpgkey))
+
+
+class TestPersonGPGView(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_edit_pgp_keys_login_redirect(self):
+        """+editpgpkeys should redirect to force you to re-authenticate."""
+        person = self.factory.makePerson()
+        login_person(person)
+        view = create_initialized_view(person, "+editpgpkeys")
+        response = view.request.response
+        self.assertEqual(302, response.getStatus())
+        expected_url = (
+            '%s/+editpgpkeys/+login?reauth=1' % canonical_url(person))
+        self.assertEqual(expected_url, response.getHeader('location'))

=== modified file 'lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt'
--- lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt	2012-08-10 18:16:36 +0000
+++ lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt	2012-08-21 14:43:20 +0000
@@ -6,6 +6,9 @@
     >>> import email
     >>> from lp.testing.keyserver import KeyServerTac
     >>> from lp.services.mail import stub
+    >>> from zope.component import getUtility
+    >>> from lp.registry.interfaces.person import IPersonSet
+    >>> from lp.testing.pages import setupBrowserFreshLogin
 
 Set up the stub KeyServer:
 
@@ -19,7 +22,10 @@
 
 Start out with a clean page containing no imported keys:
 
-    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> login(ANONYMOUS)
+    >>> name12 = getUtility(IPersonSet).getByEmail('[e-mail]')
+    >>> logout()
+    >>> browser = setupBrowserFreshLogin(name12)
     >>> browser.open("http://launchpad.dev/~name12")
     >>> browser.getLink(url='+editpgpkeys').click()
     >>> print browser.title
@@ -86,7 +92,6 @@
 
 Import the secret keys needed for this test:
 
-    >>> from zope.component import getUtility
     >>> from lp.services.gpg.interfaces import IGPGHandler
 
     >>> from lp.testing.gpgkeys import (
@@ -141,13 +146,7 @@
 added as an email address:
 
     >>> from lp.testing.pages import strip_label
-    >>> from lp.registry.interfaces.person import IPersonSet
-    >>> from lp.testing.pages import setupBrowserFreshLogin
 
-    >>> login(ANONYMOUS)
-    >>> person = getUtility(IPersonSet).getByEmail('[e-mail]')
-    >>> logout()
-    >>> browser = setupBrowserFreshLogin(person)
     >>> browser.open("http://launchpad.dev/~name12/+editemails")
     >>> control = browser.getControl(name='field.UNVALIDATED_SELECTED')
     >>> [strip_label(label) for label in sorted(control.displayOptions)]
@@ -461,6 +460,7 @@
 
 Now Sample Person will deactivate his key...
 
+    >>> browser = setupBrowserFreshLogin(name12)
     >>> browser.open('http://launchpad.dev/~name12/+editpgpkeys')
     >>> browser.url
     'http://launchpad.dev/~name12/+editpgpkeys'

=== modified file 'lib/lp/registry/stories/person/xx-person-editgpgkeys-invalid-key.txt'
--- lib/lp/registry/stories/person/xx-person-editgpgkeys-invalid-key.txt	2011-12-24 15:18:32 +0000
+++ lib/lp/registry/stories/person/xx-person-editgpgkeys-invalid-key.txt	2012-08-21 14:43:20 +0000
@@ -8,6 +8,7 @@
     ...     ILoginTokenSet)
     >>> from lp.testing.keyserver import KeyServerTac
     >>> from lp.registry.interfaces.person import IPersonSet
+    >>> from lp.testing.pages import setupBrowserFreshLogin
 
     >>> tokenset = getUtility(ILoginTokenSet)
     >>> tac = KeyServerTac()
@@ -27,7 +28,10 @@
 
 Attempts to claim a revoked OpenPGP key fail:
 
-    >>> browser.addHeader('Authorization', 'Basic [e-mail]:test')
+    >>> login(ANONYMOUS)
+    >>> name12 = getUtility(IPersonSet).getByEmail('[e-mail]')
+    >>> logout()
+    >>> browser = setupBrowserFreshLogin(name12)
     >>> browser.open('http://launchpad.dev/~name12/+editpgpkeys')
     >>> browser.getControl(
     ...     name='fingerprint').value = (