Merge "PCI-DSS Disable inactive users requirements"

keystone/common/sql/core.py

@@ -53,6 +53,7 @@ Integer = sql.Integer
 Enum = sql.Enum
 ForeignKey = sql.ForeignKey
 DateTime = sql.DateTime
+Date = sql.Date
 IntegrityError = sql.exc.IntegrityError
 DBDuplicateEntry = db_exception.DBDuplicateEntry
 OperationalError = sql.exc.OperationalError

keystone/common/sql/migrate_repo/versions/107_add_user_date_columns.py

@@ -0,0 +1,30 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import datetime
+
+import sqlalchemy as sql
+
+
+def upgrade(migrate_engine):
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+
+    created_at = sql.Column('created_at', sql.DateTime(), nullable=True)
+    last_active_at = sql.Column('last_active_at', sql.Date(), nullable=True)
+    user_table = sql.Table('user', meta, autoload=True)
+    user_table.create_column(created_at)
+    user_table.create_column(last_active_at)
+
+    now = datetime.datetime.utcnow()
+    stmt = user_table.update().values(created_at=now)
+    stmt.execute()

keystone/conf/security_compliance.py

@@ -17,10 +17,14 @@ from keystone.conf import utils
 
 disable_user_account_days_inactive = cfg.IntOpt(
     'disable_user_account_days_inactive',
-    default=0,
+    default=None,
+    min=1,
     help=utils.fmt("""
 Number of days for which a user can be inactive before the account becomes
-disabled. Setting the value to 0 disables this feature.
+disabled. This feature is disabled by default. Note: this feature is only
+supported via the SQL backend driver for identity. In addition, whether or
+not a user is disabled will be handled by the API and may not match the
+user table enabled column in the database.
 """))
 
 lockout_failure_attempts = cfg.IntOpt(

keystone/exception.py

@@ -212,6 +212,10 @@ class MissingGroups(Unauthorized):
                        "mapping %(mapping_id)s")
 
 
+class UserDisabled(Unauthorized):
+    message_format = _("The account is disabled for user: %(user_id)s")
+
+
 class AuthMethodNotSupported(AuthPluginException):
     message_format = _("Attempted to authenticate with an unsupported method.")
 

keystone/identity/backends/sql.py

@@ -12,6 +12,8 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import datetime
+
 import sqlalchemy
 
 from keystone.common import driver_hints
@@ -48,14 +50,15 @@ class Identity(base.IdentityDriverV8):
     # Identity interface
     def authenticate(self, user_id, password):
         with sql.session_for_read() as session:
-            user_ref = None
             try:
                 user_ref = self._get_user(session, user_id)
             except exception.UserNotFound:
                 raise AssertionError(_('Invalid user / password'))
-            if not self._check_password(password, user_ref):
-                raise AssertionError(_('Invalid user / password'))
-            return base.filter_user(user_ref.to_dict())
+        if not self._check_password(password, user_ref):
+            raise AssertionError(_('Invalid user / password'))
+        elif not user_ref.enabled:
+            raise exception.UserDisabled(user_id=user_id)
+        return base.filter_user(user_ref.to_dict())
 
     # user crud
 
@@ -64,6 +67,7 @@ class Identity(base.IdentityDriverV8):
         user = utils.hash_user_password(user)
         with sql.session_for_write() as session:
             user_ref = model.User.from_dict(user)
+            user_ref.created_at = datetime.datetime.utcnow()
             session.add(user_ref)
             return base.filter_user(user_ref.to_dict())
 

keystone/identity/backends/sql_model.py

@@ -14,6 +14,7 @@
 
 import datetime
 
+from oslo_config import cfg
 import sqlalchemy
 from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy import orm
@@ -21,12 +22,15 @@ from sqlalchemy import orm
 from keystone.common import sql
 
 
+CONF = cfg.CONF
+
+
 class User(sql.ModelBase, sql.DictBase):
     __tablename__ = 'user'
     attributes = ['id', 'name', 'domain_id', 'password', 'enabled',
                   'default_project_id']
     id = sql.Column(sql.String(64), primary_key=True)
