From e9e8ac415847a0c4a9302e1c5afb1f6a6daf1952 Mon Sep 17 00:00:00 2001 From: Ian Cordasco Date: Wed, 28 Sep 2016 11:24:24 -0500 Subject: [PATCH] Switch to using keystoneauth by default By using keystoneauth as the default transport layer for cratonclient we can handle both the case where craton is deployed without Keystone as well as the case where it is deployed with Keystone for identity and authentication. Change-Id: I69a6a742832c3571523d30a04be1cc4bd3da1e7b --- cratonclient/exceptions.py | 8 ++- cratonclient/session.py | 78 +++++++++++++++++----- cratonclient/tests/test_session.py | 62 ------------------ cratonclient/tests/unit/test_session.py | 87 +++++++++++++++++++++++++ requirements.txt | 1 + 5 files changed, 155 insertions(+), 81 deletions(-) delete mode 100644 cratonclient/tests/test_session.py create mode 100644 cratonclient/tests/unit/test_session.py diff --git a/cratonclient/exceptions.py b/cratonclient/exceptions.py index b0b17f4..2358670 100644 --- a/cratonclient/exceptions.py +++ b/cratonclient/exceptions.py @@ -29,6 +29,12 @@ class ClientException(Exception): super(ClientException, self).__init__(message) +class UnableToAuthenticate(ClientException): + """There are insufficient parameters for authentication.""" + + message = "Some of the parameters required to authenticate were missing.""" + + class Timeout(ClientException): """Catch-all class for connect and read timeouts from requests.""" @@ -90,7 +96,7 @@ class ConnectionFailed(HTTPError): class HTTPClientError(HTTPError): """Base exception for client side errors (4xx status codes).""" - message = "Something went wrong with the request.""" + message = "Something went wrong with the request." class BadRequest(HTTPClientError): diff --git a/cratonclient/session.py b/cratonclient/session.py index d823b78..b15f0bb 100644 --- a/cratonclient/session.py +++ b/cratonclient/session.py @@ -14,9 +14,10 @@ """Craton-specific session details.""" import logging +from keystoneauth1 import plugin +from keystoneauth1 import session as ksa_session from oslo_utils import encodeutils from oslo_utils import strutils -import requests from requests import exceptions as requests_exc import six @@ -39,9 +40,8 @@ class Session(object): """Initialize our Session. :param session: - The session instance (either keystoneauth1's session or requests - Session) to use as an underlying HTTP transport. If not provided, - we will create a requests Session object. + The session instance to use as an underlying HTTP transport. If + not provided, we will create a keystoneauth1 Session object. :param str username: The username of the person authenticating against the API. :param str token: @@ -49,20 +49,17 @@ class Session(object): :param str project_id: The user's project id in Craton. """ + self._auth = None if session is None: - session = requests.Session() + self._auth = CratonAuth(username=username, + project_id=project_id, + token=token) + craton_user_agent = 'python-cratonclient/{0}'.format( + cratonclient.__version__) + session = ksa_session.Session(auth=self._auth, + user_agent=craton_user_agent) self.project_id = project_id self._session = session - self._session.headers['X-Auth-User'] = username - self._session.headers['X-Auth-Project'] = str(project_id) - self._session.headers['X-Auth-Token'] = token - - craton_version = 'python-cratonclient/{0} '.format( - cratonclient.__version__) - old_user_agent = self._session.headers['User-Agent'] - - self._session.headers['User-Agent'] = craton_version + old_user_agent - self._session.headers['Accept'] = 'application/json' def delete(self, url, **kwargs): """Make a DELETE request with url and optional parameters. @@ -200,6 +197,19 @@ class Session(object): """ return self.request('PATCH', url, **kwargs) + def _request(self, **kwargs): + """Make a request and optionally remove the Keystone parameters.""" + # Default the Keystone specific arguments + kwargs.setdefault('service_type', 'fleet_management') + try: + response = self._session.request(**kwargs) + except TypeError: + # If we're using a Session object that doesn't support Keystone + # parameters, we need to remove them and retry. + kwargs.pop('service_type') + response = self._session.request(**kwargs) + return response + def request(self, method, url, **kwargs): """Make a request with a method, url, and optional parameters. @@ -221,9 +231,9 @@ class Session(object): data=kwargs.get('data'), headers=kwargs.get('headers', {}).copy()) try: - response = self._session.request(method=method, - url=url, - **kwargs) + response = self._request(method=method, + url=url, + **kwargs) except requests_exc.HTTPError as err: raise exc.HTTPError(exception=err, response=err.response) # NOTE(sigmavirus24): The ordering of Timeout before ConnectionError @@ -295,3 +305,35 @@ class Session(object): strutils.mask_password(response.text)) logger.debug(' '.join(string_parts)) + + +class CratonAuth(plugin.BaseAuthPlugin): + """Custom authentication plugin for keystoneauth1. + + This is specifically for the case where we're not using Keystone for + authentication. + """ + + def __init__(self, username, project_id, token): + """Initialize our craton authentication class.""" + self.username = username + self.project_id = project_id + self.token = token + + def get_token(self, session, **kwargs): + """Return our token.""" + return self.token + + def get_headers(self, session, **kwargs): + """Return the craton authentication headers.""" + headers = super(CratonAuth, self).get_headers(session, **kwargs) + if headers is None: + # NOTE(sigmavirus24): This means that the token must be None. We + # should not allow this to go further. We're using built-in Craton + # authentication (not authenticating against Keystone) so we will + # be unable to authenticate. + raise exc.UnableToAuthenticate() + + headers['X-Auth-User'] = self.username + headers['X-Auth-Project'] = '{}'.format(self.project_id) + return headers diff --git a/cratonclient/tests/test_session.py b/cratonclient/tests/test_session.py deleted file mode 100644 index 116c0bd..0000000 --- a/cratonclient/tests/test_session.py +++ /dev/null @@ -1,62 +0,0 @@ -# -*- coding: utf-8 -*- - -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -"""Tests for `cratonclient.session` module.""" -import uuid - -import requests - -import cratonclient -from cratonclient import session -from cratonclient.tests import base - -USERNAME = 'example' -TOKEN = uuid.uuid4().hex -PROJECT_ID = 1 - - -class TestSession(base.TestCase): - """Test our session class.""" - - def test_uses_provided_session(self): - """Verify that cratonclient does not override the session parameter.""" - requests_session = requests.Session() - craton = session.Session(session=requests_session) - self.assertIs(requests_session, craton._session) - - def test_creates_new_session(self): - """Verify that cratonclient creates a new session.""" - craton = session.Session() - self.assertIsInstance(craton._session, requests.Session) - - def test_sets_authentication_parameters_as_headers(self): - """Verify we set auth parameters as headers on the session.""" - requests_session = requests.Session() - craton = session.Session( - username=USERNAME, - token=TOKEN, - project_id=PROJECT_ID, - ) - expected_headers = { - 'X-Auth-User': USERNAME, - 'X-Auth-Token': TOKEN, - 'X-Auth-Project': str(PROJECT_ID), - 'User-Agent': 'python-cratonclient/{0} {1}'.format( - cratonclient.__version__, - requests_session.headers['User-Agent']), - 'Connection': 'keep-alive', - 'Accept-Encoding': 'gzip, deflate', - 'Accept': 'application/json', - } - self.assertItemsEqual(expected_headers, craton._session.headers) diff --git a/cratonclient/tests/unit/test_session.py b/cratonclient/tests/unit/test_session.py new file mode 100644 index 0000000..ee79497 --- /dev/null +++ b/cratonclient/tests/unit/test_session.py @@ -0,0 +1,87 @@ +# -*- coding: utf-8 -*- + +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +"""Session specific unit tests.""" +from keystoneauth1 import session as ksa_session + +from cratonclient import session +from cratonclient.tests import base + + +TEST_USERNAME_0 = 'test' +TEST_PROJECT_0 = 1 +TEST_TOKEN_0 = 'fake-token' + + +class TestCratonAuth(base.TestCase): + """Craton authentication keystoneauth plugin tests.""" + + def test_stores_authentication_details(self): + """Verify that our plugin stores auth details.""" + plugin = session.CratonAuth(username=TEST_USERNAME_0, + project_id=TEST_PROJECT_0, + token=TEST_TOKEN_0) + self.assertEqual(TEST_USERNAME_0, plugin.username) + self.assertEqual(TEST_PROJECT_0, plugin.project_id) + self.assertEqual(TEST_TOKEN_0, plugin.token) + + def test_generates_appropriate_headers(self): + """Verify we generate the X-Auth-* headers.""" + fake_session = object() + plugin = session.CratonAuth(username=TEST_USERNAME_0, + project_id=TEST_PROJECT_0, + token=TEST_TOKEN_0) + self.assertDictEqual( + { + 'X-Auth-Token': TEST_TOKEN_0, + 'X-Auth-User': TEST_USERNAME_0, + 'X-Auth-Project': '{}'.format(TEST_PROJECT_0), + }, + plugin.get_headers(fake_session) + ) + + def test_stores_token(self): + """Verify get_token returns our token.""" + fake_session = object() + plugin = session.CratonAuth(username=TEST_USERNAME_0, + project_id=TEST_PROJECT_0, + token=TEST_TOKEN_0) + + self.assertEqual(TEST_TOKEN_0, plugin.get_token(fake_session)) + + +class TestSession(base.TestCase): + """Unit tests for cratonclient's Session abstraction.""" + + def test_creates_craton_auth_plugin(self): + """Verify we default to using keystoneauth plugin auth.""" + craton_session = session.Session(username=TEST_USERNAME_0, + project_id=TEST_PROJECT_0, + token=TEST_TOKEN_0) + + self.assertIsInstance(craton_session._auth, session.CratonAuth) + + def test_creates_keystoneauth_session(self): + """Verify we default to keystoneauth sessions and semantics.""" + craton_session = session.Session(username=TEST_USERNAME_0, + project_id=TEST_PROJECT_0, + token=TEST_TOKEN_0) + + self.assertIsInstance(craton_session._session, ksa_session.Session) + + def test_will_use_the_existing_session(self): + """Verify we don't overwrite an existing session object.""" + ksa_session_obj = ksa_session.Session() + craton_session = session.Session(session=ksa_session_obj) + + self.assertIs(ksa_session_obj, craton_session._session) diff --git a/requirements.txt b/requirements.txt index 99f155e..bee1aa0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,3 +7,4 @@ six>=1.9.0 # MIT oslo.utils>=3.16.0 # Apache-2.0 pbr>=1.6 # Apache-2.0 requests>=2.10.0 # Apache-2.0 +keystoneauth1>=2.10.0 # Apache-2.0