Cleanup of FakeAuthManager

6 files changed, 115 insertions, 80 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 29811ea16..486845399 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 546ce7061..75eade4d0 100644
--- a/nova/tests/api/openstack/fakes.py
+++ b/nova/tests/api/openstack/fakes.py
@@ -234,52 +234,57 @@ class FakeAuthDatabase(object):
 
 
 class FakeAuthManager(object):
-    auth_data = {}
+    #NOTE(justinsb): Accessing static variables through instances is FUBAR
+    #NOTE(justinsb): This should also be private!
+    auth_data = []
     projects = {}
 
     @classmethod
     def clear_fakes(cls):
-        cls.auth_data = {}
+        cls.auth_data = []
         cls.projects = {}
 
     @classmethod
     def reset_fake_data(cls):
-        cls.auth_data = dict(acc1=User('guy1', 'guy1', 'acc1',
-                                       'fortytwo!', False))
+        u1 = User('id1', 'guy1', 'acc1', 'secret1', False)
+        cls.auth_data = [u1]
         cls.projects = dict(testacct=Project('testacct',
                                              'testacct',
-                                             'guy1',
+                                             'id1',
                                              'test',
                                               []))
 
-    def add_user(self, key, user):
-        FakeAuthManager.auth_data[key] = user
+    def add_user(self, user):
+        FakeAuthManager.auth_data.append(user)
 
     def get_users(self):
-        return FakeAuthManager.auth_data.values()
+        return FakeAuthManager.auth_data
 
     def get_user(self, uid):
-        for k, v in FakeAuthManager.auth_data.iteritems():
-            if v.id == uid:
-                return v
+        for user in FakeAuthManager.auth_data:
+            if user.id == uid:
+                return user
+        return None
+
+    def get_user_from_access_key(self, key):
+        for user in FakeAuthManager.auth_data:
+            if user.access == key:
+                return user
         return None
 
     def delete_user(self, uid):
-        for k, v in FakeAuthManager.auth_data.items():
-            if v.id == uid:
-                del FakeAuthManager.auth_data[k]
+        for user in FakeAuthManager.auth_data:
+            if user.id == uid:
+                FakeAuthManager.auth_data.remove(user)
         return None
 
     def create_user(self, name, access=None, secret=None, admin=False):
         u = User(name, name, access, secret, admin)
-        FakeAuthManager.auth_data[access] = u
+        FakeAuthManager.auth_data.append(u)
         return u
 
     def modify_user(self, user_id, access=None, secret=None, admin=None):
-        user = None
-        for k, v in FakeAuthManager.auth_data.iteritems():
-            if v.id == user_id:
-                user = v
+        user = self.get_user(user_id)
         if user:
             user.access = access
             user.secret = secret
@@ -326,12 +331,6 @@ class FakeAuthManager(object):
                     if (user.id in p.member_ids) or
                        (user.id == p.project_manager_id)]
 
-    def get_user_from_access_key(self, key):
-        try:
-            return FakeAuthManager.auth_data[key]
-        except KeyError:
-            raise exc.NotFound
-
 
 class FakeRateLimiter(object):
     def __init__(self, application):
diff --git a/nova/tests/api/openstack/test_accounts.py b/nova/tests/api/openstack/test_accounts.py
index 60edce769..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,10 +55,10 @@ class AccountsTest(test.TestCase):
         self.allow_admin = FLAGS.allow_admin_api
         FLAGS.allow_admin_api = True
         fakemgr = fakes.FakeAuthManager()
-        joeuser = User('guy1', 'guy1', 'acc1', 'fortytwo!', False)
-        superuser = User('guy2', 'guy2', 'acc2', 'swordfish', True)
-        fakemgr.add_user(joeuser.access, joeuser)
-        fakemgr.add_user(superuser.access, superuser)
+        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)
         fakemgr.create_project('test2', superuser)
 
@@ -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 0448ed701..21596fb25 100644
--- a/nova/tests/api/openstack/test_auth.py
+++ b/nova/tests/api/openstack/test_auth.py
@@ -39,7 +39,7 @@ class Test(test.TestCase):
         self.stubs.Set(nova.api.openstack.auth.AuthMiddleware,
             '__init__', fakes.fake_auth_init)
         self.stubs.Set(context, 'RequestContext', fakes.FakeRequestContext)
