diff --git a/keystone/common/ldap/core.py b/keystone/common/ldap/core.py index 2dfe13b260..86602b528a 100644 --- a/keystone/common/ldap/core.py +++ b/keystone/common/ldap/core.py @@ -1672,6 +1672,92 @@ class BaseLdap(object): 'entries': not_deleted_nodes[:3], 'dots': '...' if len(not_deleted_nodes) > 3 else ''}) + def filter_query(self, hints, query=None): + """Applies filtering to a query. + + :param hints: contains the list of filters, which may be None, + indicating that there are no filters to be applied. + If it's not None, then any filters satisfied here will be + removed so that the caller will know if any filters + remain to be applied. + :param query: LDAP query into which to include filters + + :returns query: LDAP query, updated with any filters satisfied + + """ + def build_filter(filter_, hints): + """Build a filter for the query. + + :param filter_: the dict that describes this filter + :param hints: contains the list of filters yet to be satisfied. + Any filters satisfied here will be removed so that + the caller will know if any filters remain. + + :returns query: LDAP query term to be added + + """ + ldap_attr = self.attribute_mapping[filter_['name']] + val_esc = ldap.filter.escape_filter_chars(filter_['value']) + + if filter_['case_sensitive']: + # NOTE(henry-nash): Although dependent on the schema being + # used, most LDAP attributes are configured with case + # insensitive matching rules, so we'll leave this to the + # controller to filter. + return + + if filter_['name'] == 'enabled': + # NOTE(henry-nash): Due to the different options for storing + # the enabled attribute (e,g, emulated or not), for now we + # don't try and filter this at the driver level - we simply + # leave the filter to be handled by the controller. It seems + # unlikley that this will cause a signifcant performance + # issue. + return + + # TODO(henry-nash): Currently there are no booleans (other than + # 'enabled' that is handled above) on which you can filter. If + # there were, we would need to add special handling here to + # convert the booleans values to 'TRUE' and 'FALSE'. To do that + # we would also need to know which filter keys were actually + # booleans (this is related to bug #1411478). + + if filter_['comparator'] == 'equals': + query_term = (u'(%(attr)s=%(val)s)' + % {'attr': ldap_attr, 'val': val_esc}) + elif filter_['comparator'] == 'contains': + query_term = (u'(%(attr)s=*%(val)s*)' + % {'attr': ldap_attr, 'val': val_esc}) + elif filter_['comparator'] == 'startswith': + query_term = (u'(%(attr)s=%(val)s*)' + % {'attr': ldap_attr, 'val': val_esc}) + elif filter_['comparator'] == 'endswith': + query_term = (u'(%(attr)s=*%(val)s)' + % {'attr': ldap_attr, 'val': val_esc}) + else: + # It's a filter we don't understand, so let the caller + # work out if they need to do something with it. + return + + hints.filters.remove(filter_) + return query_term + + if hints is None: + return query + + filter_list = [] + + for filter_ in hints.filters: + if filter_['name'] not in self.attribute_mapping: + continue + new_filter = build_filter(filter_, hints) + if new_filter is not None: + filter_list.append(new_filter) + + if filter_list: + query = u'(&%s%s)' % (query, ''.join(filter_list)) + return query + class EnabledEmuMixIn(BaseLdap): """Emulates boolean 'enabled' attribute if turned on. diff --git a/keystone/identity/backends/ldap.py b/keystone/identity/backends/ldap.py index 6abc49f11a..1082880785 100644 --- a/keystone/identity/backends/ldap.py +++ b/keystone/identity/backends/ldap.py @@ -79,7 +79,7 @@ class Identity(identity.Driver): return self.user.get_filtered(user_id) def list_users(self, hints): - return self.user.get_all_filtered() + return self.user.get_all_filtered(hints) def get_user_by_name(self, user_name, domain_id): # domain_id will already have been handled in the Manager layer, @@ -158,10 +158,10 @@ class Identity(identity.Driver): def list_groups_for_user(self, user_id, hints): user_ref = self._get_user(user_id) user_dn = user_ref['dn'] - return self.group.list_user_groups_filtered(user_dn) + return self.group.list_user_groups_filtered(user_dn, hints) def list_groups(self, hints): - return self.group.get_all_filtered() + return self.group.get_all_filtered(hints) def list_users_in_group(self, group_id, hints): users = [] @@ -264,8 +264,9 @@ class UserApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap): user = self.get(user_id) return self.filter_attributes(user) - def get_all_filtered(self): - return [self.filter_attributes(user) for user in self.get_all()] + def get_all_filtered(self, hints): + query = self.filter_query(hints) + return [self.filter_attributes(user) for user in self.get_all(query)] def filter_attributes(self, user): return identity.filter_user(common_ldap.filter_entity(user)) @@ -357,7 +358,7 @@ class GroupApi(common_ldap.BaseLdap): self.ldap_filter or '') return self.get_all(query) - def list_user_groups_filtered(self, user_dn): + def list_user_groups_filtered(self, user_dn, hints): """Return a filtered list of groups for which the user is a member.""" user_dn_esc = ldap.filter.escape_filter_chars(user_dn) @@ -365,7 +366,7 @@ class GroupApi(common_ldap.BaseLdap): self.member_attribute, user_dn_esc, self.ldap_filter or '') - return self.get_all_filtered(query) + return self.get_all_filtered(hints, query) def list_group_users(self, group_id): """Return a list of user dns which are members of a group.""" @@ -395,6 +396,7 @@ class GroupApi(common_ldap.BaseLdap): group = self.get_by_name(group_name) return common_ldap.filter_entity(group) - def get_all_filtered(self, query=None): + def get_all_filtered(self, hints, query=None): + query = self.filter_query(hints, query) return [common_ldap.filter_entity(group) for group in self.get_all(query)] diff --git a/keystone/resource/backends/ldap.py b/keystone/resource/backends/ldap.py index 54797fd080..9a84cc1661 100644 --- a/keystone/resource/backends/ldap.py +++ b/keystone/resource/backends/ldap.py @@ -79,7 +79,7 @@ class Resource(resource.Driver): def list_projects(self, hints): return self._set_default_attributes( - self.project.get_all()) + self.project.get_all_filtered(hints)) def list_projects_in_domain(self, domain_id): # We don't support multiple domains within this driver, so ignore @@ -190,3 +190,7 @@ class ProjectApi(common_ldap.ProjectLdapStructureMixin, def update(self, project_id, values): old_obj = self.get(project_id) return super(ProjectApi, self).update(project_id, values, old_obj) + + def get_all_filtered(self, hints): + query = self.filter_query(hints) + return super(ProjectApi, self).get_all(query) diff --git a/keystone/tests/unit/fakeldap.py b/keystone/tests/unit/fakeldap.py index cf881c351a..7812422d41 100644 --- a/keystone/tests/unit/fakeldap.py +++ b/keystone/tests/unit/fakeldap.py @@ -130,9 +130,36 @@ def _paren_groups(source): def _match(key, value, attrs): """Match a given key and value against an attribute list.""" + + def match_with_wildcards(norm_val, val_list): + # Case insensitive checking with wildcards + if norm_val.startswith('*'): + if norm_val.endswith('*'): + # Is the string anywhere in the target? + for x in val_list: + if norm_val[1:-1] in x: + return True + else: + # Is the string at the end of the target? + for x in val_list: + if (norm_val[1:] == + x[len(x) - len(norm_val) + 1:]): + return True + elif norm_val.endswith('*'): + # Is the string at the start of the target? + for x in val_list: + if norm_val[:-1] == x[:len(norm_val) - 1]: + return True + else: + # Is the string an exact match? + for x in val_list: + if check_value == x: + return True + return False + if key not in attrs: return False - # This is a wild card search. Implemented as all or nothing for now. + # This is a pure wild card search, so the answer must be yes! if value == '*': return True if key == 'serviceId': @@ -145,7 +172,7 @@ def _match(key, value, attrs): check_value = _internal_attr(key, value)[0].lower() norm_values = list( _internal_attr(key, x)[0].lower() for x in attrs[key]) - return check_value in norm_values + return match_with_wildcards(check_value, norm_values) # it is an objectclass check, so check subclasses values = _subs(value) for v in values: diff --git a/keystone/tests/unit/test_backend.py b/keystone/tests/unit/test_backend.py index 3f1b7b265a..5c8fc3e59a 100644 --- a/keystone/tests/unit/test_backend.py +++ b/keystone/tests/unit/test_backend.py @@ -5447,6 +5447,58 @@ class FilterTests(filtering.FilterTests): self._delete_test_data('user', user_list) + def test_groups_for_user_filtered(self): + """Test use of filtering doesn't break groups_for_user listing. + + Some backends may use filtering to achieve the list of groups for a + user, so test that it can combine a second filter. + + Test Plan: + + - Create 10 groups, some with names we can filter on + - Create 2 users + - Assign 1 of those users to most of the groups, including some of the + well known named ones + - Assign the other user to other groups as spoilers + - Ensure that when we list groups for users with a filter on the group + name, both restrictions have been enforced on what is returned. + + """ + + number_of_groups = 10 + group_name_data = { + # entity index: name for entity + 5: 'The', + 6: 'The Ministry', + 9: 'The Ministry of Silly Walks', + } + group_list = self._create_test_data( + 'group', number_of_groups, + domain_id=DEFAULT_DOMAIN_ID, name_dict=group_name_data) + user_list = self._create_test_data('user', 2) + + for group in range(7): + # Create membership, including with two out of the three groups + # with well know names + self.identity_api.add_user_to_group(user_list[0]['id'], + group_list[group]['id']) + # ...and some spoiler memberships + for group in range(7, number_of_groups): + self.identity_api.add_user_to_group(user_list[1]['id'], + group_list[group]['id']) + + hints = driver_hints.Hints() + hints.add_filter('name', 'The', comparator='startswith') + groups = self.identity_api.list_groups_for_user( + user_list[0]['id'], hints=hints) + # We should only get back 2 out of the 3 groups that start with 'The' + # hence showing that both "filters" have been applied + self.assertThat(len(groups), matchers.Equals(2)) + self.assertIn(group_list[5]['id'], [groups[0]['id'], groups[1]['id']]) + self.assertIn(group_list[6]['id'], [groups[0]['id'], groups[1]['id']]) + self._delete_test_data('user', user_list) + self._delete_test_data('group', group_list) + class LimitTests(filtering.FilterTests): ENTITIES = ['user', 'group', 'project'] diff --git a/keystone/tests/unit/test_backend_ldap.py b/keystone/tests/unit/test_backend_ldap.py index a85ba7dae7..17c11b0332 100644 --- a/keystone/tests/unit/test_backend_ldap.py +++ b/keystone/tests/unit/test_backend_ldap.py @@ -3002,3 +3002,37 @@ class DomainSpecificSQLIdentity(DomainSpecificLDAPandSQLIdentity): [tests.TESTCONF + '/domain_configs_one_extra_sql/' + 'keystone.domain2.conf'], 'domain2') + + +class LdapFilterTests(test_backend.FilterTests, tests.TestCase): + + def setUp(self): + super(LdapFilterTests, self).setUp() + self.useFixture(database.Database()) + self.clear_database() + + common_ldap.register_handler('fake://', fakeldap.FakeLdap) + self.load_backends() + self.load_fixtures(default_fixtures) + + self.engine = sql.get_engine() + self.addCleanup(sql.cleanup) + sql.ModelBase.metadata.create_all(bind=self.engine) + + self.addCleanup(sql.ModelBase.metadata.drop_all, bind=self.engine) + self.addCleanup(common_ldap_core._HANDLERS.clear) + + def config_overrides(self): + super(LdapFilterTests, self).config_overrides() + self.config_fixture.config( + group='identity', + driver='keystone.identity.backends.ldap.Identity') + + def config_files(self): + config_files = super(LdapFilterTests, self).config_files() + config_files.append(tests.dirs.tests_conf('backend_ldap.conf')) + return config_files + + def clear_database(self): + for shelf in fakeldap.FakeShelves: + fakeldap.FakeShelves[shelf].clear() diff --git a/keystone/tests/unit/test_backend_sql.py b/keystone/tests/unit/test_backend_sql.py index e93f9e3072..5784607d5c 100644 --- a/keystone/tests/unit/test_backend_sql.py +++ b/keystone/tests/unit/test_backend_sql.py @@ -760,6 +760,21 @@ class SqlFilterTests(SqlTests, test_backend.FilterTests): groups = self.identity_api.list_groups() self.assertTrue(len(groups) > 0) + def test_groups_for_user_filtered(self): + # The SQL identity driver currently does not support filtering on the + # listing groups for a given user, so will fail this test. This is + # raised as bug #1412447. + try: + super(SqlFilterTests, self).test_groups_for_user_filtered() + except matchers.MismatchError: + return + # We shouldn't get here...if we do, it means someone has fixed the + # above defect, so we can remove this test override. As an aside, it + # would be nice to have used self.assertRaises() around the call above + # to achieve the logic here...but that does not seem to work when + # wrapping another assert (it won't seem to catch the error). + self.assertTrue(False) + class SqlLimitTests(SqlTests, test_backend.LimitTests): def setUp(self):