From cecf6048f2018e7cea864e3a6ff18b9088ac4254 Mon Sep 17 00:00:00 2001 From: Steve Martinelli Date: Fri, 11 Mar 2016 02:45:53 -0500 Subject: [PATCH] Support `id` and `enabled` attributes when listing service providers list SPs currently doesn't support to filter records by any attributes, but this is used somewhere, such as OpenStack Client using `name` to filter the record. SP doesn't has `name` attribute but has `id`, `enabled` attributes instead. This patch enables the filtering of Service Provider based on `id`, `enabled` attributes so that OpenStack Client or the CURL query can benefit from it. based off of: Ib672ba759d26bdd0eecd48451994b3451fb8648a Closes-Bug: 1555830 Change-Id: Icdecaa44415786397ee8bb22de16d25cb8fe603a --- keystone/federation/backends/sql.py | 5 +- keystone/federation/controllers.py | 9 +-- keystone/federation/core.py | 41 +++++++++---- .../legacy_drivers/federation/V8/api_v3.py | 6 ++ keystone/tests/unit/test_v3_federation.py | 59 +++++++++++++++++++ .../enable-filter-idp-d0135f4615178cfc.yaml | 4 ++ 6 files changed, 107 insertions(+), 17 deletions(-) diff --git a/keystone/federation/backends/sql.py b/keystone/federation/backends/sql.py index 6bba07daf7..78a7831ba1 100644 --- a/keystone/federation/backends/sql.py +++ b/keystone/federation/backends/sql.py @@ -358,9 +358,10 @@ class Federation(core.FederationDriverV9): raise exception.ServiceProviderNotFound(sp_id=sp_id) return sp_ref - def list_sps(self): + def list_sps(self, hints=None): with sql.session_for_read() as session: - sps = session.query(ServiceProviderModel) + query = session.query(ServiceProviderModel) + sps = sql.filter_limit_query(ServiceProviderModel, query, hints) sps_list = [sp.to_dict() for sp in sps] return sps_list diff --git a/keystone/federation/controllers.py b/keystone/federation/controllers.py index 3f9405100d..b9e2d88365 100644 --- a/keystone/federation/controllers.py +++ b/keystone/federation/controllers.py @@ -480,11 +480,12 @@ class ServiceProvider(_ControllerBase): response = ServiceProvider.wrap_member(context, sp_ref) return wsgi.render_response(body=response, status=('201', 'Created')) - @controller.protected() - def list_service_providers(self, context): - ref = self.federation_api.list_sps() + @controller.filterprotected('id', 'enabled') + def list_service_providers(self, context, filters): + hints = self.build_driver_hints(context, filters) + ref = self.federation_api.list_sps(hints=hints) ref = [self.filter_params(x) for x in ref] - return ServiceProvider.wrap_collection(context, ref) + return ServiceProvider.wrap_collection(context, ref, hints=hints) @controller.protected() def get_service_provider(self, context, sp_id): diff --git a/keystone/federation/core.py b/keystone/federation/core.py index a93e3bb5cb..23028dfdb7 100644 --- a/keystone/federation/core.py +++ b/keystone/federation/core.py @@ -375,16 +375,6 @@ class FederationDriverBase(object): """ raise exception.NotImplemented() # pragma: no cover - @abc.abstractmethod - def list_sps(self): - """List all service providers. - - :returns: List of service provider ref objects - :rtype: list of dicts - - """ - raise exception.NotImplemented() # pragma: no cover - @abc.abstractmethod def get_sp(self, sp_id): """Get a service provider. @@ -459,6 +449,16 @@ class FederationDriverV8(FederationDriverBase): """ raise exception.NotImplemented() # pragma: no cover + @abc.abstractmethod + def list_sps(self): + """List all service providers. + + :returns: List of service provider ref objects + :rtype: list of dicts + + """ + raise exception.NotImplemented() # pragma: no cover + class FederationDriverV9(FederationDriverBase): """New or redefined methods from V8. @@ -483,6 +483,21 @@ class FederationDriverV9(FederationDriverBase): """ raise exception.NotImplemented() # pragma: no cover + @abc.abstractmethod + def list_sps(self, hints): + """List all service providers. + + :param hints: filter hints which the driver should + implement if at all possible. + :returns: List of service provider ref objects + :rtype: list of dicts + + :raises keystone.exception.ServiceProviderNotFound: If the SP + doesn't exist. + + """ + raise exception.NotImplemented() # pragma: no cover + class V9FederationWrapperForV8Driver(FederationDriverV9): """Wrapper class to supported a V8 legacy driver. @@ -576,7 +591,11 @@ class V9FederationWrapperForV8Driver(FederationDriverV9): def delete_sp(self, sp_id): self.driver.delete_sp(sp_id) - def list_sps(self): + # NOTE(davechen): The hints is ignored here to support legacy drivers, + # but the filters in hints will be remain unsatisfied and V3Controller + # wrapper will apply these filters at the end. So that the result get + # returned for list SPs will still be filtered with the legacy drivers. + def list_sps(self, hints): return self.driver.list_sps() def get_sp(self, sp_id): diff --git a/keystone/tests/unit/backend/legacy_drivers/federation/V8/api_v3.py b/keystone/tests/unit/backend/legacy_drivers/federation/V8/api_v3.py index ce621b9f8c..d5469768fe 100644 --- a/keystone/tests/unit/backend/legacy_drivers/federation/V8/api_v3.py +++ b/keystone/tests/unit/backend/legacy_drivers/federation/V8/api_v3.py @@ -100,3 +100,9 @@ class ServiceProviderTestsV8( def config_overrides(self): super(ServiceProviderTestsV8, self).config_overrides() self.useV8driver() + + def test_filter_list_sp_by_id(self): + self.skipTest('Operation not supported in v8 and earlier drivers') + + def test_filter_list_sp_by_enabled(self): + self.skipTest('Operation not supported in v8 and earlier drivers') diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index 7dc9067b9c..27218760fb 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -3264,6 +3264,15 @@ class ServiceProviderTests(test_v3.RestfulTestCase): return '/OS-FEDERATION/service_providers/' + str(suffix) return '/OS-FEDERATION/service_providers' + def _create_default_sp(self, body=None): + """Create default Service Provider.""" + url = self.base_url(suffix=uuid.uuid4().hex) + if body is None: + body = self.sp_ref() + resp = self.put(url, body={'service_provider': body}, + expected_status=http_client.CREATED) + return resp + def test_get_service_provider(self): url = self.base_url(suffix=self.SERVICE_PROVIDER_ID) resp = self.get(url) @@ -3413,6 +3422,56 @@ class ServiceProviderTests(test_v3.RestfulTestCase): url = self.base_url(suffix=uuid.uuid4().hex) self.delete(url, expected_status=http_client.NOT_FOUND) + def test_filter_list_sp_by_id(self): + def get_id(resp): + sp = resp.result.get('service_provider') + return sp.get('id') + + sp1_id = get_id(self._create_default_sp()) + sp2_id = get_id(self._create_default_sp()) + + # list the SP, should get SPs. + url = self.base_url() + resp = self.get(url) + sps = resp.result.get('service_providers') + entities_ids = [e['id'] for e in sps] + self.assertIn(sp1_id, entities_ids) + self.assertIn(sp2_id, entities_ids) + + # filter the SP by 'id'. Only SP1 should appear. + url = self.base_url() + '?id=' + sp1_id + resp = self.get(url) + sps = resp.result.get('service_providers') + entities_ids = [e['id'] for e in sps] + self.assertIn(sp1_id, entities_ids) + self.assertNotIn(sp2_id, entities_ids) + + def test_filter_list_sp_by_enabled(self): + def get_id(resp): + sp = resp.result.get('service_provider') + return sp.get('id') + + sp1_id = get_id(self._create_default_sp()) + sp2_ref = self.sp_ref() + sp2_ref['enabled'] = False + sp2_id = get_id(self._create_default_sp(body=sp2_ref)) + + # list the SP, should get two SPs. + url = self.base_url() + resp = self.get(url) + sps = resp.result.get('service_providers') + entities_ids = [e['id'] for e in sps] + self.assertIn(sp1_id, entities_ids) + self.assertIn(sp2_id, entities_ids) + + # filter the SP by 'enabled'. Only SP1 should appear. + url = self.base_url() + '?enabled=True' + resp = self.get(url) + sps = resp.result.get('service_providers') + entities_ids = [e['id'] for e in sps] + self.assertIn(sp1_id, entities_ids) + self.assertNotIn(sp2_id, entities_ids) + class WebSSOTests(FederatedTokenTests): """A class for testing Web SSO.""" diff --git a/releasenotes/notes/enable-filter-idp-d0135f4615178cfc.yaml b/releasenotes/notes/enable-filter-idp-d0135f4615178cfc.yaml index ea99014bdd..f4c1bbe76e 100644 --- a/releasenotes/notes/enable-filter-idp-d0135f4615178cfc.yaml +++ b/releasenotes/notes/enable-filter-idp-d0135f4615178cfc.yaml @@ -4,3 +4,7 @@ features: [`bug 1525317 `_] Enable filtering of identity providers based on `id`, and `enabled` attributes. + - > + [`bug 1555830 `_] + Enable filtering of service providers based on `id`, and `enabled` + attributes. \ No newline at end of file