diff --git a/murano/api/v1/actions.py b/murano/api/v1/actions.py index 96b1e9ed9..f64c61616 100644 --- a/murano/api/v1/actions.py +++ b/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 ' - '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 '.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 '.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) diff --git a/murano/api/v1/deployments.py b/murano/api/v1/deployments.py index 396a69f60..bd7016a03 100644 --- a/murano/api/v1/deployments.py +++ b/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) diff --git a/murano/api/v1/environments.py b/murano/api/v1/environments.py index e7d344fc1..df8308612 100644 --- a/murano/api/v1/environments.py +++ b/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 '.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 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 '.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 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: diff --git a/murano/api/v1/sessions.py b/murano/api/v1/sessions.py index 7d858250f..2c42a19b2 100644 --- a/murano/api/v1/sessions.py +++ b/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 ' - ' 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 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 '.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) diff --git a/murano/tests/functional/api/v1/test_envs.py b/murano/tests/functional/api/v1/test_envs.py index ce0032cbc..7f8ad7a4f 100644 --- a/murano/tests/functional/api/v1/test_envs.py +++ b/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']) diff --git a/murano/tests/functional/api/v1/test_sessions.py b/murano/tests/functional/api/v1/test_sessions.py index 41d9eb352..2c5347de7 100644 --- a/murano/tests/functional/api/v1/test_sessions.py +++ b/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']) diff --git a/murano/tests/unit/api/v1/test_environments.py b/murano/tests/unit/api/v1/test_environments.py index 5760b3044..6c5a87702 100644 --- a/murano/tests/unit/api/v1/test_environments.py +++ b/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.""" - self._set_policy_rules( - {'show_environment': '@'} - ) - self.expect_policy_check('show_environment', - {'environment_id': 'no-such-id'}) + """Check that a missing environment results in an HTTPNotFound. + 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) diff --git a/murano/tests/unit/api/v1/test_sessions.py b/murano/tests/unit/api/v1/test_sessions.py index 7e40dd17c..b5f6011ba 100644 --- a/murano/tests/unit/api/v1/test_sessions.py +++ b/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! - self.assertEqual(response.status_code, 401) + # Should be forbidden! + self.assertEqual(response.status_code, 403) diff --git a/murano/utils.py b/murano/utils.py index a320ca54a..15ffd9113 100644 --- a/murano/utils.py +++ b/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):