From a55b8427a8c328a47277cedcd287e860424221ca Mon Sep 17 00:00:00 2001 From: Brian Tully Date: Thu, 25 Sep 2014 16:39:54 -0400 Subject: [PATCH] Missing validation for environment name in API Add environment name validation to API so that when names are entered by user via the murano client CLI they get validated in the same way they do via the murano dashboard UI, i.e., name must contain only alphanumeric or "_-." characters, must start with alpha. If not valid, raise an exception with a 400 error. Change-Id: Ib125e017acbdd6b6881ec705763c17170b80e44b Closes-Bug: 1373045 --- murano/api/v1/environments.py | 38 ++++++++++++++----- murano/tests/unit/api/v1/test_environments.py | 20 ++++++++-- 2 files changed, 45 insertions(+), 13 deletions(-) diff --git a/murano/api/v1/environments.py b/murano/api/v1/environments.py index d7abaade..e5da2291 100644 --- a/murano/api/v1/environments.py +++ b/murano/api/v1/environments.py @@ -12,6 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. +import re + from oslo.db import exception as db_exc from sqlalchemy import desc from webob import exc @@ -33,6 +35,8 @@ LOG = logging.getLogger(__name__) API_NAME = 'Environments' +VALID_NAME_REGEX = re.compile('^[a-zA-Z]+[\w-]*$') + class Controller(object): @request_statistics.stats_count(API_NAME, 'Index') @@ -51,15 +55,22 @@ class Controller(object): def create(self, request, body): LOG.debug('Environments:Create '.format(body)) policy.check('create_environment', request.context) - - try: - environment = envs.EnvironmentServices.create( - body.copy(), - request.context.tenant) - except db_exc.DBDuplicateEntry: - msg = _('Environment with specified name already exists') + LOG.debug('ENV NAME: {0}>'.format(body['name'])) + if VALID_NAME_REGEX.match(str(body['name'])): + try: + environment = envs.EnvironmentServices.create( + body.copy(), + request.context.tenant) + except db_exc.DBDuplicateEntry: + msg = _('Environment with specified name already exists') + LOG.exception(msg) + raise exc.HTTPConflict(msg) + else: + msg = _('Environment name must contain only alphanumeric ' + 'or "_-." characters, must start with alpha') LOG.exception(msg) - raise exc.HTTPConflict(msg) + raise exc.HTTPClientError(msg) + return environment.to_dict() @request_statistics.stats_count(API_NAME, 'Show') @@ -114,8 +125,15 @@ class Controller(object): 'this tenant resources.')) raise exc.HTTPUnauthorized - environment.update(body) - environment.save(session) + LOG.debug('ENV NAME: {0}>'.format(body['name'])) + if VALID_NAME_REGEX.match(str(body['name'])): + environment.update(body) + environment.save(session) + else: + msg = _('Environment name must contain only alphanumeric ' + 'or "_-." characters, must start with alpha') + LOG.exception(msg) + raise exc.HTTPClientError(msg) return environment.to_dict() diff --git a/murano/tests/unit/api/v1/test_environments.py b/murano/tests/unit/api/v1/test_environments.py index d7cd27eb..5caf6a15 100644 --- a/murano/tests/unit/api/v1/test_environments.py +++ b/murano/tests/unit/api/v1/test_environments.py @@ -89,6 +89,20 @@ class TestEnvironmentApi(tb.ControllerTest, tb.MuranoApiTestCase): self.assertEqual(expected, json.loads(result.body)) self.assertEqual(3, mock_uuid.call_count) + def test_illegal_environment_name_create(self): + """Check that an illegal env name results in an HTTPClientError.""" + self._set_policy_rules( + {'list_environments': '@', + 'create_environment': '@', + 'show_environment': '@'} + ) + self.expect_policy_check('create_environment') + + body = {'name': 'my+#env'} + req = self._post('/environments', json.dumps(body)) + result = req.get_response(self.api) + self.assertEqual(400, result.status_code) + def test_missing_environment(self): """Check that a missing environment results in an HTTPNotFound.""" self._set_policy_rules( @@ -125,7 +139,7 @@ class TestEnvironmentApi(tb.ControllerTest, tb.MuranoApiTestCase): 'Objects': { '?': {'id': '12345'} }, - 'Attributes': {} + 'Attributes': [] } ) e = models.Environment(**expected) @@ -137,11 +151,11 @@ class TestEnvironmentApi(tb.ControllerTest, tb.MuranoApiTestCase): del expected['description'] expected['services'] = [] expected['status'] = 'ready' - expected['name'] = 'renamed env' + expected['name'] = 'renamed_env' expected['updated'] = fake_now body = { - 'name': 'renamed env' + 'name': 'renamed_env' } req = self._put('/environments/12345', json.dumps(body)) result = req.get_response(self.api)