Merge "Handle the exception from creating access token properly"
This commit is contained in:
commit
77213e6c0d
|
@ -14,6 +14,7 @@
|
||||||
|
|
||||||
"""Extensions supporting OAuth1."""
|
"""Extensions supporting OAuth1."""
|
||||||
|
|
||||||
|
from oslo_log import log
|
||||||
from oslo_serialization import jsonutils
|
from oslo_serialization import jsonutils
|
||||||
from oslo_utils import timeutils
|
from oslo_utils import timeutils
|
||||||
from six.moves import http_client
|
from six.moves import http_client
|
||||||
|
@ -33,6 +34,7 @@ from keystone.oauth1 import validator
|
||||||
|
|
||||||
|
|
||||||
CONF = keystone.conf.CONF
|
CONF = keystone.conf.CONF
|
||||||
|
LOG = log.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
def _emit_user_oauth_consumer_token_invalidate(payload):
|
def _emit_user_oauth_consumer_token_invalidate(payload):
|
||||||
|
@ -301,30 +303,54 @@ class OAuthControllerV3(controller.V3Controller):
|
||||||
access_verifier = oauth1.AccessTokenEndpoint(
|
access_verifier = oauth1.AccessTokenEndpoint(
|
||||||
request_validator=validator.OAuthValidator(),
|
request_validator=validator.OAuthValidator(),
|
||||||
token_generator=oauth1.token_generator)
|
token_generator=oauth1.token_generator)
|
||||||
|
try:
|
||||||
h, b, s = access_verifier.create_access_token_response(
|
h, b, s = access_verifier.create_access_token_response(
|
||||||
url,
|
url,
|
||||||
http_method='POST',
|
http_method='POST',
|
||||||
body=request.params,
|
body=request.params,
|
||||||
headers=request.headers)
|
headers=request.headers)
|
||||||
params = oauth1.extract_non_oauth_params(b)
|
except NotImplementedError:
|
||||||
if params:
|
# Client key or request token validation failed, since keystone
|
||||||
msg = _('There should not be any non-oauth parameters')
|
# does not yet support dummy client or dummy request token,
|
||||||
|
# so we will raise Unauthorized exception instead.
|
||||||
|
try:
|
||||||
|
self.oauth_api.get_consumer(consumer_id)
|
||||||
|
except exception.NotFound:
|
||||||
|
msg = _('Provided consumer does not exist.')
|
||||||
|
LOG.warning(msg)
|
||||||
raise exception.Unauthorized(message=msg)
|
raise exception.Unauthorized(message=msg)
|
||||||
|
|
||||||
if req_token['consumer_id'] != consumer_id:
|
if req_token['consumer_id'] != consumer_id:
|
||||||
msg = _('provided consumer key does not match stored consumer key')
|
msg = _('Provided consumer key does not match stored '
|
||||||
|
'consumer key.')
|
||||||
|
LOG.warning(msg)
|
||||||
raise exception.Unauthorized(message=msg)
|
raise exception.Unauthorized(message=msg)
|
||||||
|
# The response body is empty since either one of the following reasons
|
||||||
|
if not b:
|
||||||
if req_token['verifier'] != oauth_verifier:
|
if req_token['verifier'] != oauth_verifier:
|
||||||
msg = _('provided verifier does not match stored verifier')
|
msg = _('Provided verifier does not match stored verifier')
|
||||||
|
else:
|
||||||
|
msg = _('Invalid signature.')
|
||||||
|
LOG.warning(msg)
|
||||||
raise exception.Unauthorized(message=msg)
|
raise exception.Unauthorized(message=msg)
|
||||||
|
params = oauth1.extract_non_oauth_params(b)
|
||||||
if req_token['id'] != request_token_id:
|
# Invalid request would end up with the body like below:
|
||||||
msg = _('provided request key does not match stored request key')
|
# 'error=invalid_request&description=missing+resource+owner+key'
|
||||||
|
# Log this detail message so that we will know where is the
|
||||||
|
# validation failed.
|
||||||
|
if params:
|
||||||
|
if 'error' in params:
|
||||||
|
msg = _(
|
||||||
|
'Validation failed with errors: %(error)s, detail '
|
||||||
|
'message is: %(desc)s.') % {
|
||||||
|
'error': params['error'],
|
||||||
|
'desc': params['error_description']}
|
||||||
|
else:
|
||||||
|
msg = _('There should not be any non-oauth parameters.')
|
||||||
|
LOG.warning(msg)
|
||||||
raise exception.Unauthorized(message=msg)
|
raise exception.Unauthorized(message=msg)
|
||||||
|
|
||||||
if not req_token.get('authorizing_user_id'):
|
if not req_token.get('authorizing_user_id'):
|
||||||
msg = _('Request Token does not have an authorizing user id')
|
msg = _('Request Token does not have an authorizing user id.')
|
||||||
|
LOG.warning(msg)
|
||||||
raise exception.Unauthorized(message=msg)
|
raise exception.Unauthorized(message=msg)
|
||||||
|
|
||||||
access_token_duration = CONF.oauth1.access_token_duration
|
access_token_duration = CONF.oauth1.access_token_duration
|
||||||
|
|
|
@ -101,7 +101,11 @@ class OAuthValidator(oauth1.RequestValidator):
|
||||||
|
|
||||||
def validate_request_token(self, client_key, token, request):
|
def validate_request_token(self, client_key, token, request):
|
||||||
try:
|
try:
|
||||||
return self.oauth_api.get_request_token(token) is not None
|
req_token = self.oauth_api.get_request_token(token)
|
||||||
|
if req_token:
|
||||||
|
return req_token['consumer_id'] == client_key
|
||||||
|
else:
|
||||||
|
return False
|
||||||
except exception.NotFound:
|
except exception.NotFound:
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
|
@ -13,6 +13,7 @@
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
import copy
|
import copy
|
||||||
|
import random
|
||||||
import uuid
|
import uuid
|
||||||
|
|
||||||
import mock
|
import mock
|
||||||
|
@ -680,6 +681,7 @@ class MaliciousOAuth1Tests(OAuth1Tests):
|
||||||
self.post(url, headers=headers, expected_status=http_client.NOT_FOUND)
|
self.post(url, headers=headers, expected_status=http_client.NOT_FOUND)
|
||||||
|
|
||||||
def test_bad_verifier(self):
|
def test_bad_verifier(self):
|
||||||
|
self.config_fixture.config(debug=True, insecure_debug=True)
|
||||||
consumer = self._create_single_consumer()
|
consumer = self._create_single_consumer()
|
||||||
consumer_id = consumer['id']
|
consumer_id = consumer['id']
|
||||||
consumer_secret = consumer['secret']
|
consumer_secret = consumer['secret']
|
||||||
|
@ -702,8 +704,77 @@ class MaliciousOAuth1Tests(OAuth1Tests):
|
||||||
|
|
||||||
request_token.set_verifier(uuid.uuid4().hex)
|
request_token.set_verifier(uuid.uuid4().hex)
|
||||||
url, headers = self._create_access_token(consumer, request_token)
|
url, headers = self._create_access_token(consumer, request_token)
|
||||||
self.post(url, headers=headers,
|
resp = self.post(url, headers=headers,
|
||||||
expected_status=http_client.UNAUTHORIZED)
|
expected_status=http_client.UNAUTHORIZED)
|
||||||
|
resp_data = jsonutils.loads(resp.body)
|
||||||
|
self.assertIn('Validation failed with errors',
|
||||||
|
resp_data.get('error', {}).get('message'))
|
||||||
|
|
||||||
|
def test_validate_access_token_request_failed(self):
|
||||||
|
self.config_fixture.config(debug=True, insecure_debug=True)
|
||||||
|
consumer = self._create_single_consumer()
|
||||||
|
consumer_id = consumer['id']
|
||||||
|
consumer_secret = consumer['secret']
|
||||||
|
consumer = {'key': consumer_id, 'secret': consumer_secret}
|
||||||
|
|
||||||
|
url, headers = self._create_request_token(consumer, self.project_id)
|
||||||
|
content = self.post(
|
||||||
|
url, headers=headers,
|
||||||
|
response_content_type='application/x-www-form-urlencoded')
|
||||||
|
credentials = _urllib_parse_qs_text_keys(content.result)
|
||||||
|
request_key = credentials['oauth_token'][0]
|
||||||
|
request_secret = credentials['oauth_token_secret'][0]
|
||||||
|
request_token = oauth1.Token(request_key, request_secret)
|
||||||
|
|
||||||
|
url = self._authorize_request_token(request_key)
|
||||||
|
body = {'roles': [{'id': self.role_id}]}
|
||||||
|
resp = self.put(url, body=body, expected_status=http_client.OK)
|
||||||
|
verifier = resp.result['token']['oauth_verifier']
|
||||||
|
request_token.set_verifier(verifier)
|
||||||
|
|
||||||
|
# 1. Invalid signature.
|
||||||
|
# Update the secret, so it will fail to validate the signature.
|
||||||
|
consumer.update({'secret': uuid.uuid4().hex})
|
||||||
|
url, headers = self._create_access_token(consumer, request_token)
|
||||||
|
resp = self.post(url, headers=headers,
|
||||||
|
expected_status=http_client.UNAUTHORIZED)
|
||||||
|
resp_data = jsonutils.loads(resp.body)
|
||||||
|
self.assertIn('Invalid signature',
|
||||||
|
resp_data.get('error', {}).get('message'))
|
||||||
|
|
||||||
|
# 2. Invalid verifier.
|
||||||
|
# Even though the verifier is well formatted, it is not verifier
|
||||||
|
# that is stored in the backend, this is different with the testcase
|
||||||
|
# above `test_bad_verifier` where it test that `verifier` is not
|
||||||
|
# well formatted.
|
||||||
|
verifier = ''.join(random.SystemRandom().sample(base.VERIFIER_CHARS,
|
||||||
|
8))
|
||||||
|
request_token.set_verifier(verifier)
|
||||||
|
url, headers = self._create_access_token(consumer, request_token)
|
||||||
|
resp = self.post(url, headers=headers,
|
||||||
|
expected_status=http_client.UNAUTHORIZED)
|
||||||
|
resp_data = jsonutils.loads(resp.body)
|
||||||
|
self.assertIn('Provided verifier',
|
||||||
|
resp_data.get('error', {}).get('message'))
|
||||||
|
|
||||||
|
# 3. The provided consumer does not exist.
|
||||||
|
consumer.update({'key': uuid.uuid4().hex})
|
||||||
|
url, headers = self._create_access_token(consumer, request_token)
|
||||||
|
resp = self.post(url, headers=headers,
|
||||||
|
expected_status=http_client.UNAUTHORIZED)
|
||||||
|
resp_data = jsonutils.loads(resp.body)
|
||||||
|
self.assertIn('Provided consumer does not exist',
|
||||||
|
resp_data.get('error', {}).get('message'))
|
||||||
|
|
||||||
|
# 4. The consumer key provided does not match stored consumer key.
|
||||||
|
consumer2 = self._create_single_consumer()
|
||||||
|
consumer.update({'key': consumer2['id']})
|
||||||
|
url, headers = self._create_access_token(consumer, request_token)
|
||||||
|
resp = self.post(url, headers=headers,
|
||||||
|
expected_status=http_client.UNAUTHORIZED)
|
||||||
|
resp_data = jsonutils.loads(resp.body)
|
||||||
|
self.assertIn('Provided consumer key',
|
||||||
|
resp_data.get('error', {}).get('message'))
|
||||||
|
|
||||||
def test_bad_authorizing_roles(self):
|
def test_bad_authorizing_roles(self):
|
||||||
consumer = self._create_single_consumer()
|
consumer = self._create_single_consumer()
|
||||||
|
@ -725,6 +796,40 @@ class MaliciousOAuth1Tests(OAuth1Tests):
|
||||||
self.admin_request(path=url, method='PUT',
|
self.admin_request(path=url, method='PUT',
|
||||||
body=body, expected_status=http_client.NOT_FOUND)
|
body=body, expected_status=http_client.NOT_FOUND)
|
||||||
|
|
||||||
|
def test_no_authorizing_user_id(self):
|
||||||
|
consumer = self._create_single_consumer()
|
||||||
|
consumer_id = consumer['id']
|
||||||
|
consumer_secret = consumer['secret']
|
||||||
|
consumer = {'key': consumer_id, 'secret': consumer_secret}
|
||||||
|
|
||||||
|
url, headers = self._create_request_token(consumer, self.project_id)
|
||||||
|
content = self.post(
|
||||||
|
url, headers=headers,
|
||||||
|
response_content_type='application/x-www-form-urlencoded')
|
||||||
|
credentials = _urllib_parse_qs_text_keys(content.result)
|
||||||
|
request_key = credentials['oauth_token'][0]
|
||||||
|
request_secret = credentials['oauth_token_secret'][0]
|
||||||
|
request_token = oauth1.Token(request_key, request_secret)
|
||||||
|
|
||||||
|
url = self._authorize_request_token(request_key)
|
||||||
|
body = {'roles': [{'id': self.role_id}]}
|
||||||
|
resp = self.put(url, body=body, expected_status=http_client.OK)
|
||||||
|
verifier = resp.result['token']['oauth_verifier']
|
||||||
|
request_token.set_verifier(verifier)
|
||||||
|
request_token_created = self.oauth_api.get_request_token(
|
||||||
|
request_key.decode('utf-8'))
|
||||||
|
request_token_created.update({'authorizing_user_id': ''})
|
||||||
|
# Update the request token that is created instead of mocking
|
||||||
|
# the whole token object to focus on what's we want to test
|
||||||
|
# here and avoid any other factors that will result in the same
|
||||||
|
# exception.
|
||||||
|
with mock.patch.object(self.oauth_api,
|
||||||
|
'get_request_token') as mock_token:
|
||||||
|
mock_token.return_value = request_token_created
|
||||||
|
url, headers = self._create_access_token(consumer, request_token)
|
||||||
|
self.post(url, headers=headers,
|
||||||
|
expected_status=http_client.UNAUTHORIZED)
|
||||||
|
|
||||||
def test_expired_authorizing_request_token(self):
|
def test_expired_authorizing_request_token(self):
|
||||||
self.config_fixture.config(group='oauth1', request_token_duration=-1)
|
self.config_fixture.config(group='oauth1', request_token_duration=-1)
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,8 @@
|
||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- >
|
||||||
|
[`bug 1616424 <https://bugs.launchpad.net/keystone/+bug/1616424>`_]
|
||||||
|
Python build-in exception was raised if create request token or access token
|
||||||
|
request from client with invalid request parameters, invalid signature for example.
|
||||||
|
The implementation is hardened by showing proper exception and displaying the
|
||||||
|
failure reasons if existent.
|
Loading…
Reference in New Issue