Merge "Move user fileds type check to identity.Manager"

5 files changed, 70 insertions, 57 deletions

diff --git a/keystone/identity/backends/kvs.py b/keystone/identity/backends/kvs.py
index 339d2e75..2eea08cf 100644
--- a/keystone/identity/backends/kvs.py
+++ b/keystone/identity/backends/kvs.py
@@ -182,8 +182,6 @@ class Identity(kvs.Base, identity.Driver):
 
     # CRUD
     def create_user(self, user_id, user):
-        user['name'] = clean.user_name(user['name'])
-        user['enabled'] = clean.user_enabled(user.get('enabled', True))
         try:
             self.get_user(user_id)
         except exception.UserNotFound:
@@ -214,13 +212,10 @@ class Identity(kvs.Base, identity.Driver):
 
     def update_user(self, user_id, user):
         if 'name' in user:
-            user['name'] = clean.user_name(user['name'])
             existing = self.db.get('user_name-%s' % user['name'])
             if existing and user_id != existing['id']:
                 msg = 'Duplicate name, %s.' % user['name']
                 raise exception.Conflict(type='user', details=msg)
-        if 'enabled' in user:
-            user['enabled'] = clean.user_enabled(user['enabled'])
         # get the old name and delete it too
         try:
             old_user = self.db.get('user-%s' % user_id)
diff --git a/keystone/identity/backends/ldap/core.py b/keystone/identity/backends/ldap/core.py
index 07cd83b6..3bc9a9c0 100644
--- a/keystone/identity/backends/ldap/core.py
+++ b/keystone/identity/backends/ldap/core.py
@@ -192,17 +192,11 @@ class Identity(identity.Driver):
     # CRUD
     def create_user(self, user_id, user):
         user = self._validate_domain(user)
-        user['name'] = clean.user_name(user['name'])
-        user['enabled'] = clean.user_enabled(user.get('enabled', True))
         user_ref = self.user.create(user)
         return self._set_default_domain(identity.filter_user(user_ref))
 
     def update_user(self, user_id, user):
         user = self._validate_domain(user)
-        if 'name' in user:
-            user['name'] = clean.user_name(user['name'])
-        if 'enabled' in user:
-            user['enabled'] = clean.user_enabled(user['enabled'])
         return self._set_default_domain(self.user.update(user_id, user))
 
     def create_project(self, tenant_id, tenant):
diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py
index b2be8774..f81feb1d 100644
--- a/keystone/identity/backends/sql.py
+++ b/keystone/identity/backends/sql.py
@@ -626,8 +626,6 @@ class Identity(sql.Base, identity.Driver):
 
     @sql.handle_conflicts(type='user')
     def create_user(self, user_id, user):
-        user['name'] = clean.user_name(user['name'])
-        user['enabled'] = clean.user_enabled(user.get('enabled', True))
         user = utils.hash_user_password(user)
         session = self.get_session()
         with session.begin():
@@ -664,10 +662,6 @@ class Identity(sql.Base, identity.Driver):
 
     @sql.handle_conflicts(type='user')
     def update_user(self, user_id, user):
-        if 'name' in user:
-            user['name'] = clean.user_name(user['name'])
-        if 'enabled' in user:
-            user['enabled'] = clean.user_enabled(user['enabled'])
         session = self.get_session()
         if 'id' in user and user_id != user['id']:
             raise exception.ValidationError('Cannot change user ID')
diff --git a/keystone/identity/core.py b/keystone/identity/core.py
index 1c2a5508..c6cc0008 100644
--- a/keystone/identity/core.py
+++ b/keystone/identity/core.py
@@ -75,22 +75,29 @@ class Manager(manager.Manager):
 
     def create_user(self, context, user_id, user_ref):
         user = user_ref.copy()
+        user['name'] = clean.user_name(user['name'])
         user.setdefault('enabled', True)
         user['enabled'] = clean.user_enabled(user['enabled'])
         return self.driver.create_user(user_id, user)
 
+    def update_user(self, context, user_id, user_ref):
+        user = user_ref.copy()
+        if 'name' in user:
+            user['name'] = clean.user_name(user['name'])
+        if 'enabled' in user:
+            user['enabled'] = clean.user_enabled(user['enabled'])
+        return self.driver.update_user(user_id, user)
+
     def create_group(self, context, group_id, group_ref):
         group = group_ref.copy()
-        if 'description' not in group:
-            group['description'] = ''
+        group.setdefault('description', '')
         return self.driver.create_group(group_id, group)
 
     def create_project(self, context, tenant_id, tenant_ref):
         tenant = tenant_ref.copy()
         tenant.setdefault('enabled', True)
         tenant['enabled'] = clean.project_enabled(tenant['enabled'])
