Updated the default scopes of the client credentials grant #32
Conversation
|
@nasthagiri please review. The client credentials grant type doesn't have a grant, like the other grant types, so there is nothing to check to ensure the client is not requesting greater access than is actually permitted. The client can request any scopes. Ultimately, the provider needs to enforce the limitation for either all, or specific, clients. |
clintonb
force-pushed
the
clintonb/client-credential-update
branch
5 times, most recently
from
db100a2 to
c6b191e
Compare
clintonb
force-pushed
the
clintonb/client-credential-update
branch
from
c6b191e to
7ab63cb
Compare
| # 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Yes, we end up with circular dependencies.
| @@ -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') | |||
There was a problem hiding this comment.
Can you use constants.READ here instead?
Also, didn't these 2 tests (read and None) work even before this PR?
Fixes issue introduced by #25.
ECOM-4196