Fix urldecoding of client_id/secret

[lucacorti]

I read a bit of the RFC and it seems client_id/secret should be urlencoded (http://tools.ietf.org/html/rfc6749#section-2.3.1), so i tried this on my test system too:

$ alias urlencode='python -c "import sys, urllib as ul; print ul.quote_plus(sys.argv[1])"'

$ alias urldecode='python -c "import sys, urllib as ul; print ul.unquote_plus(sys.argv[1])"'

$ urlencode 'y \aRGH={`ga[Sade9nC(=jZeZz/B[{1aN\k%dX{'́                                                                                                          

y+%5CaRGH%3D%7B%60ga%5BSade9nC%28%3DjZeZz%2FB%5B%7B1aN%5Ck%25dX%7B

$ urlencode 'Ca-#K:":R1:+(;4M6vTD<c._*g@>dlS&/:s;(<\tP;p1+oUd}\ZKhPLJ_EH 5V#fLmy&]P&:-]#-; vN4 # MNF\lI-`-Um: J0+jZD/fV6;tvrBhOl_[(gu`r&Z"?wZ'

Ca-%23K%3A%22%3AR1%3A%2B%28%3B4M6vTD%3Cc._%2Ag%40%3EdlS%26%2F%3As%3B%28%3C%5CtP%3Bp1%2BoUd%7D%5CZKhPLJ_EH+5V%23fLmy%26%5DP%26%3A-%5D%23-%3B+vN4+%23+MNF%5ClI-%60-Um%3A+J0%2BjZD%2FfV6%3BtvrBhOl_%5B%28gu%60r%26Z%22%3FwZ

$ curl -v -K user -X POST -d "grant_type=password&username=myuser&password=mypassword&scope=read" http://localhost:8000/o/token/

* About to connect() to localhost port 8000 (#0)
*   Trying ::1...
* Connection refused
*   Trying 127.0.0.1...
* connected
* Connected to localhost (127.0.0.1) port 8000 (#0)
* Server auth using Basic with user 'y+%5CaRGH%3D%7B%60ga%5BSade9nC%28%3DjZeZz%2FB%5B%7B1aN%5Ck%25dX%7B'
> POST /o/token/ HTTP/1.1
> Authorization: Basic eSslNUNhUkdIJTNEJTdCJTYwZ2ElNUJTYWRlOW5DJTI4JTNEalplWnolMkZCJTVCJTdCMWFOJTVDayUyNWRYJTdCOkNhLSUyM0slM0ElMjIlM0FSMSUzQSUyQiUyOCUzQjRNNnZURCUzQ2MuXyUyQWclNDAlM0VkbFMlMjYlMkYlM0FzJTNCJTI4JTNDJTVDdFAlM0JwMSUyQm9VZCU3RCU1Q1pLaFBMSl9FSCs1ViUyM2ZMbXklMjYlNURQJTI2JTNBLSU1RCUyMy0lM0Irdk40KyUyMytNTkYlNUNsSS0lNjAtVW0lM0ErSjAlMkJqWkQlMkZmVjYlM0J0dnJCaE9sXyU1QiUyOGd1JTYwciUyNlolMjIlM0Z3Wg==
> User-Agent: curl/7.24.0 (x86_64-apple-darwin12.0) libcurl/7.24.0 OpenSSL/0.9.8x zlib/1.2.5
> Host: localhost:8000
> Accept: */*
> Content-Length: 63
> Content-Type: application/x-www-form-urlencoded
> 
* upload completely sent off: 63 out of 63 bytes
* HTTP 1.0, assume close after body
< HTTP/1.0 400 BAD REQUEST
< Date: Mon, 16 Sep 2013 19:28:26 GMT
< Server: WSGIServer/0.1 Python/2.7.5
< X-Frame-Options: SAMEORIGIN
< Content-Type: application/json;charset=UTF-8
< Pragma: no-cache
< Cache-Control: no-store
< 
* Closing connection #0

And the django console still says:

Dispatching grant_type password request to <oauthlib.oauth2.rfc6749.grant_types.resource_owner_password_credentials.ResourceOwnerPasswordCredentialsGrant object at 0x109803510>.
Authenticating client, <oauthlib.common.Request object at 0x109803c10>.
Client authentication failed, <oauthlib.common.Request object at 0x109803c10>.
Client error in token request, .
[16/Sep/2013 21:30:16] "POST /o/token/ HTTP/1.1" 400 27

But:

$ echo eSslNUNhUkdIJTNEJTdCJTYwZ2ElNUJTYWRlOW5DJTI4JTNEalplWnolMkZCJTVCJTdCMWFOJTVDayUyNWRYJTdCOkNhLSUyM0slM0ElMjIlM0FSMSUzQSUyQiUyOCUzQjRNNnZURCUzQ2MuXyUyQWclNDAlM0VkbFMlMjYlMkYlM0FzJTNCJTI4JTNDJTVDdFAlM0JwMSUyQm9VZCU3RCU1Q1pLaFBMSl9FSCs1ViUyM2ZMbXklMjYlNURQJTI2JTNBLSU1RCUyMy0lM0Irdk40KyUyMytNTkYlNUNsSS0lNjAtVW0lM0ErSjAlMkJqWkQlMkZmVjYlM0J0dnJCaE9sXyU1QiUyOGd1JTYwciUyNlolMjIlM0Z3Wg== | base64 -D

y+%5CaRGH%3D%7B%60ga%5BSade9nC%28%3DjZeZz%2FB%5B%7B1aN%5Ck%25dX%7B:Ca-%23K%3A%22%3AR1%3A%2B%28%3B4M6vTD%3Cc._%2Ag%40%3EdlS%26%2F%3As%3B%28%3C%5CtP%3Bp1%2BoUd%7D%5CZKhPLJ_EH+5V%23fLmy%26%5DP%26%3A-%5D%23-%3B+vN4+%23+MNF%5ClI-%60-Um%3A+J0%2BjZD%2FfV6%3BtvrBhOl_%5B%28gu%60r%26Z%22%3FwZ

$ urldecode 'y+%5CaRGH%3D%7B%60ga%5BSade9nC%28%3DjZeZz%2FB%5B%7B1aN%5Ck%25dX%7B'

y \aRGH={`ga[Sade9nC(=jZeZz/B[{1aN\k%dX{

$ urldecode 'Ca-%23K%3A%22%3AR1%3A%2B%28%3B4M6vTD%3Cc._%2Ag%40%3EdlS%26%2F%3As%3B%28%3C%5CtP%3Bp1%2BoUd%7D%5CZKhPLJ_EH+5V%23fLmy%26%5DP%26%3A-%5D%23-%3B+vN4+%23+MNF%5ClI-%60-Um%3A+J0%2BjZD%2FfV6%3BtvrBhOl_%5B%28gu%60r%26Z%22%3FwZ'

Ca-#K:":R1:+(;4M6vTD<c._*g@>dlS&/:s;(<\tP;p1+oUd}\ZKhPLJ_EH 5V#fLmy&]P&:-]#-; vN4 # MNF\lI-`-Um: J0+jZD/fV6;tvrBhOl_[(gu`r&Z"?wZ

Now, this is the authentication method from oauth2_validators.py:

class OAuth2Validator(RequestValidator):
    def authenticate_client(self, request, *args, **kwargs):
        """
        Check if client exists and it's authenticating itself as in rfc:`3.2.1`
        """
        auth = request.headers.get('HTTP_AUTHORIZATION', None)

        if not auth:
            return False

        auth_type, auth_string = auth.split(' ')
        encoding = request.encoding or 'utf-8'

        auth_string_decoded = base64.b64decode(auth_string).decode(encoding)
        client_id, client_secret = auth_string_decoded.split(':', 1)

        try:
            request.client = Application.objects.get(client_id=client_id, client_secret=client_secret)
            return True

        except Application.DoesNotExist:
            return False

As you can see, the comment lies. Section 3.2.1 says authentication is done as specified in section 2.3.1, The code decodes the base 64 but then tries to fetch the application model without urldecoding the client id and secret. So I went ahead and added urlib.unquote_plus() to the decoding, and ta-da:

$ curl -v -K user -X POST -d "grant_type=password&username=myuser&password=mypass&scope=read" http://localhost:8000/o/token/
* About to connect() to localhost port 8000 (#0)
*   Trying ::1...
* Connection refused
*   Trying 127.0.0.1...
* connected
* Connected to localhost (127.0.0.1) port 8000 (#0)
* Server auth using Basic with user 'y+%5CaRGH%3D%7B%60ga%5BSade9nC%28%3DjZeZz%2FB%5B%7B1aN%5Ck%25dX%7B'
> POST /o/token/ HTTP/1.1
> Authorization: Basic eSslNUNhUkdIJTNEJTdCJTYwZ2ElNUJTYWRlOW5DJTI4JTNEalplWnolMkZCJTVCJTdCMWFOJTVDayUyNWRYJTdCOkNhLSUyM0slM0ElMjIlM0FSMSUzQSUyQiUyOCUzQjRNNnZURCUzQ2MuXyUyQWclNDAlM0VkbFMlMjYlMkYlM0FzJTNCJTI4JTNDJTVDdFAlM0JwMSUyQm9VZCU3RCU1Q1pLaFBMSl9FSCs1ViUyM2ZMbXklMjYlNURQJTI2JTNBLSU1RCUyMy0lM0Irdk40KyUyMytNTkYlNUNsSS0lNjAtVW0lM0ErSjAlMkJqWkQlMkZmVjYlM0J0dnJCaE9sXyU1QiUyOGd1JTYwciUyNlolMjIlM0Z3Wg==
> User-Agent: curl/7.24.0 (x86_64-apple-darwin12.0) libcurl/7.24.0 OpenSSL/0.9.8x zlib/1.2.5
> Host: localhost:8000
> Accept: */*
> Content-Length: 63
> Content-Type: application/x-www-form-urlencoded
> 
* upload completely sent off: 63 out of 63 bytes
* HTTP 1.0, assume close after body
< HTTP/1.0 200 OK
< Date: Mon, 16 Sep 2013 20:36:33 GMT
< Server: WSGIServer/0.1 Python/2.7.5
< X-Frame-Options: SAMEORIGIN
< Content-Type: application/json;charset=UTF-8
< Pragma: no-cache
< Cache-Control: no-store
< 
* Closing connection #0
{"access_token": "jivquDuUw8xQyrNiTJ0NoXge5bObKJ", "token_type": "Bearer", "expires_in": 36000, "refresh_token": "5GeHBmmayfd9R30sKgYhycYTMh78Pm", "scope": "read"}

My debugger confirms this, I think this is a bug.

[masci]

Hi @lucacorti thanks for your work, much appreciated!

Just a couple of things beside the bug: as per our guidelines (see https://django-oauth-toolkit.readthedocs.org/en/latest/contributing.html) we will merge only PR passing tests, and yours does not. Could you please fix test code and possibly add a test case that highlights the bug you are fixing? (e.g., make a test that fails if credential strings are not quoted correctly).

TIA

[synasius]

I suggest you to run tests with tox. That way you can check if your code is python 3 compatible

[lucacorti]

@synasius Man, I already fixed the python2/python3 urllib issues locallly ;)

My code currently passes the testsuite and I'm struggling to make a testcase for my code as per @masci instructions. I'll update my pull request when these are ready.

I had issues running the testsuite in my environment btw, probably due to newer tox and assumptions about sqlite availability. I'll open up another pull request for this.

[synasius]

I had issues running the testsuite in my environment btw, probably due to newer tox and assumptions about sqlite > availability. I'll open up another pull request for this.

look at the current master because there was a problem in the .travis.yml

[coveralls]

Coverage remained the same when pulling 0e71410 on lucacorti:master into 314d544 on evonove:master.

[lucacorti]

I'm closing this, please see the new pull request from the fix-urlencode-clientid-secret branch.