Merge lp:~salgado/launchpad/openid-consumer into lp:launchpad

[Guilherme Salgado]

[Guilherme Salgado (salgado)]

This branch changes the +login page to use OpenID to authenticate, also adding a callback view for OpenID providers to send the users to, when the authentication is completed.

Some existing tests were changed to use AppServerLayer so that they can do
OpenID login, so they now use launchpad.dev:8085

There's a bug fix in baseopenidstore.py, with an ammended test in
test_baseopenidstore.py

Some tests relied on the +login page but they didn't really need to, so
instead of changing them to use the AppServerLayer (which is a lot more
expensive), I've changed them to use HTTP basic auth.

lib/canonical/launchpad/testing/cookie.py is no longer needed as
testbrowser.Browser now provides a .cookies

BasicLoginPage is now enabled for development and the test runner (including
the AppServerLayer, which uses a different config) so that Windmill tests can
use it to login -- using OpenID on windmill doesn't work because it can't
cleanly switch between domains (e.g. launchpad.dev <-> testopenid.dev).

The existing +login page was renamed to +login-old and it'll be removed
(already is, in fact) in a later branch.

[Gary Poster (gary)]

merge-conditional

Thank you for this excellent branch. I can tell it was a lot of hard work--the mechanize testing code alone must have been "fun" to come up with.

Thanks also for the many clean-ups in this branch, ranging from the removal of the now-unnecessary cookie test code to the salt fix.

I had several comments that we talked about on IRC. Here is a summary of what we discussed.

- You removed the test of "locationless pages" and the fact that they do not offer to redirect. I wondered if the +login page were in fact the only locationless page. You reported that you had done a search, and I did a spot check (+graphics) and concluded that it was fine.

- I had a concern about the +basiclogin login registration: it is hidden. If I were to try to find out where +basiclogin appeared so I could change something about it then I would look in zcml, and not find it. We have no other "do this arbitrary thing now" kind of mechanism. Moreover, it is handled by a package import which is never lovely. I suggested that you make this registration an event handler for the server starting (which Zope broadcasts). The subscription could be registered in zcml with a zcml comment duplicating what the standard registration would be. You agreed and showed me http://paste.ubuntu.com/378472/ .

- Typo: s/where/were/ within "# once that's done they must be sent back to the URL they where when"

- Move imports of transaction and removeSecurityProxy out of functions/methods to top of file (e.g., see OpenIDLogin.render and other code in the same file).

- Typo: s/\(will/will/ within "# Commit because the consumer.complete() call above (will create"

- I asked why we had an explicit transaction commit in OpenIDCallbackView.render. You explained that it was "because the requests will use GET, so the transaction gets rolled back at the end of the request." We agreed that it would be a good idea to make that clear in the comment.

- I noted that I don't like properties that write when you get them, so I don't like OpenIDCallbackView.openid_response as a property. I'd prefer a method that has a name that gives more of a hint that there might be a database write--or, perhaps simpler, you could calculate the response in the __init__ and actually put it on the view class as a normal attribute. Then you could add a comment that this writes to the database in the call in the __init__, the code is even smaller, and it is clear that you will need that value for all code paths (which you do afaict). You agreed, which I think means that you were going to pursue the second option.

- In in no-anonymous-session-cookies.txt, I suggested changing """We want to check whether the browser has a session cookie set, not whether the server has sent a "set-cookie" header, so we have to dig into the underlying mechanize browser (which is done transparently by browser.cookies, though).""" to """Note that we are checking whether the browser has a session cookie set, not whether the server has sent a "set-cookie" header.""" You agreed.

- I noted that I saw four instances of the same test code to log in (e.g., ``browser.getControl(name='field.email').value = ...``). We a...

review: Approve (code)

=== modified file 'BRANCH.TODO'
--- BRANCH.TODO	2010-02-18 10:55:29 +0000
+++ BRANCH.TODO	2010-02-18 10:55:33 +0000
@@ -3,14 +3,20 @@
 # stuff still here when you are ready to land, the items should probably
 # be converted to bugs so they can be scheduled.
 
-* Do we want to change c-i-p to use a separate cookie so that logging into
-login.lp.net doesn't cause you to end up logged into lp.net or should we just
-wait for ISD to roll a rebranded version of login.u.c on login.lp.net, which
-will probably use separate cookies.
- - Apart from not being trivial to change the cookie name for a given vhost
-   we also need to worry about making sure the new cookie (which is supposed
-   to be valid only for login.lp.net) is not valid for all other vhosts, which
-   would normally happen thanks to LaunchpadCookieClientIdManager.
+1. Email Matt describing what I, as a LP user, would need to know about the
+change to use OpenID.
+2. Ping Tom when the config change to make the login servers use a different
+cookie is ready to land.
+
+* Need a migration plan, for existing NEWACCOUNT/RESETPASSWORD login
+tokens that end up used only after we roll out.
+   * This is specially important if we take into account that we plan to
+     roll this out together with the main/auth DBs split, meaning it won't
+     be possible for people to create new accounts and/or reset passwords in
+     Launchpad itself.
+
+* The LoginToken pages for creating new accounts and reset passwords can be
+removed.
 
 * We still rely heavily on the Account table and expect all users to have an
 Account. We need to fix that if we're going to copy the Account table into the
@@ -18,40 +24,4 @@
 that, though, the Account table won't be necessary so there'll be no point in
 copying it.  *I'm not sure this is feasible.*
 
-* As discussed with Francis, it's not worth porting the team-restricted login
-stuff to the new system, so we'll drop it.
-
-* Functionality needed in the callback view:
- - reactivate accounts when credentials belong to a deactivated profile
- - create Person entries when the credentials don't exist in our db
- - forbid to log suspended accounts in. btw, I guess we're going to copy the
-   AccountStatus table to the main db.
-
-    # To test all this I'll just have to monkey patch
-    # OpenIDCallbackView.openid_response to return my hand-crafted
-    # openid response.
-
-* Some tests use anon_browser.open() on protected pages so that they get
-reidrected to +login and can show how the protected page works when the user
-is not logged in, including the preserved query string. If we want to keep
-doing that in these tests, we'll need an OpenID provider accessible to the
-test suite. One option would be to include a crippled version of the one in
-c-i-p in our tree, to be used only in tests.
-(lib/canonical/launchpad/pagetests/oauth/authorize-token.txt is one of the
-tests that rely on +login)
-
-* Does not work for /people/+me, because when the OpenID provider sends the
-user back to /people/+me/+openid-callback, there's a redirect to
-/~person/+openid-callback, which causes the openid dance to fail: *
-<openid.consumer.consumer.FailureResponse id=None message="return_to does not
-match return URL. Expected 'https://launchpad.dev/%7Ename16/+openid-callback',
-got
-u'https://launchpad.dev/people/+me/+openid-callback?janrain_nonce=2009-12-23T14%3A47%3A47ZsgdbOJ'">
-  I think this is a general problem with OpenID and login.lp.net because when
-you log into login.lp.net and is sent to the callback page (in lp.net), that
-page will see you as logged in, regardless of the openid response, because the
-cookie is shared, and that causes /people/+me/+openid-callback to redirect to
-/~name16/+openid-callback, which causes the error above.
-  One way around this is to always use /+openid-callback as the return_to URL,
-and include a lp_redirect_to URL in the query string, where LP will send the
-user to, once the openid dance is completed.
+* We will need to copy the status from Account back to Person.

=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf	2010-02-18 10:55:29 +0000
+++ configs/development/launchpad-lazr.conf	2010-02-18 10:55:33 +0000
@@ -125,6 +125,7 @@
 
 [launchpad]
 enable_test_openid_provider: True
+openid_provider_vhost: testopenid
 code_domain: code.launchpad.dev
 default_batch_size: 5
 max_attachment_size: 2097152

=== modified file 'configs/testrunner-appserver/launchpad-lazr.conf'
--- configs/testrunner-appserver/launchpad-lazr.conf	2009-04-29 19:10:17 +0000
+++ configs/testrunner-appserver/launchpad-lazr.conf	2010-02-18 10:55:33 +0000
@@ -43,6 +43,9 @@
 [vhost.openid]
 rooturl: http://openid.launchpad.dev:8085/
 
+[vhost.testopenid]
+rooturl: http://testopenid.dev:8085/
+
 [vhost.shipitubuntu]
 rooturl: http://shipit.ubuntu.dev:8085/
 

=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf	2010-02-18 10:55:29 +0000
+++ lib/canonical/config/schema-lazr.conf	2010-02-18 10:55:33 +0000
@@ -898,6 +898,10 @@
 # datatype: boolean
 launch: True
 
+# The vhost used as the OpenID provider to log into Launchpad.
+# datatype: string
+openid_provider_vhost: openid
+
 # If true, the main template will be styled so that it is
 # obvious to the end user that they are using a demo system
 # and that any changes they make will be lost at some point.

=== modified file 'lib/canonical/launchpad/database/baseopenidstore.py'
--- lib/canonical/launchpad/database/baseopenidstore.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/database/baseopenidstore.py	2010-02-18 10:55:33 +0000
@@ -130,7 +130,7 @@
             return False
 
         server_url = server_url.decode('UTF-8')
-        salt = server_url.decode('ASCII')
+        salt = salt.decode('ASCII')
 
         store = IMasterStore(self.Nonce)
         old_nonce = store.get(self.Nonce, (server_url, timestamp, salt))

=== modified file 'lib/canonical/launchpad/database/tests/test_baseopenidstore.py'
--- lib/canonical/launchpad/database/tests/test_baseopenidstore.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/database/tests/test_baseopenidstore.py	2010-02-18 10:55:33 +0000
@@ -115,6 +115,11 @@
         # The nonce can only be used once.
         self.assertEqual(
             self.store.useNonce('server-url', timestamp, 'salt'), True)
+        storm_store = IMasterStore(self.store.Nonce)
+        nonce = storm_store.get(
+            self.store.Nonce, (u'server-url', timestamp, u'salt'))
+        self.assertIsNot(None, nonce)
+
         self.assertEqual(
             self.store.useNonce('server-url', timestamp, 'salt'), False)
         self.assertEqual(

=== modified file 'lib/canonical/launchpad/doc/webapp-publication.txt'
--- lib/canonical/launchpad/doc/webapp-publication.txt	2009-10-23 16:17:40 +0000
+++ lib/canonical/launchpad/doc/webapp-publication.txt	2010-02-18 10:55:33 +0000
@@ -77,6 +77,10 @@
     rooturl: http://shipit.ubuntu.dev/
     althosts:
     ----
+    testopenid @ testopenid.dev
+    rooturl: http://testopenid.dev/
+    althosts:
+    ----
     translations @ translations.launchpad.dev
     rooturl: http://translations.launchpad.dev/
     althosts:
@@ -112,6 +116,7 @@
     shipit.edubuntu.dev
     shipit.kubuntu.dev
     shipit.ubuntu.dev
+    testopenid.dev
     translations.launchpad.dev
     ubuntu-openid.launchpad.dev
     xmlrpc-private.launchpad.dev

=== modified file 'lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt'
--- lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt	2009-09-18 23:48:15 +0000
+++ lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt	2010-02-18 10:55:33 +0000
@@ -103,15 +103,6 @@
     ...     'a', onclick="setBetaRedirect(false)"))
     Disable edge redirect.
 
