Fix 500 error for refresh with revoked access token.

[robrap]

fixes #585

Note that there are no integration tests, so the unit tests don't
actually show the 500 error that would have been seen with a call
to oauth2_provider/oauth2_backends.py:create_token_response.

Also, there was no coverage for validate_refresh_token. I added some coverage since I was updating it, but I did not add full coverage.

[coveralls]

Pull Request Test Coverage Report for Build 943

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.03%) to 92.913%

---

Totals
Change from base Build 913:	0.03%
Covered Lines:	1193
Relevant Lines:	1284

---

💛 - Coveralls

[robrap]

UPDATE: This did in fact fix my issue, and now returns a 4xx instead of a 5xx error.

[jleclanche]

This doesn't look right... at first glance it should probably be happening in the same transaction as what's happening before.

[robrap]

@jleclanche: Thanks for looking at this. I'm not sure what "...the same transaction as what's happening before" refers to?

Here are a lot more details about the problem.

Our original code used django-auth-toolkit 0.12.0. In that version, if you do the following:

1. get a refresh token with an access token.
2. revoke the access token.
3. call create_token_response with this refresh token.
   We would get a 401.

When I upgraded to django-auth-toolkit 1.1.2, the behavior had changed, due to an issue I will describe in more detail.

For reference for my description below, here are the relevant lines in oauthlib in validate_token_request:
https://github.com/oauthlib/oauthlib/blob/v2.1.0/oauthlib/oauth2/rfc6749/grant_types/refresh_token.py#L109-L116

The problem:
In oauthlib: validate_token_request, the refresh token is currently being returned as valid by request_validator.validate_refresh_token, but when it moves on to request_validator.get_original_scopes, it can't find the access token because it has been revoked and returns an error, that in turn becomes a 500 error.

What is probably warranted is an integration test that shows the whole flow between create_token_response, through oauthlib, calling back to the relevant oauth2_provider code.

Here are additional details of the error I am seeing...

Error Message:
DoesNotExist: AccessToken matching query does not exist.

Relevant stacktrace:

...
../../edx-venv/local/lib/python2.7/site-packages/oauth2_provider/views/base.py:206: in post
    url, headers, body, status = self.create_token_response(request)
../../edx-venv/local/lib/python2.7/site-packages/oauth2_provider/views/mixins.py:125: in create_token_response
    return core.create_token_response(request)
../../edx-venv/local/lib/python2.7/site-packages/oauth2_provider/oauth2_backends.py:140: in create_token_response
    headers, extra_credentials)
../../edx-venv/local/lib/python2.7/site-packages/oauthlib/oauth2/rfc6749/endpoints/base.py:64: in wrapper
    return f(endpoint, uri, *args, **kwargs)
../../edx-venv/local/lib/python2.7/site-packages/oauthlib/oauth2/rfc6749/endpoints/token.py:117: in create_token_response
    request, self.default_token_type)
../../edx-venv/local/lib/python2.7/site-packages/oauthlib/oauth2/rfc6749/grant_types/refresh_token.py:59: in create_token_response
    self.validate_token_request(request)
../../edx-venv/local/lib/python2.7/site-packages/oauthlib/oauth2/rfc6749/grant_types/refresh_token.py:117: in validate_token_request
    request.refresh_token, request))
../../edx-venv/local/lib/python2.7/site-packages/oauth2_provider/oauth2_validators.py:608: in get_original_scopes
    return AccessToken.objects.get(source_refresh_token_id=rt.id).scope

[robrap]

@jleclanche: I also did not find any indication that the refresh token should be invalid if the access token is revoked in the RFC. So maybe the refresh token should continue to work as long as it is not revoked? If so, maybe this was a bug in 0.12.0, and we want to correct the behavior?

In that case, would the solution be to have the refresh token include the original scopes in its model so it doesn't need to refer back to the (possibly revoked) access token? Other?

[robrap]

At this time, I think that both the old version (0.12.0) and the new version (1.1.2) have different versions of the same bug. The old version returned a 401. The new version returns a 500. In both cases, this should probably be a 200. I'm going to close this PR, which was just replicating an earlier bug, rather than fixing the issue.