Merge lp:~deryck/launchpad/support-pape-max-auth-age into lp:launchpad

Proposed by Deryck Hodge
Status: Merged
Approved by: Deryck Hodge
Approved revision: no longer in the source branch.
Merged at revision: 15759
Proposed branch: lp:~deryck/launchpad/support-pape-max-auth-age
Merge into: lp:launchpad
Diff against target: 102 lines (+35/-7)
3 files modified
lib/lp/services/webapp/login.py (+14/-3)
lib/lp/services/webapp/tests/test_login.py (+20/-1)
versions.cfg (+1/-3)
To merge this branch: bzr merge lp:~deryck/launchpad/support-pape-max-auth-age
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+117781@code.launchpad.net

Commit message

Add support for PAPE extension max_auth_age to Launchpad login, which will allow us to force users to refresh their logins.

Description of the change

This branch is a first step in fixing bug 363916. That bug notes that we need to re-authenticate users when they try to update their email, gpg, or ssh info. In order to redirect them to Ubuntu SSO and refresh their login, we need to make use of PAPE extensions in our login process and set max_auth_age to 0.

This branch updates the login process to accept a reauth=1 query parameter. If that param is present when you redirect a user to +login, then the user will be prompted to re-authenticate and get a fresh login.

This work is being landed in isolation. There is a test to confirm this works, but otherwise, it's unused at this point. The branch to make use of it for updating email addresses was growing large, so I split this off by itself.

We also need a newer version of python-openid that supports pape extensions, so I've updated versions.cfg, too. This version of python-openid was committed to download-cache sometime ago, when I started in this work. I confirmed with wgrant that we had our patch in the fork version applied upstream. So we're fine to run a regular release now.

In terms of LOC change, this branch adds lines of code, but I have other branches that preceded this that changed doctests or dropped tests altogether which has given me some LOC credit for this work. Those tests were related to updating emails and didn't play well with re-authenticating, so the doctests were dropped in favor of unit tests, which gave me about 350 LOC credit.

I had a couple pre-implementation style calls with Francis about this work when I started work on it.

The branch is lint free.

To post a comment you must log in.
Revision history for this message
Abel Deuring (adeuring) wrote :

looks good

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/services/webapp/login.py'
2--- lib/lp/services/webapp/login.py 2012-01-20 06:29:10 +0000
3+++ lib/lp/services/webapp/login.py 2012-08-06 13:07:21 +0000
4@@ -16,7 +16,10 @@
5 FAILURE,
6 SUCCESS,
7 )
8-from openid.extensions import sreg
9+from openid.extensions import (
10+ pape,
11+ sreg,
12+ )
13 from openid.fetchers import (
14 setDefaultFetcher,
15 Urllib2Fetcher,
16@@ -194,7 +197,10 @@
17 return Consumer(session, openid_store)
18
19 def render(self):
20- if self.account is not None:
21+ # Reauthentication is called for by a query string parameter.
22+ reauth_qs = self.request.query_string_params.get('reauth', ['0'])
23+ do_reauth = int(reauth_qs[0])
24+ if self.account is not None and not do_reauth:
25 return AlreadyLoggedInView(self.context, self.request)()
26
27 # Allow unauthenticated users to have sessions for the OpenID
28@@ -215,6 +221,11 @@
29 self.openid_request.addExtension(
30 sreg.SRegRequest(required=['email', 'fullname']))
31
32+ # Force the Open ID handshake to re-authenticate, using
33+ # pape extension's max_auth_age, if the URL indicates it.
34+ if do_reauth:
35+ self.openid_request.addExtension(pape.Request(max_auth_age=0))
36+
37 assert not self.openid_request.shouldSendRedirect(), (
38 "Our fixed OpenID server should not need us to redirect.")
39 # Once the user authenticates with the OpenID provider they will be
40@@ -242,7 +253,7 @@
41 def starting_url(self):
42 starting_url = self.request.getURL(1)
43 query_string = "&".join([arg for arg in self.form_args])
44- if query_string:
45+ if query_string and query_string != 'reauth=1':
46 starting_url += "?%s" % query_string
47 return starting_url
48
49
50=== modified file 'lib/lp/services/webapp/tests/test_login.py'
51--- lib/lp/services/webapp/tests/test_login.py 2012-01-20 06:29:10 +0000
52+++ lib/lp/services/webapp/tests/test_login.py 2012-08-06 13:07:21 +0000
53@@ -28,7 +28,10 @@
54 FAILURE,
55 SUCCESS,
56 )
57-from openid.extensions import sreg
58+from openid.extensions import (
59+ pape,
60+ sreg,
61+ )
62 from openid.yadis.discover import DiscoveryFailure
63 from testtools.matchers import Contains
64 from zope.component import getUtility
65@@ -731,6 +734,22 @@
66 self.assertEquals(sorted(sreg_extension.required),
67 sorted(sreg_extension.allRequestedFields()))
68
69+ def test_pape_extension_added_with_reauth_query(self):
70+ # We can signal that a request should be reauthenticated via
71+ # a reauth URL parameter, which should add PAPE extension's
72+ # max_auth_age paramter.
73+ request = LaunchpadTestRequest(QUERY_STRING='reauth=1')
74+ # This is a hack to make the request.getURL(1) call issued by the view
75+ # not raise an IndexError.
76+ request._app_names = ['foo']
77+ view = StubbedOpenIDLogin(object(), request)
78+ view()
79+ extensions = view.openid_request.extensions
80+ self.assertIsNot(None, extensions)
81+ pape_extension = extensions[1]
82+ self.assertIsInstance(pape_extension, pape.Request)
83+ self.assertEqual(0, pape_extension.max_auth_age)
84+
85 def test_logs_to_timeline(self):
86 # Beginning an OpenID association makes an HTTP request to the
87 # OP, so it's a potentially long action. It is logged to the
88
89=== modified file 'versions.cfg'
90--- versions.cfg 2012-08-06 09:29:34 +0000
91+++ versions.cfg 2012-08-06 13:07:21 +0000
92@@ -100,9 +100,7 @@
93 # python setup.py egg_info -bDEV-r`bzr revno` sdist
94 # mv dist/python-memcached-1.49DEV-r57.tar.gz [LOCATION]/download-cache/dist
95 python-memcached = 1.49DEV-r57
96-# 2.2.1 with the one-liner Expect: 100-continue fix from
97-# lp:~wgrant/python-openid/python-openid-2.2.1-fix676372.
98-python-openid = 2.2.1-fix676372
99+python-openid = 2.2.5
100 python-subunit = 0.0.8beta
101 pytz = 2012c
102 rdflib = 3.1.0