Merge "Remove duplicated check env code"

murano/api/v1/actions.py

@@ -17,39 +17,27 @@ from webob import exc
 
 from murano.common import policy
 from murano.common import wsgi
-from murano.db import models
 from murano.db.services import environments as envs
 from murano.db.services import sessions
 from murano.db import session as db_session
 from murano.common.i18n import _LI, _LE, _
 from murano.services import actions
 from murano.services import states
+from murano.utils import verify_env
 
 
 LOG = logging.getLogger(__name__)
 
 
 class Controller(object):
-    def _validate_environment(self, unit, request, environment_id):
-        environment = unit.query(models.Environment).get(environment_id)
-
-        if environment is None:
-            LOG.info(_LI('Environment <EnvId {0}> '
-                         'is not found').format(environment_id))
-            raise exc.HTTPNotFound
-
-        if environment.tenant_id != request.context.tenant:
-            LOG.info(_LI('User is not authorized to access '
-                         'this tenant resources.'))
-            raise exc.HTTPUnauthorized
 
+    @verify_env
     def execute(self, request, environment_id, action_id, body):
         policy.check("execute_action", request.context, {})
 
         LOG.debug('Action:Execute <ActionId: {0}>'.format(action_id))
 
         unit = db_session.get_session()
-        self._validate_environment(unit, request, environment_id)
 
         # no new session can be opened if environment has deploying status
         env_status = envs.EnvironmentServices.get_status(environment_id)
@@ -72,13 +60,13 @@ class Controller(object):
             action_id, session, unit, request.context.auth_token, body or {})
         return {'task_id': task_id}
 
+    @verify_env
     def get_result(self, request, environment_id, task_id):
         policy.check("execute_action", request.context, {})
 
         LOG.debug('Action:GetResult <TaskId: {0}>'.format(task_id))
 
         unit = db_session.get_session()
-        self._validate_environment(unit, request, environment_id)
         result = actions.ActionServices.get_result(environment_id, task_id,
                                                    unit)
 

murano/api/v1/deployments.py

@@ -24,6 +24,7 @@ from murano.common import utils
 from murano.common import wsgi
 from murano.db import models
 from murano.db import session as db_session
+from murano.utils import verify_env
 
 LOG = logging.getLogger(__name__)
 
@@ -32,12 +33,12 @@ API_NAME = 'Deployments'
 
 class Controller(object):
     @request_statistics.stats_count(API_NAME, 'Index')
+    @verify_env
     def index(self, request, environment_id):
         target = {"environment_id": environment_id}
         policy.check("list_deployments", request.context, target)
 
         unit = db_session.get_session()
-        verify_and_get_env(unit, environment_id, request)
         query = unit.query(models.Task) \
             .filter_by(environment_id=environment_id) \
             .order_by(desc(models.Task.created))
@@ -76,20 +77,6 @@ class Controller(object):
         return {'reports': [status.to_dict() for status in result]}
 
 
-def verify_and_get_env(db_session, environment_id, request):
-    environment = db_session.query(models.Environment).get(environment_id)
-    if not environment:
-        LOG.info(_LI(
-            'Environment with id {0} not found').format(environment_id))
-        raise exc.HTTPNotFound
-
-    if environment.tenant_id != request.context.tenant:
-        LOG.info(_LI(
-            'User is not authorized to access this tenant resources.'))
-        raise exc.HTTPUnauthorized
-    return environment
-
-
 def _patch_description(description):
     description['services'] = description.pop('applications', [])
     return token_sanitizer.TokenSanitizer().sanitize(description)

murano/api/v1/environments.py

@@ -21,6 +21,7 @@ from webob import exc
 
 from murano.api.v1 import request_statistics
 from murano.api.v1 import sessions
+from murano.common.i18n import _
 from murano.common import policy
 from murano.common import utils
 from murano.common import wsgi
@@ -28,8 +29,8 @@ from murano.db import models
 from murano.db.services import core_services
 from murano.db.services import environments as envs
 from murano.db import session as db_session
