Merge lp:~sinzui/launchpad/expire-oauth-token into lp:launchpad

Proposed by Curtis Hovey on 2010-02-25
Status: Merged
Merged at revision: not available
Proposed branch: lp:~sinzui/launchpad/expire-oauth-token
Merge into: lp:launchpad
Diff against target: 33 lines (+11/-1)
2 files modified
lib/canonical/launchpad/doc/oauth.txt (+10/-0)
lib/canonical/launchpad/security.py (+1/-1)
To merge this branch: bzr merge lp:~sinzui/launchpad/expire-oauth-token
Reviewer Review Type Date Requested Status
Michael Nelson (community) code 2010-02-25 Approve on 2010-02-25
Review via email: mp+20094@code.launchpad.net
To post a comment you must log in.
Curtis Hovey (sinzui) wrote :

This is my branch to fix OAuth access token permissions.

    lp:~sinzui/launchpad/portlet-package-summary-timeout
    Diff size: 34
    Launchpad bug: https://bugs.launchpad.net/bugs/511567
    Test command: ./bin/test -vv -t doc/oauth.txt
    Pre-implementation: no one
    Target release: 10.02

Fix OAuth access token permissions
----------------------------------

I have bughugger authorised as an application which can access launchpad on
my behalf.

Out of curiosity, I tried to remove its authorisation from launchpad:
And I get the following error:

    Not allowed here
    Sorry, you don't have permission to access this page.

Rules
-----

    * Fix the security checker, which is checking a decorator, not the person.
      * It was broken when henning added his nice security utility...the
        test uses an admin, not a regualr user, so the problem was not caught.

QA
--

    * Visit https://edge.launchpad.net/people/+me/+oauth-tokens
    * Revoke a script.
    * Verify it is gone; there is no 403 error.

Lint
----

Linting changed files:
  lib/canonical/launchpad/security.py
  lib/canonical/launchpad/doc/oauth.txt

Test
----

    * lib/canonical/launchpad/doc/oauth.txt
      * Added a test to show a *non-admin* can change his access token.

Implementation
--------------

    * lib/canonical/launchpad/security.py
      * Updated the security checker to user user.person not the decorator
        object.

Michael Nelson (michael.nelson) wrote :

Thanks Curtis for the explanation and simple fix.

I wondered at first why you hadn't revoked the token in your test (I expected to see something like api_access_token.revoke() encapsulating the setting of date_expires), but there is no such method on IOAuthAccessToken.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/doc/oauth.txt'
2--- lib/canonical/launchpad/doc/oauth.txt 2010-02-17 11:13:06 +0000
3+++ lib/canonical/launchpad/doc/oauth.txt 2010-02-25 00:05:32 +0000
4@@ -265,6 +265,16 @@
5 >>> salgado.oauth_request_tokens.count()
6 4
7
8+A user has edit permission over his own access tokens, he can expire them.
9+
10+ >>> api_user = factory.makePerson()
11+ >>> login_person(api_user)
12+ >>> api_request_token = consumer.newRequestToken()
13+ >>> api_request_token.review(api_user, OAuthPermission.WRITE_PUBLIC)
14+ >>> api_access_token = api_request_token.createAccessToken()
15+ >>> api_access_token.date_expires = (
16+ ... datetime.now(pytz.timezone('UTC')) - timedelta(hours=1))
17+
18
19 == Nonces and timestamps ==
20
21
22=== modified file 'lib/canonical/launchpad/security.py'
23--- lib/canonical/launchpad/security.py 2010-02-23 19:40:45 +0000
24+++ lib/canonical/launchpad/security.py 2010-02-25 00:05:32 +0000
25@@ -297,7 +297,7 @@
26 usedfor = IOAuthAccessToken
27
28 def checkAuthenticated(self, user):
29- return self.obj.person == user or user.in_admin
30+ return self.obj.person == user.person or user.in_admin
31
32
33 class EditOAuthRequestToken(EditOAuthAccessToken):