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

keystone/identity/core.py

@@ -408,6 +408,17 @@ def update_user(self, context, user_id, user):
             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 @@ def set_user_enabled(self, context, user_id, user):
         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."""

keystone/service.py

@@ -28,6 +28,9 @@
 from keystone.common import wsgi
 
 
+LOG = logging.getLogger(__name__)
+
+
 class AdminRouter(wsgi.ComposingRouter):
     def __init__(self):
         mapper = routes.Mapper()
@@ -275,7 +278,8 @@ def authenticate(self, context, auth=None):
 
                 # 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 @@ def authenticate(self, context, auth=None):
 
             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:

tests/test_keystoneclient.py

@@ -309,6 +309,23 @@ def test_change_password_invalidates_token(self):
                           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 @@ def test_user_create_update_delete(self):
         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')
@@ -871,7 +888,7 @@ def test_user_create_update_delete(self):
         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')