Merge lp:~salgado/launchpad/bug-530738 into lp:launchpad

[Guilherme Salgado]

[Guilherme Salgado (salgado)]

We're currently passing the application URL as the realm when starting
an OpenID authentication. That means when you login on, say,
bugs.lp.net, the provider will be told that the realm is bugs.lp.net,
but that's not really true as the authentication is valid for all of
launchpad.net. That is also how the spec says the realm should be used:

  A realm is designed to give the end user an indication of the scope
  of the authentication request.
  (http://openid.net/specs/openid-authentication-2_0.html#realms)

This branch fixes that by always using the mainsite's root URL as the
realm (aka trust root).

[Brad Crittenden (bac)]

Looks great Salgado. I'm surprised this bug has been around so long. Good catch.

review: Approve (code)

[Francis J. Lacoste (flacoste)]

review: Approve (release-critical)

=== modified file 'lib/canonical/launchpad/webapp/login.py'
--- lib/canonical/launchpad/webapp/login.py	2010-03-01 19:25:42 +0000
+++ lib/canonical/launchpad/webapp/login.py	2010-03-02 15:19:14 +0000
@@ -186,7 +186,6 @@
         self.openid_request.addExtension(
             sreg.SRegRequest(optional=['email', 'fullname']))
 
-        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
@@ -196,8 +195,8 @@
         # '+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')
+        trust_root = allvhosts.configs['mainsite'].rooturl
+        return_to = urlappend(trust_root, '+openid-callback')
         return_to = "%s?%s" % (return_to, starting_url)
         form_html = self.openid_request.htmlMarkup(trust_root, return_to)
 

=== modified file 'lib/canonical/launchpad/webapp/tests/test_login.py'
--- lib/canonical/launchpad/webapp/tests/test_login.py	2010-03-01 19:25:42 +0000
+++ lib/canonical/launchpad/webapp/tests/test_login.py	2010-03-02 15:19:14 +0000
@@ -352,6 +352,31 @@
                           sorted(sreg_extension.allRequestedFields()))
 
 
+class TestOpenIDRealm(TestCaseWithFactory):
+    # The realm (aka trust_root) specified by the RP is "designed to give the
+    # end user an indication of the scope of the authentication request", so
+    # for us the realm will always be the root URL of the mainsite.
+    layer = AppServerLayer
+
+    def test_realm_for_mainsite(self):
+        browser = Browser()
+        browser.open('http://launchpad.dev:8085/+login')
+        # At this point browser.contents contains a hidden form which would've
+        # been auto-submitted if we had in-browser JS support, but since we
+        # don't we can easily inspect what's in the form.
+        self.assertEquals('http://launchpad.dev:8085/',
+                          browser.getControl(name='openid.realm').value)
+
+    def test_realm_for_vhosts(self):
+        browser = Browser()
+        browser.open('http://bugs.launchpad.dev:8085/+login')
+        # At this point browser.contents contains a hidden form which would've
+        # been auto-submitted if we had in-browser JS support, but since we
+        # don't we can easily inspect what's in the form.
+        self.assertEquals('http://launchpad.dev:8085/',
+                          browser.getControl(name='openid.realm').value)
+
+
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest(unittest.TestLoader().loadTestsFromName(__name__))