Merge lp:~jameinel/launchpad/py27-xmlrpc-auth-1019292 into lp:launchpad

[John A Meinel]

Fix XMLRPC Authentication for python-2.7

= Summary =

This fixes another python-2.7 vs 2.6 change in XMLRPC.
Specifically, we have a wrapper that avoids having to set up a real XMLRPC
server and talking to it (XMLRPCTestTransport).
However, it overrode a function that changed in 2.7. This specific fix is
because 'make_connection' in 2.7 now has a side effect of parsing the
extra headers that need to be passed to the server. Without them, you lose
the authentication information.

== Proposed fix ==

In 2.7 make_connection just saves the extra headers as
'self._extra_headers'. So we just follow suite. In 2.6 it just means an
extra attribute that will be ignored.

== LOC Rationale ==

This adds about 30 LoC. However the bulk of this is moving all of the doc
tests over to be unittests. Which should be more maintainable. (It
certainly was easier to debug the failure, since I could step around with
pdb.)

== Tests ==

No new tests added, just moved from doc tests to unit tests.

bin/test -vvv -m lp.xmlrpc.tests

Should run the test suite for this.

== Demo and Q/A ==

This seems to all be test-suite infrastructure, so the QA is 'do the tests
pass, even on python-2.7'.

[Brad Crittenden (bac)]

Hi John,

Thanks for the fix and going the extra step to convert to a unit test.

Rather than login(ANONYMOUS) I think using a with statement with the 'person_logged_in' context manager is cleaner.

typo: hello() returns Anonymous

Otherwise it looks good.

review: Approve (code)

=== modified file 'lib/lp/testing/xmlrpc.py'
--- lib/lp/testing/xmlrpc.py	2012-06-29 10:32:56 +0000
+++ lib/lp/testing/xmlrpc.py	2012-07-02 09:14:27 +0000
@@ -103,5 +103,8 @@
 
     def make_connection(self, host):
         """Return our custom HTTPCaller HTTPConnection."""
-        host, extra_headers, x509 = self.get_host_info(host)
+        # In Python2.7, make_connection caches the extra_headers, which is
+        # where authorization is stored. In 2.6 it is safe to cache them,
+        # because it just ignores the extra attribute.
+        host, self._extra_headers, x509 = self.get_host_info(host)
         return HTTPCallerHTTPConnection(host)

