Authenticating against non-existent tenant (fixed #859927)

- Revised sampledata & management api to utlize object names exclusively (related to #849007)
  - Partial fix for bug #854228

Change-Id: I026d3e543e183266ba023a961c9652f5ba9e34c6

4 files changed, 108 insertions, 61 deletions

diff --git a/keystone/logic/service.py b/keystone/logic/service.py
index 7fb340bf..0c2fce78 100755
--- a/keystone/logic/service.py
+++ b/keystone/logic/service.py
@@ -51,8 +51,10 @@ class IdentityService(object):
             return api.USER.check_password(duser, auth_request.password)
 
         if auth_request.tenant_name:
-            dtenant = api.TENANT.get_by_name(auth_request.tenant_name)
+            dtenant = self.__validate_tenant_by_name(auth_request.tenant_name)
             auth_request.tenant_id = dtenant.id
+        elif auth_request.tenant_id:
+            dtenant = self.__validate_tenant_by_id(auth_request.tenant_id)
 
         user = api.USER.get_by_name(auth_request.username)
         if not user:
@@ -73,10 +75,10 @@ class IdentityService(object):
         _token, user = self.__validate_token(auth_request.token_id)
 
         if auth_request.tenant_name:
-            dtenant = api.TENANT.get_by_name(auth_request.tenant_name)
+            dtenant = self.__validate_tenant_by_name(auth_request.tenant_name)
             auth_request.tenant_id = dtenant.id
-
-        self.__validate_tenant(auth_request.tenant_id)
+        elif auth_request.tenant_id:
+            dtenant = self.__validate_tenant_by_id(auth_request.tenant_id)
 
         def validate(duser):
             # The user is already authenticated
@@ -540,17 +542,31 @@ class IdentityService(object):
 
         return auth.ValidateData(token, user)
 
-    def __validate_tenant(self, tenant_id):
+    def __validate_tenant(self, dtenant):
+        if not dtenant:
+            raise fault.UnauthorizedFault("Tenant not found")
+
+        if not dtenant.enabled:
+            raise fault.TenantDisabledFault("Tenant %s has been disabled!"
+                % dtenant.id)
+
+        return dtenant
+
+    def __validate_tenant_by_id(self, tenant_id):
         if not tenant_id:
-            raise fault.UnauthorizedFault("Missing tenant")
+            raise fault.UnauthorizedFault("Missing tenant id")
 
-        tenant = api.TENANT.get(tenant_id)
-        if not tenant:
-            tenant = api.TENANT.get_by_name(name=tenant_id)
+        dtenant = api.TENANT.get(tenant_id)
 
-        if not tenant.enabled:
-            raise fault.TenantDisabledFault("Tenant %s has been disabled!"
-                                          % tenant.id)
+        return self.__validate_tenant(dtenant)
+
+    def __validate_tenant_by_name(self, tenant_name):
+        if not tenant_name:
+            raise fault.UnauthorizedFault("Missing tenant name")
+
+        dtenant = api.TENANT.get_by_name(name=tenant_name)
+
+        return self.__validate_tenant(dtenant)
 
     def __validate_token(self, token_id, belongs_to=None):
         if not token_id:
@@ -569,10 +585,10 @@ class IdentityService(object):
                 % user.id)
 
         if user.tenant_id:
-            self.__validate_tenant(user.tenant_id)
+            self.__validate_tenant_by_id(user.tenant_id)
 
         if token.tenant_id:
-            self.__validate_tenant(token.tenant_id)
+            self.__validate_tenant_by_id(token.tenant_id)
 
         if belongs_to and unicode(token.tenant_id) != unicode(belongs_to):
             raise fault.UnauthorizedFault("Unauthorized on this tenant")
diff --git a/keystone/manage/api.py b/keystone/manage/api.py
index 99ac741a..0d013360 100644
--- a/keystone/manage/api.py
+++ b/keystone/manage/api.py
@@ -5,9 +5,8 @@ import keystone.backends.models as db_models
 
 
 def add_user(name, password, tenant=None):
-    dbtenant = db_api.TENANT.get_by_name(tenant)
-    if dbtenant:
-        tenant = dbtenant.id
+    if tenant:
+        tenant = db_api.TENANT.get_by_name(tenant).id
 
     obj = db_models.User()
     obj.name = name
@@ -19,7 +18,7 @@ def add_user(name, password, tenant=None):
 
 def disable_user(name):
     user = db_api.USER.get_by_name(name)
-    if user == None:
+    if user is None:
         raise IndexError("User %s not found" % name)
     user.enabled = False
     return db_api.USER.update(user.id, user)
@@ -28,7 +27,7 @@ def disable_user(name):
 def list_users():
     objects = db_api.USER.get_all()
     if objects == None:
-        raise IndexError("Users not found")
+        raise IndexError("No users found")
     return [[o.id, o.enabled, o.tenant_id] for o in objects]
 
 
@@ -69,11 +68,8 @@ def list_role_assignments(tenant):
 
 
 def list_roles(tenant=None):
-
     if tenant:
-        dbtenant = db_api.TENANT.get_by_name(tenant)
-        if dbtenant:
-            tenant = dbtenant.id
+        tenant = db_api.TENANT.get_by_name(tenant).id
         return list_role_assignments(tenant)
     else:
         objects = db_api.ROLE.get_all()
@@ -84,17 +80,11 @@ def list_roles(tenant=None):
 
 def grant_role(role, user, tenant=None):
     """Grants `role` to `user` (and optionally, on `tenant`)"""
-    drole = db_api.ROLE.get_by_name(name=role)
-    if drole:
-        role = drole.id
+    role = db_api.ROLE.get_by_name(name=role).id
+    user = db_api.USER.get_by_name(name=user).id
 
-    duser = db_api.USER.get_by_name(name=user)
-    if duser:
-        user = duser.id
-
-    dtenant = db_api.TENANT.get_by_name(name=tenant)
-    if dtenant:
-        tenant = dtenant.id
+    if tenant:
+        tenant = db_api.TENANT.get_by_name(name=tenant).id
 
     obj = db_models.UserRoleAssociation()
     obj.role_id = role
@@ -107,7 +97,7 @@ def grant_role(role, user, tenant=None):
 def add_endpoint_template(region, service, public_url, admin_url, internal_url,
     enabled, is_global):
     db_service = db_api.SERVICE.get_by_name(service)
-    if db_service == None:
+    if db_service is None:
         raise IndexError("Service %s not found" % service)
     obj = db_models.EndpointTemplates()
     obj.region = region
@@ -137,6 +127,8 @@ def list_endpoint_templates():
 
 
 def add_endpoint(tenant, endpoint_template):
+    tenant = db_api.TENANT.get_by_name(name=tenant).id
+
     obj = db_models.Endpoints()
     obj.tenant_id = tenant
     obj.endpoint_template_id = endpoint_template
@@ -145,6 +137,9 @@ def add_endpoint(tenant, endpoint_template):
 
 
 def add_token(token, user, tenant, expires):
+    user = db_api.USER.get_by_name(name=user).id
+    tenant = db_api.TENANT.get_by_name(name=tenant).id
+
     obj = db_models.Token()
     obj.id = token
     obj.user_id = user
@@ -184,14 +179,10 @@ def list_services():
 
 
 def add_credentials(user, type, key, secrete, tenant=None):
+    user = db_api.USER.get_by_name(user).id
 
-    duser = db_api.USER.get_by_name(name=user)
-    if duser:
-        user = duser.id
-
-    dbtenant = db_api.TENANT.get_by_name(tenant)
-    if dbtenant:
-        tenant = dbtenant.id
+    if tenant:
+        tenant = db_api.TENANT.get_by_name(tenant).id
 
     obj = db_models.Token()
     obj.user_id = user
diff --git a/keystone/test/functional/test_auth.py b/keystone/test/functional/test_auth.py
index f2bc6dc6..50cda9b3 100644
--- a/keystone/test/functional/test_auth.py
+++ b/keystone/test/functional/test_auth.py
@@ -216,6 +216,42 @@ class TestServiceAuthentication(common.FunctionalTestCase):
         self.assertEqual(access['token']['tenant']['id'], tenant['id'])
         self.assertEqual(access['token']['tenant']['name'], tenant['name'])
 
+    def test_user_auth_against_nonexistent_tenant(self):
+        # Create an unscoped token
+        unscoped = self.post_token(as_json={
+            'auth': {
+                'passwordCredentials': {
+                    'username': self.user['name'],
+                    'password': self.user['password']}}}).json['access']
+
+        # Invalid tenant id
+        self.post_token(assert_status=401, as_json={
+            'auth': {
+                'token': {'id': unscoped['token']['id']},
+                'tenantId': common.unique_str()}})
+
+        # Invalid tenant name
+        self.post_token(assert_status=401, as_json={
+            'auth': {
+                'token': {'id': unscoped['token']['id']},
+                'tenantName': common.unique_str()}})
+
+        # Invalid tenant id
+        self.post_token(assert_status=401, as_json={
+            'auth': {
+                'passwordCredentials': {
+                    'username': self.user['name'],
+                    'password': self.user['password']},
+                'tenantId': common.unique_str()}})
+
+        # Invalid tenant name
+        self.post_token(assert_status=401, as_json={
+            'auth': {
+                'passwordCredentials': {
+                    'username': self.user['name'],
+                    'password': self.user['password']},
+                'tenantName': common.unique_str()}})
+
     def test_scope_to_tenant_by_bad_request(self):
         # Additonal setUp
         tenant = self.create_tenant().json['tenant']
diff --git a/keystone/test/sampledata.py b/keystone/test/sampledata.py
index becd06a3..4a5e9a0b 100644
--- a/keystone/test/sampledata.py
+++ b/keystone/test/sampledata.py
@@ -2,26 +2,26 @@ import keystone.manage
 
 DEFAULT_FIXTURE = [
 # Tenants
-    ('tenant', 'add', '1234'),
+    ('tenant', 'add', 'customer-x'),
     ('tenant', 'add', 'ANOTHER:TENANT'),
-    ('tenant', 'add', '0000'),
-    ('tenant', 'disable', '0000'),
+    ('tenant', 'add', 'project-y'),
+    ('tenant', 'disable', 'project-y'),
 # Users
-    ('user', 'add', 'joeuser', 'secrete', '1234'),
-    ('user', 'add', 'joeadmin', 'secrete', '1234'),
+    ('user', 'add', 'joeuser', 'secrete', 'customer-x'),
+    ('user', 'add', 'joeadmin', 'secrete', 'customer-x'),
     ('user', 'add', 'admin', 'secrete'),
-    ('user', 'add', 'serviceadmin', 'secrete', '1234'),
-    ('user', 'add', 'disabled', 'secrete', '1234'),
+    ('user', 'add', 'serviceadmin', 'secrete', 'customer-x'),
+    ('user', 'add', 'disabled', 'secrete', 'customer-x'),
     ('user', 'disable', 'disabled'),
 # Roles
     ('role', 'add', 'Admin'),
     ('role', 'add', 'KeystoneServiceAdmin'),
     ('role', 'grant', 'Admin', 'admin'),
     ('role', 'grant', 'KeystoneServiceAdmin', 'serviceadmin'),
-    ('role', 'grant', 'Admin', 'joeadmin', '1234'),
+    ('role', 'grant', 'Admin', 'joeadmin', 'customer-x'),
     ('role', 'grant', 'Admin', 'joeadmin', 'ANOTHER:TENANT'),
     ('role', 'add', 'Member'),
-    ('role', 'grant', 'Member', 'joeuser', '1234'),
+    ('role', 'grant', 'Member', 'joeuser', 'customer-x'),
 # Add Services
     #1 Service Name:exampleservice Type:example type
     ('service', 'add', 'exampleservice',
@@ -87,20 +87,24 @@ DEFAULT_FIXTURE = [
         'http://keystone.publicinternets.com/v2.0',
         'http://127.0.0.1:5001/v2.0', 'http://127.0.0.1:5000/v2.0', '1', '1'),
 # Tokens
-    ('token', 'add', '887665443383838', 'joeuser', '1234', '2012-02-05T00:00'),
-    ('token', 'add', '999888777666', 'admin', '1234', '2015-02-05T00:00'),
-    ('token', 'add', '111222333444', 'serviceadmin', '1234',
+    ('token', 'add', '887665443383838', 'joeuser', 'customer-x',
+        '2012-02-05T00:00'),
+    ('token', 'add', '999888777666', 'admin', 'customer-x',
+        '2015-02-05T00:00'),
+    ('token', 'add', '111222333444', 'serviceadmin', 'customer-x',
+        '2015-02-05T00:00'),
+    ('token', 'add', '000999', 'admin', 'customer-x', '2010-02-05T00:00'),
+    ('token', 'add', '999888777', 'disabled', 'customer-x',
         '2015-02-05T00:00'),
-    ('token', 'add', '000999', 'admin', '1234', '2010-02-05T00:00'),
-    ('token', 'add', '999888777', 'disabled', '1234', '2015-02-05T00:00'),
 # Tenant endpointsGlobal endpoint not added
-    ('endpoint', 'add', '1234', '1'),
-    ('endpoint', 'add', '1234', '2'),
-    ('endpoint', 'add', '1234', '3'),
-    ('endpoint', 'add', '1234', '4'),
-    ('endpoint', 'add', '1234', '5'),
+    ('endpoint', 'add', 'customer-x', '1'),
+    ('endpoint', 'add', 'customer-x', '2'),
+    ('endpoint', 'add', 'customer-x', '3'),
+    ('endpoint', 'add', 'customer-x', '4'),
+    ('endpoint', 'add', 'customer-x', '5'),
 # Add Credentials
-    ('credentials', 'add', 'admin', 'EC2', 'admin:admin', 'admin', '1'),
+    ('credentials', 'add', 'admin', 'EC2', 'admin:admin', 'admin',
+        'customer-x'),
 ]