-        if 'description' not in tenant:
-            tenant['description'] = ''
+        tenant.setdefault('description', '')
         return self.driver.create_project(tenant_id, tenant)
 
     def update_project(self, context, tenant_id, tenant_ref):
diff --git a/tests/test_backend.py b/tests/test_backend.py
index e8042ab1..bd9d53ca 100644
--- a/tests/test_backend.py
+++ b/tests/test_backend.py
@@ -128,7 +128,7 @@ class IdentityTests(object):
             'domain_id': DEFAULT_DOMAIN_ID,
             'password': 'no_meta2',
         }
-        self.identity_api.create_user(user['id'], user)
+        self.identity_man.create_user(EMPTY_CONTEXT, user['id'], user)
         self.identity_api.add_user_to_project(self.tenant_baz['id'],
                                               user['id'])
         user_ref, tenant_ref, metadata_ref = self.identity_man.authenticate(
@@ -155,7 +155,7 @@ class IdentityTests(object):
                 'name': unicode_name,
                 'domain_id': DEFAULT_DOMAIN_ID,
                 'password': uuid.uuid4().hex}
-        ref = self.identity_api.create_user(user['id'], user)
+        ref = self.identity_man.create_user(EMPTY_CONTEXT, user['id'], user)
         self.assertEqual(unicode_name, ref['name'])
 
     def test_get_project(self):
@@ -290,7 +290,8 @@ class IdentityTests(object):
         self.identity_man.create_user(EMPTY_CONTEXT, 'fake1', user)
         user['name'] = 'fake2'
         self.assertRaises(exception.Conflict,
-                          self.identity_man.create_user, EMPTY_CONTEXT,
+                          self.identity_man.create_user,
+                          EMPTY_CONTEXT,
                           'fake1',
                           user)
 
@@ -303,7 +304,8 @@ class IdentityTests(object):
         self.identity_man.create_user(EMPTY_CONTEXT, 'fake1', user)
         user['id'] = 'fake2'
         self.assertRaises(exception.Conflict,
-                          self.identity_man.create_user, EMPTY_CONTEXT,
+                          self.identity_man.create_user,
+                          EMPTY_CONTEXT,
                           'fake2',
                           user)
 
@@ -332,7 +334,7 @@ class IdentityTests(object):
                 'password': uuid.uuid4().hex}
         self.identity_man.create_user(EMPTY_CONTEXT, user['id'], user)
         user['domain_id'] = domain2['id']
-        self.identity_api.update_user(user['id'], user)
+        self.identity_man.update_user(EMPTY_CONTEXT, user['id'], user)
 
     def test_move_user_between_domains_with_clashing_names_fails(self):
         domain1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex}
@@ -356,7 +358,8 @@ class IdentityTests(object):
         # fail since the names clash
         user1['domain_id'] = domain2['id']
         self.assertRaises(exception.Conflict,
-                          self.identity_api.update_user,
+                          self.identity_man.update_user,
+                          EMPTY_CONTEXT,
                           user1['id'],
                           user1)
 
@@ -371,11 +374,12 @@ class IdentityTests(object):
                  'domain_id': DEFAULT_DOMAIN_ID,
                  'password': 'fakepass',
                  'tenants': ['bar']}
-        self.identity_api.create_user('fake1', user1)
-        self.identity_api.create_user('fake2', user2)
+        self.identity_man.create_user(EMPTY_CONTEXT, 'fake1', user1)
+        self.identity_man.create_user(EMPTY_CONTEXT, 'fake2', user2)
         user2['name'] = 'fake1'
         self.assertRaises(exception.Conflict,
-                          self.identity_api.update_user,
+                          self.identity_man.update_user,
+                          EMPTY_CONTEXT,
                           'fake2',
                           user2)
 
@@ -385,10 +389,11 @@ class IdentityTests(object):
                 'domain_id': DEFAULT_DOMAIN_ID,
                 'password': 'fakepass',
                 'tenants': ['bar']}
-        self.identity_api.create_user('fake1', user)
+        self.identity_man.create_user(EMPTY_CONTEXT, 'fake1', user)
         user['id'] = 'fake2'
         self.assertRaises(exception.ValidationError,
-                          self.identity_api.update_user,
+                          self.identity_man.update_user,
+                          EMPTY_CONTEXT,
                           'fake1',
                           user)
         user_ref = self.identity_api.get_user('fake1')
@@ -538,11 +543,15 @@ class IdentityTests(object):
         new_user1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex,
                      'password': uuid.uuid4().hex, 'enabled': True,
                      'domain_id': new_domain['id']}
-        self.identity_api.create_user(new_user1['id'], new_user1)
+        self.identity_man.create_user(EMPTY_CONTEXT,
+                                      new_user1['id'],
+                                      new_user1)
         new_user2 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex,
                      'password': uuid.uuid4().hex, 'enabled': True,
                      'domain_id': new_domain['id']}
