Invalidate user tokens when a user is disabled
Fixes Bug 997194 Delete valid tokens for a user when they have been disabled Moved logic to delete tokens into update_user, as this can be called directly form the REST API. Also checks if a user is enabled when creating a token from another token, this helps in cases there the backend didn't support listing of tokens (and as a result weren't deleted) Change-Id: Ib5ed73a7873bfa66ef31bf6d0f0322f50e677688
This commit is contained in:
parent
3c9c38a8e0
commit
628149b3dc
|
@ -408,6 +408,17 @@ class UserController(wsgi.Application):
|
|||
raise exception.UserNotFound(user_id=user_id)
|
||||
|
||||
user_ref = self.identity_api.update_user(context, user_id, user)
|
||||
|
||||
# If the password was changed or the user was disabled we clear tokens
|
||||
if user.get('password') or user.get('enabled', True) == False:
|
||||
try:
|
||||
for token_id in self.token_api.list_tokens(context, user_id):
|
||||
self.token_api.delete_token(context, token_id)
|
||||
except exception.NotImplemented:
|
||||
# The users status has been changed but tokens remain valid for
|
||||
# backends that can't list tokens for users
|
||||
LOG.warning('User %s status has changed, but existing tokens '
|
||||
'remain valid' % user_id)
|
||||
return {'user': user_ref}
|
||||
|
||||
def delete_user(self, context, user_id):
|
||||
|
@ -421,16 +432,7 @@ class UserController(wsgi.Application):
|
|||
return self.update_user(context, user_id, user)
|
||||
|
||||
def set_user_password(self, context, user_id, user):
|
||||
user_ref = self.update_user(context, user_id, user)
|
||||
try:
|
||||
for token_id in self.token_api.list_tokens(context, user_id):
|
||||
self.token_api.delete_token(context, token_id)
|
||||
except exception.NotImplemented:
|
||||
# The password has been changed but tokens remain valid for
|
||||
# backends that can't list tokens for users
|
||||
LOG.warning('Password changed for %s, but existing tokens remain '
|
||||
'valid' % user_id)
|
||||
return user_ref
|
||||
return self.update_user(context, user_id, user)
|
||||
|
||||
def update_user_tenant(self, context, user_id, user):
|
||||
"""Update the default tenant."""
|
||||
|
|
|
@ -28,6 +28,9 @@ from keystone.common import utils
|
|||
from keystone.common import wsgi
|
||||
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class AdminRouter(wsgi.ComposingRouter):
|
||||
def __init__(self):
|
||||
mapper = routes.Mapper()
|
||||
|
@ -275,7 +278,8 @@ class TokenController(wsgi.Application):
|
|||
|
||||
# If the user is disabled don't allow them to authenticate
|
||||
if not user_ref.get('enabled', True):
|
||||
raise exception.Forbidden(message='User has been disabled')
|
||||
LOG.warning('User %s is disabled' % user_id)
|
||||
raise exception.Unauthorized()
|
||||
except AssertionError as e:
|
||||
raise exception.Unauthorized(e.message)
|
||||
|
||||
|
@ -314,6 +318,14 @@ class TokenController(wsgi.Application):
|
|||
|
||||
user_ref = old_token_ref['user']
|
||||
|
||||
# If the user is disabled don't allow them to authenticate
|
||||
current_user_ref = self.identity_api.get_user(
|
||||
context=context,
|
||||
user_id=user_ref['id'])
|
||||
if not current_user_ref.get('enabled', True):
|
||||
LOG.warning('User %s is disabled' % user_ref['id'])
|
||||
raise exception.Unauthorized()
|
||||
|
||||
tenants = self.identity_api.get_tenants_for_user(context,
|
||||
user_ref['id'])
|
||||
if tenant_id:
|
||||
|
|
|
@ -309,6 +309,23 @@ class KeystoneClientTests(object):
|
|||
client.tokens.authenticate,
|
||||
token=token_id)
|
||||
|
||||
def test_disable_user_invalidates_token(self):
|
||||
from keystoneclient import exceptions as client_exceptions
|
||||
|
||||
admin_client = self.get_client(admin=True)
|
||||
foo_client = self.get_client(self.user_foo)
|
||||
|
||||
admin_client.users.update_enabled(user=self.user_foo['id'],
|
||||
enabled=False)
|
||||
|
||||
self.assertRaises(client_exceptions.Unauthorized,
|
||||
foo_client.tokens.authenticate,
|
||||
token=foo_client.auth_token)
|
||||
|
||||
self.assertRaises(client_exceptions.Unauthorized,
|
||||
self.get_client,
|
||||
self.user_foo)
|
||||
|
||||
def test_user_create_update_delete(self):
|
||||
from keystoneclient import exceptions as client_exceptions
|
||||
|
||||
|
@ -332,7 +349,7 @@ class KeystoneClientTests(object):
|
|||
user = client.users.get(user.id)
|
||||
self.assertFalse(user.enabled)
|
||||
|
||||
self.assertRaises(client_exceptions.AuthorizationFailure,
|
||||
self.assertRaises(client_exceptions.Unauthorized,
|
||||
self._client,
|
||||
username=test_username,
|
||||
password='password')
|
||||
|
@ -880,7 +897,7 @@ class KcEssex3TestCase(CompatTestCase, KeystoneClientTests):
|
|||
user = client.users.get(user.id)
|
||||
self.assertFalse(user.enabled)
|
||||
|
||||
self.assertRaises(client_exceptions.AuthorizationFailure,
|
||||
self.assertRaises(client_exceptions.Unauthorized,
|
||||
self._client,
|
||||
username=test_username,
|
||||
password='password')
|
||||
|
|
Loading…
Reference in New Issue