Validate identity providers during token validation

Previously, it was possible to validate a federated keystone token
after the identity provider associated by that token was deleted,
which is a security concern.

This commit does two things. First it makes it so that the token
cache is invalidated when identity providers are deleted. Second,
it validates the identity provider in the token data and ensures it
actually exists in the system before considering the token valid.

Change-Id: I57491c5a7d657b25cc436452acd7fcc4cd285839
Closes-Bug: 1291157

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)

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

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

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, },
             },
         }

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

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)

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.