-The disable-redirect link will not appear for locationless pages, such
-as the login page, or any other view that does not inherit from
-LaunchpadView and therefore does not provide isBetaUser().
-
-    >>> beta_browser.open('http://launchpad.dev/+login')
-    >>> print extract_text(find_tags_by_class(
-    ...     beta_browser.contents, 'sitemessage')[0])
-    This is a beta site.
-
 The disable-redirect link will not appear in the site_message when
 browsed by non-beta users.
 

=== modified file 'lib/canonical/launchpad/pagetests/oauth/authorize-token.txt'
--- lib/canonical/launchpad/pagetests/oauth/authorize-token.txt	2009-09-09 20:08:36 +0000
+++ lib/canonical/launchpad/pagetests/oauth/authorize-token.txt	2010-02-18 10:55:33 +0000
@@ -19,29 +19,21 @@
 The oauth_token parameter, on the other hand, is required in the
 Launchpad implementation.
 
-The +authorize-token page is restricted to logged in users, so users
-will first be asked to log in.
+The +authorize-token page is restricted to logged in users, so users will
+first be asked to log in. (We won't show the actual login process because
+it involves OpenID, which would complicate this test quite a bit.)
 
-    # Set handleErrors to True so that the Unauthorized exception is handled
-    # by the publisher and we get redirected to the +login page.
-    >>> browser.handleErrors = True
     >>> from urllib import urlencode
     >>> params = dict(
     ...     oauth_token=token.key, oauth_callback='http://launchpad.dev/bzr')
-    >>> browser.open(
-    ...     "http://launchpad.dev/+authorize-token?%s" % urlencode(params))
-    >>> browser.url
-    'http://.../+authorize-token/+login?oauth_token=...&oauth_callback=...'
-
-And then the user is redirected back to the +authorize-token page.
-Note how the query parameters are kept.
-
-    >>> browser.getControl('E-mail address:', index=0).value = (
-    ...     '[e-mail]')
-    >>> browser.getControl('Password:').value = 'test'
-    >>> browser.getControl(name='loginpage_submit_login').click()
-    >>> browser.url
-    'http://.../+authorize-token?oauth_token=...&oauth_callback=...'
+    >>> url = "http://launchpad.dev/+authorize-token?%s" % urlencode(params)
+    >>> browser.open(url)
+    Traceback (most recent call last):
+    ...
+    Unauthorized:...
+
+    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> browser.open(url)
     >>> browser.title
     'Authorize application to access Launchpad on your behalf'
 

=== added file 'lib/canonical/launchpad/templates/login-already.pt'
--- lib/canonical/launchpad/templates/login-already.pt	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/templates/login-already.pt	2010-02-18 10:55:33 +0000
@@ -0,0 +1,18 @@
+<tal:root
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+  omit-tag="">
+<html metal:use-macro="view/macro:page/locationless">
+  <body>
+    <div class="top-portlet" metal:fill-slot="main">
+      <h1>You are already logged in</h1>
+      <p>
+        You are already logged in as
+        <strong tal:content="request/lp:person/displayname">name</strong>.
+        If this is not you, please <a href="/+logout">log out now</a>.
+      </p>
+    </div>
+  </body>
+</html>
+</tal:root>

=== added file 'lib/canonical/launchpad/templates/login-error.pt'
--- lib/canonical/launchpad/templates/login-error.pt	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/templates/login-error.pt	2010-02-18 10:55:33 +0000
@@ -0,0 +1,17 @@
+<html
+  xmlns="http://www.w3.org/1999/xhtml"
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+  metal:use-macro="view/macro:page/locationless"
+  i18n:domain="launchpad">
+
+  <body>
+    <div class="top-portlet" metal:fill-slot="main">
+    
+      <h1>Your login was unsuccessful</h1>
+      <p class="error" tal:content="view/login_error" />
+
+    </div>
+  </body>
+</html>

=== added file 'lib/canonical/launchpad/templates/login-suspended-account.pt'
--- lib/canonical/launchpad/templates/login-suspended-account.pt	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/templates/login-suspended-account.pt	2010-02-18 10:55:33 +0000
@@ -0,0 +1,20 @@
+<html
+  xmlns="http://www.w3.org/1999/xhtml"
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+  metal:use-macro="view/macro:page/locationless"
+  i18n:domain="launchpad">
+
+  <body>
+    <div class="top-portlet" metal:fill-slot="main">
+    
+      <h1>This account has been suspended</h1>
+      <p class="error">
+        Contact a <a href="mailto:[e-mail]?subject=SUSPENDED%20account"
+          >Launchpad admin</a> about this issue.
+      </p>
+
+    </div>
+  </body>
+</html>

=== modified file 'lib/canonical/launchpad/testing/browser.py'
--- lib/canonical/launchpad/testing/browser.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/testing/browser.py	2010-02-18 10:55:33 +0000
@@ -30,6 +30,9 @@
 
 from zope.testbrowser.browser import Browser as _Browser
 
+from canonical.launchpad.testing.pages import (
+    extract_text, find_main_content, find_tag_by_id, get_feedback_messages)
+
 
 class SocketClosingOnErrorHandler(urllib2.BaseHandler):
     """A handler that ensures that the socket gets closed on errors.
@@ -56,10 +59,10 @@
 
 
 class Browser(_Browser):
-    """A browser subsclass that knows about basic auth."""
+    """A browser subclass that knows about basic auth."""
 
-    def __init__(self, auth=None):
-        super(Browser, self).__init__()
+    def __init__(self, auth=None, mech_browser=None):
+        super(Browser, self).__init__(mech_browser=mech_browser)
         # We have to add the error handler to the mechanize browser underlying
         # the Zope browser, because it's the former that's actually doing all
         # the work.
@@ -95,6 +98,11 @@
 def setUp(test):
     """Set up appserver tests."""
     test.globs['Browser'] = Browser
+    test.globs['browser'] = Browser()
+    test.globs['find_tag_by_id'] = find_tag_by_id
+    test.globs['find_main_content'] = find_main_content
+    test.globs['get_feedback_messages'] = get_feedback_messages
+    test.globs['extract_text'] = extract_text
 
 
 def tearDown(test):

=== removed file 'lib/canonical/launchpad/testing/cookie.py'
--- lib/canonical/launchpad/testing/cookie.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/testing/cookie.py	1970-01-01 00:00:00 +0000
@@ -1,67 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Helper to analyze cookies in a zope.testbrowser browser.
-
-This API and idea has been accepted into zope.testbrowser, upstream, so this
-code will be removed from here once that change has landed and we are able to
-use it.  https://bugs.edge.launchpad.net/zope3/+bug/286842
-"""
-
-__metaclass__ = type
-__all__ = [
-    'Cookies',
-    ]
-
-from UserDict import DictMixin
-import datetime
-import pytz
-
-class Cookies(DictMixin):
-    """Cookies for testbrowser.  Currently does not implement setting.
-    """
-
-    def __init__(self, testbrowser):
-        self.testbrowser = testbrowser
-
-    @property
-    def _jar(self):
-        for handler in self.testbrowser.mech_browser.handlers:
-            if getattr(handler, 'cookiejar', None) is not None:
-                return handler.cookiejar
-        raise RuntimeError('no cookiejar found')
-
-    def _get(self, key):
-        for ck in self._jar:
-            if ck.name == key:
-                return ck
-
-    def __getitem__(self, key):
-        ck = self._get(key)
-        if ck is None:
-            raise KeyError(key)
-        return ck.value
-
-    def keys(self):
-        return [ck.name for ck in self._jar]
-
-    def __contains__(self, key):
-        return self._get(key) is not None
-
-    def getInfo(self, key):
-        ck = self._get(key)
-        if ck is None:
-            raise KeyError(key)
-        res = {'value': ck.value,
-               'port': ck.port,
-               'domain': ck.domain,
-               'path': ck.path,
-               'secure': ck.secure,
-               'expires': None}
-        if ck.expires is not None:
-            res['expires'] = datetime.datetime.fromtimestamp(
-                ck.expires, pytz.UTC)
-        return res
-
-    def __len__(self):
-        return len(self._jar)

