User ids are strings, and are not necessarily == name. Also fix so that non-existent user gives a 404, not a 500.

6 files changed, 79 insertions, 41 deletions

diff --git a/nova/api/openstack/users.py b/nova/api/openstack/users.py
index ebd0f4512..d3ab3d553 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 450ab803a..793499629 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 c2ae48ce4..5decb2bad 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 5cb08ffd2..64abcf48c 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 e1f936bb1..446c5c149 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 652aac936..effb2f592 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)