Repository: wndhydrnt/python-oauth2

Issue:
Returned state token not url-encoded
Current implementation reads state from request query parameter with request.get_param(), which handles url-decoding automatically. When state is written back to a redirect url it is not url-encoded. This causes state mismatch in client. Only applies to clients that use state strings that yield are url-decodable.

Reproducable with /examples/authorization_code_grant.py, modifying ClientApplication.

Generate a state token in client:
```
def _request_auth_token(self):
        print("Requesting authorization token...")
        auth_endpoint = self.api_server_url + "/authorize"
        self.state = "123123%25"
        query = urllib.urlencode({"client_id": "abc",
                                  "redirect_uri": self.callback_url,
                                  "response_type": "code",
                                  "state":self.state,
                                  })

        location = "%s?%s" % (auth_endpoint, query)
```

And check state when receiving token:
```
def _read_auth_token(self, env):
    print("Receiving authorization token...")

    query_params = urlparse.parse_qs(env["QUERY_STRING"])
    self.auth_token = query_params["code"][0]
    
    if self.state and self.state != query_params["state"][0]:
        raise Exception("Received auth token with invalid state. If this was a browser, CSRF")
```   

Fix by url-encoding the state when applying it to redirect urls.
/oauth2/grant.py
```
469      query += "&state=" + quote(self.state)

708      uri_with_fragment += "&state=" + quote(self.state)

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 186083e..e1fe12f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ Improvements:
 Bugfixes:
 
   - Fix Resource Owner Grant responding with HTTP status code '500' in case an owner could not be authorized ([@wndhydrnt][])
+  - Fix "scope" parameter not being urlencoded ([@wndhydrnt][])
 
 ## 0.7.0
 
diff --git a/oauth2/grant.py b/oauth2/grant.py
index ff9d65b..5dd586f 100644
--- a/oauth2/grant.py
+++ b/oauth2/grant.py
@@ -466,7 +466,7 @@ class AuthorizationCodeAuthHandler(AuthorizeMixin, AuthRequestMixin,
         query = "code=" + code
 
         if self.state is not None:
-            query += "&state=" + self.state
+            query += "&state=" + quote(self.state)
 
         return "%s?%s" % (self.client.redirect_uri, query)
 
@@ -705,7 +705,7 @@ class ImplicitGrantHandler(AuthorizeMixin, AuthRequestMixin, GrantHandler):
             format(self.client.redirect_uri, token)
 
         if self.state is not None:
-            uri_with_fragment += "&state=" + self.state
+            uri_with_fragment += "&state=" + quote(self.state)
 
         if self.scope_handler.send_back is True:
             scopes_as_string = encode_scopes(self.scope_handler.scopes,


Test patch:
diff --git a/oauth2/test/test_grant.py b/oauth2/test/test_grant.py
index 1cb7cb2..231c3ca 100644
--- a/oauth2/test/test_grant.py
+++ b/oauth2/test/test_grant.py
@@ -1,6 +1,7 @@
 from mock import Mock, call, patch
 import json
 from oauth2.client_authenticator import ClientAuthenticator
+from oauth2.compatibility import quote
 from oauth2.test import unittest
 from oauth2.web import Request, Response, ResourceOwnerGrantSiteAdapter, \
     ImplicitGrantSiteAdapter, AuthorizationCodeGrantSiteAdapter
@@ -196,11 +197,11 @@ class AuthorizationCodeAuthHandlerTestCase(unittest.TestCase):
         code = "abcd"
         environ = {"session": "data"}
         scopes = ["scope"]
-        state = "mystate"
+        state = "my%state"
         redirect_uri = "https://callback"
         user_data = {"user_id": 789}
 
-        location_uri = "%s?code=%s&state=%s" % (redirect_uri, code, state)
+        location_uri = "%s?code=%s&state=%s" % (redirect_uri, code, quote(state))
 
         auth_code_store_mock = Mock(spec=AuthCodeStore)
 
@@ -793,6 +794,7 @@ class ImplicitGrantTestCase(unittest.TestCase):
         request_mock.get_param.assert_called_with("response_type")
         self.assertEqual(result_class, None)
 
+
 class ImplicitGrantHandlerTestCase(unittest.TestCase):
     def test_process_redirect_with_token(self):
         client_id = "abc"
@@ -848,11 +850,11 @@ class ImplicitGrantHandlerTestCase(unittest.TestCase):
         ImplicitGrantHandler should include the value of the "state" query parameter from request in redirect
         """
         redirect_uri = "http://callback"
-        state = "XHGFI"
+        state = "XH%GFI"
         token = "tokencode"
         user_data = ({}, 1)
 
-        expected_redirect_uri = "%s#access_token=%s&token_type=bearer&state=%s" % (redirect_uri, token, state)
+        expected_redirect_uri = "%s#access_token=%s&token_type=bearer&state=%s" % (redirect_uri, token, quote(state))
 
         response_mock = Mock(spec=Response)