-    enabled = sql.Column(sql.Boolean)
+    _enabled = sql.Column('enabled', sql.Boolean)
     extra = sql.Column(sql.JsonBlob())
     default_project_id = sql.Column(sql.String(64))
     local_user = orm.relationship('LocalUser', uselist=False,
@@ -42,6 +46,8 @@ class User(sql.ModelBase, sql.DictBase):
                                       lazy='subquery',
                                       cascade='all,delete-orphan',
                                       backref='user')
+    created_at = sql.Column(sql.DateTime, nullable=True)
+    last_active_at = sql.Column(sql.Date, nullable=True)
 
     # name property
     @hybrid_property
@@ -123,6 +129,33 @@ class User(sql.ModelBase, sql.DictBase):
     def domain_id(cls):
         return LocalUser.domain_id
 
+    # enabled property
+    @hybrid_property
+    def enabled(self):
+        if self._enabled:
+            max_days = (
+                CONF.security_compliance.disable_user_account_days_inactive)
+            last_active = self.last_active_at
+            if not last_active and self.created_at:
+                last_active = self.created_at.date()
+            if max_days and last_active:
+                now = datetime.datetime.utcnow().date()
+                days_inactive = (now - last_active).days
+                if days_inactive >= max_days:
+                    self._enabled = False
+        return self._enabled
+
+    @enabled.setter
+    def enabled(self, value):
+        if (value and
+                CONF.security_compliance.disable_user_account_days_inactive):
+            self.last_active_at = datetime.datetime.utcnow().date()
+        self._enabled = value
+
+    @enabled.expression
+    def enabled(cls):
+        return User._enabled
+
     def to_dict(self, include_extra_dict=False):
         d = super(User, self).to_dict(include_extra_dict=include_extra_dict)
         if 'default_project_id' in d and d['default_project_id'] is None:

keystone/identity/core.py

@@ -827,7 +827,9 @@ class Manager(manager.Manager):
         ref = driver.authenticate(entity_id, password)
         ref = self._set_domain_id_and_mapping(
             ref, domain_id, driver, mapping.EntityType.USER)
-        return self._shadow_nonlocal_user(ref)
+        ref = self._shadow_nonlocal_user(ref)
+        self.shadow_users_api.set_last_active_at(ref['id'])
+        return ref
 
     def _assert_default_project_id_is_not_domain(self, default_project_id):
         if default_project_id:
@@ -1260,6 +1262,7 @@ class Manager(manager.Manager):
             }
             user_dict = self.shadow_users_api.create_federated_user(
                 federated_dict)
+        self.shadow_users_api.set_last_active_at(user_dict['id'])
         return user_dict
 
 

keystone/identity/shadow_backends/base.py

@@ -78,7 +78,7 @@ class ShadowUsersDriverV10(ShadowUsersDriverBase):
     def get_user(self, user_id):
         """Return the found user.
 
-        :param user_id: Identity of the user
+        :param user_id: Unique identifier of the user
         :returns dict: Containing the user reference
 
         """
