openstack/deb-python-oauth2client
Code Issues Proposed changes

Try to revoke token with POST when getting a 405 (#662)

Browse Source 
The OAuth spec does not specify the HTTP verb explicitly but it does hint that POST is the correct verb.
When using the client library with other OAuth services that implement revocation token via a POST, revoking the token will fail.

This commit adds the ability to re-try the revocation process if we get a 405 with the POST verb.
This commit is contained in:
Liran Nuna
2016-10-06 12:53:16 -07:00
committed by Jon Wayne Parrott
parent 3f9fdbd5a3
commit 999de3ac8b
2 changed files with 43 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
oauth2client/client.py
View File

@@ -836,6 +836,10 @@ class OAuth2Credentials(Credentials):
token_revoke_uri = _helpers.update_query_params( token_revoke_uri = _helpers.update_query_params(
self.revoke_uri, query_params) self.revoke_uri, query_params)
resp, content = transport.request(http, token_revoke_uri) resp, content = transport.request(http, token_revoke_uri)
if resp.status == http_client.METHOD_NOT_ALLOWED:
body = urllib.parse.urlencode(query_params)
resp, content = transport.request(http, token_revoke_uri,
method='POST', body=body)
if resp.status == http_client.OK: if resp.status == http_client.OK:
self.invalid = True self.invalid = True
else: else:

58
tests/test_client.py
View File

@@ -819,8 +819,8 @@ class DummyDeleteStorage(client.Storage):
self.delete_called = True self.delete_called = True
def _token_revoke_test_helper(testcase, status, revoke_raise, def _token_revoke_test_helper(testcase, revoke_raise, valid_bool_value,
valid_bool_value, token_attr): token_attr, http_mock):
current_store = getattr(testcase.credentials, 'store', None) current_store = getattr(testcase.credentials, 'store', None)
dummy_store = DummyDeleteStorage() dummy_store = DummyDeleteStorage()
@@ -834,12 +834,11 @@ def _token_revoke_test_helper(testcase, status, revoke_raise,
return actual_do_revoke(http, token) return actual_do_revoke(http, token)
testcase.credentials._do_revoke = do_revoke_stub testcase.credentials._do_revoke = do_revoke_stub
http = http_mock.HttpMock(headers={'status': status})
if revoke_raise: if revoke_raise:
testcase.assertRaises(client.TokenRevokeError, testcase.assertRaises(client.TokenRevokeError,
testcase.credentials.revoke, http) testcase.credentials.revoke, http_mock)
else: else:
testcase.credentials.revoke(http) testcase.credentials.revoke(http_mock)
testcase.assertEqual(getattr(testcase.credentials, token_attr), testcase.assertEqual(getattr(testcase.credentials, token_attr),
testcase.token_from_revoke) testcase.token_from_revoke)
@@ -922,21 +921,38 @@ class BasicCredentialsTests(unittest.TestCase):
self.assertEqual(None, self.credentials.token_response) self.assertEqual(None, self.credentials.token_response)
def test_token_revoke_success(self): def test_token_revoke_success(self):
http = http_mock.HttpMock(headers={'status': http_client.OK})
_token_revoke_test_helper( _token_revoke_test_helper(
self, '200', revoke_raise=False, self, revoke_raise=False, valid_bool_value=True,
valid_bool_value=True, token_attr='refresh_token') token_attr='refresh_token', http_mock=http)
def test_token_revoke_failure(self): def test_token_revoke_failure(self):
http = http_mock.HttpMock(headers={'status': http_client.BAD_REQUEST})
_token_revoke_test_helper( _token_revoke_test_helper(
self, '400', revoke_raise=True, self, revoke_raise=True, valid_bool_value=False,
valid_bool_value=False, token_attr='refresh_token') token_attr='refresh_token', http_mock=http)
def test_token_revoke_fallback(self): def test_token_revoke_fallback(self):
original_credentials = self.credentials.to_json() original_credentials = self.credentials.to_json()
self.credentials.refresh_token = None self.credentials.refresh_token = None
http = http_mock.HttpMock(headers={'status': http_client.OK})
_token_revoke_test_helper( _token_revoke_test_helper(
self, '200', revoke_raise=False, self, revoke_raise=False, valid_bool_value=True,
valid_bool_value=True, token_attr='access_token') token_attr='access_token', http_mock=http)
self.credentials = self.credentials.from_json(original_credentials)
def test_token_revoke_405(self):
original_credentials = self.credentials.to_json()
self.credentials.refresh_token = None
http = http_mock.HttpMockSequence([
({'status': http_client.METHOD_NOT_ALLOWED}, b''),
({'status': http_client.OK}, b''),
])
_token_revoke_test_helper(
self, revoke_raise=False, valid_bool_value=True,
token_attr='access_token', http_mock=http)
self.credentials = self.credentials.from_json(original_credentials) self.credentials = self.credentials.from_json(original_credentials)
def test_non_401_error_response(self): def test_non_401_error_response(self):
@@ -1483,14 +1499,16 @@ class AccessTokenCredentialsTests(unittest.TestCase):
resp, content = transport.request(http, 'http://example.com') resp, content = transport.request(http, 'http://example.com')
def test_token_revoke_success(self): def test_token_revoke_success(self):
http = http_mock.HttpMock(headers={'status': http_client.OK})
_token_revoke_test_helper( _token_revoke_test_helper(
self, '200', revoke_raise=False, self, revoke_raise=False, valid_bool_value=True,
valid_bool_value=True, token_attr='access_token') token_attr='access_token', http_mock=http)
def test_token_revoke_failure(self): def test_token_revoke_failure(self):
http = http_mock.HttpMock(headers={'status': http_client.BAD_REQUEST})
_token_revoke_test_helper( _token_revoke_test_helper(
self, '400', revoke_raise=True, self, revoke_raise=True, valid_bool_value=False,
valid_bool_value=False, token_attr='access_token') token_attr='access_token', http_mock=http)
def test_non_401_error_response(self): def test_non_401_error_response(self):
http = http_mock.HttpMock(headers={'status': http_client.BAD_REQUEST}) http = http_mock.HttpMock(headers={'status': http_client.BAD_REQUEST})
@@ -1543,14 +1561,16 @@ class TestAssertionCredentials(unittest.TestCase):
self.assertEqual(b'Bearer 1/3w', content[b'Authorization']) self.assertEqual(b'Bearer 1/3w', content[b'Authorization'])
def test_token_revoke_success(self): def test_token_revoke_success(self):
http = http_mock.HttpMock(headers={'status': http_client.OK})
_token_revoke_test_helper( _token_revoke_test_helper(
self, '200', revoke_raise=False, self, revoke_raise=False, valid_bool_value=True,
valid_bool_value=True, token_attr='access_token') token_attr='access_token', http_mock=http)
def test_token_revoke_failure(self): def test_token_revoke_failure(self):
http = http_mock.HttpMock(headers={'status': http_client.BAD_REQUEST})
_token_revoke_test_helper( _token_revoke_test_helper(
self, '400', revoke_raise=True, self, revoke_raise=True, valid_bool_value=False,
valid_bool_value=False, token_attr='access_token') token_attr='access_token', http_mock=http)
def test_sign_blob_abstract(self): def test_sign_blob_abstract(self):
credentials = client.AssertionCredentials(None) credentials = client.AssertionCredentials(None)