=== modified file 'lib/canonical/launchpad/webapp/login.py'
--- lib/canonical/launchpad/webapp/login.py	2010-02-09 00:17:40 +0000
+++ lib/canonical/launchpad/webapp/login.py	2010-02-18 10:55:33 +0000
@@ -14,21 +14,31 @@
 
 from BeautifulSoup import UnicodeDammit
 
-from zope.component import getUtility
+from openid.consumer.consumer import CANCEL, Consumer, FAILURE, SUCCESS
+from openid.fetchers import setDefaultFetcher, Urllib2Fetcher
+
+import transaction
+
+from zope.app.security.interfaces import IUnauthenticatedPrincipal
+from zope.component import getUtility, getSiteManager
+from zope.event import notify
+from zope.interface import Interface
+from zope.publisher.browser import BrowserPage
+from zope.publisher.interfaces.http import IHTTPApplicationRequest
+from zope.security.proxy import removeSecurityProxy
 from zope.session.interfaces import ISession, IClientIdManager
-from zope.event import notify
-from zope.app.security.interfaces import IUnauthenticatedPrincipal
 
 from z3c.ptcompat import ViewPageTemplateFile
 
 from canonical.cachedproperty import cachedproperty
 from canonical.config import config
 from canonical.launchpad import _
-from canonical.launchpad.interfaces.account import AccountStatus
+from canonical.launchpad.interfaces.account import AccountStatus, IAccountSet
 from canonical.launchpad.interfaces.authtoken import (
     IAuthTokenSet, LoginTokenType)
 from canonical.launchpad.interfaces.emailaddress import IEmailAddressSet
 from canonical.launchpad.interfaces.logintoken import ILoginTokenSet
+from canonical.launchpad.interfaces.openidconsumer import IOpenIDConsumerStore
 from lp.registry.interfaces.person import (
     IPerson, IPersonSet, PersonCreationRationale)
 from canonical.launchpad.interfaces.validation import valid_password
@@ -36,10 +46,12 @@
 from canonical.launchpad.validators.email import valid_email
 from canonical.launchpad.webapp.error import SystemErrorView
 from canonical.launchpad.webapp.interfaces import (
-    CookieAuthLoggedInEvent, ILaunchpadPrincipal, IPlacelessAuthUtility,
-    IPlacelessLoginSource, LoggedOutEvent)
+    CookieAuthLoggedInEvent, ILaunchpadApplication, ILaunchpadPrincipal,
+    IPlacelessAuthUtility, IPlacelessLoginSource, LoggedOutEvent)
 from canonical.launchpad.webapp.metazcml import ILaunchpadPermission
+from canonical.launchpad.webapp.publisher import LaunchpadView
 from canonical.launchpad.webapp.url import urlappend
+from canonical.launchpad.webapp.vhosts import allvhosts
 
 
 class UnauthorizedView(SystemErrorView):
@@ -119,14 +131,14 @@
         return urlappend(current_url, '+login' + query_string)
 
 
-class BasicLoginPage:
+class BasicLoginPage(BrowserPage):
 
     def isSameHost(self, url):
         """Returns True if the url appears to be from the same host as we are.
         """
         return url.startswith(self.request.getApplicationURL())
 
-    def login(self):
+    def __call__(self):
         if IUnauthenticatedPrincipal.providedBy(self.request.principal):
             self.request.principal.__parent__.unauthorized(
                 self.request.principal.id, self.request)
@@ -139,6 +151,19 @@
         return ''
 
 
+def register_basiclogin(event):
+    # The +basiclogin page should only be enabled for development and tests,
+    # but we can't rely on config.devmode because it's turned off for
+    # AppServerLayer tests, so we (ab)use the config switch for the test
+    # OpenID provider, which has similar requirements.
+    if config.launchpad.enable_test_openid_provider:
+        getSiteManager().registerAdapter(
+            BasicLoginPage,
+            required=(ILaunchpadApplication, IHTTPApplicationRequest),
+            provided=Interface,
+            name='+basiclogin')
+
+
 class RestrictedLoginInfo:
     """On a team-restricted launchpad server, show who may access the server.
 
@@ -193,6 +218,160 @@
         return '%d + %d =' % (op1, op2)
 
 
+# The Python OpenID package uses pycurl by default, but pycurl chokes on
+# self-signed certificates (like the ones we use when developing), so we
+# change the default to urllib2 here.  That's also a good thing because it
+# ensures we test the same thing that we run on production.
+setDefaultFetcher(Urllib2Fetcher())
+
+
+class OpenIDLogin(LaunchpadView):
+    """A view which initiates the OpenID handshake with our provider."""
+    _openid_session_ns = 'OPENID'
+
+    def _getConsumer(self):
+        session = ISession(self.request)[self._openid_session_ns]
+        openid_store = getUtility(IOpenIDConsumerStore)
+        return Consumer(session, openid_store)
+
+    def render(self):
+        if self.account is not None:
+            return AlreadyLoggedInView(self.context, self.request)()
+
+        # Allow unauthenticated users to have sessions for the OpenID
+        # handshake to work.
+        allowUnauthenticatedSession(self.request)
+        consumer = self._getConsumer()
+        openid_vhost = config.launchpad.openid_provider_vhost
+        self.openid_request = consumer.begin(
+            allvhosts.configs[openid_vhost].rooturl)
+
+        trust_root = self.request.getApplicationURL()
+        assert not self.openid_request.shouldSendRedirect(), (
+            "Our fixed OpenID server should not need us to redirect.")
+        # Once the user authenticates with the OpenID provider they will be
+        # sent to the /+openid-callback page, where we log them in, but
+        # once that's done they must be sent back to the URL they were when
+        # they started the login process (i.e. the current URL without the
+        # '+login' bit). To do that we encode that URL as a query arg in the
+        # return_to URL passed to the OpenID Provider
+        starting_url = urllib.urlencode([('starting_url', self.starting_url)])
+        return_to = urlappend(
+            self.request.getApplicationURL(), '+openid-callback')
+        return_to = "%s?%s" % (return_to, starting_url)
+        form_html = self.openid_request.htmlMarkup(trust_root, return_to)
+
+        # The consumer.begin() call above will insert rows into the 
+        # OpenIDAssociations table, but since this will be a GET request, the
+        # transaction would be rolled back, so we need an explicit commit
+        # here.
+        transaction.commit()
+
+        return form_html
+
+    @property
+    def starting_url(self):
+        starting_url = self.request.getURL(1)
+        query_string = "&".join([arg for arg in self.form_args])
+        if query_string:
+            starting_url += "?%s" % query_string
+        return starting_url
+
+    @property
+    def form_args(self):
+        """Iterate over form args, yielding 'key=value' strings for them.
+
+        Exclude things such as 'loggingout' and starting with 'openid.', which
+        we don't want.
+        """
+        for name, value in self.request.form.items():
+            if name == 'loggingout' or name.startswith('openid.'):
+                continue
+            if isinstance(value, list):
+                value_list = value
+            else:
+                value_list = [value]
+            for value_list_item in value_list:
+                # Thanks to apport (https://launchpad.net/bugs/61171), we need
+                # to do this here.
+                value_list_item = UnicodeDammit(value_list_item).markup
+                yield "%s=%s" % (name, value_list_item)
+
+
+class OpenIDCallbackView(OpenIDLogin):
+    """The OpenID callback page for logging into Launchpad.
+
+    This is the page the OpenID provider will send the user's browser to,
+    after the user has authenticated on the provider.
+    """
+
+    suspended_account_template = ViewPageTemplateFile(
+        '../templates/login-suspended-account.pt')
+
+    def initialize(self):
+        self.openid_response = self._getConsumer().complete(
+            self.request.form, self.request.getURL())
+
+    def login(self, account):
+        loginsource = getUtility(IPlacelessLoginSource)
+        # We don't have a logged in principal, so we must remove the security
+        # proxy of the account's preferred email.
+        email = removeSecurityProxy(account.preferredemail).email
+        logInPrincipal(
+            self.request, loginsource.getPrincipalByLogin(email), email)
+
+    def render(self):
+        if self.openid_response.status == SUCCESS:
+            account = getUtility(IAccountSet).getByOpenIDIdentifier(
+                self.openid_response.identity_url.split('/')[-1])
+            if account.status == AccountStatus.SUSPENDED:
+                return self.suspended_account_template()
+            if IPerson(account, None) is None:
+                removeSecurityProxy(account).createPerson(
+                    PersonCreationRationale.OWNER_CREATED_LAUNCHPAD)
+            self.login(account)
+            target = self.request.form.get('starting_url')
+            if target is None:
+                target = self.getApplicationURL()
+            self.request.response.redirect(target, temporary_if_possible=True)
+            # No need to return anything as we redirect above.
+            retval = None
+        else:
+            retval = OpenIDLoginErrorView(
+                self.context, self.request, self.openid_response)()
+
+        # The consumer.complete() call above will create entries in
+        # OpenIDConsumerNonce to prevent replay attacks, but since this will
+        # be a GET request, the transaction would be rolled back, so we need
+        # an explicit commit here.
+        transaction.commit()
+
+        return retval
+
+
+class OpenIDLoginErrorView(LaunchpadView):
+
+    page_title = 'Error logging in'
+    template = ViewPageTemplateFile("../templates/login-error.pt")
+
+    def __init__(self, context, request, openid_response):
+        super(OpenIDLoginErrorView, self).__init__(context, request)
+        assert self.account is None, (
+            "Don't try to render this page when the user is logged in.")
+        if openid_response.status == CANCEL:
+            self.login_error = "User cancelled"
+        elif openid_response.status == FAILURE:
+            self.login_error = openid_response.message
+        else:
+            self.login_error = "Unknown error: %s" % openid_response
+
+
+class AlreadyLoggedInView(LaunchpadView):
+
+    page_title = 'Already logged in'
+    template = ViewPageTemplateFile("../templates/login-already.pt")
+
+
 class LoginOrRegister(CaptchaMixin):
     """Merges the former CookieLoginPage and JoinLaunchpadView classes
     to allow the two forms to appear on a single page.
