launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

Deryck Hodge has proposed merging lp:~deryck/launchpad/reauth-gpg-363916 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~deryck/launchpad/reauth-gpg-363916/+merge/120401

= 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
-- 
https://code.launchpad.net/~deryck/launchpad/reauth-gpg-363916/+merge/120401
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/reauth-gpg-363916 into lp:launchpad.

=== 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-20 14:49:21 +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-20 14:49:21 +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'))