Merge lp:~leonardr/launchpad/publish-tokens-2 into lp:launchpad/db-devel
- publish-tokens-2
- Merge into db-devel
Status: | Rejected |
---|---|
Rejected by: | Aaron Bentley |
Proposed branch: | lp:~leonardr/launchpad/publish-tokens-2 |
Merge into: | lp:launchpad/db-devel |
Diff against target: |
988 lines (+380/-120) 21 files modified
lib/canonical/launchpad/browser/oauth.py (+3/-9) lib/canonical/launchpad/database/oauth.py (+58/-6) lib/canonical/launchpad/doc/oauth.txt (+32/-8) lib/canonical/launchpad/doc/webapp-authorization.txt (+3/-11) lib/canonical/launchpad/doc/webapp-publication.txt (+7/-1) lib/canonical/launchpad/interfaces/_schema_circular_imports.py (+3/-0) lib/canonical/launchpad/interfaces/oauth.py (+80/-30) lib/canonical/launchpad/security.py (+37/-7) lib/canonical/launchpad/testing/pages.py (+15/-5) lib/canonical/launchpad/tests/test_webservice_oauth.py (+82/-0) lib/canonical/launchpad/webapp/authentication.py (+1/-6) lib/canonical/launchpad/webapp/authorization.py (+2/-1) lib/canonical/launchpad/webapp/servers.py (+7/-23) lib/canonical/launchpad/webapp/tests/test_publication.py (+6/-1) lib/canonical/launchpad/zcml/oauth.zcml (+6/-1) lib/lp/bugs/tests/test_bugs_webservice.py (+8/-0) lib/lp/registry/browser/person.py (+9/-0) lib/lp/registry/interfaces/person.py (+5/-1) lib/lp/registry/stories/webservice/xx-person.txt (+2/-0) lib/lp/testing/_webservice.py (+13/-9) versions.cfg (+1/-1) |
To merge this branch: | bzr merge lp:~leonardr/launchpad/publish-tokens-2 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Guilherme Salgado (community) | code | Approve | |
Aaron Bentley (community) | Needs Fixing | ||
Review via email: mp+34123@code.launchpad.net |
Commit message
Description of the change
This branch publishes OAuth access tokens through the web service. This meant adding traversal rules an canonical URL data for OAuthAccessToken objects.
A user can only view their own access tokens, and only by using a client that's been given the GRANT_PERMISSIONS access level. The old rules apply when accessing access tokens through the website: a user can always access their own tokens, and admins can access other peoples'.
Because I changed the rules for permission checks on OAuth access tokens to check the current request, I've added removeSecurityProxy calls in test helper classes like oauth_token_
This branch takes advantage of a new version of lazr.restful so that the read-write 'permission' field of IOAuthAccessToken can be published as read-only in the web service. This code is tested in lazr.restful, but I can add a Launchpad test as well.
Guilherme Salgado (salgado) wrote : | # |
Leonard asked me to do a follow up review of http://
The signature of check_oauth_
Also, it'd be nice to follow the naming scheme we use for other interfaces that are divided into multiple ones based on the permissions of the attributes/methods (e.g. IFooPublic, IFooEditRestric
Guilherme Salgado (salgado) wrote : | # |
And here's another follow up review: http://
+class OAuthTokenBase(
+ """Base implementation of code to check an OAuth-signed request."""
I'd change that docstring to say just that this is a base class for OAuthToken classes, as we may want to add code not related to signature checking here in the future.
+
+ def checkSignature(
+ return check_oauth_
And here you should add a docstring pointing to the interface where the method is defined.
-Now consider a principal authorized to create OAuth tokens. Whenever
-it's not creating OAuth tokens, it has a level of permission
-equivalent to READ_PUBLIC.
+A principal with the GRANT_PERMISSIONS authorization level has a of
+permission equivalent to WRITE_PRIVATE.
Why is the permission changing in this incremental diff? Also, s/of//?
- >>> access_token = token.createAcc
+ >>> access_token = removeSecurityP
Why do you need to remove the security proxy now?
Leonard Richardson (leonardr) wrote : | # |
> -Now consider a principal authorized to create OAuth tokens. Whenever
> -it's not creating OAuth tokens, it has a level of permission
> -equivalent to READ_PUBLIC.
> +A principal with the GRANT_PERMISSIONS authorization level has a of
> +permission equivalent to WRITE_PRIVATE.
>
> Why is the permission changing in this incremental diff? Also, s/of//?
The permission changed last time, after we decided there was no easy way to make GRANT_PERMISSIONS have a 'write' level of access for OAuth tokens and a 'read' level for everything else. I'm just fixing the test.
> - >>> access_token = token.createAcc
> + >>> access_token = removeSecurityP
>
> Why do you need to remove the security proxy now?
Again, I'm just fixing a test failure. The security proxy started breaking this test as soon as I introduced the security checker that assumes there's a current request.
Guilherme Salgado (salgado) : | # |
Preview Diff
1 | === modified file 'lib/canonical/launchpad/browser/oauth.py' | |||
2 | --- lib/canonical/launchpad/browser/oauth.py 2010-08-24 16:44:42 +0000 | |||
3 | +++ lib/canonical/launchpad/browser/oauth.py 2010-09-02 21:17:47 +0000 | |||
4 | @@ -83,7 +83,7 @@ | |||
5 | 83 | if consumer is None: | 83 | if consumer is None: |
6 | 84 | consumer = consumer_set.new(key=consumer_key) | 84 | consumer = consumer_set.new(key=consumer_key) |
7 | 85 | 85 | ||
9 | 86 | if not check_oauth_signature(self.request, consumer, None): | 86 | if not check_oauth_signature(self.request, consumer, ''): |
10 | 87 | return u'' | 87 | return u'' |
11 | 88 | 88 | ||
12 | 89 | token = consumer.newRequestToken() | 89 | token = consumer.newRequestToken() |
13 | @@ -284,13 +284,12 @@ | |||
14 | 284 | self.request.unauthorized(OAUTH_CHALLENGE) | 284 | self.request.unauthorized(OAUTH_CHALLENGE) |
15 | 285 | return u'No request token specified.' | 285 | return u'No request token specified.' |
16 | 286 | 286 | ||
18 | 287 | if not check_oauth_signature(self.request, consumer, token): | 287 | if not token.checkSignature(self.request): |
19 | 288 | return u'Invalid OAuth signature.' | 288 | return u'Invalid OAuth signature.' |
20 | 289 | 289 | ||
21 | 290 | if not token.is_reviewed: | 290 | if not token.is_reviewed: |
22 | 291 | self.request.unauthorized(OAUTH_CHALLENGE) | 291 | self.request.unauthorized(OAUTH_CHALLENGE) |
23 | 292 | return u'Request token has not yet been reviewed. Try again later.' | 292 | return u'Request token has not yet been reviewed. Try again later.' |
24 | 293 | |||
25 | 294 | if token.permission == OAuthPermission.UNAUTHORIZED: | 293 | if token.permission == OAuthPermission.UNAUTHORIZED: |
26 | 295 | # The end-user explicitly refused to authorize this | 294 | # The end-user explicitly refused to authorize this |
27 | 296 | # token. We send 403 ("Forbidden") instead of 401 | 295 | # token. We send 403 ("Forbidden") instead of 401 |
28 | @@ -301,9 +300,4 @@ | |||
29 | 301 | return u'End-user refused to authorize request token.' | 300 | return u'End-user refused to authorize request token.' |
30 | 302 | 301 | ||
31 | 303 | access_token = token.createAccessToken() | 302 | access_token = token.createAccessToken() |
38 | 304 | context_name = None | 303 | return access_token.form_encoded |
33 | 305 | if access_token.context is not None: | ||
34 | 306 | context_name = access_token.context.name | ||
35 | 307 | body = u'oauth_token=%s&oauth_token_secret=%s&lp.context=%s' % ( | ||
36 | 308 | access_token.key, access_token.secret, context_name) | ||
37 | 309 | return body | ||
39 | 310 | 304 | ||
40 | === modified file 'lib/canonical/launchpad/database/oauth.py' | |||
41 | --- lib/canonical/launchpad/database/oauth.py 2010-08-20 20:31:18 +0000 | |||
42 | +++ lib/canonical/launchpad/database/oauth.py 2010-09-02 21:17:47 +0000 | |||
43 | @@ -24,6 +24,7 @@ | |||
44 | 24 | from storm.expr import And | 24 | from storm.expr import And |
45 | 25 | from zope.component import getUtility | 25 | from zope.component import getUtility |
46 | 26 | from zope.interface import implements | 26 | from zope.interface import implements |
47 | 27 | from zope.security.interfaces import Unauthorized | ||
48 | 27 | 28 | ||
49 | 28 | from canonical.database.constants import UTC_NOW | 29 | from canonical.database.constants import UTC_NOW |
50 | 29 | from canonical.database.datetimecol import UtcDateTimeCol | 30 | from canonical.database.datetimecol import UtcDateTimeCol |
51 | @@ -36,21 +37,25 @@ | |||
52 | 36 | from canonical.launchpad.interfaces import ( | 37 | from canonical.launchpad.interfaces import ( |
53 | 37 | ClockSkew, | 38 | ClockSkew, |
54 | 38 | IOAuthAccessToken, | 39 | IOAuthAccessToken, |
55 | 40 | IOAuthAccessTokenPublic, | ||
56 | 39 | IOAuthConsumer, | 41 | IOAuthConsumer, |
57 | 40 | IOAuthConsumerSet, | 42 | IOAuthConsumerSet, |
58 | 41 | IOAuthNonce, | 43 | IOAuthNonce, |
59 | 42 | IOAuthRequestToken, | 44 | IOAuthRequestToken, |
60 | 43 | IOAuthRequestTokenSet, | 45 | IOAuthRequestTokenSet, |
61 | 46 | IOAuthTokenPublic, | ||
62 | 44 | NonceAlreadyUsed, | 47 | NonceAlreadyUsed, |
63 | 45 | TimestampOrderingError, | 48 | TimestampOrderingError, |
64 | 46 | ) | 49 | ) |
65 | 47 | from canonical.launchpad.webapp.interfaces import ( | 50 | from canonical.launchpad.webapp.interfaces import ( |
66 | 48 | AccessLevel, | 51 | AccessLevel, |
67 | 52 | ICanonicalUrlData, | ||
68 | 49 | IStoreSelector, | 53 | IStoreSelector, |
69 | 50 | MAIN_STORE, | 54 | MAIN_STORE, |
70 | 51 | MASTER_FLAVOR, | 55 | MASTER_FLAVOR, |
71 | 52 | OAuthPermission, | 56 | OAuthPermission, |
72 | 53 | ) | 57 | ) |
73 | 58 | from canonical.launchpad.webapp.authentication import check_oauth_signature | ||
74 | 54 | from lp.registry.interfaces.distribution import IDistribution | 59 | from lp.registry.interfaces.distribution import IDistribution |
75 | 55 | from lp.registry.interfaces.distributionsourcepackage import ( | 60 | from lp.registry.interfaces.distributionsourcepackage import ( |
76 | 56 | IDistributionSourcePackage, | 61 | IDistributionSourcePackage, |
77 | @@ -134,9 +139,28 @@ | |||
78 | 134 | return OAuthConsumer.selectOneBy(key=key) | 139 | return OAuthConsumer.selectOneBy(key=key) |
79 | 135 | 140 | ||
80 | 136 | 141 | ||
82 | 137 | class OAuthAccessToken(OAuthBase): | 142 | class OAuthTokenBase(OAuthBase): |
83 | 143 | """Base implementation of code for OAuth tokens.""" | ||
84 | 144 | |||
85 | 145 | def checkSignature(self, request): | ||
86 | 146 | """See `IOAuthTokenPublic`.""" | ||
87 | 147 | return check_oauth_signature(request, self.consumer, self.secret) | ||
88 | 148 | |||
89 | 149 | |||
90 | 150 | class OAuthAccessToken(OAuthTokenBase): | ||
91 | 138 | """See `IOAuthAccessToken`.""" | 151 | """See `IOAuthAccessToken`.""" |
93 | 139 | implements(IOAuthAccessToken) | 152 | implements( |
94 | 153 | IOAuthAccessToken, IOAuthAccessTokenPublic, ICanonicalUrlData) | ||
95 | 154 | |||
96 | 155 | @property | ||
97 | 156 | def inside(self): | ||
98 | 157 | return self.person | ||
99 | 158 | |||
100 | 159 | @property | ||
101 | 160 | def path(self): | ||
102 | 161 | return "+oauth-access-token/" + self.key | ||
103 | 162 | |||
104 | 163 | rootsite = 'api' | ||
105 | 140 | 164 | ||
106 | 141 | consumer = ForeignKey( | 165 | consumer = ForeignKey( |
107 | 142 | dbName='consumer', foreignKey='OAuthConsumer', notNull=True) | 166 | dbName='consumer', foreignKey='OAuthConsumer', notNull=True) |
108 | @@ -178,7 +202,7 @@ | |||
109 | 178 | return None | 202 | return None |
110 | 179 | 203 | ||
111 | 180 | def checkNonceAndTimestamp(self, nonce, timestamp): | 204 | def checkNonceAndTimestamp(self, nonce, timestamp): |
113 | 181 | """See `IOAuthAccessToken`.""" | 205 | """See `IOAuthAccessTokenPublic`.""" |
114 | 182 | timestamp = float(timestamp) | 206 | timestamp = float(timestamp) |
115 | 183 | date = datetime.fromtimestamp(timestamp, pytz.UTC) | 207 | date = datetime.fromtimestamp(timestamp, pytz.UTC) |
116 | 184 | # Determine if the timestamp is too far off from now. | 208 | # Determine if the timestamp is too far off from now. |
117 | @@ -209,10 +233,38 @@ | |||
118 | 209 | return OAuthNonce( | 233 | return OAuthNonce( |
119 | 210 | access_token=self, nonce=nonce, request_timestamp=date) | 234 | access_token=self, nonce=nonce, request_timestamp=date) |
120 | 211 | 235 | ||
123 | 212 | 236 | def authorize(self, request, nonce, timestamp): | |
124 | 213 | class OAuthRequestToken(OAuthBase): | 237 | """See `IOAuthAccessTokenPublic`.""" |
125 | 238 | try: | ||
126 | 239 | self.checkNonceAndTimestamp(nonce, timestamp) | ||
127 | 240 | except (NonceAlreadyUsed, TimestampOrderingError, ClockSkew), e: | ||
128 | 241 | raise Unauthorized('Invalid nonce/timestamp: %s' % e) | ||
129 | 242 | now = datetime.now(pytz.timezone('UTC')) | ||
130 | 243 | if self.permission == OAuthPermission.UNAUTHORIZED: | ||
131 | 244 | raise Unauthorized('Unauthorized token (%s).' % self.key) | ||
132 | 245 | elif self.date_expires is not None and self.date_expires <= now: | ||
133 | 246 | raise Unauthorized('Expired token (%s).' % self.key) | ||
134 | 247 | elif not self.checkSignature(request): | ||
135 | 248 | raise Unauthorized('Invalid signature.') | ||
136 | 249 | else: | ||
137 | 250 | # Everything is fine, let's return the principal. | ||
138 | 251 | pass | ||
139 | 252 | return (self.person.account.id, self.permission, self.context) | ||
140 | 253 | |||
141 | 254 | @property | ||
142 | 255 | def form_encoded(self): | ||
143 | 256 | """See `IOAuthAccessTokenPublic`.""" | ||
144 | 257 | context_name = None | ||
145 | 258 | if self.context is not None: | ||
146 | 259 | context_name = self.context.name | ||
147 | 260 | body = u'oauth_token=%s&oauth_token_secret=%s&lp.context=%s' % ( | ||
148 | 261 | self.key, self.secret, context_name) | ||
149 | 262 | return body | ||
150 | 263 | |||
151 | 264 | |||
152 | 265 | class OAuthRequestToken(OAuthTokenBase): | ||
153 | 214 | """See `IOAuthRequestToken`.""" | 266 | """See `IOAuthRequestToken`.""" |
155 | 215 | implements(IOAuthRequestToken) | 267 | implements(IOAuthRequestToken, IOAuthTokenPublic) |
156 | 216 | 268 | ||
157 | 217 | consumer = ForeignKey( | 269 | consumer = ForeignKey( |
158 | 218 | dbName='consumer', foreignKey='OAuthConsumer', notNull=True) | 270 | dbName='consumer', foreignKey='OAuthConsumer', notNull=True) |
159 | 219 | 271 | ||
160 | === modified file 'lib/canonical/launchpad/doc/oauth.txt' | |||
161 | --- lib/canonical/launchpad/doc/oauth.txt 2010-04-16 15:06:55 +0000 | |||
162 | +++ lib/canonical/launchpad/doc/oauth.txt 2010-09-02 21:17:47 +0000 | |||
163 | @@ -186,6 +186,13 @@ | |||
164 | 186 | True | 186 | True |
165 | 187 | >>> request_token.permission | 187 | >>> request_token.permission |
166 | 188 | <DBItem OAuthPermission.WRITE_PUBLIC... | 188 | <DBItem OAuthPermission.WRITE_PUBLIC... |
167 | 189 | |||
168 | 190 | Because an access token can only be viewed or edited by its owner or | ||
169 | 191 | an admin, we need to log in at this point. | ||
170 | 192 | |||
171 | 193 | >>> token_owner = request_token.person | ||
172 | 194 | >>> login_person(token_owner) | ||
173 | 195 | |||
174 | 189 | >>> access_token = request_token.createAccessToken() | 196 | >>> access_token = request_token.createAccessToken() |
175 | 190 | >>> verifyObject(IOAuthAccessToken, access_token) | 197 | >>> verifyObject(IOAuthAccessToken, access_token) |
176 | 191 | True | 198 | True |
177 | @@ -245,14 +252,30 @@ | |||
178 | 245 | >>> consumer.getAccessToken(access_token.key) == access_token | 252 | >>> consumer.getAccessToken(access_token.key) == access_token |
179 | 246 | True | 253 | True |
180 | 247 | 254 | ||
189 | 248 | An access token can only be changed by the person associated with it. | 255 | In general, one user cannot edit another user's access token. |
190 | 249 | 256 | ||
191 | 250 | >>> access_token.permission = OAuthPermission.WRITE_PUBLIC | 257 | >>> login('no-priv@canonical.com') |
192 | 251 | Traceback (most recent call last): | 258 | >>> access_token.permission |
193 | 252 | ... | 259 | Traceback (most recent call last): |
194 | 253 | Unauthorized:... | 260 | ... |
195 | 254 | >>> login_person(access_token.person) | 261 | Unauthorized:... |
196 | 255 | >>> access_token.permission = AccessLevel.WRITE_PUBLIC | 262 | |
197 | 263 | >>> access_token.permission = AccessLevel.WRITE_PUBLIC | ||
198 | 264 | Traceback (most recent call last): | ||
199 | 265 | ... | ||
200 | 266 | Unauthorized:... | ||
201 | 267 | |||
202 | 268 | An admin can view and edit someone else's access token. | ||
203 | 269 | |||
204 | 270 | >>> login("foo.bar@canonical.com") | ||
205 | 271 | >>> print access_token.permission.name | ||
206 | 272 | WRITE_PUBLIC | ||
207 | 273 | >>> access_token.permission = AccessLevel.WRITE_PUBLIC | ||
208 | 274 | |||
209 | 275 | And a user can edit their own access tokens. | ||
210 | 276 | |||
211 | 277 | >>> login_person(token_owner) | ||
212 | 278 | >>> access_token.permission = AccessLevel.WRITE_PRIVATE | ||
213 | 256 | 279 | ||
214 | 257 | From any given person it's possible to retrieve his non-expired access | 280 | From any given person it's possible to retrieve his non-expired access |
215 | 258 | tokens. | 281 | tokens. |
216 | @@ -305,6 +328,7 @@ | |||
217 | 305 | 328 | ||
218 | 306 | - We can use a nonce. | 329 | - We can use a nonce. |
219 | 307 | 330 | ||
220 | 331 | >>> login_person(token_owner) | ||
221 | 308 | >>> import time | 332 | >>> import time |
222 | 309 | >>> now = time.time() - 1 | 333 | >>> now = time.time() - 1 |
223 | 310 | >>> nonce1 = access_token.checkNonceAndTimestamp('boo', now) | 334 | >>> nonce1 = access_token.checkNonceAndTimestamp('boo', now) |
224 | 311 | 335 | ||
225 | === modified file 'lib/canonical/launchpad/doc/webapp-authorization.txt' | |||
226 | --- lib/canonical/launchpad/doc/webapp-authorization.txt 2010-08-24 16:44:42 +0000 | |||
227 | +++ lib/canonical/launchpad/doc/webapp-authorization.txt 2010-09-02 21:17:47 +0000 | |||
228 | @@ -79,9 +79,8 @@ | |||
229 | 79 | >>> check_permission('launchpad.View', bug_1) | 79 | >>> check_permission('launchpad.View', bug_1) |
230 | 80 | False | 80 | False |
231 | 81 | 81 | ||
235 | 82 | Now consider a principal authorized to create OAuth tokens. Whenever | 82 | A principal with the GRANT_PERMISSIONS authorization level has a level |
236 | 83 | it's not creating OAuth tokens, it has a level of permission | 83 | of permission equivalent to WRITE_PUBLIC. |
234 | 84 | equivalent to READ_PUBLIC. | ||
237 | 85 | 84 | ||
238 | 86 | >>> principal.access_level = AccessLevel.GRANT_PERMISSIONS | 85 | >>> principal.access_level = AccessLevel.GRANT_PERMISSIONS |
239 | 87 | >>> setupInteraction(principal) | 86 | >>> setupInteraction(principal) |
240 | @@ -89,14 +88,7 @@ | |||
241 | 89 | False | 88 | False |
242 | 90 | 89 | ||
243 | 91 | >>> check_permission('launchpad.Edit', sample_person) | 90 | >>> check_permission('launchpad.Edit', sample_person) |
252 | 92 | False | 91 | True |
245 | 93 | |||
246 | 94 | This may seem useless from a security standpoint, since once a | ||
247 | 95 | malicious client is authorized to create OAuth tokens, it can escalate | ||
248 | 96 | its privileges at any time by creating a new token for itself. The | ||
249 | 97 | security benefit is more subtle: by discouraging feature creep in | ||
250 | 98 | clients that have this super-access level, we reduce the risk that a | ||
251 | 99 | bug in a _trusted_ client will enable privilege escalation attacks. | ||
253 | 100 | 92 | ||
254 | 101 | Users logged in through the web application have full access, which | 93 | Users logged in through the web application have full access, which |
255 | 102 | means they can read/change any object they have access to. | 94 | means they can read/change any object they have access to. |
256 | 103 | 95 | ||
257 | === modified file 'lib/canonical/launchpad/doc/webapp-publication.txt' | |||
258 | --- lib/canonical/launchpad/doc/webapp-publication.txt 2010-08-05 13:23:52 +0000 | |||
259 | +++ lib/canonical/launchpad/doc/webapp-publication.txt 2010-09-02 21:17:47 +0000 | |||
260 | @@ -1156,14 +1156,20 @@ | |||
261 | 1156 | principal's access_level and scope will match what was specified in the | 1156 | principal's access_level and scope will match what was specified in the |
262 | 1157 | token. | 1157 | token. |
263 | 1158 | 1158 | ||
264 | 1159 | First let's create a token. | ||
265 | 1160 | |||
266 | 1159 | >>> from lp.registry.interfaces.product import IProductSet | 1161 | >>> from lp.registry.interfaces.product import IProductSet |
267 | 1162 | >>> from zope.security.proxy import removeSecurityProxy | ||
268 | 1160 | >>> getUtility(IStoreSelector).push(MasterDatabasePolicy()) | 1163 | >>> getUtility(IStoreSelector).push(MasterDatabasePolicy()) |
269 | 1161 | >>> salgado = getUtility(IPersonSet).getByName('salgado') | 1164 | >>> salgado = getUtility(IPersonSet).getByName('salgado') |
270 | 1162 | >>> consumer = getUtility(IOAuthConsumerSet).getByKey('foobar123451432') | 1165 | >>> consumer = getUtility(IOAuthConsumerSet).getByKey('foobar123451432') |
271 | 1163 | >>> token = consumer.newRequestToken() | 1166 | >>> token = consumer.newRequestToken() |
272 | 1164 | >>> firefox = getUtility(IProductSet)['firefox'] | 1167 | >>> firefox = getUtility(IProductSet)['firefox'] |
273 | 1165 | >>> token.review(salgado, OAuthPermission.WRITE_PUBLIC, context=firefox) | 1168 | >>> token.review(salgado, OAuthPermission.WRITE_PUBLIC, context=firefox) |
275 | 1166 | >>> access_token = token.createAccessToken() | 1169 | >>> access_token = removeSecurityProxy(token.createAccessToken()) |
276 | 1170 | |||
277 | 1171 | Now let's use it to make a request. | ||
278 | 1172 | |||
279 | 1167 | >>> form = dict( | 1173 | >>> form = dict( |
280 | 1168 | ... oauth_consumer_key='foobar123451432', | 1174 | ... oauth_consumer_key='foobar123451432', |
281 | 1169 | ... oauth_token=access_token.key, | 1175 | ... oauth_token=access_token.key, |
282 | 1170 | 1176 | ||
283 | === modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py' | |||
284 | --- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2010-08-27 11:19:54 +0000 | |||
285 | +++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2010-09-02 21:17:47 +0000 | |||
286 | @@ -32,6 +32,7 @@ | |||
287 | 32 | IMessage, | 32 | IMessage, |
288 | 33 | IUserToUserEmail, | 33 | IUserToUserEmail, |
289 | 34 | ) | 34 | ) |
290 | 35 | from canonical.launchpad.interfaces.oauth import IOAuthToken | ||
291 | 35 | from lp.blueprints.interfaces.specification import ISpecification | 36 | from lp.blueprints.interfaces.specification import ISpecification |
292 | 36 | from lp.blueprints.interfaces.specificationbranch import ISpecificationBranch | 37 | from lp.blueprints.interfaces.specificationbranch import ISpecificationBranch |
293 | 37 | from lp.bugs.interfaces.bug import ( | 38 | from lp.bugs.interfaces.bug import ( |
294 | @@ -196,6 +197,8 @@ | |||
295 | 196 | patch_choice_parameter_type( | 197 | patch_choice_parameter_type( |
296 | 197 | IHasBugs, 'searchTasks', 'hardware_bus', HWBus) | 198 | IHasBugs, 'searchTasks', 'hardware_bus', HWBus) |
297 | 198 | 199 | ||
298 | 200 | IOAuthToken['person'].schema = IPerson | ||
299 | 201 | |||
300 | 199 | IPreviewDiff['branch_merge_proposal'].schema = IBranchMergeProposal | 202 | IPreviewDiff['branch_merge_proposal'].schema = IBranchMergeProposal |
301 | 200 | 203 | ||
302 | 201 | patch_reference_property(IPersonPublic, 'archive', IArchive) | 204 | patch_reference_property(IPersonPublic, 'archive', IArchive) |
303 | 202 | 205 | ||
304 | === modified file 'lib/canonical/launchpad/interfaces/oauth.py' | |||
305 | --- lib/canonical/launchpad/interfaces/oauth.py 2010-08-20 20:31:18 +0000 | |||
306 | +++ lib/canonical/launchpad/interfaces/oauth.py 2010-09-02 21:17:47 +0000 | |||
307 | @@ -11,12 +11,15 @@ | |||
308 | 11 | 'OAUTH_REALM', | 11 | 'OAUTH_REALM', |
309 | 12 | 'OAUTH_CHALLENGE', | 12 | 'OAUTH_CHALLENGE', |
310 | 13 | 'IOAuthAccessToken', | 13 | 'IOAuthAccessToken', |
311 | 14 | 'IOAuthAccessTokenPublic', | ||
312 | 14 | 'IOAuthConsumer', | 15 | 'IOAuthConsumer', |
313 | 15 | 'IOAuthConsumerSet', | 16 | 'IOAuthConsumerSet', |
314 | 17 | 'IOAuthToken', | ||
315 | 16 | 'IOAuthNonce', | 18 | 'IOAuthNonce', |
316 | 17 | 'IOAuthRequestToken', | 19 | 'IOAuthRequestToken', |
317 | 18 | 'IOAuthRequestTokenSet', | 20 | 'IOAuthRequestTokenSet', |
318 | 19 | 'IOAuthSignedRequest', | 21 | 'IOAuthSignedRequest', |
319 | 22 | 'IOAuthTokenPublic', | ||
320 | 20 | 'NonceAlreadyUsed', | 23 | 'NonceAlreadyUsed', |
321 | 21 | 'TimestampOrderingError', | 24 | 'TimestampOrderingError', |
322 | 22 | 'ClockSkew', | 25 | 'ClockSkew', |
323 | @@ -34,12 +37,14 @@ | |||
324 | 34 | TextLine, | 37 | TextLine, |
325 | 35 | ) | 38 | ) |
326 | 36 | 39 | ||
327 | 40 | from lazr.restful.declarations import ( | ||
328 | 41 | export_as_webservice_entry, exported) | ||
329 | 42 | |||
330 | 37 | from canonical.launchpad import _ | 43 | from canonical.launchpad import _ |
331 | 38 | from canonical.launchpad.webapp.interfaces import ( | 44 | from canonical.launchpad.webapp.interfaces import ( |
332 | 39 | AccessLevel, | 45 | AccessLevel, |
333 | 40 | OAuthPermission, | 46 | OAuthPermission, |
334 | 41 | ) | 47 | ) |
335 | 42 | from lp.registry.interfaces.person import IPerson | ||
336 | 43 | 48 | ||
337 | 44 | # The challenge included in responses with a 401 status. | 49 | # The challenge included in responses with a 401 status. |
338 | 45 | OAUTH_REALM = 'https://api.launchpad.net' | 50 | OAUTH_REALM = 'https://api.launchpad.net' |
339 | @@ -128,23 +133,28 @@ | |||
340 | 128 | schema=IOAuthConsumer, title=_('The consumer.'), | 133 | schema=IOAuthConsumer, title=_('The consumer.'), |
341 | 129 | description=_("The consumer which will access Launchpad on the " | 134 | description=_("The consumer which will access Launchpad on the " |
342 | 130 | "user's behalf.")) | 135 | "user's behalf.")) |
360 | 131 | person = Object( | 136 | person = exported( |
361 | 132 | schema=IPerson, title=_('Person'), required=False, readonly=False, | 137 | Object(schema=Interface, title=_('Person'), required=False, |
362 | 133 | description=_('The user on whose behalf the consumer is accessing.')) | 138 | readonly=False, description=_( |
363 | 134 | date_created = Datetime( | 139 | 'The user on whose behalf the consumer is accessing.')), |
364 | 135 | title=_('Date created'), required=True, readonly=True) | 140 | readonly=True) |
365 | 136 | date_expires = Datetime( | 141 | date_created = exported( |
366 | 137 | title=_('Date expires'), required=False, readonly=False, | 142 | Datetime(title=_('Date created'), required=True, readonly=True)) |
367 | 138 | description=_('From this date onwards this token can not be used ' | 143 | date_expires = exported( |
368 | 139 | 'by the consumer to access protected resources.')) | 144 | Datetime( |
369 | 140 | key = TextLine( | 145 | title=_('Date expires'), required=False, readonly=False, |
370 | 141 | title=_('Key'), required=True, readonly=True, | 146 | description=_('From this date onwards this token can not be used ' |
371 | 142 | description=_('The key used to identify this token. It is included ' | 147 | 'by the consumer to access protected resources.'))) |
372 | 143 | 'by the consumer in each request.')) | 148 | key = exported( |
373 | 144 | secret = TextLine( | 149 | TextLine( |
374 | 145 | title=_('Secret'), required=True, readonly=True, | 150 | title=_('Key'), required=True, readonly=True, |
375 | 146 | description=_('The secret associated with this token. It is used ' | 151 | description=_('The key used to identify this token. It is ' |
376 | 147 | 'by the consumer to sign its requests.')) | 152 | 'included by the consumer in each request.'))) |
377 | 153 | secret = exported( | ||
378 | 154 | TextLine( | ||
379 | 155 | title=_('Secret'), required=True, readonly=True, | ||
380 | 156 | description=_('The secret associated with this token. It is used ' | ||
381 | 157 | 'by the consumer to sign its requests.'))) | ||
382 | 148 | product = Choice(title=_('Project'), required=False, vocabulary='Product') | 158 | product = Choice(title=_('Project'), required=False, vocabulary='Product') |
383 | 149 | project = Choice( | 159 | project = Choice( |
384 | 150 | title=_('Project'), required=False, vocabulary='ProjectGroup') | 160 | title=_('Project'), required=False, vocabulary='ProjectGroup') |
385 | @@ -155,18 +165,22 @@ | |||
386 | 155 | context = Attribute("FIXME") | 165 | context = Attribute("FIXME") |
387 | 156 | 166 | ||
388 | 157 | 167 | ||
401 | 158 | class IOAuthAccessToken(IOAuthToken): | 168 | class IOAuthTokenPublic(Interface): |
402 | 159 | """A token used by a consumer to access protected resources in LP. | 169 | """Public access to methods necessary to authenticate OAuth tokens.""" |
403 | 160 | 170 | ||
404 | 161 | It's created automatically once a user logs in and grants access to a | 171 | def checkSignature(request): |
405 | 162 | consumer. The consumer then exchanges an `IOAuthRequestToken` for it. | 172 | """Check the signature of an incoming request. |
406 | 163 | """ | 173 | |
407 | 164 | 174 | :param request: A request that's supposedly signed with this | |
408 | 165 | permission = Choice( | 175 | token's secret. |
409 | 166 | title=_('Access level'), required=True, readonly=False, | 176 | :return: True if the request is correctly signed, False otherwise. |
410 | 167 | vocabulary=AccessLevel, | 177 | """ |
411 | 168 | description=_('The level of access given to the application acting ' | 178 | |
412 | 169 | 'on your behalf.')) | 179 | |
413 | 180 | class IOAuthAccessTokenPublic(IOAuthTokenPublic): | ||
414 | 181 | """Public access to methods necessary to authenticate access tokens.""" | ||
415 | 182 | |||
416 | 183 | form_encoded = Attribute("Form-encoded description of the token.") | ||
417 | 170 | 184 | ||
418 | 171 | def checkNonceAndTimestamp(nonce, timestamp): | 185 | def checkNonceAndTimestamp(nonce, timestamp): |
419 | 172 | """Verify the nonce and timestamp. | 186 | """Verify the nonce and timestamp. |
420 | @@ -189,6 +203,42 @@ | |||
421 | 189 | and associated with this token. | 203 | and associated with this token. |
422 | 190 | """ | 204 | """ |
423 | 191 | 205 | ||
424 | 206 | def authorize(request, nonce, timestamp): | ||
425 | 207 | """Authorize an incoming request against an OAuth token. | ||
426 | 208 | |||
427 | 209 | :param request: A request that's supposedly signed with this | ||
428 | 210 | token's secret. | ||
429 | 211 | :param nonce: An OAuth nonce provided with the request. | ||
430 | 212 | :param timestamp: An OAuth timestamp provided with the request. | ||
431 | 213 | |||
432 | 214 | :raise Unauthorized: If the request is not correctly signed | ||
433 | 215 | with this token's secret, if the nonce or timestamp are | ||
434 | 216 | bad, if the token has expired, if the token does not | ||
435 | 217 | actually grant access, or if for any other reason the | ||
436 | 218 | request cannot be authorized. | ||
437 | 219 | |||
438 | 220 | :return: A 3-tuple (account_id, access_level, context) | ||
439 | 221 | containing all the information necessary to identify the | ||
440 | 222 | authenticated principal. | ||
441 | 223 | """ | ||
442 | 224 | |||
443 | 225 | |||
444 | 226 | class IOAuthAccessToken(IOAuthToken): | ||
445 | 227 | """A token used by a consumer to access protected resources in LP. | ||
446 | 228 | |||
447 | 229 | It's created automatically once a user logs in and grants access to a | ||
448 | 230 | consumer. The consumer then exchanges an `IOAuthRequestToken` for it. | ||
449 | 231 | """ | ||
450 | 232 | export_as_webservice_entry() | ||
451 | 233 | |||
452 | 234 | permission = exported( | ||
453 | 235 | Choice( | ||
454 | 236 | title=_('Access level'), required=True, readonly=False, | ||
455 | 237 | vocabulary=AccessLevel, | ||
456 | 238 | description=_('The level of access given to the application acting ' | ||
457 | 239 | 'on your behalf.')), | ||
458 | 240 | readonly=True) | ||
459 | 241 | |||
460 | 192 | 242 | ||
461 | 193 | class IOAuthRequestToken(IOAuthToken): | 243 | class IOAuthRequestToken(IOAuthToken): |
462 | 194 | """A token used by a consumer to ask the user to authenticate on LP. | 244 | """A token used by a consumer to ask the user to authenticate on LP. |
463 | 195 | 245 | ||
464 | === modified file 'lib/canonical/launchpad/security.py' | |||
465 | --- lib/canonical/launchpad/security.py 2010-08-31 20:12:22 +0000 | |||
466 | +++ lib/canonical/launchpad/security.py 2010-09-02 21:17:47 +0000 | |||
467 | @@ -33,12 +33,15 @@ | |||
468 | 33 | from canonical.launchpad.interfaces.oauth import ( | 33 | from canonical.launchpad.interfaces.oauth import ( |
469 | 34 | IOAuthAccessToken, | 34 | IOAuthAccessToken, |
470 | 35 | IOAuthRequestToken, | 35 | IOAuthRequestToken, |
471 | 36 | IOAuthSignedRequest, | ||
472 | 36 | ) | 37 | ) |
473 | 37 | from canonical.launchpad.webapp.authorization import check_permission | 38 | from canonical.launchpad.webapp.authorization import check_permission |
474 | 38 | from canonical.launchpad.webapp.interfaces import ( | 39 | from canonical.launchpad.webapp.interfaces import ( |
475 | 40 | AccessLevel, | ||
476 | 39 | IAuthorization, | 41 | IAuthorization, |
477 | 40 | ILaunchpadRoot, | 42 | ILaunchpadRoot, |
478 | 41 | ) | 43 | ) |
479 | 44 | from canonical.lazr.utils import get_current_browser_request | ||
480 | 42 | from lp.answers.interfaces.faq import IFAQ | 45 | from lp.answers.interfaces.faq import IFAQ |
481 | 43 | from lp.answers.interfaces.faqtarget import IFAQTarget | 46 | from lp.answers.interfaces.faqtarget import IFAQTarget |
482 | 44 | from lp.answers.interfaces.question import IQuestion | 47 | from lp.answers.interfaces.question import IQuestion |
483 | @@ -373,15 +376,42 @@ | |||
484 | 373 | return user.in_admin or user.in_registry_experts | 376 | return user.in_admin or user.in_registry_experts |
485 | 374 | 377 | ||
486 | 375 | 378 | ||
490 | 376 | class EditOAuthAccessToken(AuthorizationBase): | 379 | class OAuthToken(AuthorizationBase): |
491 | 377 | permission = 'launchpad.Edit' | 380 | """Special rules for access to OAuth tokens. |
492 | 378 | usedfor = IOAuthAccessToken | 381 | |
493 | 382 | Through the website, a person can always access their own OAuth | ||
494 | 383 | tokens, and an admin can access someone else's OAuth tokens. | ||
495 | 384 | |||
496 | 385 | To minimize the risk of privilege escalation attacks, much greater | ||
497 | 386 | restrictions apply through the web service. A person can only | ||
498 | 387 | access their own OAuth tokens, and they must be using a client | ||
499 | 388 | authorized at the GRANT_PERMISSIONS access level. | ||
500 | 389 | """ | ||
501 | 379 | 390 | ||
502 | 380 | def checkAuthenticated(self, user): | 391 | def checkAuthenticated(self, user): |
507 | 381 | return self.obj.person == user.person or user.in_admin | 392 | request = get_current_browser_request() |
508 | 382 | 393 | if IOAuthSignedRequest.providedBy(request): | |
509 | 383 | 394 | # This is a web service request. | |
510 | 384 | class EditOAuthRequestToken(EditOAuthAccessToken): | 395 | if self.obj.person != user.person: |
511 | 396 | return False | ||
512 | 397 | return (request.principal.access_level == | ||
513 | 398 | AccessLevel.GRANT_PERMISSIONS) | ||
514 | 399 | else: | ||
515 | 400 | # This is a website request. | ||
516 | 401 | return self.obj.person == user.person or user.in_admin | ||
517 | 402 | |||
518 | 403 | |||
519 | 404 | class ViewOAuthAccessToken(OAuthToken): | ||
520 | 405 | permission = 'launchpad.View' | ||
521 | 406 | usedfor = IOAuthAccessToken | ||
522 | 407 | |||
523 | 408 | |||
524 | 409 | class EditOAuthAccessToken(OAuthToken): | ||
525 | 410 | permission = 'launchpad.Edit' | ||
526 | 411 | usedfor = IOAuthAccessToken | ||
527 | 412 | |||
528 | 413 | |||
529 | 414 | class EditOAuthRequestToken(OAuthToken): | ||
530 | 385 | permission = 'launchpad.Edit' | 415 | permission = 'launchpad.Edit' |
531 | 386 | usedfor = IOAuthRequestToken | 416 | usedfor = IOAuthRequestToken |
532 | 387 | 417 | ||
533 | 388 | 418 | ||
534 | === modified file 'lib/canonical/launchpad/testing/pages.py' | |||
535 | --- lib/canonical/launchpad/testing/pages.py 2010-08-20 20:31:18 +0000 | |||
536 | +++ lib/canonical/launchpad/testing/pages.py 2010-09-02 21:17:47 +0000 | |||
537 | @@ -153,10 +153,16 @@ | |||
538 | 153 | self.consumer = OAuthConsumer(oauth_consumer_key, '') | 153 | self.consumer = OAuthConsumer(oauth_consumer_key, '') |
539 | 154 | self.access_token = OAuthToken(oauth_access_key, '') | 154 | self.access_token = OAuthToken(oauth_access_key, '') |
540 | 155 | else: | 155 | else: |
545 | 156 | # The client wants to make an authorized request | 156 | # The client wants to make authorized requests |
546 | 157 | # using a recognized consumer key. | 157 | # using a recognized consumer key. The security |
547 | 158 | self.access_token = self.consumer.getAccessToken( | 158 | # policy for IOAuthAccessToken assumes the request |
548 | 159 | oauth_access_key) | 159 | # is being processed on the server side and has |
549 | 160 | # already been signed with some access token. | ||
550 | 161 | # | ||
551 | 162 | # Here, the requests haven't even gone out yet, | ||
552 | 163 | # so it's okay to strip the security proxy. | ||
553 | 164 | self.access_token = removeSecurityProxy( | ||
554 | 165 | self.consumer.getAccessToken(oauth_access_key)) | ||
555 | 160 | logout() | 166 | logout() |
556 | 161 | else: | 167 | else: |
557 | 162 | self.consumer = None | 168 | self.consumer = None |
558 | @@ -694,7 +700,11 @@ | |||
559 | 694 | consumer = oacs.new(consumer_key) | 700 | consumer = oacs.new(consumer_key) |
560 | 695 | request_token = consumer.newRequestToken() | 701 | request_token = consumer.newRequestToken() |
561 | 696 | request_token.review(person, permission, context) | 702 | request_token.review(person, permission, context) |
563 | 697 | access_token = request_token.createAccessToken() | 703 | # The security proxy assumes that an OAuth token has been |
564 | 704 | # associated with the current HTTP request. Since we're using an | ||
565 | 705 | # OAuth token to construct the HTTP requests in the first place, | ||
566 | 706 | # we need to strip the security proxy. | ||
567 | 707 | access_token = removeSecurityProxy(request_token.createAccessToken()) | ||
568 | 698 | logout() | 708 | logout() |
569 | 699 | return LaunchpadWebServiceCaller(consumer_key, access_token.key) | 709 | return LaunchpadWebServiceCaller(consumer_key, access_token.key) |
570 | 700 | 710 | ||
571 | 701 | 711 | ||
572 | === added file 'lib/canonical/launchpad/tests/test_webservice_oauth.py' | |||
573 | --- lib/canonical/launchpad/tests/test_webservice_oauth.py 1970-01-01 00:00:00 +0000 | |||
574 | +++ lib/canonical/launchpad/tests/test_webservice_oauth.py 2010-09-02 21:17:47 +0000 | |||
575 | @@ -0,0 +1,82 @@ | |||
576 | 1 | # Copyright 2010 Canonical Ltd. This software is licensed under the | ||
577 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | ||
578 | 3 | |||
579 | 4 | __metaclass__ = type | ||
580 | 5 | |||
581 | 6 | from datetime import date | ||
582 | 7 | import simplejson | ||
583 | 8 | |||
584 | 9 | from canonical.launchpad.testing.pages import LaunchpadWebServiceCaller | ||
585 | 10 | from canonical.testing import LaunchpadFunctionalLayer | ||
586 | 11 | from lp.testing import ( | ||
587 | 12 | launchpadlib_for, | ||
588 | 13 | TestCase, | ||
589 | 14 | ) | ||
590 | 15 | |||
591 | 16 | class TestWebServiceAccessToOAuth(TestCase): | ||
592 | 17 | """Tests of OAuth tokens as published in the web service.""" | ||
593 | 18 | |||
594 | 19 | layer = LaunchpadFunctionalLayer | ||
595 | 20 | |||
596 | 21 | def setUp(self): | ||
597 | 22 | """Setup.""" | ||
598 | 23 | super(TestWebServiceAccessToOAuth, self).setUp() | ||
599 | 24 | self.launchpad = launchpadlib_for( | ||
600 | 25 | 'Token management test', 'salgado', 'GRANT_PERMISSIONS') | ||
601 | 26 | |||
602 | 27 | def test_authorized_read(self): | ||
603 | 28 | """You can read tokens if your client has the right access level.""" | ||
604 | 29 | # The person 'salgado' has three associated tokens: two | ||
605 | 30 | # created as part of sample data, and one ("Grant | ||
606 | 31 | # Permissions") created in the launchpadlib_for() call | ||
607 | 32 | # in setUp(). | ||
608 | 33 | my_tokens = [token.permission for token in | ||
609 | 34 | self.launchpad.me.oauth_access_tokens] | ||
610 | 35 | self.assertEquals( | ||
611 | 36 | sorted(my_tokens), [u'Change Anything', u'Grant Permissions', | ||
612 | 37 | u'Read Non-Private Data']) | ||
613 | 38 | |||
614 | 39 | def test_authorized_write(self): | ||
615 | 40 | """You can write tokens if your client has the right access level.""" | ||
616 | 41 | # Change a token's expiration date. | ||
617 | 42 | token = [token for token in self.launchpad.me.oauth_access_tokens | ||
618 | 43 | if token.permission == "Read Non-Private Data"][0] | ||
619 | 44 | old_date_expires = token.date_expires | ||
620 | 45 | new_date_expires = str(date(9000, 1, 1)) | ||
621 | 46 | token.date_expires = new_date_expires | ||
622 | 47 | token.lp_save() | ||
623 | 48 | |||
624 | 49 | # Change it back. | ||
625 | 50 | token.date_expires = old_date_expires | ||
626 | 51 | token.lp_save() | ||
627 | 52 | |||
628 | 53 | def test_others_tokens_are_invisible(self): | ||
629 | 54 | """No one else can see your tokens.""" | ||
630 | 55 | someone_else = launchpadlib_for( | ||
631 | 56 | 'Token management test', 'name12', 'GRANT_PERMISSIONS') | ||
632 | 57 | tokens = [token for token in | ||
633 | 58 | someone_else.people['salgado'].oauth_access_tokens] | ||
634 | 59 | self.assertEquals(tokens, []) | ||
635 | 60 | |||
636 | 61 | def test_insufficient_access_level(self): | ||
637 | 62 | """A client can't read your tokens without GRANT_PERMISSIONS.""" | ||
638 | 63 | write_private = launchpadlib_for( | ||
639 | 64 | 'Token management test', 'salgado', 'WRITE_PRIVATE') | ||
640 | 65 | my_tokens = [token for token in | ||
641 | 66 | write_private.me.oauth_access_tokens] | ||
642 | 67 | self.assertEquals(my_tokens, []) | ||
643 | 68 | |||
644 | 69 | def test_url_hacking_doesnt_work(self): | ||
645 | 70 | """You can't access tokens by URL hacking.""" | ||
646 | 71 | webservice = LaunchpadWebServiceCaller( | ||
647 | 72 | 'launchpad-library', 'salgado-change-anything') | ||
648 | 73 | |||
649 | 74 | # A read request gets a 401. | ||
650 | 75 | url = '/~salgado/+oauth-access-token/salgado-read-nonprivate' | ||
651 | 76 | response = webservice.get(url) | ||
652 | 77 | self.assertEqual(response.status, 401) | ||
653 | 78 | |||
654 | 79 | # A write request gets a 401. | ||
655 | 80 | body = simplejson.dumps(dict(date_expires=str(date(9000, 1, 1)))) | ||
656 | 81 | response = webservice.patch(url, 'application/json', body) | ||
657 | 82 | self.assertEqual(response.status, 401) | ||
658 | 0 | 83 | ||
659 | === modified file 'lib/canonical/launchpad/webapp/authentication.py' | |||
660 | --- lib/canonical/launchpad/webapp/authentication.py 2010-08-20 20:31:18 +0000 | |||
661 | +++ lib/canonical/launchpad/webapp/authentication.py 2010-09-02 21:17:47 +0000 | |||
662 | @@ -363,24 +363,19 @@ | |||
663 | 363 | return request.form | 363 | return request.form |
664 | 364 | 364 | ||
665 | 365 | 365 | ||
667 | 366 | def check_oauth_signature(request, consumer, token): | 366 | def check_oauth_signature(request, consumer, token_secret): |
668 | 367 | """Check that the given OAuth request is correctly signed. | 367 | """Check that the given OAuth request is correctly signed. |
669 | 368 | 368 | ||
670 | 369 | If the signature is incorrect or its method is not supported, set the | 369 | If the signature is incorrect or its method is not supported, set the |
671 | 370 | appropriate status in the request's response and return False. | 370 | appropriate status in the request's response and return False. |
672 | 371 | """ | 371 | """ |
673 | 372 | authorization = get_oauth_authorization(request) | 372 | authorization = get_oauth_authorization(request) |
674 | 373 | |||
675 | 374 | if authorization.get('oauth_signature_method') != 'PLAINTEXT': | 373 | if authorization.get('oauth_signature_method') != 'PLAINTEXT': |
676 | 375 | # XXX: 2008-03-04, salgado: Only the PLAINTEXT method is supported | 374 | # XXX: 2008-03-04, salgado: Only the PLAINTEXT method is supported |
677 | 376 | # now. Others will be implemented later. | 375 | # now. Others will be implemented later. |
678 | 377 | request.response.setStatus(400) | 376 | request.response.setStatus(400) |
679 | 378 | return False | 377 | return False |
680 | 379 | 378 | ||
681 | 380 | if token is not None: | ||
682 | 381 | token_secret = token.secret | ||
683 | 382 | else: | ||
684 | 383 | token_secret = '' | ||
685 | 384 | expected_signature = "&".join([consumer.secret, token_secret]) | 379 | expected_signature = "&".join([consumer.secret, token_secret]) |
686 | 385 | if expected_signature != authorization.get('oauth_signature'): | 380 | if expected_signature != authorization.get('oauth_signature'): |
687 | 386 | request.unauthorized(OAUTH_CHALLENGE) | 381 | request.unauthorized(OAUTH_CHALLENGE) |
688 | 387 | 382 | ||
689 | === modified file 'lib/canonical/launchpad/webapp/authorization.py' | |||
690 | --- lib/canonical/launchpad/webapp/authorization.py 2010-08-20 20:31:18 +0000 | |||
691 | +++ lib/canonical/launchpad/webapp/authorization.py 2010-09-02 21:17:47 +0000 | |||
692 | @@ -61,7 +61,8 @@ | |||
693 | 61 | lp_permission = getUtility(ILaunchpadPermission, permission) | 61 | lp_permission = getUtility(ILaunchpadPermission, permission) |
694 | 62 | if lp_permission.access_level == "write": | 62 | if lp_permission.access_level == "write": |
695 | 63 | required_access_level = [ | 63 | required_access_level = [ |
697 | 64 | AccessLevel.WRITE_PUBLIC, AccessLevel.WRITE_PRIVATE] | 64 | AccessLevel.WRITE_PUBLIC, AccessLevel.WRITE_PRIVATE, |
698 | 65 | AccessLevel.GRANT_PERMISSIONS] | ||
699 | 65 | if access_level not in required_access_level: | 66 | if access_level not in required_access_level: |
700 | 66 | return False | 67 | return False |
701 | 67 | elif lp_permission.access_level == "read": | 68 | elif lp_permission.access_level == "read": |
702 | 68 | 69 | ||
703 | === modified file 'lib/canonical/launchpad/webapp/servers.py' | |||
704 | --- lib/canonical/launchpad/webapp/servers.py 2010-08-28 03:23:19 +0000 | |||
705 | +++ lib/canonical/launchpad/webapp/servers.py 2010-09-02 21:17:47 +0000 | |||
706 | @@ -8,7 +8,6 @@ | |||
707 | 8 | __metaclass__ = type | 8 | __metaclass__ = type |
708 | 9 | 9 | ||
709 | 10 | import cgi | 10 | import cgi |
710 | 11 | from datetime import datetime | ||
711 | 12 | import threading | 11 | import threading |
712 | 13 | import xmlrpclib | 12 | import xmlrpclib |
713 | 14 | 13 | ||
714 | @@ -70,11 +69,8 @@ | |||
715 | 70 | IWebServiceApplication, | 69 | IWebServiceApplication, |
716 | 71 | ) | 70 | ) |
717 | 72 | from canonical.launchpad.interfaces.oauth import ( | 71 | from canonical.launchpad.interfaces.oauth import ( |
718 | 73 | ClockSkew, | ||
719 | 74 | IOAuthConsumerSet, | 72 | IOAuthConsumerSet, |
720 | 75 | IOAuthSignedRequest, | 73 | IOAuthSignedRequest, |
721 | 76 | NonceAlreadyUsed, | ||
722 | 77 | TimestampOrderingError, | ||
723 | 78 | ) | 74 | ) |
724 | 79 | import canonical.launchpad.layers | 75 | import canonical.launchpad.layers |
725 | 80 | from canonical.launchpad.webapp.adapter import ( | 76 | from canonical.launchpad.webapp.adapter import ( |
726 | @@ -82,7 +78,6 @@ | |||
727 | 82 | RequestExpired, | 78 | RequestExpired, |
728 | 83 | ) | 79 | ) |
729 | 84 | from canonical.launchpad.webapp.authentication import ( | 80 | from canonical.launchpad.webapp.authentication import ( |
730 | 85 | check_oauth_signature, | ||
731 | 86 | get_oauth_authorization, | 81 | get_oauth_authorization, |
732 | 87 | ) | 82 | ) |
733 | 88 | from canonical.launchpad.webapp.authorization import ( | 83 | from canonical.launchpad.webapp.authorization import ( |
734 | @@ -1276,27 +1271,16 @@ | |||
735 | 1276 | token = consumer.getAccessToken(token_key) | 1271 | token = consumer.getAccessToken(token_key) |
736 | 1277 | if token is None: | 1272 | if token is None: |
737 | 1278 | raise Unauthorized('Unknown access token (%s).' % token_key) | 1273 | raise Unauthorized('Unknown access token (%s).' % token_key) |
738 | 1274 | |||
739 | 1279 | nonce = form.get('oauth_nonce') | 1275 | nonce = form.get('oauth_nonce') |
740 | 1280 | timestamp = form.get('oauth_timestamp') | 1276 | timestamp = form.get('oauth_timestamp') |
755 | 1281 | try: | 1277 | # If there's a problem with the token, authorize() will raise |
756 | 1282 | token.checkNonceAndTimestamp(nonce, timestamp) | 1278 | # Unauthorized. |
757 | 1283 | except (NonceAlreadyUsed, TimestampOrderingError, ClockSkew), e: | 1279 | (account_id, access_level, scope) = token.authorize( |
758 | 1284 | raise Unauthorized('Invalid nonce/timestamp: %s' % e) | 1280 | request, nonce, timestamp) |
759 | 1285 | now = datetime.now(pytz.timezone('UTC')) | 1281 | principal = getUtility(IPlacelessLoginSource).getPrincipal( |
760 | 1286 | if token.permission == OAuthPermission.UNAUTHORIZED: | 1282 | account_id, access_level=access_level, scope=scope) |
747 | 1287 | raise Unauthorized('Unauthorized token (%s).' % token.key) | ||
748 | 1288 | elif token.date_expires is not None and token.date_expires <= now: | ||
749 | 1289 | raise Unauthorized('Expired token (%s).' % token.key) | ||
750 | 1290 | elif not check_oauth_signature(request, consumer, token): | ||
751 | 1291 | raise Unauthorized('Invalid signature.') | ||
752 | 1292 | else: | ||
753 | 1293 | # Everything is fine, let's return the principal. | ||
754 | 1294 | pass | ||
761 | 1295 | alsoProvides(request, IOAuthSignedRequest) | 1283 | alsoProvides(request, IOAuthSignedRequest) |
762 | 1296 | principal = getUtility(IPlacelessLoginSource).getPrincipal( | ||
763 | 1297 | token.person.account.id, access_level=token.permission, | ||
764 | 1298 | scope=token.context) | ||
765 | 1299 | |||
766 | 1300 | return principal | 1284 | return principal |
767 | 1301 | 1285 | ||
768 | 1302 | 1286 | ||
769 | 1303 | 1287 | ||
770 | === modified file 'lib/canonical/launchpad/webapp/tests/test_publication.py' | |||
771 | --- lib/canonical/launchpad/webapp/tests/test_publication.py 2010-08-20 20:31:18 +0000 | |||
772 | +++ lib/canonical/launchpad/webapp/tests/test_publication.py 2010-09-02 21:17:47 +0000 | |||
773 | @@ -27,6 +27,7 @@ | |||
774 | 27 | ) | 27 | ) |
775 | 28 | from zope.publisher.interfaces import Retry | 28 | from zope.publisher.interfaces import Retry |
776 | 29 | from zope.publisher.interfaces.browser import IBrowserRequest | 29 | from zope.publisher.interfaces.browser import IBrowserRequest |
777 | 30 | from zope.security.proxy import removeSecurityProxy | ||
778 | 30 | 31 | ||
779 | 31 | from canonical.config import dbconfig | 32 | from canonical.config import dbconfig |
780 | 32 | from canonical.launchpad.database.emailaddress import EmailAddress | 33 | from canonical.launchpad.database.emailaddress import EmailAddress |
781 | @@ -254,7 +255,11 @@ | |||
782 | 254 | request_token = consumer.newRequestToken() | 255 | request_token = consumer.newRequestToken() |
783 | 255 | request_token.review( | 256 | request_token.review( |
784 | 256 | person, permission=OAuthPermission.READ_PUBLIC, context=None) | 257 | person, permission=OAuthPermission.READ_PUBLIC, context=None) |
786 | 257 | access_token = request_token.createAccessToken() | 258 | # The security proxy protects against unauthorized requests to |
787 | 259 | # access the OAuth token data. We're using the OAuth token | ||
788 | 260 | # data to _create_ a request, so we can strip the security | ||
789 | 261 | # proxy. | ||
790 | 262 | access_token = removeSecurityProxy(request_token.createAccessToken()) | ||
791 | 258 | 263 | ||
792 | 259 | # Use oauth.OAuthRequest just to generate a dictionary containing all | 264 | # Use oauth.OAuthRequest just to generate a dictionary containing all |
793 | 260 | # the parameters we need to use in a valid OAuth request, using the | 265 | # the parameters we need to use in a valid OAuth request, using the |
794 | 261 | 266 | ||
795 | === modified file 'lib/canonical/launchpad/zcml/oauth.zcml' | |||
796 | --- lib/canonical/launchpad/zcml/oauth.zcml 2009-07-13 18:15:02 +0000 | |||
797 | +++ lib/canonical/launchpad/zcml/oauth.zcml 2010-09-02 21:17:47 +0000 | |||
798 | @@ -26,6 +26,7 @@ | |||
799 | 26 | 26 | ||
800 | 27 | <class class="canonical.launchpad.database.OAuthRequestToken"> | 27 | <class class="canonical.launchpad.database.OAuthRequestToken"> |
801 | 28 | <allow interface="canonical.launchpad.interfaces.IOAuthRequestToken"/> | 28 | <allow interface="canonical.launchpad.interfaces.IOAuthRequestToken"/> |
802 | 29 | <allow interface="canonical.launchpad.interfaces.IOAuthTokenPublic" /> | ||
803 | 29 | <require | 30 | <require |
804 | 30 | permission="launchpad.Edit" | 31 | permission="launchpad.Edit" |
805 | 31 | set_schema="canonical.launchpad.interfaces.IOAuthRequestToken"/> | 32 | set_schema="canonical.launchpad.interfaces.IOAuthRequestToken"/> |
806 | @@ -39,7 +40,11 @@ | |||
807 | 39 | </securedutility> | 40 | </securedutility> |
808 | 40 | 41 | ||
809 | 41 | <class class="canonical.launchpad.database.OAuthAccessToken"> | 42 | <class class="canonical.launchpad.database.OAuthAccessToken"> |
811 | 42 | <allow interface="canonical.launchpad.interfaces.IOAuthAccessToken"/> | 43 | <allow interface="canonical.launchpad.webapp.interfaces.ICanonicalUrlData" /> |
812 | 44 | <allow interface="canonical.launchpad.interfaces.IOAuthAccessTokenPublic"/> | ||
813 | 45 | <require | ||
814 | 46 | permission="launchpad.View" | ||
815 | 47 | interface="canonical.launchpad.interfaces.IOAuthAccessToken"/> | ||
816 | 43 | <require | 48 | <require |
817 | 44 | permission="launchpad.Edit" | 49 | permission="launchpad.Edit" |
818 | 45 | set_schema="canonical.launchpad.interfaces.IOAuthAccessToken"/> | 50 | set_schema="canonical.launchpad.interfaces.IOAuthAccessToken"/> |
819 | 46 | 51 | ||
820 | === modified file 'lib/lp/bugs/tests/test_bugs_webservice.py' | |||
821 | --- lib/lp/bugs/tests/test_bugs_webservice.py 2010-08-30 05:19:46 +0000 | |||
822 | +++ lib/lp/bugs/tests/test_bugs_webservice.py 2010-09-02 21:17:47 +0000 | |||
823 | @@ -10,6 +10,7 @@ | |||
824 | 10 | from BeautifulSoup import BeautifulSoup | 10 | from BeautifulSoup import BeautifulSoup |
825 | 11 | from lazr.lifecycle.interfaces import IDoNotSnapshot | 11 | from lazr.lifecycle.interfaces import IDoNotSnapshot |
826 | 12 | from simplejson import dumps | 12 | from simplejson import dumps |
827 | 13 | from storm.store import Store | ||
828 | 13 | from testtools.matchers import ( | 14 | from testtools.matchers import ( |
829 | 14 | Equals, | 15 | Equals, |
830 | 15 | LessThan, | 16 | LessThan, |
831 | @@ -149,6 +150,7 @@ | |||
832 | 149 | def test_attachments_query_counts_constant(self): | 150 | def test_attachments_query_counts_constant(self): |
833 | 150 | login(USER_EMAIL) | 151 | login(USER_EMAIL) |
834 | 151 | self.bug = self.factory.makeBug() | 152 | self.bug = self.factory.makeBug() |
835 | 153 | store = Store.of(self.bug) | ||
836 | 152 | self.factory.makeBugAttachment(self.bug) | 154 | self.factory.makeBugAttachment(self.bug) |
837 | 153 | self.factory.makeBugAttachment(self.bug) | 155 | self.factory.makeBugAttachment(self.bug) |
838 | 154 | webservice = LaunchpadWebServiceCaller( | 156 | webservice = LaunchpadWebServiceCaller( |
839 | @@ -157,6 +159,9 @@ | |||
840 | 157 | collector.register() | 159 | collector.register() |
841 | 158 | self.addCleanup(collector.unregister) | 160 | self.addCleanup(collector.unregister) |
842 | 159 | url = '/bugs/%d/attachments' % self.bug.id | 161 | url = '/bugs/%d/attachments' % self.bug.id |
843 | 162 | # XXX [bug=619017] First request. | ||
844 | 163 | store.flush() | ||
845 | 164 | store.reset() | ||
846 | 160 | response = webservice.get(url) | 165 | response = webservice.get(url) |
847 | 161 | self.assertThat(collector, HasQueryCount(LessThan(24))) | 166 | self.assertThat(collector, HasQueryCount(LessThan(24))) |
848 | 162 | with_2_count = collector.count | 167 | with_2_count = collector.count |
849 | @@ -164,6 +169,9 @@ | |||
850 | 164 | login(USER_EMAIL) | 169 | login(USER_EMAIL) |
851 | 165 | self.factory.makeBugAttachment(self.bug) | 170 | self.factory.makeBugAttachment(self.bug) |
852 | 166 | logout() | 171 | logout() |
853 | 172 | # XXX [bug=619017] Second request. | ||
854 | 173 | store.flush() | ||
855 | 174 | store.reset() | ||
856 | 167 | response = webservice.get(url) | 175 | response = webservice.get(url) |
857 | 168 | # XXX: Permit the second call to be == or less, because storm | 176 | # XXX: Permit the second call to be == or less, because storm |
858 | 169 | # caching bugs (such as) https://bugs.launchpad.net/storm/+bug/619017 | 177 | # caching bugs (such as) https://bugs.launchpad.net/storm/+bug/619017 |
859 | 170 | 178 | ||
860 | === modified file 'lib/lp/registry/browser/person.py' | |||
861 | --- lib/lp/registry/browser/person.py 2010-08-31 14:00:54 +0000 | |||
862 | +++ lib/lp/registry/browser/person.py 2010-09-02 21:17:47 +0000 | |||
863 | @@ -492,6 +492,15 @@ | |||
864 | 492 | return None | 492 | return None |
865 | 493 | return email | 493 | return email |
866 | 494 | 494 | ||
867 | 495 | @stepthrough('+oauth-access-token') | ||
868 | 496 | def traverse_access_token(self, key): | ||
869 | 497 | """Traverse to an OAuth access token on the webservice layer.""" | ||
870 | 498 | matches = [token for token in self.context.oauth_access_tokens | ||
871 | 499 | if token.key == key] | ||
872 | 500 | if len(matches) > 0: | ||
873 | 501 | return matches[0] | ||
874 | 502 | return None | ||
875 | 503 | |||
876 | 495 | @stepthrough('+wikiname') | 504 | @stepthrough('+wikiname') |
877 | 496 | def traverse_wikiname(self, id): | 505 | def traverse_wikiname(self, id): |
878 | 497 | """Traverse to this person's WikiNames on the webservice layer.""" | 506 | """Traverse to this person's WikiNames on the webservice layer.""" |
879 | 498 | 507 | ||
880 | === modified file 'lib/lp/registry/interfaces/person.py' | |||
881 | --- lib/lp/registry/interfaces/person.py 2010-08-22 03:09:51 +0000 | |||
882 | +++ lib/lp/registry/interfaces/person.py 2010-09-02 21:17:47 +0000 | |||
883 | @@ -103,6 +103,7 @@ | |||
884 | 103 | IHasMugshot, | 103 | IHasMugshot, |
885 | 104 | IPrivacy, | 104 | IPrivacy, |
886 | 105 | ) | 105 | ) |
887 | 106 | from canonical.launchpad.interfaces.oauth import IOAuthAccessToken | ||
888 | 106 | from canonical.launchpad.interfaces.validation import validate_new_team_email | 107 | from canonical.launchpad.interfaces.validation import validate_new_team_email |
889 | 107 | from canonical.launchpad.validators import LaunchpadValidationError | 108 | from canonical.launchpad.validators import LaunchpadValidationError |
890 | 108 | from canonical.launchpad.validators.email import email_validator | 109 | from canonical.launchpad.validators.email import email_validator |
891 | @@ -596,7 +597,10 @@ | |||
892 | 596 | # apart from here. | 597 | # apart from here. |
893 | 597 | registrant = Attribute('The user who created this profile.') | 598 | registrant = Attribute('The user who created this profile.') |
894 | 598 | 599 | ||
896 | 599 | oauth_access_tokens = Attribute(_("Non-expired access tokens")) | 600 | oauth_access_tokens = exported( |
897 | 601 | CollectionField( | ||
898 | 602 | title=u"Non-expired access tokens", | ||
899 | 603 | value_type=Reference(schema=IOAuthAccessToken))) | ||
900 | 600 | 604 | ||
901 | 601 | oauth_request_tokens = Attribute(_("Non-expired request tokens")) | 605 | oauth_request_tokens = Attribute(_("Non-expired request tokens")) |
902 | 602 | 606 | ||
903 | 603 | 607 | ||
904 | === modified file 'lib/lp/registry/stories/webservice/xx-person.txt' | |||
905 | --- lib/lp/registry/stories/webservice/xx-person.txt 2010-07-13 15:29:08 +0000 | |||
906 | +++ lib/lp/registry/stories/webservice/xx-person.txt 2010-09-02 21:17:47 +0000 | |||
907 | @@ -37,6 +37,7 @@ | |||
908 | 37 | memberships_details_collection_link: u'http://.../~salgado/memberships_details' | 37 | memberships_details_collection_link: u'http://.../~salgado/memberships_details' |
909 | 38 | mugshot_link: u'http://.../~salgado/mugshot' | 38 | mugshot_link: u'http://.../~salgado/mugshot' |
910 | 39 | name: u'salgado' | 39 | name: u'salgado' |
911 | 40 | oauth_access_tokens_collection_link: u'http://.../~salgado/oauth_access_tokens' | ||
912 | 40 | open_membership_invitations_collection_link: u'http://.../~salgado/open_membership_invitations' | 41 | open_membership_invitations_collection_link: u'http://.../~salgado/open_membership_invitations' |
913 | 41 | participants_collection_link: u'http://.../~salgado/participants' | 42 | participants_collection_link: u'http://.../~salgado/participants' |
914 | 42 | ppas_collection_link: u'http://.../~salgado/ppas' | 43 | ppas_collection_link: u'http://.../~salgado/ppas' |
915 | @@ -86,6 +87,7 @@ | |||
916 | 86 | memberships_details_collection_link: u'http://.../~ubuntu-team/memberships_details' | 87 | memberships_details_collection_link: u'http://.../~ubuntu-team/memberships_details' |
917 | 87 | mugshot_link: u'http://.../~ubuntu-team/mugshot' | 88 | mugshot_link: u'http://.../~ubuntu-team/mugshot' |
918 | 88 | name: u'ubuntu-team' | 89 | name: u'ubuntu-team' |
919 | 90 | oauth_access_tokens_collection_link: u'http://.../~ubuntu-team/oauth_access_tokens' | ||
920 | 89 | open_membership_invitations_collection_link: u'http://.../~ubuntu-team/open_membership_invitations' | 91 | open_membership_invitations_collection_link: u'http://.../~ubuntu-team/open_membership_invitations' |
921 | 90 | participants_collection_link: u'http://.../~ubuntu-team/participants' | 92 | participants_collection_link: u'http://.../~ubuntu-team/participants' |
922 | 91 | ppas_collection_link: u'http://.../~ubuntu-team/ppas' | 93 | ppas_collection_link: u'http://.../~ubuntu-team/ppas' |
923 | 92 | 94 | ||
924 | === modified file 'lib/lp/testing/_webservice.py' | |||
925 | --- lib/lp/testing/_webservice.py 2010-08-20 20:31:18 +0000 | |||
926 | +++ lib/lp/testing/_webservice.py 2010-09-02 21:17:47 +0000 | |||
927 | @@ -24,6 +24,7 @@ | |||
928 | 24 | from zope.app.publication.interfaces import IEndRequestEvent | 24 | from zope.app.publication.interfaces import IEndRequestEvent |
929 | 25 | from zope.app.testing import ztapi | 25 | from zope.app.testing import ztapi |
930 | 26 | from zope.component import getUtility | 26 | from zope.component import getUtility |
931 | 27 | from zope.security.proxy import removeSecurityProxy | ||
932 | 27 | import zope.testing.cleanup | 28 | import zope.testing.cleanup |
933 | 28 | 29 | ||
934 | 29 | from canonical.launchpad.interfaces import ( | 30 | from canonical.launchpad.interfaces import ( |
935 | @@ -72,19 +73,23 @@ | |||
936 | 72 | # We didn't have to create the consumer. Maybe this user | 73 | # We didn't have to create the consumer. Maybe this user |
937 | 73 | # already has an access token for this | 74 | # already has an access token for this |
938 | 74 | # consumer+person+permission? | 75 | # consumer+person+permission? |
945 | 75 | existing_token = [token for token in person.oauth_access_tokens | 76 | for token in person.oauth_access_tokens: |
946 | 76 | if (token.consumer == consumer | 77 | # The security proxy on OAuthAccessToken assumes an access |
947 | 77 | and token.permission == permission | 78 | # token has already been associated with the current |
948 | 78 | and token.context == context)] | 79 | # request. Since there is no current request, it doesn't |
949 | 79 | if len(existing_token) >= 1: | 80 | # make sense to run those security checks. |
950 | 80 | return existing_token[0] | 81 | token = removeSecurityProxy(token) |
951 | 82 | if (token.consumer == consumer | ||
952 | 83 | and token.permission == permission | ||
953 | 84 | and token.context == context): | ||
954 | 85 | return token | ||
955 | 81 | 86 | ||
956 | 82 | # There is no existing access token for this | 87 | # There is no existing access token for this |
957 | 83 | # consumer+person+permission+context. Create one and review it. | 88 | # consumer+person+permission+context. Create one and review it. |
958 | 84 | request_token = consumer.newRequestToken() | 89 | request_token = consumer.newRequestToken() |
959 | 85 | request_token.review(person, permission, context) | 90 | request_token.review(person, permission, context) |
960 | 86 | access_token = request_token.createAccessToken() | 91 | access_token = request_token.createAccessToken() |
962 | 87 | return access_token | 92 | return removeSecurityProxy(access_token) |
963 | 88 | 93 | ||
964 | 89 | 94 | ||
965 | 90 | def launchpadlib_credentials_for( | 95 | def launchpadlib_credentials_for( |
966 | @@ -109,8 +114,7 @@ | |||
967 | 109 | access_token = oauth_access_token_for( | 114 | access_token = oauth_access_token_for( |
968 | 110 | consumer_name, person, permission, context) | 115 | consumer_name, person, permission, context) |
969 | 111 | logout() | 116 | logout() |
972 | 112 | launchpadlib_token = AccessToken( | 117 | launchpadlib_token = AccessToken(access_token.key, access_token.secret) |
971 | 113 | access_token.key, access_token.secret) | ||
973 | 114 | return Credentials(consumer_name=consumer_name, | 118 | return Credentials(consumer_name=consumer_name, |
974 | 115 | access_token=launchpadlib_token) | 119 | access_token=launchpadlib_token) |
975 | 116 | 120 | ||
976 | 117 | 121 | ||
977 | === modified file 'versions.cfg' | |||
978 | --- versions.cfg 2010-09-01 08:41:40 +0000 | |||
979 | +++ versions.cfg 2010-09-02 21:17:47 +0000 | |||
980 | @@ -31,7 +31,7 @@ | |||
981 | 31 | lazr.delegates = 1.2.0 | 31 | lazr.delegates = 1.2.0 |
982 | 32 | lazr.enum = 1.1.2 | 32 | lazr.enum = 1.1.2 |
983 | 33 | lazr.lifecycle = 1.1 | 33 | lazr.lifecycle = 1.1 |
985 | 34 | lazr.restful = 0.11.3 | 34 | lazr.restful = 0.12.0 |
986 | 35 | lazr.restfulclient = 0.10.0 | 35 | lazr.restfulclient = 0.10.0 |
987 | 36 | lazr.smtptest = 1.1 | 36 | lazr.smtptest = 1.1 |
988 | 37 | lazr.testing = 0.1.1 | 37 | lazr.testing = 0.1.1 |
lib/canonical/ launchpad/ pagetests/ webservice/ oauth.txt and most of the changes to lib/canonical/ launchpad/ doc/oauth. txt look like their primary purpose is to provide test coverage, not to provide documentation, so please write them as UnitTests.
A lot of the formatting of line-wraps doesn't match our official style: https:/ /dev.launchpad. net/PythonStyle Guide#Whitespac e%20and% 20Wrapping and often matches the "Do not indent your code like this" style. Please fix.
It appears that the removeSecurityProxy in lib/canonical/ launchpad/ webapp/ servers. py could be eliminated by moving some of the code into a method on OAuthAccessToken. Please try this, and if it's not possible, provide a follow-up comment explaining this.