@@ -438,6 +617,7 @@
             L.append(html % (name, cgi.escape(value, quote=True)))
         return '\n'.join(L)
 
+
 def logInPrincipal(request, principal, email):
     """Log the principal in. Password validation must be done in callsites."""
     session = ISession(request)

=== modified file 'lib/canonical/launchpad/webapp/session.py'
--- lib/canonical/launchpad/webapp/session.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/webapp/session.py	2010-02-18 10:55:33 +0000
@@ -11,8 +11,9 @@
 
 from storm.zope.interfaces import IZStorm
 
+from lazr.uri import URI
+
 from canonical.config import config
-from canonical.launchpad.webapp.url import urlparse
 
 
 SECONDS = 1
@@ -99,22 +100,19 @@
         We also log the referrer url on creation of a new
         requestid so we can track where first time users arrive from.
         """
-        # XXX: SteveAlexander, 2007-04-01.
-        #      This is on the codepath where anon users get a session cookie
-        #      set unnecessarily.
         CookieClientIdManager.setRequestId(self, request, id)
 
         cookie = request.response.getCookie(self.namespace)
-        protocol, request_domain = urlparse(request.getURL())[:2]
+        uri = URI(request.getURL())
 
         # Set secure flag on cookie.
-        if protocol != 'http':
+        if uri.scheme != 'http':
             cookie['secure'] = True
         else:
             cookie['secure'] = False
 
         # Set domain attribute on cookie if vhosting requires it.
-        cookie_domain = get_cookie_domain(request_domain)
+        cookie_domain = get_cookie_domain(uri.host)
         if cookie_domain is not None:
             cookie['domain'] = cookie_domain
 

=== renamed file 'lib/canonical/launchpad/pagetests/standalone/xx-cookie-authentication.txt' => 'lib/canonical/launchpad/webapp/tests/cookie-authentication.txt'
--- lib/canonical/launchpad/pagetests/standalone/xx-cookie-authentication.txt	2008-10-11 00:37:08 +0000
+++ lib/canonical/launchpad/webapp/tests/cookie-authentication.txt	2010-02-18 10:55:33 +0000
@@ -2,45 +2,56 @@
 on http instead of https, it cannot read the secure cookie on https,
 so it cannot tell that it will end up overwriting the existing cookie.
 
-
-    >>> from canonical.launchpad.testing.cookie import Cookies
-    >>> cookies = Cookies(browser)
-    >>> browser.open('http://feeds.launchpad.dev/announcements.atom')
+    >>> browser.open('http://feeds.launchpad.dev:8085/announcements.atom')
     >>> browser.url
-    'http://feeds.launchpad.dev/announcements.atom'
-    >>> len(cookies)
+    'http://feeds.launchpad.dev:8085/announcements.atom'
+    >>> len(browser.cookies)
     0
 
 Our cookies need to have their domain attribute set to ensure that they
 are sent to other vhosts in the same domain.
 
-    >>> browser.open('http://blueprints.launchpad.dev/+login')
-    >>> browser.getControl('E-mail', index=0).value = '[e-mail]'
-    >>> browser.getControl('Password').value = 'test'
-    >>> browser.getControl('Log In').click()
+    >>> browser.open('http://blueprints.launchpad.dev:8085/+login')
+
+    # On a browser with JS support, this page would've been automatically
+    # submitted (thanks to the onload handler), but testbrowser doesn't support
+    # JS, so we have to submit the form manually.
+    >>> print browser.contents
+    <html>...<body onload="document.forms[0].submit();"...
+    >>> browser.getControl('Continue').click()
+
+    >>> from canonical.launchpad.webapp.tests.test_new_login import (
+    ...     fill_login_form_and_submit)
+    >>> fill_login_form_and_submit(browser, '[e-mail]', 'test')
     >>> print extract_text(find_tag_by_id(browser.contents, 'logincontrol'))
     Foo Bar...
-    >>> len(cookies)
+
+    # Open a page again so that we see the cookie for a launchpad.dev request
+    # and not a testopenid.dev request (as above).
+    >>> browser.open('http://blueprints.launchpad.dev:8085')
+    >>> len(browser.cookies)
     1
-    >>> session_cookie_name = cookies.keys()[0]
-    >>> cookies.getInfo(session_cookie_name)['domain']
+    >>> browser.cookies.keys()
+    ['launchpad_tests']
+    >>> session_cookie_name = browser.cookies.keys()[0]
+    >>> browser.cookies.getinfo(session_cookie_name)['domain']
     '.launchpad.dev'
 
 If we visit another vhost in the domain, we remain logged in.
 
-    >>> browser.open('http://launchpad.dev/')
+    >>> browser.open('http://launchpad.dev:8085/')
     >>> browser.url
-    'http://launchpad.dev/'
+    'http://launchpad.dev:8085/'
     >>> print extract_text(find_tag_by_id(browser.contents, 'logincontrol'))
     Foo Bar...
-    >>> cookies.getInfo(session_cookie_name)['domain']
+    >>> browser.cookies.getinfo(session_cookie_name)['domain']
     '.launchpad.dev'
 
 Even if the browser passes in a cookie, the feeds vhost should not set one.
 
-    >>> browser.open('http://feeds.launchpad.dev/announcements.atom')
+    >>> browser.open('http://feeds.launchpad.dev:8085/announcements.atom')
     >>> browser.url
-    'http://feeds.launchpad.dev/announcements.atom'
+    'http://feeds.launchpad.dev:8085/announcements.atom'
     >>> print browser.headers.get('Set-Cookie')
     None
 

=== added file 'lib/canonical/launchpad/webapp/tests/login.txt'
--- lib/canonical/launchpad/webapp/tests/login.txt	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/webapp/tests/login.txt	2010-02-18 10:55:33 +0000
@@ -0,0 +1,47 @@
+======================
+Logging into Launchpad
+======================
+
+Launchpad is an OpenID Relying Party that uses the Login Service as its fixed
+OpenID Provider. Because of that, when a user clicks the 'Log in / Register'
+link, they'll be sent to the OP to authenticate.
+
+    # Set handleErrors to True so that the Unauthorized exception is handled
+    # by the publisher and we get redirected to the +login page.
+    >>> browser = Browser()
+    >>> browser.handleErrors = True
+    >>> browser.open(
+    ...     'http://launchpad.dev:8085/people/?name=foo&searchfor=all')
+    >>> browser.getLink('Log in / Register').click()
+
+    # On a browser with JS support, this page would've been automatically
+    # submitted (thanks to the onload handler), but testbrowser doesn't support
+    # JS, so we have to submit the form manually.
+    >>> print browser.contents
+    <html>...<body onload="document.forms[0].submit();"...
+    >>> browser.getControl('Continue').click()
+
+The OpenID Provider will ask us to authenticate.
+
+    >>> print browser.title
+    Login
+    >>> from canonical.launchpad.webapp.tests.test_new_login import (
+    ...     fill_login_form_and_submit)
+    >>> fill_login_form_and_submit(browser, '[e-mail]', 'test')
+
+Once authenticated, we're redirected back to the page where we started, with
+the query args preserved.
+
+    >>> url = browser.url
+    >>> print url
+    http://launchpad.dev:8085/people?...
+    >>> import re
+    >>> print sorted(re.sub('.*\?', '', url).split('&'))
+    ['name=foo', 'searchfor=all']
+
+If we load the +login page while already logged in, it will say we're already
+logged in and ask us to log out if we're somebody else.
+
+    >>> browser.open('http://launchpad.dev:8085/+login')
+    >>> print extract_text(find_main_content(browser.contents))
+    You are already logged in...

=== renamed file 'lib/canonical/launchpad/pagetests/standalone/xx-no-anonymous-session-cookies.txt' => 'lib/canonical/launchpad/webapp/tests/no-anonymous-session-cookies.txt'
--- lib/canonical/launchpad/pagetests/standalone/xx-no-anonymous-session-cookies.txt	2008-10-11 00:37:08 +0000
+++ lib/canonical/launchpad/webapp/tests/no-anonymous-session-cookies.txt	2010-02-18 10:55:33 +0000
@@ -1,20 +1,12 @@
 We will verify that we do not put session cookies in anonymous requests. This
 is important for cacheing anonymous requests in front of Zope, such as with
-Squid.
-
-Because testbrowser does not have easy access to cookies, we'll start with a
-helper class. We want to check whether the browser has a session cookie set,
-not whether the server has sent a "set-cookie" header, so we have to dig into
-the underlying mechanize browser.
-
-    >>> from canonical.launchpad.testing.cookie import Cookies
-    >>> cookies = Cookies(browser)
-
-Now we'll actually begin the demonstration. When we go to launchpad as an
-anonymous user, the browser has no cookies.
-
-    >>> browser.open('http://launchpad.dev')
-    >>> len(cookies)
+Squid.  Note that we are checking whether the browser has a session cookie
+set, not whether the server has sent a "set-cookie" header.
+
+When we go to launchpad as an anonymous user, the browser has no cookies.
+
+    >>> browser.open('http://launchpad.dev:8085')
+    >>> len(browser.cookies)
     0
 
 Now let's log in and show that the session cookie is set.
@@ -24,20 +16,33 @@
     >>> now = datetime.datetime.now(pytz.UTC).replace(microsecond=0)
     >>> year_from_now = now + datetime.timedelta(days=365)
     >>> year_plus_from_now = year_from_now + datetime.timedelta(minutes=1)
-    >>> browser.open('http://launchpad.dev/+login')
-    >>> browser.getControl('E-mail', index=0).value = '[e-mail]'
-    >>> browser.getControl('Password').value = 'test'
-    >>> browser.getControl('Log In').click()
+    >>> browser.open('http://launchpad.dev:8085/+login')
+
+    # On a browser with JS support, this page would've been automatically
+    # submitted (thanks to the onload handler), but testbrowser doesn't support
+    # JS, so we have to submit the form manually.
+    >>> print browser.contents
+    <html>...<body onload="document.forms[0].submit();"...
+    >>> browser.getControl('Continue').click()
+
+    >>> from canonical.launchpad.webapp.tests.test_new_login import (
+    ...     fill_login_form_and_submit)
+    >>> fill_login_form_and_submit(browser, '[e-mail]', 'test')
     >>> print extract_text(find_tag_by_id(browser.contents, 'logincontrol'))
     Foo Bar...
-    >>> len(cookies)
+
+    # Open a page again so that we see the cookie for a launchpad.dev request
+    # and not a testopenid.dev request (as above).
+    >>> browser.open('http://launchpad.dev:8085')
+
+    >>> len(browser.cookies)
     1
-    >>> cookies.keys()
+    >>> browser.cookies.keys()
     ['launchpad_tests']
-    >>> expires = cookies.getInfo('launchpad_tests')['expires']
+    >>> expires = browser.cookies.getinfo('launchpad_tests')['expires']
     >>> year_from_now <= expires < year_plus_from_now
     True
-    >>> cookies.getInfo('launchpad_tests')['domain']
+    >>> browser.cookies.getinfo('launchpad_tests')['domain']
     '.launchpad.dev'
 
 The cookie will be set to expire in ten minutes when you log out.  The ten
@@ -46,9 +51,9 @@
 time for browsers with bad system clocks.
 
     >>> browser.getControl('Log Out').click()
-    >>> len(cookies)
+    >>> len(browser.cookies)
     1
-    >>> expires = cookies.getInfo('launchpad_tests')['expires']
+    >>> expires = browser.cookies.getinfo('launchpad_tests')['expires']
     >>> ten_minutes_from_now = now + datetime.timedelta(minutes=10)
     >>> eleven_minutes_from_now = now + datetime.timedelta(minutes=11)
     >>> ten_minutes_from_now <= expires < eleven_minutes_from_now

=== added file 'lib/canonical/launchpad/webapp/tests/test_cookie_authentication.py'
--- lib/canonical/launchpad/webapp/tests/test_cookie_authentication.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/webapp/tests/test_cookie_authentication.py	2010-02-18 10:55:33 +0000
@@ -0,0 +1,24 @@
+# Copyright 2010 Canonical Ltd.  All rights reserved.
+
+"""Test harness for running the cookie-authentication.txt tests."""
+
+__metaclass__ = type
+
+__all__ = []
+
+import unittest
+
+from canonical.launchpad.testing.systemdocs import LayeredDocFileSuite
+from canonical.launchpad.testing.browser import setUp, tearDown
+from canonical.testing.layers import AppServerLayer
+
+
+def test_suite():
+    suite = unittest.TestSuite()
+    # We run this test on the AppServerLayer because it needs the cookie login
+    # page (+login), which cannot be used through the normal testbrowser that
+    # goes straight to zope's publication instead of making HTTP requests.
+    suite.addTest(LayeredDocFileSuite(
+        'cookie-authentication.txt', setUp=setUp, tearDown=tearDown,
+        layer=AppServerLayer))
+    return suite

=== added file 'lib/canonical/launchpad/webapp/tests/test_new_login.py'
--- lib/canonical/launchpad/webapp/tests/test_new_login.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/webapp/tests/test_new_login.py	2010-02-18 10:55:33 +0000
@@ -0,0 +1,188 @@
+# Copyright 2009 Canonical Ltd.  All rights reserved.
+
+"""Test harness for running the new-login.txt tests."""
+
+__metaclass__ = type
+
+__all__ = []
+
+import httplib
+import unittest
+
+import mechanize
+
+from openid.consumer.consumer import FAILURE, SUCCESS
+
+from canonical.launchpad.interfaces.account import AccountStatus
+from canonical.launchpad.testing.pages import (
+    extract_text, find_main_content, find_tag_by_id, find_tags_by_class)
+from canonical.launchpad.testing.systemdocs import LayeredDocFileSuite
+from canonical.launchpad.testing.browser import Browser, setUp, tearDown
+from canonical.launchpad.webapp.login import OpenIDCallbackView
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
+from canonical.testing.layers import AppServerLayer, DatabaseFunctionalLayer
+
+from lp.registry.interfaces.person import IPerson
+from lp.testopenid.interfaces.server import ITestOpenIDPersistentIdentity
+from lp.testing import TestCaseWithFactory
+
+
+class FakeOpenIDResponse:
+
+    def __init__(self, identity_url, status=SUCCESS, message=''):
+        self.message = message
+        self.status = status
+        self.identity_url = identity_url
+
+
+class StubbedOpenIDCallbackView(OpenIDCallbackView):
+    login_called = False
+
+    def login(self, account):
+        self.login_called = True
+
+
+class TestOpenIDCallbackView(TestCaseWithFactory):
+    layer = DatabaseFunctionalLayer
+
+    def _createView(self, account, response_status=SUCCESS, response_msg=''):
+        request = LaunchpadTestRequest(
+            form={'starting_url': 'http://launchpad.dev/after-login'},
+            environ={'PATH_INFO': '/'})
+        view = StubbedOpenIDCallbackView(object(), request)
+        view.initialize()
+        view.openid_response = FakeOpenIDResponse(
+            ITestOpenIDPersistentIdentity(account).openid_identity_url,
+            status=response_status, message=response_msg)
+        return view
+
+    def test_full_fledged_account(self):
+        # In the common case we just login and redirect to the URL specified
+        # in the 'starting_url' query arg.
+        person = self.factory.makePerson()
+        view = self._createView(person.account)
+        view.render()
+        self.assertTrue(view.login_called)
+        response = view.request.response
+        self.assertEquals(httplib.TEMPORARY_REDIRECT, response.getStatus())
+        self.assertEquals(view.request.form['starting_url'],
+                          response.getHeader('Location'))
+
+    def test_personless_account(self):
+        # When there is no Person record associated with the account, we
+        # create one.
+        account = self.factory.makeAccount('Test account')
+        self.assertIs(None, IPerson(account, None))
+        view = self._createView(account)
+        view.render()
+        self.assertIsNot(None, IPerson(account, None))
+        self.assertTrue(view.login_called)
+        response = view.request.response
+        self.assertEquals(httplib.TEMPORARY_REDIRECT, response.getStatus())
+        self.assertEquals(view.request.form['starting_url'],
+                          response.getHeader('Location'))
+
+    def test_deactivated_account(self):
+        # The user has the account's password and is trying to login, so we'll
+        # just re-activate their account.
+        account = self.factory.makeAccount(
+            'Test account', status=AccountStatus.DEACTIVATED)
+        self.assertIs(None, IPerson(account, None))
+        view = self._createView(account)
+        view.render()
+        self.assertIsNot(None, IPerson(account, None))
+        self.assertTrue(view.login_called)
+        response = view.request.response
+        self.assertEquals(httplib.TEMPORARY_REDIRECT, response.getStatus())
+        self.assertEquals(view.request.form['starting_url'],
+                          response.getHeader('Location'))
+
+    def test_suspended_account(self):
+        # There's a chance that our OpenID Provider lets a suspended account
+        # login, but we must not allow that.
+        account = self.factory.makeAccount(
+            'Test account', status=AccountStatus.SUSPENDED)
+        view = self._createView(account)
+        html = view.render()
+        self.assertFalse(view.login_called)
+        main_content = extract_text(find_main_content(html))
+        self.assertIn('This account has been suspended', main_content)
+
+    def test_negative_openid_assertion(self):
+        # The OpenID provider responded with a negative assertion, so the
+        # login error page is shown.
+        account = self.factory.makeAccount('Test account')
+        view = self._createView(
+            account, response_status=FAILURE,
+            response_msg='Server denied check_authentication')
+        html = view.render()
+        self.assertFalse(view.login_called)
+        main_content = extract_text(find_main_content(html))
+        self.assertIn('Your login was unsuccessful', main_content)
+
+
+urls_redirected_to = []
+
+
+class MyHTTPRedirectHandler(mechanize.HTTPRedirectHandler):
+    """Custom HTTPRedirectHandler which stores the URLs redirected to."""
+
+    def redirect_request(self, newurl, req, fp, code, msg, headers):
+        urls_redirected_to.append(newurl)
+        return mechanize.HTTPRedirectHandler.redirect_request(
+            self, newurl, req, fp, code, msg, headers)
+
+
+class MyMechanizeBrowser(mechanize.Browser):
+    """Custom Browser which uses MyHTTPRedirectHandler to handle redirects."""
+    handler_classes = mechanize.Browser.handler_classes.copy()
+    handler_classes['_redirect'] = MyHTTPRedirectHandler
+
+
+class TestOpenIDReplayAttack(TestCaseWithFactory):
+    layer = AppServerLayer
+
+    def test_replay_attacks_do_not_succeed(self):
+        browser = Browser(mech_browser=MyMechanizeBrowser())
+        browser.open('http://launchpad.dev:8085/+login')
+        # On a JS-enabled browser this page would've been auto-submitted
+        # (thanks to the onload handler), but here we have to do it manually.
+        self.assertIn('body onload', browser.contents)
+        browser.getControl('Continue').click()
+
+        self.assertEquals('Login', browser.title)
+        fill_login_form_and_submit(browser, '[e-mail]', 'test')
+        login_status = extract_text(
+            find_tag_by_id(browser.contents, 'logincontrol'))
+        self.assertIn('Sample Person', login_status)
+
+        # Now we look up (in urls_redirected_to) the +openid-callback URL that
+        # was used to complete the authentication and open it on a different
+        # browser with a fresh set of cookies.
+        replay_browser = Browser()
+        [callback_url] = [
+            url for url in urls_redirected_to if '+openid-callback' in url]
+        self.assertIsNot(None, callback_url)
+        replay_browser.open(callback_url)
+        login_status = extract_text(
+            find_tag_by_id(replay_browser.contents, 'logincontrol'))
+        self.assertEquals('Log in / Register', login_status)
+        error_msg = find_tags_by_class(replay_browser.contents, 'error')[0]
+        self.assertEquals('Nonce already used or out of range',
+                          extract_text(error_msg))
+
+
+def fill_login_form_and_submit(browser, email_address, password):
+    assert browser.getControl(name='field.email') is not None, (
+        "We don't seem to be looking at a login form.")
+    browser.getControl(name='field.email').value = email_address
+    browser.getControl(name='field.password').value = password
+    browser.getControl('Continue').click()
+
+
+def test_suite():
+    suite = unittest.TestSuite()
+    suite.addTest(unittest.TestLoader().loadTestsFromName(__name__))
+    suite.addTest(LayeredDocFileSuite(
+        'login.txt', setUp=setUp, tearDown=tearDown, layer=AppServerLayer))
+    return suite

=== added file 'lib/canonical/launchpad/webapp/tests/test_no_anonymous_session_cookies.py'
--- lib/canonical/launchpad/webapp/tests/test_no_anonymous_session_cookies.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/webapp/tests/test_no_anonymous_session_cookies.py	2010-02-18 10:55:33 +0000
@@ -0,0 +1,24 @@
+# Copyright 2010 Canonical Ltd.  All rights reserved.
+
+"""Test harness for running the no-anonymous-session-cookies.txt tests."""
+
+__metaclass__ = type
+
+__all__ = []
+
+import unittest
+
+from canonical.launchpad.testing.systemdocs import LayeredDocFileSuite
+from canonical.launchpad.testing.browser import setUp, tearDown
+from canonical.testing.layers import AppServerLayer
+
+
+def test_suite():
+    suite = unittest.TestSuite()
+    # We run this test on the AppServerLayer because it needs the cookie login
+    # page (+login), which cannot be used through the normal testbrowser that
+    # goes straight to zope's publication instead of making HTTP requests.
+    suite.addTest(LayeredDocFileSuite(
+        'no-anonymous-session-cookies.txt', setUp=setUp, tearDown=tearDown,
+        layer=AppServerLayer))
+    return suite

=== modified file 'lib/canonical/launchpad/windmill/testing/lpuser.py'
--- lib/canonical/launchpad/windmill/testing/lpuser.py	2010-02-02 11:36:08 +0000
+++ lib/canonical/launchpad/windmill/testing/lpuser.py	2010-02-18 10:55:33 +0000
@@ -6,6 +6,8 @@
 __metaclass__ = type
 __all__ = []
 
+import windmill
+
 from canonical.launchpad.windmill.testing import constants
 
 
@@ -20,31 +22,22 @@
     def ensure_login(self, client):
         """Ensure that this user is logged on the page under windmill."""
         client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
-        result = client.asserts.assertNode(
-            name=u'loginpage_submit_login', assertion=False)
-        already_on_login_page = result['result']
-        if not already_on_login_page:
-            result = client.asserts.assertNode(
-                link=u'Log in / Register', assertion=False)
-            if not result['result']:
-                # User is probably logged in.
-                # Check under which name they are logged in.
-                result = client.commands.execJS(
-                    code="""lookupNode({xpath: '//div[@id="logincontrol"]//a'}).text""")
-                if (result['result'] is not None and
-                    result['result'].strip() == self.display_name):
-                    # We are logged as that user.
-                    return
-                client.click(name="logout")
-                client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
-                client.waits.forElement(
-                    link=u'Log in / Register', timeout=constants.FOR_ELEMENT)
-            client.click(link=u'Log in / Register')
+        lookup_user = (
+            """lookupNode({xpath: '//div[@id="logincontrol"]//a'}).text""")
+        result = client.commands.execJS(code=lookup_user)
+        if (result['result'] is not None and
+            result['result'].strip() == self.display_name):
+            # We are logged in as that user already.
+            return
+
+        current_url = client.commands.execJS(
+            code='windmill.testWin().location;')['result']['href']
+        base_url = windmill.settings['TEST_URL']
+        basic_auth_url = base_url.replace('http://', 'http://%s:%s@')
+        basic_auth_url = basic_auth_url + '+basiclogin'
+        client.open(url=basic_auth_url % (self.email, self.password))
         client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
-        client.waits.forElement(timeout=constants.FOR_ELEMENT, id=u'email')
-        client.type(text=self.email, id=u'email')
-        client.type(text=self.password, id=u'password')
-        client.click(name=u'loginpage_submit_login')
+        client.open(url=current_url)
         client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
 
 
@@ -58,8 +51,13 @@
             link=u'Log in / Register', assertion=False)
         if result['result']:
             return
-        client.waits.forElement(name="logout", timeout=constants.FOR_ELEMENT)
-        client.click(name="logout")
+
+        # Open a page with invalid HTTP Basic Auth credentials just to
+        # invalidate the ones previously used.
+        current_url = client.commands.execJS(
+            code='windmill.testWin().location;')['result']['href']
+        current_url = current_url.replace('http://', 'http://foo:foo@')
+        client.open(url=current_url)
         client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
 
 

=== modified file 'lib/canonical/launchpad/windmill/testing/widgets.py'
--- lib/canonical/launchpad/windmill/testing/widgets.py	2009-12-11 19:54:04 +0000
+++ lib/canonical/launchpad/windmill/testing/widgets.py	2010-02-18 10:55:33 +0000
@@ -60,10 +60,9 @@
         * reloads and verifies that the new value sticked.
         """
         client = WindmillTestClient(self.suite)
