diff --git a/keystone/common/controller.py b/keystone/common/controller.py index ccebe34c9c..c32c777491 100644 --- a/keystone/common/controller.py +++ b/keystone/common/controller.py @@ -305,6 +305,7 @@ class V2Controller(wsgi.Application): * v2.0 users are not domain aware, and should have domain_id removed * v2.0 users expect the use of tenantId instead of default_project_id * v2.0 users have a username attribute + * v2.0 remove password_expires_at If ref is a list type, we will iterate through each element and do the conversion. @@ -324,6 +325,7 @@ class V2Controller(wsgi.Application): def _normalize_and_filter_user_properties(ref): """Run through the various filter/normalization methods.""" _format_default_project_id(ref) + ref.pop('password_expires_at', None) V2Controller.filter_domain(ref) V2Controller.filter_domain_id(ref) V2Controller.normalize_username_in_response(ref) diff --git a/keystone/conf/security_compliance.py b/keystone/conf/security_compliance.py index 2999fe4dbb..d0afccfe66 100644 --- a/keystone/conf/security_compliance.py +++ b/keystone/conf/security_compliance.py @@ -57,12 +57,14 @@ driver`. password_expires_days = cfg.IntOpt( 'password_expires_days', - default=0, - min=0, + default=None, + min=1, help=utils.fmt(""" -The number of days which a password will be considered valid before requiring -the user to change it. Setting the value to zero (the default) disables this -feature. This feature depends on the `sql` backend for the `[identity] driver`. +The number of days for which a password will be considered valid +before requiring it to be changed. This feature is disabled by default. If +enabled, new password changes will have an expiration date, however existing +passwords would not be impacted. This feature depends on the `sql` backend for +the `[identity] driver`. """)) unique_last_password_count = cfg.IntOpt( diff --git a/keystone/exception.py b/keystone/exception.py index b29633ff95..bc41bfad3a 100644 --- a/keystone/exception.py +++ b/keystone/exception.py @@ -232,6 +232,11 @@ class Unauthorized(SecurityError): title = 'Unauthorized' +class PasswordExpired(Unauthorized): + message_format = _("The password is expired and needs to be reset by an " + "administrator for user: %(user_id)s") + + class AuthPluginException(Unauthorized): message_format = _("Authentication plugin error.") diff --git a/keystone/identity/backends/base.py b/keystone/identity/backends/base.py index 9b73546b7f..32d620f462 100644 --- a/keystone/identity/backends/base.py +++ b/keystone/identity/backends/base.py @@ -43,6 +43,8 @@ def filter_user(user_ref): except KeyError: # nosec # ok to not have extra in the user_ref. pass + if 'password_expires_at' not in user_ref: + user_ref['password_expires_at'] = None return user_ref @@ -60,6 +62,9 @@ class IdentityDriverV8(object): * the domain_id should not be returned in user / group refs. They'll be overwritten. + The password_expires_at in the user schema is a read-only attribute, + meaning that it is expected in the response, but not in the request. + User schema (if driver is domain aware):: type: object @@ -72,6 +77,8 @@ class IdentityDriverV8(object): type: string password: type: string + password_expires_at: + type: datetime enabled: type: boolean default_project_id: @@ -89,6 +96,8 @@ class IdentityDriverV8(object): type: string password: type: string + password_expires_at: + type: datetime enabled: type: boolean default_project_id: diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py index 393c3040d7..e5d11f8d13 100644 --- a/keystone/identity/backends/sql.py +++ b/keystone/identity/backends/sql.py @@ -67,6 +67,8 @@ class Identity(base.IdentityDriverV8): raise AssertionError(_('Invalid user / password')) elif not user_ref.enabled: raise exception.UserDisabled(user_id=user_id) + elif user_ref.password_is_expired: + raise exception.PasswordExpired(user_id=user_id) # successful auth, reset failed count if present if user_ref.local_user.failed_auth_count: self._reset_failed_auth(user_id) @@ -165,7 +167,7 @@ class Identity(base.IdentityDriverV8): old_user_dict[k] = user[k] new_user = model.User.from_dict(old_user_dict) for attr in model.User.attributes: - if attr != 'id': + if attr not in model.User.readonly_attributes: setattr(user_ref, attr, getattr(new_user, attr)) user_ref.extra = new_user.extra return base.filter_user( diff --git a/keystone/identity/backends/sql_model.py b/keystone/identity/backends/sql_model.py index 5ba862fece..3dcf144b48 100644 --- a/keystone/identity/backends/sql_model.py +++ b/keystone/identity/backends/sql_model.py @@ -14,21 +14,22 @@ import datetime -from oslo_config import cfg import sqlalchemy from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy import orm from keystone.common import sql +import keystone.conf -CONF = cfg.CONF +CONF = keystone.conf.CONF class User(sql.ModelBase, sql.DictBase): __tablename__ = 'user' attributes = ['id', 'name', 'domain_id', 'password', 'enabled', - 'default_project_id'] + 'default_project_id', 'password_expires_at'] + readonly_attributes = ['id', 'password_expires_at'] id = sql.Column(sql.String(64), primary_key=True) _enabled = sql.Column('enabled', sql.Boolean) extra = sql.Column(sql.JsonBlob()) @@ -91,6 +92,12 @@ class User(sql.ModelBase, sql.DictBase): return self.password_ref.expires_at return None + @hybrid_property + def password_is_expired(self): + if self.password_expires_at: + return datetime.datetime.utcnow() >= self.password_expires_at + return False + @password.setter def password(self, value): now = datetime.datetime.utcnow() @@ -103,8 +110,16 @@ class User(sql.ModelBase, sql.DictBase): new_password_ref = Password() new_password_ref.password = value new_password_ref.created_at = now + new_password_ref.expires_at = self._get_password_expires_at(now) self.local_user.passwords.append(new_password_ref) + def _get_password_expires_at(self, created_at): + expires_days = CONF.security_compliance.password_expires_days + if expires_days: + expired_date = (created_at + datetime.timedelta(days=expires_days)) + return expired_date.replace(microsecond=0) + return None + @password.expression def password(cls): return Password.password @@ -165,6 +180,24 @@ class User(sql.ModelBase, sql.DictBase): del d['default_project_id'] return d + @classmethod + def from_dict(cls, user_dict): + """Override from_dict to remove password_expires_at attribute. + + Overriding this method to remove password_expires_at attribute to + support update_user and unit tests where password_expires_at + inadvertently gets added by calling to_dict followed by from_dict. + + :param user_dict: User entity dictionary + :returns User: User object + + """ + new_dict = user_dict.copy() + password_expires_at_key = 'password_expires_at' + if password_expires_at_key in user_dict: + del new_dict[password_expires_at_key] + return super(User, cls).from_dict(new_dict) + class LocalUser(sql.ModelBase, sql.DictBase): __tablename__ = 'local_user' diff --git a/keystone/tests/unit/identity/backends/test_base.py b/keystone/tests/unit/identity/backends/test_base.py index 45e4c07249..c89a70e9f1 100644 --- a/keystone/tests/unit/identity/backends/test_base.py +++ b/keystone/tests/unit/identity/backends/test_base.py @@ -107,6 +107,7 @@ class IdentityDriverV8Tests(object): 'password': uuid.uuid4().hex, 'enabled': True, 'default_project_id': uuid.uuid4().hex, + 'password_expires_at': None } if self.driver.is_domain_aware(): user['domain_id'] = uuid.uuid4().hex diff --git a/keystone/tests/unit/identity/test_backend_sql.py b/keystone/tests/unit/identity/test_backend_sql.py index 2c6e19d429..9db80b0bb6 100644 --- a/keystone/tests/unit/identity/test_backend_sql.py +++ b/keystone/tests/unit/identity/test_backend_sql.py @@ -15,6 +15,7 @@ import uuid import freezegun +from keystone.common import controller from keystone.common import sql from keystone.common import utils import keystone.conf @@ -388,3 +389,79 @@ class LockingOutUserTests(test_backend_sql.SqlTests): self.make_request(), user_id=user_id, password=wrong_password) + + +class PasswordExpiresValidationTests(test_backend_sql.SqlTests): + def setUp(self): + super(PasswordExpiresValidationTests, self).setUp() + self.password = uuid.uuid4().hex + self.user_dict = self._get_test_user_dict(self.password) + self.config_fixture.config( + group='security_compliance', + password_expires_days=90) + + def test_authenticate_with_expired_password(self): + # set password created_at so that the password will expire + password_created_at = ( + datetime.datetime.utcnow() - + datetime.timedelta( + days=CONF.security_compliance.password_expires_days + 1) + ) + user = self._create_user(self.user_dict, password_created_at) + # test password is expired + self.assertRaises(exception.PasswordExpired, + self.identity_api.authenticate, + self.make_request(), + user_id=user['id'], + password=self.password) + + def test_authenticate_with_expired_password_v2(self): + # set password created_at so that the password will expire + password_created_at = ( + datetime.datetime.utcnow() - + datetime.timedelta( + days=CONF.security_compliance.password_expires_days + 1) + ) + user = self._create_user(self.user_dict, password_created_at) + # test password_expires_at is not returned for v2 + user = controller.V2Controller.v3_to_v2_user(user) + self.assertNotIn('password_expires_at', user) + # test password is expired + self.assertRaises(exception.PasswordExpired, + self.identity_api.authenticate, + self.make_request(), + user_id=user['id'], + password=self.password) + + def test_authenticate_with_non_expired_password(self): + # set password created_at so that the password will not expire + password_created_at = ( + datetime.datetime.utcnow() - + datetime.timedelta( + days=CONF.security_compliance.password_expires_days - 1) + ) + user = self._create_user(self.user_dict, password_created_at) + # test password is not expired + self.identity_api.authenticate(self.make_request(), + user_id=user['id'], + password=self.password) + + def _get_test_user_dict(self, password): + test_user_dict = { + 'id': uuid.uuid4().hex, + 'name': uuid.uuid4().hex, + 'domain_id': CONF.identity.default_domain_id, + 'enabled': True, + 'password': password + } + return test_user_dict + + def _create_user(self, user_dict, password_created_at): + 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.password_ref.created_at = password_created_at + user_ref.password_ref.expires_at = ( + user_ref._get_password_expires_at(password_created_at)) + session.add(user_ref) + return base.filter_user(user_ref.to_dict()) diff --git a/keystone/tests/unit/identity/test_backends.py b/keystone/tests/unit/identity/test_backends.py index 6d7103593c..30b06ee54e 100644 --- a/keystone/tests/unit/identity/test_backends.py +++ b/keystone/tests/unit/identity/test_backends.py @@ -121,6 +121,13 @@ class IdentityTests(object): self.user_foo.pop('password') self.assertDictEqual(self.user_foo, user_ref) + def test_get_user_returns_required_attributes(self): + user_ref = self.identity_api.get_user(self.user_foo['id']) + self.assertIn('id', user_ref) + self.assertIn('name', user_ref) + self.assertIn('enabled', user_ref) + self.assertIn('password_expires_at', user_ref) + @unit.skip_if_cache_disabled('identity') def test_cache_layer_get_user(self): user = unit.new_user_ref(domain_id=CONF.identity.default_domain_id) diff --git a/keystone/tests/unit/identity/test_core.py b/keystone/tests/unit/identity/test_core.py index 9eadbbd4fb..47dfcee20c 100644 --- a/keystone/tests/unit/identity/test_core.py +++ b/keystone/tests/unit/identity/test_core.py @@ -194,8 +194,9 @@ class TestShadowUsers(unit.TestCase): fed_user['display_name']) ) self.assertIsNotNone(user['id']) - self.assertEqual(4, len(user.keys())) + self.assertEqual(5, len(user.keys())) self.assertIsNotNone(user['name']) + self.assertIsNone(user['password_expires_at']) self.assertIsNone(user['domain_id']) self.assertEqual(True, user['enabled']) diff --git a/keystone/tests/unit/test_v3.py b/keystone/tests/unit/test_v3.py index 566a954336..98bc647b31 100644 --- a/keystone/tests/unit/test_v3.py +++ b/keystone/tests/unit/test_v3.py @@ -982,6 +982,7 @@ class RestfulTestCase(unit.SQLDriverOverrides, rest.RestfulTestCase, self.assertIsNotNone(entity.get('email')) self.assertIsNone(entity.get('password')) self.assertNotIn('tenantId', entity) + self.assertIn('password_expires_at', entity) if ref: self.assertEqual(ref['domain_id'], entity['domain_id']) self.assertEqual(ref['email'], entity['email']) diff --git a/keystone/tests/unit/test_v3_identity.py b/keystone/tests/unit/test_v3_identity.py index d615ea5600..7b2cee7024 100644 --- a/keystone/tests/unit/test_v3_identity.py +++ b/keystone/tests/unit/test_v3_identity.py @@ -316,6 +316,16 @@ class IdentityTestCase(test_v3.RestfulTestCase): user = self.identity_api.create_user(user) self.assertNotIn('created_at', user) self.assertNotIn('last_active_at', user) + + def test_get_user_includes_required_attributes(self): + """Call ``GET /users/{user_id}`` required attributes are included.""" + user = unit.new_user_ref(domain_id=self.domain_id, + project_id=self.project_id) + user = self.identity_api.create_user(user) + self.assertIn('id', user) + self.assertIn('name', user) + self.assertIn('enabled', user) + self.assertIn('password_expires_at', user) r = self.get('/users/%(user_id)s' % {'user_id': user['id']}) self.assertValidUserResponse(r, user) @@ -674,6 +684,13 @@ class IdentityV3toV2MethodsTestCase(unit.TestCase): name=user_id, tenantId=project_id, domain_id=CONF.identity.default_domain_id) + # User with password_expires_at + self.user5 = self.new_user_ref( + id=user_id, + name=user_id, + project_id=project_id, + domain_id=CONF.identity.default_domain_id, + password_expires_at=None) # Expected result if the user is meant to have a tenantId element self.expected_user = {'id': user_id, @@ -700,9 +717,16 @@ class IdentityV3toV2MethodsTestCase(unit.TestCase): updated_user4 = controller.V2Controller.v3_to_v2_user(self.user4) self.assertIs(self.user4, updated_user4) self.assertDictEqual(self.expected_user_no_tenant_id, self.user4) + # password_expires_at filter test + password_expires_at_key = 'password_expires_at' + self.assertIn(password_expires_at_key, self.user5) + updated_user5 = controller.V2Controller.v3_to_v2_user(self.user5) + self.assertIs(self.user5, updated_user5) + self.assertNotIn(password_expires_at_key, updated_user5) def test_v3_to_v2_user_method_list(self): - user_list = [self.user1, self.user2, self.user3, self.user4] + user_list = [self.user1, self.user2, self.user3, self.user4, + self.user5] updated_list = controller.V2Controller.v3_to_v2_user(user_list) self.assertEqual(len(user_list), len(updated_list)) @@ -710,6 +734,7 @@ class IdentityV3toV2MethodsTestCase(unit.TestCase): for i, ref in enumerate(updated_list): # Order should not change. self.assertIs(ref, user_list[i]) + self.assertNotIn('password_expires_at', user_list[i]) self.assertDictEqual(self.expected_user, self.user1) self.assertDictEqual(self.expected_user_no_tenant_id, self.user2)