Skip to content

Commit

Permalink
fixed client credential flow which now returns an access token not bo…
Browse files Browse the repository at this point in the history
…und to an user. ref issue #38
  • Loading branch information
synasius committed Mar 22, 2015
1 parent 113c731 commit 628f9e6
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 3 deletions.
2 changes: 1 addition & 1 deletion oauth2_provider/oauth2_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def save_bearer_token(self, token, request, *args, **kwargs):

expires = timezone.now() + timedelta(seconds=oauth2_settings.ACCESS_TOKEN_EXPIRE_SECONDS)
if request.grant_type == 'client_credentials':
request.user = request.client.user
request.user = None

This comment has been minimized.

Copy link
@leonid-s-usov

leonid-s-usov Nov 3, 2015

Please explain me, why was this implemented by clearing the user on request object, instead of just making sure to not store the user in the created AccessToken and RefreshToken?
In the current version this validation method has a side effect of removing the current user from request, which is IMHO unacceptable given that the request object is not owned by this class, nor by the OAuth lib in general

This comment has been minimized.

Copy link
@synasius

synasius Nov 3, 2015

Author Contributor

The request here is an instance of 'oauthlib.Request'. Can you describe the side effect you have in Django?

This comment has been minimized.

Copy link
@synasius

synasius Nov 3, 2015

Author Contributor

@leonid-s-usov ok, I just read your comment on #38

This comment has been minimized.

Copy link
@leonid-s-usov

leonid-s-usov Nov 3, 2015

Actually, I was reading the source before planning to implement my use case, and I was sure that the values from this request instance are being copied to the Django request. Now that you mentioned this I have double checked and saw that the request returned from validate_request is not being merged with the HttpRequest object.
So, thanks for clarification.

Nevertheless, other points from my comment apply.

This comment has been minimized.

Copy link
@vinayan3

vinayan3 Nov 17, 2015

The impact to my use of this library is that now the AccessToken which in older versions would have a user associated with now has no user associated with it. We used that AccessToken's user column to determine which user is contacting me on subsequent requests.


access_token = AccessToken(
user=request.user,
Expand Down
15 changes: 13 additions & 2 deletions oauth2_provider/tests/test_client_credential.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

from oauthlib.oauth2 import BackendApplicationServer

from ..models import get_application_model
from ..models import get_application_model, AccessToken
from ..oauth2_validators import OAuth2Validator
from ..settings import oauth2_settings
from ..views import ProtectedResourceView
Expand Down Expand Up @@ -93,6 +93,17 @@ def test_client_credential_does_not_issue_refresh_token(self):
content = json.loads(response.content.decode("utf-8"))
self.assertNotIn("refresh_token", content)

def test_client_credential_user_is_none_on_access_token(self):
token_request_data = {'grant_type': 'client_credentials'}
auth_headers = self.get_basic_auth_header(self.application.client_id, self.application.client_secret)

response = self.client.post(reverse('oauth2_provider:token'), data=token_request_data, **auth_headers)
self.assertEqual(response.status_code, 200)

content = json.loads(response.content.decode("utf-8"))
access_token = AccessToken.objects.get(token=content["access_token"])
self.assertIsNone(access_token.user)


class TestExtendedRequest(BaseTest):
@classmethod
Expand Down Expand Up @@ -130,7 +141,7 @@ def get_scopes(self):

valid, r = test_view.verify_request(request)
self.assertTrue(valid)
self.assertEqual(r.user, self.dev_user)
self.assertIsNone(r.user)
self.assertEqual(r.client, self.application)
self.assertEqual(r.scopes, ['read', 'write'])

Expand Down

0 comments on commit 628f9e6

Please sign in to comment.