diff options
author | Jonathan Huot <JonathanHuot@users.noreply.github.com> | 2019-02-21 11:30:55 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-02-21 11:30:55 +0100 |
commit | f0fc21c7bfd59bba0103577c5f9ea810978dc8cb (patch) | |
tree | fa82d70ef532771e15167b8f695a95f982abddf1 | |
parent | 00c0c3613879396e6511e9fc48d6ba5a6d7d746f (diff) | |
parent | 2904de612a5e52c14776978dd5a31cdde2bfc34e (diff) | |
download | oauthlib-f0fc21c7bfd59bba0103577c5f9ea810978dc8cb.tar.gz |
Merge pull request #653 from oauthlib/652-token_state
Fix 652: removed "state" from /token response.
-rw-r--r-- | CHANGELOG.rst | 4 | ||||
-rw-r--r-- | oauthlib/oauth2/rfc6749/grant_types/authorization_code.py | 4 | ||||
-rw-r--r-- | oauthlib/oauth2/rfc6749/grant_types/client_credentials.py | 3 | ||||
-rw-r--r-- | oauthlib/oauth2/rfc6749/grant_types/implicit.py | 5 | ||||
-rw-r--r-- | oauthlib/oauth2/rfc6749/grant_types/refresh_token.py | 3 | ||||
-rw-r--r-- | oauthlib/oauth2/rfc6749/grant_types/resource_owner_password_credentials.py | 3 | ||||
-rw-r--r-- | oauthlib/oauth2/rfc6749/tokens.py | 18 | ||||
-rw-r--r-- | oauthlib/openid/connect/core/grant_types/base.py | 3 | ||||
-rw-r--r-- | oauthlib/openid/connect/core/grant_types/implicit.py | 5 | ||||
-rw-r--r-- | oauthlib/openid/connect/core/tokens.py | 2 | ||||
-rw-r--r-- | tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py | 15 | ||||
-rw-r--r-- | tests/oauth2/rfc6749/test_server.py | 39 | ||||
-rw-r--r-- | tests/openid/connect/core/test_server.py | 16 |
13 files changed, 70 insertions, 50 deletions
diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a5cb324..9e0efda 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,10 @@ Changelog ========= +TBD +------------------ +* #652: Fixed OIDC /token response which wrongly returned "&state=None" + 3.0.1 (2019-01-24) ------------------ * Fixed OAuth2.0 regression introduced in 3.0.0: Revocation with Basic auth no longer possible #644 diff --git a/oauthlib/oauth2/rfc6749/grant_types/authorization_code.py b/oauthlib/oauth2/rfc6749/grant_types/authorization_code.py index 6463391..5f03d9c 100644 --- a/oauthlib/oauth2/rfc6749/grant_types/authorization_code.py +++ b/oauthlib/oauth2/rfc6749/grant_types/authorization_code.py @@ -305,9 +305,11 @@ class AuthorizationCodeGrant(GrantTypeBase): headers.update(e.headers) return headers, e.json, e.status_code - token = token_handler.create_token(request, refresh_token=self.refresh_token, save_token=False) + token = token_handler.create_token(request, refresh_token=self.refresh_token) + for modifier in self._token_modifiers: token = modifier(token, token_handler, request) + self.request_validator.save_token(token, request) self.request_validator.invalidate_authorization_code( request.client_id, request.code, request) diff --git a/oauthlib/oauth2/rfc6749/grant_types/client_credentials.py b/oauthlib/oauth2/rfc6749/grant_types/client_credentials.py index c966795..7e50857 100644 --- a/oauthlib/oauth2/rfc6749/grant_types/client_credentials.py +++ b/oauthlib/oauth2/rfc6749/grant_types/client_credentials.py @@ -76,10 +76,11 @@ class ClientCredentialsGrant(GrantTypeBase): headers.update(e.headers) return headers, e.json, e.status_code - token = token_handler.create_token(request, refresh_token=False, save_token=False) + token = token_handler.create_token(request, refresh_token=False) for modifier in self._token_modifiers: token = modifier(token) + self.request_validator.save_token(token, request) log.debug('Issuing token to client id %r (%r), %r.', diff --git a/oauthlib/oauth2/rfc6749/grant_types/implicit.py b/oauthlib/oauth2/rfc6749/grant_types/implicit.py index d6de906..48bae7a 100644 --- a/oauthlib/oauth2/rfc6749/grant_types/implicit.py +++ b/oauthlib/oauth2/rfc6749/grant_types/implicit.py @@ -237,10 +237,13 @@ class ImplicitGrant(GrantTypeBase): # "id_token token" - return the access token and the id token # "id_token" - don't return the access token if "token" in request.response_type.split(): - token = token_handler.create_token(request, refresh_token=False, save_token=False) + token = token_handler.create_token(request, refresh_token=False) else: token = {} + if request.state is not None: + token['state'] = request.state + for modifier in self._token_modifiers: token = modifier(token, token_handler, request) diff --git a/oauthlib/oauth2/rfc6749/grant_types/refresh_token.py b/oauthlib/oauth2/rfc6749/grant_types/refresh_token.py index bd519e8..fc61d65 100644 --- a/oauthlib/oauth2/rfc6749/grant_types/refresh_token.py +++ b/oauthlib/oauth2/rfc6749/grant_types/refresh_token.py @@ -64,10 +64,11 @@ class RefreshTokenGrant(GrantTypeBase): return headers, e.json, e.status_code token = token_handler.create_token(request, - refresh_token=self.issue_new_refresh_tokens, save_token=False) + refresh_token=self.issue_new_refresh_tokens) for modifier in self._token_modifiers: token = modifier(token) + self.request_validator.save_token(token, request) log.debug('Issuing new token to client id %r (%r), %r.', diff --git a/oauthlib/oauth2/rfc6749/grant_types/resource_owner_password_credentials.py b/oauthlib/oauth2/rfc6749/grant_types/resource_owner_password_credentials.py index f765d91..5929afb 100644 --- a/oauthlib/oauth2/rfc6749/grant_types/resource_owner_password_credentials.py +++ b/oauthlib/oauth2/rfc6749/grant_types/resource_owner_password_credentials.py @@ -104,10 +104,11 @@ class ResourceOwnerPasswordCredentialsGrant(GrantTypeBase): headers.update(e.headers) return headers, e.json, e.status_code - token = token_handler.create_token(request, self.refresh_token, save_token=False) + token = token_handler.create_token(request, self.refresh_token) for modifier in self._token_modifiers: token = modifier(token) + self.request_validator.save_token(token, request) log.debug('Issuing token %r to client id %r (%r) and username %s.', diff --git a/oauthlib/oauth2/rfc6749/tokens.py b/oauthlib/oauth2/rfc6749/tokens.py index d78df09..7973923 100644 --- a/oauthlib/oauth2/rfc6749/tokens.py +++ b/oauthlib/oauth2/rfc6749/tokens.py @@ -12,6 +12,7 @@ from __future__ import absolute_import, unicode_literals import hashlib import hmac from binascii import b2a_base64 +import warnings from oauthlib import common from oauthlib.common import add_params_to_qs, add_params_to_uri, unicode_type @@ -296,15 +297,18 @@ class BearerToken(TokenBase): ) self.expires_in = expires_in or 3600 - def create_token(self, request, refresh_token=False, save_token=True): + def create_token(self, request, refresh_token=False, **kwargs): """ Create a BearerToken, by default without refresh token. - + :param request: OAuthlib request. :type request: oauthlib.common.Request :param refresh_token: - :param save_token: """ + if "save_token" in kwargs: + warnings.warn("`save_token` has been deprecated, it was not called internally." + "If you do, call `request_validator.save_token()` instead.", + DeprecationWarning) if callable(self.expires_in): expires_in = self.expires_in(request) @@ -325,9 +329,6 @@ class BearerToken(TokenBase): if request.scopes is not None: token['scope'] = ' '.join(request.scopes) - if request.state is not None: - token['state'] = request.state - if refresh_token: if (request.refresh_token and not self.request_validator.rotate_refresh_token(request)): @@ -336,10 +337,7 @@ class BearerToken(TokenBase): token['refresh_token'] = self.refresh_token_generator(request) token.update(request.extra_credentials or {}) - token = OAuth2Token(token) - if save_token: - self.request_validator.save_bearer_token(token, request) - return token + return OAuth2Token(token) def validate_request(self, request): """ diff --git a/oauthlib/openid/connect/core/grant_types/base.py b/oauthlib/openid/connect/core/grant_types/base.py index fa578a5..05cdd37 100644 --- a/oauthlib/openid/connect/core/grant_types/base.py +++ b/oauthlib/openid/connect/core/grant_types/base.py @@ -58,9 +58,6 @@ class GrantTypeBase(object): if request.response_type and 'id_token' not in request.response_type: return token - if 'state' not in token: - token['state'] = request.state - if request.max_age: d = datetime.datetime.utcnow() token['auth_time'] = d.isoformat("T") + "Z" diff --git a/oauthlib/openid/connect/core/grant_types/implicit.py b/oauthlib/openid/connect/core/grant_types/implicit.py index 0eaa5b3..0a6fcb7 100644 --- a/oauthlib/openid/connect/core/grant_types/implicit.py +++ b/oauthlib/openid/connect/core/grant_types/implicit.py @@ -26,3 +26,8 @@ class ImplicitGrant(GrantTypeBase): self.custom_validators.post_auth.append( self.openid_implicit_authorization_validator) self.register_token_modifier(self.add_id_token) + + def add_id_token(self, token, token_handler, request): + if 'state' not in token: + token['state'] = request.state + return super(ImplicitGrant, self).add_id_token(token, token_handler, request) diff --git a/oauthlib/openid/connect/core/tokens.py b/oauthlib/openid/connect/core/tokens.py index 6b68891..b67cdf2 100644 --- a/oauthlib/openid/connect/core/tokens.py +++ b/oauthlib/openid/connect/core/tokens.py @@ -25,7 +25,7 @@ class JWTToken(TokenBase): ) self.expires_in = expires_in or 3600 - def create_token(self, request, refresh_token=False, save_token=False): + def create_token(self, request, refresh_token=False): """Create a JWT Token, using requestvalidator method.""" if callable(self.expires_in): diff --git a/tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py b/tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py index 1a2f66b..e7c66b6 100644 --- a/tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py +++ b/tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py @@ -29,12 +29,6 @@ class PreservationTest(TestCase): self.web = WebApplicationServer(self.validator) self.mobile = MobileApplicationServer(self.validator) - def set_state(self, state): - def set_request_state(client_id, code, client, request): - request.state = state - return True - return set_request_state - def set_client(self, request): request.client = mock.MagicMock() request.client.client_id = 'mocked' @@ -42,18 +36,13 @@ class PreservationTest(TestCase): def test_state_preservation(self): auth_uri = 'http://example.com/path?state=xyz&client_id=abc&response_type=' - token_uri = 'http://example.com/path' # authorization grant h, _, s = self.web.create_authorization_response( auth_uri + 'code', scopes=['random']) self.assertEqual(s, 302) self.assertIn('Location', h) - code = get_query_credentials(h['Location'])['code'][0] - self.validator.validate_code.side_effect = self.set_state('xyz') - _, body, _ = self.web.create_token_response(token_uri, - body='grant_type=authorization_code&code=%s' % code) - self.assertEqual(json.loads(body)['state'], 'xyz') + self.assertEqual(get_query_credentials(h['Location'])['state'][0], 'xyz') # implicit grant h, _, s = self.mobile.create_authorization_response( @@ -133,7 +122,7 @@ class PreservationTest(TestCase): # was not given in the authorization AND not in the token request. self.validator.confirm_redirect_uri.return_value = True code = get_query_credentials(h['Location'])['code'][0] - self.validator.validate_code.side_effect = self.set_state('xyz') + self.validator.validate_code.return_value = True _, body, s = self.web.create_token_response(token_uri, body='grant_type=authorization_code&code=%s' % code) self.assertEqual(s, 200) diff --git a/tests/oauth2/rfc6749/test_server.py b/tests/oauth2/rfc6749/test_server.py index b623a9b..2c6ecff 100644 --- a/tests/oauth2/rfc6749/test_server.py +++ b/tests/oauth2/rfc6749/test_server.py @@ -144,7 +144,7 @@ class TokenEndpointTest(TestCase): @mock.patch('oauthlib.common.generate_token', new=lambda: 'abc') def test_authorization_grant(self): - body = 'grant_type=authorization_code&code=abc&scope=all+of+them&state=xyz' + body = 'grant_type=authorization_code&code=abc&scope=all+of+them' headers, body, status_code = self.endpoint.create_token_response( '', body=body) token = { @@ -152,23 +152,27 @@ class TokenEndpointTest(TestCase): 'expires_in': self.expires_in, 'access_token': 'abc', 'refresh_token': 'abc', - 'scope': 'all of them', - 'state': 'xyz' + 'scope': 'all of them' } self.assertEqual(json.loads(body), token) - body = 'grant_type=authorization_code&code=abc&state=xyz' + body = 'grant_type=authorization_code&code=abc' headers, body, status_code = self.endpoint.create_token_response( '', body=body) token = { 'token_type': 'Bearer', 'expires_in': self.expires_in, 'access_token': 'abc', - 'refresh_token': 'abc', - 'state': 'xyz' + 'refresh_token': 'abc' } self.assertEqual(json.loads(body), token) + # try with additional custom variables + body = 'grant_type=authorization_code&code=abc&state=foobar' + headers, body, status_code = self.endpoint.create_token_response( + '', body=body) + self.assertEqual(json.loads(body), token) + @mock.patch('oauthlib.common.generate_token', new=lambda: 'abc') def test_password_grant(self): body = 'grant_type=password&username=a&password=hello&scope=all+of+them' @@ -277,7 +281,7 @@ twIDAQAB @mock.patch('oauthlib.common.generate_token', new=lambda: 'abc') def test_authorization_grant(self): - body = 'client_id=me&redirect_uri=http%3A%2F%2Fback.to%2Fme&grant_type=authorization_code&code=abc&scope=all+of+them&state=xyz' + body = 'client_id=me&redirect_uri=http%3A%2F%2Fback.to%2Fme&grant_type=authorization_code&code=abc&scope=all+of+them' headers, body, status_code = self.endpoint.create_token_response( '', body=body) body = json.loads(body) @@ -286,12 +290,11 @@ twIDAQAB 'expires_in': self.expires_in, 'access_token': body['access_token'], 'refresh_token': 'abc', - 'scope': 'all of them', - 'state': 'xyz' + 'scope': 'all of them' } self.assertEqual(body, token) - body = 'client_id=me&redirect_uri=http%3A%2F%2Fback.to%2Fme&grant_type=authorization_code&code=abc&state=xyz' + body = 'client_id=me&redirect_uri=http%3A%2F%2Fback.to%2Fme&grant_type=authorization_code&code=abc' headers, body, status_code = self.endpoint.create_token_response( '', body=body) body = json.loads(body) @@ -299,8 +302,20 @@ twIDAQAB 'token_type': 'Bearer', 'expires_in': self.expires_in, 'access_token': body['access_token'], - 'refresh_token': 'abc', - 'state': 'xyz' + 'refresh_token': 'abc' + } + self.assertEqual(body, token) + + # try with additional custom variables + body = 'client_id=me&redirect_uri=http%3A%2F%2Fback.to%2Fme&grant_type=authorization_code&code=abc&state=foobar' + headers, body, status_code = self.endpoint.create_token_response( + '', body=body) + body = json.loads(body) + token = { + 'token_type': 'Bearer', + 'expires_in': self.expires_in, + 'access_token': body['access_token'], + 'refresh_token': 'abc' } self.assertEqual(body, token) diff --git a/tests/openid/connect/core/test_server.py b/tests/openid/connect/core/test_server.py index ffab7b0..756c9d0 100644 --- a/tests/openid/connect/core/test_server.py +++ b/tests/openid/connect/core/test_server.py @@ -143,7 +143,7 @@ class TokenEndpointTest(TestCase): @mock.patch('oauthlib.common.generate_token', new=lambda: 'abc') def test_authorization_grant(self): - body = 'grant_type=authorization_code&code=abc&scope=all+of+them&state=xyz' + body = 'grant_type=authorization_code&code=abc&scope=all+of+them' headers, body, status_code = self.endpoint.create_token_response( '', body=body) token = { @@ -151,23 +151,27 @@ class TokenEndpointTest(TestCase): 'expires_in': self.expires_in, 'access_token': 'abc', 'refresh_token': 'abc', - 'scope': 'all of them', - 'state': 'xyz' + 'scope': 'all of them' } self.assertEqual(json.loads(body), token) - body = 'grant_type=authorization_code&code=abc&state=xyz' + body = 'grant_type=authorization_code&code=abc' headers, body, status_code = self.endpoint.create_token_response( '', body=body) token = { 'token_type': 'Bearer', 'expires_in': self.expires_in, 'access_token': 'abc', - 'refresh_token': 'abc', - 'state': 'xyz' + 'refresh_token': 'abc' } self.assertEqual(json.loads(body), token) + # ignore useless fields + body = 'grant_type=authorization_code&code=abc&state=foobar' + headers, body, status_code = self.endpoint.create_token_response( + '', body=body) + self.assertEqual(json.loads(body), token) + def test_missing_type(self): _, body, _ = self.endpoint.create_token_response('', body='') token = {'error': 'unsupported_grant_type'} |