Skip to content
This repository was archived by the owner on Mar 3, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion provider/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = '1.0.2'
__version__ = '1.0.3'
19 changes: 12 additions & 7 deletions provider/oauth2/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from django.utils.encoding import smart_unicode
from django.utils.translation import ugettext as _

from provider import constants, scope
from provider import scope
from provider.constants import RESPONSE_TYPE_CHOICES, SCOPES
from provider.forms import OAuthForm, OAuthValidationError
from provider.oauth2.models import Client, Grant, RefreshToken
Expand Down Expand Up @@ -340,10 +340,15 @@ def clean(self):

class ClientCredentialsGrantForm(ScopeMixin, OAuthForm):
""" Validate a client credentials grant request. """
scope = forms.CharField(required=False)

def clean(self):
cleaned_data = super(ClientCredentialsGrantForm, self).clean()
# We do not fully support scopes for this grant type; however, a scope is required
# in order to create an access token. Default to read-only access.
cleaned_data['scope'] = constants.READ
return cleaned_data
def clean_scope(self):
# NOTE (CCB): This is a horrible hack, like much of our OAuth work. The scopes are declared in

Choose a reason for hiding this comment

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

Is this a temporary hack that will be cleaned up once we migrate completely to DOT?

# edx-oauth2-provider. (See edx_oauth2_provider/constants.py.) However, we need to provide a default scope
# that (a) gives the token basic read access and (b) allows access to the user info endpoint. This value
# represents the following scopes: openid (1), profile (2), email (4), permissions (32). At present, this is
# all scopes except course_staff and course_instructor. These scopes are normally associated with actual
# users, whereas the client credentials grant will primarily be used by service users.
#
# In the future, we should limit the allowable scopes either at a global or per-client level.
return 39
Copy link

Choose a reason for hiding this comment

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

Why not use the actual scopes instead of a magic number with a lengthy comment describing it?

return constants.OPENID | constants.PROFILE | constants.EMAIL | constants.PERMISSION

Are we trying to avoid making edx-django-oauth2-provider dependent on edx-oauth2-provider?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, we end up with circular dependencies.

12 changes: 9 additions & 3 deletions provider/oauth2/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ def setUp(self):
super(ClientCredentialsAccessTokenTests, self).setUp()
AccessToken.objects.all().delete()

def request_access_token(self, client_id=None, client_secret=None):
def request_access_token(self, client_id=None, client_secret=None, scope=None):
""" Issues an access token request using the client credentials grant.

Arguments:
Expand All @@ -614,6 +614,11 @@ def request_access_token(self, client_id=None, client_secret=None):
'client_secret': client_secret or client.client_secret,
}

if scope:
data.update({
'scope': scope,
})

return self.client.post(self.access_token_url(), data)

def assert_valid_access_token_response(self, access_token, response):
Expand All @@ -634,9 +639,10 @@ def assert_valid_access_token_response(self, access_token, response):
def get_latest_access_token(self):
return AccessToken.objects.filter(client=self.get_client()).order_by('-id')[0]

def test_authorize_success(self):
@ddt.data(None, 'read')

Choose a reason for hiding this comment

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

Can you use constants.READ here instead?
Also, didn't these 2 tests (read and None) work even before this PR?

def test_authorize_success(self, scope):
""" Verify the endpoint successfully issues an access token using the client credentials grant. """
response = self.request_access_token()
response = self.request_access_token(scope=scope)
self.assertEqual(200, response.status_code, response.content)

access_token = self.get_latest_access_token()
Expand Down