diff options
| -rw-r--r-- | keystone/federation/core.py | 13 | ||||
| -rw-r--r-- | keystone/notifications.py | 1 | ||||
| -rw-r--r-- | keystone/tests/unit/test_v3_federation.py | 12 | ||||
| -rw-r--r-- | keystone/tests/unit/token/test_fernet_provider.py | 12 | ||||
| -rw-r--r-- | keystone/token/provider.py | 15 | ||||
| -rw-r--r-- | keystone/token/providers/common.py | 7 | ||||
| -rw-r--r-- | releasenotes/notes/bug-1291157-00b5c714a097e84c.yaml | 7 |
7 files changed, 57 insertions, 10 deletions
diff --git a/keystone/federation/core.py b/keystone/federation/core.py index 1b7c2e161..677e78343 100644 --- a/keystone/federation/core.py +++ b/keystone/federation/core.py @@ -22,6 +22,7 @@ import keystone.conf from keystone import exception from keystone.federation import utils from keystone.i18n import _ +from keystone import notifications # This is a general cache region for service providers. @@ -77,6 +78,18 @@ class Manager(manager.Manager): self._cleanup_idp_domain(idp['domain_id']) raise + def delete_idp(self, idp_id): + self.driver.delete_idp(idp_id) + # NOTE(lbragstad): If an identity provider is removed from the system, + # then we need to invalidate the token cache. Otherwise it will be + # possible for federated tokens to be considered valid after a service + # provider removes a federated identity provider resource. The `idp_id` + # isn't actually used when invalidating the token cache but we have to + # pass something. + notifications.Audit.internal( + notifications.INVALIDATE_TOKEN_CACHE_DELETED_IDP, idp_id + ) + def _cleanup_idp_domain(self, domain_id): domain = {'enabled': False} PROVIDERS.resource_api.update_domain(domain_id, domain) diff --git a/keystone/notifications.py b/keystone/notifications.py index 7fc41ae04..40e7c31ac 100644 --- a/keystone/notifications.py +++ b/keystone/notifications.py @@ -79,6 +79,7 @@ CONF = keystone.conf.CONF INVALIDATE_USER_TOKEN_PERSISTENCE = 'invalidate_user_tokens' INVALIDATE_USER_PROJECT_TOKEN_PERSISTENCE = 'invalidate_user_project_tokens' INVALIDATE_USER_OAUTH_CONSUMER_TOKENS = 'invalidate_user_consumer_tokens' +INVALIDATE_TOKEN_CACHE_DELETED_IDP = 'invalidate_token_cache_from_deleted_idp' class Audit(object): diff --git a/keystone/tests/unit/test_v3_federation.py b/keystone/tests/unit/test_v3_federation.py index 2ab0d6681..f21dfe8fe 100644 --- a/keystone/tests/unit/test_v3_federation.py +++ b/keystone/tests/unit/test_v3_federation.py @@ -43,7 +43,6 @@ from keystone.tests.unit import federation_fixtures from keystone.tests.unit import ksfixtures from keystone.tests.unit import mapping_fixtures from keystone.tests.unit import test_v3 -from keystone.tests.unit import utils from keystone.token.providers import common as token_common @@ -2061,9 +2060,7 @@ class FederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin): self.TOKEN_SCOPE_PROJECT_EMPLOYEE_FROM_CUSTOMER, expected_status=http_client.FORBIDDEN) - @utils.wip('This will fail because of bug #1291157. The token should be ' - 'invalid after deleting the identity provider.') - def test_validate_token_after_deleting_idp_fails(self): + def test_validate_token_after_deleting_idp_raises_not_found(self): token = self.v3_create_token( self.TOKEN_SCOPE_PROJECT_EMPLOYEE_FROM_ADMIN ) @@ -2074,13 +2071,14 @@ class FederatedTokenTests(test_v3.RestfulTestCase, FederatedSetupMixin): headers = { 'X-Subject-Token': token_id } - # FIXME(lbragstad): This should raise a 401 Unauthorized exception - # since the identity provider is gone. + # NOTE(lbragstad): This raises a 404 NOT FOUND because the identity + # provider is no longer present. We raise 404 NOT FOUND when we + # validate a token and a project or domain no longer exists. self.get( '/auth/tokens/', token=token_id, headers=headers, - expected_status=http_client.UNAUTHORIZED + expected_status=http_client.NOT_FOUND ) def test_scope_to_bad_project(self): diff --git a/keystone/tests/unit/token/test_fernet_provider.py b/keystone/tests/unit/token/test_fernet_provider.py index 7d4fd4257..9ee253ed8 100644 --- a/keystone/tests/unit/token/test_fernet_provider.py +++ b/keystone/tests/unit/token/test_fernet_provider.py @@ -135,13 +135,19 @@ class TestValidate(unit.TestCase): method_names = ['mapped'] group_ids = [uuid.uuid4().hex, ] - identity_provider = uuid.uuid4().hex + idp_id = uuid.uuid4().hex + idp_ref = { + 'id': idp_id, + 'description': uuid.uuid4().hex, + 'enabled': True + } + self.federation_api.create_idp(idp_id, idp_ref) protocol = uuid.uuid4().hex auth_context_params = { 'user_id': user_ref['id'], 'user_name': user_ref['name'], 'group_ids': group_ids, - federation_constants.IDENTITY_PROVIDER: identity_provider, + federation_constants.IDENTITY_PROVIDER: idp_id, federation_constants.PROTOCOL: protocol, } auth_context = auth.core.AuthContext(**auth_context_params) @@ -157,7 +163,7 @@ class TestValidate(unit.TestCase): 'name': CONF.federation.federated_domain_name, }, federation_constants.FEDERATION: { 'groups': [{'id': group_id} for group_id in group_ids], - 'identity_provider': {'id': identity_provider, }, + 'identity_provider': {'id': idp_id, }, 'protocol': {'id': protocol, }, }, } diff --git a/keystone/token/provider.py b/keystone/token/provider.py index d9d81ee2e..67cffe15f 100644 --- a/keystone/token/provider.py +++ b/keystone/token/provider.py @@ -91,6 +91,8 @@ class Manager(manager.Manager): self._delete_user_project_tokens_callback], [notifications.INVALIDATE_USER_OAUTH_CONSUMER_TOKENS, self._delete_user_oauth_consumer_tokens_callback], + [notifications.INVALIDATE_TOKEN_CACHE_DELETED_IDP, + self._invalidate_token_cache_from_deleted_idp_callback], ] } @@ -329,3 +331,16 @@ class Manager(manager.Manager): if CONF.token.cache_on_issue: # NOTE(amakarov): preserving behavior TOKENS_REGION.invalidate() + + def _invalidate_token_cache_from_deleted_idp_callback(self, + service, + resource_type, + operation, payload): + """Callback to invalidate the token cache after deleting an idp. + + While this callback doesn't use the information about the deleted + identity provider to invalidate the cache, the method name and payload + are emitted when logging at the DEBUG level. + + """ + TOKENS_REGION.invalidate() diff --git a/keystone/token/providers/common.py b/keystone/token/providers/common.py index 740550401..64f14dd58 100644 --- a/keystone/token/providers/common.py +++ b/keystone/token/providers/common.py @@ -462,6 +462,12 @@ class V3TokenDataHelper(provider_api.ProviderAPIMixin, object): if service_providers: token_data['service_providers'] = service_providers + def _validate_identity_provider(self, token_data): + federated_info = token_data['user'].get('OS-FEDERATION') + if federated_info: + idp_id = federated_info['identity_provider']['id'] + PROVIDERS.federation_api.get_idp(idp_id) + def _populate_token_dates(self, token_data, expires=None, issued_at=None): if not expires: expires = default_expire_time() @@ -518,6 +524,7 @@ class V3TokenDataHelper(provider_api.ProviderAPIMixin, object): token_data, user_id, system, domain_id, project_id, trust ) self._populate_service_providers(token_data) + self._validate_identity_provider(token_data) self._populate_token_dates(token_data, expires=expires, issued_at=issued_at) self._populate_oauth_section(token_data, access_token) diff --git a/releasenotes/notes/bug-1291157-00b5c714a097e84c.yaml b/releasenotes/notes/bug-1291157-00b5c714a097e84c.yaml new file mode 100644 index 000000000..43414cfb1 --- /dev/null +++ b/releasenotes/notes/bug-1291157-00b5c714a097e84c.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + [`bug 1291157 <https://bugs.launchpad.net/keystone/+bug/1291157>`_] + Identity provider information is now validated in during token validation. + If an identity provider is removed from a keystone service provider, tokens + associated to that identity provider will be considered invalid. |