From e5def7c3ad7751d374986a6585680c339cc8e22d Mon Sep 17 00:00:00 2001 From: wangxiyuan Date: Tue, 23 Oct 2018 16:41:10 +0800 Subject: [PATCH] Remove useless "clean" file The request input format validation now is handled by jsonschema check. The clean file is useless now. Change-Id: I9806722cca8bcd1d4c73618cf1b36107929d37b0 --- keystone/common/clean.py | 87 ------------------- keystone/identity/core.py | 12 +-- keystone/identity/schema.py | 3 +- keystone/resource/core.py | 6 +- keystone/tests/unit/identity/test_backends.py | 59 ------------- keystone/tests/unit/resource/test_backends.py | 36 -------- keystone/tests/unit/test_backend_sql.py | 11 --- keystone/tests/unit/test_validation.py | 4 +- 8 files changed, 8 insertions(+), 210 deletions(-) delete mode 100644 keystone/common/clean.py diff --git a/keystone/common/clean.py b/keystone/common/clean.py deleted file mode 100644 index 38564e0be7..0000000000 --- a/keystone/common/clean.py +++ /dev/null @@ -1,87 +0,0 @@ -# Copyright 2012 OpenStack Foundation -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -import six - -from keystone import exception -from keystone.i18n import _ - - -def check_length(property_name, value, min_length=1, max_length=64): - if len(value) < min_length: - if min_length == 1: - msg = _("%s cannot be empty.") % property_name - else: - msg = (_("%(property_name)s cannot be less than " - "%(min_length)s characters.") % dict( - property_name=property_name, min_length=min_length)) - raise exception.ValidationError(msg) - if len(value) > max_length: - msg = (_("%(property_name)s should not be greater than " - "%(max_length)s characters.") % dict( - property_name=property_name, max_length=max_length)) - - raise exception.ValidationError(msg) - - -def check_type(property_name, value, expected_type, display_expected_type): - if not isinstance(value, expected_type): - msg = (_("%(property_name)s is not a " - "%(display_expected_type)s") % dict( - property_name=property_name, - display_expected_type=display_expected_type)) - raise exception.ValidationError(msg) - - -def check_enabled(property_name, enabled): - # Allow int and it's subclass bool - check_type('%s enabled' % property_name, enabled, int, 'boolean') - return bool(enabled) - - -def check_name(property_name, name, min_length=1, max_length=64): - check_type('%s name' % property_name, name, six.string_types, - 'str or unicode') - name = name.strip() - check_length('%s name' % property_name, name, - min_length=min_length, max_length=max_length) - return name - - -def domain_name(name): - return check_name('Domain', name) - - -def domain_enabled(enabled): - return check_enabled('Domain', enabled) - - -def project_name(name): - return check_name('Project', name) - - -def project_enabled(enabled): - return check_enabled('Project', enabled) - - -def user_name(name): - return check_name('User', name, max_length=255) - - -def user_enabled(enabled): - return check_enabled('User', enabled) - - -def group_name(name): - return check_name('Group', name) diff --git a/keystone/identity/core.py b/keystone/identity/core.py index 5dc4fc28a6..07ef63df90 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -28,7 +28,6 @@ from pycadf import reason from keystone import assignment # TODO(lbragstad): Decouple this dependency from keystone.common import cache -from keystone.common import clean from keystone.common import driver_hints from keystone.common import manager from keystone.common import provider_api @@ -935,9 +934,8 @@ class Manager(manager.Manager): user = user_ref.copy() if 'password' in user: validators.validate_password(user['password']) - user['name'] = clean.user_name(user['name']) + user['name'] = user['name'].strip() user.setdefault('enabled', True) - user['enabled'] = clean.user_enabled(user['enabled']) domain_id = user['domain_id'] PROVIDERS.resource_api.get_domain(domain_id) @@ -1085,9 +1083,7 @@ class Manager(manager.Manager): if 'password' in user: validators.validate_password(user['password']) if 'name' in user: - user['name'] = clean.user_name(user['name']) - if 'enabled' in user: - user['enabled'] = clean.user_enabled(user['enabled']) + user['name'] = user['name'].strip() if 'id' in user: if user_id != user['id']: raise exception.ValidationError(_('Cannot change user ID')) @@ -1172,7 +1168,7 @@ class Manager(manager.Manager): # the underlying driver so that it could conform to rules set down by # that particular driver type. group['id'] = uuid.uuid4().hex - group['name'] = clean.group_name(group['name']) + group['name'] = group['name'].strip() ref = driver.create_group(group['id'], group) notifications.Audit.created(self._GROUP, group['id'], initiator) @@ -1207,7 +1203,7 @@ class Manager(manager.Manager): self._get_domain_driver_and_entity_id(group_id)) group = self._clear_domain_id_if_domain_unaware(driver, group) if 'name' in group: - group['name'] = clean.group_name(group['name']) + group['name'] = group['name'].strip() ref = driver.update_group(entity_id, group) self.get_group.invalidate(self, group_id) notifications.Audit.updated(self._GROUP, group_id, initiator) diff --git a/keystone/identity/schema.py b/keystone/identity/schema.py index 6235b86692..77377e0764 100644 --- a/keystone/identity/schema.py +++ b/keystone/identity/schema.py @@ -15,11 +15,10 @@ from keystone.common.validation import parameter_types from keystone.identity.backends import resource_options as ro -# NOTE(lhcheng): the max length is not applicable since it is specific -# to the SQL backend, LDAP does not have length limitation. _identity_name = { 'type': 'string', 'minLength': 1, + 'maxLength': 255, 'pattern': '[\S]+' } diff --git a/keystone/resource/core.py b/keystone/resource/core.py index bc558040e7..f56abe43df 100644 --- a/keystone/resource/core.py +++ b/keystone/resource/core.py @@ -17,7 +17,6 @@ import six from keystone import assignment from keystone.common import cache -from keystone.common import clean from keystone.common import driver_hints from keystone.common import manager from keystone.common import provider_api @@ -307,8 +306,6 @@ class Manager(manager.Manager): if original_project['is_domain']: domain = self._get_domain_from_project(original_project) self.assert_domain_not_federated(project_id, domain) - if 'enabled' in domain: - domain['enabled'] = clean.domain_enabled(domain['enabled']) url_safe_option = CONF.resource.domain_name_url_safe exception_entity = 'Domain' else: @@ -322,8 +319,7 @@ class Manager(manager.Manager): self._raise_reserved_character_exception(exception_entity, project['name']) elif project_name_changed: - project['name'] = clean.project_name(project['name']) - + project['name'] = project['name'].strip() parent_id = original_project.get('parent_id') if 'parent_id' in project and project.get('parent_id') != parent_id: raise exception.ForbiddenNotSecurity( diff --git a/keystone/tests/unit/identity/test_backends.py b/keystone/tests/unit/identity/test_backends.py index 64a95a1ff2..47e4916e1c 100644 --- a/keystone/tests/unit/identity/test_backends.py +++ b/keystone/tests/unit/identity/test_backends.py @@ -374,20 +374,6 @@ class IdentityTests(object): PROVIDERS.identity_api.delete_user, uuid.uuid4().hex) - def test_create_user_long_name_fails(self): - user = unit.new_user_ref(name='a' * 256, - domain_id=CONF.identity.default_domain_id) - self.assertRaises(exception.ValidationError, - PROVIDERS.identity_api.create_user, - user) - - def test_create_user_blank_name_fails(self): - user = unit.new_user_ref(name='', - domain_id=CONF.identity.default_domain_id) - self.assertRaises(exception.ValidationError, - PROVIDERS.identity_api.create_user, - user) - def test_create_user_missed_password(self): user = unit.new_user_ref(domain_id=CONF.identity.default_domain_id) user = PROVIDERS.identity_api.create_user(user) @@ -421,36 +407,6 @@ class IdentityTests(object): user_id=user['id'], password=None) - def test_create_user_invalid_name_fails(self): - user = unit.new_user_ref(name=None, - domain_id=CONF.identity.default_domain_id) - self.assertRaises(exception.ValidationError, - PROVIDERS.identity_api.create_user, - user) - - user = unit.new_user_ref(name=123, - domain_id=CONF.identity.default_domain_id) - self.assertRaises(exception.ValidationError, - PROVIDERS.identity_api.create_user, - user) - - def test_create_user_invalid_enabled_type_string(self): - user = unit.new_user_ref(domain_id=CONF.identity.default_domain_id, - # invalid string value - enabled='true') - self.assertRaises(exception.ValidationError, - PROVIDERS.identity_api.create_user, - user) - - def test_update_user_long_name_fails(self): - user = unit.new_user_ref(domain_id=CONF.identity.default_domain_id) - user = PROVIDERS.identity_api.create_user(user) - user['name'] = 'a' * 256 - self.assertRaises(exception.ValidationError, - PROVIDERS.identity_api.update_user, - user['id'], - user) - def test_list_users(self): users = PROVIDERS.identity_api.list_users( domain_scope=self._set_domain_scope( @@ -648,21 +604,6 @@ class IdentityTests(object): user_ref = PROVIDERS.identity_api.get_user(user['id']) self.assertTrue(user_ref['enabled']) - # Integers are valid Python's booleans. Explicitly test it. - user['enabled'] = 0 - PROVIDERS.identity_api.update_user(user['id'], user) - user_ref = PROVIDERS.identity_api.get_user(user['id']) - self.assertFalse(user_ref['enabled']) - - # Any integers other than 0 are interpreted as True - user['enabled'] = -42 - PROVIDERS.identity_api.update_user(user['id'], user) - user_ref = PROVIDERS.identity_api.get_user(user['id']) - # NOTE(breton): below, attribute `enabled` is explicitly tested to be - # equal True. assertTrue should not be used, because it converts - # the passed value to bool(). - self.assertIs(True, user_ref['enabled']) - def test_update_user_name(self): user = unit.new_user_ref(domain_id=CONF.identity.default_domain_id) user = PROVIDERS.identity_api.create_user(user) diff --git a/keystone/tests/unit/resource/test_backends.py b/keystone/tests/unit/resource/test_backends.py index 8909c76e15..c792cf1dde 100644 --- a/keystone/tests/unit/resource/test_backends.py +++ b/keystone/tests/unit/resource/test_backends.py @@ -206,42 +206,6 @@ class ResourceTests(object): project['id'], project) - def test_update_project_blank_name_fails(self): - project = unit.new_project_ref( - name='fake1', domain_id=CONF.identity.default_domain_id) - PROVIDERS.resource_api.create_project(project['id'], project) - project['name'] = '' - self.assertRaises(exception.ValidationError, - PROVIDERS.resource_api.update_project, - project['id'], - project) - - def test_update_project_long_name_fails(self): - project = unit.new_project_ref( - name='fake1', domain_id=CONF.identity.default_domain_id) - PROVIDERS.resource_api.create_project(project['id'], project) - project['name'] = 'a' * 65 - self.assertRaises(exception.ValidationError, - PROVIDERS.resource_api.update_project, - project['id'], - project) - - def test_update_project_invalid_name_fails(self): - project = unit.new_project_ref( - name='fake1', domain_id=CONF.identity.default_domain_id) - PROVIDERS.resource_api.create_project(project['id'], project) - project['name'] = None - self.assertRaises(exception.ValidationError, - PROVIDERS.resource_api.update_project, - project['id'], - project) - - project['name'] = 123 - self.assertRaises(exception.ValidationError, - PROVIDERS.resource_api.update_project, - project['id'], - project) - def test_create_project_invalid_domain_id(self): project = unit.new_project_ref(domain_id=uuid.uuid4().hex) self.assertRaises(exception.DomainNotFound, diff --git a/keystone/tests/unit/test_backend_sql.py b/keystone/tests/unit/test_backend_sql.py index 64ddfa2b4b..721c6a7eb3 100644 --- a/keystone/tests/unit/test_backend_sql.py +++ b/keystone/tests/unit/test_backend_sql.py @@ -282,17 +282,6 @@ class SqlIdentity(SqlTests, PROVIDERS.assignment_api.list_projects_for_user, user['id']) - def test_create_null_user_name(self): - user = unit.new_user_ref(name=None, - domain_id=CONF.identity.default_domain_id) - self.assertRaises(exception.ValidationError, - PROVIDERS.identity_api.create_user, - user) - self.assertRaises(exception.UserNotFound, - PROVIDERS.identity_api.get_user_by_name, - user['name'], - CONF.identity.default_domain_id) - def test_create_user_case_sensitivity(self): # user name case sensitivity is down to the fact that it is marked as # an SQL UNIQUE column, which may not be valid for other backends, like diff --git a/keystone/tests/unit/test_validation.py b/keystone/tests/unit/test_validation.py index 800d0ab7d2..6f9a4dc02f 100644 --- a/keystone/tests/unit/test_validation.py +++ b/keystone/tests/unit/test_validation.py @@ -107,7 +107,7 @@ _VALID_FILTERS = [{'interface': 'admin'}, _INVALID_FILTERS = ['some string', 1, 0, True, False] -_INVALID_NAMES = [True, 24, ' ', ''] +_INVALID_NAMES = [True, 24, ' ', '', 'a' * 256, None] class CommonValidationTestCase(unit.BaseTestCase): @@ -415,7 +415,7 @@ class ProjectValidationTestCase(unit.BaseTestCase): def test_validate_project_create_fails_with_invalid_name(self): """Exception when validating a create request with invalid `name`.""" - for invalid_name in _INVALID_NAMES: + for invalid_name in _INVALID_NAMES + ['a' * 65]: request_to_validate = {'name': invalid_name} self.assertRaises(exception.SchemaValidationError, self.create_project_validator.validate,