=== removed directory 'lib/lp/xmlrpc/doc'
=== removed file 'lib/lp/xmlrpc/tests/test_doc.py'
--- lib/lp/xmlrpc/tests/test_doc.py	2012-01-01 02:58:52 +0000
+++ lib/lp/xmlrpc/tests/test_doc.py	1970-01-01 00:00:00 +0000
@@ -1,18 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""
-Run the doctests and pagetests.
-"""
-
-import os
-
-from lp.services.testing import build_test_suite
-from lp.testing.layers import LaunchpadFunctionalLayer
-
-
-here = os.path.dirname(os.path.realpath(__file__))
-
-
-def test_suite():
-    return build_test_suite(here, {}, layer=LaunchpadFunctionalLayer)

=== renamed file 'lib/lp/xmlrpc/doc/private-xmlrpc.txt' => 'lib/lp/xmlrpc/tests/test_private_xmlrpc.py'
--- lib/lp/xmlrpc/doc/private-xmlrpc.txt	2011-12-23 16:15:14 +0000
+++ lib/lp/xmlrpc/tests/test_private_xmlrpc.py	2012-07-02 09:14:27 +0000
@@ -1,68 +1,91 @@
-Private XML-RPC ports
-=====================
-
-Several internal services require access to Launchpad data, typically over
-XML-RPC.  Because these services are internal and the data they expose is not
-needed by the outside world -- nor should it be -- Launchpad exposes an
-internal port for these private XML-RPC end points.  These internal-only end
-points are not available on the public XML-RPC port.
-
-    >>> public_root = 'http://[e-mail]:[e-mail]/'
-    >>> private_root = 'http://xmlrpc-private.launchpad.dev:8087/'
-
-For example, the team mailing list feature requires a connection between an
-internal Mailman server and Launchpad.  This end point is not available on the
-external XML-RPC port.
-
-    >>> import xmlrpclib
-    >>> from lp.testing.xmlrpc import XMLRPCTestTransport
-    >>> external_api = xmlrpclib.ServerProxy(
-    ...    public_root + 'mailinglists/',
-    ...    transport=XMLRPCTestTransport())
-    >>> external_api.getPendingActions()
-    Traceback (most recent call last):
-    ...
-    ProtocolError: <ProtocolError for
-    [e-mail]:[e-mail]/mailinglists/:
-    404 404 Not Found>
-
-However, the end point is available on the internal port and does not require
-authentication.
-
-    >>> internal_api = xmlrpclib.ServerProxy(
-    ...     private_root + 'mailinglists/',
-    ...     transport=XMLRPCTestTransport())
-    >>> internal_api.getPendingActions()
-    {}
-
-The bugs API on the other hand is an external service so it is available on
-the external port.
-
-    >>> external_api = xmlrpclib.ServerProxy(
-    ...    public_root + 'bugs/',
-    ...    transport=XMLRPCTestTransport())
-    >>> external_api.filebug(dict(
-    ...     product='firefox', summary='the summary', comment='the comment'))
-    'http://bugs.launchpad.dev/bugs/16'
-
-There is an interal bugs api, too, but that doesn't share the same
-methods as those exposed publicly.
-
-    >>> internal_api = xmlrpclib.ServerProxy(
-    ...     private_root + 'bugs/',
-    ...     transport=XMLRPCTestTransport())
-    >>> internal_api.filebug(dict(
-    ...     product='firefox', summary='the summary', comment='the comment'))
-    Traceback (most recent call last):
-    ...
-    ProtocolError: <ProtocolError for xmlrpc-private.launchpad.dev:8087/bugs/:
-    404 404 Not Found>
-
-    >>> from zope.component import getUtility
-    >>> from lp.services.verification.interfaces.logintoken import (
-    ...     ILoginTokenSet)
-    >>> token_string = internal_api.newBugTrackerToken()
-    >>> token = getUtility(ILoginTokenSet)[token_string]
-    >>> token
-    <LoginToken at ...>
-
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Private XMLRPC tests.
+"""
+
+import xmlrpclib
+
+from zope.component import getUtility
+
+from lp.services.verification.interfaces.logintoken import ILoginTokenSet
+
+from lp.testing import (
+    anonymous_logged_in,
+    person_logged_in,
+    )
+from lp.testing.layers import LaunchpadFunctionalLayer
+from lp.testing.xmlrpc import XMLRPCTestTransport
+from lp.testing import TestCase
+
+
+class TestPrivateXMLRPC(TestCase):
+    """Several internal services require access to Launchpad data, typically
+    over XML-RPC.  Because these services are internal and the data they expose
+    is not needed by the outside world -- nor should it be -- Launchpad exposes
+    an internal port for these private XML-RPC end points.  These internal-only
+    end points are not available on the public XML-RPC port.
+    """
+
+    layer = LaunchpadFunctionalLayer
+
+    public_root = 'http://[e-mail]:[e-mail]/'
+    private_root = 'http://xmlrpc-private.launchpad.dev:8087/'
+
+    def get_public_proxy(self, path):
+        """Get an xmlrpclib.ServerProxy pointing at the public URL"""
+        return xmlrpclib.ServerProxy(
+            self.public_root + path,
+            transport=XMLRPCTestTransport())
+
+    def get_private_proxy(self, path):
+        """Get an xmlrpclib.ServerProxy pointing at the private URL"""
+        return xmlrpclib.ServerProxy(
+            self.private_root + path,
+            transport=XMLRPCTestTransport())
+
+    def test_mailing_lists_not_public(self):
+        """For example, the team mailing list feature requires a connection
+        between an internal Mailman server and Launchpad.  This end point is
+        not available on the external XML-RPC port.
+        """
+        external_api = self.get_public_proxy('mailinglists/')
+        e = self.assertRaises(xmlrpclib.ProtocolError,
+                              external_api.getPendingActions)
+        self.assertEqual(404, e.errcode)
+
+    def test_mailing_lists_internally_available(self):
+        """However, the end point is available on the internal port and does
+        not require authentication.
+        """
+        internal_api = self.get_private_proxy('mailinglists/')
+        self.assertEqual({}, internal_api.getPendingActions())
+
+    def test_external_bugs_api(self):
+        """The bugs API on the other hand is an external service so it is
+        available on the external port.
+        """
+        with anonymous_logged_in():
+            external_api = self.get_public_proxy('bugs/')
+            bug_dict = dict(
+                product='firefox', summary='the summary', comment='the comment')
+            result = external_api.filebug(bug_dict)
+            self.assertEqual('http://bugs.launchpad.dev/bugs/16', result)
+
+    def test_internal_bugs_api(self):
+        """There is an interal bugs api, too, but that doesn't share the same
+        methods as those exposed publicly.
+        """
+        internal_api = self.get_private_proxy('bugs/')
+        bug_dict = dict(
+            product='firefox', summary='the summary', comment='the comment')
+        e = self.assertRaises(xmlrpclib.ProtocolError,
+                              internal_api.filebug, bug_dict)
+        self.assertEqual(404, e.errcode)
+
+    def test_get_utility(self):
+        with anonymous_logged_in():
+            internal_api = self.get_private_proxy('bugs/')
+            token_string = internal_api.newBugTrackerToken()
+            token = getUtility(ILoginTokenSet)[token_string]
+            self.assertEqual('LoginToken', token.__class__.__name__)

