-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce a new Permission class to enforce scopes #1
Changes from 4 commits
4972ace
6f08c49
8208023
214b719
6ef597b
fe45a0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,58 @@ | ||
""" Permission classes. """ | ||
from rest_framework.permissions import BasePermission | ||
from provider.oauth2.models import AccessToken as DOPAccessToken | ||
from edx_rest_framework_extensions.utils import jwt_decode_handler | ||
from oauth2_provider.ext.rest_framework.permissions import TokenHasScope | ||
|
||
|
||
class IsSuperuser(BasePermission): | ||
""" Allows access only to superusers. """ | ||
|
||
def has_permission(self, request, view): | ||
return request.user and request.user.is_superuser | ||
|
||
|
||
class JWTRestrictedApplicationPermission(BasePermission): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This might be better as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
""" | ||
The request is authenticated and has the required scopes and organization filters | ||
""" | ||
|
||
def _token_filters(self, decoded_token): | ||
# get filters list from jwt token and return dict | ||
if 'filters' in decoded_token: | ||
filters_list = decoded_token['filters'] | ||
filters = {} | ||
for each in filters_list: | ||
each = each.split(':') | ||
if each[0] in filters.keys(): | ||
filters[each[0]].append(each[1]) | ||
else: | ||
filters[each[0]] = [each[1]] | ||
return filters | ||
|
||
def has_permission(self, request, view): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reminder: check for feature toggle. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
""" | ||
Implement the business logic discussed above | ||
""" | ||
|
||
token = request.auth | ||
decoded_token = jwt_decode_handler(token) | ||
# check to see if token is a DOP token | ||
# if so this represents a client which is implicitly trusted | ||
# (since it is an internal Open edX application) | ||
if isinstance(token, DOPAccessToken): | ||
return True | ||
|
||
if not float(decoded_token['version']) <= 1.0: | ||
return False | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nasthagiri Is this what you were thinking when verifying the token version? I'm not sure when to return There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Get the current version "1.0" from the setting file instead of hardcoding. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
||
if hasattr(view, 'required_scopes'): | ||
if not getattr(view, 'required_scopes')[0] in decoded_token['scopes']: | ||
return False | ||
|
||
has_permission = super(JWTRestrictedApplicationPermission, self).has_permission(request, view) | ||
|
||
if has_permission: | ||
setattr(request, 'oauth_scopes_filters', self._token_filters(decoded_token)) | ||
|
||
return has_permission |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class needs to extend DOT's TokenHasScope, otherwise never actually verify the token's scopes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Earlier we used TokenHasScope, but its restricted to OAuth2Authentication.
Instead, user getattr from the view to validate the scope.