-        fakes.FakeAuthManager.auth_data = {}
+        fakes.FakeAuthManager.clear_fakes()
         fakes.FakeAuthDatabase.data = {}
         fakes.stub_out_rate_limiting(self.stubs)
         fakes.stub_out_networking(self.stubs)
@@ -51,8 +51,8 @@ class Test(test.TestCase):
 
     def test_authorize_user(self):
         f = fakes.FakeAuthManager()
-        f.add_user('user1_key',
-            nova.auth.manager.User(1, 'user1', None, None, None))
+        user = nova.auth.manager.User('id1', 'user1', 'user1_key', None, None)
+        f.add_user(user)
 
         req = webob.Request.blank('/v1.0/')
         req.headers['X-Auth-User'] = 'user1'
@@ -66,9 +66,9 @@ class Test(test.TestCase):
 
     def test_authorize_token(self):
         f = fakes.FakeAuthManager()
-        u = nova.auth.manager.User(1, 'user1', None, None, None)
-        f.add_user('user1_key', u)
-        f.create_project('user1_project', u)
+        user = nova.auth.manager.User('id1', 'user1', 'user1_key', None, None)
+        f.add_user(user)
+        f.create_project('user1_project', user)
 
         req = webob.Request.blank('/v1.0/', {'HTTP_HOST': 'foo'})
         req.headers['X-Auth-User'] = 'user1'
@@ -124,8 +124,8 @@ class Test(test.TestCase):
 
     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)
+        user = nova.auth.manager.User('id1', 'user1', 'user1_key', None, None)
+        f.add_user(user)
 
         req = webob.Request.blank('/v1.0/')
         req.headers['X-Auth-User'] = 'unknown_user'
@@ -179,7 +179,7 @@ class TestLimiter(test.TestCase):
         self.stubs.Set(nova.api.openstack.auth.AuthMiddleware,
             '__init__', fakes.fake_auth_init)
         self.stubs.Set(context, 'RequestContext', fakes.FakeRequestContext)
-        fakes.FakeAuthManager.auth_data = {}
+        fakes.FakeAuthManager.clear_fakes()
         fakes.FakeAuthDatabase.data = {}
         fakes.stub_out_networking(self.stubs)
 
@@ -190,9 +190,9 @@ class TestLimiter(test.TestCase):
 
     def test_authorize_token(self):
         f = fakes.FakeAuthManager()
-        u = nova.auth.manager.User(1, 'user1', None, None, None)
-        f.add_user('user1_key', u)
-        f.create_project('test', u)
+        user = nova.auth.manager.User('id1', 'user1', 'user1_key', None, None)
+        f.add_user(user)
+        f.create_project('test', user)
 
         req = webob.Request.blank('/v1.0/')
         req.headers['X-Auth-User'] = 'user1'
diff --git a/nova/tests/api/openstack/test_users.py b/nova/tests/api/openstack/test_users.py
index 2dda4319b..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.auth_data = {}
+        fakes.FakeAuthManager.clear_fakes()
         fakes.FakeAuthManager.projects = dict(testacct=Project('testacct',
                                                                'testacct',
-                                                               'guy1',
+                                                               'id1',
                                                                'test',
                                                                []))
         fakes.FakeAuthDatabase.data = {}
@@ -61,10 +60,8 @@ class UsersTest(test.TestCase):
         self.allow_admin = FLAGS.allow_admin_api
         FLAGS.allow_admin_api = True
         fakemgr = fakes.FakeAuthManager()
-        fakemgr.add_user('acc1', User('guy1', 'guy1', 'acc1',
-                                      'fortytwo!', False))
-        fakemgr.add_user('acc2', User('guy2', '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()
@@ -80,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
-                        fakes.FakeAuthManager.auth_data.values()])
+        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"
@@ -112,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.values()])
-        self.assertEqual(len(fakes.FakeAuthManager.auth_data.values()), 3)
+                        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)
@@ -134,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)