@@ -94,6 +94,15 @@ class ShadowUsersDriverV10(ShadowUsersDriverBase):
         """
         raise exception.NotImplemented()
 
+    @abc.abstractmethod
+    def set_last_active_at(self, user_id):
+        """Set the last active at date for the user.
+
+        :param user_id: Unique identifier of the user
+
+        """
+        raise exception.NotImplemented()
+
 
 class V10ShadowUsersWrapperForV9Driver(ShadowUsersDriverV10):
     def get_user(self, user_id):
@@ -101,3 +110,6 @@ class V10ShadowUsersWrapperForV9Driver(ShadowUsersDriverV10):
 
     def create_nonlocal_user(self, user_dict):
         return user_dict
+
+    def set_last_active_at(self, user_id):
+        pass

keystone/identity/shadow_backends/sql.py

@@ -11,8 +11,11 @@
 # under the License.
 
 import copy
+import datetime
 import uuid
 
+from oslo_config import cfg
+
 from keystone.common import sql
 from keystone import exception
 from keystone.identity.backends import base as identity_base
@@ -20,6 +23,9 @@ from keystone.identity.backends import sql_model as model
 from keystone.identity.shadow_backends import base
 
 
+CONF = cfg.CONF
+
+
 class ShadowUsers(base.ShadowUsersDriverV10):
     @sql.handle_conflicts(conflict_type='federated_user')
     def create_federated_user(self, federated_dict):
@@ -30,6 +36,7 @@ class ShadowUsers(base.ShadowUsersDriverV10):
         with sql.session_for_write() as session:
             federated_ref = model.FederatedUser.from_dict(federated_dict)
             user_ref = model.User.from_dict(user)
+            user_ref.created_at = datetime.datetime.utcnow()
             user_ref.federated_users.append(federated_ref)
             session.add(user_ref)
             return identity_base.filter_user(user_ref.to_dict())
@@ -60,6 +67,12 @@ class ShadowUsers(base.ShadowUsersDriverV10):
                 raise exception.UserNotFound(user_id=unique_id)
             return user_ref
 
+    def set_last_active_at(self, user_id):
+        if CONF.security_compliance.disable_user_account_days_inactive:
+            with sql.session_for_write() as session:
+                user_ref = session.query(model.User).get(user_id)
+                user_ref.last_active_at = datetime.datetime.utcnow().date()
+
     @sql.handle_conflicts(conflict_type='federated_user')
     def update_federated_user_display_name(self, idp_id, protocol_id,
                                            unique_id, display_name):
@@ -85,6 +98,7 @@ class ShadowUsers(base.ShadowUsersDriverV10):
             new_nonlocal_user_ref = model.NonLocalUser.from_dict(
                 new_nonlocal_user_dict)
             new_user_ref = model.User.from_dict(new_user_dict)
+            new_user_ref.created_at = datetime.datetime.utcnow()
             new_user_ref.nonlocal_users.append(new_nonlocal_user_ref)
             session.add(new_user_ref)
             return identity_base.filter_user(new_user_ref.to_dict())

keystone/tests/unit/identity/test_backend_sql.py

@@ -0,0 +1,152 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import datetime
+import uuid
+
+from oslo_config import cfg
+
+from keystone.common import sql
+from keystone.common import utils
+from keystone import exception
+from keystone.identity.backends import base
+from keystone.identity.backends import sql_model as model
+from keystone.tests.unit import test_backend_sql
+
+
+CONF = cfg.CONF
+
+
+class DisableInactiveUserTests(test_backend_sql.SqlTests):
+    def setUp(self):
+        super(DisableInactiveUserTests, self).setUp()
+        self.password = uuid.uuid4().hex
+        self.user_dict = self._get_user_dict(self.password)
+        self.max_inactive_days = 90
+        self.config_fixture.config(
+            group='security_compliance',
+            disable_user_account_days_inactive=self.max_inactive_days)
+
+    def test_authenticate_user_disabled_due_to_inactivity(self):
+        # create user and set last_active_at beyond the max
+        last_active_at = (
+            datetime.datetime.utcnow() -
+            datetime.timedelta(days=self.max_inactive_days + 1))
+        user = self._create_user(self.user_dict, last_active_at.date())
+        self.assertRaises(exception.UserDisabled,
+                          self.identity_api.authenticate,
+                          context={},
+                          user_id=user['id'],
+                          password=self.password)
+        # verify that the user is actually disabled
+        user = self.identity_api.get_user(user['id'])
+        self.assertFalse(user['enabled'])
+        # set the user to enabled and authenticate
+        user['enabled'] = True
+        self.identity_api.update_user(user['id'], user)
+        user = self.identity_api.authenticate(context={},
+                                              user_id=user['id'],
+                                              password=self.password)
+        self.assertTrue(user['enabled'])
+
+    def test_authenticate_user_not_disabled_due_to_inactivity(self):
+        # create user and set last_active_at just below the max
+        last_active_at = (
+            datetime.datetime.utcnow() -
+            datetime.timedelta(days=self.max_inactive_days - 1)).date()
+        user = self._create_user(self.user_dict, last_active_at)
+        user = self.identity_api.authenticate(context={},
+                                              user_id=user['id'],
+                                              password=self.password)
+        self.assertTrue(user['enabled'])
+
+    def test_get_user_disabled_due_to_inactivity(self):
+        user = self.identity_api.create_user(self.user_dict)
+        # set last_active_at just beyond the max
+        last_active_at = (
+            datetime.datetime.utcnow() -
+            datetime.timedelta(self.max_inactive_days + 1)).date()
+        self._update_user(user['id'], last_active_at)
+        # get user and verify that the user is actually disabled
+        user = self.identity_api.get_user(user['id'])
+        self.assertFalse(user['enabled'])
+        # set enabled and test
+        user['enabled'] = True
+        self.identity_api.update_user(user['id'], user)
+        user = self.identity_api.get_user(user['id'])
+        self.assertTrue(user['enabled'])
+
+    def test_get_user_not_disabled_due_to_inactivity(self):
+        user = self.identity_api.create_user(self.user_dict)
+        self.assertTrue(user['enabled'])
+        # set last_active_at just below the max
+        last_active_at = (
+            datetime.datetime.utcnow() -
+            datetime.timedelta(self.max_inactive_days - 1)).date()
+        self._update_user(user['id'], last_active_at)
+        # get user and verify that the user is still enabled
+        user = self.identity_api.get_user(user['id'])
+        self.assertTrue(user['enabled'])
+
+    def test_enabled_after_create_update_user(self):
+        self.config_fixture.config(group='security_compliance',
+                                   disable_user_account_days_inactive=90)
+        # create user without enabled; assert enabled
+        del self.user_dict['enabled']
+        user = self.identity_api.create_user(self.user_dict)
+        user_ref = self._get_user_ref(user['id'])
+        self.assertTrue(user_ref.enabled)
+        now = datetime.datetime.utcnow().date()
+        self.assertGreaterEqual(now, user_ref.last_active_at)
+        # set enabled and test
+        user['enabled'] = True
+        self.identity_api.update_user(user['id'], user)
+        user_ref = self._get_user_ref(user['id'])
+        self.assertTrue(user_ref.enabled)
+        # set disabled and test
+        user['enabled'] = False
+        self.identity_api.update_user(user['id'], user)
+        user_ref = self._get_user_ref(user['id'])
+        self.assertFalse(user_ref.enabled)
+        # re-enable user and test
+        user['enabled'] = True
+        self.identity_api.update_user(user['id'], user)
+        user_ref = self._get_user_ref(user['id'])
+        self.assertTrue(user_ref.enabled)
+
+    def _get_user_dict(self, password):
+        user = {
+            'name': uuid.uuid4().hex,
+            'domain_id': CONF.identity.default_domain_id,
+            'enabled': True,
+            'password': password
+        }
+        return user
+
+    def _get_user_ref(self, user_id):
+        with sql.session_for_read() as session:
+            return session.query(model.User).get(user_id)
+
+    def _create_user(self, user_dict, last_active_at):
+        user_dict['id'] = uuid.uuid4().hex
+        user_dict = utils.hash_user_password(user_dict)
+        with sql.session_for_write() as session:
+            user_ref = model.User.from_dict(user_dict)
+            user_ref.last_active_at = last_active_at
+            session.add(user_ref)
+            return base.filter_user(user_ref.to_dict())
+
+    def _update_user(self, user_id, last_active_at):
+        with sql.session_for_write() as session:
+            user_ref = session.query(model.User).get(user_id)
+            user_ref.last_active_at = last_active_at
+            return user_ref

keystone/tests/unit/identity/test_backends.py

@@ -12,6 +12,7 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import datetime
 import uuid
 
 import mock
@@ -19,8 +20,10 @@ from six.moves import range
 from testtools import matchers
 
 from keystone.common import driver_hints
+from keystone.common import sql
 import keystone.conf
 from keystone import exception
+from keystone.identity.backends import sql_model as model
 from keystone.tests import unit
 from keystone.tests.unit import default_fixtures
 from keystone.tests.unit import filtering
@@ -1385,7 +1388,7 @@ class ShadowUsersTests(object):
     def test_create_federated_user_unique_constraint(self):
         federated_dict = unit.new_federated_user_ref()
         user_dict = self.shadow_users_api.create_federated_user(federated_dict)
-        user_dict = self.identity_api.get_user(user_dict["id"])
+        user_dict = self.shadow_users_api.get_user(user_dict["id"])
         self.assertIsNotNone(user_dict["id"])
         self.assertRaises(exception.Conflict,
                           self.shadow_users_api.create_federated_user,
@@ -1419,3 +1422,28 @@ class ShadowUsersTests(object):
         self.assertEqual(user_ref.federated_users[0].display_name,
                          new_display_name)
         self.assertEqual(user_dict_create["id"], user_ref.id)
+
+    def test_set_last_active_at(self):
+        self.config_fixture.config(group='security_compliance',
+                                   disable_user_account_days_inactive=90)
+        now = datetime.datetime.utcnow().date()
+        user_ref = self.identity_api.authenticate(
+            context={},
+            user_id=self.user_sna['id'],
+            password=self.user_sna['password'])
+        user_ref = self._get_user_ref(user_ref['id'])
+        self.assertGreaterEqual(now, user_ref.last_active_at)
+
+    def test_set_last_active_at_when_config_setting_is_none(self):
+        self.config_fixture.config(group='security_compliance',
+                                   disable_user_account_days_inactive=None)
+        user_ref = self.identity_api.authenticate(
+            context={},
+            user_id=self.user_sna['id'],
+            password=self.user_sna['password'])
+        user_ref = self._get_user_ref(user_ref['id'])
+        self.assertIsNone(user_ref.last_active_at)
+
+    def _get_user_ref(self, user_id):
+        with sql.session_for_read() as session:
+            return session.query(model.User).get(user_id)

keystone/tests/unit/test_backend_sql.py

@@ -132,7 +132,9 @@ class SqlModels(SqlTests):
         cols = (('id', sql.String, 64),
                 ('default_project_id', sql.String, 64),
                 ('enabled', sql.Boolean, None),
-                ('extra', sql.JsonBlob, None))
+                ('extra', sql.JsonBlob, None),
+                ('created_at', sql.DateTime, None),
+                ('last_active_at', sqlalchemy.Date, None))
         self.assertExpectedSchema('user', cols)
 
     def test_local_user_model(self):

keystone/tests/unit/test_sql_upgrade.py

@@ -1446,6 +1446,23 @@ class SqlUpgradeTests(SqlMigrateBase):
                                           autoload=True)
         self.assertTrue(password_table.c.password.nullable)
 
+    def test_migration_107_add_user_date_columns(self):
+        user_table = 'user'
+        self.upgrade(106)
+        self.assertTableColumns(user_table,
+                                ['id',
+                                 'extra',
+                                 'enabled',
+                                 'default_project_id'])
+        self.upgrade(107)
+        self.assertTableColumns(user_table,
+                                ['id',
+                                 'extra',
+                                 'enabled',
+                                 'default_project_id',
+                                 'created_at',
+                                 'last_active_at'])
+
 
 class MySQLOpportunisticUpgradeTestCase(SqlUpgradeTests):
     FIXTURE = test_base.MySQLOpportunisticFixture

keystone/tests/unit/test_v3_identity.py

@@ -309,6 +309,16 @@ class IdentityTestCase(test_v3.RestfulTestCase):
         self.assertValidUserResponse(r, self.user)
         self.head(resource_url, expected_status=http_client.OK)
 
+    def test_get_user_does_not_include_extra_attributes(self):
+        """Call ``GET /users/{user_id}`` extra attributes are not included."""
+        user = unit.new_user_ref(domain_id=self.domain_id,
+                                 project_id=self.project_id)
+        user = self.identity_api.create_user(user)
+        self.assertNotIn('created_at', user)
+        self.assertNotIn('last_active_at', user)
+        r = self.get('/users/%(user_id)s' % {'user_id': user['id']})
+        self.assertValidUserResponse(r, user)
+
     def test_get_user_with_default_project(self):
         """Call ``GET /users/{user_id}`` making sure of default_project_id."""
         user = unit.new_user_ref(domain_id=self.domain_id,