Merge lp:~leonardr/lazr.restfulclient/system-wide-consumer into lp:lazr.restfulclient

Proposed by Leonard Richardson
Status: Merged
Merged at revision: 110
Proposed branch: lp:~leonardr/lazr.restfulclient/system-wide-consumer
Merge into: lp:lazr.restfulclient
Diff against target: 217 lines (+157/-1)
3 files modified
src/lazr/restfulclient/NEWS.txt (+5/-1)
src/lazr/restfulclient/authorize/oauth.py (+46/-0)
src/lazr/restfulclient/tests/test_oauth.py (+106/-0)
To merge this branch: bzr merge lp:~leonardr/lazr.restfulclient/system-wide-consumer
Reviewer Review Type Date Requested Status
Edwin Grubbs (community) code Approve
Review via email: mp+39552@code.launchpad.net

Description of the change

This branch introduces the SystemWideConsumer class. With a normal OAuth consumer, you specify the consumer key (the name of your application). With the SystemWideConsumer, the consumer key is automatically derived from your platform (eg. "Ubuntu") and the hostname of your computer. You still specify an "application name", but it's only present in the User-Agent. It's not used in OAuth.

The purpose of this mighty consumer is to integrate an entire desktop with a web service, rather than having to integrate each application individually. The KEY_FORMAT happens to be the format that Launchpad recognizes as a system-wide consumer key, but the code is generic--it (or a simple subclass) could work for any lazr.restful service, whether or not it specifically recognizes system-wide keys.

If KEY_FORMAT bothers you because of its Launchpad-specificness I could make it a constructor argument and move KEY_FORMAT itself to launchpadlib.

To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Leonard,

This is a nice branch. I just have one concern listed below.

-Edwin

>=== modified file 'src/lazr/restfulclient/authorize/oauth.py'
>--- src/lazr/restfulclient/authorize/oauth.py 2010-10-28 15:49:09 +0000
>+++ src/lazr/restfulclient/authorize/oauth.py 2010-10-28 20:57:14 +0000
>@@ -73,6 +76,49 @@
> self.context = context
>
>
>+class SystemWideConsumer(Consumer):
>+ """A consumer associated with the logged-in user rather than an app.
>+
>+ This can be used to share a single OAuth token among multiple
>+ desktop applications. The OAuth consumer key will be derived from
>+ system information (platform and hostname).
>+ """
>+ KEY_FORMAT = "System-wide: %s (%s)"
>+
>+ def __init__(self, application_name, secret=''):
>+ """Constructor.
>+
>+ :param application_name: An application name. This will be
>+ used in the User-Agent header.
>+ :param secret: The OAuth consumer secret. Don't use this. It's
>+ a misfeature, and lazr.restful doesn't expect it.
>+ """
>+ super(SystemWideConsumer, self).__init__(
>+ self.consumer_key, secret, application_name)
>+
>+ @property
>+ def consumer_key(self):
>+ """The system-wide OAuth consumer key for this computer.
>+
>+ This key identifies the platform and the computer's
>+ hostname. It does not identify the active user.
>+ """
>+ try:
>+ distname, version, release_id = platform.linux_distribution()
>+ except Exception, e:
>+ # This can happen in pre-2.6 versions of Python.
>+ try:
>+ distname, version, release_id = platform.dist()

There appears to be an annoying bug in some versions of python where platform.dist() will randomly get its data from either /etc/debian_version or /etc/lsb-release. Since lsb-release has the more accurate info, it might be worthwhile to use "/usr/bin/lsb_release -si" when that executable exists.

http://bugs.python.org/issue9514

>+ except Exception, e:
>+ # This should never happen--non-Linux platforms return
>+ # empty strings from linux_distribution() or
>+ # dist()--but just in case.
>+ distname = ''
>+ if distname == '':
>+ distname = platform.system() # (eg. "Windows")
>+ return self.KEY_FORMAT % (distname, socket.gethostname())
>+
>+

review: Approve (code)
Revision history for this message
Leonard Richardson (leonardr) wrote :

