Merge branch 'master' of git://github.com/rackspace/keystone

6 files changed, 143 insertions, 107 deletions

diff --git a/keystone/backends/sqlalchemy/api/tenant.py b/keystone/backends/sqlalchemy/api/tenant.py
index 56a8ba84..bdb0c36f 100755
--- a/keystone/backends/sqlalchemy/api/tenant.py
+++ b/keystone/backends/sqlalchemy/api/tenant.py
@@ -74,29 +74,29 @@ class TenantAPI(BaseTenantAPI):
             return (None, None)
         if marker is None:
             marker = first.id
-        next = q3.filter(tenant.id > marker).order_by(\
+        next_page = q3.filter(tenant.id > marker).order_by(\
                         tenant.id).limit(limit).all()
-        prev = q3.filter(tenant.id > marker).order_by(\
+        prev_page = q3.filter(tenant.id > marker).order_by(\
                         tenant.id.desc()).limit(int(limit)).all()
-        if len(next) == 0:
-            next = last
+        if len(next_page) == 0:
+            next_page = last
         else:
-            for t in next:
-                next = t
-        if len(prev) == 0:
-            prev = first
+            for t in next_page:
+                next_page = t
+        if len(prev_page) == 0:
+            prev_page = first
         else:
-            for t in prev:
-                prev = t
-        if prev.id == marker:
-            prev = None
+            for t in prev_page:
+                prev_page = t
+        if prev_page.id == marker:
+            prev_page = None
         else:
-            prev = prev.id
-        if next.id == last.id:
-            next = None
+            prev_page = prev_page.id
+        if next_page.id == last.id:
+            next_page = None
         else:
-            next = next.id
-        return (prev, next)
+            next_page = next_page.id
+        return (prev_page, next_page)
     
     
     def get_page(self, marker, limit, session=None):
@@ -123,31 +123,31 @@ class TenantAPI(BaseTenantAPI):
             return (None, None)
         if marker is None:
             marker = first.id
-        next = session.query(models.Tenant).filter("id > :marker").params(\
+        next_page = session.query(models.Tenant).filter("id > :marker").params(\
                         marker='%s' % marker).order_by(\
                         models.Tenant.id).limit(limit).all()
-        prev = session.query(models.Tenant).filter("id < :marker").params(\
+        prev_page = session.query(models.Tenant).filter("id < :marker").params(\
                         marker='%s' % marker).order_by(\
                         models.Tenant.id.desc()).limit(int(limit)).all()
-        if len(next) == 0:
-            next = last
+        if len(next_page) == 0:
+            next_page = last
         else:
-            for t in next:
-                next = t
-        if len(prev) == 0:
-            prev = first
+            for t in next_page:
+                next_page = t
+        if len(prev_page) == 0:
+            prev_page = first
         else:
-            for t in prev:
-                prev = t
-        if prev.id == marker:
-            prev = None
+            for t in prev_page:
+                prev_page = t
+        if prev_page.id == marker:
+            prev_page = None
         else:
-            prev = prev.id
-        if next.id == last.id:
-            next = None
+            prev_page = prev_page.id
+        if next_page.id == last.id:
+            next_page = None
         else:
-            next = next.id
-        return (prev, next)
+            next_page = next_page.id
+        return (prev_page, next_page)
     
     
     def is_empty(self, id, session=None):
diff --git a/keystone/backends/sqlalchemy/api/user.py b/keystone/backends/sqlalchemy/api/user.py
index 891161f4..8c53207b 100755
--- a/keystone/backends/sqlalchemy/api/user.py
+++ b/keystone/backends/sqlalchemy/api/user.py
@@ -98,31 +98,31 @@ class UserAPI(BaseUserAPI):
             return (None, None)
         if marker is None:
             marker = first.id
-        next = session.query(models.User).filter("id > :marker").params(\
+        next_page = session.query(models.User).filter("id > :marker").params(\
                         marker='%s' % marker).order_by(\
                         models.User.id).limit(limit).all()
-        prev = session.query(models.User).filter("id < :marker").params(\
+        prev_page = session.query(models.User).filter("id < :marker").params(\
                         marker='%s' % marker).order_by(\
                         models.User.id.desc()).limit(int(limit)).all()
-        if len(next) == 0:
-            next = last
+        if len(next_page) == 0:
+            next_page = last
         else:
-            for t in next:
-                next = t
-        if len(prev) == 0:
-            prev = first
+            for t in next_page:
+                next_page = t
+        if len(prev_page) == 0:
+            prev_page = first
         else:
-            for t in prev:
-                prev = t
-        if prev.id == marker:
-            prev = None
+            for t in prev_page:
+                prev_page = t
+        if prev_page.id == marker:
+            prev_page = None
         else:
-            prev = prev.id
-        if next.id == last.id:
-            next = None
+            prev_page = prev_page.id
+        if next_page.id == last.id:
+            next_page = None
         else:
-            next = next.id
-        return (prev, next)
+            next_page = next_page.id
+        return (prev_page, next_page)
     
     
     def get_by_email(self, email, session=None):
@@ -190,37 +190,37 @@ class UserAPI(BaseUserAPI):
             return (None, None)
         if marker is None:
             marker = first.id
-        next = session.query(user).join(
+        next_page = session.query(user).join(
                                 (uga, uga.user_id == user.id)).\
                                 filter(uga.group_id == group_id).\
                                 filter("id > :marker").params(\
                                 marker='%s' % marker).order_by(\
                                 user.id).limit(limit).all()
-        prev = session.query(user).join(\
+        prev_page = session.query(user).join(\
                                 (uga, uga.user_id == user.id)).\
                                 filter(uga.group_id == group_id).\
                                 filter("id < :marker").params(\
                                 marker='%s' % marker).order_by(\
                                 user.id.desc()).limit(int(limit)).all()
-        if len(next) == 0:
-            next = last
+        if len(next_page) == 0:
+            next_page = last
         else:
-            for t in next:
-                next = t
-        if len(prev) == 0:
-            prev = first
+            for t in next_page:
+                next_page = t
+        if len(prev_page) == 0:
+            prev_page = first
         else:
-            for t in prev:
-                prev = t
-        if prev.id == marker:
-            prev = None
+            for t in prev_page:
+                prev_page = t
+        if prev_page.id == marker:
+            prev_page = None
         else:
-            prev = prev.id
-        if next.id == last.id:
-            next = None
+            prev_page = prev_page.id
+        if next_page.id == last.id:
+            next_page = None
         else:
-            next = next.id
-        return (prev, next)
+            next_page = next_page.id
+        return (prev_page, next_page)
     
     
     def delete(self, id, session=None):
@@ -323,36 +323,36 @@ class UserAPI(BaseUserAPI):
             return (None, None)
         if marker is None:
             marker = first.id
-        next = session.query(user).\
+        next_page = session.query(user).\
                         filter("id > :marker").params(\
                         marker='%s' % marker).order_by(user.id).\
                         limit(int(limit)).all()
-        prev = session.query(user).\
+        prev_page = session.query(user).\
                         filter("id < :marker").params(
                         marker='%s' % marker).order_by(
                         user.id.desc()).limit(int(limit)).all()
-        next_len = len(next)
-        prev_len = len(prev)
+        next_len = len(next_page)
+        prev_len = len(prev_page)
     
         if next_len == 0:
-            next = last
+            next_page = last
         else:
-            for t in next:
-                next = t
+            for t in next_page:
+                next_page = t
         if prev_len == 0:
-            prev = first
+            prev_page = first
         else:
-            for t in prev:
-                prev = t
+            for t in prev_page:
+                prev_page = t
         if first.id == marker:
-            prev = None
+            prev_page = None
         else:
-            prev = prev.id
+            prev_page = prev_page.id
         if marker == last.id:
-            next = None
+            next_page = None
         else:
-            next = next.id
-        return (prev, next)
+            next_page = next_page.id
+        return (prev_page, next_page)
     
     
     def users_get_by_tenant_get_page(self, tenant_id, marker, limit, session=None):
@@ -400,38 +400,38 @@ class UserAPI(BaseUserAPI):
             return (None, None)
         if marker is None:
             marker = first.id
-        next = session.query(user).\
+        next_page = session.query(user).\
                         filter(user.tenant_id == tenant_id).\
                         filter("id > :marker").params(\
                         marker='%s' % marker).order_by(user.id).\
                         limit(int(limit)).all()
-        prev = session.query(user).\
+        prev_page = session.query(user).\
                         filter(user.tenant_id == tenant_id).\
                         filter("id < :marker").params(
                         marker='%s' % marker).order_by(
                         user.id.desc()).limit(int(limit)).all()
-        next_len = len(next)
-        prev_len = len(prev)
+        next_len = len(next_page)
+        prev_len = len(prev_page)
     
         if next_len == 0:
-            next = last
+            next_page = last
         else:
-            for t in next:
-                next = t
+            for t in next_page:
+                next_page = t
         if prev_len == 0:
-            prev = first
+            prev_page = first
         else:
-            for t in prev:
-                prev = t
+            for t in prev_page:
+                prev_page = t
         if first.id == marker:
-            prev = None
+            prev_page = None
         else:
-            prev = prev.id
+            prev_page = prev_page.id
         if marker == last.id:
-            next = None
+            next_page = None
         else:
-            next = next.id
-        return (prev, next)
+            next_page = next_page.id
+        return (prev_page, next_page)
     
     def user_groups_get_all(self, user_id, session=None):
         if not session:
diff --git a/keystone/logic/service.py b/keystone/logic/service.py
index 63b60f6d..524ccc34 100755
--- a/keystone/logic/service.py
+++ b/keystone/logic/service.py
@@ -439,7 +439,7 @@ class IdentityService(object):
         if not isinstance(user, User):
             raise fault.BadRequestFault("Expecting a User")
 
-        if user.user_id == None:
+        if user.user_id == None or len(user.user_id.strip()) == 0:
             raise fault.BadRequestFault("Expecting a unique User Id")
 
         if api.user.get(user.user_id) != None:
diff --git a/keystone/logic/types/user.py b/keystone/logic/types/user.py
index f1b89a1f..95bcaa38 100755
--- a/keystone/logic/types/user.py
+++ b/keystone/logic/types/user.py
@@ -44,9 +44,9 @@ class User(object):
             email = root.get("email")
             password = root.get("password")
             enabled = root.get("enabled")
-            if user_id == None:
+            if user_id == None or len(user_id.strip()) == 0:
                 raise fault.BadRequestFault("Expecting User")
-            elif password == None:
+            elif password == None or len(password.strip()) == 0:
                 raise fault.BadRequestFault("Expecting User password")
             elif email == None:
                 raise fault.BadRequestFault("Expecting User email")
@@ -73,9 +73,16 @@ class User(object):
                 user_id = None
             else:
                 user_id = user["id"]
+
             if not "password" in user:
                 raise fault.BadRequestFault("Expecting User Password")
             password = user["password"]
+            
+            if user_id == None or len(user_id.strip()) == 0:
+                raise fault.BadRequestFault("Expecting User")
+            elif password == None or len(password.strip()) == 0:
+                raise fault.BadRequestFault("Expecting User password")
+
             if "tenantId" in user:
                 tenant_id = user["tenantId"]
             else:
diff --git a/keystone/test/unit/test_common.py b/keystone/test/unit/test_common.py
index c35787b3..fb265a30 100755
--- a/keystone/test/unit/test_common.py
+++ b/keystone/test/unit/test_common.py
@@ -256,14 +256,14 @@ def delete_tenant_group_xml(groupid, tenantid, auth_token):
     return (resp, content)
 
 
-def create_user(tenantid, userid, auth_token, email=None):
+def create_user(tenantid, userid, auth_token, email=None, password = 'secrete'):
     header = httplib2.Http(".cache")
     url = '%susers' % (URL_V2)
     if email is not None:
         email_id = email
     else:
         email_id = "%s@openstack.org" % userid
-    body = {"user": {"password": "secrete",
+    body = {"user": {"password": password,
                      "id": userid,
                      "tenantId": tenantid,
                      "email": "%s" % email_id,
@@ -283,7 +283,7 @@ def delete_user(userid, auth_token):
     return resp
 
 
-def create_user_xml(tenantid, userid, auth_token, email=None):
+def create_user_xml(tenantid, userid, auth_token, email=None, password = 'secrete'):
     header = httplib2.Http(".cache")
     url = '%susers' % (URL_V2)
     if email is not None:
@@ -294,7 +294,7 @@ def create_user_xml(tenantid, userid, auth_token, email=None):
             <user xmlns="http://docs.openstack.org/identity/api/v2.0" \
             email="%s" \
             tenantId="%s" id="%s" \
-            enabled="true" password="secrete"/>' % (email_id, tenantid, userid)
+            enabled="true" password="%s"/>' % (email_id, tenantid, userid, password)
     resp, content = header.request(url, "PUT", body=body,
                               headers={"Content-Type": "application/xml",
                                        "X-Auth-Token": auth_token,
diff --git a/keystone/test/unit/test_users.py b/keystone/test/unit/test_users.py
index ae0fac90..9db4219d 100755
--- a/keystone/test/unit/test_users.py
+++ b/keystone/test/unit/test_users.py
@@ -148,6 +148,35 @@ class CreateUserTest(UserTest):
                                utils.content_type(resp))
         self.assertEqual(409, int(resp['status']))
         self.assertEqual('application/xml', utils.content_type(resp))
+        
+    def test_a_user_create_empty_password(self):
+        #JSON 
+        resp, content = utils.create_user(self.tenant,
+                              self.user,
+                              str(self.auth_token),
+                              self.email, '')
+        self.assertEqual(400, int(resp['status']))
+
+        #Blank Password        
+        resp, content = utils.create_user(self.tenant,
+                              self.user,
+                              str(self.auth_token),
+                              self.email, '')
+        self.assertEqual(400, int(resp['status']))        
+
+    def test_a_user_create_empty_username(self):
+        resp, content = utils.create_user_xml(self.tenant,
+                              '',
+                              str(self.auth_token),
+                              self.email)
+        self.assertEqual(400, int(resp['status']))
+
+        resp, content = utils.create_user(self.tenant,
+                              '',
+                              str(self.auth_token),
+                              self.email)
+        self.assertEqual(400, int(resp['status']))
+
 
     def test_a_user_create_expired_token(self):
         resp, content = utils.create_user(self.tenant, self.user,
@@ -851,7 +880,7 @@ class UpdateUserTest(UserTest):
         self.assertEqual(200, resp_val)
         #Resetting to empty email to allow other tests to pass.
         utils.user_update_json(self.auth_token,
-                                               self.userdisabled, None)
+                                               self.userdisabled, '')
 
     def test_user_update_user_disabled_xml(self):
         utils.user_set_enabled(self.auth_token, self.user, enabled=False)
@@ -867,7 +896,7 @@ class UpdateUserTest(UserTest):
         self.assertEqual('application/xml', utils.content_type(resp))
         #Resetting to empty email to allow other tests to pass.
         utils.user_update_xml(self.auth_token,
-            self.userdisabled, None)
+            self.userdisabled, '')
 
 
     def test_user_update_email_conflict(self):