diff --git a/keystone/cmd/cli.py b/keystone/cmd/cli.py index d3e0b8d746..849f076bd2 100644 --- a/keystone/cmd/cli.py +++ b/keystone/cmd/cli.py @@ -502,6 +502,47 @@ class DbVersion(BaseApp): print(upgrades.get_db_version()) +class ResetLastActive(BaseApp): + """Reset null values for all users to current time.""" + + name = "reset_last_active" + + @classmethod + def add_argument_parser(cls, subparsers): + parser = super().add_argument_parser(subparsers) + parser.add_argument( + '--force', + action='store_true', + help='Write to the database without asking for confirmation', + ) + return parser + + @staticmethod + def main(): + if not CONF.command.force: + confirm = input( + "Security Warning: reset_last_active will update all users\n" + "in the database with a NULL value for last_active_at to be\n" + "last active at the current time. This includes users that\n" + "have never logged in. If your Keystone deployment is\n" + "configured to use disable_user_account_days_inactive, these\n" + "users will still be enabled and won't be disabled until the\n" + "configured amount of time has passed after this command is\n" + "run.\n" + "Are you sure you want to continue [y/N]? " + ) + if confirm.lower() not in ('y', 'yes'): + raise SystemExit('reset_last_active aborted.') + + LOG.debug( + "Resetting null values to current time %s", + datetime.datetime.utcnow, + ) + drivers = backends.load_backends() + identity_api = drivers['identity_api'] + identity_api.reset_last_active() + + class BasePermissionsSetup(BaseApp): """Common user/group setup for file permissions.""" @@ -1624,6 +1665,7 @@ CMDS = [ ProjectSetup, ReceiptRotate, ReceiptSetup, + ResetLastActive, SamlIdentityProviderMetadata, TokenRotate, TokenSetup, diff --git a/keystone/identity/backends/base.py b/keystone/identity/backends/base.py index b83b332935..cc782ecaab 100644 --- a/keystone/identity/backends/base.py +++ b/keystone/identity/backends/base.py @@ -361,6 +361,16 @@ class IdentityDriverBase(metaclass=abc.ABCMeta): """ raise exception.NotImplemented() # pragma: no cover + @abc.abstractmethod + def reset_last_active(self): + """Resets null last_active_at values. + + This method looks for all users in the database that have a null + value for last_updated_at and resets that value to the current + time. + """ + raise exception.NotImplemented() # pragma: no cover + # group crud @abc.abstractmethod diff --git a/keystone/identity/backends/ldap/core.py b/keystone/identity/backends/ldap/core.py index fb92d78093..b9b9d8784d 100644 --- a/keystone/identity/backends/ldap/core.py +++ b/keystone/identity/backends/ldap/core.py @@ -170,6 +170,9 @@ class Identity(base.IdentityDriverBase): def delete_user(self, user_id): raise exception.Forbidden(READ_ONLY_LDAP_ERROR_MESSAGE) + def reset_last_active(self): + raise exception.Forbidden(READ_ONLY_LDAP_ERROR_MESSAGE) + def change_password(self, user_id, new_password): raise exception.Forbidden(READ_ONLY_LDAP_ERROR_MESSAGE) diff --git a/keystone/identity/backends/sql.py b/keystone/identity/backends/sql.py index abd32f9354..f93f351942 100644 --- a/keystone/identity/backends/sql.py +++ b/keystone/identity/backends/sql.py @@ -420,6 +420,14 @@ class Identity(base.IdentityDriverBase): session.delete(ref) + def reset_last_active(self): + with sql.session_for_write() as session: + session.query(model.User).filter( + model.User.last_active_at.is_(None).update( + {'last_active_at': datetime.datetime.utcnow()} + ) + ) + # group crud @sql.handle_conflicts(conflict_type='group') diff --git a/keystone/identity/shadow_backends/sql.py b/keystone/identity/shadow_backends/sql.py index 039794dcae..07e78d1ca7 100644 --- a/keystone/identity/shadow_backends/sql.py +++ b/keystone/identity/shadow_backends/sql.py @@ -163,11 +163,10 @@ class ShadowUsers(base.ShadowUsersDriverBase): 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.get(model.User, user_id) - if user_ref: - user_ref.last_active_at = datetime.datetime.utcnow().date() + with sql.session_for_write() as session: + user_ref = session.get(model.User, user_id) + if user_ref: + user_ref.last_active_at = datetime.datetime.utcnow().date() @sql.handle_conflicts(conflict_type='federated_user') def update_federated_user_display_name( diff --git a/keystone/tests/unit/identity/backends/fake_driver.py b/keystone/tests/unit/identity/backends/fake_driver.py index be7b2bd67e..6bebf25c99 100644 --- a/keystone/tests/unit/identity/backends/fake_driver.py +++ b/keystone/tests/unit/identity/backends/fake_driver.py @@ -72,6 +72,9 @@ class FooDriver(base.IdentityDriverBase): def get_user_by_name(self, user_name, domain_id): raise exception.NotImplemented() # pragma: no cover + def reset_last_active(self): + raise exception.NotImplemented() # pragma: no cover + def create_group(self, group_id, group): raise exception.NotImplemented() # pragma: no cover diff --git a/keystone/tests/unit/identity/shadow_users/test_backend.py b/keystone/tests/unit/identity/shadow_users/test_backend.py index dd23f22b3e..76a28c451a 100644 --- a/keystone/tests/unit/identity/shadow_users/test_backend.py +++ b/keystone/tests/unit/identity/shadow_users/test_backend.py @@ -176,6 +176,7 @@ class ShadowUsersBackendTests: group='security_compliance', disable_user_account_days_inactive=None, ) + now = datetime.datetime.utcnow().date() password = uuid.uuid4().hex user = self._create_user(password) with self.make_request(): @@ -183,7 +184,7 @@ class ShadowUsersBackendTests: user_id=user['id'], password=password ) user_ref = self._get_user_ref(user_auth['id']) - self.assertIsNone(user_ref.last_active_at) + self.assertGreaterEqual(now, user_ref.last_active_at) def _add_nonlocal_user(self, nonlocal_user): with sql.session_for_write() as session: diff --git a/releasenotes/notes/bug-2074018-28f7bbe8f28f5efe.yaml b/releasenotes/notes/bug-2074018-28f7bbe8f28f5efe.yaml new file mode 100644 index 0000000000..d5758ffbe5 --- /dev/null +++ b/releasenotes/notes/bug-2074018-28f7bbe8f28f5efe.yaml @@ -0,0 +1,29 @@ +--- +features: + - | + Added a new command to the admin cli tool: + `keystone-manage reset_last_active`. This new command updates the database + to overwritet any NULL values in `last_active_at` in the user table to the + current time. This is a necessary step to fix Bug #2074018. See launchpad + for details. +fixes: + - | + Fixed Bug #2074018: Changed the user model to always save the date of the + last user activity in `last_active_at`. Previous to this change, the + `last_active_at` field was only updated when the option for + `[security_compliance] disable_user_account_days_inactive` was set. + If your deployment is affected by this bug, you must run + `keystone-manage reset_last_active` before setting the + `disable_user_account_days_inactive` option. +security: + - | + The new `keystone-manage rest_last_active` command resets all NULL values + in `last_active_at` in the user table to help fix Bug #2074018. Running + this command may be necessary in environments that have been deployed for + a long time and later decide to adopt the + `[security_compliance disable_user_account_days_inactive = X` option. + See Bug #2074018 for details. + + A side-effect of this command is that it resets the amount of time that an + unused account is active for. Unused accounts will remain active until the + configured days have elapsed since the day the command is run.