I've gotten rid of the use of dist(). We'll now fall back directly to system().

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lazr/restfulclient/NEWS.txt'
2--- src/lazr/restfulclient/NEWS.txt 2010-10-27 14:18:31 +0000
3+++ src/lazr/restfulclient/NEWS.txt 2010-10-28 17:46:02 +0000
4@@ -2,13 +2,17 @@
5 NEWS for lazr.restfulclient
6 ===========================
7
8-0.11.0 (2010-10-27)
9+0.11.0 (2010-10-28)
10 ===================
11
12 - Make it possibly to specify an "application name" separate from the
13 OAuth consumer key. If present, the application name is used in the
14 User-Agent header; otherwise, the OAuth consumer key is used.
15
16+ - Add a "system-wide consumer" which can be used to authorize a
17+ user's entire account to use a web service, rather than doing it
18+ one application at a time.
19+
20 0.10.0 (2010-08-12)
21 ===================
22
23
24=== modified file 'src/lazr/restfulclient/authorize/oauth.py'
25--- src/lazr/restfulclient/authorize/oauth.py 2010-10-28 15:49:09 +0000
26+++ src/lazr/restfulclient/authorize/oauth.py 2010-10-28 17:46:02 +0000
27@@ -21,7 +21,9 @@
28
29 from ConfigParser import SafeConfigParser
30 import os
31+import platform
32 import stat
33+import socket
34 # Work around relative import behavior. The below is equivalent to
35 # from oauth import oauth
36 oauth = __import__('oauth.oauth', {}).oauth
37@@ -37,6 +39,7 @@
38 'AccessToken',
39 'Consumer',
40 'OAuthAuthorizer',
41+ 'SystemWideConsumer',
42 ]
43
44
45@@ -73,6 +76,49 @@
46 self.context = context
47
48
49+class SystemWideConsumer(Consumer):
50+ """A consumer associated with the logged-in user rather than an app.
51+
52+ This can be used to share a single OAuth token among multiple
53+ desktop applications. The OAuth consumer key will be derived from
54+ system information (platform and hostname).
55+ """
56+ KEY_FORMAT = "System-wide: %s (%s)"
57+
58+ def __init__(self, application_name, secret=''):
59+ """Constructor.
60+
61+ :param application_name: An application name. This will be
62+ used in the User-Agent header.
63+ :param secret: The OAuth consumer secret. Don't use this. It's
64+ a misfeature, and lazr.restful doesn't expect it.
65+ """
66+ super(SystemWideConsumer, self).__init__(
67+ self.consumer_key, secret, application_name)
68+
69+ @property
70+ def consumer_key(self):
71+ """The system-wide OAuth consumer key for this computer.
72+
73+ This key identifies the platform and the computer's
74+ hostname. It does not identify the active user.
75+ """
76+ try:
77+ distname, version, release_id = platform.linux_distribution()
78+ except Exception, e:
79+ # This can happen in pre-2.6 versions of Python.
80+ try:
81+ distname, version, release_id = platform.dist()
82+ except Exception, e:
83+ # This should never happen--non-Linux platforms return
84+ # empty strings from linux_distribution() or
85+ # dist()--but just in case.
86+ distname = ''
87+ if distname == '':
88+ distname = platform.system() # (eg. "Windows")
89+ return self.KEY_FORMAT % (distname, socket.gethostname())
90+
91+
92 class OAuthAuthorizer(HttpAuthorizer):
93 """A client that signs every outgoing request with OAuth credentials."""
94
95
96=== modified file 'src/lazr/restfulclient/tests/test_oauth.py'
97--- src/lazr/restfulclient/tests/test_oauth.py 2010-10-28 15:49:09 +0000
98+++ src/lazr/restfulclient/tests/test_oauth.py 2010-10-28 17:46:02 +0000
99@@ -27,6 +27,7 @@
100 import tempfile
101 import unittest
102
103+from lazr.restfulclient.authorize import oauth
104 from lazr.restfulclient.authorize.oauth import (
105 AccessToken,
106 Consumer,
107@@ -47,6 +48,111 @@
108 consumer = Consumer("key", "secret")
109 self.assertEquals(consumer.application_name, None)
110
111+
112+class TestSystemWideConsumer(unittest.TestCase):
113+
114+ def setUp(self):
115+ """Save the original 'platform' and 'socket' modules.
116+
117+ The tests will be replacing them with dummies.
118+ """
119+ self.original_platform = oauth.platform
120+ self.original_socket = oauth.socket
121+
122+ def tearDown(self):
123+ """Replace the original 'platform' and 'socket' modules."""
124+ oauth.platform = self.original_platform
125+ oauth.socket = self.original_socket
126+
127+ def _set_hostname(self, hostname):
128+ """Changes the socket module to simulate the given hostname."""
129+ class DummySocket:
130+ def gethostname(self):
131+ return hostname
132+ oauth.socket = DummySocket()
133+
134+ def _set_platform(self, linux_distribution, system, dist=None):
135+ """Changes the platform module to simulate different behavior.
136+
137+ :param linux_distribution: A tuple to be returned by
138+ linux_distribution(), or a callable that implements
139+ linux_distribution().
140+ :param system: A string to be returned by system()
141+ :param dist: A callable that implements dist(). If this is
142+ None, dist() will behave exactly the same as
143+ linux_distribution().
144+ """
145+
146+ if isinstance(linux_distribution, tuple):
147+ def get_linux_distribution(self):
148+ return linux_distribution
149+ else:
150+ # The caller provided their own implementation of
151+ # linux_distribution().
152+ get_linux_distribution = linux_distribution
153+
154+ if dist is None:
155+ # The caller declined to provide an implementation of dist().
156+ # Make it act like linux_distribution().
157+ get_dist = linux_distribution
158+ else:
159+ get_dist = dist
160+
161+ class DummyPlatform:
162+ linux_distribution = get_linux_distribution
163+ dist = get_dist
164+ def system(self):
165+ return system
166+ oauth.platform = DummyPlatform()
167+
168+ def _broken(self):
169+ """Raises an exception."""
170+ raise Exception("Oh noes!")
171+
172+ def test_useful_linux_distribution(self):
173+ # If platform.linux_distribution returns a tuple of useful
174+ # strings, as it does on Ubuntu, we'll use the first string
175+ # for the system type.
176+ self._set_platform(('Fooix', 'String2', 'String3'), 'FooOS')
177+ self._set_hostname("foo")
178+ consumer = oauth.SystemWideConsumer("app name")
179+ self.assertEquals(
180+ consumer.key, 'System-wide: Fooix (foo)')
181+
182+ def test_empty_linux_distribution(self):
183+ # If platform.linux_distribution returns a tuple of empty
184+ # strings, as it does on Windows and Mac OS X, we fall back to
185+ # the result of platform.system().
186+ self._set_platform(('', '', ''), 'BarOS')
187+ self._set_hostname("bar")
188+ consumer = oauth.SystemWideConsumer("app name")
189+ self.assertEquals(
190+ consumer.key, 'System-wide: BarOS (bar)')
191+
192+ def test_broken_linux_distribution(self):
193+ # If platform.linux_distribution raises an exception (which
194+ # can happen with older versions of Python), we fall back to
195+ # the result of platform.dist().
196+ def dist(self):
197+ return ('Bazix', 'String2', 'String3')
198+ self._set_platform(self._broken, 'BazOS', dist)
199+ self._set_hostname("baz")
200+ consumer = oauth.SystemWideConsumer("app name")
201+ self.assertEquals(
202+ consumer.key, 'System-wide: Bazix (baz)')
203+
204+ def test_broken_linux_distribution_and_dist(self):
205+ # If both platform.linux_distribution and platform.dist raise
206+ # exceptions (which should never actually happen)
207+ # we fall back to the result of platform.system().
208+ self._set_platform(self._broken, 'QuuxOS', self._broken)
209+ self._set_hostname("quux")
210+ consumer = oauth.SystemWideConsumer("app name")
211+ self.assertEquals(
212+ consumer.key, 'System-wide: QuuxOS (quux)')
213+
214+
215+
216 class TestOAuthAuthorizer(unittest.TestCase):
217 """Test for the OAuth Authorizer."""
218

Subscribers

People subscribed via source and target branches