-from murano.common.i18n import _, _LI
 from murano.utils import check_env
+from murano.utils import verify_env
 
 LOG = logging.getLogger(__name__)
 
@@ -84,6 +85,7 @@ class Controller(object):
         return environment.to_dict()
 
     @request_statistics.stats_count(API_NAME, 'Show')
+    @verify_env
     def show(self, request, environment_id):
         LOG.debug('Environments:Show <Id: {0}>'.format(environment_id))
         target = {"environment_id": environment_id}
@@ -91,17 +93,6 @@ class Controller(object):
 
         session = db_session.get_session()
         environment = session.query(models.Environment).get(environment_id)
-
-        if environment is None:
-            LOG.info(_LI('Environment <EnvId {0}> is not found').format(
-                environment_id))
-            raise exc.HTTPNotFound
-
-        if environment.tenant_id != request.context.tenant:
-            LOG.info(_LI('User is not authorized to access '
-                         'this tenant resources.'))
-            raise exc.HTTPUnauthorized
-
         env = environment.to_dict()
         env['status'] = envs.EnvironmentServices.get_status(env['id'])
 
@@ -116,6 +107,7 @@ class Controller(object):
         return env
 
     @request_statistics.stats_count(API_NAME, 'Update')
+    @verify_env
     def update(self, request, environment_id, body):
         LOG.debug('Environments:Update <Id: {0}, '
                   'Body: {1}>'.format(environment_id, body))
@@ -124,17 +116,6 @@ class Controller(object):
 
         session = db_session.get_session()
         environment = session.query(models.Environment).get(environment_id)
-
-        if environment is None:
-            LOG.info(_LI('Environment <EnvId {0}> not '
-                         'found').format(environment_id))
-            raise exc.HTTPNotFound
-
-        if environment.tenant_id != request.context.tenant:
-            LOG.info(_LI('User is not authorized to access '
-                         'this tenant resources.'))
-            raise exc.HTTPUnauthorized
-
         LOG.debug('ENV NAME: {0}>'.format(body['name']))
         if VALID_NAME_REGEX.match(str(body['name'])):
             try:

murano/api/v1/sessions.py

@@ -23,6 +23,7 @@ from murano.db.services import environments as envs
 from murano.db.services import sessions
 from murano.db import session as db_session
 from murano.services import states
+from murano.utils import check_env
 
 LOG = logging.getLogger(__name__)
 API_NAME = 'Sessions'
@@ -30,22 +31,6 @@ API_NAME = 'Sessions'
 
 class Controller(object):
 
-    def _check_environment(self, request, environment_id):
-        unit = db_session.get_session()
-        environment = unit.query(models.Environment).get(environment_id)
-
-        if environment is None:
-            msg = _('Environment <EnvId {0}>'
-                    ' is not found').format(environment_id)
-            LOG.error(msg)
-            raise exc.HTTPNotFound(explanation=msg)
-
-        if environment.tenant_id != request.context.tenant:
-            msg = _('User is not authorized to access '
-                    'this tenant resources.')
-            LOG.error(msg)
-            raise exc.HTTPUnauthorized(explanation=msg)
-
     def _check_session(self, request, environment_id, session, session_id):
         if session is None:
             msg = _('Session <SessionId {0}> is not found').format(session_id)
@@ -58,13 +43,13 @@ class Controller(object):
             LOG.error(msg)
             raise exc.HTTPNotFound(explanation=msg)
 
-        self._check_environment(request, environment_id)
+        check_env(request, environment_id)
 
     @request_statistics.stats_count(API_NAME, 'Create')
     def configure(self, request, environment_id):
         LOG.debug('Session:Configure <EnvId: {0}>'.format(environment_id))
 
-        self._check_environment(request, environment_id)
+        check_env(request, environment_id)
 
         # no new session can be opened if environment has deploying status
         env_status = envs.EnvironmentServices.get_status(environment_id)

murano/tests/functional/api/v1/test_envs.py