+        self.user.ensure_login(client)
         client.open(url=self.url)
 
-        self.user.ensure_login(client)
-
         client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
         widget_base = u"//%s[@id='%s']" % (self.widget_tag, self.widget_id)
         client.waits.forElement(
@@ -298,12 +297,11 @@
 
     def __call__(self):
         client = WindmillTestClient(self.suite)
+        self.user.ensure_login(client)
 
         # Load page.
         client.open(url=self.url)
 
-        self.user.ensure_login(client)
-
         # Click on "Choose" link to show picker for the given field.
         client.waits.forElement(
             id=self.choose_link_id, timeout=constants.PAGE_LOAD)

=== modified file 'lib/canonical/launchpad/zcml/launchpad.zcml'
--- lib/canonical/launchpad/zcml/launchpad.zcml	2010-01-12 22:07:13 +0000
+++ lib/canonical/launchpad/zcml/launchpad.zcml	2010-02-18 10:55:33 +0000
@@ -175,14 +175,40 @@
       permission="zope.Public"
       />
 
-  <!-- Login and logout pages, and login status. -->
+
+  <!-- The +basiclogin view is registered using Python code so that we can do
+       it only for development and tests. Below is what its declaration would
+       look like, and it's here so that someone grepping zcml files for its
+       name will find it.
   <browser:page
       for="canonical.launchpad.webapp.interfaces.ILaunchpadApplication"
       class="canonical.launchpad.webapp.login.BasicLoginPage"
       name="+basiclogin"
-      permission="zope.Public"
-      attribute="login"
-      layer="canonical.launchpad.layers.DebugLayer"
+      permission="zope.Public" />
+  -->
+
+  <class class="canonical.launchpad.webapp.login.BasicLoginPage">
+    <allow attributes="__call__" />
+    <allow interface="zope.publisher.interfaces.browser.IBrowserPublisher" />
+  </class>
+
+  <subscriber
+      for="zope.processlifetime.ProcessStarting"
+      handler="canonical.launchpad.webapp.login.register_basiclogin"
+      />
+
+  <!-- OpenID RP views -->
+  <browser:page
+      for="canonical.launchpad.webapp.interfaces.ILaunchpadApplication"
+      class="canonical.launchpad.webapp.login.OpenIDLogin"
+      permission="zope.Public"
+      name="+login"
+      />
+  <browser:page
+      for="canonical.launchpad.webapp.interfaces.ILaunchpadApplication"
+      class="canonical.launchpad.webapp.login.OpenIDCallbackView"
+      permission="zope.Public"
+      name="+openid-callback"
       />
 
   <browser:page
@@ -190,7 +216,7 @@
       template="../templates/launchpad-login.pt"
       class="canonical.launchpad.webapp.login.LoginOrRegister"
       permission="zope.Public"
-      name="+login"
+      name="+login-old"
       />
 
   <browser:page

=== modified file 'lib/canonical/testing/layers.py'
--- lib/canonical/testing/layers.py	2010-02-05 12:16:34 +0000
+++ lib/canonical/testing/layers.py	2010-02-18 10:55:33 +0000
@@ -1880,6 +1880,7 @@
                 'rooturl: http://blueprints.launchpad.dev:8085/'),
             ('vhost.bugs', 'rooturl: http://bugs.launchpad.dev:8085/'),
             ('vhost.code', 'rooturl: http://code.launchpad.dev:8085/'),
