From e2562942279ad832f80b27e9629cf4d46a1c9bc5 Mon Sep 17 00:00:00 2001 From: Jonathan Huot Date: Thu, 31 Jan 2019 13:44:21 +0100 Subject: Add 3.0.1 changelog --- CHANGELOG.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2cc0dd3..a5cb324 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,10 @@ Changelog ========= +3.0.1 (2019-01-24) +------------------ +* Fixed OAuth2.0 regression introduced in 3.0.0: Revocation with Basic auth no longer possible #644 + 3.0.0 (2019-01-01) ------------------ OAuth2.0 Provider - outstanding Features -- cgit v1.2.1 From 8c9f0a3cee9fab35fdf7269441daab666b931f59 Mon Sep 17 00:00:00 2001 From: Jonathan Huot Date: Wed, 20 Feb 2019 14:30:03 +0100 Subject: Fix 652: removed "state" from /token response. Fix OIDC /token flow where &state=None was always returned, and fix OAuth2.0 /token flow where &state=foobar was returned if &state=foobar was present in the token request. Remove "save_token" from create_token() signature cuz it was not used internally. Deprecated the option to let upstream libraries have a chance to remove it, if ever used. --- CHANGELOG.rst | 4 +++ .../rfc6749/grant_types/authorization_code.py | 4 ++- .../rfc6749/grant_types/client_credentials.py | 3 +- oauthlib/oauth2/rfc6749/grant_types/implicit.py | 5 ++- .../oauth2/rfc6749/grant_types/refresh_token.py | 3 +- .../resource_owner_password_credentials.py | 3 +- oauthlib/oauth2/rfc6749/tokens.py | 18 +++++----- oauthlib/openid/connect/core/grant_types/base.py | 3 -- .../openid/connect/core/grant_types/implicit.py | 5 +++ oauthlib/openid/connect/core/tokens.py | 2 +- .../endpoints/test_credentials_preservation.py | 12 ------- tests/oauth2/rfc6749/test_server.py | 39 +++++++++++++++------- tests/openid/connect/core/test_server.py | 16 +++++---- 13 files changed, 68 insertions(+), 49 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..44a9a97 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 used internally." + "If you do, use `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..c77d18e 100644 --- a/tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py +++ b/tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py @@ -42,18 +42,6 @@ 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') # implicit grant h, _, s = self.mobile.create_authorization_response( 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'} -- cgit v1.2.1 From 58f1c3fe4020d13d4c2f7b80902b2c157fde807d Mon Sep 17 00:00:00 2001 From: Jonathan Huot Date: Thu, 21 Feb 2019 10:01:29 +0100 Subject: Add clarity to the deprecation warning --- oauthlib/oauth2/rfc6749/tokens.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/oauthlib/oauth2/rfc6749/tokens.py b/oauthlib/oauth2/rfc6749/tokens.py index 44a9a97..7973923 100644 --- a/oauthlib/oauth2/rfc6749/tokens.py +++ b/oauthlib/oauth2/rfc6749/tokens.py @@ -306,8 +306,8 @@ class BearerToken(TokenBase): :param refresh_token: """ if "save_token" in kwargs: - warnings.warn("`save_token` has been deprecated, it was not used internally." - "If you do, use `request_validator.save_token()` instead.", + 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): -- cgit v1.2.1 From 4205dc1b4240e30d966c3fd4fe872f83413b2e2c Mon Sep 17 00:00:00 2001 From: Jonathan Huot Date: Thu, 21 Feb 2019 10:16:55 +0100 Subject: Add authorization "state" preservation back for AuthCode --- tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py b/tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py index c77d18e..c0cf86d 100644 --- a/tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py +++ b/tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py @@ -43,6 +43,13 @@ class PreservationTest(TestCase): def test_state_preservation(self): auth_uri = 'http://example.com/path?state=xyz&client_id=abc&response_type=' + # authorization grant + h, _, s = self.web.create_authorization_response( + auth_uri + 'code', scopes=['random']) + self.assertEqual(s, 302) + self.assertIn('Location', h) + self.assertEqual(get_query_credentials(h['Location'])['state'][0], 'xyz') + # implicit grant h, _, s = self.mobile.create_authorization_response( auth_uri + 'token', scopes=['random']) -- cgit v1.2.1 From 2904de612a5e52c14776978dd5a31cdde2bfc34e Mon Sep 17 00:00:00 2001 From: Jonathan Huot Date: Thu, 21 Feb 2019 10:17:23 +0100 Subject: Removed useless set_state internal function Does not have purpose for /token request --- tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py b/tests/oauth2/rfc6749/endpoints/test_credentials_preservation.py index c0cf86d..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' @@ -128,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) -- cgit v1.2.1