From fbe558885f29e2c545e3260927a625b1027995ec Mon Sep 17 00:00:00 2001 From: Clay Gerrard <clay.gerrard@gmail.com> Date: Fri, 24 Oct 2014 01:02:53 -0700 Subject: [PATCH] Make preauth params work If you specify a token and storage url when creating a Connection, regardless of the auth api version the first request will be made directly to swift. You can either provide a preauthurl and preauthtoken or fall back to os_options' object_storage_url and auth_token keys (exposed as --os-storage-url and --os-auth-token on the command line or OS_STORAGE_URL and OS_AUTH_TOKEN in the environment). If a _retry wrapped request on a Connection fails because of invalid authentication (401) the Connection's cached token and url will be invalidated. If the Connection's retries attribute is > 0 the subsequent attempt will call get_auth to refresh the token, but the pre-configured storage_url will always be re-used. This is consistent with current auth v2 behavior and less surprising for auth v1. The pre-existing, but previously undocumented behavior/interface of get_auth would override the storage_url returned by the auth service if the 'os_storage_url' option was provided in the os_options dict. To ensure that this behavior is consistent across auth v1 and v2 from the command line and when using the Connection class as a library - the preauthurl is stashed in the os_options dict when provided. Improved Connection.get_capabilities storage_url handling to better support the consistent behavior of a preauthurl/object_storage_url on the connection regardless of auth version. Fixed up some test infrastructure to enable setting up and testing multiple requests/responses. Change-Id: I6950fb73f3e28fdddb62760cae9320e2f4336776 --- swiftclient/client.py | 25 +- tests/unit/test_shell.py | 102 ++++++- tests/unit/test_swiftclient.py | 513 +++++++++++++++++++++++++-------- tests/unit/utils.py | 168 +++++++++-- 4 files changed, 651 insertions(+), 157 deletions(-) diff --git a/swiftclient/client.py b/swiftclient/client.py index 9a6fcd84..9851b1f2 100644 --- a/swiftclient/client.py +++ b/swiftclient/client.py @@ -353,6 +353,15 @@ def get_auth(auth_url, user, key, **kwargs): """ Get authentication/authorization credentials. + :kwarg auth_version: the api version of the supplied auth params + :kwarg os_options: a dict, the openstack idenity service options + + :returns: a tuple, (storage_url, token) + + N.B. if the optional os_options paramater includes an non-empty + 'object_storage_url' key it will override the the default storage url + returned by the auth service. + The snet parameter is used for Rackspace's ServiceNet internal network implementation. In this function, it simply adds *snet-* to the beginning of the host name for the returned storage URL. With Rackspace Cloud Files, @@ -371,13 +380,6 @@ def get_auth(auth_url, user, key, **kwargs): kwargs.get('snet'), insecure=insecure) elif auth_version in AUTH_VERSIONS_V2 + AUTH_VERSIONS_V3: - # We are allowing to specify a token/storage-url to re-use - # without having to re-authenticate. - if (os_options.get('object_storage_url') and - os_options.get('auth_token')): - return (os_options.get('object_storage_url'), - os_options.get('auth_token')) - # We are handling a special use case here where the user argument # specifies both the user name and tenant name in the form tenant:user if user and not kwargs.get('tenant_name') and ':' in user: @@ -1173,8 +1175,6 @@ class Connection(object): self.key = key self.retries = retries self.http_conn = None - self.url = preauthurl - self.token = preauthtoken self.attempts = 0 self.snet = snet self.starting_backoff = starting_backoff @@ -1183,6 +1183,10 @@ class Connection(object): self.os_options = os_options or {} if tenant_name: self.os_options['tenant_name'] = tenant_name + if preauthurl: + self.os_options['object_storage_url'] = preauthurl + self.url = preauthurl or self.os_options.get('object_storage_url') + self.token = preauthtoken or self.os_options.get('auth_token') self.cacert = cacert self.insecure = insecure self.ssl_compression = ssl_compression @@ -1194,6 +1198,8 @@ class Connection(object): and len(self.http_conn) > 1): conn = self.http_conn[1] if hasattr(conn, 'close') and callable(conn.close): + # XXX: Our HTTPConnection object has no close, should be + # trying to close the requests.Session here? conn.close() self.http_conn = None @@ -1378,6 +1384,7 @@ class Connection(object): response_dict=response_dict) def get_capabilities(self, url=None): + url = url or self.url if not url: url, _ = self.get_auth() scheme = urlparse(url).scheme diff --git a/tests/unit/test_shell.py b/tests/unit/test_shell.py index 617fba22..b1080b1b 100644 --- a/tests/unit/test_shell.py +++ b/tests/unit/test_shell.py @@ -27,7 +27,8 @@ import swiftclient.utils from os.path import basename, dirname from tests.unit.test_swiftclient import MockHttpTest -from tests.unit.utils import CaptureOutput +from tests.unit.utils import CaptureOutput, fake_get_auth_keystone + if six.PY2: BUILTIN_OPEN = '__builtin__.open' @@ -40,6 +41,12 @@ mocked_os_environ = { 'ST_KEY': 'testing' } +clean_os_environ = {} +environ_prefixes = ('ST_', 'OS_') +for key in os.environ: + if any(key.startswith(m) for m in environ_prefixes): + clean_os_environ[key] = '' + def _make_args(cmd, opts, os_opts, separator='-', flags=None, cmd_args=None): """ @@ -1185,3 +1192,96 @@ class TestKeystoneOptions(MockHttpTest): opts = {'auth-version': '2.0'} self._test_options(opts, os_opts) + + +@mock.patch.dict(os.environ, clean_os_environ) +class TestAuth(MockHttpTest): + + def test_pre_authed_request(self): + url = 'https://swift.storage.example.com/v1/AUTH_test' + token = 'AUTH_tk5b6b12' + + pre_auth_env = { + 'OS_STORAGE_URL': url, + 'OS_AUTH_TOKEN': token, + } + fake_conn = self.fake_http_connection(200) + with mock.patch('swiftclient.client.http_connection', new=fake_conn): + with mock.patch.dict(os.environ, pre_auth_env): + argv = ['', 'stat'] + swiftclient.shell.main(argv) + self.assertRequests([ + ('HEAD', url, '', {'x-auth-token': token}), + ]) + + # and again with re-auth + pre_auth_env.update(mocked_os_environ) + pre_auth_env['OS_AUTH_TOKEN'] = 'expired' + fake_conn = self.fake_http_connection(401, 200, 200, headers={ + 'x-auth-token': token + '_new', + 'x-storage-url': url + '_not_used', + }) + with mock.patch.multiple('swiftclient.client', + http_connection=fake_conn, + sleep=mock.DEFAULT): + with mock.patch.dict(os.environ, pre_auth_env): + argv = ['', 'stat'] + swiftclient.shell.main(argv) + self.assertRequests([ + ('HEAD', url, '', { + 'x-auth-token': 'expired', + }), + ('GET', mocked_os_environ['ST_AUTH'], '', { + 'x-auth-user': mocked_os_environ['ST_USER'], + 'x-auth-key': mocked_os_environ['ST_KEY'], + }), + ('HEAD', url, '', { + 'x-auth-token': token + '_new', + }), + ]) + + def test_os_pre_authed_request(self): + url = 'https://swift.storage.example.com/v1/AUTH_test' + token = 'AUTH_tk5b6b12' + + pre_auth_env = { + 'OS_STORAGE_URL': url, + 'OS_AUTH_TOKEN': token, + } + fake_conn = self.fake_http_connection(200) + with mock.patch('swiftclient.client.http_connection', new=fake_conn): + with mock.patch.dict(os.environ, pre_auth_env): + argv = ['', 'stat'] + swiftclient.shell.main(argv) + self.assertRequests([ + ('HEAD', url, '', {'x-auth-token': token}), + ]) + + # and again with re-auth + os_environ = { + 'OS_AUTH_URL': 'https://keystone.example.com/v2.0/', + 'OS_TENANT_NAME': 'demo', + 'OS_USERNAME': 'demo', + 'OS_PASSWORD': 'admin', + } + os_environ.update(pre_auth_env) + os_environ['OS_AUTH_TOKEN'] = 'expired' + + fake_conn = self.fake_http_connection(401, 200) + fake_keystone = fake_get_auth_keystone(storage_url=url + '_not_used', + token=token + '_new') + with mock.patch.multiple('swiftclient.client', + http_connection=fake_conn, + get_auth_keystone=fake_keystone, + sleep=mock.DEFAULT): + with mock.patch.dict(os.environ, os_environ): + argv = ['', 'stat'] + swiftclient.shell.main(argv) + self.assertRequests([ + ('HEAD', url, '', { + 'x-auth-token': 'expired', + }), + ('HEAD', url, '', { + 'x-auth-token': token + '_new', + }), + ]) diff --git a/tests/unit/test_swiftclient.py b/tests/unit/test_swiftclient.py index 796f93c4..03600163 100644 --- a/tests/unit/test_swiftclient.py +++ b/tests/unit/test_swiftclient.py @@ -13,8 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -# TODO: More tests import logging +import json try: from unittest import mock @@ -29,8 +29,7 @@ import warnings from six.moves.urllib.parse import urlparse from six.moves import reload_module -# TODO: mock http connection class with more control over headers -from .utils import MockHttpTest, fake_get_auth_keystone +from .utils import MockHttpTest, fake_get_auth_keystone, StubResponse from swiftclient import client as c import swiftclient.utils @@ -66,12 +65,7 @@ class TestClientException(testtools.TestCase): class TestJsonImport(testtools.TestCase): def tearDown(self): - try: - import json - except ImportError: - pass - else: - reload_module(json) + reload_module(json) try: import simplejson @@ -84,7 +78,7 @@ class TestJsonImport(testtools.TestCase): def test_any(self): self.assertTrue(hasattr(c, 'json_loads')) - def test_no_simplejson(self): + def test_no_simplejson_falls_back_to_stdlib_when_reloaded(self): # break simplejson try: import simplejson @@ -92,16 +86,10 @@ class TestJsonImport(testtools.TestCase): # not installed, so we don't have to break it for these tests pass else: - delattr(simplejson, 'loads') - reload_module(c) + delattr(simplejson, 'loads') # break simple json + reload_module(c) # reload to repopulate json_loads - try: - from json import loads - except ImportError: - # this case is stested in _no_json - pass - else: - self.assertEqual(loads, c.json_loads) + self.assertEqual(c.json_loads, json.loads) class MockHttpResponse(): @@ -234,7 +222,6 @@ class TestGetAuth(MockHttpTest): self.assertEqual(token, None) def test_invalid_auth(self): - c.http_connection = self.fake_http_connection(200) self.assertRaises(c.ClientException, c.get_auth, 'http://www.tests.com', 'asdf', 'asdf', auth_version="foo") @@ -247,7 +234,7 @@ class TestGetAuth(MockHttpTest): self.assertEqual(token, 'someauthtoken') def test_auth_v1_insecure(self): - c.http_connection = self.fake_http_connection(200, auth_v1=True) + c.http_connection = self.fake_http_connection(200, 200, auth_v1=True) url, token = c.get_auth('http://www.test.com/invalid_cert', 'asdf', 'asdf', auth_version='1.0', @@ -255,10 +242,12 @@ class TestGetAuth(MockHttpTest): self.assertEqual(url, 'storageURL') self.assertEqual(token, 'someauthtoken') - self.assertRaises(c.ClientException, c.get_auth, - 'http://www.test.com/invalid_cert', - 'asdf', 'asdf', - auth_version='1.0') + e = self.assertRaises(c.ClientException, c.get_auth, + 'http://www.test.com/invalid_cert', + 'asdf', 'asdf', auth_version='1.0') + # TODO: this test is really on validating the mock and not the + # the full plumbing into the requests's 'verify' option + self.assertIn('invalid_certificate', str(e)) def test_auth_v2_with_tenant_name(self): os_options = {'tenant_name': 'asdf'} @@ -499,23 +488,29 @@ class TestGetAccount(MockHttpTest): class TestHeadAccount(MockHttpTest): def test_ok(self): - c.http_connection = self.fake_http_connection(200) - value = c.head_account('http://www.tests.com', 'asdf') - # TODO: Hmm. This doesn't really test too much as it uses a fake that - # always returns the same dict. I guess it "exercises" the code, so - # I'll leave it for now. - self.assertEqual(type(value), dict) + c.http_connection = self.fake_http_connection(200, headers={ + 'x-account-meta-color': 'blue', + }) + resp_headers = c.head_account('http://www.tests.com', 'asdf') + self.assertEqual(resp_headers['x-account-meta-color'], 'blue') + self.assertRequests([ + ('HEAD', 'http://www.tests.com', '', {'x-auth-token': 'asdf'}) + ]) def test_server_error(self): body = 'c' * 65 c.http_connection = self.fake_http_connection(500, body=body) - self.assertRaises(c.ClientException, c.head_account, - 'http://www.tests.com', 'asdf') - try: - c.head_account('http://www.tests.com', 'asdf') - except c.ClientException as e: - new_body = "[first 60 chars of response] " + body[0:60] - self.assertEqual(e.__str__()[-89:], new_body) + e = self.assertRaises(c.ClientException, c.head_account, + 'http://www.tests.com', 'asdf') + self.assertEqual(e.http_response_content, body) + self.assertEqual(e.http_status, 500) + self.assertRequests([ + ('HEAD', 'http://www.tests.com', '', {'x-auth-token': 'asdf'}) + ]) + # TODO: this is a fairly brittle test of the __repr__ on the + # ClientException which should probably be in a targeted test + new_body = "[first 60 chars of response] " + body[0:60] + self.assertEqual(e.__str__()[-89:], new_body) class TestGetContainer(MockHttpTest): @@ -566,16 +561,29 @@ class TestGetContainer(MockHttpTest): class TestHeadContainer(MockHttpTest): + def test_head_ok(self): + fake_conn = self.fake_http_connection( + 200, headers={'x-container-meta-color': 'blue'}) + with mock.patch('swiftclient.client.http_connection', + new=fake_conn): + resp = c.head_container('https://example.com/v1/AUTH_test', + 'token', 'container') + self.assertEqual(resp['x-container-meta-color'], 'blue') + self.assertRequests([ + ('HEAD', 'https://example.com/v1/AUTH_test/container', '', + {'x-auth-token': 'token'}), + ]) + def test_server_error(self): body = 'c' * 60 c.http_connection = self.fake_http_connection(500, body=body) - self.assertRaises(c.ClientException, c.head_container, - 'http://www.test.com', 'asdf', 'asdf', - ) - try: - c.head_container('http://www.test.com', 'asdf', 'asdf') - except c.ClientException as e: - self.assertEqual(e.http_response_content, body) + e = self.assertRaises(c.ClientException, c.head_container, + 'http://www.test.com', 'asdf', 'container') + self.assertRequests([ + ('HEAD', '/container', '', {'x-auth-token': 'asdf'}), + ]) + self.assertEqual(e.http_status, 500) + self.assertEqual(e.http_response_content, body) class TestPutContainer(MockHttpTest): @@ -588,13 +596,12 @@ class TestPutContainer(MockHttpTest): def test_server_error(self): body = 'c' * 60 c.http_connection = self.fake_http_connection(500, body=body) - self.assertRaises(c.ClientException, c.put_container, - 'http://www.test.com', 'asdf', 'asdf', - ) - try: - c.put_container('http://www.test.com', 'asdf', 'asdf') - except c.ClientException as e: - self.assertEqual(e.http_response_content, body) + e = self.assertRaises(c.ClientException, c.put_container, + 'http://www.test.com', 'token', 'container') + self.assertEqual(e.http_response_content, body) + self.assertRequests([ + ('PUT', '/container', '', {'x-auth-token': 'token'}), + ]) class TestDeleteContainer(MockHttpTest): @@ -617,26 +624,25 @@ class TestGetObject(MockHttpTest): query_string="hello=20") c.get_object('http://www.test.com', 'asdf', 'asdf', 'asdf', query_string="hello=20") + for req in self.iter_request_log(): + self.assertEqual(req['method'], 'GET') + self.assertEqual(req['parsed_path'].path, '/asdf/asdf') + self.assertEqual(req['parsed_path'].query, 'hello=20') + self.assertEqual(req['body'], '') + self.assertEqual(req['headers']['x-auth-token'], 'asdf') def test_request_headers(self): - request_args = {} - - def fake_request(method, url, body=None, headers=None): - request_args['method'] = method - request_args['url'] = url - request_args['body'] = body - request_args['headers'] = headers - return - conn = self.fake_http_connection(200)('http://www.test.com/') - conn[1].request = fake_request + c.http_connection = self.fake_http_connection(200) + conn = c.http_connection('http://www.test.com') headers = {'Range': 'bytes=1-2'} c.get_object('url_is_irrelevant', 'TOKEN', 'container', 'object', http_conn=conn, headers=headers) - self.assertFalse(request_args['headers'] is None, - "No headers in the request") - self.assertTrue('Range' in request_args['headers'], - "No Range header in the request") - self.assertEqual(request_args['headers']['Range'], 'bytes=1-2') + self.assertRequests([ + ('GET', '/container/object', '', { + 'x-auth-token': 'TOKEN', + 'range': 'bytes=1-2', + }), + ]) class TestHeadObject(MockHttpTest): @@ -702,17 +708,23 @@ class TestPutObject(MockHttpTest): body = 'c' * 60 c.http_connection = self.fake_http_connection(500, body=body) args = ('http://www.test.com', 'asdf', 'asdf', 'asdf', 'asdf') - self.assertRaises(c.ClientException, c.put_object, *args) - try: - c.put_object(*args) - except c.ClientException as e: - self.assertEqual(e.http_response_content, body) + e = self.assertRaises(c.ClientException, c.put_object, *args) + self.assertEqual(e.http_response_content, body) + self.assertEqual(e.http_status, 500) + self.assertRequests([ + ('PUT', '/asdf/asdf', 'asdf', {'x-auth-token': 'asdf'}), + ]) def test_query_string(self): c.http_connection = self.fake_http_connection(200, query_string="hello=20") c.put_object('http://www.test.com', 'asdf', 'asdf', 'asdf', query_string="hello=20") + for req in self.iter_request_log(): + self.assertEqual(req['method'], 'PUT') + self.assertEqual(req['parsed_path'].path, '/asdf/asdf') + self.assertEqual(req['parsed_path'].query, 'hello=20') + self.assertEqual(req['headers']['x-auth-token'], 'asdf') def test_raw_upload(self): # Raw upload happens when content_length is passed to put_object @@ -821,12 +833,14 @@ class TestPostObject(MockHttpTest): def test_server_error(self): body = 'c' * 60 c.http_connection = self.fake_http_connection(500, body=body) - args = ('http://www.test.com', 'asdf', 'asdf', 'asdf', {}) - self.assertRaises(c.ClientException, c.post_object, *args) - try: - c.post_object(*args) - except c.ClientException as e: - self.assertEqual(e.http_response_content, body) + args = ('http://www.test.com', 'token', 'container', 'obj', {}) + e = self.assertRaises(c.ClientException, c.post_object, *args) + self.assertEqual(e.http_response_content, body) + self.assertRequests([ + ('POST', 'http://www.test.com/container/obj', '', { + 'x-auth-token': 'token', + }), + ]) class TestDeleteObject(MockHttpTest): @@ -852,14 +866,112 @@ class TestGetCapabilities(MockHttpTest): def test_ok(self): conn = self.fake_http_connection(200, body='{}') http_conn = conn('http://www.test.com/info') - self.assertEqual(type(c.get_capabilities(http_conn)), dict) - self.assertTrue(http_conn[1].has_been_read) + info = c.get_capabilities(http_conn) + self.assertRequests([ + ('GET', '/info'), + ]) + self.assertEqual(info, {}) + self.assertTrue(http_conn[1].resp.has_been_read) def test_server_error(self): conn = self.fake_http_connection(500) http_conn = conn('http://www.test.com/info') self.assertRaises(c.ClientException, c.get_capabilities, http_conn) + def test_conn_get_capabilities_with_auth(self): + auth_headers = { + 'x-auth-token': 'token', + 'x-storage-url': 'http://storage.example.com/v1/AUTH_test' + } + auth_v1_response = StubResponse(headers=auth_headers) + stub_info = {'swift': {'fake': True}} + info_response = StubResponse(body=json.dumps(stub_info)) + fake_conn = self.fake_http_connection(auth_v1_response, info_response) + + conn = c.Connection('http://auth.example.com/auth/v1.0', + 'user', 'key') + with mock.patch('swiftclient.client.http_connection', + new=fake_conn): + info = conn.get_capabilities() + self.assertEqual(info, stub_info) + self.assertRequests([ + ('GET', '/auth/v1.0'), + ('GET', 'http://storage.example.com/info'), + ]) + + def test_conn_get_capabilities_with_os_auth(self): + fake_keystone = fake_get_auth_keystone( + storage_url='http://storage.example.com/v1/AUTH_test') + stub_info = {'swift': {'fake': True}} + info_response = StubResponse(body=json.dumps(stub_info)) + fake_conn = self.fake_http_connection(info_response) + + os_options = {'project_id': 'test'} + conn = c.Connection('http://keystone.example.com/v3.0', + 'user', 'key', os_options=os_options, + auth_version=3) + with mock.patch.multiple('swiftclient.client', + get_auth_keystone=fake_keystone, + http_connection=fake_conn): + info = conn.get_capabilities() + self.assertEqual(info, stub_info) + self.assertRequests([ + ('GET', 'http://storage.example.com/info'), + ]) + + def test_conn_get_capabilities_with_url_param(self): + stub_info = {'swift': {'fake': True}} + info_response = StubResponse(body=json.dumps(stub_info)) + fake_conn = self.fake_http_connection(info_response) + + conn = c.Connection('http://auth.example.com/auth/v1.0', + 'user', 'key') + with mock.patch('swiftclient.client.http_connection', + new=fake_conn): + info = conn.get_capabilities( + 'http://other-storage.example.com/info') + self.assertEqual(info, stub_info) + self.assertRequests([ + ('GET', 'http://other-storage.example.com/info'), + ]) + + def test_conn_get_capabilities_with_preauthurl_param(self): + stub_info = {'swift': {'fake': True}} + info_response = StubResponse(body=json.dumps(stub_info)) + fake_conn = self.fake_http_connection(info_response) + + storage_url = 'http://storage.example.com/v1/AUTH_test' + conn = c.Connection('http://auth.example.com/auth/v1.0', + 'user', 'key', preauthurl=storage_url) + with mock.patch('swiftclient.client.http_connection', + new=fake_conn): + info = conn.get_capabilities() + self.assertEqual(info, stub_info) + self.assertRequests([ + ('GET', 'http://storage.example.com/info'), + ]) + + def test_conn_get_capabilities_with_os_options(self): + stub_info = {'swift': {'fake': True}} + info_response = StubResponse(body=json.dumps(stub_info)) + fake_conn = self.fake_http_connection(info_response) + + storage_url = 'http://storage.example.com/v1/AUTH_test' + os_options = { + 'project_id': 'test', + 'object_storage_url': storage_url, + } + conn = c.Connection('http://keystone.example.com/v3.0', + 'user', 'key', os_options=os_options, + auth_version=3) + with mock.patch('swiftclient.client.http_connection', + new=fake_conn): + info = conn.get_capabilities() + self.assertEqual(info, stub_info) + self.assertRequests([ + ('GET', 'http://storage.example.com/info'), + ]) + class TestHTTPConnection(MockHttpTest): @@ -903,12 +1015,39 @@ class TestConnection(MockHttpTest): args = {'preauthtoken': 'atoken123', 'preauthurl': 'http://www.test.com:8080/v1/AUTH_123456'} conn = c.Connection(**args) - self.assertEqual(type(conn), c.Connection) + self.assertEqual(conn.url, args['preauthurl']) + self.assertEqual(conn.token, args['preauthtoken']) + + def test_instance_kwargs_os_token(self): + storage_url = 'http://storage.example.com/v1/AUTH_test' + token = 'token' + args = { + 'os_options': { + 'object_storage_url': storage_url, + 'auth_token': token, + } + } + conn = c.Connection(**args) + self.assertEqual(conn.url, storage_url) + self.assertEqual(conn.token, token) + + def test_instance_kwargs_token_precedence(self): + storage_url = 'http://storage.example.com/v1/AUTH_test' + token = 'token' + args = { + 'preauthurl': storage_url, + 'preauthtoken': token, + 'os_options': { + 'auth_token': 'less-specific-token', + 'object_storage_url': 'less-specific-storage-url', + } + } + conn = c.Connection(**args) + self.assertEqual(conn.url, storage_url) + self.assertEqual(conn.token, token) def test_storage_url_override(self): static_url = 'http://overridden.storage.url' - c.http_connection = self.fake_http_connection( - 200, body='[]', storage_url=static_url) conn = c.Connection('http://auth.url/', 'some_user', 'some_key', os_options={ 'object_storage_url': static_url}) @@ -930,7 +1069,15 @@ class TestConnection(MockHttpTest): mock_get_auth.return_value = ('http://auth.storage.url', 'tToken') for method, args in method_signatures: + c.http_connection = self.fake_http_connection( + 200, body='[]', storage_url=static_url) method(*args) + self.assertEqual(len(self.request_log), 1) + for request in self.iter_request_log(): + self.assertEqual(request['parsed_path'].netloc, + 'overridden.storage.url') + self.assertEqual(request['headers']['x-auth-token'], + 'tToken') def test_get_capabilities(self): conn = c.Connection() @@ -947,35 +1094,46 @@ class TestConnection(MockHttpTest): self.assertEqual(parsed.netloc, 'storage.test.com') def test_retry(self): - c.http_connection = self.fake_http_connection(500) - def quick_sleep(*args): pass c.sleep = quick_sleep conn = c.Connection('http://www.test.com', 'asdf', 'asdf') + code_iter = [500] * (conn.retries + 1) + c.http_connection = self.fake_http_connection(*code_iter) + self.assertRaises(c.ClientException, conn.head_account) self.assertEqual(conn.attempts, conn.retries + 1) def test_retry_on_ratelimit(self): - c.http_connection = self.fake_http_connection(498) def quick_sleep(*args): pass c.sleep = quick_sleep # test retries - conn = c.Connection('http://www.test.com', 'asdf', 'asdf', + conn = c.Connection('http://www.test.com/auth/v1.0', 'asdf', 'asdf', retry_on_ratelimit=True) - self.assertRaises(c.ClientException, conn.head_account) + code_iter = [200] + [498] * (conn.retries + 1) + auth_resp_headers = { + 'x-auth-token': 'asdf', + 'x-storage-url': 'http://storage/v1/test', + } + c.http_connection = self.fake_http_connection( + *code_iter, headers=auth_resp_headers) + e = self.assertRaises(c.ClientException, conn.head_account) + self.assertIn('Account HEAD failed', str(e)) self.assertEqual(conn.attempts, conn.retries + 1) # test default no-retry - conn = c.Connection('http://www.test.com', 'asdf', 'asdf') - self.assertRaises(c.ClientException, conn.head_account) + c.http_connection = self.fake_http_connection( + 200, 498, + headers=auth_resp_headers) + conn = c.Connection('http://www.test.com/auth/v1.0', 'asdf', 'asdf') + e = self.assertRaises(c.ClientException, conn.head_account) + self.assertIn('Account HEAD failed', str(e)) self.assertEqual(conn.attempts, 1) def test_resp_read_on_server_error(self): - c.http_connection = self.fake_http_connection(500) conn = c.Connection('http://www.test.com', 'asdf', 'asdf', retries=0) def get_auth(*args, **kwargs): @@ -998,25 +1156,28 @@ class TestConnection(MockHttpTest): ) for method, args in method_signatures: + c.http_connection = self.fake_http_connection(500) self.assertRaises(c.ClientException, method, *args) - try: - self.assertTrue(conn.http_conn[1].has_been_read) - except AssertionError: + requests = list(self.iter_request_log()) + self.assertEqual(len(requests), 1) + for req in requests: msg = '%s did not read resp on server error' % method.__name__ - self.fail(msg) - except Exception as e: - raise e.__class__("%s - %s" % (method.__name__, e)) + self.assertTrue(req['resp'].has_been_read, msg) def test_reauth(self): - c.http_connection = self.fake_http_connection(401) + c.http_connection = self.fake_http_connection(401, 200) def get_auth(*args, **kwargs): + # this mock, and by extension this test are not + # represenative of the unit under test. The real get_auth + # method will always return the os_option dict's + # object_storage_url which will be overridden by the + # preauthurl paramater to Connection if it is provided. return 'http://www.new.com', 'new' def swap_sleep(*args): self.swap_sleep_called = True c.get_auth = get_auth - c.http_connection = self.fake_http_connection(200) c.sleep = swap_sleep self.swap_sleep_called = False @@ -1036,6 +1197,129 @@ class TestConnection(MockHttpTest): self.assertEqual(conn.url, 'http://www.new.com') self.assertEqual(conn.token, 'new') + def test_reauth_preauth(self): + conn = c.Connection( + 'http://auth.example.com', 'user', 'password', + preauthurl='http://storage.example.com/v1/AUTH_test', + preauthtoken='expired') + auth_v1_response = StubResponse(200, headers={ + 'x-auth-token': 'token', + 'x-storage-url': 'http://storage.example.com/v1/AUTH_user', + }) + fake_conn = self.fake_http_connection(401, auth_v1_response, 200) + with mock.patch.multiple('swiftclient.client', + http_connection=fake_conn, + sleep=mock.DEFAULT): + conn.head_account() + self.assertRequests([ + ('HEAD', '/v1/AUTH_test', '', {'x-auth-token': 'expired'}), + ('GET', 'http://auth.example.com', '', { + 'x-auth-user': 'user', + 'x-auth-key': 'password'}), + ('HEAD', '/v1/AUTH_test', '', {'x-auth-token': 'token'}), + ]) + + def test_reauth_os_preauth(self): + os_preauth_options = { + 'tenant_name': 'demo', + 'object_storage_url': 'http://storage.example.com/v1/AUTH_test', + 'auth_token': 'expired', + } + conn = c.Connection('http://auth.example.com', 'user', 'password', + os_options=os_preauth_options, auth_version=2) + fake_keystone = fake_get_auth_keystone(os_preauth_options) + fake_conn = self.fake_http_connection(401, 200) + with mock.patch.multiple('swiftclient.client', + get_auth_keystone=fake_keystone, + http_connection=fake_conn, + sleep=mock.DEFAULT): + conn.head_account() + self.assertRequests([ + ('HEAD', '/v1/AUTH_test', '', {'x-auth-token': 'expired'}), + ('HEAD', '/v1/AUTH_test', '', {'x-auth-token': 'token'}), + ]) + + def test_preauth_token_with_no_storage_url_requires_auth(self): + conn = c.Connection( + 'http://auth.example.com', 'user', 'password', + preauthtoken='expired') + auth_v1_response = StubResponse(200, headers={ + 'x-auth-token': 'token', + 'x-storage-url': 'http://storage.example.com/v1/AUTH_user', + }) + fake_conn = self.fake_http_connection(auth_v1_response, 200) + with mock.patch.multiple('swiftclient.client', + http_connection=fake_conn, + sleep=mock.DEFAULT): + conn.head_account() + self.assertRequests([ + ('GET', 'http://auth.example.com', '', { + 'x-auth-user': 'user', + 'x-auth-key': 'password'}), + ('HEAD', '/v1/AUTH_user', '', {'x-auth-token': 'token'}), + ]) + + def test_os_preauth_token_with_no_storage_url_requires_auth(self): + os_preauth_options = { + 'tenant_name': 'demo', + 'auth_token': 'expired', + } + conn = c.Connection('http://auth.example.com', 'user', 'password', + os_options=os_preauth_options, auth_version=2) + storage_url = 'http://storage.example.com/v1/AUTH_user' + fake_keystone = fake_get_auth_keystone(storage_url=storage_url) + fake_conn = self.fake_http_connection(200) + with mock.patch.multiple('swiftclient.client', + get_auth_keystone=fake_keystone, + http_connection=fake_conn, + sleep=mock.DEFAULT): + conn.head_account() + self.assertRequests([ + ('HEAD', '/v1/AUTH_user', '', {'x-auth-token': 'token'}), + ]) + + def test_preauth_url_trumps_auth_url(self): + storage_url = 'http://storage.example.com/v1/AUTH_pre_url' + conn = c.Connection( + 'http://auth.example.com', 'user', 'password', + preauthurl=storage_url) + auth_v1_response = StubResponse(200, headers={ + 'x-auth-token': 'post_token', + 'x-storage-url': 'http://storage.example.com/v1/AUTH_post_url', + }) + fake_conn = self.fake_http_connection(auth_v1_response, 200) + with mock.patch.multiple('swiftclient.client', + http_connection=fake_conn, + sleep=mock.DEFAULT): + conn.head_account() + self.assertRequests([ + ('GET', 'http://auth.example.com', '', { + 'x-auth-user': 'user', + 'x-auth-key': 'password'}), + ('HEAD', '/v1/AUTH_pre_url', '', {'x-auth-token': 'post_token'}), + ]) + + def test_os_preauth_url_trumps_auth_url(self): + storage_url = 'http://storage.example.com/v1/AUTH_pre_url' + os_preauth_options = { + 'tenant_name': 'demo', + 'object_storage_url': storage_url, + } + conn = c.Connection('http://auth.example.com', 'user', 'password', + os_options=os_preauth_options, auth_version=2) + fake_keystone = fake_get_auth_keystone( + storage_url='http://storage.example.com/v1/AUTH_post_url', + token='post_token') + fake_conn = self.fake_http_connection(200) + with mock.patch.multiple('swiftclient.client', + get_auth_keystone=fake_keystone, + http_connection=fake_conn, + sleep=mock.DEFAULT): + conn.head_account() + self.assertRequests([ + ('HEAD', '/v1/AUTH_pre_url', '', {'x-auth-token': 'post_token'}), + ]) + def test_reset_stream(self): class LocalContents(object): @@ -1263,30 +1547,16 @@ class TestLogging(MockHttpTest): 'http://www.test.com', 'asdf', 'asdf', 'asdf') def test_get_error(self): - body = 'c' * 65 - conn = self.fake_http_connection( - 404, body=body)('http://www.test.com/') - request_args = {} - - def fake_request(method, url, body=None, headers=None): - request_args['method'] = method - request_args['url'] = url - request_args['body'] = body - request_args['headers'] = headers - return - conn[1].request = fake_request - headers = {'Range': 'bytes=1-2'} - self.assertRaises( - c.ClientException, - c.get_object, - 'url_is_irrelevant', 'TOKEN', 'container', 'object', - http_conn=conn, headers=headers) + c.http_connection = self.fake_http_connection(404) + e = self.assertRaises(c.ClientException, c.get_object, + 'http://www.test.com', 'asdf', 'asdf', 'asdf') + self.assertEqual(e.http_status, 404) class TestCloseConnection(MockHttpTest): def test_close_none(self): - c.http_connection = self.fake_http_connection(200) + c.http_connection = self.fake_http_connection() conn = c.Connection('http://www.test.com', 'asdf', 'asdf') self.assertEqual(conn.http_conn, None) conn.close() @@ -1294,15 +1564,12 @@ class TestCloseConnection(MockHttpTest): def test_close_ok(self): url = 'http://www.test.com' - c.http_connection = self.fake_http_connection(200) conn = c.Connection(url, 'asdf', 'asdf') self.assertEqual(conn.http_conn, None) - conn.http_conn = c.http_connection(url) self.assertEqual(type(conn.http_conn), tuple) self.assertEqual(len(conn.http_conn), 2) http_conn_obj = conn.http_conn[1] - self.assertEqual(http_conn_obj.isclosed(), False) + self.assertIsInstance(http_conn_obj, c.HTTPConnection) + self.assertFalse(hasattr(http_conn_obj, 'close')) conn.close() - self.assertEqual(http_conn_obj.isclosed(), True) - self.assertEqual(conn.http_conn, None) diff --git a/tests/unit/utils.py b/tests/unit/utils.py index 3cbb1606..080bfec0 100644 --- a/tests/unit/utils.py +++ b/tests/unit/utils.py @@ -15,24 +15,34 @@ import functools import sys from requests import RequestException +from requests.structures import CaseInsensitiveDict from time import sleep +import unittest import testtools import mock import six from six.moves import reload_module +from six.moves.urllib.parse import urlparse, ParseResult from swiftclient import client as c from swiftclient import shell as s -def fake_get_auth_keystone(os_options, exc=None, **kwargs): +def fake_get_auth_keystone(expected_os_options=None, exc=None, + storage_url='http://url/', token='token', + **kwargs): def fake_get_auth_keystone(auth_url, user, key, actual_os_options, **actual_kwargs): if exc: raise exc('test') - if actual_os_options != os_options: + # TODO: some way to require auth_url, user and key? + if expected_os_options and actual_os_options != expected_os_options: return "", None + if 'required_kwargs' in kwargs: + for k, v in kwargs['required_kwargs'].items(): + if v != actual_kwargs.get(k): + return "", None if auth_url.startswith("https") and \ auth_url.endswith("invalid-certificate") and \ @@ -45,20 +55,36 @@ def fake_get_auth_keystone(os_options, exc=None, **kwargs): actual_kwargs['cacert'] is None: from swiftclient import client as c raise c.ClientException("unverified-certificate") - if 'required_kwargs' in kwargs: - for k, v in kwargs['required_kwargs'].items(): - if v != actual_kwargs.get(k): - return "", None - return "http://url/", "token" + return storage_url, token return fake_get_auth_keystone +class StubResponse(object): + """ + Placeholder structure for use with fake_http_connect's code_iter to modify + response attributes (status, body, headers) on a per-request basis. + """ + + def __init__(self, status=200, body='', headers=None): + self.status = status + self.body = body + self.headers = headers or {} + + def fake_http_connect(*code_iter, **kwargs): + """ + Generate a callable which yields a series of stubbed responses. Because + swiftclient will reuse an HTTP connection across pipelined requests it is + not always the case that this fake is used strictly for mocking an HTTP + connection, but rather each HTTP response (i.e. each call to requests + get_response). + """ class FakeConn(object): - def __init__(self, status, etag=None, body='', timestamp='1'): + def __init__(self, status, etag=None, body='', timestamp='1', + headers=None): self.status = status self.reason = 'Fake' self.host = '1.2.3.4' @@ -69,6 +95,7 @@ def fake_http_connect(*code_iter, **kwargs): self.body = body self.timestamp = timestamp self._is_closed = True + self.headers = headers or {} def connect(self): self._is_closed = False @@ -92,6 +119,8 @@ def fake_http_connect(*code_iter, **kwargs): return FakeConn(100) def getheaders(self): + if self.headers: + return self.headers.items() headers = {'content-length': len(self.body), 'content-type': 'x-application/test', 'x-timestamp': self.timestamp, @@ -154,15 +183,20 @@ def fake_http_connect(*code_iter, **kwargs): if 'give_connect' in kwargs: kwargs['give_connect'](*args, **ckwargs) status = next(code_iter) - etag = next(etag_iter) - timestamp = next(timestamps_iter) - if status <= 0: + if isinstance(status, StubResponse): + fake_conn = FakeConn(status.status, body=status.body, + headers=status.headers) + else: + etag = next(etag_iter) + timestamp = next(timestamps_iter) + fake_conn = FakeConn(status, etag, body=kwargs.get('body', ''), + timestamp=timestamp) + if fake_conn.status <= 0: raise RequestException() - fake_conn = FakeConn(status, etag, body=kwargs.get('body', ''), - timestamp=timestamp) fake_conn.connect() return fake_conn + connect.code_iter = code_iter return connect @@ -170,10 +204,14 @@ class MockHttpTest(testtools.TestCase): def setUp(self): super(MockHttpTest, self).setUp() + self.fake_connect = None + self.request_log = [] def fake_http_connection(*args, **kwargs): + self.validateMockedRequestsConsumed() + self.request_log = [] + self.fake_connect = fake_http_connect(*args, **kwargs) _orig_http_connection = c.http_connection - return_read = kwargs.get('return_read') query_string = kwargs.get('query_string') storage_url = kwargs.get('storage_url') auth_token = kwargs.get('auth_token') @@ -185,9 +223,28 @@ class MockHttpTest(testtools.TestCase): self.assertEqual(storage_url, url) parsed, _conn = _orig_http_connection(url, proxy=proxy) - conn = fake_http_connect(*args, **kwargs)() + + class RequestsWrapper(object): + pass + conn = RequestsWrapper() def request(method, url, *args, **kwargs): + try: + conn.resp = self.fake_connect() + except StopIteration: + self.fail('Unexpected %s request for %s' % ( + method, url)) + self.request_log.append((parsed, method, url, args, + kwargs, conn.resp)) + conn.host = conn.resp.host + conn.isclosed = conn.resp.isclosed + conn.resp.has_been_read = False + _orig_read = conn.resp.read + + def read(*args, **kwargs): + conn.resp.has_been_read = True + return _orig_read(*args, **kwargs) + conn.resp.read = read if auth_token: headers = args[1] self.assertTrue('X-Auth-Token' in headers) @@ -198,25 +255,88 @@ class MockHttpTest(testtools.TestCase): if url.endswith('invalid_cert') and not insecure: from swiftclient import client as c raise c.ClientException("invalid_certificate") - elif exc: + if exc: raise exc - return + return conn.resp conn.request = request - conn.has_been_read = False - _orig_read = conn.read - - def read(*args, **kwargs): - conn.has_been_read = True - return _orig_read(*args, **kwargs) - conn.read = return_read or read + def getresponse(): + return conn.resp + conn.getresponse = getresponse return parsed, conn return wrapper self.fake_http_connection = fake_http_connection + def iter_request_log(self): + for parsed, method, path, args, kwargs, resp in self.request_log: + parts = parsed._asdict() + parts['path'] = path + full_path = ParseResult(**parts).geturl() + args = list(args) + log = dict(zip(('body', 'headers'), args)) + log.update({ + 'method': method, + 'full_path': full_path, + 'parsed_path': urlparse(full_path), + 'path': path, + 'headers': CaseInsensitiveDict(log.get('headers')), + 'resp': resp, + 'status': resp.status, + }) + yield log + + orig_assertEqual = unittest.TestCase.assertEqual + + def assertRequests(self, expected_requests): + """ + Make sure some requests were made like you expected, provide a list of + expected requests, typically in the form of [(method, path), ...] + """ + real_requests = self.iter_request_log() + for expected in expected_requests: + method, path = expected[:2] + real_request = next(real_requests) + if urlparse(path).scheme: + match_path = real_request['full_path'] + else: + match_path = real_request['path'] + self.assertEqual((method, path), (real_request['method'], + match_path)) + if len(expected) > 2: + body = expected[2] + real_request['expected'] = body + err_msg = 'Body mismatch for %(method)s %(path)s, ' \ + 'expected %(expected)r, and got %(body)r' % real_request + self.orig_assertEqual(body, real_request['body'], err_msg) + + if len(expected) > 3: + headers = expected[3] + for key, value in headers.items(): + real_request['key'] = key + real_request['expected_value'] = value + real_request['value'] = real_request['headers'].get(key) + err_msg = ( + 'Header mismatch on %(key)r, ' + 'expected %(expected_value)r and got %(value)r ' + 'for %(method)s %(path)s %(headers)r' % real_request) + self.orig_assertEqual(value, real_request['value'], + err_msg) + + def validateMockedRequestsConsumed(self): + if not self.fake_connect: + return + unused_responses = list(self.fake_connect.code_iter) + if unused_responses: + self.fail('Unused responses %r' % (unused_responses,)) + def tearDown(self): + self.validateMockedRequestsConsumed() super(MockHttpTest, self).tearDown() + # TODO: this nuke from orbit clean up seems to be encouraging + # un-hygienic mocking on the swiftclient.client module; which may lead + # to some unfortunate test order dependency bugs by way of the broken + # window theory if any other modules are similarly patched reload_module(c)