From 446a23d0ed2d3ffa76c5fb5e9576d6876bdbf04f Mon Sep 17 00:00:00 2001 From: Simon Pichugin Date: Fri, 28 Mar 2025 11:28:54 -0700 Subject: [PATCH] Issue 6686 - CLI - Re-enabling user accounts that reached inactivity limit fails with error (#6687) Description: When attempting to unlock a user account that has been locked due to exceeding the Account Policy Plugin's inactivity limit, the dsidm account unlock command fails with a Python type error: "float() argument must be a string or a number, not 'NoneType'". Enhance the unlock method to properly handle different account locking states, including inactivity limit exceeded states. Add test cases to verify account inactivity locking/unlocking functionality with CoS and role-based indirect locking. Fix CoS template class to include the required 'ldapsubentry' objectClass. Improv error messages to provide better guidance on unlocking indirectly locked accounts. Fixes: https://github.com/389ds/389-ds-base/issues/6686 Reviewed by: @mreynolds389 (Thanks!) --- .../clu/dsidm_account_inactivity_test.py | 329 ++++++++++++++++++ src/lib389/lib389/cli_idm/account.py | 25 +- src/lib389/lib389/idm/account.py | 28 +- 3 files changed, 377 insertions(+), 5 deletions(-) create mode 100644 dirsrvtests/tests/suites/clu/dsidm_account_inactivity_test.py diff --git a/dirsrvtests/tests/suites/clu/dsidm_account_inactivity_test.py b/dirsrvtests/tests/suites/clu/dsidm_account_inactivity_test.py new file mode 100644 index 000000000..88a34abf6 --- /dev/null +++ b/dirsrvtests/tests/suites/clu/dsidm_account_inactivity_test.py @@ -0,0 +1,329 @@ +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2025 Red Hat, Inc. +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK --- +# +import ldap +import time +import pytest +import logging +import os +from datetime import datetime, timedelta + +from lib389 import DEFAULT_SUFFIX, DN_PLUGIN, DN_CONFIG +from lib389.cli_idm.account import entry_status, unlock +from lib389.topologies import topology_st +from lib389.cli_base import FakeArgs +from lib389.utils import ds_is_older +from lib389.plugins import AccountPolicyPlugin, AccountPolicyConfigs +from lib389.idm.role import FilteredRoles +from lib389.idm.user import UserAccounts +from lib389.cos import CosTemplate, CosPointerDefinition +from lib389.idm.domain import Domain +from . import check_value_in_log_and_reset + +pytestmark = pytest.mark.tier0 + +logging.getLogger(__name__).setLevel(logging.DEBUG) +log = logging.getLogger(__name__) + +# Constants +PLUGIN_ACCT_POLICY = "Account Policy Plugin" +ACCP_DN = f"cn={PLUGIN_ACCT_POLICY},{DN_PLUGIN}" +ACCP_CONF = f"{DN_CONFIG},{ACCP_DN}" +POLICY_NAME = "Account Inactivity Policy" +POLICY_DN = f"cn={POLICY_NAME},{DEFAULT_SUFFIX}" +COS_TEMPLATE_NAME = "TemplateCoS" +COS_TEMPLATE_DN = f"cn={COS_TEMPLATE_NAME},{DEFAULT_SUFFIX}" +COS_DEFINITION_NAME = "DefinitionCoS" +COS_DEFINITION_DN = f"cn={COS_DEFINITION_NAME},{DEFAULT_SUFFIX}" +TEST_USER_NAME = "test_inactive_user" +TEST_USER_DN = f"uid={TEST_USER_NAME},{DEFAULT_SUFFIX}" +TEST_USER_PW = "password" +INACTIVITY_LIMIT = 30 + + +@pytest.fixture(scope="function") +def account_policy_setup(topology_st, request): + """Set up account policy plugin, configuration, and CoS objects""" + log.info("Setting up Account Policy Plugin and CoS") + + # Enable Account Policy Plugin + plugin = AccountPolicyPlugin(topology_st.standalone) + if not plugin.status(): + plugin.enable() + plugin.set('nsslapd-pluginarg0', ACCP_CONF) + + # Configure Account Policy + accp_configs = AccountPolicyConfigs(topology_st.standalone) + accp_config = accp_configs.ensure_state( + properties={ + 'cn': 'config', + 'alwaysrecordlogin': 'yes', + 'stateattrname': 'lastLoginTime', + 'altstateattrname': '1.1', + 'specattrname': 'acctPolicySubentry', + 'limitattrname': 'accountInactivityLimit' + } + ) + + # Add ACI for anonymous access if it doesn't exist + domain = Domain(topology_st.standalone, DEFAULT_SUFFIX) + anon_aci = '(targetattr="*")(version 3.0; acl "Anonymous read access"; allow (read,search,compare) userdn="ldap:///anyone";)' + domain.ensure_present('aci', anon_aci) + + # Restart the server to apply plugin configuration + topology_st.standalone.restart() + + # Create or update account policy entry + accp_configs = AccountPolicyConfigs(topology_st.standalone, basedn=DEFAULT_SUFFIX) + policy = accp_configs.ensure_state( + properties={ + 'cn': POLICY_NAME, + 'objectClass': ['top', 'ldapsubentry', 'extensibleObject', 'accountpolicy'], + 'accountInactivityLimit': str(INACTIVITY_LIMIT) + } + ) + + # Create or update CoS template entry + cos_template = CosTemplate(topology_st.standalone, dn=COS_TEMPLATE_DN) + cos_template.ensure_state( + properties={ + 'cn': COS_TEMPLATE_NAME, + 'objectClass': ['top', 'cosTemplate', 'extensibleObject'], + 'acctPolicySubentry': policy.dn + } + ) + + # Create or update CoS definition entry + cos_def = CosPointerDefinition(topology_st.standalone, dn=COS_DEFINITION_DN) + cos_def.ensure_state( + properties={ + 'cn': COS_DEFINITION_NAME, + 'objectClass': ['top', 'ldapsubentry', 'cosSuperDefinition', 'cosPointerDefinition'], + 'cosTemplateDn': COS_TEMPLATE_DN, + 'cosAttribute': 'acctPolicySubentry default operational-default' + } + ) + + # Restart server to ensure CoS is applied + topology_st.standalone.restart() + + def fin(): + log.info('Cleaning up Account Policy settings') + try: + # Delete CoS and policy entries + if cos_def.exists(): + cos_def.delete() + if cos_template.exists(): + cos_template.delete() + if policy.exists(): + policy.delete() + + # Disable the plugin + if plugin.status(): + plugin.disable() + topology_st.standalone.restart() + except Exception as e: + log.error(f'Failed to clean up: {e}') + + request.addfinalizer(fin) + + return topology_st.standalone + + +@pytest.fixture(scope="function") +def create_test_user(topology_st, account_policy_setup, request): + """Create a test user for the inactivity test""" + log.info('Creating test user') + + users = UserAccounts(topology_st.standalone, DEFAULT_SUFFIX) + user = users.ensure_state( + properties={ + 'uid': TEST_USER_NAME, + 'cn': TEST_USER_NAME, + 'sn': TEST_USER_NAME, + 'userPassword': TEST_USER_PW, + 'uidNumber': '1000', + 'gidNumber': '2000', + 'homeDirectory': f'/home/{TEST_USER_NAME}' + } + ) + + def fin(): + log.info('Deleting test user') + if user.exists(): + user.delete() + + request.addfinalizer(fin) + return user + + +@pytest.mark.skipif(ds_is_older("1.4.2"), reason="Indirect account locking not implemented") +def test_dsidm_account_inactivity_lock_unlock(topology_st, create_test_user): + """Test dsidm account unlock functionality with indirectly locked accounts + + :id: d7b57083-6111-4dbf-af84-6fca7fc7fb31 + :setup: Standalone instance with Account Policy Plugin and CoS configured + :steps: + 1. Create a test user + 2. Bind as the test user to set lastLoginTime + 3. Check account status - should be active + 4. Set user's lastLoginTime to a time in the past that exceeds inactivity limit + 5. Check account status - should be locked due to inactivity + 6. Attempt to bind as the user - should fail with constraint violation + 7. Unlock the account using dsidm account unlock + 8. Verify account status is active again + 9. Verify the user can bind again + :expectedresults: + 1. Success + 2. Success + 3. Account status shows as activated + 4. Success + 5. Account status shows as inactivity limit exceeded + 6. Bind attempt fails with constraint violation + 7. Account unlocked successfully + 8. Account status shows as activated + 9. User can bind successfully + """ + standalone = topology_st.standalone + user = create_test_user + + # Set up FakeArgs for dsidm commands + args = FakeArgs() + args.dn = user.dn + args.json = False + args.details = False + + # 1. Check initial account status - should be active + log.info('Step 1: Checking initial account status') + entry_status(standalone, DEFAULT_SUFFIX, topology_st.logcap.log, args) + check_value_in_log_and_reset(topology_st, check_value='Entry State: activated') + + # 2. Bind as test user to set initial lastLoginTime + log.info('Step 2: Binding as test user to set lastLoginTime') + try: + conn = user.bind(TEST_USER_PW) + conn.unbind() + log.info("Successfully bound as test user") + except ldap.LDAPError as e: + pytest.fail(f"Failed to bind as test user: {e}") + + # 3. Set lastLoginTime to a time in the past that exceeds inactivity limit + log.info('Step 3: Setting lastLoginTime to the past') + past_time = datetime.utcnow() - timedelta(seconds=INACTIVITY_LIMIT * 2) + past_time_str = past_time.strftime('%Y%m%d%H%M%SZ') + user.replace('lastLoginTime', past_time_str) + + # 4. Check account status - should now be locked due to inactivity + log.info('Step 4: Checking account status after setting old lastLoginTime') + entry_status(standalone, DEFAULT_SUFFIX, topology_st.logcap.log, args) + check_value_in_log_and_reset(topology_st, check_value='Entry State: inactivity limit exceeded') + + # 5. Attempt to bind as the user - should fail + log.info('Step 5: Attempting to bind as user (should fail)') + with pytest.raises(ldap.CONSTRAINT_VIOLATION) as excinfo: + conn = user.bind(TEST_USER_PW) + assert "Account inactivity limit exceeded" in str(excinfo.value) + + # 6. Unlock the account using dsidm account unlock + log.info('Step 6: Unlocking the account with dsidm') + unlock(standalone, DEFAULT_SUFFIX, topology_st.logcap.log, args) + check_value_in_log_and_reset(topology_st, + check_value='now unlocked by resetting lastLoginTime') + + # 7. Verify account status is active again + log.info('Step 7: Checking account status after unlock') + entry_status(standalone, DEFAULT_SUFFIX, topology_st.logcap.log, args) + check_value_in_log_and_reset(topology_st, check_value='Entry State: activated') + + # 8. Verify the user can bind again + log.info('Step 8: Verifying user can bind again') + try: + conn = user.bind(TEST_USER_PW) + conn.unbind() + log.info("Successfully bound as test user after unlock") + except ldap.LDAPError as e: + pytest.fail(f"Failed to bind as test user after unlock: {e}") + + +@pytest.mark.skipif(ds_is_older("1.4.2"), reason="Indirect account locking not implemented") +def test_dsidm_indirectly_locked_via_role(topology_st, create_test_user): + """Test dsidm account unlock functionality with accounts indirectly locked via role + + :id: 7bfe69bb-cf99-4214-a763-051ab2b9cf89 + :setup: Standalone instance with Role and user configured + :steps: + 1. Create a test user + 2. Create a Filtered Role that includes the test user + 3. Lock the role + 4. Check account status - should be indirectly locked through the role + 5. Attempt to unlock the account - should fail with appropriate message + 6. Unlock the role + 7. Verify account status is active again + :expectedresults: + 1. Success + 2. Success + 3. Success + 4. Account status shows as indirectly locked + 5. Unlock attempt fails with appropriate error message + 6. Success + 7. Account status shows as activated + """ + standalone = topology_st.standalone + user = create_test_user + + # Use FilteredRoles and ensure_state for role creation + log.info('Step 1: Creating Filtered Role') + roles = FilteredRoles(standalone, DEFAULT_SUFFIX) + role = roles.ensure_state( + properties={ + 'cn': 'TestFilterRole', + 'nsRoleFilter': f'(uid={TEST_USER_NAME})' + } + ) + + # Set up FakeArgs for dsidm commands + args = FakeArgs() + args.dn = user.dn + args.json = False + args.details = False + + # 2. Check account status before locking role + log.info('Step 2: Checking account status before locking role') + entry_status(standalone, DEFAULT_SUFFIX, topology_st.logcap.log, args) + check_value_in_log_and_reset(topology_st, check_value='Entry State: activated') + + # 3. Lock the role + log.info('Step 3: Locking the role') + role.lock() + + # 4. Check account status - should be indirectly locked + log.info('Step 4: Checking account status after locking role') + entry_status(standalone, DEFAULT_SUFFIX, topology_st.logcap.log, args) + check_value_in_log_and_reset(topology_st, check_value='Entry State: indirectly locked through a Role') + + # 5. Attempt to unlock the account - should fail + log.info('Step 5: Attempting to unlock indirectly locked account') + unlock(standalone, DEFAULT_SUFFIX, topology_st.logcap.log, args) + check_value_in_log_and_reset(topology_st, + check_value='Account is locked through role') + + # 6. Unlock the role + log.info('Step 6: Unlocking the role') + role.unlock() + + # 7. Verify account status is active again + log.info('Step 7: Checking account status after unlocking role') + entry_status(standalone, DEFAULT_SUFFIX, topology_st.logcap.log, args) + check_value_in_log_and_reset(topology_st, check_value='Entry State: activated') + + +if __name__ == '__main__': + # Run isolated + # -s for DEBUG mode + CURRENT_FILE = os.path.realpath(__file__) + pytest.main(["-s", CURRENT_FILE]) \ No newline at end of file diff --git a/src/lib389/lib389/cli_idm/account.py b/src/lib389/lib389/cli_idm/account.py index 15f766588..a0dfd8f65 100644 --- a/src/lib389/lib389/cli_idm/account.py +++ b/src/lib389/lib389/cli_idm/account.py @@ -176,8 +176,29 @@ def unlock(inst, basedn, log, args): dn = _get_dn_arg(args.dn, msg="Enter dn to unlock") accounts = Accounts(inst, basedn) acct = accounts.get(dn=dn) - acct.unlock() - log.info(f'Entry {dn} is unlocked') + + try: + # Get the account status before attempting to unlock + status = acct.status() + state = status["state"] + + # Attempt to unlock the account + acct.unlock() + + # Success message + log.info(f'Entry {dn} is unlocked') + if state == AccountState.DIRECTLY_LOCKED: + log.info(f'The entry was directly locked') + elif state == AccountState.INACTIVITY_LIMIT_EXCEEDED: + log.info(f'The entry was locked due to inactivity and is now unlocked by resetting lastLoginTime') + + except ValueError as e: + # Provide a more detailed error message based on failure reason + if "through role" in str(e): + log.error(f"Cannot unlock {dn}: {str(e)}") + log.info("To unlock this account, you must modify the role that's locking it.") + else: + log.error(f"Failed to unlock {dn}: {str(e)}") def reset_password(inst, basedn, log, args): diff --git a/src/lib389/lib389/idm/account.py b/src/lib389/lib389/idm/account.py index 4b823b662..faf6f6f16 100644 --- a/src/lib389/lib389/idm/account.py +++ b/src/lib389/lib389/idm/account.py @@ -140,7 +140,8 @@ class Account(DSLdapObject): "nsAccountLock", state_attr]) last_login_time = self._dict_get_with_ignore_indexerror(account_data, state_attr) - if not last_login_time: + # if last_login_time not exist then check alt_state_attr only if its not disabled and exist + if not last_login_time and alt_state_attr in account_data: last_login_time = self._dict_get_with_ignore_indexerror(account_data, alt_state_attr) create_time = self._dict_get_with_ignore_indexerror(account_data, "createTimestamp") @@ -203,12 +204,33 @@ class Account(DSLdapObject): self.replace('nsAccountLock', 'true') def unlock(self): - """Unset nsAccountLock""" + """Unset nsAccountLock if it's set and reset lastLoginTime if account is locked due to inactivity""" current_status = self.status() + if current_status["state"] == AccountState.ACTIVATED: raise ValueError("Account is already active") - self.remove('nsAccountLock', None) + + if current_status["state"] == AccountState.DIRECTLY_LOCKED: + # Account is directly locked with nsAccountLock attribute + self.remove('nsAccountLock', None) + elif current_status["state"] == AccountState.INACTIVITY_LIMIT_EXCEEDED: + # Account is locked due to inactivity - reset lastLoginTime to current time + # The lastLoginTime attribute stores its value in GMT/UTC time (Zulu time zone) + current_time = time.strftime('%Y%m%d%H%M%SZ', time.gmtime()) + self.replace('lastLoginTime', current_time) + elif current_status["state"] == AccountState.INDIRECTLY_LOCKED: + # Account is locked through a role + role_dn = current_status.get("role_dn") + if role_dn: + raise ValueError(f"Account is locked through role {role_dn}. " + f"Please modify the role to unlock this account.") + else: + raise ValueError("Account is locked through an unknown role. " + "Please check the roles configuration to unlock this account.") + else: + # Should not happen, but just in case + raise ValueError(f"Unknown lock state: {current_status['state'].value}") # If the account can be bound to, this will attempt to do so. We don't check # for exceptions, just pass them back! -- 2.49.0