+            ('vhost.testopenid', 'rooturl: http://testopenid.dev:8085/'),
             ('vhost.translations',
                 'rooturl: http://translations.launchpad.dev:8085/'))
         for site in sites:

=== modified file 'lib/devscripts/ec2test/instance.py'
--- lib/devscripts/ec2test/instance.py	2010-02-09 14:20:54 +0000
+++ lib/devscripts/ec2test/instance.py	2010-02-18 10:55:33 +0000
@@ -105,7 +105,7 @@
     shipit.edubuntu.dev
     shipit.kubuntu.dev
     shipit.ubuntu.dev
-    testopenid.launchpad.dev
+    testopenid.dev
     translations.launchpad.dev
     xmlrpc-private.launchpad.dev
     xmlrpc.launchpad.dev

=== modified file 'lib/lp/app/stories/basics/xx-beta-testers-redirection.txt'
--- lib/lp/app/stories/basics/xx-beta-testers-redirection.txt	2010-01-08 20:57:08 +0000
+++ lib/lp/app/stories/basics/xx-beta-testers-redirection.txt	2010-02-18 10:55:33 +0000
@@ -157,29 +157,6 @@
     >>> print beta_browser.url
     http://launchpad.dev/ubuntu
 
-If a beta tester comes to Launchpad but is not yet logged in, they
-will be redirected once they do log in.  First we'll go to the Ubuntu
-product page and try to log in.  We get redirected back to the Ubuntu
-page after authenticating successfully:
-
-    >>> browser.mech_browser.set_handle_redirect(False)
-    >>> browser.open('http://launchpad.dev/ubuntu')
-    >>> browser.getLink('Log in / Register').click()
-    >>> print browser.url
-    http://launchpad.dev/ubuntu/+login
-    >>> browser.getControl('E-mail address',
-    ...                    index=0).value = '[e-mail]'
-    >>> browser.getControl('Password').value = 'test'
-    >>> check(browser.getControl('Log In').click)
-    HTTP Error 303: See Other
-    Location: http://launchpad.dev/ubuntu
-
-The Ubuntu product page then redirects to the beta site:
-
-    >>> check(browser.open, 'http://launchpad.dev/ubuntu')
-    HTTP Error 303: See Other
-    Location: http://beta.launchpad.dev/ubuntu
-
 
 == Shortcut redirection for bugs ==
 

