Merge "Remove explicit access to is_admin in context"

climate/api/context.py

@@ -17,7 +17,6 @@ import json
 
 from climate import context
 from climate import exceptions
-from climate import policy
 
 
 def ctx_from_headers(headers):
@@ -28,7 +27,7 @@ def ctx_from_headers(headers):
     except TypeError:
         raise exceptions.WrongFormat()
 
-    ctx = context.ClimateContext(
+    return context.ClimateContext(
         user_id=headers['X-User-Id'],
         tenant_id=headers['X-Tenant-Id'],
         auth_token=headers['X-Auth-Token'],
@@ -37,7 +36,3 @@ def ctx_from_headers(headers):
         tenant_name=headers['X-Tenant-Name'],
         roles=map(unicode.strip, headers['X-Roles'].split(',')),
     )
-    target = {'tenant_id': ctx.tenant_id, 'user_id': ctx.user_id}
-    if policy.enforce(ctx, "admin", target, do_raise=False):
-        ctx.is_admin = True
-    return ctx

climate/api/service.py

@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from climate import context
 from climate import exceptions
 from climate.manager import rpcapi as manager_rpcapi
 from climate.openstack.common import log as logging
@@ -32,7 +33,12 @@ class API(object):
     @policy.authorize('leases', 'get')
     def get_leases(self):
         """List all existing leases."""
-        return self.manager_rpcapi.list_leases()
+        ctx = context.current()
+        if policy.enforce(ctx, 'admin', {}, do_raise=False):
+            tenant_id = None
+        else:
+            tenant_id = ctx.tenant_id
+        return self.manager_rpcapi.list_leases(tenant_id=tenant_id)
 
     @policy.authorize('leases', 'create')
     def create_lease(self, data):

climate/context.py

@@ -96,14 +96,3 @@ def current():
 
 def elevated():
     return ClimateContext.elevated()
-
-
-def is_user_context(context):
-    """Indicates if the request context is a normal user"""
-    if not context:
-        return False
-    if context.is_admin:
-        return False
-    if not context.user_id or not context.tenant_id:
-        return False
-    return True

climate/db/api.py

@@ -171,9 +171,9 @@ def lease_get(lease_id):
 
 
 @to_dict
-def lease_list():
+def lease_list(tenant_id=None):
     """Return a list of all existing leases."""
-    return IMPL.lease_list()
+    return IMPL.lease_list(tenant_id)
 
 
 def lease_destroy(lease_id):

climate/db/sqlalchemy/api.py

@@ -21,7 +21,6 @@ import sqlalchemy as sa
 from sqlalchemy.sql.expression import asc
 from sqlalchemy.sql.expression import desc
 
-from climate import context
 from climate.db.sqlalchemy import models
 from climate.openstack.common.db import exception as db_exc
 from climate.openstack.common.db.sqlalchemy import session as db_session
@@ -40,7 +39,7 @@ def get_backend():
     return sys.modules[__name__]
 
 
-def model_query(model, session=None, project_only=False):
+def model_query(model, session=None):
     """Query helper.
 
     :param model: base model to query
@@ -49,23 +48,7 @@ def model_query(model, session=None, project_only=False):
     """
     session = session or get_session()
 
-    query = session.query(model)
+    return session.query(model)
-
-    if project_only and 'tenant_id' not in model.__table__.columns:
-        # model not having tenant_id as attribute
-        LOG.debug("Query not filtered per tenant")
-        return query
-
-    if project_only:
-        try:
-            ctx = context.current()
-        except RuntimeError:
-            # Context is not available
-            return query
-        if context.is_user_context(ctx):
-            query = query.filter_by(tenant_id=ctx.tenant_id)
-
-    return query
 
 
 def setup_db():
@@ -224,8 +207,11 @@ def lease_get_all_by_user(user_id):
     raise NotImplementedError
 
 
-def lease_list():
+def lease_list(tenant_id=None):
-    return model_query(models.Lease, get_session(), project_only=True).all()
+    query = model_query(models.Lease, get_session())
+    if tenant_id is not None:
+        query = query.filter_by(tenant_id=tenant_id)
+    return query.all()
 
 
 def lease_create(values):

climate/manager/rpcapi.py

@@ -30,9 +30,9 @@ class ManagerRPCAPI(service.RPCClient):
         """Get detailed info about some lease."""
         return self.call('get_lease', lease_id=lease_id)
 
-    def list_leases(self):
+    def list_leases(self, tenant_id=None):
         """List all leases."""
-        return self.call('list_leases')
+        return self.call('list_leases', tenant_id=tenant_id)
 
     def create_lease(self, lease_values):
         """Create lease with specified parameters."""

climate/manager/service.py

@@ -150,8 +150,8 @@ class ManagerService(service_utils.RPCServer):
     def get_lease(self, lease_id):
         return db_api.lease_get(lease_id)
 
-    def list_leases(self):
+    def list_leases(self, tenant_id=None):
-        return db_api.lease_list()
+        return db_api.lease_list(tenant_id)
 
     def create_lease(self, lease_values):
         """Create a lease with reservations.

climate/tests/api/test_context.py

@@ -18,7 +18,6 @@ import json
 from climate.api import context as api_context
 from climate import context
 from climate import exceptions
-from climate import policy
 from climate import tests
 
 
@@ -48,13 +47,6 @@ class ContextTestCase(tests.TestCase):
                                              tenant_id=u'1',
                                              user_name=u'user_name')
 
-    def test_ctx_from_headers_with_admin(self):
-        catalog = json.dumps({'nova': 'catalog'})
-        self.fake_headers[u'X-Service-Catalog'] = catalog
-        self.patch(policy, 'enforce').return_value = True
-        ctx = api_context.ctx_from_headers(self.fake_headers)
-        self.assertEqual(True, ctx.is_admin)
-
     def test_ctx_from_headers_no_catalog(self):
         self.assertRaises(
             exceptions.ServiceCatalogNotFound,

climate/tests/db/sqlalchemy/test_sqlalchemy_api.py

@@ -16,7 +16,6 @@
 import datetime
 import operator
 
-from climate import context
 from climate.db.sqlalchemy import api as db_api
 from climate.db.sqlalchemy import models
 from climate.openstack.common import uuidutils
@@ -185,27 +184,10 @@ class SQLAlchemyDBApiTestCase(tests.DBTestCase):
     def setUp(self):
         super(SQLAlchemyDBApiTestCase, self).setUp()
 
-        self.set_context(context.ClimateContext(user_id='fake',
-                                                tenant_id='fake'))
-
     def test_model_query(self):
-        db_api.lease_create(_get_fake_virt_lease_values())
+        lease = db_api.lease_create(_get_fake_virt_lease_values())
-        query = db_api.model_query(models.Lease, project_only=None)
+        query = db_api.model_query(models.Lease)
-        self.assertEqual(1, len(query.all()))
+        self.assertEqual([lease.to_dict()], [l.to_dict() for l in query.all()])
-        query = db_api.model_query(models.Lease, project_only=True)
-        self.assertEqual(1, len(query.all()))
-        self.set_context(context.ClimateContext(user_id='fake',
-                                                tenant_id='wrong'))
-        query = db_api.model_query(models.Lease, project_only=True)
-        self.assertEqual(0, len(query.all()))
-
-    def test_model_query_as_admin(self):
-        db_api.lease_create(_get_fake_virt_lease_values())
-        self.set_context(context.ClimateContext(user_id='fake',
-                                                tenant_id='wrong',
-                                                is_admin=True))
-        query = db_api.model_query(models.Lease, project_only=True)
-        self.assertEqual(1, len(query.all()))
 
     def test_create_virt_lease(self):
         """Create a virtual lease and verify that all tables have been

climate/tests/manager/test_rpcapi.py

@@ -35,8 +35,8 @@ class RPCAPITestCase(tests.TestCase):
         self.call.assert_called_once_with('get_lease', lease_id=1)
 
     def test_list_leases(self):
-        self.manager.list_leases()
+        self.manager.list_leases('fake')
-        self.call.assert_called_once_with('list_leases')
+        self.call.assert_called_once_with('list_leases', tenant_id='fake')
 
     def test_create_lease(self):
         self.manager.create_lease(self.fake_values)

climate/tests/test_context.py

@@ -102,14 +102,3 @@ class TestClimateContext(tests.TestCase):
             self.assertEqual(ctx.user_id, "user")
             self.assertEqual(ctx.tenant_id, "tenant")
             self.assertEqual(ctx.is_admin, True)
-
-    def test_is_user_context(self):
-        ctx = context.ClimateContext(user_id="user", tenant_id="tenant")
-        self.assertEqual(True, context.is_user_context(ctx))
-        ctx = ctx.elevated()
-        self.assertEqual(False, context.is_user_context(ctx))
-
-    def test_is_user_context_with_empty_context(self):
-        ctx = context.ClimateContext()
-        self.assertEqual(False, context.is_user_context(ctx))
-        self.assertEqual(False, context.is_user_context(None))