From f22194af48d01bf14395101ce5266326f5edc063 Mon Sep 17 00:00:00 2001 From: Dolph Mathews Date: Fri, 22 Jul 2011 15:29:32 -0500 Subject: Fixing naming conflict with builtin function next() --- keystone/backends/sqlalchemy/api/tenant.py | 68 +++++++-------- keystone/backends/sqlalchemy/api/user.py | 128 ++++++++++++++--------------- 2 files changed, 98 insertions(+), 98 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: -- cgit From f415cf64b68fe5bc49aebbec363f5647a900983b Mon Sep 17 00:00:00 2001 From: Yogeshwar Srikrishnan Date: Fri, 22 Jul 2011 15:56:29 -0500 Subject: #3 Preventing creation of users with empty user id and pwds. --- keystone/logic/service.py | 2 +- keystone/logic/types/user.py | 11 +++++++++-- keystone/test/unit/test_common.py | 8 ++++---- keystone/test/unit/test_users.py | 33 +++++++++++++++++++++++++++++++-- 4 files changed, 45 insertions(+), 9 deletions(-) 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 9bb33608..bcd7d49e 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): ' % (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 6dcc7543..53297704 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, @@ -848,7 +877,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.create_user(self.tenant, self.user, str(self.auth_token)) @@ -864,7 +893,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): -- cgit