From cdd3ac454c2850ab72883963aa6bd6a0d80fe56f Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Sat, 12 Dec 2015 08:51:50 +0800 Subject: [PATCH] Enable `id`, `enabled` attributes filtering for list IdP API list IdP 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. IdP doesn't has `name` attribute but has `id`, `enabled` attributes instead. This patch enables the filtering of Identity Provider based on `id`, `enabled` attributes so that OpenStack Client or the CURL query can benefit from it. Change-Id: Ib672ba759d26bdd0eecd48451994b3451fb8648a Related-Bug: #1479837 Closes-Bug: #1525317 --- keystone/federation/backends/sql.py | 5 +- keystone/federation/controllers.py | 9 ++-- keystone/federation/core.py | 46 +++++++++++------ keystone/tests/unit/test_v3_federation.py | 51 +++++++++++++++++++ .../enable-filter-idp-d0135f4615178cfc.yaml | 6 +++ 5 files changed, 96 insertions(+), 21 deletions(-) create mode 100644 releasenotes/notes/enable-filter-idp-d0135f4615178cfc.yaml diff --git a/keystone/federation/backends/sql.py b/keystone/federation/backends/sql.py index 26fd51c1a6..0e5f200900 100644 --- a/keystone/federation/backends/sql.py +++ b/keystone/federation/backends/sql.py @@ -186,9 +186,10 @@ class Federation(core.FederationDriverV9): except sql.NotFound: raise exception.IdentityProviderNotFound(idp_id=remote_id) - def list_idps(self): + def list_idps(self, hints=None): with sql.transaction() as session: - idps = session.query(IdentityProviderModel) + query = session.query(IdentityProviderModel) + idps = sql.filter_limit_query(IdentityProviderModel, query, hints) idps_list = [idp.to_dict() for idp in idps] return idps_list diff --git a/keystone/federation/controllers.py b/keystone/federation/controllers.py index 56bddddd55..629b4fc23c 100644 --- a/keystone/federation/controllers.py +++ b/keystone/federation/controllers.py @@ -98,11 +98,12 @@ class IdentityProvider(_ControllerBase): response = IdentityProvider.wrap_member(context, idp_ref) return wsgi.render_response(body=response, status=('201', 'Created')) - @controller.protected() - def list_identity_providers(self, context): - ref = self.federation_api.list_idps() + @controller.filterprotected('id', 'enabled') + def list_identity_providers(self, context, filters): + hints = self.build_driver_hints(context, filters) + ref = self.federation_api.list_idps(hints=hints) ref = [self.filter_params(x) for x in ref] - return IdentityProvider.wrap_collection(context, ref) + return IdentityProvider.wrap_collection(context, ref, hints=hints) @controller.protected() def get_identity_provider(self, context, idp_id): diff --git a/keystone/federation/core.py b/keystone/federation/core.py index a2e911fc09..956c5dde22 100644 --- a/keystone/federation/core.py +++ b/keystone/federation/core.py @@ -136,18 +136,6 @@ class FederationDriverBase(object): """ raise exception.NotImplemented() # pragma: no cover - @abc.abstractmethod - def list_idps(self): - """List all identity providers. - - :raises keystone.exception.IdentityProviderNotFound: If the IdP - doesn't exist. - :returns: list of idp refs - :rtype: list of dicts - - """ - raise exception.NotImplemented() # pragma: no cover - @abc.abstractmethod def get_idp(self, idp_id): """Get an identity provider by ID. @@ -459,7 +447,18 @@ class FederationDriverV8(FederationDriverBase): """ - pass + @abc.abstractmethod + def list_idps(self): + """List all identity providers. + + :returns: list of idp refs + :rtype: list of dicts + + :raises keystone.exception.IdentityProviderNotFound: If the IdP + doesn't exist. + + """ + raise exception.NotImplemented() # pragma: no cover class FederationDriverV9(FederationDriverBase): @@ -470,7 +469,20 @@ class FederationDriverV9(FederationDriverBase): """ - pass + @abc.abstractmethod + def list_idps(self, hints): + """List all identity providers. + + :param hints: filter hints which the driver should + implement if at all possible. + :returns: list of idp refs + :rtype: list of dicts + + :raises keystone.exception.IdentityProviderNotFound: If the IdP + doesn't exist. + + """ + raise exception.NotImplemented() # pragma: no cover class V9FederationWrapperForV8Driver(FederationDriverV9): @@ -509,7 +521,11 @@ class V9FederationWrapperForV8Driver(FederationDriverV9): def delete_idp(self, idp_id): self.driver.delete_idp(idp_id) - def list_idps(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 IdP will still be filtered with the legacy drivers. + def list_idps(self, hints): return self.driver.list_idps() def get_idp(self, idp_id): diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index 4ab02fa2d6..970a80949d 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -1013,6 +1013,57 @@ class FederatedIdentityProviderTests(test_v3.RestfulTestCase): ids_intersection = entities_ids.intersection(ids) self.assertEqual(ids_intersection, ids) + def test_filter_list_idp_by_id(self): + def get_id(resp): + r = self._fetch_attribute_from_response(resp, + 'identity_provider') + return r.get('id') + + idp1_id = get_id(self._create_default_idp()) + idp2_id = get_id(self._create_default_idp()) + + # list the IdP, should get two IdP. + url = self.base_url() + resp = self.get(url) + entities = self._fetch_attribute_from_response(resp, + 'identity_providers') + entities_ids = [e['id'] for e in entities] + self.assertItemsEqual(entities_ids, [idp1_id, idp2_id]) + + # filter the IdP by ID. + url = self.base_url() + '?id=' + idp1_id + resp = self.get(url) + filtered_service_list = resp.json['identity_providers'] + self.assertThat(filtered_service_list, matchers.HasLength(1)) + self.assertEqual(idp1_id, filtered_service_list[0].get('id')) + + def test_filter_list_idp_by_enabled(self): + def get_id(resp): + r = self._fetch_attribute_from_response(resp, + 'identity_provider') + return r.get('id') + + idp1_id = get_id(self._create_default_idp()) + + body = self.default_body.copy() + body['enabled'] = False + idp2_id = get_id(self._create_default_idp(body=body)) + + # list the IdP, should get two IdP. + url = self.base_url() + resp = self.get(url) + entities = self._fetch_attribute_from_response(resp, + 'identity_providers') + entities_ids = [e['id'] for e in entities] + self.assertItemsEqual(entities_ids, [idp1_id, idp2_id]) + + # filter the IdP by 'enabled'. + url = self.base_url() + '?enabled=True' + resp = self.get(url) + filtered_service_list = resp.json['identity_providers'] + self.assertThat(filtered_service_list, matchers.HasLength(1)) + self.assertEqual(idp1_id, filtered_service_list[0].get('id')) + def test_check_idp_uniqueness(self): """Add same IdP twice. diff --git a/releasenotes/notes/enable-filter-idp-d0135f4615178cfc.yaml b/releasenotes/notes/enable-filter-idp-d0135f4615178cfc.yaml new file mode 100644 index 0000000000..6f6c222d49 --- /dev/null +++ b/releasenotes/notes/enable-filter-idp-d0135f4615178cfc.yaml @@ -0,0 +1,6 @@ +--- +features: + - > + [`bug 1525317 `_] + Enable filtering of identity providers based on ``id``, and ``enabled`` + attributes.