openstack/swift
Code Issues Proposed changes

We don't have to keep the retrieved token anymore

Browse Source 
Since the change in s3_token_middleware to retrieve the auth info
from keystone directly, now, we don't need to have any tokens provided
by keystone in the request header as X-Auth-Token.

Note that this makes the pipeline ordering change documented in the
related changes mandatory, even when working with a v2 Keystone server.

Change-Id: I7c251a758dfc1fedb3fb61e351de305b431afa79
Related-Change: I21e38884a2aefbb94b76c76deccd815f01db7362
Related-Change: Ic9af387b9192f285f0f486e7171eefb23968007e
This commit is contained in:
Kota Tsuyuzaki 2018-10-09 16:42:18 -07:00 committed by Tim Burke
parent 032cf3b3b4
commit 4aa71aa25c
7 changed files with 72 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
swift/common/middleware/s3api/s3request.py
View File

@ -534,7 +534,6 @@ class S3Request(swob.Request):
'string_to_sign': self.string_to_sign,
'check_signature': self.check_signature,
}
self.token = None
self.account = None
self.user_id = None
self.slo_enabled = slo_enabled
@ -1134,8 +1133,6 @@ class S3Request(swob.Request):
if method is not None:
env['REQUEST_METHOD'] = method
env['HTTP_X_AUTH_TOKEN'] = self.token
if obj:
path = '/v1/%s/%s/%s' % (account, container, obj)
elif container:
@ -1327,7 +1324,7 @@ class S3Request(swob.Request):
except swob.HTTPException as err:
sw_resp = err
else:
# reuse account and tokens
# reuse account
_, self.account, _ = split_path(sw_resp.environ['PATH_INFO'],
2, 3, True)
@ -1337,10 +1334,11 @@ class S3Request(swob.Request):
if not self.user_id:
if 'HTTP_X_USER_NAME' in sw_resp.environ:
# keystone
self.user_id = \
utf8encode("%s:%s" %
(sw_resp.environ['HTTP_X_TENANT_NAME'],
sw_resp.environ['HTTP_X_USER_NAME']))
self.user_id = "%s:%s" % (
sw_resp.environ['HTTP_X_TENANT_NAME'],
sw_resp.environ['HTTP_X_USER_NAME'])
if six.PY2 and not isinstance(self.user_id, bytes):
self.user_id = self.user_id.encode('utf8')
else:
# tempauth
self.user_id = self.access_key
@ -1501,8 +1499,8 @@ class S3AclRequest(S3Request):
# keystone
self.user_id = "%s:%s" % (sw_resp.environ['HTTP_X_TENANT_NAME'],
sw_resp.environ['HTTP_X_USER_NAME'])
self.user_id = utf8encode(self.user_id)
self.token = sw_resp.environ.get('HTTP_X_AUTH_TOKEN')
if six.PY2 and not isinstance(self.user_id, bytes):
self.user_id = self.user_id.encode('utf8')
else:
# tempauth
self.user_id = self.access_key

22
swift/common/middleware/s3api/s3token.py
View File

@ -111,10 +111,7 @@ def parse_v2_response(token):
'X-Project-Id': access_info['token']['tenant']['id'],
'X-Project-Name': access_info['token']['tenant']['name'],
}
return (
headers,
access_info['token'].get('id'),
access_info['token']['tenant'])
return headers, access_info['token']['tenant']
def parse_v3_response(token):
@ -134,7 +131,7 @@ def parse_v3_response(token):
'X-Project-Domain-Id': token['project']['domain']['id'],
'X-Project-Domain-Name': token['project']['domain']['name'],
}
return headers, None, token['project']
return headers, token['project']
class S3Token(object):
@ -308,7 +305,13 @@ class S3Token(object):
if memcache_client:
cached_auth_data = memcache_client.get(memcache_token_key)
if cached_auth_data:
headers, token_id, tenant, secret = cached_auth_data
if len(cached_auth_data) == 4:
# Old versions of swift may have cached token, too,
# but we don't need it
headers, _token, tenant, secret = cached_auth_data
else:
headers, tenant, secret = cached_auth_data
if s3_auth_details['check_signature'](secret):
self._logger.debug("Cached creds valid")
else:
@ -348,9 +351,9 @@ class S3Token(object):
try:
token = resp.json()
if 'access' in token:
headers, token_id, tenant = parse_v2_response(token)
headers, tenant = parse_v2_response(token)
elif 'token' in token:
headers, token_id, tenant = parse_v3_response(token)
headers, tenant = parse_v3_response(token)
else:
raise ValueError
if memcache_client:
@ -363,7 +366,7 @@ class S3Token(object):
access=access)
memcache_client.set(
memcache_token_key,
(headers, token_id, tenant, cred_ref.secret),
(headers, tenant, cred_ref.secret),
time=self._secret_cache_duration)
self._logger.debug("Cached keystone credentials")
except Exception:
@ -391,7 +394,6 @@ class S3Token(object):
environ, start_response)
req.headers.update(headers)
req.headers['X-Auth-Token'] = token_id
tenant_to_connect = force_tenant or tenant['id']
if six.PY2 and isinstance(tenant_to_connect, six.text_type):
tenant_to_connect = tenant_to_connect.encode('utf-8')

