openstack/nova
Code Issues Proposed changes

Fixed lp732866 by catching relevant exception.NotFound exception. Tests did not uncover this vulnerability due to "incorrect" FakeAuthManager. I say "incorrect" because potentially different implementations (LDAP or Database driven) of AuthManager might return different errors from get_user_from_access_key.

Browse Source 
Also, removed all references to 'bacon', 'ham', 'herp', and 'derp' and replaced them with hopefully more helpful terms.

Long story short it addresses the immediate issue while throughly ignoring the larger issue, which is correctly testing all implementations of Auth. I find this acceptable as currently the future of auth is in flux.
This commit is contained in:
Brian Lamar 2011-03-15 19:01:27 +00:00 committed by Tarmac
commit d36b4d5f37
3 changed files with 43 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
nova/api/openstack/auth.py
View File

@ -135,7 +135,11 @@ class AuthMiddleware(wsgi.Middleware):
req - wsgi.Request object
"""
ctxt = context.get_admin_context()
user = self.auth.get_user_from_access_key(key)
try:
user = self.auth.get_user_from_access_key(key)
except exception.NotFound:
user = None
if user and user.name == username:
token_hash = hashlib.sha1('%s%s%f' % (username, key,

5
nova/tests/api/openstack/fakes.py
View File

@ -321,7 +321,10 @@ class FakeAuthManager(object):
(user.id == p.project_manager_id)]
def get_user_from_access_key(self, key):
return FakeAuthManager.auth_data.get(key, None)
try:
return FakeAuthManager.auth_data[key]
except KeyError:
raise exc.NotFound
class FakeRateLimiter(object):

56
nova/tests/api/openstack/test_auth.py
View File

@ -51,11 +51,12 @@ class Test(test.TestCase):
def test_authorize_user(self):
f = fakes.FakeAuthManager()
f.add_user('derp', nova.auth.manager.User(1, 'herp', None, None, None))
f.add_user('user1_key',
nova.auth.manager.User(1, 'user1', None, None, None))
req = webob.Request.blank('/v1.0/')
req.headers['X-Auth-User'] = 'herp'
req.headers['X-Auth-Key'] = 'derp'
req.headers['X-Auth-User'] = 'user1'
req.headers['X-Auth-Key'] = 'user1_key'
result = req.get_response(fakes.wsgi_app())
self.assertEqual(result.status, '204 No Content')
self.assertEqual(len(result.headers['X-Auth-Token']), 40)
@ -65,13 +66,13 @@ class Test(test.TestCase):
def test_authorize_token(self):
f = fakes.FakeAuthManager()
u = nova.auth.manager.User(1, 'herp', None, None, None)
f.add_user('derp', u)
f.create_project('test', u)
u = nova.auth.manager.User(1, 'user1', None, None, None)
f.add_user('user1_key', u)
f.create_project('user1_project', u)
req = webob.Request.blank('/v1.0/', {'HTTP_HOST': 'foo'})
req.headers['X-Auth-User'] = 'herp'
req.headers['X-Auth-Key'] = 'derp'
req.headers['X-Auth-User'] = 'user1'
req.headers['X-Auth-Key'] = 'user1_key'
result = req.get_response(fakes.wsgi_app())
self.assertEqual(result.status, '204 No Content')
self.assertEqual(len(result.headers['X-Auth-Token']), 40)
@ -92,7 +93,7 @@ class Test(test.TestCase):
def test_token_expiry(self):
self.destroy_called = False
token_hash = 'bacon'
token_hash = 'token_hash'
def destroy_token_mock(meh, context, token):
self.destroy_called = True
@ -109,15 +110,26 @@ class Test(test.TestCase):
bad_token)
req = webob.Request.blank('/v1.0/')
req.headers['X-Auth-Token'] = 'bacon'
req.headers['X-Auth-Token'] = 'token_hash'
result = req.get_response(fakes.wsgi_app())
self.assertEqual(result.status, '401 Unauthorized')
self.assertEqual(self.destroy_called, True)
def test_bad_user(self):
def test_bad_user_bad_key(self):
req = webob.Request.blank('/v1.0/')
req.headers['X-Auth-User'] = 'herp'
req.headers['X-Auth-Key'] = 'derp'
req.headers['X-Auth-User'] = 'unknown_user'
req.headers['X-Auth-Key'] = 'unknown_user_key'
result = req.get_response(fakes.wsgi_app())
self.assertEqual(result.status, '401 Unauthorized')
def test_bad_user_good_key(self):
f = fakes.FakeAuthManager()
u = nova.auth.manager.User(1, 'user1', None, None, None)
f.add_user('user1_key', u)
req = webob.Request.blank('/v1.0/')
req.headers['X-Auth-User'] = 'unknown_user'
req.headers['X-Auth-Key'] = 'user1_key'
result = req.get_response(fakes.wsgi_app())
self.assertEqual(result.status, '401 Unauthorized')
@ -128,7 +140,7 @@ class Test(test.TestCase):
def test_bad_token(self):
req = webob.Request.blank('/v1.0/')
req.headers['X-Auth-Token'] = 'baconbaconbacon'
req.headers['X-Auth-Token'] = 'unknown_token'
result = req.get_response(fakes.wsgi_app())
self.assertEqual(result.status, '401 Unauthorized')
@ -137,11 +149,11 @@ class TestFunctional(test.TestCase):
def test_token_expiry(self):
ctx = context.get_admin_context()
tok = db.auth_token_create(ctx, dict(
token_hash='bacon',
token_hash='test_token_hash',
cdn_management_url='',
server_management_url='',
storage_url='',
user_id='ham',
user_id='user1',
))
db.auth_token_update(ctx, tok.token_hash, dict(
@ -149,13 +161,13 @@ class TestFunctional(test.TestCase):
))
req = webob.Request.blank('/v1.0/')
req.headers['X-Auth-Token'] = 'bacon'
req.headers['X-Auth-Token'] = 'test_token_hash'
result = req.get_response(fakes.wsgi_app())
self.assertEqual(result.status, '401 Unauthorized')
def test_token_doesnotexist(self):
req = webob.Request.blank('/v1.0/')
req.headers['X-Auth-Token'] = 'ham'
req.headers['X-Auth-Token'] = 'nonexistant_token_hash'
result = req.get_response(fakes.wsgi_app())
self.assertEqual(result.status, '401 Unauthorized')
@ -178,13 +190,13 @@ class TestLimiter(test.TestCase):
def test_authorize_token(self):
f = fakes.FakeAuthManager()
u = nova.auth.manager.User(1, 'herp', None, None, None)
f.add_user('derp', u)
u = nova.auth.manager.User(1, 'user1', None, None, None)
f.add_user('user1_key', u)
f.create_project('test', u)
req = webob.Request.blank('/v1.0/')
req.headers['X-Auth-User'] = 'herp'
req.headers['X-Auth-Key'] = 'derp'
req.headers['X-Auth-User'] = 'user1'
req.headers['X-Auth-Key'] = 'user1_key'
result = req.get_response(fakes.wsgi_app())
self.assertEqual(len(result.headers['X-Auth-Token']), 40)