openstack/keystone
Code Issues Proposed changes

Delete user tokens after role grant/revoke

Browse Source 
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
This commit is contained in:
Dolph Mathews 2012-09-07 14:35:21 -05:00 committed by Thierry Carrez
parent ee31114d9d
commit efb6b3fca0
3 changed files with 26 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
keystone/identity/core.py
View File

@ -590,6 +590,8 @@ class RoleController(wsgi.Application):
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 @@ class RoleController(wsgi.Application):
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 @@ class RoleController(wsgi.Application):
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 @@ class RoleController(wsgi.Application):
if not roles:
self.identity_api.remove_user_from_tenant(
context, tenant_id, user_id)
self.token_api.revoke_tokens(context, user_id)

11
keystone/token/core.py
View File

@ -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 @@ class Driver(object):
"""
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.

18
tests/test_keystoneclient.py
View File

@ -794,15 +794,15 @@ class KcMasterTestCase(CompatTestCase, KeystoneClientTests):
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,16 +1001,16 @@ class KcEssex3TestCase(CompatTestCase, KeystoneClientTests):
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']):
@ -1018,11 +1018,11 @@ class KcEssex3TestCase(CompatTestCase, KeystoneClientTests):
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])