Merge "Enable filtering in LDAP backend for listing entities"

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.

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):
+    def get_all_filtered(self, hints):
-        return [self.filter_attributes(user) for user in self.get_all()]
+        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)]

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)

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:

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']

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()

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):