From 47ee1d7e175471483b9885eafc4514a7281c35e9 Mon Sep 17 00:00:00 2001 From: gholt Date: Sat, 10 Nov 2012 16:39:25 +0000 Subject: [PATCH] Better TempAuth storage URL guessing I know it's just TempAuth, but bug #959953 just caught my eye as something interesting to solve. This does a best guess on the storage URL to return for a given request. It allows $HOST to be used in the storage URL configuration, where $HOST will resolve to scheme://host:port. It bases the scheme on how the server is running or on storage_url_scheme if set. The host:port comes from the request's Host header if it exists, and falls back to the WSGI SERVER_NAME:SERVER_PORT otherwise. Fixes: bug #959953 DocImpact Change-Id: Ia494bcb99a04490911ee8d2cb8b12a94e77820c5 --- doc/source/deployment_guide.rst | 21 +++++-- etc/proxy-server.conf-sample | 11 ++-- swift/common/middleware/tempauth.py | 23 +++---- swift/common/swob.py | 17 +++-- test/unit/common/middleware/test_tempauth.py | 65 ++++++++++++++------ test/unit/common/test_swob.py | 63 +++++++++++++++++++ 6 files changed, 153 insertions(+), 47 deletions(-) diff --git a/doc/source/deployment_guide.rst b/doc/source/deployment_guide.rst index 205e74f30e..0dc88eda60 100644 --- a/doc/source/deployment_guide.rst +++ b/doc/source/deployment_guide.rst @@ -653,6 +653,15 @@ auth_prefix /auth/ The HTTP request path letter `v`. token_life 86400 The number of seconds a token is valid. +storage_url_scheme default Scheme to return with + storage urls: http, + https, or default + (chooses based on what + the server is running + as) This can be useful + with an SSL load + balancer in front of a + non-SSL server. ===================== =============================== ======================= Additionally, you need to list all the accounts/users you want here. The format @@ -677,12 +686,14 @@ that have been explicitly allowed for them by a .admin or .reseller_admin. The trailing optional storage_url allows you to specify an alternate url to hand back to the user upon authentication. If not specified, this defaults to:: - http[s]://:/v1/_ + $HOST/v1/_ -Where http or https depends on whether cert_file is specified in the [DEFAULT] -section, and are based on the [DEFAULT] section's bind_ip and -bind_port (falling back to 127.0.0.1 and 8080), is from this -section, and is from the user__ name. +Where $HOST will do its best to resolve to what the requester would need to use +to reach this host, is from this section, and is +from the user__ name. Note that $HOST cannot possibly handle +when you have a load balancer in front of it that does https while TempAuth +itself runs with http; in such a case, you'll have to specify the +storage_url_scheme configuration value as an override. Here are example entries, required for running the tests:: diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index 5b9080bbd4..9c941db13b 100644 --- a/etc/proxy-server.conf-sample +++ b/etc/proxy-server.conf-sample @@ -110,6 +110,10 @@ use = egg:swift#tempauth # you're not going to use such middleware and you want a bit of extra security, # you can set this to false. # allow_overrides = true +# This specifies what scheme to return with storage urls: +# http, https, or default (chooses based on what the server is running as) +# This can be useful with an SSL load balancer in front of a non-SSL server. +# storage_url_scheme = default # Lastly, you need to list all the accounts/users you want here. The format is: # user__ = [group] [group] [...] [storage_url] # or if you want underscores in or , you can base64 encode them @@ -122,11 +126,8 @@ use = egg:swift#tempauth # that have been explicitly allowed for them by a .admin or .reseller_admin. # The trailing optional storage_url allows you to specify an alternate url to # hand back to the user upon authentication. If not specified, this defaults to -# http[s]://:/v1/_ where http or https -# depends on whether cert_file is specified in the [DEFAULT] section, and -# are based on the [DEFAULT] section's bind_ip and bind_port (falling -# back to 127.0.0.1 and 8080), is from this section, and -# is from the user__ name. +# $HOST/v1/_ where $HOST will do its best to resolve +# to what the requester would need to use to reach this host. # Here are example entries, required for running the tests: user_admin_admin = admin .admin .reseller_admin user_test_tester = testing .admin diff --git a/swift/common/middleware/tempauth.py b/swift/common/middleware/tempauth.py index 6ca7c1b0fb..eb49b5f258 100644 --- a/swift/common/middleware/tempauth.py +++ b/swift/common/middleware/tempauth.py @@ -90,6 +90,7 @@ class TempAuth(object): if h.strip()] self.allow_overrides = config_true_value( conf.get('allow_overrides', 't')) + self.storage_url_scheme = conf.get('storage_url_scheme', 'default') self.users = {} for conf_key in conf: if conf_key.startswith('user_') or conf_key.startswith('user64_'): @@ -105,16 +106,10 @@ class TempAuth(object): if not values: raise ValueError('%s has no key set' % conf_key) key = values.pop(0) - if values and '://' in values[-1]: + if values and ('://' in values[-1] or '$HOST' in values[-1]): url = values.pop() else: - url = 'https://' if 'cert_file' in conf else 'http://' - ip = conf.get('bind_ip', '127.0.0.1') - if ip == '0.0.0.0': - ip = '127.0.0.1' - url += ip - url += ':' + conf.get('bind_port', '8080') + '/v1/' + \ - self.reseller_prefix + account + url = '$HOST/v1/%s%s' % (self.reseller_prefix, account) self.users[account + ':' + username] = { 'key': key, 'url': url, 'groups': values} @@ -471,11 +466,13 @@ class TempAuth(object): '%s/user/%s' % (self.reseller_prefix, account_user) memcache_client.set(memcache_user_key, token, timeout=float(expires - time())) - return Response(request=req, - headers={ - 'x-auth-token': token, - 'x-storage-token': token, - 'x-storage-url': self.users[account_user]['url']}) + resp = Response(request=req, headers={ + 'x-auth-token': token, 'x-storage-token': token}) + url = self.users[account_user]['url'].replace('$HOST', resp.host_url()) + if self.storage_url_scheme != 'default': + url = self.storage_url_scheme + ':' + url.split(':', 1)[1] + resp.headers['x-storage-url'] = url + return resp def posthooklogger(self, env, req): if not req.path.startswith(self.auth_prefix): diff --git a/swift/common/swob.py b/swift/common/swob.py index baca9313c9..026632033c 100755 --- a/swift/common/swob.py +++ b/swift/common/swob.py @@ -955,12 +955,11 @@ class Response(object): return [body] return [''] - def absolute_location(self): + def host_url(self): """ - Attempt to construct an absolute location. + Returns the best guess that can be made for an absolute location up to + the path, for example: https://host.com:1234 """ - if not self.location.startswith('/'): - return self.location if 'HTTP_HOST' in self.environ: host = self.environ['HTTP_HOST'] else: @@ -971,7 +970,15 @@ class Response(object): host, port = host.rsplit(':', 1) elif scheme == 'https' and host.endswith(':443'): host, port = host.rsplit(':', 1) - return '%s://%s%s' % (scheme, host, self.location) + return '%s://%s' % (scheme, host) + + def absolute_location(self): + """ + Attempt to construct an absolute location. + """ + if not self.location.startswith('/'): + return self.location + return self.host_url() + self.location def __call__(self, env, start_response): self.environ = env diff --git a/test/unit/common/middleware/test_tempauth.py b/test/unit/common/middleware/test_tempauth.py index d9790074e5..241035b978 100644 --- a/test/unit/common/middleware/test_tempauth.py +++ b/test/unit/common/middleware/test_tempauth.py @@ -429,6 +429,49 @@ class TestAuth(unittest.TestCase): headers={'X-Auth-User': 'act:usr'}).get_response(self.test_auth) self.assertEquals(resp.status_int, 401) + def test_storage_url_default(self): + self.test_auth = \ + auth.filter_factory({'user_test_tester': 'testing'})(FakeApp()) + req = self._make_request( + '/auth/v1.0', + headers={'X-Auth-User': 'test:tester', 'X-Auth-Key': 'testing'}) + del req.environ['HTTP_HOST'] + req.environ['SERVER_NAME'] = 'bob' + req.environ['SERVER_PORT'] = '1234' + resp = req.get_response(self.test_auth) + self.assertEquals(resp.status_int, 200) + self.assertEquals(resp.headers['x-storage-url'], + 'http://bob:1234/v1/AUTH_test') + + def test_storage_url_based_on_host(self): + self.test_auth = \ + auth.filter_factory({'user_test_tester': 'testing'})(FakeApp()) + req = self._make_request( + '/auth/v1.0', + headers={'X-Auth-User': 'test:tester', 'X-Auth-Key': 'testing'}) + req.environ['HTTP_HOST'] = 'somehost:5678' + req.environ['SERVER_NAME'] = 'bob' + req.environ['SERVER_PORT'] = '1234' + resp = req.get_response(self.test_auth) + self.assertEquals(resp.status_int, 200) + self.assertEquals(resp.headers['x-storage-url'], + 'http://somehost:5678/v1/AUTH_test') + + def test_storage_url_overriden_scheme(self): + self.test_auth = \ + auth.filter_factory({'user_test_tester': 'testing', + 'storage_url_scheme': 'fake'})(FakeApp()) + req = self._make_request( + '/auth/v1.0', + headers={'X-Auth-User': 'test:tester', 'X-Auth-Key': 'testing'}) + req.environ['HTTP_HOST'] = 'somehost:5678' + req.environ['SERVER_NAME'] = 'bob' + req.environ['SERVER_PORT'] = '1234' + resp = req.get_response(self.test_auth) + self.assertEquals(resp.status_int, 200) + self.assertEquals(resp.headers['x-storage-url'], + 'fake://somehost:5678/v1/AUTH_test') + def test_allowed_sync_hosts(self): a = auth.filter_factory({'super_admin_key': 'supertest'})(FakeApp()) self.assertEquals(a.allowed_sync_hosts, ['127.0.0.1']) @@ -598,18 +641,17 @@ class TestParseUserCreation(unittest.TestCase): def test_parse_user_creation(self): auth_filter = auth.filter_factory({ 'reseller_prefix': 'ABC', - 'bind_ip': '1.2.3.4', 'user_test_tester3': 'testing', 'user_has_url': 'urlly .admin http://a.b/v1/DEF_has', 'user_admin_admin': 'admin .admin .reseller_admin', })(FakeApp()) self.assertEquals(auth_filter.users, { 'admin:admin': { - 'url': 'http://1.2.3.4:8080/v1/ABC_admin', + 'url': '$HOST/v1/ABC_admin', 'groups': ['.admin', '.reseller_admin'], 'key': 'admin' }, 'test:tester3': { - 'url': 'http://1.2.3.4:8080/v1/ABC_test', + 'url': '$HOST/v1/ABC_test', 'groups': [], 'key': 'testing' }, 'has:url': { @@ -622,7 +664,6 @@ class TestParseUserCreation(unittest.TestCase): def test_base64_encoding(self): auth_filter = auth.filter_factory({ 'reseller_prefix': 'ABC', - 'bind_ip': '1.2.3.4', 'user64_%s_%s' % ( b64encode('test').rstrip('='), b64encode('tester3').rstrip('=')): @@ -634,7 +675,7 @@ class TestParseUserCreation(unittest.TestCase): })(FakeApp()) self.assertEquals(auth_filter.users, { 'test:tester3': { - 'url': 'http://1.2.3.4:8080/v1/ABC_test', + 'url': '$HOST/v1/ABC_test', 'groups': ['.reseller_admin'], 'key': 'testing' }, 'user_foo:ab': { @@ -644,20 +685,6 @@ class TestParseUserCreation(unittest.TestCase): }, }) - def test_bind_ip_all_zeroes(self): - auth_filter = auth.filter_factory({ - 'reseller_prefix': 'ABC', - 'bind_ip': '0.0.0.0', - 'user_admin_admin': 'admin .admin .reseller_admin', - })(FakeApp()) - self.assertEquals(auth_filter.users, { - 'admin:admin': { - 'url': 'http://127.0.0.1:8080/v1/ABC_admin', - 'groups': ['.admin', '.reseller_admin'], - 'key': 'admin', - }, - }) - def test_key_with_no_value(self): self.assertRaises(ValueError, auth.filter_factory({ 'user_test_tester3': 'testing', diff --git a/test/unit/common/test_swob.py b/test/unit/common/test_swob.py index e088e8f019..7c85f9c64a 100755 --- a/test/unit/common/test_swob.py +++ b/test/unit/common/test_swob.py @@ -681,6 +681,69 @@ class TestResponse(unittest.TestCase): resp.etag = None self.assert_('etag' not in resp.headers) + def test_host_url_default(self): + resp = self._get_response() + env = resp.environ + env['wsgi.url_scheme'] = 'http' + env['SERVER_NAME'] = 'bob' + env['SERVER_PORT'] = '1234' + del env['HTTP_HOST'] + self.assertEquals(resp.host_url(), 'http://bob:1234') + + def test_host_url_default_port_squelched(self): + resp = self._get_response() + env = resp.environ + env['wsgi.url_scheme'] = 'http' + env['SERVER_NAME'] = 'bob' + env['SERVER_PORT'] = '80' + del env['HTTP_HOST'] + self.assertEquals(resp.host_url(), 'http://bob') + + def test_host_url_https(self): + resp = self._get_response() + env = resp.environ + env['wsgi.url_scheme'] = 'https' + env['SERVER_NAME'] = 'bob' + env['SERVER_PORT'] = '1234' + del env['HTTP_HOST'] + self.assertEquals(resp.host_url(), 'https://bob:1234') + + def test_host_url_https_port_squelched(self): + resp = self._get_response() + env = resp.environ + env['wsgi.url_scheme'] = 'https' + env['SERVER_NAME'] = 'bob' + env['SERVER_PORT'] = '443' + del env['HTTP_HOST'] + self.assertEquals(resp.host_url(), 'https://bob') + + def test_host_url_host_override(self): + resp = self._get_response() + env = resp.environ + env['wsgi.url_scheme'] = 'http' + env['SERVER_NAME'] = 'bob' + env['SERVER_PORT'] = '1234' + env['HTTP_HOST'] = 'someother' + self.assertEquals(resp.host_url(), 'http://someother') + + def test_host_url_host_port_override(self): + resp = self._get_response() + env = resp.environ + env['wsgi.url_scheme'] = 'http' + env['SERVER_NAME'] = 'bob' + env['SERVER_PORT'] = '1234' + env['HTTP_HOST'] = 'someother:5678' + self.assertEquals(resp.host_url(), 'http://someother:5678') + + def test_host_url_host_https(self): + resp = self._get_response() + env = resp.environ + env['wsgi.url_scheme'] = 'https' + env['SERVER_NAME'] = 'bob' + env['SERVER_PORT'] = '1234' + env['HTTP_HOST'] = 'someother:5678' + self.assertEquals(resp.host_url(), 'https://someother:5678') + class TestUTC(unittest.TestCase): def test_tzname(self):