Skip to content

Commit

Permalink
Add permissions to the login api
Browse files Browse the repository at this point in the history
  • Loading branch information
mattbasta committed May 2, 2013
1 parent bf8a15a commit fbfb205
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 0 deletions.
10 changes: 10 additions & 0 deletions docs/api/topics/authentication.rst
Expand Up @@ -38,6 +38,9 @@ stored as a cookie.
sent with authorized requests as a query string parameter named
``_user``.
:param settings: user account settings.
:param permissions: permissions and properties for the user account. It
contains boolean values which describe whether the user has the
permission described by the key of the field.

This comment has been minimized.

Copy link
@andymckay

andymckay May 2, 2013

seems odd that the only way i can get this data is on login with sharedsecret rather than being on a resource i can access any time

This comment has been minimized.

Copy link
@mattbasta

mattbasta May 2, 2013

Author Owner

I would have otherwise created a resource like /settings/mine, but we wouldn't use it since it's really unlikely that these values are going to change during a session. Perhaps someday.

This comment has been minimized.

Copy link
@andymckay

andymckay May 2, 2013

yeah but you can't see this information if you use oauth

This comment has been minimized.

Copy link
@mattbasta

mattbasta via email May 2, 2013

Author Owner

This comment has been minimized.

Copy link
@andymckay

andymckay May 2, 2013

In the case of three legged oauth you have a third party (such as PhoneGap) operating as a user. In that scenario they would not know what permissions they have. I can't imagine that they might need that, but there seems no point in excluding users from using an API if theres no good reason for doing that. It would be pretty straightforward for example to make permissions/mine and then pull that into the logged in.


Example:

Expand All @@ -50,6 +53,13 @@ stored as a cookie.
"display_name": "fred foobar",
"email": "ffoob@example.com",
"region": "appistan"
},
"permissions": {
"reviewer": false,
"admin": false,
"localizer": false,
"lookup": true,
"developer": true
}
}
Expand Down
10 changes: 10 additions & 0 deletions mkt/account/api.py
Expand Up @@ -10,6 +10,7 @@
from tastypie.exceptions import ImmediateHttpResponse
from tastypie.throttle import CacheThrottle

from access import acl
from amo.utils import send_mail_jinja
from mkt.api.authentication import (OAuthAuthentication,
OptionalOAuthAuthentication,
Expand Down Expand Up @@ -104,6 +105,15 @@ def post_list(self, request, **kwargs):
'display_name': (UserProfile.objects
.get(user=request.user).display_name),
'email': request.user.email,
},
'permissions': {
'reviewer': acl.check_reviewer(request),
'admin': acl.action_allowed(request, 'Admin', '%'),
'localizer': acl.action_allowed(
request, 'Localizers', '%'),
'lookup': acl.action_allowed(
request, 'AccountLookup', '%'),
'developer': request.amo_user.is_app_developer,
}
})
return res
Expand Down
5 changes: 5 additions & 0 deletions mkt/account/tests/test_api.py
Expand Up @@ -154,6 +154,11 @@ def test_login_success(self, http_request):
'cvan@mozilla.com,95c9063d9f249aacfe5697fc83192ed6480c01463e2a80b3'
'5af5ecaef11754700f4be33818d0e83a0cfc2cab365d60ba53b3c2b9f8f6589d1'
'c43e9bbb876eef0,000000')
assert 'permissions' in res.content
assert all(x in res.content['permissions'] for x in
['reviewer', 'admin', 'localizer', 'lookup', 'developer'])
assert all(isinstance(x, bool) for x in
res.content['permissions'].values())

@patch('requests.post')
def test_login_failure(self, http_request):
Expand Down

2 comments on commit fbfb205

@andymckay
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there a bug # for this?

@mattbasta
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, it's to address a kruplaint

Please sign in to comment.