launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

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

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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


-- 
https://code.launchpad.net/~deryck/launchpad/reauth-ssh-363916/+merge/120000
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/reauth-ssh-363916 into lp:launchpad.

=== modified file 'lib/lp/code/stories/branches/xx-upload-directions.txt'
--- lib/lp/code/stories/branches/xx-upload-directions.txt	2011-05-31 22:28:58 +0000
+++ lib/lp/code/stories/branches/xx-upload-directions.txt	2012-08-16 18:35:25 +0000
@@ -9,7 +9,13 @@
 We will need multiple browsers, logged in with different users. Set them up
 now.
 
-    >>> name12_browser = setupBrowser(auth='Basic test@xxxxxxxxxxxxx:test')
+    >>> from zope.component import getUtility
+    >>> from lp.registry.interfaces.person import IPersonSet
+    >>> from lp.testing.pages import setupBrowserFreshLogin
+    >>> login(ANONYMOUS)
+    >>> name12 = getUtility(IPersonSet).getByEmail('test@xxxxxxxxxxxxx')
+    >>> logout()
+    >>> name12_browser = setupBrowserFreshLogin(name12)
     >>> ddaa_browser = setupBrowser(
     ...     auth='Basic david.allouche@xxxxxxxxxxxxx:test')
 

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2012-08-15 17:18:09 +0000
+++ lib/lp/registry/browser/person.py	2012-08-16 18:35:25 +0000
@@ -259,8 +259,8 @@
     IOpenLaunchBag,
     )
 from lp.services.webapp.login import (
-    isFreshLogin,
     logoutPerson,
+    require_fresh_login,
     )
 from lp.services.webapp.menu import get_current_view
 from lp.services.webapp.publisher import LaunchpadView
@@ -2413,6 +2413,8 @@
     error_message = None
 
     def initialize(self):
+        require_fresh_login(self.request, self.context, '+editsshkeys')
+
         if self.request.method != "POST":
             # Nothing to do
             return
@@ -2780,11 +2782,7 @@
     label = 'Change your e-mail settings'
 
     def initialize(self):
-        if not isFreshLogin(self.request):
-            reauth_query = '+login?reauth=1'
-            base_url = canonical_url(self.context, view_name='+editemails')
-            login_url = '%s/%s' % (base_url, reauth_query)
-            self.request.response.redirect(login_url)
+        require_fresh_login(self.request, self.context, '+editemails')
         if self.context.is_team:
             # +editemails is not available on teams.
             name = self.request['PATH_INFO'].split('/')[-1]

=== modified file 'lib/lp/registry/browser/tests/test_person.py'
--- lib/lp/registry/browser/tests/test_person.py	2012-08-16 13:05:53 +0000
+++ lib/lp/registry/browser/tests/test_person.py	2012-08-16 18:35:25 +0000
@@ -621,6 +621,15 @@
             " doesn't seem to be a valid email address.")
         self._assertEmailAndError(xss_email, expected_msg)
 
+    def test_edit_email_login_redirect(self):
+        """+editemails should redirect to +login to force you to re-authenticate."""
+        view = create_initialized_view(self.person, "+editemails")
+        response = view.request.response
+        self.assertEqual(302, response.getStatus())
+        expected_url = (
+            '%s/+editemails/+login?reauth=1' % canonical_url(self.person))
+        self.assertEqual(expected_url, response.getHeader('location'))
+
 
 class PersonAdministerViewTestCase(TestPersonRenameFormMixin,
                                    TestCaseWithFactory):

=== modified file 'lib/lp/registry/browser/tests/test_sshkey.py'
--- lib/lp/registry/browser/tests/test_sshkey.py	2012-06-14 05:18:22 +0000
+++ lib/lp/registry/browser/tests/test_sshkey.py	2012-08-16 18:35:25 +0000
@@ -10,6 +10,7 @@
 from lp.registry.interfaces.ssh import ISSHKeySet
 from lp.services.webapp import canonical_url
 from lp.testing import (
+    login_person,
     person_logged_in,
     TestCaseWithFactory,
     )