=== modified file 'lib/lp/app/stories/launchpad-root/site-search.txt'
--- lib/lp/app/stories/launchpad-root/site-search.txt	2009-11-22 15:51:50 +0000
+++ lib/lp/app/stories/launchpad-root/site-search.txt	2010-02-18 10:55:33 +0000
@@ -39,15 +39,6 @@
     ...
     LookupError
 
-And on the login and registration pages, the global form is also omitted to
-reduce confusion.
-
-    >>> anon_browser.open('http://launchpad.dev/+login')
-    >>> global_search_form = anon_browser.getForm('globalsearch')
-    Traceback (most recent call last):
-    ...
-    LookupError
-
 If by chance someone ends up at /+search with no search parameters, they get
 an explanation of the search function.
 

=== modified file 'lib/lp/bugs/stories/bugtracker/xx-bugtracker-remote-bug.txt'
--- lib/lp/bugs/stories/bugtracker/xx-bugtracker-remote-bug.txt	2009-10-22 13:02:12 +0000
+++ lib/lp/bugs/stories/bugtracker/xx-bugtracker-remote-bug.txt	2010-02-18 10:55:33 +0000
@@ -92,13 +92,14 @@
   Bug #2: Blackhole Trash folder
 
 For the case where the private bug is the only one watching the given
-remote bug, we don't perform the redirect ahead of time:
+remote bug, we don't perform the redirect ahead of time (i.e. before the
+user logs in):
 
-  >>> anon_browser.handleErrors = True
   >>> anon_browser.open(
   ...     'http://bugs.launchpad.dev/bugs/bugtrackers/mozilla.org/2000')
-  >>> print_feedback_messages(anon_browser.contents)
-  To continue, you must log in to Launchpad.
+  Traceback (most recent call last):
+  ...
+  Unauthorized:...
 
 Set the bug back to public:
 

=== modified file 'lib/lp/bugs/stories/upstream-bugprivacy/10-file-private-upstream-bug.txt'
--- lib/lp/bugs/stories/upstream-bugprivacy/10-file-private-upstream-bug.txt	2009-08-14 18:02:29 +0000
+++ lib/lp/bugs/stories/upstream-bugprivacy/10-file-private-upstream-bug.txt	2010-02-18 10:55:33 +0000
@@ -109,21 +109,18 @@
 redirects the anonymous user to the login page.
 
     >>> browser = setupBrowser()
-    >>> browser.handleErrors = True
     >>> browser.open(
     ...     "http://launchpad.dev/firefox/+bug/%s/+editstatus" % bug_id)
