launchpadlib

Merge lp:~leonardr/launchpadlib/choose-your-client into lp:launchpadlib

  1. choose-your-client
  2. Merge into trunk
Proposed by Leonard Richardson
Status: Merged
Merged at revision: not available
Proposed branch: lp:~leonardr/launchpadlib/choose-your-client
Merge into: lp:launchpadlib
Diff against target: 391 lines
8 files modified
setup.py (+1/-1)
src/launchpadlib/credentials.py (+44/-3)
src/launchpadlib/docs/command-line.txt (+1/-1)
src/launchpadlib/docs/hosted-files.txt (+1/-1)
src/launchpadlib/docs/introduction.txt (+78/-13)
src/launchpadlib/docs/people.txt (+1/-0)
src/launchpadlib/launchpad.py (+19/-12)
src/launchpadlib/testing/helpers.py (+19/-3)
To merge this branch: bzr merge lp:~leonardr/launchpadlib/choose-your-client
Reviewer Review Type Date Requested Status
Graham Binns (community) Approve
Review via email: mp+14361@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Leonard Richardson (leonardr) wrote :

Previous branches added the RequestTokenAuthorizationEngine, a generic way of getting the end-user to enter their Launchpad username and password to authorize a request token. I also added the TrustedTokenAuthorizationConsoleApp, a RequestTokenAuthorizationEngine that guides the user through the process in the console. But if you wanted to use convenience methods like login_with(), you still had to use the default authorization workflow which opens the user's web browser and tells them to hit enter when they're done.

