Delete user tokens after role grant/revoke

Delete user tokens when a new role is granted or revoked, in order to
prevent old tokens to continue to be valid for the original set of
roles for the remainder of the token's lifespan.

Addresses CVE-2012-4413.
Fixes bug 1041396.

Change-Id: Iecf891f274b67408f568b949a7028362c4c30312

keystone/identity/core.py

@@ -590,6 +590,8 @@ def add_role_to_user(self, context, user_id, role_id, tenant_id=None):
         self.identity_api.add_user_to_tenant(context, tenant_id, user_id)
         self.identity_api.add_role_to_user_and_tenant(
             context, user_id, tenant_id, role_id)
+        self.token_api.revoke_tokens(context, user_id)
+
         role_ref = self.identity_api.get_role(context, role_id)
         return {'role': role_ref}
 
@@ -614,7 +616,7 @@ def remove_role_from_user(self, context, user_id, role_id, tenant_id=None):
         if not roles:
             self.identity_api.remove_user_from_tenant(
                 context, tenant_id, user_id)
-        return
+        self.token_api.revoke_tokens(context, user_id)
 
     # COMPAT(diablo): CRUD extension
     def get_role_refs(self, context, user_id):
@@ -657,6 +659,8 @@ def create_role_ref(self, context, user_id, role):
         self.identity_api.add_user_to_tenant(context, tenant_id, user_id)
         self.identity_api.add_role_to_user_and_tenant(
             context, user_id, tenant_id, role_id)
+        self.token_api.revoke_tokens(context, user_id)
+
         role_ref = self.identity_api.get_role(context, role_id)
         return {'role': role_ref}
 
@@ -684,3 +688,4 @@ def delete_role_ref(self, context, user_id, role_ref_id):
         if not roles:
             self.identity_api.remove_user_from_tenant(
                 context, tenant_id, user_id)
+        self.token_api.revoke_tokens(context, user_id)

keystone/token/core.py

@@ -39,6 +39,10 @@ class Manager(manager.Manager):
     def __init__(self):
         super(Manager, self).__init__(CONF.token.driver)
 
+    def revoke_tokens(self, context, user_id):
+        for token_id in self.list_tokens(context, user_id):
+            self.delete_token(context, token_id)
+
 
 class Driver(object):
     """Interface description for a Token driver."""
@@ -106,6 +110,13 @@ def list_revoked_tokens(self):
         """
         raise exception.NotImplemented()
 
+    def revoke_tokens(self, user_id):
+        """Invalidates all tokens held by a user.
+
+        :raises: keystone.exception.UserNotFound
+        """
+        raise exception.NotImplemented()
+
     def _get_default_expire_time(self):
         """Determine when a token should expire based on the config.
 

tests/test_keystoneclient.py

@@ -794,15 +794,15 @@ def get_checkout(self):
     def test_tenant_add_and_remove_user(self):
         client = self.get_client(admin=True)
         client.roles.add_user_role(tenant=self.tenant_baz['id'],
-                                   user=self.user_foo['id'],
+                                   user=self.user_two['id'],
                                    role=self.role_useless['id'])
         user_refs = client.tenants.list_users(tenant=self.tenant_baz['id'])
-        self.assert_(self.user_foo['id'] in [x.id for x in user_refs])
+        self.assert_(self.user_two['id'] in [x.id for x in user_refs])
         client.roles.remove_user_role(tenant=self.tenant_baz['id'],
-                                      user=self.user_foo['id'],
+                                      user=self.user_two['id'],
                                       role=self.role_useless['id'])
         user_refs = client.tenants.list_users(tenant=self.tenant_baz['id'])
-        self.assert_(self.user_foo['id'] not in [x.id for x in user_refs])
+        self.assert_(self.user_two['id'] not in [x.id for x in user_refs])
 
     def test_user_role_add_404(self):
         from keystoneclient import exceptions as client_exceptions
@@ -1001,28 +1001,28 @@ def get_checkout(self):
     def test_tenant_add_and_remove_user(self):
         client = self.get_client(admin=True)
         client.roles.add_user_to_tenant(tenant_id=self.tenant_baz['id'],
-                                        user_id=self.user_foo['id'],
+                                        user_id=self.user_two['id'],
                                         role_id=self.role_useless['id'])
         role_refs = client.roles.get_user_role_refs(
-            user_id=self.user_foo['id'])
+            user_id=self.user_two['id'])
         self.assert_(self.tenant_baz['id'] in [x.tenantId for x in role_refs])
 
         # get the "role_refs" so we get the proper id, this is how the clients
         # do it
         roleref_refs = client.roles.get_user_role_refs(
-            user_id=self.user_foo['id'])
+            user_id=self.user_two['id'])
         for roleref_ref in roleref_refs:
             if (roleref_ref.roleId == self.role_useless['id']
                     and roleref_ref.tenantId == self.tenant_baz['id']):
                 # use python's scope fall through to leave roleref_ref set
                 break
 
         client.roles.remove_user_from_tenant(tenant_id=self.tenant_baz['id'],
-                                             user_id=self.user_foo['id'],
+                                             user_id=self.user_two['id'],
                                              role_id=roleref_ref.id)
 
         role_refs = client.roles.get_user_role_refs(
-            user_id=self.user_foo['id'])
+            user_id=self.user_two['id'])
         self.assert_(self.tenant_baz['id'] not in
                      [x.tenantId for x in role_refs])