-
-XXX: Brad Bollenbach, 2005-09-13: This redirect is going to the wrong
-URL. See https://launchpad.net/malone/bugs/2265.
-
-    >>> print browser.url
-    http://launchpad.dev/firefox/+bug/.../+editstatus/+login
+    Traceback (most recent call last):
+    ...
+    Unauthorized:...
 
 The no-privs user cannot access bug #10, because it's filed on a private bug on
 which the no-privs is not an explicit subscriber.
 
     >>> browser = setupBrowser(auth="Basic [e-mail]:test")
-    >>> browser.open("http://launchpad.dev/firefox/+bug/%s/+editstatus" % bug_id)
+    >>> browser.open(
+    ...     "http://launchpad.dev/firefox/+bug/%s/+editstatus" % bug_id)
     Traceback (most recent call last):
     ...
     Unauthorized:...

=== modified file 'lib/lp/bugs/windmill/tests/test_bug_tags_entry.py'
--- lib/lp/bugs/windmill/tests/test_bug_tags_entry.py	2010-02-01 18:37:00 +0000
+++ lib/lp/bugs/windmill/tests/test_bug_tags_entry.py	2010-02-18 10:55:33 +0000
@@ -70,7 +70,7 @@
         client.click(id=u'edit-tags-trigger')
         client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
         client.asserts.assertJS(
-            js=u'window.location.href.indexOf("+login") > 0')
+            js=u'window.location.href.indexOf("+openid") > 0')
 
 
 def test_suite():

=== modified file 'lib/lp/bugs/windmill/tests/test_filebug_dupe_finder.py'
--- lib/lp/bugs/windmill/tests/test_filebug_dupe_finder.py	2010-02-02 11:26:31 +0000
+++ lib/lp/bugs/windmill/tests/test_filebug_dupe_finder.py	2010-02-18 10:55:33 +0000
@@ -39,11 +39,11 @@
         more information if they wish.
         """
         client = self.client
+        lpuser.SAMPLE_PERSON.ensure_login(client)
 
         # Go to the +filebug page for Firefox
         client.open(url=FILEBUG_URL)
         client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
-        lpuser.SAMPLE_PERSON.ensure_login(client)
 
         # Ensure the "search" field has finished loading, then enter a simple
         # search and hit search.

=== modified file 'lib/lp/code/stories/branches/xx-register-a-branch.txt'
--- lib/lp/code/stories/branches/xx-register-a-branch.txt	2009-09-18 15:24:30 +0000
+++ lib/lp/code/stories/branches/xx-register-a-branch.txt	2010-02-18 10:55:33 +0000
@@ -10,16 +10,10 @@
     Register a branch...
 
 Clicking the link as an anonymous user should take you to a login page. Once
-you've logged in, you should be redirected to the registration page.
+you've logged in, you will be redirected to the registration page.
 
-    >>> anon_browser.handleErrors = True
     >>> anon_browser.open('http://code.launchpad.dev/')
     >>> anon_browser.getLink('Register a branch').click()
-    >>> print anon_browser.title
-    Log in or register with Launchpad
-    >>> anon_browser.getControl('E-mail address', index=0).value = (
-    ...     '[e-mail]')
-    >>> anon_browser.getControl('Password').value = 'test'
-    >>> anon_browser.getControl('Log In').click()
-    >>> print anon_browser.title
-    Register a branch...
+    Traceback (most recent call last):
+    ...
+    Unauthorized:...

=== modified file 'lib/lp/registry/stories/person/xx-deactivate-account.txt'
--- lib/lp/registry/stories/person/xx-deactivate-account.txt	2009-09-16 17:56:17 +0000
+++ lib/lp/registry/stories/person/xx-deactivate-account.txt	2010-02-18 10:55:33 +0000
@@ -4,17 +4,11 @@
 accounts, so they stop receiving emails from Launchpad and make it impossible
 to use their accounts to log in.
 
-    >>> browser.open('http://launchpad.dev/+login')
-    >>> browser.getControl('E-mail', index=0).value = '[e-mail]'
-    >>> browser.getControl('Password').value = 'test'
-    >>> browser.getControl('Log In').click()
-    >>> print extract_text(find_tag_by_id(browser.contents, 'logincontrol'))
-    Sample Person...
-
 Deactivating a user's account will un-assign all their bug tasks. To
 demonstrate this, we'll assign a bug to the user that we're going to
 deactivate.
 
+    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
     >>> edit_bug_url = ("http://bugs.launchpad.dev/debian/sarge/+source/"
     ...     "mozilla-firefox/+bug/3/+editstatus")
     >>> browser.open(edit_bug_url)
@@ -67,16 +61,13 @@
     ...     print msg
     Your account has been deactivated.
 
-Sample Person can't log into Launchpad anymore.
+And now the Launchpad page for Sample Person person will clearly say he
+does not use Launchpad.
 
-    >>> browser.open('http://launchpad.dev/+login')
-    >>> browser.getControl('E-mail', index=0).value = '[e-mail]'
-    >>> browser.getControl('Password').value = 'test'
-    >>> browser.getControl('Log In').click()
-    >>> for msg in get_feedback_messages(browser.contents):
-    ...     print msg
-    The email address belongs to a deactivated account.
-    Use the "Forgotten your password" link to reactivate it.
+    >>> browser.open('http://launchpad.dev/~name12-deactivatedaccount')
+    >>> print extract_text(
+    ...     find_tag_by_id(browser.contents, 'not-lp-user-or-team'))
+    Sample Person does not use Launchpad.
 
 The bugs that were assigned to Sample Person will no longer have an
 assignee.
@@ -89,14 +80,6 @@
     Bug #3
     ...Assigned to unknown...
 
-And now the Launchpad page for that person will clearly say (s)he does not use
-Launchpad.
-
-    >>> browser.open('http://launchpad.dev/~name12-deactivatedaccount')
-    >>> print extract_text(
-    ...     find_tag_by_id(browser.contents, 'not-lp-user-or-team'))
-    Sample Person does not use Launchpad.
-
 Although teams have NOACCOUNT as their account_status, they are teams and so
 it makes no sense to say they don't use Launchpad.
 

=== modified file 'lib/lp/scripts/utilities/importfascist.py'
--- lib/lp/scripts/utilities/importfascist.py	2010-02-04 03:07:25 +0000
+++ lib/lp/scripts/utilities/importfascist.py	2010-02-18 10:55:33 +0000
@@ -59,6 +59,7 @@
 valid_imports_not_in_all = {
     'cookielib': set(['domain_match']),
     'email.Utils': set(['mktime_tz']),
+    'openid.fetchers': set(['Urllib2Fetcher']),
     'storm.database': set(['STATE_DISCONNECTED']),
     'textwrap': set(['dedent']),
     'zope.component': set(

=== modified file 'lib/lp/translations/stories/standalone/xx-person-editlanguages.txt'
--- lib/lp/translations/stories/standalone/xx-person-editlanguages.txt	2009-11-07 04:30:07 +0000
+++ lib/lp/translations/stories/standalone/xx-person-editlanguages.txt	2010-02-18 10:55:33 +0000
@@ -124,18 +124,20 @@
 with launchpad.AnyPerson as permission. This is the page to which we
 direct non-logged in users to edit their preferred languages.
 
-    >>> anon_browser.handleErrors = True
+The launchpad.AnyPerson permission means that when an anonymous user goes
+to that page, they'll be asked to login.
+
     >>> anon_browser.open('http://launchpad.dev/+editmylanguages')
-    >>> anon_browser.url
-    'http://launchpad.dev/+editmylanguages/+login'
-
-    >>> anon_browser.open('http://launchpad.dev/+editmylanguages/+login')
-    >>> anon_browser.getControl('E-mail address:', index=0).value = (
-    ...     '[e-mail]')
-    >>> anon_browser.getControl('Password:').value = 'test'
-    >>> anon_browser.getControl(name='loginpage_submit_login').click()
-    >>> anon_browser.url
-    'http://launchpad.dev/~no-priv/+editlanguages'
+    Traceback (most recent call last):
+    ...
+    Unauthorized: ...
+
+But a logged in user will be sent straight to their /~user/+editlanguages
+page.
+
+    >>> browser.open('http://launchpad.dev/+editmylanguages')
+    >>> browser.url
+    'http://launchpad.dev/~name12/+editlanguages'
 
 
 == Adding languages to teams ==

=== modified file 'lib/lp/translations/windmill/tests/test_documentation_links.py'
--- lib/lp/translations/windmill/tests/test_documentation_links.py	2010-02-01 18:37:00 +0000
+++ lib/lp/translations/windmill/tests/test_documentation_links.py	2010-02-18 10:55:33 +0000
@@ -38,10 +38,8 @@
         """
         client = self.client
 
-        start_url = 'http://translations.launchpad.dev:8085/'
         user = lpuser.TRANSLATIONS_ADMIN
 
-
         # Create a translation group with documentation to use in the test.
         group = self.factory.makeTranslationGroup(
             name='testing-group', title='Testing group',