Merge "Invalidate user tokens when a user is disabled"

3 files changed, 44 insertions, 13 deletions

diff --git a/keystone/identity/core.py b/keystone/identity/core.py
index a0cacb3e..6421ea26 100644
--- a/keystone/identity/core.py
+++ b/keystone/identity/core.py
@@ -418,6 +418,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):
@@ -431,16 +442,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."""
diff --git a/keystone/service.py b/keystone/service.py
index 1cb455dd..2b024f4e 100644
--- a/keystone/service.py
+++ b/keystone/service.py
@@ -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:
diff --git a/tests/test_keystoneclient.py b/tests/test_keystoneclient.py
index 28792662..68f190fa 100644
--- a/tests/test_keystoneclient.py
+++ b/tests/test_keystoneclient.py
@@ -316,6 +316,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
 
@@ -339,7 +356,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')
@@ -903,7 +920,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')