From e3a5b61d51ffb4bdabd132e047d1a9963cdbc65a Mon Sep 17 00:00:00 2001 From: Ronald De Rose Date: Fri, 10 Jun 2016 18:24:42 +0000 Subject: [PATCH] PCI-DSS Disable inactive users requirements This patch satisfies the following PCI-DSS disable inactive user requirements: * PCI-DSS 8.1.4: Remove/disable inactive user accounts within 90 days. The number of inactive days is configurable. Partially-implements: blueprint pci-dss Change-Id: Ief0572de014f4719e001dfd558f501c99a8850dd --- keystone/common/sql/core.py | 1 + .../versions/107_add_user_date_columns.py | 30 ++++ keystone/conf/security_compliance.py | 8 +- keystone/exception.py | 4 + keystone/identity/backends/sql.py | 12 +- keystone/identity/backends/sql_model.py | 35 +++- keystone/identity/core.py | 5 +- keystone/identity/shadow_backends/base.py | 14 +- keystone/identity/shadow_backends/sql.py | 14 ++ .../tests/unit/identity/test_backend_sql.py | 152 ++++++++++++++++++ keystone/tests/unit/identity/test_backends.py | 30 +++- keystone/tests/unit/test_backend_sql.py | 4 +- keystone/tests/unit/test_sql_upgrade.py | 17 ++ keystone/tests/unit/test_v3_identity.py | 10 ++ 14 files changed, 325 insertions(+), 11 deletions(-) create mode 100644 keystone/common/sql/migrate_repo/versions/107_add_user_date_columns.py create mode 100644 keystone/tests/unit/identity/test_backend_sql.py diff --git a/keystone/common/sql/core.py b/keystone/common/sql/core.py index f9cd5ac1e0..e97af08f5b 100644 --- a/keystone/common/sql/core.py +++ b/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 diff --git a/keystone/common/sql/migrate_repo/versions/107_add_user_date_columns.py b/keystone/common/sql/migrate_repo/versions/107_add_user_date_columns.py new file mode 100644 index 0000000000..64a1d11631 --- /dev/null +++ b/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() diff --git a/keystone/conf/security_compliance.py b/keystone/conf/security_compliance.py index 344989fe79..e92c6b4074 100644 --- a/keystone/conf/security_compliance.py +++ b/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( diff --git a/keystone/exception.py b/keystone/exception.py index cb67a67e38..0d09445553 100644 --- a/keystone/exception.py +++ b/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.") diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py index f7fad29beb..cca60b6735 100644 --- a/keystone/identity/backends/sql.py +++ b/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()) diff --git a/keystone/identity/backends/sql_model.py b/keystone/identity/backends/sql_model.py index d9ee7d1716..5b2609354c 100644 --- a/keystone/identity/backends/sql_model.py +++ b/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: diff --git a/keystone/identity/core.py b/keystone/identity/core.py index e7a75bab73..a6fc25118d 100644 --- a/keystone/identity/core.py +++ b/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 diff --git a/keystone/identity/shadow_backends/base.py b/keystone/identity/shadow_backends/base.py index 9364aa24be..64b86a3a7c 100644 --- a/keystone/identity/shadow_backends/base.py +++ b/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 diff --git a/keystone/identity/shadow_backends/sql.py b/keystone/identity/shadow_backends/sql.py index 1e85998874..2a2fa3ff52 100644 --- a/keystone/identity/shadow_backends/sql.py +++ b/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()) diff --git a/keystone/tests/unit/identity/test_backend_sql.py b/keystone/tests/unit/identity/test_backend_sql.py new file mode 100644 index 0000000000..22339e9c04 --- /dev/null +++ b/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 diff --git a/keystone/tests/unit/identity/test_backends.py b/keystone/tests/unit/identity/test_backends.py index 5642d25afa..5846bbe78f 100644 --- a/keystone/tests/unit/identity/test_backends.py +++ b/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) diff --git a/keystone/tests/unit/test_backend_sql.py b/keystone/tests/unit/test_backend_sql.py index 0e9ddf8917..d9d62861dc 100644 --- a/keystone/tests/unit/test_backend_sql.py +++ b/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): diff --git a/keystone/tests/unit/test_sql_upgrade.py b/keystone/tests/unit/test_sql_upgrade.py index baf01f0b12..7dc5f0ed21 100644 --- a/keystone/tests/unit/test_sql_upgrade.py +++ b/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 diff --git a/keystone/tests/unit/test_v3_identity.py b/keystone/tests/unit/test_v3_identity.py index d46f939898..d74deef378 100644 --- a/keystone/tests/unit/test_v3_identity.py +++ b/keystone/tests/unit/test_v3_identity.py @@ -300,6 +300,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,