@@ -18,6 +19,7 @@
     extract_text,
     find_tags_by_class,
     )
+from lp.testing.views import create_initialized_view
 
 
 class TestCanonicalUrl(TestCaseWithFactory):
@@ -55,3 +57,14 @@
             self.assertEqual(
                 extract_text(find_tags_by_class(browser.contents, 'message')[0]),
                 msg)
+
+    def test_edit_ssh_keys_login_redirect(self):
+        """+editsshkeys should redirect to +login to force you to re-authenticate."""
+        person = self.factory.makePerson()
+        login_person(person)
+        view = create_initialized_view(person, "+editsshkeys")
+        response = view.request.response
+        self.assertEqual(302, response.getStatus())
+        expected_url = (
+            '%s/+editsshkeys/+login?reauth=1' % canonical_url(person))
+        self.assertEqual(expected_url, response.getHeader('location'))

=== modified file 'lib/lp/registry/stories/person/xx-add-sshkey.txt'
--- lib/lp/registry/stories/person/xx-add-sshkey.txt	2012-04-16 12:31:43 +0000
+++ lib/lp/registry/stories/person/xx-add-sshkey.txt	2012-08-16 18:35:25 +0000
@@ -28,8 +28,14 @@
 
 Salgado sees a message explaining that he can register his ssh key.
 
-    >>> browser = setupBrowser(
-    ...     auth='Basic guilherme.salgado@xxxxxxxxxxxxx:test')
+    >>> from zope.component import getUtility
+    >>> from lp.registry.interfaces.person import IPersonSet
+    >>> from lp.testing.pages import setupBrowserFreshLogin
+    >>> login(ANONYMOUS)
+    >>> salgado = getUtility(IPersonSet).getByEmail(
+    ...     'guilherme.salgado@xxxxxxxxxxxxx')
+    >>> logout()
+    >>> browser = setupBrowserFreshLogin(salgado)
     >>> browser.open('http://launchpad.dev/~salgado')
     >>> print browser.title
     Guilherme Salgado in Launchpad
@@ -97,6 +103,10 @@
 
 Launchpad administrators are not allowed to poke at other user's ssh keys.
 
+    >>> login(ANONYMOUS)
+    >>> foo_bar = getUtility(IPersonSet).getByEmail('foo.bar@xxxxxxxxxxxxx')
+    >>> logout()
+    >>> admin_browser = setupBrowserFreshLogin(foo_bar)
     >>> admin_browser.open('http://launchpad.dev/~salgado/+editsshkeys')
     Traceback (most recent call last):
     ...

=== modified file 'lib/lp/services/webapp/login.py'
--- lib/lp/services/webapp/login.py	2012-08-13 16:09:26 +0000
+++ lib/lp/services/webapp/login.py	2012-08-16 18:35:25 +0000
@@ -52,6 +52,7 @@
 from lp.services.openid.interfaces.openidconsumer import IOpenIDConsumerStore
 from lp.services.propertycache import cachedproperty
 from lp.services.timeline.requesttimeline import get_request_timeline
+from lp.services.webapp import canonical_url
 from lp.services.webapp.dbpolicy import MasterDatabasePolicy
 from lp.services.webapp.error import SystemErrorView
 from lp.services.webapp.interfaces import (
@@ -460,6 +461,15 @@
     return False
 
 
+def require_fresh_login(request, context, view_name):
+    """Redirect request to login if the request is not recently logged in."""
+    if not isFreshLogin(request):
+        reauth_query = '+login?reauth=1'
+        base_url = canonical_url(context, view_name=view_name)
+        login_url = '%s/%s' % (base_url, reauth_query)
+        request.response.redirect(login_url)
+
+
 def logInPrincipal(request, principal, email, when=None):
     """Log the principal in. Password validation must be done in callsites."""
     # Force a fresh session, per Bug #828638. Any changes to any