-        self.identity_api.create_user(new_user2['id'], new_user2)
+        self.identity_man.create_user(EMPTY_CONTEXT,
+                                      new_user2['id'],
+                                      new_user2)
         roles_ref = self.identity_api.list_grants(
             user_id=new_user1['id'],
             domain_id=new_domain['id'])
@@ -588,7 +597,9 @@ class IdentityTests(object):
         new_user1 = {'id': uuid.uuid4().hex, 'name': uuid.uuid4().hex,
                      'password': uuid.uuid4().hex, 'enabled': True,
                      'domain_id': new_domain['id']}
-        self.identity_api.create_user(new_user1['id'], new_user1)
+        self.identity_man.create_user(EMPTY_CONTEXT,
+                                      new_user1['id'],
+                                      new_user1)
 
         self.assertRaises(exception.UserNotFound,
                           self.identity_api.get_roles_for_user_and_domain,
@@ -1414,7 +1425,8 @@ class IdentityTests(object):
     def test_update_user_404(self):
         user_id = uuid.uuid4().hex
         self.assertRaises(exception.UserNotFound,
-                          self.identity_api.update_user,
+                          self.identity_man.update_user,
+                          EMPTY_CONTEXT,
                           user_id,
                           {'id': user_id})
 
@@ -1423,7 +1435,7 @@ class IdentityTests(object):
                 'name': uuid.uuid4().hex,
                 'domain_id': DEFAULT_DOMAIN_ID,
                 'password': uuid.uuid4().hex}
-        self.identity_api.create_user(user['id'], user)
+        self.identity_man.create_user(EMPTY_CONTEXT, user['id'], user)
         self.identity_api.add_user_to_project(self.tenant_bar['id'],
                                               user['id'])
         self.identity_api.delete_user(user['id'])
@@ -1436,7 +1448,7 @@ class IdentityTests(object):
                 'name': uuid.uuid4().hex,
                 'domain_id': DEFAULT_DOMAIN_ID,
                 'password': uuid.uuid4().hex}