=== renamed file 'lib/lp/xmlrpc/doc/xmlrpc-selftest.txt' => 'lib/lp/xmlrpc/tests/test_xmlrpc_selftest.py'
--- lib/lp/xmlrpc/doc/xmlrpc-selftest.txt	2011-12-24 17:49:30 +0000
+++ lib/lp/xmlrpc/tests/test_xmlrpc_selftest.py	2012-07-02 09:14:27 +0000
@@ -1,83 +1,100 @@
-= XMLRPC self-test API =
-
-The Launchpad root object has a simple XMLRPC API to show that XMLRPC works.
-
-    >>> from lp.xmlrpc.application import SelfTest, ISelfTest
-    >>> from lp.services.webapp.testing import verifyObject
-    >>> selftestview = SelfTest('somecontext', 'somerequest')
-    >>> verifyObject(ISelfTest, selftestview)
-    True
-    >>> selftestview.concatenate('foo', 'bar')
-    u'foo bar'
-    >>> selftestview.make_fault()
-    <Fault 666: 'Yoghurt and spanners.'>
-
-We can test our XMLRPC APIs using xmlrpclib, using a custom Transport
-which talks with the publisher directly.
-
-    >>> import xmlrpclib
-    >>> from lp.testing.xmlrpc import XMLRPCTestTransport
-    >>> selftest = xmlrpclib.ServerProxy(
-    ...     'http://xmlrpc.launchpad.dev/', transport=XMLRPCTestTransport())
-    >>> selftest.concatenate('foo', 'bar')
-    'foo bar'
-    >>> selftest.make_fault()
-    Traceback (most recent call last):
-    ...
-    Fault: <Fault 666: 'Yoghurt and spanners.'>
-
-
-== Unexpected Exceptions ==
-
-Sometimes an XML-RPC method will be buggy, and raise an exception
-other than xmlrpclib.Fault.  We have such a method on the self test
-view:
-
-    >>> selftestview.raise_exception()
-    Traceback (most recent call last):
-      ...
-    RuntimeError: selftest exception
-
-
-As with normal browser requests, we don't want to expose these error
-messages to the user since they could contain confidential
-information.  Such exceptions get converted to a fault listing the
-OOPS ID (assuming one was generated):
-
-    >>> selftest.raise_exception()
-    Traceback (most recent call last):
-      ...
-    Fault: <Fault -1: 'OOPS-...'>
-
-
-== Authentication ==
-
-    >>> selftest.hello()
-    'Hello Anonymous.'
-
-The last call returned 'Anonymous', since we didn't provided a username
-and a password. If we do that, hello() will print the name of the
-logged in user:
-
-    >>> selftest = xmlrpclib.ServerProxy(
-    ...     'http://[e-mail]:[e-mail]/',
-    ...     transport=XMLRPCTestTransport())
-    >>> selftest.hello()
-    'Hello Sample Person.'
-
-The interactions in this test, and the interaction in the XMLRPC
-methods are different, so we still have an anonymous interaction in
-this test.
-
-    >>> getUtility(ILaunchBag).user is None
-    True
-
-Even if we log in as Foo Bar here, the XMLRPC method will see Sample
-Person as the logged in user.
-
-    >>> login('[e-mail]')
-    >>> selftest.hello()
-    'Hello Sample Person.'
-
-    >>> print getUtility(ILaunchBag).user.displayname
-    Foo Bar
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""XMLRPC self-test api.
+"""
+
+import xmlrpclib
+
+from zope.component import getUtility
+
+from lp.services.webapp.interfaces import ILaunchBag
+from lp.xmlrpc.application import SelfTest, ISelfTest
+from lp.services.webapp.testing import verifyObject
+from lp.testing import (
+    anonymous_logged_in,
+    TestCaseWithFactory,
+    person_logged_in,
+    )
+from lp.testing.layers import LaunchpadFunctionalLayer
+from lp.testing.xmlrpc import XMLRPCTestTransport
+
+
+class TestXMLRPCSelfTest(TestCaseWithFactory):
+
+    layer = LaunchpadFunctionalLayer
+
+    def make_proxy(self):
+        return xmlrpclib.ServerProxy(
+            'http://xmlrpc.launchpad.dev/', transport=XMLRPCTestTransport())
+
+    def make_logged_in_proxy(self):
+        return xmlrpclib.ServerProxy(
+            'http://[e-mail]:[e-mail]/',
+            transport=XMLRPCTestTransport())
+
+    def test_launchpad_root_object(self):
+        """The Launchpad root object has a simple XMLRPC API to show that
+        XMLRPC works.
+        """
+        selftestview = SelfTest('somecontext', 'somerequest')
+        self.assertTrue(verifyObject(ISelfTest, selftestview))
+        self.assertEqual(u'foo bar', selftestview.concatenate('foo', 'bar'))
+        fault = selftestview.make_fault()
+        self.assertEqual("<Fault 666: 'Yoghurt and spanners.'>", str(fault))
+
+    def test_custom_transport(self):
+        """We can test our XMLRPC APIs using xmlrpclib, using a custom
+        Transport which talks with the publisher directly.
+        """
+        selftest = self.make_proxy()
+        self.assertEqual('foo bar', selftest.concatenate('foo', 'bar'))
+        fault = self.assertRaises(xmlrpclib.Fault, selftest.make_fault)
+        self.assertEqual("<Fault 666: 'Yoghurt and spanners.'>", str(fault))
+
+    def test_unexpected_exception(self):
+        """Sometimes an XML-RPC method will be buggy, and raise an exception
+        other than xmlrpclib.Fault.  We have such a method on the self test
+        view.
+        """
+        selftestview = SelfTest('somecontext', 'somerequest')
+        self.assertRaises(RuntimeError, selftestview.raise_exception)
+
+    def test_exception_converted_to_fault(self):
+        """As with normal browser requests, we don't want to expose these error
+        messages to the user since they could contain confidential information.
+        Such exceptions get converted to a fault listing the OOPS ID (assuming
+        one was generated):
+        """
+        selftest = self.make_proxy()
+        e = self.assertRaises(xmlrpclib.Fault, selftest.raise_exception)
+        self.assertStartsWith(str(e), "<Fault -1: 'OOPS-")
+
+    def test_anonymous_authentication(self):
+        """hello() returns Anonymous because we haven't logged in."""
+        selftest = self.make_proxy()
+        self.assertEqual('Hello Anonymous.', selftest.hello())
+
+    def test_user_pass_authentication(self):
+        """If we provide a username and password, hello() will
+        include the name of the logged in user.
+
+        The interactions in this test, and the interaction in the XMLRPC
+        methods are different, so we still have an anonymous interaction in
+        this test.
+        """
+        with anonymous_logged_in():
+            self.assertIs(None, getUtility(ILaunchBag).user)
+            selftest = self.make_logged_in_proxy()
+            self.assertEqual('Hello Sample Person.', selftest.hello())
+
+    def test_login_differences(self):
+        """Even if we log in as Foo Bar here, the XMLRPC method will see Sample
+        Person as the logged in user.
+        """
+        person = self.factory.makePerson()
+        with person_logged_in(person):
+            selftest = self.make_logged_in_proxy()
+            self.assertEqual('Hello Sample Person.', selftest.hello())
+            self.assertEqual(person.displayname,
+                             getUtility(ILaunchBag).user.displayname)