Merge ~cjwatson/launchpad:python-openid2 into launchpad:master

Proposed by Colin Watson on 2020-07-22
Status: Merged
Approved by: Colin Watson on 2020-08-12
Approved revision: ac8b30ac3de0fcfc1f69b82926f76e77083655b5
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:python-openid2
Merge into: launchpad:master
Diff against target: 342 lines (+38/-53)
15 files modified
constraints.txt (+1/-4)
lib/launchpad_loggerhead/app.py (+1/-5)
lib/launchpad_loggerhead/wsgi.py (+0/-5)
lib/lp/scripts/utilities/test.py (+0/-18)
lib/lp/services/openid/browser/openiddiscovery.py (+4/-2)
lib/lp/services/openid/extensions/macaroon.py (+6/-2)
lib/lp/services/openid/extensions/tests/test_macaroon.py (+2/-0)
lib/lp/services/openid/fetcher.py (+13/-1)
lib/lp/services/webapp/login.py (+0/-4)
lib/lp/testopenid/browser/server.py (+1/-7)
lib/lp/testopenid/interfaces/server.py (+3/-1)
lib/lp/testopenid/stories/basics.txt (+1/-1)
lib/lp/testopenid/testing/helpers.py (+3/-2)
lib/lp_sitecustomize.py (+2/-0)
setup.py (+1/-1)
Reviewer Review Type Date Requested Status
Thiago F. Pappacena 2020-07-22 Approve on 2020-08-11
Review via email: mp+387907@code.launchpad.net

Commit message

Upgrade to python-openid2 3.2

Description of the change

This adds Python 3 support.

Our fork is no longer needed, because as of e6d54f6311 we force the OpenID fetcher to Urllib2Fetcher in loggerhead as well as in lp.services.webapp.login, and the fork was only ever needed to fix CurlFetcher.

I've removed several patches for problems with python-openid that have been fixed in python-openid2.

Dependencies MP: https://code.launchpad.net/~cjwatson/lp-source-dependencies/+git/lp-source-dependencies/+merge/387906

To post a comment you must log in.
~cjwatson/launchpad:python-openid2 updated on 2020-08-10
ac8b30a... by Colin Watson on 2020-08-10

Upgrade to python-openid2 3.2

This adds Python 3 support.

Our fork is no longer needed, because as of e6d54f6311 we force the
OpenID fetcher to Urllib2Fetcher in loggerhead as well as in
lp.services.webapp.login, and the fork was only ever needed to fix
CurlFetcher.

I've removed several patches for problems with python-openid that have
been fixed in python-openid2.

