Merge lp:~gary/launchpad/loggerheadlogout into lp:launchpad
- loggerheadlogout
- Merge into devel
Proposed by
Gary Poster
Status: | Merged |
---|---|
Approved by: | Gary Poster |
Approved revision: | no longer in the source branch. |
Merged at revision: | 10855 |
Proposed branch: | lp:~gary/launchpad/loggerheadlogout |
Merge into: | lp:launchpad |
Diff against target: |
408 lines (+258/-13) 8 files modified
buildout-templates/bin/test.in (+1/-1) lib/canonical/launchpad/tests/test_login.py (+60/-2) lib/canonical/launchpad/webapp/login.py (+5/-4) lib/canonical/launchpad/webapp/tests/no-anonymous-session-cookies.txt (+14/-3) lib/launchpad_loggerhead/app.py (+15/-1) lib/launchpad_loggerhead/session.py (+9/-2) lib/launchpad_loggerhead/tests.py (+146/-0) lib/lp/testopenid/browser/server.py (+8/-0) |
To merge this branch: | bzr merge lp:~gary/launchpad/loggerheadlogout |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Paul Hummer (community) | Approve | ||
Review via email: mp+25108@code.launchpad.net |
Commit message
log out from bzr and openid after logging out from Launchpad.
Description of the change
This is a merge of changes from a branch that's been applied to production. Please see the previous MP for details:
https:/
Thank you
Gary
To post a comment you must log in.
Revision history for this message
Paul Hummer (rockstar) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'buildout-templates/bin/test.in' |
2 | --- buildout-templates/bin/test.in 2010-04-26 16:00:31 +0000 |
3 | +++ buildout-templates/bin/test.in 2010-05-11 22:19:24 +0000 |
4 | @@ -162,7 +162,7 @@ |
5 | # Find tests in the tests and ftests directories |
6 | 'tests_pattern': '^f?tests$', |
7 | 'test_path': [${buildout:directory/lib|path-repr}], |
8 | - 'package': ['canonical', 'lp', 'devscripts'], |
9 | + 'package': ['canonical', 'lp', 'devscripts', 'launchpad_loggerhead'], |
10 | 'layer': ['!(MailmanLayer)'], |
11 | } |
12 | |
13 | |
14 | === modified file 'lib/canonical/launchpad/tests/test_login.py' |
15 | --- lib/canonical/launchpad/tests/test_login.py 2010-03-30 20:02:53 +0000 |
16 | +++ lib/canonical/launchpad/tests/test_login.py 2010-05-11 22:19:24 +0000 |
17 | @@ -1,9 +1,11 @@ |
18 | -# Copyright 2009 Canonical Ltd. This software is licensed under the |
19 | +# Copyright 2009-2010 Canonical Ltd. This software is licensed under the |
20 | # GNU Affero General Public License version 3 (see the file LICENSE). |
21 | |
22 | +import cgi |
23 | from datetime import datetime |
24 | import unittest |
25 | |
26 | +import lazr.uri |
27 | from zope.component import getUtility |
28 | from zope.event import notify |
29 | from zope.session.interfaces import ISession |
30 | @@ -17,7 +19,8 @@ |
31 | from canonical.launchpad.webapp.authentication import LaunchpadPrincipal |
32 | from canonical.launchpad.webapp.interfaces import ( |
33 | CookieAuthLoggedInEvent, ILaunchpadPrincipal, IPlacelessAuthUtility) |
34 | -from canonical.launchpad.webapp.login import logInPrincipal, logoutPerson |
35 | +from canonical.launchpad.webapp.login import ( |
36 | + CookieLogoutPage, logInPrincipal, logoutPerson) |
37 | from canonical.launchpad.webapp.servers import LaunchpadTestRequest |
38 | from canonical.testing import DatabaseFunctionalLayer |
39 | |
40 | @@ -71,6 +74,61 @@ |
41 | self.request) |
42 | self.failUnless(principal is None) |
43 | |
44 | + def test_CookieLogoutPage(self): |
45 | + # This test shows that the CookieLogoutPage redirects as we expect: |
46 | + # first to loggerhead for it to log out (see bug 574493) and then |
47 | + # to our OpenId provider for it to log out (see bug 568106). This |
48 | + # will need to be readdressed when we want to accept other OpenId |
49 | + # providers, unfortunately. |
50 | + |
51 | + # This is to setup an interaction so that we can call logInPrincipal |
52 | + # below. |
53 | + login('foo.bar@example.com') |
54 | + |
55 | + logInPrincipal(self.request, self.principal, 'foo.bar@example.com') |
56 | + |
57 | + # Normally CookieLogoutPage is magically mixed in with a base class |
58 | + # that accepts context and request and sets up other things. We're |
59 | + # just going to put the request on the base class ourselves for this |
60 | + # test. |
61 | + |
62 | + view = CookieLogoutPage() |
63 | + view.request = self.request |
64 | + |
65 | + # We need to set the session cookie so it can be expired. |
66 | + self.request.response.setCookie( |
67 | + config.launchpad_session.cookie, 'xxx') |
68 | + |
69 | + # Now we logout. |
70 | + |
71 | + result = view.logout() |
72 | + |
73 | + # We should, in fact, be logged out (this calls logoutPerson). |
74 | + |
75 | + principal = getUtility(IPlacelessAuthUtility).authenticate( |
76 | + self.request) |
77 | + self.failUnless(principal is None) |
78 | + |
79 | + # The view should have redirected us, with no actual response body. |
80 | + |
81 | + self.assertEquals(self.request.response.getStatus(), 302) |
82 | + self.assertEquals(result, '') |
83 | + |
84 | + # We are redirecting to Loggerhead, to ask it to logout. |
85 | + |
86 | + location = lazr.uri.URI(self.request.response.getHeader('location')) |
87 | + self.assertEquals(location.host, 'bazaar.launchpad.dev') |
88 | + self.assertEquals(location.scheme, 'https') |
89 | + self.assertEquals(location.path, '/+logout') |
90 | + |
91 | + # That page should then redirect to our OpenId provider to logout, |
92 | + # which we provide in our query string. See |
93 | + # launchpad_loggerhead.tests.TestLogout for the pertinent tests. |
94 | + |
95 | + query = cgi.parse_qs(location.query) |
96 | + self.assertEquals( |
97 | + query['next_to'][0], 'http://testopenid.dev/+logout') |
98 | + |
99 | def test_logging_in_and_logging_out_the_old_way(self): |
100 | # A test showing that we can authenticate a request that had the |
101 | # person/account ID stored in the 'personid' session variable instead |
102 | |
103 | === modified file 'lib/canonical/launchpad/webapp/login.py' |
104 | --- lib/canonical/launchpad/webapp/login.py 2010-04-27 18:48:31 +0000 |
105 | +++ lib/canonical/launchpad/webapp/login.py 2010-05-11 22:19:24 +0000 |
106 | @@ -502,10 +502,11 @@ |
107 | |
108 | def logout(self): |
109 | logoutPerson(self.request) |
110 | - self.request.response.addNoticeNotification( |
111 | - _(u'You have been logged out') |
112 | - ) |
113 | - target = '%s/?loggingout=1' % self.request.URL[-1] |
114 | + openid_vhost = config.launchpad.openid_provider_vhost |
115 | + openid_root = allvhosts.configs[openid_vhost].rooturl |
116 | + target = '%s+logout?%s' % ( |
117 | + config.codehosting.secure_codebrowse_root, |
118 | + urllib.urlencode(dict(next_to='%s+logout' % (openid_root,)))) |
119 | self.request.response.redirect(target) |
120 | return '' |
121 | |
122 | |
123 | === modified file 'lib/canonical/launchpad/webapp/tests/no-anonymous-session-cookies.txt' |
124 | --- lib/canonical/launchpad/webapp/tests/no-anonymous-session-cookies.txt 2010-02-25 10:50:31 +0000 |
125 | +++ lib/canonical/launchpad/webapp/tests/no-anonymous-session-cookies.txt 2010-05-11 22:19:24 +0000 |
126 | @@ -1,5 +1,5 @@ |
127 | We will verify that we do not put session cookies in anonymous requests. This |
128 | -is important for cacheing anonymous requests in front of Zope, such as with |
129 | +is important for caching anonymous requests in front of Zope, such as with |
130 | Squid. Note that we are checking whether the browser has a session cookie |
131 | set, not whether the server has sent a "set-cookie" header. |
132 | |
133 | @@ -49,8 +49,19 @@ |
134 | minute time interval (set in canonical.launchpad.webapp.login and enforced |
135 | with an assert in canonical.launchpad.webapp.session) is intended to be fudge |
136 | time for browsers with bad system clocks. |
137 | - |
138 | - >>> browser.getControl('Log Out').click() |
139 | + >>> # XXX 2010-05-08 bac bug=577596 This work-around for the fact |
140 | + >>> # that loggerhead is not running needs to be replaced with |
141 | + >>> # something more robust and clear. |
142 | + >>> from urllib2 import HTTPError, URLError |
143 | + >>> try: |
144 | + ... browser.getControl('Log Out').click() |
145 | + ... except (HTTPError, URLError): |
146 | + ... pass |
147 | + |
148 | +After ensuring the browser has not left the launchpad.dev domain, the |
149 | +single cookie is shown to have the ten minute expiration. |
150 | + |
151 | + >>> browser.open('http://launchpad.dev:8085') |
152 | >>> len(browser.cookies) |
153 | 1 |
154 | >>> expires = browser.cookies.getinfo('launchpad_tests')['expires'] |
155 | |
156 | === modified file 'lib/launchpad_loggerhead/app.py' |
157 | --- lib/launchpad_loggerhead/app.py 2010-05-04 23:42:28 +0000 |
158 | +++ lib/launchpad_loggerhead/app.py 2010-05-11 22:19:24 +0000 |
159 | @@ -123,7 +123,7 @@ |
160 | elif response.status == CANCEL: |
161 | self.log.error('open id response: CANCEL') |
162 | exc = HTTPUnauthorized() |
163 | - exc.explanation = "Authetication cancelled." |
164 | + exc.explanation = "Authentication cancelled." |
165 | raise exc |
166 | else: |
167 | self.log.error('open id response: UNKNOWN') |
168 | @@ -131,6 +131,18 @@ |
169 | exc.explanation = "Unknown OpenID response." |
170 | raise exc |
171 | |
172 | + def _logout(self, environ, start_response): |
173 | + """Logout of loggerhead. |
174 | + |
175 | + Clear the cookie and redirect to `next_to`. |
176 | + """ |
177 | + environ[self.session_var].clear() |
178 | + query = dict(parse_querystring(environ)) |
179 | + next_url = query.get('next_to') |
180 | + if next_url is None: |
181 | + next_url = allvhosts.configs['mainsite'].rooturl |
182 | + raise HTTPMovedPermanently(next_url) |
183 | + |
184 | def __call__(self, environ, start_response): |
185 | environ['loggerhead.static.url'] = environ['SCRIPT_NAME'] |
186 | if environ['PATH_INFO'].startswith('/static/'): |
187 | @@ -142,6 +154,8 @@ |
188 | return robots_app(environ, start_response) |
189 | elif environ['PATH_INFO'].startswith('/+login'): |
190 | return self._complete_login(environ, start_response) |
191 | + elif environ['PATH_INFO'].startswith('/+logout'): |
192 | + return self._logout(environ, start_response) |
193 | path = environ['PATH_INFO'] |
194 | trailingSlashCount = len(path) - len(path.rstrip('/')) |
195 | user = environ[self.session_var].get('user', LAUNCHPAD_ANONYMOUS) |
196 | |
197 | === modified file 'lib/launchpad_loggerhead/session.py' |
198 | --- lib/launchpad_loggerhead/session.py 2010-04-27 01:35:56 +0000 |
199 | +++ lib/launchpad_loggerhead/session.py 2010-05-11 22:19:24 +0000 |
200 | @@ -1,4 +1,4 @@ |
201 | -# Copyright 2009 Canonical Ltd. This software is licensed under the |
202 | +# Copyright 2009-2010 Canonical Ltd. This software is licensed under the |
203 | # GNU Affero General Public License version 3 (see the file LICENSE). |
204 | |
205 | """Simple paste-y session manager tuned for the needs of launchpad-loggerhead. |
206 | @@ -64,10 +64,17 @@ |
207 | session = pickle.loads(environ[self.session_var]) |
208 | else: |
209 | session = {} |
210 | + existed = bool(session) |
211 | environ[self.session_var] = session |
212 | def response_hook(status, response_headers, exc_info=None): |
213 | session = environ.pop(self.session_var) |
214 | - if session: |
215 | + # paste.auth.cookie does not delete cookies (see |
216 | + # http://trac.pythonpaste.org/pythonpaste/ticket/139). A |
217 | + # reasonable workaround is to make the value empty. Therefore, |
218 | + # we explicitly set the value in the session (to be encrypted) |
219 | + # if the value is non-empty *or* if it was non-empty at the start |
220 | + # of the request. |
221 | + if existed or session: |
222 | environ[self.session_var] = pickle.dumps(session) |
223 | return start_response(status, response_headers, exc_info) |
224 | return self.application(environ, response_hook) |
225 | |
226 | === added file 'lib/launchpad_loggerhead/tests.py' |
227 | --- lib/launchpad_loggerhead/tests.py 1970-01-01 00:00:00 +0000 |
228 | +++ lib/launchpad_loggerhead/tests.py 2010-05-11 22:19:24 +0000 |
229 | @@ -0,0 +1,146 @@ |
230 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
231 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
232 | + |
233 | +import unittest |
234 | +import urllib |
235 | + |
236 | +import lazr.uri |
237 | +import wsgi_intercept |
238 | +from wsgi_intercept.urllib2_intercept import install_opener, uninstall_opener |
239 | +import wsgi_intercept.zope_testbrowser |
240 | +from paste.httpexceptions import HTTPExceptionHandler |
241 | + |
242 | +from canonical.config import config |
243 | +from canonical.launchpad.webapp.vhosts import allvhosts |
244 | +from canonical.testing import DatabaseFunctionalLayer |
245 | +from launchpad_loggerhead.app import RootApp |
246 | +from launchpad_loggerhead.session import SessionHandler |
247 | +from lp.testing import TestCase |
248 | + |
249 | +SESSION_VAR = 'lh.session' |
250 | + |
251 | +# See sourcecode/launchpad-loggerhead/start-loggerhead.py for the production |
252 | +# mechanism for getting the secret. |
253 | +SECRET = 'secret' |
254 | + |
255 | + |
256 | +def session_scribbler(app, test): |
257 | + """Squirrel away the session variable.""" |
258 | + def scribble(environ, start_response): |
259 | + test.session = environ[SESSION_VAR] # Yay for mutables. |
260 | + return app(environ, start_response) |
261 | + return scribble |
262 | + |
263 | + |
264 | +def dummy_destination(environ, start_response): |
265 | + """Return a fake response.""" |
266 | + start_response('200 OK', [('Content-type','text/plain')]) |
267 | + return ['This is a dummy destination.\n'] |
268 | + |
269 | + |
270 | +class SimpleLogInRootApp(RootApp): |
271 | + """A mock root app that doesn't require open id.""" |
272 | + def _complete_login(self, environ, start_response): |
273 | + environ[SESSION_VAR]['user'] = 'bob' |
274 | + start_response('200 OK', [('Content-type','text/plain')]) |
275 | + return ['\n'] |
276 | + |
277 | + |
278 | +class TestLogout(TestCase): |
279 | + layer = DatabaseFunctionalLayer |
280 | + |
281 | + def intercept(self, uri, app): |
282 | + """Install wsgi interceptors for the uri, app tuple.""" |
283 | + if isinstance(uri, basestring): |
284 | + uri = lazr.uri.URI(uri) |
285 | + port = uri.port |
286 | + if port is None: |
287 | + if uri.scheme == 'http': |
288 | + port = 80 |
289 | + elif uri.scheme == 'https': |
290 | + port = 443 |
291 | + else: |
292 | + raise NotImplementedError(uri.scheme) |
293 | + else: |
294 | + port = int(port) |
295 | + wsgi_intercept.add_wsgi_intercept(uri.host, port, lambda: app) |
296 | + self.intercepted.append((uri.host, port)) |
297 | + |
298 | + def setUp(self): |
299 | + TestCase.setUp(self) |
300 | + self.intercepted = [] |
301 | + self.session = None |
302 | + self.root = app = SimpleLogInRootApp(SESSION_VAR) |
303 | + app = session_scribbler(app, self) |
304 | + app = HTTPExceptionHandler(app) |
305 | + app = SessionHandler(app, SESSION_VAR, SECRET) |
306 | + self.cookie_name = app.cookie_handler.cookie_name |
307 | + self.intercept(config.codehosting.codebrowse_root, app) |
308 | + self.intercept(config.codehosting.secure_codebrowse_root, app) |
309 | + self.intercept(allvhosts.configs['mainsite'].rooturl, |
310 | + dummy_destination) |
311 | + install_opener() |
312 | + self.browser = wsgi_intercept.zope_testbrowser.WSGI_Browser() |
313 | + # We want to pretend we are not a robot, or else mechanize will honor |
314 | + # robots.txt. |
315 | + self.browser.mech_browser.set_handle_robots(False) |
316 | + self.browser.open( |
317 | + config.codehosting.secure_codebrowse_root + '+login') |
318 | + |
319 | + def tearDown(self): |
320 | + uninstall_opener() |
321 | + for host, port in self.intercepted: |
322 | + wsgi_intercept.remove_wsgi_intercept(host, port) |
323 | + TestCase.tearDown(self) |
324 | + |
325 | + def testLoggerheadLogout(self): |
326 | + # We start logged in as 'bob'. |
327 | + self.assertEqual(self.session['user'], 'bob') |
328 | + self.browser.open( |
329 | + config.codehosting.secure_codebrowse_root + 'favicon.ico') |
330 | + self.assertEqual(self.session['user'], 'bob') |
331 | + self.failUnless(self.browser.cookies.get(self.cookie_name)) |
332 | + |
333 | + # When we visit +logout, our session is gone. |
334 | + self.browser.open( |
335 | + config.codehosting.secure_codebrowse_root + '+logout') |
336 | + self.assertEqual(self.session, {}) |
337 | + |
338 | + # By default, we have been redirected to the Launchpad root. |
339 | + self.assertEqual( |
340 | + self.browser.url, allvhosts.configs['mainsite'].rooturl) |
341 | + |
342 | + # The session cookie still exists, because of how |
343 | + # paste.auth.cookie works (see |
344 | + # http://trac.pythonpaste.org/pythonpaste/ticket/139 ) but the user |
345 | + # does in fact have an empty session now. |
346 | + self.browser.open( |
347 | + config.codehosting.secure_codebrowse_root + 'favicon.ico') |
348 | + self.assertEqual(self.session, {}) |
349 | + |
350 | + def testLoggerheadLogoutRedirect(self): |
351 | + # When we visit +logout with a 'next_to' value in the query string, |
352 | + # the logout page will redirect to the given URI. As of this |
353 | + # writing, this is used by Launchpad to redirect to our OpenId |
354 | + # provider (see canonical.launchpad.tests.test_login. |
355 | + # TestLoginAndLogout.test_CookieLogoutPage). |
356 | + |
357 | + # Here, we will have a more useless example of the basic machinery. |
358 | + dummy_root = 'http://dummy.dev/' |
359 | + self.intercept(dummy_root, dummy_destination) |
360 | + self.browser.open( |
361 | + config.codehosting.secure_codebrowse_root + |
362 | + '+logout?' + |
363 | + urllib.urlencode(dict(next_to=dummy_root + '+logout'))) |
364 | + |
365 | + # We are logged out, as before. |
366 | + self.assertEqual(self.session, {}) |
367 | + |
368 | + # Now, though, we are redirected to the ``next_to`` destination. |
369 | + self.assertEqual(self.browser.url, dummy_root + '+logout') |
370 | + self.assertEqual(self.browser.contents, |
371 | + 'This is a dummy destination.\n') |
372 | + |
373 | + |
374 | +def test_suite(): |
375 | + return unittest.TestLoader().loadTestsFromName(__name__) |
376 | |
377 | === modified file 'lib/lp/testopenid/browser/server.py' |
378 | --- lib/lp/testopenid/browser/server.py 2010-03-31 19:38:32 +0000 |
379 | +++ lib/lp/testopenid/browser/server.py 2010-05-11 22:19:24 +0000 |
380 | @@ -24,6 +24,7 @@ |
381 | from openid import oidutil |
382 | from openid.server.server import CheckIDRequest, Server |
383 | from openid.store.memstore import MemoryStore |
384 | +from openid.extensions.sreg import SRegRequest, SRegResponse |
385 | |
386 | from canonical.cachedproperty import cachedproperty |
387 | from canonical.launchpad import _ |
388 | @@ -41,6 +42,7 @@ |
389 | from lp.testopenid.interfaces.server import ( |
390 | get_server_url, ITestOpenIDApplication, ITestOpenIDLoginForm, |
391 | ITestOpenIDPersistentIdentity) |
392 | +from lp.registry.interfaces.person import IPerson |
393 | |
394 | |
395 | OPENID_REQUEST_SESSION_KEY = 'testopenid.request' |
396 | @@ -196,6 +198,12 @@ |
397 | else: |
398 | response = self.openid_request.answer(True) |
399 | |
400 | + sreg_fields = dict(nickname=IPerson(self.account).name) |
401 | + sreg_request = SRegRequest.fromOpenIDRequest(self.openid_request) |
402 | + sreg_response = SRegResponse.extractResponse( |
403 | + sreg_request, sreg_fields) |
404 | + response.addExtension(sreg_response) |
405 | + |
406 | return response |
407 | |
408 | def createFailedResponse(self): |