2
swift/common/middleware/s3api/subresource.py
View File

@ -232,6 +232,8 @@ class Owner(object):
"""
def __init__(self, id, name):
self.id = id
if not (name is None or isinstance(name, six.string_types)):
raise TypeError('name must be a string or None')
self.name = name

4
swift/common/wsgi.py
View File

@ -538,8 +538,8 @@ class SwiftHttpProxiedProtocol(SwiftHttpProtocol):
return SwiftHttpProtocol.handle(self)
def get_environ(self):
environ = SwiftHttpProtocol.get_environ(self)
def get_environ(self, *args, **kwargs):
environ = SwiftHttpProtocol.get_environ(self, *args, **kwargs)
if self.proxy_address:
environ['SERVER_ADDR'] = self.proxy_address[0]
environ['SERVER_PORT'] = self.proxy_address[1]

70
test/unit/common/middleware/s3api/test_s3api.py
View File

@ -22,7 +22,6 @@ import hashlib
import mock
import requests
import json
import copy
import six
from six.moves.urllib.parse import unquote, quote
@ -34,6 +33,7 @@ from swift.common.swob import Request
from keystonemiddleware.auth_token import AuthProtocol
from keystoneauth1.access import AccessInfoV2
from test.unit import debug_logger
from test.unit.common.middleware.s3api import S3ApiTestCase
from test.unit.common.middleware.s3api.helpers import FakeSwift
from test.unit.common.middleware.s3api.test_s3token import \
@ -335,7 +335,7 @@ class TestS3ApiMiddleware(S3ApiTestCase):
self.assertEqual(status.split()[0], '200', body)
for _, _, headers in self.swift.calls_with_headers:
self.assertNotIn('Authorization', headers)
self.assertIsNone(headers['X-Auth-Token'])
self.assertNotIn('X-Auth-Token', headers)
def test_signed_urls_v4_bad_credential(self):
def test(credential, message, extra=b''):
@ -767,7 +767,7 @@ class TestS3ApiMiddleware(S3ApiTestCase):
self.assertEqual(status.split()[0], '200', body)
for _, _, headers in self.swift.calls_with_headers:
self.assertEqual(authz_header, headers['Authorization'])
self.assertIsNone(headers['X-Auth-Token'])
self.assertNotIn('X-Auth-Token', headers)
def test_signature_v4_no_date(self):
environ = {
@ -1096,6 +1096,7 @@ class TestS3ApiMiddleware(S3ApiTestCase):
self.s3_token = S3Token(
self.keystone_auth, {'auth_uri': 'https://fakehost/identity'})
self.s3api = S3ApiMiddleware(self.s3_token, self.conf)
self.s3api.logger = debug_logger()
req = Request.blank(
'/bucket',
environ={'REQUEST_METHOD': 'PUT'},
@ -1122,6 +1123,7 @@ class TestS3ApiMiddleware(S3ApiTestCase):
self.s3_token = S3Token(
self.keystone_auth, {'auth_uri': 'https://fakehost/identity'})
self.s3api = S3ApiMiddleware(self.s3_token, self.conf)
self.s3api.logger = debug_logger()
req = Request.blank(
'/bucket',
environ={'REQUEST_METHOD': 'PUT'},
@ -1150,6 +1152,7 @@ class TestS3ApiMiddleware(S3ApiTestCase):
self.s3_token = S3Token(
self.auth_token, {'auth_uri': 'https://fakehost/identity'})
self.s3api = S3ApiMiddleware(self.s3_token, self.conf)
self.s3api.logger = debug_logger()
req = Request.blank(
'/bucket',
environ={'REQUEST_METHOD': 'PUT'},
@ -1162,6 +1165,8 @@ class TestS3ApiMiddleware(S3ApiTestCase):
with patch.object(self.s3_token, '_json_request') as mock_req:
with patch.object(self.auth_token,
'_do_fetch_token') as mock_fetch:
# sanity check
self.assertIn('id', GOOD_RESPONSE_V2['access']['token'])
mock_resp = requests.Response()
mock_resp._content = json.dumps(
GOOD_RESPONSE_V2).encode('ascii')
@ -1174,21 +1179,28 @@ class TestS3ApiMiddleware(S3ApiTestCase):
mock_fetch.return_value = (MagicMock(), mock_access_info)
status, headers, body = self.call_s3api(req)
self.assertEqual(body, b'')
# Even though s3token got a token back from keystone, we drop
# it on the floor, resulting in a 401 Unauthorized at
# `swift.common.middleware.keystoneauth` because
# keystonemiddleware's auth_token strips out all auth headers,
# significantly 'X-Identity-Status'. Without a token, it then
# sets 'X-Identity-Status: Invalid' and never contacts
# Keystone.
self.assertEqual('403 Forbidden', status)
self.assertEqual(1, mock_req.call_count)
# With X-Auth-Token, auth_token will call _do_fetch_token to
# connect to keystone in auth_token, again
self.assertEqual(1, mock_fetch.call_count)
# it never even tries to contact keystone
self.assertEqual(0, mock_fetch.call_count)
def test_s3api_with_s3_token_no_pass_token_to_auth_token(self):
def test_s3api_with_only_s3_token_in_s3acl(self):
self.swift = FakeSwift()
self.keystone_auth = KeystoneAuth(
self.swift, {'operator_roles': 'swift-user'})
self.auth_token = AuthProtocol(
self.keystone_auth, {'delay_auth_decision': 'True'})
self.s3_token = S3Token(
self.auth_token, {'auth_uri': 'https://fakehost/identity'})
self.keystone_auth, {'auth_uri': 'https://fakehost/identity'})
self.conf['s3_acl'] = True
self.s3api = S3ApiMiddleware(self.s3_token, self.conf)
self.s3api.logger = debug_logger()
req = Request.blank(
'/bucket',
environ={'REQUEST_METHOD': 'PUT'},
@ -1196,41 +1208,21 @@ class TestS3ApiMiddleware(S3ApiTestCase):
'Date': self.get_date_header()})
self.swift.register('PUT', '/v1/AUTH_TENANT_ID/bucket',
swob.HTTPCreated, {}, None)
self.swift.register('HEAD', '/v1/AUTH_TENANT_ID',
swob.HTTPOk, {}, None)
# For now, s3 acl commits the bucket owner acl via POST
# after PUT container so we need to register the resposne here
self.swift.register('POST', '/v1/AUTH_TENANT_ID/bucket',
swob.HTTPNoContent, {}, None)
self.swift.register('TEST', '/v1/AUTH_TENANT_ID',
swob.HTTPMethodNotAllowed, {}, None)
with patch.object(self.s3_token, '_json_request') as mock_req:
with patch.object(self.auth_token,
'_do_fetch_token') as mock_fetch:
mock_resp = requests.Response()
no_token_id_good_resp = copy.deepcopy(GOOD_RESPONSE_V2)
# delete token id
del no_token_id_good_resp['access']['token']['id']
mock_resp._content = json.dumps(
no_token_id_good_resp).encode('ascii')
mock_resp._content = json.dumps(GOOD_RESPONSE_V2).encode('ascii')
mock_resp.status_code = 201
mock_req.return_value = mock_resp
mock_access_info = AccessInfoV2(GOOD_RESPONSE_V2)
mock_access_info.will_expire_soon = \
lambda stale_duration: False
mock_fetch.return_value = (MagicMock(), mock_access_info)
status, headers, body = self.call_s3api(req)
# No token provided from keystone result in 401 Unauthorized
# at `swift.common.middleware.keystoneauth` because auth_token
# will remove all auth headers including 'X-Identity-Status'[1]
# and then, set X-Identity-Status: Invalid at [2]
#
# 1: https://github.com/openstack/keystonemiddleware/blob/
# master/keystonemiddleware/auth_token/__init__.py#L620
# 2: https://github.com/openstack/keystonemiddleware/blob/
# master/keystonemiddleware/auth_token/__init__.py#L627-L629
self.assertEqual('403 Forbidden', status)
self.assertEqual(body, b'')
self.assertEqual(1, mock_req.call_count)
# if no token provided from keystone, we can skip the call to
# fetch the token
self.assertEqual(0, mock_fetch.call_count)
if __name__ == '__main__':
unittest.main()

9
test/unit/common/middleware/s3api/test_s3request.py
View File

@ -249,9 +249,11 @@ class TestRequest(S3ApiTestCase):
m_swift_resp.return_value = FakeSwiftResponse()
s3_req = S3AclRequest(req.environ, MagicMock())
self.assertNotIn('s3api.auth_details', s3_req.environ)
self.assertEqual(s3_req.token, 'token')
def test_to_swift_req_Authorization_not_exist_in_swreq(self):
# the difference from
# test_authenticate_delete_Authorization_from_s3req_headers above is
# this method asserts *to_swift_req* method.
container = 'bucket'
obj = 'obj'
method = 'GET'
@ -264,9 +266,12 @@ class TestRequest(S3ApiTestCase):
m_swift_resp.return_value = FakeSwiftResponse()
s3_req = S3AclRequest(req.environ, MagicMock())
# Yes, we *want* to assert this
sw_req = s3_req.to_swift_req(method, container, obj)
# So since the result of S3AclRequest init tests and with this
# result to_swift_req doesn't add Authorization header and token
self.assertNotIn('s3api.auth_details', sw_req.environ)
self.assertEqual(sw_req.headers['X-Auth-Token'], 'token')
self.assertNotIn('X-Auth-Token', sw_req.headers)
def test_to_swift_req_subrequest_proxy_access_log(self):
container = 'bucket'

15
test/unit/common/middleware/s3api/test_s3token.py
View File

@ -196,11 +196,11 @@ class S3TokenMiddlewareTestGood(S3TokenMiddlewareTestBase):
self.middleware(req.environ, self.start_fake_response)
self.assertEqual(self.response_status, 200)
def _assert_authorized(self, req, expect_token=True,
account_path='/v1/AUTH_TENANT_ID/'):
def _assert_authorized(self, req, account_path='/v1/AUTH_TENANT_ID/'):
self.assertTrue(
req.path.startswith(account_path),
'%r does not start with %r' % (req.path, account_path))
self.assertNotIn('X-Auth-Token', req.headers)
expected_headers = {
'X-Identity-Status': 'Confirmed',
'X-Roles': 'swift-user,_member_',
@ -210,12 +210,8 @@ class S3TokenMiddlewareTestGood(S3TokenMiddlewareTestBase):
'X-Tenant-Name': 'TENANT_NAME',
'X-Project-Id': 'TENANT_ID',
'X-Project-Name': 'TENANT_NAME',
'X-Auth-Token': 'TOKEN_ID',
}
for header, value in expected_headers.items():
if header == 'X-Auth-Token' and not expect_token:
self.assertNotIn(header, req.headers)
continue
self.assertIn(header, req.headers)
self.assertEqual(value, req.headers[header])
# WSGI wants native strings for headers
@ -253,7 +249,7 @@ class S3TokenMiddlewareTestGood(S3TokenMiddlewareTestBase):
'string_to_sign': u'token',
}
req.get_response(self.middleware)
self._assert_authorized(req, expect_token=False)
self._assert_authorized(req)
def test_authorized_bytes(self):
req = Request.blank('/v1/AUTH_cfa/c/o')
@ -533,7 +529,7 @@ class S3TokenMiddlewareTestGood(S3TokenMiddlewareTestBase):
cache = MOCK_CACHE_FROM_ENV.return_value
fake_cache_response = ({}, 'token_id', {'id': 'tenant_id'}, 'secret')
fake_cache_response = ({}, {'id': 'tenant_id'}, 'secret')
cache.get.return_value = fake_cache_response
MOCK_REQUEST.return_value = TestResponse({
@ -600,8 +596,7 @@ class S3TokenMiddlewareTestGood(S3TokenMiddlewareTestBase):
self.assertTrue(MOCK_REQUEST.called)
tenant = GOOD_RESPONSE_V2['access']['token']['tenant']
token = GOOD_RESPONSE_V2['access']['token']['id']
expected_cache = (expected_headers, token, tenant, 'secret')
expected_cache = (expected_headers, tenant, 'secret')
cache.set.assert_called_once_with('s3secret/access', expected_cache,
time=20)