Thiago F. Pappacena (pappacena) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/constraints.txt b/constraints.txt
2index 706f7da..00eecfa 100644
3--- a/constraints.txt
4+++ b/constraints.txt
5@@ -278,10 +278,7 @@ python-dateutil==2.8.1
6 python-debian==0.1.32
7 python-keystoneclient==0.7.1
8 python-memcached==1.58
9-# XXX: deryck 2012-08-10
10-# See lp:~deryck/python-openid/python-openid-fix1034376 which
11-# reapplied a patch from wgrant to get codehosting going again.
12-python-openid==2.2.5-fix1034376
13+python-openid2==3.2
14 python-swiftclient==2.0.3
15 PyYAML==3.10
16 rabbitfixture==0.4.2
17diff --git a/lib/launchpad_loggerhead/app.py b/lib/launchpad_loggerhead/app.py
18index a671d73..d6a653b 100644
19--- a/lib/launchpad_loggerhead/app.py
20+++ b/lib/launchpad_loggerhead/app.py
21@@ -53,7 +53,6 @@ from lp.code.interfaces.codehosting import (
22 from lp.codehosting.vfs import get_lp_server
23 from lp.services.config import config
24 from lp.services.webapp.errorlog import ErrorReportingUtility
25-from lp.services.webapp.openid import set_default_openid_fetcher
26 from lp.services.webapp.vhosts import allvhosts
27 from lp.xmlrpc import faults
28
29@@ -69,9 +68,6 @@ robots_app = DataApp(robots_txt, content_type='text/plain')
30 thread_locals = threading.local()
31
32
33-set_default_openid_fetcher()
34-
35-
36 def check_fault(fault, *fault_classes):
37 """Check if 'fault's faultCode matches any of 'fault_classes'.
38
39@@ -124,7 +120,7 @@ class RootApp:
40 openid_request = self._make_consumer(environ).begin(
41 config.launchpad.openid_provider_root)
42 openid_request.addExtension(
43- SRegRequest(required=['nickname']))
44+ SRegRequest(required=[u'nickname']))
45 back_to = construct_url(environ)
46 raise HTTPMovedPermanently(openid_request.redirectURL(
47 config.codehosting.secure_codebrowse_root,
48diff --git a/lib/launchpad_loggerhead/wsgi.py b/lib/launchpad_loggerhead/wsgi.py
49index 6002448..be5601b 100644
50--- a/lib/launchpad_loggerhead/wsgi.py
51+++ b/lib/launchpad_loggerhead/wsgi.py
52@@ -19,7 +19,6 @@ import traceback
53
54 from gunicorn.app.base import Application
55 from gunicorn.glogging import Logger
56-from openid import oidutil
57 from paste.deploy.config import PrefixMiddleware
58 from paste.httpexceptions import HTTPExceptionHandler
59 from paste.request import construct_url
60@@ -89,10 +88,6 @@ class LoggerheadLogger(Logger):
61 ['-q', '--ms', '--log-file=DEBUG:%s' % cfg.errorlog])
62 logger(log_options)
63
64- # Make the OpenID library use proper logging rather than writing to
65- # stderr.
66- oidutil.log = lambda message, level=0: log.debug(message)
67-
68
69 def _on_starting_hook(arbiter):
70 # Normally lp.services.pidfile.make_pidfile does this, but in this case
71diff --git a/lib/lp/scripts/utilities/test.py b/lib/lp/scripts/utilities/test.py
72index 208c764..167d5c4 100755
73--- a/lib/lp/scripts/utilities/test.py
74+++ b/lib/lp/scripts/utilities/test.py
75@@ -106,24 +106,6 @@ def filter_warnings():
76 warnings.filterwarnings(
77 'ignore', 'bzrlib.*was deprecated', DeprecationWarning,
78 )
79- # The next one is caused by an infelicity in python-openid. The
80- # following change to openid/server/server.py would make the warning
81- # filter unnecessary:
82- # 978c974,974
83- # > try:
84- # > namespace = request.message.getOpenIDNamespace()
85- # > except AttributeError:
86- # > namespace = request.namespace
87- # > self.fields = Message(namespace)
88- # ---
89- # < self.fields = Message(request.namespace)
90- warnings.filterwarnings(
91- 'ignore',
92- (r'The \"namespace\" attribute of CheckIDRequest objects is '
93- r'deprecated.\s+'
94- r'Use \"message.getOpenIDNamespace\(\)\" instead'),
95- DeprecationWarning
96- )
97 # XXX cjwatson 2019-10-18: This can be dropped once the port to Breezy
98 # is complete.
99 warnings.filterwarnings(
100diff --git a/lib/lp/services/openid/browser/openiddiscovery.py b/lib/lp/services/openid/browser/openiddiscovery.py
101index 260f57c..2fe7023 100644
102--- a/lib/lp/services/openid/browser/openiddiscovery.py
103+++ b/lib/lp/services/openid/browser/openiddiscovery.py
104@@ -13,6 +13,7 @@ from openid.yadis.constants import (
105 YADIS_CONTENT_TYPE,
106 YADIS_HEADER_NAME,
107 )
108+import six
109
110 from lp.services.openid.adapters.openid import CurrentOpenIDEndPoint
111 from lp.services.propertycache import cachedproperty
112@@ -55,9 +56,10 @@ class XRDSContentNegotiationMixin:
113 # the value of the "Accept" header.
114 self.request.response.setHeader('Vary', 'Accept')
115
116- accept_content = self.request.get('HTTP_ACCEPT', '')
117+ accept_content = six.ensure_text(
118+ self.request.get('HTTP_ACCEPT', ''), encoding='ISO-8859-1')
119 acceptable = getAcceptable(accept_content,
120- ['text/html', YADIS_CONTENT_TYPE])
121+ [u'text/html', YADIS_CONTENT_TYPE])
122 # Return the XRDS document if it is preferred to text/html.
123 for mtype in acceptable:
124 if mtype == 'text/html':
125diff --git a/lib/lp/services/openid/extensions/macaroon.py b/lib/lp/services/openid/extensions/macaroon.py
126index 30e1680..97a1731 100644
127--- a/lib/lp/services/openid/extensions/macaroon.py
128+++ b/lib/lp/services/openid/extensions/macaroon.py
129@@ -39,7 +39,8 @@ __all__ = [
130 'MacaroonResponse',
131 ]
132
133-from openid import oidutil
134+import logging
135+
136 from openid.extension import Extension
137 from openid.message import (
138 NamespaceAliasRegistrationError,
139@@ -50,10 +51,13 @@ from openid.message import (
140 MACAROON_NS = 'http://ns.login.ubuntu.com/2016/openid-macaroon'
141
142
143+logger = logging.getLogger(__name__)
144+
145+
146 try:
147 registerNamespaceAlias(MACAROON_NS, 'macaroon')
148 except NamespaceAliasRegistrationError as e:
149- oidutil.log(
150+ logger.exception(
151 'registerNamespaceAlias(%r, %r) failed: %s' % (
152 MACAROON_NS, 'macaroon', e))
153
154diff --git a/lib/lp/services/openid/extensions/tests/test_macaroon.py b/lib/lp/services/openid/extensions/tests/test_macaroon.py
155index bcbc8a8..c87f6cc 100644
156--- a/lib/lp/services/openid/extensions/tests/test_macaroon.py
157+++ b/lib/lp/services/openid/extensions/tests/test_macaroon.py
158@@ -1,6 +1,8 @@
159 # Copyright 2016 Canonical Ltd. This software is licensed under the
160 # GNU Affero General Public License version 3 (see the file LICENSE).
161
162+from __future__ import absolute_import, print_function, unicode_literals
163+
164 __metaclass__ = type
165
166 from openid.consumer.consumer import SuccessResponse
167diff --git a/lib/lp/services/webapp/openid.py b/lib/lp/services/openid/fetcher.py
168similarity index 84%
169rename from lib/lp/services/webapp/openid.py
170rename to lib/lp/services/openid/fetcher.py
171index 5e3f34e..3575124 100644
172--- a/lib/lp/services/webapp/openid.py
173+++ b/lib/lp/services/openid/fetcher.py
174@@ -21,12 +21,24 @@ from openid.fetchers import (
175 from six.moves.urllib.request import urlopen
176
177 from lp.services.config import config
178+from lp.services.encoding import wsgi_native_string
179+
180+
181+class WSGIFriendlyUrllib2Fetcher(Urllib2Fetcher):
182+
183+ def fetch(self, url, body=None, headers=None):
184+ if headers is not None:
185+ headers = {
186+ wsgi_native_string(key): wsgi_native_string(value)
187+ for key, value in headers.items()}
188+ return super(WSGIFriendlyUrllib2Fetcher, self).fetch(
189+ url, body=body, headers=headers)
190
191
192 def set_default_openid_fetcher():
193 # Make sure we're using the same fetcher that we use in production, even
194 # if pycurl is installed.
195- fetcher = Urllib2Fetcher()
196+ fetcher = WSGIFriendlyUrllib2Fetcher()
197 if config.launchpad.enable_test_openid_provider:
198 # Tests have an instance name that looks like 'testrunner-appserver'
199 # or similar. We're in 'development' there, so just use that config.
200diff --git a/lib/lp/services/webapp/login.py b/lib/lp/services/webapp/login.py
201index fdbd079..5367047 100644
202--- a/lib/lp/services/webapp/login.py
203+++ b/lib/lp/services/webapp/login.py
204@@ -66,7 +66,6 @@ from lp.services.webapp.interfaces import (
205 IPlacelessLoginSource,
206 LoggedOutEvent,
207 )
208-from lp.services.webapp.openid import set_default_openid_fetcher
209 from lp.services.webapp.publisher import LaunchpadView
210 from lp.services.webapp.url import urlappend
211 from lp.services.webapp.vhosts import allvhosts
212@@ -160,9 +159,6 @@ def register_basiclogin(event):
213 name='+basiclogin')
214
215
216-set_default_openid_fetcher()
217-
218-
219 class OpenIDLogin(LaunchpadView):
220 """A view which initiates the OpenID handshake with our provider."""
221 _openid_session_ns = 'OPENID'
222diff --git a/lib/lp/testopenid/browser/server.py b/lib/lp/testopenid/browser/server.py
223index c3d540a..f2ce757 100644
224--- a/lib/lp/testopenid/browser/server.py
225+++ b/lib/lp/testopenid/browser/server.py
226@@ -15,7 +15,6 @@ __all__ = [
227
228 from datetime import timedelta
229
230-from openid import oidutil
231 from openid.extensions.sreg import (
232 SRegRequest,
233 SRegResponse,
234@@ -78,10 +77,6 @@ SESSION_PKG_KEY = 'TestOpenID'
235 openid_store = MemoryStore()
236
237
238-# Shut up noisy OpenID library
239-oidutil.log = lambda message, level=0: None
240-
241-
242 @implementer(ICanonicalUrlData)
243 class TestOpenIDRootUrlData:
244 """`ICanonicalUrlData` for the test OpenID provider."""
245@@ -151,8 +146,7 @@ class OpenIDMixin:
246 query = {}
247 for key, value in self.request.form.items():
248 if key.startswith('openid.'):
249- # All OpenID query args are supposed to be ASCII.
250- query[key.encode('US-ASCII')] = value.encode('US-ASCII')
251+ query[key] = value
252 return query
253
254 def getSession(self):
255diff --git a/lib/lp/testopenid/interfaces/server.py b/lib/lp/testopenid/interfaces/server.py
256index 434fee3..669476f 100644
257--- a/lib/lp/testopenid/interfaces/server.py
258+++ b/lib/lp/testopenid/interfaces/server.py
259@@ -9,6 +9,7 @@ __all__ = [
260 'ITestOpenIDPersistentIdentity',
261 ]
262
263+import six
264 from zope.interface import Interface
265 from zope.schema import TextLine
266
267@@ -36,4 +37,5 @@ def get_server_url():
268 This is wrapped in a function (instead of a constant) to make sure the
269 vhost.testopenid section is not required in production configs.
270 """
271- return urlappend(allvhosts.configs['testopenid'].rooturl, '+openid')
272+ return six.ensure_text(
273+ urlappend(allvhosts.configs['testopenid'].rooturl, '+openid'))
274diff --git a/lib/lp/testopenid/stories/basics.txt b/lib/lp/testopenid/stories/basics.txt
275index 56c6f4e..6672e40 100644
276--- a/lib/lp/testopenid/stories/basics.txt
277+++ b/lib/lp/testopenid/stories/basics.txt
278@@ -37,7 +37,7 @@ POST request.
279 >>> print anon_browser.headers
280 Status: 200 Ok
281 ...
282- Content-Type: text/plain
283+ Content-Type: text/plain;charset=utf-8
284 ...
285 >>> print anon_browser.contents
286 assoc_handle:{HMAC-SHA1}{...}{...}
287diff --git a/lib/lp/testopenid/testing/helpers.py b/lib/lp/testopenid/testing/helpers.py
288index 27f9855..99cd52d 100644
289--- a/lib/lp/testopenid/testing/helpers.py
290+++ b/lib/lp/testopenid/testing/helpers.py
291@@ -21,6 +21,7 @@ from openid.consumer.discover import (
292 from six.moves.urllib.error import HTTPError
293 from zope.testbrowser.wsgi import Browser
294
295+from lp.services.encoding import wsgi_native_string
296 from lp.services.webapp import LaunchpadView
297 from lp.testopenid.interfaces.server import get_server_url
298
299@@ -44,8 +45,8 @@ class ZopeFetcher(fetchers.HTTPFetcher):
300 browser = Browser()
301 if headers is not None:
302 for key, value in headers.items():
303- browser.addHeader(key, value)
304- browser.addHeader('X-Zope-Handle-Errors', 'True')
305+ browser.addHeader(key, wsgi_native_string(value))
306+ browser.addHeader('X-Zope-Handle-Errors', wsgi_native_string('True'))
307 try:
308 browser.open(url, data=body)
309 except HTTPError as e:
310diff --git a/lib/lp_sitecustomize.py b/lib/lp_sitecustomize.py
311index d23cd74..26085c0 100644
312--- a/lib/lp_sitecustomize.py
313+++ b/lib/lp_sitecustomize.py
314@@ -21,6 +21,7 @@ from lp.services.log import loglevels
315 from lp.services.log.logger import LaunchpadLogger
316 from lp.services.log.mappingfilter import MappingFilter
317 from lp.services.mime import customizeMimetypes
318+from lp.services.openid.fetcher import set_default_openid_fetcher
319
320
321 def add_custom_loglevels():
322@@ -178,6 +179,7 @@ def main(instance_name=None):
323 customizeMimetypes()
324 silence_warnings()
325 customize_logger()
326+ set_default_openid_fetcher()
327 checker.BasicTypes.update({defaultdict: checker.NoProxy})
328 checker.BasicTypes.update({Deferred: checker.NoProxy})
329 checker.BasicTypes.update({DeferredList: checker.NoProxy})
330diff --git a/setup.py b/setup.py
331index b724650..db7963e 100644
332--- a/setup.py
333+++ b/setup.py
334@@ -216,7 +216,7 @@ setup(
335 'python-keystoneclient',
336 'python-memcached',
337 'python-mimeparse',
338- 'python-openid',
339+ 'python-openid2',
340 'python-subunit',
341 'python-swiftclient',
342 'pytz',

Subscribers

People subscribed via source and target branches