From 7de1ef791296d547c2691454d5cb5451087cd76b Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Wed, 16 Mar 2011 12:15:57 -0700 Subject: [PATCH] User ids are strings, and are not necessarily == name. Also fix so that non-existent user gives a 404, not a 500. --- nova/api/openstack/users.py | 17 +++++-- nova/auth/manager.py | 11 ++++- nova/tests/api/openstack/fakes.py | 4 +- nova/tests/api/openstack/test_accounts.py | 22 ++++----- nova/tests/api/openstack/test_auth.py | 8 ++-- nova/tests/api/openstack/test_users.py | 58 +++++++++++++++-------- 6 files changed, 79 insertions(+), 41 deletions(-) diff --git a/nova/api/openstack/users.py b/nova/api/openstack/users.py index ebd0f451253c..d3ab3d553afa 100644 --- a/nova/api/openstack/users.py +++ b/nova/api/openstack/users.py @@ -13,13 +13,14 @@ # License for the specific language governing permissions and limitations # under the License. -import common +from webob import exc from nova import exception from nova import flags from nova import log as logging from nova import wsgi - +from nova.api.openstack import common +from nova.api.openstack import faults from nova.auth import manager FLAGS = flags.FLAGS @@ -63,7 +64,17 @@ class Controller(wsgi.Controller): def show(self, req, id): """Return data about the given user id""" - user = self.manager.get_user(id) + + #NOTE(justinsb): The drivers are a little inconsistent in how they + # deal with "NotFound" - some throw, some return None. + try: + user = self.manager.get_user(id) + except exception.NotFound: + user = None + + if user is None: + raise faults.Fault(exc.HTTPNotFound()) + return dict(user=_translate_keys(user)) def delete(self, req, id): diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 450ab803a24e..79349962960d 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -96,10 +96,19 @@ class AuthBase(object): class User(AuthBase): - """Object representing a user""" + """Object representing a user + + The following attributes are defined: + :id: A system identifier for the user. A string (for LDAP) + :name: The user name, potentially in some more friendly format + :access: The 'username' for EC2 authentication + :secret: The 'password' for EC2 authenticatoin + :admin: ??? + """ def __init__(self, id, name, access, secret, admin): AuthBase.__init__(self) + assert isinstance(id, basestring) self.id = id self.name = name self.access = access diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py index c2ae48ce4632..5decb2badcdc 100644 --- a/nova/tests/api/openstack/fakes.py +++ b/nova/tests/api/openstack/fakes.py @@ -240,10 +240,10 @@ class FakeAuthManager(object): @classmethod def reset_fake_data(cls): - cls.auth_data = dict(acc1=User(1, 'guy1', 'acc1', 'fortytwo!', False)) + cls.auth_data = dict(u1=User('id1', 'guy1', 'acc1', 'secret1', False)) cls.projects = dict(testacct=Project('testacct', 'testacct', - 'guy1', + 'id1', 'test', [])) diff --git a/nova/tests/api/openstack/test_accounts.py b/nova/tests/api/openstack/test_accounts.py index 5cb08ffd206a..64abcf48ce33 100644 --- a/nova/tests/api/openstack/test_accounts.py +++ b/nova/tests/api/openstack/test_accounts.py @@ -19,11 +19,9 @@ import json import stubout import webob -import nova.api -import nova.api.openstack.auth -from nova import context from nova import flags from nova import test +from nova.api.openstack import accounts from nova.auth.manager import User from nova.tests.api.openstack import fakes @@ -44,9 +42,9 @@ class AccountsTest(test.TestCase): def setUp(self): super(AccountsTest, self).setUp() self.stubs = stubout.StubOutForTesting() - self.stubs.Set(nova.api.openstack.accounts.Controller, '__init__', + self.stubs.Set(accounts.Controller, '__init__', fake_init) - self.stubs.Set(nova.api.openstack.accounts.Controller, '_check_admin', + self.stubs.Set(accounts.Controller, '_check_admin', fake_admin_check) fakes.FakeAuthManager.clear_fakes() fakes.FakeAuthDatabase.data = {} @@ -57,8 +55,8 @@ class AccountsTest(test.TestCase): self.allow_admin = FLAGS.allow_admin_api FLAGS.allow_admin_api = True fakemgr = fakes.FakeAuthManager() - joeuser = User(1, 'guy1', 'acc1', 'fortytwo!', False) - superuser = User(2, 'guy2', 'acc2', 'swordfish', True) + joeuser = User('id1', 'guy1', 'acc1', 'secret1', False) + superuser = User('id2', 'guy2', 'acc2', 'secret2', True) fakemgr.add_user(joeuser) fakemgr.add_user(superuser) fakemgr.create_project('test1', joeuser) @@ -76,7 +74,7 @@ class AccountsTest(test.TestCase): self.assertEqual(res_dict['account']['id'], 'test1') self.assertEqual(res_dict['account']['name'], 'test1') - self.assertEqual(res_dict['account']['manager'], 'guy1') + self.assertEqual(res_dict['account']['manager'], 'id1') self.assertEqual(res.status_int, 200) def test_account_delete(self): @@ -88,7 +86,7 @@ class AccountsTest(test.TestCase): def test_account_create(self): body = dict(account=dict(description='test account', - manager='guy1')) + manager='id1')) req = webob.Request.blank('/v1.0/accounts/newacct') req.headers["Content-Type"] = "application/json" req.method = 'PUT' @@ -101,14 +99,14 @@ class AccountsTest(test.TestCase): self.assertEqual(res_dict['account']['id'], 'newacct') self.assertEqual(res_dict['account']['name'], 'newacct') self.assertEqual(res_dict['account']['description'], 'test account') - self.assertEqual(res_dict['account']['manager'], 'guy1') + self.assertEqual(res_dict['account']['manager'], 'id1') self.assertTrue('newacct' in fakes.FakeAuthManager.projects) self.assertEqual(len(fakes.FakeAuthManager.projects.values()), 3) def test_account_update(self): body = dict(account=dict(description='test account', - manager='guy2')) + manager='id2')) req = webob.Request.blank('/v1.0/accounts/test1') req.headers["Content-Type"] = "application/json" req.method = 'PUT' @@ -121,5 +119,5 @@ class AccountsTest(test.TestCase): self.assertEqual(res_dict['account']['id'], 'test1') self.assertEqual(res_dict['account']['name'], 'test1') self.assertEqual(res_dict['account']['description'], 'test account') - self.assertEqual(res_dict['account']['manager'], 'guy2') + self.assertEqual(res_dict['account']['manager'], 'id2') self.assertEqual(len(fakes.FakeAuthManager.projects.values()), 2) diff --git a/nova/tests/api/openstack/test_auth.py b/nova/tests/api/openstack/test_auth.py index e1f936bb1230..446c5c149449 100644 --- a/nova/tests/api/openstack/test_auth.py +++ b/nova/tests/api/openstack/test_auth.py @@ -51,7 +51,7 @@ class Test(test.TestCase): def test_authorize_user(self): f = fakes.FakeAuthManager() - u = nova.auth.manager.User(1, 'user1', 'user1_key', None, None) + u = nova.auth.manager.User('id1', 'user1', 'user1_key', None, None) f.add_user(u) req = webob.Request.blank('/v1.0/') @@ -66,7 +66,7 @@ class Test(test.TestCase): def test_authorize_token(self): f = fakes.FakeAuthManager() - u = nova.auth.manager.User(1, 'user1', 'user1_key', None, None) + u = nova.auth.manager.User('id1', 'user1', 'user1_key', None, None) f.add_user(u) f.create_project('user1_project', u) @@ -124,7 +124,7 @@ class Test(test.TestCase): def test_bad_user_good_key(self): f = fakes.FakeAuthManager() - u = nova.auth.manager.User(1, 'user1', 'user1_key', None, None) + u = nova.auth.manager.User('id1', 'user1', 'user1_key', None, None) f.add_user(u) req = webob.Request.blank('/v1.0/') @@ -190,7 +190,7 @@ class TestLimiter(test.TestCase): def test_authorize_token(self): f = fakes.FakeAuthManager() - u = nova.auth.manager.User(1, 'user1', 'user1_key', None, None) + u = nova.auth.manager.User('id1', 'user1', 'user1_key', None, None) f.add_user(u) f.create_project('test', u) diff --git a/nova/tests/api/openstack/test_users.py b/nova/tests/api/openstack/test_users.py index 652aac936ed8..effb2f5926dc 100644 --- a/nova/tests/api/openstack/test_users.py +++ b/nova/tests/api/openstack/test_users.py @@ -18,11 +18,10 @@ import json import stubout import webob -import nova.api -import nova.api.openstack.auth -from nova import context from nova import flags from nova import test +from nova import utils +from nova.api.openstack import users from nova.auth.manager import User, Project from nova.tests.api.openstack import fakes @@ -43,14 +42,14 @@ class UsersTest(test.TestCase): def setUp(self): super(UsersTest, self).setUp() self.stubs = stubout.StubOutForTesting() - self.stubs.Set(nova.api.openstack.users.Controller, '__init__', + self.stubs.Set(users.Controller, '__init__', fake_init) - self.stubs.Set(nova.api.openstack.users.Controller, '_check_admin', + self.stubs.Set(users.Controller, '_check_admin', fake_admin_check) fakes.FakeAuthManager.clear_fakes() fakes.FakeAuthManager.projects = dict(testacct=Project('testacct', 'testacct', - 'guy1', + 'id1', 'test', [])) fakes.FakeAuthDatabase.data = {} @@ -61,8 +60,8 @@ class UsersTest(test.TestCase): self.allow_admin = FLAGS.allow_admin_api FLAGS.allow_admin_api = True fakemgr = fakes.FakeAuthManager() - fakemgr.add_user(User(1, 'guy1', 'acc1', 'fortytwo!', False)) - fakemgr.add_user(User(2, 'guy2', 'acc2', 'swordfish', True)) + fakemgr.add_user(User('id1', 'guy1', 'acc1', 'secret1', False)) + fakemgr.add_user(User('id2', 'guy2', 'acc2', 'secret2', True)) def tearDown(self): self.stubs.UnsetAll() @@ -78,28 +77,44 @@ class UsersTest(test.TestCase): self.assertEqual(len(res_dict['users']), 2) def test_get_user_by_id(self): - req = webob.Request.blank('/v1.0/users/guy2') + req = webob.Request.blank('/v1.0/users/id2') res = req.get_response(fakes.wsgi_app()) res_dict = json.loads(res.body) - self.assertEqual(res_dict['user']['id'], 'guy2') + self.assertEqual(res_dict['user']['id'], 'id2') self.assertEqual(res_dict['user']['name'], 'guy2') - self.assertEqual(res_dict['user']['secret'], 'swordfish') + self.assertEqual(res_dict['user']['secret'], 'secret2') self.assertEqual(res_dict['user']['admin'], True) self.assertEqual(res.status_int, 200) def test_user_delete(self): - req = webob.Request.blank('/v1.0/users/guy1') + # Check the user exists + req = webob.Request.blank('/v1.0/users/id1') + res = req.get_response(fakes.wsgi_app()) + res_dict = json.loads(res.body) + + self.assertEqual(res_dict['user']['id'], 'id1') + self.assertEqual(res.status_int, 200) + + # Delete the user + req = webob.Request.blank('/v1.0/users/id1') req.method = 'DELETE' res = req.get_response(fakes.wsgi_app()) - self.assertTrue('guy1' not in [u.id for u in + self.assertTrue('id1' not in [u.id for u in fakes.FakeAuthManager.auth_data]) self.assertEqual(res.status_int, 200) + # Check the user is not returned (and returns 404) + req = webob.Request.blank('/v1.0/users/id1') + res = req.get_response(fakes.wsgi_app()) + res_dict = json.loads(res.body) + self.assertEqual(res.status_int, 404) + def test_user_create(self): + secret = utils.generate_password() body = dict(user=dict(name='test_guy', access='acc3', - secret='invasionIsInNormandy', + secret=secret, admin=True)) req = webob.Request.blank('/v1.0/users') req.headers["Content-Type"] = "application/json" @@ -110,20 +125,25 @@ class UsersTest(test.TestCase): res_dict = json.loads(res.body) self.assertEqual(res.status_int, 200) + + # NOTE(justinsb): This is a questionable assertion in general + # fake sets id=name, but others might not... self.assertEqual(res_dict['user']['id'], 'test_guy') + self.assertEqual(res_dict['user']['name'], 'test_guy') self.assertEqual(res_dict['user']['access'], 'acc3') - self.assertEqual(res_dict['user']['secret'], 'invasionIsInNormandy') + self.assertEqual(res_dict['user']['secret'], secret) self.assertEqual(res_dict['user']['admin'], True) self.assertTrue('test_guy' in [u.id for u in fakes.FakeAuthManager.auth_data]) self.assertEqual(len(fakes.FakeAuthManager.auth_data), 3) def test_user_update(self): + new_secret = utils.generate_password() body = dict(user=dict(name='guy2', access='acc2', - secret='invasionIsInNormandy')) - req = webob.Request.blank('/v1.0/users/guy2') + secret=new_secret)) + req = webob.Request.blank('/v1.0/users/id2') req.headers["Content-Type"] = "application/json" req.method = 'PUT' req.body = json.dumps(body) @@ -132,8 +152,8 @@ class UsersTest(test.TestCase): res_dict = json.loads(res.body) self.assertEqual(res.status_int, 200) - self.assertEqual(res_dict['user']['id'], 'guy2') + self.assertEqual(res_dict['user']['id'], 'id2') self.assertEqual(res_dict['user']['name'], 'guy2') self.assertEqual(res_dict['user']['access'], 'acc2') - self.assertEqual(res_dict['user']['secret'], 'invasionIsInNormandy') + self.assertEqual(res_dict['user']['secret'], new_secret) self.assertEqual(res_dict['user']['admin'], True)