Because the convenience methods have been refactored to no longer require direct user interaction, it's now possible to test login_with() and get_token_and_login() in a doctest. I've added tests for these methods that use a dummy subclass of AuthorizeRequestTokenWithBrowser. Now the only part of launchpadlib that requires manual testing is the AuthorizeRequestTokenWithBrowser class itself. (And the command-line scripts, but those are very small and just pass their arguments into a tested App class, and I didn't change those in this branch anyway.)

I also upgraded the dependency to the newest version of lazr.restfulclient. Previously, login_with() didn't work with an 0.9.9 lazr.restfulclient, but it didn't cause any tests to fail because login_with() wasn't tested. Now it is tested, so you gotta have 0.9.10. The changes to the tests that add "..." to exception tracebacks are necessary because 0.9.10 tracebacks contain a lot more information.

I'm not really happy with the current design. When testing this code I found it clunky to pass in a RequestTokenAuthorizationEngine class that got instantiated within get_token_and_login(). It would be a lot easier if I could instantiate the object first and then pass it in. The problem is that get_token_and_login() is the code that gets the request token, and the RTAE constructor requires the request token. I'm considering changing RTAE so that it can get the request token itself. However, that goes against my general rule of keeping RTAE as simple as possible so that it's easy to audit.

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

I forgot to explicitly mention the good thing about this branch: the browser-based authorization workflow is now just the default. You can pass any RTAE class into login_with(), and launchpadlib will use that workflow to get the end-user's Launchpad username and password and authorize the request token.

> Previous branches added the RequestTokenAuthorizationEngine, a generic way of
> getting the end-user to enter their Launchpad username and password to
> authorize a request token. I also added the
> TrustedTokenAuthorizationConsoleApp, a RequestTokenAuthorizationEngine that
> guides the user through the process in the console. But if you wanted to use
> convenience methods like login_with(), you still had to use the default
> authorization workflow which opens the user's web browser and tells them to
> hit enter when they're done.
>
> Because the convenience methods have been refactored to no longer require
> direct user interaction, it's now possible to test login_with() and
> get_token_and_login() in a doctest. I've added tests for these methods that
> use a dummy subclass of AuthorizeRequestTokenWithBrowser. Now the only part of
> launchpadlib that requires manual testing is the
> AuthorizeRequestTokenWithBrowser class itself. (And the command-line scripts,
> but those are very small and just pass their arguments into a tested App
> class, and I didn't change those in this branch anyway.)
>
> I also upgraded the dependency to the newest version of lazr.restfulclient.
> Previously, login_with() didn't work with an 0.9.9 lazr.restfulclient, but it
> didn't cause any tests to fail because login_with() wasn't tested. Now it is
> tested, so you gotta have 0.9.10. The changes to the tests that add "..." to
> exception tracebacks are necessary because 0.9.10 tracebacks contain a lot
> more information.
>
> I'm not really happy with the current design. When testing this code I found
> it clunky to pass in a RequestTokenAuthorizationEngine class that got
> instantiated within get_token_and_login(). It would be a lot easier if I could
> instantiate the object first and then pass it in. The problem is that
> get_token_and_login() is the code that gets the request token, and the RTAE
> constructor requires the request token. I'm considering changing RTAE so that
> it can get the request token itself. However, that goes against my general
> rule of keeping RTAE as simple as possible so that it's easy to audit.

Revision history for this message
Graham Binns (gmb) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'setup.py'
2--- setup.py 2009-10-27 12:00:13 +0000
3+++ setup.py 2009-11-03 14:20:26 +0000
4@@ -60,7 +60,7 @@
5 license='LGPL v3',
6 install_requires=[
7 'httplib2',
8- 'lazr.restfulclient>=0.9.9',
9+ 'lazr.restfulclient>=0.9.10',
10 'lazr.uri',
11 'oauth',
12 'setuptools',
13
14=== modified file 'src/launchpadlib/credentials.py'
15--- src/launchpadlib/credentials.py 2009-10-30 19:19:07 +0000
16+++ src/launchpadlib/credentials.py 2009-11-03 14:20:26 +0000
17@@ -27,6 +27,7 @@
18 import base64
19 import cgi
20 import httplib2
21+import sys
22 import textwrap
23 from urllib import urlencode, quote
24 from urlparse import urljoin
25@@ -379,7 +380,7 @@
26 username = self.input_username(
27 cached_username, self.message(self.INPUT_USERNAME))
28 if username is None:
29- self.open_login_page_in_user_browser(
30+ self.open_page_in_user_browser(
31 urljoin(self.web_root, "+login"))
32 raise NoLaunchpadAccount(
33 self.message(self.YOU_NEED_A_LAUNCHPAD_ACCOUNT))
34@@ -423,8 +424,8 @@
35 variables.update(extra_variables)
36 return raw_message % variables
37
38- def open_login_page_in_user_browser(self, url):
39- """Open the Launchpad login page in the user's web browser."""
40+ def open_page_in_user_browser(self, url):
41+ """Open a web page in the user's web browser."""
42 webbrowser.open(url)
43
44 # You should define these methods in your subclass.
45@@ -488,20 +489,60 @@
46 self.output(suggested_message)
47
48
49+class AuthorizeRequestTokenWithBrowser(RequestTokenAuthorizationEngine):
50+ """The simplest and most secure request token authorizer.
51+
52+ This authorizer simply opens up the end-user's web browser to a
53+ Launchpad URL and lets the end-user authorize the request token
54+ themselves.
55+ """
56+
57+ def __init__(self, web_root, consumer_name, request_token,
58+ allow_access_levels=[], max_failed_attempts=3):
59+ page = "+authorize-token?oauth_token=%s" % request_token
60+ if len(allow_access_levels) > 0:
61+ page += ("&allow_permission=" +
62+ "&allow_permission=".join(allow_access_levels))
63+ self.authorization_url = urljoin(web_root, page)
64+
65+ super(AuthorizeRequestTokenWithBrowser, self).__init__(
66+ web_root, consumer_name, request_token,
67+ allow_access_levels, max_failed_attempts)
68+
69+ def __call__(self):
70+ self.open_page_in_user_browser(self.authorization_url)
71+ print "The authorization page:"
72+ print " (%s)" % self.authorization_url
73+ print "should be opening in your browser. After you have authorized"
74+ print "this program to access Launchpad on your behalf you should come"
75+ print ("back here and press <Enter> to finish the authentication "
76+ "process.")
77+ self.wait_for_request_token_authorization()
78+
79+ def wait_for_request_token_authorization(self):
80+ """Get the end-user to hit enter."""
81+ sys.stdin.readline()
82+
83+
84 class TokenAuthorizationException(Exception):
85 pass
86
87+
88 class RequestTokenAlreadyAuthorized(TokenAuthorizationException):
89 pass
90
91+
92 class ClientError(TokenAuthorizationException):
93 pass
94
95+
96 class ServerError(TokenAuthorizationException):
97 pass
98
99+
100 class NoLaunchpadAccount(TokenAuthorizationException):
101 pass
102
103+
104 class TooManyAuthenticationFailures(TokenAuthorizationException):
105 pass
106
107=== modified file 'src/launchpadlib/docs/command-line.txt'
108--- src/launchpadlib/docs/command-line.txt 2009-11-02 12:20:47 +0000
109+++ src/launchpadlib/docs/command-line.txt 2009-11-03 14:20:26 +0000
110@@ -60,7 +60,7 @@
111 performing such un-doctest-like actions.
112
113 >>> class ConsoleApp(TrustedTokenAuthorizationConsoleApp):
114- ... def open_login_page_in_user_browser(self, url):
115+ ... def open_page_in_user_browser(self, url):
116 ... """Print a status message."""
117 ... self.output("[If this were a real application, the "
118 ... "end-user's web browser would be opened "
119
120=== modified file 'src/launchpadlib/docs/hosted-files.txt'
121--- src/launchpadlib/docs/hosted-files.txt 2009-07-09 13:27:28 +0000
122+++ src/launchpadlib/docs/hosted-files.txt 2009-11-03 14:20:26 +0000
123@@ -56,7 +56,7 @@
124 Traceback (most recent call last):
125 ...
126 HTTPError: HTTP Error 400: Bad Request
127-
128+ ...
129
130 == Caching ==
131
132
133=== modified file 'src/launchpadlib/docs/introduction.txt'
134--- src/launchpadlib/docs/introduction.txt 2009-10-27 15:24:36 +0000
135+++ src/launchpadlib/docs/introduction.txt 2009-11-03 14:20:26 +0000
136@@ -213,18 +213,81 @@
137 >>> credentials.access_token.context
138 'firefox'
139
140-There's also a convenience method which does the access token
141-negotiation and logs into the web service. It uses the methods
142-documented above and once it has the request token's authorization URL
143-it opens up a web browser for the user to authoriza it and asks him to
144-come back and press <Enter> once that's done. When he does it, the
145-request token is exchanged for an access token and the authentication is
146-complete.
147-
148- # Since this will open up a web browser we're not going to actually run it
149- # here.
150- >>> # consumer_name = 'launchpadlib'
151- >>> # launchpad = Launchpad.get_token_and_login(consumer_name)
152+Authorizing the request token
153+-----------------------------
154+
155+There are also two convenience method which do the access token
156+negotiation and log into the web service: get_token_and_login() and
157+login_with(). These convenience methods use the methods documented
158+above to get a request token, and once it has the request token's
159+authorization information, it makes the end-user authorize the request
160+token by entering their Launchpad username and password.
161+
162+There are several ways of having the end-user authorize a request
163+token, but the most secure is to open up the user's own web browser
164+(other ways are described in trusted-client.txt). Because we don't
165+want to actually open a web browser during this test, we'll create a
166+fake authorizer that uses the SimulatedLaunchpadBrowser to authorize
167+the request token.
168+
169+ >>> from launchpadlib.testing.helpers import (
170+ ... DummyAuthorizeRequestTokenWithBrowser)
171+
172+ >>> class AuthorizeAsSalgado(DummyAuthorizeRequestTokenWithBrowser):
173+ ... def wait_for_request_token_authorization(self):
174+ ... """Simulate the authorizing user with their web browser."""
175+ ... username = 'salgado@ubuntu.com'
176+ ... password = 'zeca'
177+ ... browser = SimulatedLaunchpadBrowser(self.web_root)
178+ ... browser.grant_access(username, password, self.request_token,
179+ ... 'READ_PUBLIC')
180+
181+ >>> consumer_name = 'launchpadlib'
182+ >>> launchpad = Launchpad.get_token_and_login(
183+ ... consumer_name, service_root="http://api.launchpad.dev:8085/beta/",
184+ ... authorizer_class=AuthorizeAsSalgado)
185+ [If this were a real application, the end-user's web browser would
186+ be opened to http://launchpad.dev:8085/+authorize-token?oauth_token=...]
187+ The authorization page:
188+ (http://launchpad.dev:8085/+authorize-token?oauth_token=...)
189+ should be opening in your browser. After you have authorized
190+ this program to access Launchpad on your behalf you should come
191+ back here and press <Enter> to finish the authentication process.
192+
193+The login_with method will cache an access token once it gets one, so
194+that the end-user doesn't have to authorize a request token every time
195+they run the program.
196+
197+ >>> import tempfile
198+ >>> cache_dir = tempfile.mkdtemp()
199+ >>> launchpad = Launchpad.login_with(
200+ ... consumer_name, service_root="http://api.launchpad.dev:8085/beta/",
201+ ... launchpadlib_dir=cache_dir,
202+ ... authorizer_class=AuthorizeAsSalgado)
203+ [If this were a real application...]
204+ The authorization page:
205+ ...
206+ >>> print launchpad.me.name
207+ salgado
208+
209+Now that the access token is authorized, we can call login_with()
210+again and pass in a null authorizer. If there was no access token,
211+this would fail, because there would be no way to authorize the
212+request token. But since there's an access token cached in the
213+cache directory, login_with() will succeed without even trying to
214+authorize a request token.
215+
216+ >>> launchpad = Launchpad.login_with(
217+ ... consumer_name, service_root="http://api.launchpad.dev:8085/beta/",
218+ ... launchpadlib_dir=cache_dir,
219+ ... authorizer_class=None)
220+ >>> print launchpad.me.name
221+ salgado
222+
223+A bit of clean-up: removing the cache directory.
224+
225+ >>> import shutil
226+ >>> shutil.rmtree(cache_dir)
227
228 The dictionary request token
229 ============================
230@@ -347,6 +410,7 @@
231 Traceback (most recent call last):
232 ...
233 HTTPError: HTTP Error 401: Unauthorized
234+ ...
235
236 The application is not allowed to access Launchpad with a bad access token.
237
238@@ -358,6 +422,7 @@
239 Traceback (most recent call last):
240 ...
241 HTTPError: HTTP Error 401: Unauthorized
242+ ...
243
244 The application is not allowed to access Launchpad with a bad access secret.
245
246@@ -369,7 +434,7 @@
247 Traceback (most recent call last):
248 ...
249 HTTPError: HTTP Error 401: Unauthorized
250-
251+ ...
252
253 Clean up
254 ========
255
256=== modified file 'src/launchpadlib/docs/people.txt'
257--- src/launchpadlib/docs/people.txt 2009-04-16 19:30:01 +0000
258+++ src/launchpadlib/docs/people.txt 2009-11-03 14:20:26 +0000
259@@ -160,6 +160,7 @@
260 Traceback (most recent call last):
261 ...
262 HTTPError: HTTP Error 400: Bad Request
263+ ...
264
265 Actually, the exception contains other useful information.
266
267
268=== modified file 'src/launchpadlib/launchpad.py'
269--- src/launchpadlib/launchpad.py 2009-10-27 12:00:13 +0000
270+++ src/launchpadlib/launchpad.py 2009-11-03 14:20:26 +0000
271@@ -31,7 +31,8 @@
272 from lazr.restfulclient._browser import RestfulHttp
273 from lazr.restfulclient.resource import (
274 CollectionWithKeyBasedLookup, HostedFile, ServiceRoot)
275-from launchpadlib.credentials import AccessToken, Credentials
276+from launchpadlib.credentials import (
277+ AccessToken, Credentials, AuthorizeRequestTokenWithBrowser)
278 from oauth.oauth import OAuthRequest, OAuthSignatureMethod_PLAINTEXT
279 from launchpadlib import uris
280
281@@ -127,7 +128,9 @@
282 @classmethod
283 def get_token_and_login(cls, consumer_name,
284 service_root=uris.STAGING_SERVICE_ROOT,
285- cache=None, timeout=None, proxy_info=None):
286+ cache=None, timeout=None, proxy_info=None,
287+ authorizer_class=AuthorizeRequestTokenWithBrowser,
288+ allow_access_levels=[], max_failed_attempts=3):
289 """Get credentials from Launchpad and log into the service root.
290
291 This is a convenience method which will open up the user's preferred
292@@ -156,21 +159,22 @@
293 web_root_uri.path = ""
294 web_root_uri.host = web_root_uri.host.replace("api.", "", 1)
295 web_root = str(web_root_uri.ensureSlash())
296- authorization_url = credentials.get_request_token(web_root=web_root)
297- webbrowser.open(authorization_url)
298- print "The authorization page:"
299- print " (%s)" % authorization_url
300- print "should be opening in your browser. After you have authorized"
301- print "this program to access Launchpad on your behalf you should come"
302- print "back here and press <Enter> to finish the authentication process."
303- sys.stdin.readline()
304+ authorization_json = credentials.get_request_token(
305+ web_root=web_root, token_format=Credentials.DICT_TOKEN_FORMAT)
306+ authorizer = authorizer_class(
307+ web_root, authorization_json['oauth_token_consumer'],
308+ authorization_json['oauth_token'], allow_access_levels,
309+ max_failed_attempts)
310+ authorizer()
311 credentials.exchange_request_token_for_access_token(web_root)
312 return cls(credentials, service_root, cache, timeout, proxy_info)
313
314 @classmethod
315 def login_with(cls, consumer_name,
316 service_root=uris.STAGING_SERVICE_ROOT,
317- launchpadlib_dir=None, timeout=None, proxy_info=None):
318+ launchpadlib_dir=None, timeout=None, proxy_info=None,
319+ authorizer_class=AuthorizeRequestTokenWithBrowser,
320+ allow_access_levels=[], max_failed_attempts=3):
321 """Log in to Launchpad with possibly cached credentials.
322
323 This is a convenience method for either setting up new login
324@@ -229,7 +233,10 @@
325 else:
326 launchpad = cls.get_token_and_login(
327 consumer_name, service_root=service_root, cache=cache_path,
328- timeout=timeout, proxy_info=proxy_info)
329+ timeout=timeout, proxy_info=proxy_info,
330+ authorizer_class=authorizer_class,
331+ allow_access_levels=allow_access_levels,
332+ max_failed_attempts=max_failed_attempts)
333 launchpad.credentials.save_to_path(
334 os.path.join(credentials_path, consumer_name))
335 os.chmod(
336
337=== modified file 'src/launchpadlib/testing/helpers.py'
338--- src/launchpadlib/testing/helpers.py 2009-10-30 19:19:07 +0000
339+++ src/launchpadlib/testing/helpers.py 2009-11-03 14:20:26 +0000
340@@ -27,10 +27,12 @@
341 'salgado_with_full_permissions',
342 ]
343
344+import simplejson
345
346 from launchpadlib.launchpad import Launchpad
347 from launchpadlib.credentials import (
348- RequestTokenAuthorizationEngine, Credentials)
349+ AuthorizeRequestTokenWithBrowser, Credentials,
350+ RequestTokenAuthorizationEngine, SimulatedLaunchpadBrowser)
351
352
353 class TestableLaunchpad(Launchpad):
354@@ -39,7 +41,7 @@
355 # Use our test service root.
356 TESTING_SERVICE_ROOT = 'http://api.launchpad.dev:8085/beta/'
357
358- def __init__(self, credentials, service_root_ignored=None,
359+ def __init__(self, credentials, service_root=None,
360 cache=None, timeout=None, proxy_info=None):
361 super(TestableLaunchpad, self).__init__(
362 credentials, TestableLaunchpad.TESTING_SERVICE_ROOT,
363@@ -102,7 +104,7 @@
364 return self.credentials.access_token
365 return None
366
367- def open_login_page_in_user_browser(self, url):
368+ def open_page_in_user_browser(self, url):
369 """Print a status message."""
370 print ("[If this were a real application, the end-user's web "
371 "browser would be opened to %s]" % url)
372@@ -131,6 +133,20 @@
373 print message
374
375
376+class DummyAuthorizeRequestTokenWithBrowser(AuthorizeRequestTokenWithBrowser):
377+
378+ def __init__(self, web_root, consumer_name, request_token, username,
379+ password, allow_access_levels=[], max_failed_attempts=3):
380+ super(DummyAuthorizeRequestTokenWithBrowser, self).__init__(
381+ web_root, consumer_name, request_token, allow_access_levels,
382+ max_failed_attempts)
383+
384+ def open_page_in_user_browser(self, url):
385+ """Print a status message."""
386+ print ("[If this were a real application, the end-user's web "
387+ "browser would be opened to %s]" % url)
388+
389+
390 class UserInput(object):
391 """A class to store fake user input in a readable way.
392

Subscribers

People subscribed via source and target branches