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.

13 files changed, 72 insertions, 49 deletions

diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 2cc0dd3..9e0efda 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -1,6 +1,14 @@
 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
+
 3.0.0 (2019-01-01)
 ------------------
 OAuth2.0 Provider - outstanding Features
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'}