@@ -108,19 +108,19 @@ class TestEnvironmentsTenantIsolation(base.NegativeTestCase):
     def test_get_environment_from_another_tenant(self):
         env = self.create_environment('test')
 
-        self.assertRaises(exceptions.Unauthorized,
+        self.assertRaises(exceptions.Forbidden,
                           self.alt_client.get_environment, env['id'])
 
     @attr(type='negative')
     def test_update_environment_from_another_tenant(self):
         env = self.create_environment('test')
 
-        self.assertRaises(exceptions.Unauthorized,
+        self.assertRaises(exceptions.Forbidden,
                           self.alt_client.update_environment, env['id'])
 
     @attr(type='negative')
     def test_delete_environment_from_another_tenant(self):
         env = self.create_environment('test')
 
-        self.assertRaises(exceptions.Unauthorized,
+        self.assertRaises(exceptions.Forbidden,
                           self.alt_client.delete_environment, env['id'])

murano/tests/functional/api/v1/test_sessions.py

@@ -112,7 +112,7 @@ class TestSessionsTenantIsolation(base.NegativeTestCase):
     def test_create_session_in_env_from_another_tenant(self):
         env = self.create_environment('test')
 
-        self.assertRaises(exceptions.Unauthorized,
+        self.assertRaises(exceptions.Forbidden,
                           self.alt_client.create_session, env['id'])
 
     @attr(type='negative')
@@ -120,7 +120,7 @@ class TestSessionsTenantIsolation(base.NegativeTestCase):
         env = self.create_environment('test')
         sess = self.client.create_session(env['id'])[1]
 
-        self.assertRaises(exceptions.Unauthorized,
+        self.assertRaises(exceptions.Forbidden,
                           self.alt_client.delete_session, env['id'],
                           sess['id'])
 
@@ -129,7 +129,7 @@ class TestSessionsTenantIsolation(base.NegativeTestCase):
         env = self.create_environment('test')
         sess = self.client.create_session(env['id'])[1]
 
-        self.assertRaises(exceptions.Unauthorized,
+        self.assertRaises(exceptions.Forbidden,
                           self.alt_client.get_session, env['id'],
                           sess['id'])
 
@@ -139,6 +139,6 @@ class TestSessionsTenantIsolation(base.NegativeTestCase):
         env = self.create_environment('test')
         sess = self.client.create_session(env['id'])[1]
 
-        self.assertRaises(exceptions.Unauthorized,
+        self.assertRaises(exceptions.Forbidden,
                           self.alt_client.deploy_session, env['id'],
                           sess['id'])

murano/tests/unit/api/v1/test_environments.py

@@ -176,13 +176,11 @@ class TestEnvironmentApi(tb.ControllerTest, tb.MuranoApiTestCase):
                       result_msg)
 
     def test_missing_environment(self):
-        """Check that a missing environment results in an HTTPNotFound."""
+        """Check that a missing environment results in an HTTPNotFound.
-        self._set_policy_rules(
-            {'show_environment': '@'}
-        )
-        self.expect_policy_check('show_environment',
-                                 {'environment_id': 'no-such-id'})
 
+        Environment check will be made in the decorator and raises,
+        no need to check policy in this testcase.
+        """
         req = self._get('/environments/no-such-id')
         result = req.get_response(self.api)
         self.assertEqual(404, result.status_code)

murano/tests/unit/api/v1/test_sessions.py

@@ -84,5 +84,5 @@ class TestSessionsApi(tb.ControllerTest, tb.MuranoApiTestCase):
         )
         response = request.get_response(self.api)
 
-        # Should be unauthorized!
+        # Should be forbidden!
-        self.assertEqual(response.status_code, 401)
+        self.assertEqual(response.status_code, 403)

murano/utils.py

@@ -41,6 +41,7 @@ def check_env(request, environment_id):
                     ' these tenant resources')
             LOG.warning(msg)
             raise exc.HTTPForbidden(explanation=msg)
+    return environment
 
 
 def verify_env(func):