-        self.identity_api.create_user(user['id'], user)
+        self.identity_man.create_user(EMPTY_CONTEXT, user['id'], user)
         self.identity_api.add_role_to_user_and_project(
             user['id'],
             self.tenant_bar['id'],
@@ -1526,7 +1538,8 @@ class IdentityTests(object):
         user = {'id': 'fake1', 'name': 'a' * 65,
                 'domain_id': DEFAULT_DOMAIN_ID}
         self.assertRaises(exception.ValidationError,
-                          self.identity_man.create_user, EMPTY_CONTEXT,
+                          self.identity_man.create_user,
+                          EMPTY_CONTEXT,
                           'fake1',
                           user)
 
@@ -1534,7 +1547,8 @@ class IdentityTests(object):
         user = {'id': 'fake1', 'name': '',
                 'domain_id': DEFAULT_DOMAIN_ID}
         self.assertRaises(exception.ValidationError,
-                          self.identity_man.create_user, EMPTY_CONTEXT,
+                          self.identity_man.create_user,
+                          EMPTY_CONTEXT,
                           'fake1',
                           user)
 
@@ -1542,14 +1556,16 @@ class IdentityTests(object):
         user = {'id': 'fake1', 'name': None,
                 'domain_id': DEFAULT_DOMAIN_ID}
         self.assertRaises(exception.ValidationError,
-                          self.identity_man.create_user, EMPTY_CONTEXT,
+                          self.identity_man.create_user,
+                          EMPTY_CONTEXT,
                           'fake1',
                           user)
 
         user = {'id': 'fake1', 'name': 123,
                 'domain_id': DEFAULT_DOMAIN_ID}
         self.assertRaises(exception.ValidationError,
-                          self.identity_man.create_user, EMPTY_CONTEXT,
+                          self.identity_man.create_user,
+                          EMPTY_CONTEXT,
                           'fake1',
                           user)
 
@@ -1603,7 +1619,8 @@ class IdentityTests(object):
         self.identity_man.create_user(EMPTY_CONTEXT, 'fake1', user)
         user['name'] = 'a' * 65
         self.assertRaises(exception.ValidationError,
-                          self.identity_api.update_user,
+                          self.identity_man.update_user,
+                          EMPTY_CONTEXT,
                           'fake1',
                           user)
 
@@ -1613,7 +1630,8 @@ class IdentityTests(object):
         self.identity_man.create_user(EMPTY_CONTEXT, 'fake1', user)
         user['name'] = ''
         self.assertRaises(exception.ValidationError,
-                          self.identity_api.update_user,
+                          self.identity_man.update_user,
+                          EMPTY_CONTEXT,
                           'fake1',
                           user)
 
@@ -1624,13 +1642,15 @@ class IdentityTests(object):
 
         user['name'] = None
         self.assertRaises(exception.ValidationError,
-                          self.identity_api.update_user,
+                          self.identity_man.update_user,
+                          EMPTY_CONTEXT,
                           'fake1',
                           user)
 
         user['name'] = 123
         self.assertRaises(exception.ValidationError,
-                          self.identity_api.update_user,
+                          self.identity_man.update_user,
+                          EMPTY_CONTEXT,
                           'fake1',
                           user)
 
@@ -1731,54 +1751,57 @@ class IdentityTests(object):
     def test_update_user_enable(self):
         user = {'id': 'fake1', 'name': 'fake1', 'enabled': True,
                 'domain_id': DEFAULT_DOMAIN_ID}
-        self.identity_api.create_user('fake1', user)
+        self.identity_man.create_user(EMPTY_CONTEXT, 'fake1', user)
         user_ref = self.identity_api.get_user('fake1')
         self.assertEqual(user_ref['enabled'], True)
 
         user['enabled'] = False
-        self.identity_api.update_user('fake1', user)
+        self.identity_man.update_user(EMPTY_CONTEXT, 'fake1', user)
         user_ref = self.identity_api.get_user('fake1')
         self.assertEqual(user_ref['enabled'], user['enabled'])
 
         # If not present, enabled field should not be updated
         del user['enabled']
-        self.identity_api.update_user('fake1', user)
+        self.identity_man.update_user(EMPTY_CONTEXT, 'fake1', user)
         user_ref = self.identity_api.get_user('fake1')
         self.assertEqual(user_ref['enabled'], False)
 
         user['enabled'] = True
-        self.identity_api.update_user('fake1', user)
+        self.identity_man.update_user(EMPTY_CONTEXT, 'fake1', user)
         user_ref = self.identity_api.get_user('fake1')
         self.assertEqual(user_ref['enabled'], user['enabled'])
 
         del user['enabled']
-        self.identity_api.update_user('fake1', user)
+        self.identity_man.update_user(EMPTY_CONTEXT, 'fake1', user)
         user_ref = self.identity_api.get_user('fake1')
         self.assertEqual(user_ref['enabled'], True)
 
         # Integers are valid Python's booleans. Explicitly test it.
         user['enabled'] = 0
-        self.identity_api.update_user('fake1', user)
+        self.identity_man.update_user(EMPTY_CONTEXT, 'fake1', user)
         user_ref = self.identity_api.get_user('fake1')
         self.assertEqual(user_ref['enabled'], False)
 
         # Any integers other than 0 are interpreted as True
         user['enabled'] = -42
-        self.identity_api.update_user('fake1', user)
+        self.identity_man.update_user(EMPTY_CONTEXT, 'fake1', user)
         user_ref = self.identity_api.get_user('fake1')
         self.assertEqual(user_ref['enabled'], True)
 
     def test_update_user_enable_fails(self):
         user = {'id': 'fake1', 'name': 'fake1', 'enabled': True,
                 'domain_id': DEFAULT_DOMAIN_ID}
-        self.identity_api.create_user('fake1', user)
+        self.identity_man.create_user(EMPTY_CONTEXT, 'fake1', user)
         user_ref = self.identity_api.get_user('fake1')
         self.assertEqual(user_ref['enabled'], True)
 
         # Strings are not valid boolean values
         user['enabled'] = "false"
         self.assertRaises(exception.ValidationError,
-                          self.identity_api.update_user, 'fake1', user)
+                          self.identity_man.update_user,
+                          EMPTY_CONTEXT,
+                          'fake1',
+                          user)
 
     def test_update_project_enable(self):
         tenant = {'id': 'fake1', 'name': 'fake1', 'enabled': True,
@@ -2055,14 +2078,14 @@ class IdentityTests(object):
         user = {'domain_id': CONF.identity.default_domain_id,
                 'id': uuid.uuid4().hex,
                 'name': uuid.uuid4().hex, 'password': 'passw0rd'}
-        self.identity_api.create_user(user['id'], user)
+        self.identity_man.create_user(EMPTY_CONTEXT, user['id'], user)
         user_ref = self.identity_api.get_user(user['id'])
         del user['password']
         user_ref_dict = dict((x, user_ref[x]) for x in user_ref)
         self.assertDictContainsSubset(user, user_ref_dict)
 
         user['password'] = uuid.uuid4().hex
-        self.identity_api.update_user(user['id'], user)
+        self.identity_man.update_user(EMPTY_CONTEXT, user['id'], user)
         user_ref = self.identity_api.get_user(user['id'])
         del user['password']
         user_ref_dict = dict((x, user_ref[x]) for x in user_ref)