From 0d8e3de5ee4ccdadffbbc365c2e822ff62efc0fd Mon Sep 17 00:00:00 2001 From: Jericho Tolentino <68654047+jericht@users.noreply.github.com> Date: Thu, 21 Mar 2024 08:52:33 -0500 Subject: [PATCH] feat(windows-installer): grant and validate agent user permissions (#206) Signed-off-by: Jericho Tolentino <68654047+jericht@users.noreply.github.com> --- .../installer/__init__.py | 14 +- .../installer/win_installer.py | 338 ++++++++++++------ test/conftest.py | 7 + .../integ/installer/test_windows_installer.py | 177 ++++++--- test/unit/install/test_windows_installer.py | 299 +++++++++++----- 5 files changed, 606 insertions(+), 229 deletions(-) diff --git a/src/deadline_worker_agent/installer/__init__.py b/src/deadline_worker_agent/installer/__init__.py index 38e7d855..e0be2d93 100644 --- a/src/deadline_worker_agent/installer/__init__.py +++ b/src/deadline_worker_agent/installer/__init__.py @@ -33,12 +33,12 @@ def install() -> None: farm_id=args.farm_id, fleet_id=args.fleet_id, region=args.region, - worker_agent_program=scripts_path, install_service=args.install_service, start_service=args.service_start, confirm=args.confirmed, allow_shutdown=args.allow_shutdown, parser=arg_parser, + grant_required_access=args.grant_required_access, ) if args.user: installer_args.update(user_name=args.user) @@ -104,6 +104,7 @@ class ParsedCommandLineArguments(Namespace): install_service: bool telemetry_opt_out: bool vfs_install_path: str + grant_required_access: bool def get_argument_parser() -> ArgumentParser: # pragma: no cover @@ -191,5 +192,16 @@ def get_argument_parser() -> ArgumentParser: # pragma: no cover required=False, default=None, ) + parser.add_argument( + "--grant-required-access", + help=( + "Allows the installer to modify an existing user so that it can successfully run the worker agent. This will allow " + "the installer to add the user to the Administrators group and grant any missing user rights which are required to " + "run the worker agent. This option has no effect if a new user is created by the installer." + ), + action="store_true", + required=False, + default=False, + ) return parser diff --git a/src/deadline_worker_agent/installer/win_installer.py b/src/deadline_worker_agent/installer/win_installer.py index 422aefed..f9c36cc6 100644 --- a/src/deadline_worker_agent/installer/win_installer.py +++ b/src/deadline_worker_agent/installer/win_installer.py @@ -18,6 +18,7 @@ import win32api import win32net import win32netcon +import win32profile import win32security import win32service import win32serviceutil @@ -77,26 +78,26 @@ def print_banner(): ) -def check_user_existence(user_name: str) -> bool: +def check_account_existence(account_name: str) -> bool: """ - Checks if a user exists on the system by attempting to resolve the user's SID. + Checks if an account exists on the system by attempting to resolve the account's SID. This method could be used in both Ad and Non-Ad environments. Args: - user_name (str): The username to check for existence. + account_name (str): The account to check for existence. Returns: - bool: True if the user exists, otherwise False. + bool: True if the account exists, otherwise False. """ MAX_RETRIES = 5 retry_count = 0 while retry_count < MAX_RETRIES: try: - # Resolve the username to an SID - sid, _, _ = win32security.LookupAccountName(None, user_name) + # Resolve the account name to an SID + sid, _, _ = win32security.LookupAccountName(None, account_name) - # Resolve the SID back to a username as an additional check + # Resolve the SID back to a account name as an additional check win32security.LookupAccountSid(None, sid) except pywintypes.error as e: if e.winerror == winerror.ERROR_NONE_MAPPED: @@ -112,41 +113,31 @@ def check_user_existence(user_name: str) -> bool: return False -def ensure_local_queue_user_group_exists(group_name: str) -> None: +def create_local_queue_user_group(group_name: str) -> None: """ - Check if a queue user group exists on the system. If it doesn't exit then create it. + Creates the local queue user group. Parameters: - group (str): The name of the group to check for existence and creation. - + group (str): The name of the group to create. """ + logging.info(f"Creating group {group_name}") try: - win32net.NetLocalGroupGetInfo(None, group_name, 1) - except pywintypes.error as e: - group_not_found = 2220 - if e.winerror == group_not_found: - logging.info(f"Creating group {group_name}") - try: - win32net.NetLocalGroupAdd( - None, - 1, - { - "name": group_name, - "comment": ( - "This is a local group created by the Deadline Cloud Worker Agent Installer. " - "This group should contain the jobRunAs OS user for all queues associated with " - "the worker's fleet" - ), - }, - ) - except Exception as e: - logging.error(f"Failed to create group {group_name}. Error: {e}") - raise - logging.info("Done creating group") - return - else: - raise - logging.info(f"Group {group_name} already exists") + win32net.NetLocalGroupAdd( + None, + 1, + { + "name": group_name, + "comment": ( + "This is a local group created by the Deadline Cloud Worker Agent Installer. " + "This group should contain the jobRunAs OS user for all queues associated with " + "the worker's fleet" + ), + }, + ) + except Exception as e: + logging.error(f"Failed to create group {group_name}. Error: {e}") + raise + logging.info("Done creating group") def validate_deadline_id(prefix: str, text: str) -> bool: @@ -165,7 +156,7 @@ def validate_deadline_id(prefix: str, text: str) -> bool: return re.match(pattern, text) is not None -def ensure_local_agent_user(username: str, password: str) -> None: +def create_local_agent_user(username: str, password: str) -> None: """ Creates a local agent user account on Windows with a specified password and sets the account to never expire. The function sets the UF_DONT_EXPIRE_PASSWD flag to ensure the account's password never expires. @@ -173,84 +164,136 @@ def ensure_local_agent_user(username: str, password: str) -> None: Args: username (str): The username of the new agent account. password (str): The password for the new agent account. Ensure it meets Windows' password policy requirements. - """ - if check_user_existence(username): - logging.info(f"Agent User {username} already exists") - # This is only to verify the credentials. It will raise a BadCredentialsError if the - # credentials cannot be used to logon the user - WindowsSessionUser(user=username, password=password) + logging.info(f"Creating Agent user {username}") + user_info = { + "name": username, + "password": password, + "priv": win32netcon.USER_PRIV_USER, # User privilege level, Standard User + "home_dir": None, + "comment": "AWS Deadline Cloud Worker Agent User", + "flags": win32netcon.UF_DONT_EXPIRE_PASSWD, + "script_path": None, + } + + try: + win32net.NetUserAdd(None, 1, user_info) + except Exception as e: + logging.error(f"Failed to create user '{username}'. Error: {e}") + raise else: - logging.info(f"Creating Agent user {username}") - user_info = { - "name": username, - "password": password, - "priv": win32netcon.USER_PRIV_USER, # User privilege level, Standard User - "home_dir": None, - "comment": "AWS Deadline Cloud Worker Agent User", - "flags": win32netcon.UF_DONT_EXPIRE_PASSWD, - "script_path": None, - } + logging.info(f"User '{username}' created successfully.") - try: - win32net.NetUserAdd(None, 1, user_info) - logging.info(f"User '{username}' created successfully.") - except Exception as e: - logging.error(f"Failed to create user '{username}'. Error: {e}") - raise + +def ensure_user_profile_exists(username: str, password: str): + """ + Ensures a user profile is created by loading it then unloading it. + + Args: + username (str): The user whose profile to load + password (str): The user's password + """ + logging.info(f"Loading user profile for '{username}'") + logon_token = None + user_profile = None + try: + # https://timgolden.me.uk/pywin32-docs/win32security__LogonUser_meth.html + logon_token = win32security.LogonUser( + Username=username, + LogonType=win32security.LOGON32_LOGON_NETWORK_CLEARTEXT, + LogonProvider=win32security.LOGON32_PROVIDER_DEFAULT, + Password=password, + Domain=None, + ) + # https://timgolden.me.uk/pywin32-docs/win32profile__LoadUserProfile_meth.html + user_profile = win32profile.LoadUserProfile( + logon_token, + { + "UserName": username, + "Flags": win32profile.PI_NOUI, + "ProfilePath": None, + }, + ) + except Exception as e: + logging.error(f"Failed to load user profile for '{username}': {e}") + raise + else: + logging.info("Successfully loaded user profile") + finally: + if user_profile is not None: + assert logon_token is not None + win32profile.UnloadUserProfile(logon_token, user_profile) + if logon_token is not None: + # Pass the handle directly as an int since logon_user returns a ctypes.HANDLE + # and not a pywin32 PyHANDLE + win32api.CloseHandle(logon_token) -def grant_account_rights(username: str, rights: list[str]): +def grant_account_rights(account_name: str, rights: list[str]): """ - Grants rights to a user account + Grants rights to an account Args: - username (str): Name of user to grant rights to + account_name (str): Name of account to grant rights to. Can be a user or a group. rights (list[str]): The rights to grant. See https://learn.microsoft.com/en-us/windows/win32/secauthz/privilege-constants. These constants are exposed by the win32security module of pywin32. """ policy_handle = None try: - user_sid, _, _ = win32security.LookupAccountName(None, username) + acc_sid, _, _ = win32security.LookupAccountName(None, account_name) policy_handle = win32security.LsaOpenPolicy(None, win32security.POLICY_ALL_ACCESS) win32security.LsaAddAccountRights( policy_handle, - user_sid, + acc_sid, rights, ) - logging.info(f"Successfully granted the following rights to {username}: {rights}") + logging.info(f"Successfully granted the following rights to {account_name}: {rights}") except Exception as e: - logging.error(f"Failed to grant user {username} rights ({rights}): {e}") + logging.error(f"Failed to grant account {account_name} rights ({rights}): {e}") raise finally: if policy_handle is not None: win32api.CloseHandle(policy_handle) +def is_user_in_group(group_name: str, user_name: str) -> bool: + """ + Checks if a user is in a group + + Args: + group_name (str): The name of the group + user_name (str): The name of the user + + Returns: + bool: True if the user is in the group, false otherwise + """ + try: + group_members_info = win32net.NetLocalGroupGetMembers(None, group_name, 1) + except Exception as e: + logging.error(f"Failed to get group members of '{group_name}': {e}") + raise + + return any(group_member["name"] == user_name for group_member in group_members_info[0]) + + def add_user_to_group(group_name: str, user_name: str) -> None: """ - Adds a specified user to a specified local group if they are not already a member. + Adds a specified user to a specified local group. Parameters: - group_name (str): The name of the local group to which the user will be added. - user_name (str): The name of the user to be added to the group. """ try: - group_members_info = win32net.NetLocalGroupGetMembers(None, group_name, 1) - group_members = [member["name"] for member in group_members_info[0]] - - if user_name not in group_members: - # The user information must be in a dictionary with 'domainandname' key - user_info = {"domainandname": user_name} - win32net.NetLocalGroupAddMembers( - None, # the local computer is used. - group_name, - 3, # Specifies the domain and name of the new local group member. - [user_info], - ) - logging.info(f"User {user_name} is added to group {group_name}.") - else: - logging.info(f"User {user_name} is already a member of group {group_name}.") + # The user information must be in a dictionary with 'domainandname' key + user_info = {"domainandname": user_name} + win32net.NetLocalGroupAddMembers( + None, # the local computer is used. + group_name, + 3, # Specifies the domain and name of the new local group member. + [user_info], + ) + logging.info(f"User {user_name} is added to group {group_name}.") except Exception as e: logging.error( f"An error occurred during adding user {user_name} to the user group {group_name}: {e}" @@ -592,11 +635,53 @@ def _start_service() -> None: logging.info(f'Successfully started service "{service_name}"') +def get_effective_user_rights(user: str) -> set[str]: + """ + Gets a set of a user's effective rights. This includes rights granted both directly + and indirectly via group membership. + + Args: + user (str): The user to get effective rights for + + Returns: + set[str]: Set of rights the user effectively has. + """ + user_sid, _, _ = win32security.LookupAccountName(None, user) + sids_to_check = [user_sid] + + # Get SIDs of all groups the user is in + # win32net.NetUserGetLocalGroups includes the LG_INCLUDE_INDIRECT flag by default + group_names = win32net.NetUserGetLocalGroups(None, user) + for group in group_names: + group_sid, _, _ = win32security.LookupAccountName(None, group) + sids_to_check.append(group_sid) + + policy_handle = win32security.LsaOpenPolicy(None, win32security.POLICY_ALL_ACCESS) + try: + effective_rights = set() + + for sid in sids_to_check: + try: + account_rights = win32security.LsaEnumerateAccountRights(policy_handle, sid) + except pywintypes.error as e: + if e.strerror == "The system cannot find the file specified.": + # Account is not directly assigned any rights + continue + else: + raise + else: + effective_rights.update(account_rights) + + return effective_rights + finally: + if policy_handle is not None: + win32api.CloseHandle(policy_handle) + + def start_windows_installer( farm_id: str, fleet_id: str, region: str, - worker_agent_program: Path, allow_shutdown: bool, parser: ArgumentParser, user_name: str = DEFAULT_WA_USER, @@ -606,6 +691,7 @@ def start_windows_installer( start_service: bool = False, confirm: bool = False, telemetry_opt_out: bool = False, + grant_required_access: bool = False, ): logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") @@ -636,7 +722,7 @@ def print_helping_info_and_exit(): print_banner() if not password: - if check_user_existence(user_name): + if check_account_existence(user_name): password = getpass("Agent user password: ") try: WindowsSessionUser(user_name, password=password) @@ -652,10 +738,9 @@ def print_helping_info_and_exit(): f"Region: {region}\n" f"Worker agent user: {user_name}\n" f"Worker job group: {group_name}\n" - f"Worker agent program path: {str(worker_agent_program)}\n" f"Allow worker agent shutdown: {allow_shutdown}\n" f"Install Windows service: {install_service}\n" - f"Start service: {start_service}" + f"Start service: {start_service}\n" f"Telemetry opt-out: {telemetry_opt_out}" ) print() @@ -672,21 +757,78 @@ def print_helping_info_and_exit(): else: logging.warning("Not a valid choice, try again") - # List of required user rights for the worker agent - worker_user_rights: list[str] = [] - + # Set of user rights to add to the worker agent user + user_rights_to_grant: set[str] = set() if allow_shutdown: - # Grant the user privilege to shutdown the machine - worker_user_rights.append(win32security.SE_SHUTDOWN_NAME) + # User right to shutdown the machine + user_rights_to_grant.add(win32security.SE_SHUTDOWN_NAME) + if install_service: + # User right to logon as a service + user_rights_to_grant.add(win32security.SE_SERVICE_LOGON_NAME) + # User right to increase memory quota for a process + user_rights_to_grant.add(win32security.SE_INCREASE_QUOTA_NAME) + # User right to replace a process-level token + user_rights_to_grant.add(win32security.SE_ASSIGNPRIMARYTOKEN_NAME) # Check if the worker agent user exists, and create it if not - ensure_local_agent_user(user_name, password) + agent_user_created = False + if check_account_existence(user_name): + logging.info(f"Using existing user ({user_name}) as worker agent user") + + # This is only to verify the credentials. It will raise a BadCredentialsError if the + # credentials cannot be used to logon the user + WindowsSessionUser(user=user_name, password=password) + else: + create_local_agent_user(user_name, password) + agent_user_created = True + + # Load the user's profile to ensure it exists + ensure_user_profile_exists(username=user_name, password=password) + + if is_user_in_group("Administrators", user_name): + logging.info(f"Agent user '{user_name}' is already an administrator") + elif not agent_user_created and not grant_required_access: + logging.error( + f"The Worker Agent user needs to run as an administrator, but the supplied user ({user_name}) exists " + "and was not found to be in the Administrators group. Please provide an administrator user, specify a " + "new username to have one created, or provide the --grant-required-access option to allow the installer " + "to make the existing user an administrator." + ) + sys.exit(1) + else: + # Add the agent user to Administrators before evaluating missing user rights + # since it will inherit the user rights that Administrators have + logging.info(f"Adding '{user_name}' to the Administrators group") + add_user_to_group(group_name="Administrators", user_name=user_name) + + # Determine which rights we need to grant + agent_user_rights = get_effective_user_rights(user_name) + user_rights_to_grant -= agent_user_rights + + # Fail if an existing user was provided but there are rights to add and the user has not explicitly opted in + if user_rights_to_grant and not agent_user_created and not grant_required_access: + logging.error( + f"The existing worker agent user ({user_name}) is missing the following required user rights: {user_rights_to_grant}\n" + "Provide the --grant-required-access option to allow the installer to grant the missing rights to the user." + ) + sys.exit(1) + + if user_rights_to_grant: + grant_account_rights(user_name, list(user_rights_to_grant)) + else: + logging.info(f"Agent user '{user_name}' has all required user rights") # Check if the job group exists, and create it if not - ensure_local_queue_user_group_exists(group_name) + if check_account_existence(group_name): + logging.info(f"Using existing group ({group_name}) as the queue user group.") + else: + create_local_queue_user_group(group_name) - # Add the worker agent user to the job group - add_user_to_group(group_name, user_name) + if is_user_in_group(group_name, user_name): + logging.info(f"Agent user '{user_name}' is already in group '{group_name}'") + else: + # Add the worker agent user to the job group + add_user_to_group(group_name, user_name) # Create directories and configure their permissions agent_dirs = provision_directories(user_name) @@ -699,10 +841,6 @@ def print_helping_info_and_exit(): shutdown_on_stop=allow_shutdown, ) - if worker_user_rights: - # Grant the worker user the necessary rights - grant_account_rights(user_name, worker_user_rights) - if telemetry_opt_out: logging.info("Opting out of client telemetry") update_deadline_client_config( diff --git a/test/conftest.py b/test/conftest.py index 5096ae44..8bd3a867 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -78,6 +78,11 @@ def vfs_install_path() -> str: return VFS_DEFAULT_INSTALL_PATH +@pytest.fixture +def grant_required_access() -> bool: + return True + + @pytest.fixture def parsed_args( farm_id: str, @@ -92,6 +97,7 @@ def parsed_args( install_service: bool, telemetry_opt_out: bool, vfs_install_path: str, + grant_required_access: bool, ) -> ParsedCommandLineArguments: parsed_args = ParsedCommandLineArguments() parsed_args.farm_id = farm_id @@ -106,6 +112,7 @@ def parsed_args( parsed_args.install_service = install_service parsed_args.telemetry_opt_out = telemetry_opt_out parsed_args.vfs_install_path = vfs_install_path + parsed_args.grant_required_access = grant_required_access return parsed_args diff --git a/test/integ/installer/test_windows_installer.py b/test/integ/installer/test_windows_installer.py index 988fc682..4e831d7a 100644 --- a/test/integ/installer/test_windows_installer.py +++ b/test/integ/installer/test_windows_installer.py @@ -14,18 +14,22 @@ except ImportError: pytest.skip("win32api not available", allow_module_level=True) import win32net +import win32security import deadline.client.config.config_file -import deadline_worker_agent.installer.win_installer as installer_mod +from deadline_worker_agent.installer import win_installer from deadline_worker_agent.installer.win_installer import ( add_user_to_group, - check_user_existence, + check_account_existence, update_config_file, - ensure_local_agent_user, - ensure_local_queue_user_group_exists, + create_local_agent_user, + create_local_queue_user_group, generate_password, + get_effective_user_rights, + grant_account_rights, provision_directories, update_deadline_client_config, + is_user_in_group, WorkerAgentDirectories, ) @@ -35,12 +39,12 @@ def test_user_existence(): current_user = win32api.GetUserNameEx(win32api.NameSamCompatible) - result = check_user_existence(current_user) + result = check_account_existence(current_user) assert result def test_user_existence_with_without_existing_user(): - result = check_user_existence("ImpossibleUser") + result = check_account_existence("ImpossibleUser") assert not result @@ -63,79 +67,90 @@ def check_admin_privilege_and_skip_test(): if env_var_value.lower() != "true": pytest.skip( "Skipping all tests required Admin permission because RUN_AS_ADMIN is not set or false", - allow_module_level=True, ) @pytest.fixture -def user_setup_and_teardown(): +def windows_user_password(): + return generate_password() + + +@pytest.fixture +def windows_user(windows_user_password): """ Pytest fixture to create a user before the test and ensure it is deleted after the test. """ check_admin_privilege_and_skip_test() username = "InstallerTestUser" - ensure_local_agent_user(username, generate_password()) + create_local_agent_user(username, windows_user_password) yield username delete_local_user(username) -def test_ensure_local_agent_user(user_setup_and_teardown): +def test_create_local_agent_user(windows_user): """ Tests the creation of a local user and validates it exists. """ - assert check_user_existence(user_setup_and_teardown) + assert check_account_existence(windows_user) -def group_exists(group_name: str) -> bool: - """ - Check if a local group exists. - """ - try: - win32net.NetLocalGroupGetInfo(None, group_name, 1) - return True - except win32net.error: - return False +def test_ensure_user_profile_exists(windows_user, windows_user_password): + # WHEN + win_installer.ensure_user_profile_exists(windows_user, windows_user_password) + + # THEN + # Verify user profile was created by checking that the home directory exists + assert pathlib.Path(f"~{windows_user}").expanduser().exists() def delete_group(group_name: str) -> None: """ Delete a local group if it exists. """ - if group_exists(group_name): + if check_account_existence(group_name): win32net.NetLocalGroupDel(None, group_name) -def is_user_in_group(group_name, username): - group_members_info = win32net.NetLocalGroupGetMembers(None, group_name, 1) - group_members = [member["name"] for member in group_members_info[0]] - return username in group_members - - @pytest.fixture -def setup_and_teardown_group(): +def windows_group(): check_admin_privilege_and_skip_test() group_name = "user_group_for_agent_testing_only" - # Ensure the group does not exist before the test - delete_group(group_name) + win32net.NetLocalGroupAdd(None, 1, {"name": group_name}) yield group_name # This value will be used in the test function # Cleanup after test execution delete_group(group_name) -def test_ensure_local_group_exists(setup_and_teardown_group): - group_name = setup_and_teardown_group +def test_create_local_queue_user_group(): + group_name = "test_create_local_queue_user_group" # Ensure the group does not exist initially - assert not group_exists(group_name), "Group already exists before test." - ensure_local_queue_user_group_exists(group_name) - assert group_exists(group_name), "Group was not created as expected." + assert not check_account_existence( + group_name + ), f"Group '{group_name}' already exists before test." + try: + create_local_queue_user_group(group_name) + assert check_account_existence( + group_name + ), f"Group '{group_name}' was not created as expected." + finally: + delete_group(group_name) -def test_add_user_to_group(setup_and_teardown_group, user_setup_and_teardown): - group_name = setup_and_teardown_group - ensure_local_queue_user_group_exists(group_name) - user_name = user_setup_and_teardown - add_user_to_group(group_name, user_name) - assert is_user_in_group(group_name, user_name), "User was not added to group as expected." + +def test_is_user_in_group(windows_user, windows_group): + # GIVEN + assert not is_user_in_group( + windows_group, windows_user + ), f"User '{windows_user}' is already in group '{windows_group}'" + win32net.NetLocalGroupAddMembers(None, windows_group, 3, [{"domainandname": windows_user}]) + + # WHEN/THEN + assert is_user_in_group(windows_group, windows_user) + + +def test_add_user_to_group(windows_group, windows_user): + add_user_to_group(windows_group, windows_user) + assert is_user_in_group(windows_group, windows_user), "User was not added to group as expected." @pytest.fixture @@ -197,7 +212,7 @@ def test_update_config_file_creates_backup(setup_example_config): def test_provision_directories( - user_setup_and_teardown: str, + windows_user: str, tmp_path: pathlib.Path, ): # GIVEN @@ -223,8 +238,8 @@ def test_provision_directories( ), f"Cannot test provision_directories because {expected_dirs.deadline_config_subdir} already exists" # WHEN - with patch.dict(installer_mod.os.environ, {"PROGRAMDATA": str(root_dir)}): - actual_dirs = provision_directories(user_setup_and_teardown) + with patch.dict(win_installer.os.environ, {"PROGRAMDATA": str(root_dir)}): + actual_dirs = provision_directories(windows_user) # THEN assert actual_dirs == expected_dirs @@ -251,3 +266,77 @@ def test_update_deadline_client_config(tmp_path: pathlib.Path) -> None: # THEN assert deadline.client.config.config_file.get_setting("telemetry.opt_out") == "true" + + +def test_grant_account_rights(windows_user: str): + # GIVEN + rights = ["SeCreateSymbolicLinkPrivilege"] + + # WHEN + grant_account_rights(windows_user, rights) + + # THEN + user_sid, _, _ = win32security.LookupAccountName(None, windows_user) + policy_handle = win32security.LsaOpenPolicy(None, win32security.POLICY_ALL_ACCESS) + try: + actual_rights = win32security.LsaEnumerateAccountRights(policy_handle, user_sid) + finally: + if policy_handle is not None: + win32api.CloseHandle(policy_handle) + + assert set(rights).issubset(set(actual_rights)) + + +def test_get_effective_user_rights( + windows_user: str, + windows_group: str, +) -> None: + try: + # GIVEN + add_user_to_group( + group_name=windows_group, + user_name=windows_user, + ) + grant_account_rights( + account_name=windows_user, + rights=[win32security.SE_BACKUP_NAME], + ) + grant_account_rights( + account_name=windows_group, + rights=[win32security.SE_RESTORE_NAME], + ) + + # WHEN + effective_rights = get_effective_user_rights(windows_user) + + # THEN + assert effective_rights == set( + [ + win32security.SE_BACKUP_NAME, + win32security.SE_RESTORE_NAME, + ] + ) + finally: + # Clean up the added rights since they stick around in Local Security Policy + # even after the user and group have been deleted + policy_handle = win32security.LsaOpenPolicy(None, win32security.POLICY_ALL_ACCESS) + try: + # Remove backup right from user + user_sid, _, _ = win32security.LookupAccountName(None, windows_user) + win32security.LsaRemoveAccountRights( + policy_handle, + user_sid, + AllRights=False, + UserRights=[win32security.SE_BACKUP_NAME], + ) + + # Remove restore right from group + group_sid, _, _ = win32security.LookupAccountName(None, windows_group) + win32security.LsaRemoveAccountRights( + policy_handle, + group_sid, + AllRights=False, + UserRights=[win32security.SE_RESTORE_NAME], + ) + finally: + win32api.CloseHandle(policy_handle) diff --git a/test/unit/install/test_windows_installer.py b/test/unit/install/test_windows_installer.py index 3520542f..919bf23a 100644 --- a/test/unit/install/test_windows_installer.py +++ b/test/unit/install/test_windows_installer.py @@ -2,36 +2,26 @@ import string import sys -import sysconfig -from pathlib import Path -from typing import Generator -from unittest.mock import Mock, call, patch, MagicMock +import typing +from unittest.mock import Mock, call, patch, MagicMock, ANY import pytest if sys.platform != "win32": pytest.skip("Windows-specific tests", allow_module_level=True) -from deadline_worker_agent.installer.win_installer import ( - ensure_local_queue_user_group_exists, - ensure_local_agent_user, - generate_password, - start_windows_installer, - validate_deadline_id, -) +from deadline_worker_agent import installer as installer_mod +from deadline_worker_agent.installer import ParsedCommandLineArguments, win_installer, install +from deadline_worker_agent.windows.win_service import WorkerAgentWindowsService -from pywintypes import error as PyWinTypesError -from win32comext.shell import shell -from win32service import error as win_service_error -from win32serviceutil import GetServiceClassString import win32netcon +import win32profile +import win32security import win32service import winerror - -from deadline_worker_agent import installer as installer_mod -from deadline_worker_agent.installer import ParsedCommandLineArguments, install -from deadline_worker_agent.installer import win_installer -from deadline_worker_agent.windows.win_service import WorkerAgentWindowsService +from win32comext.shell import shell +from win32service import error as win_service_error +from win32serviceutil import GetServiceClassString def test_start_windows_installer( @@ -51,7 +41,6 @@ def test_start_windows_installer( farm_id=parsed_args.farm_id, fleet_id=parsed_args.fleet_id, region=parsed_args.region, - worker_agent_program=Path(sysconfig.get_path("scripts")), install_service=parsed_args.install_service, start_service=parsed_args.service_start, confirm=parsed_args.confirmed, @@ -61,6 +50,7 @@ def test_start_windows_installer( password=parsed_args.password, allow_shutdown=parsed_args.allow_shutdown, telemetry_opt_out=parsed_args.telemetry_opt_out, + grant_required_access=parsed_args.grant_required_access, ) @@ -74,11 +64,10 @@ def test_start_windows_installer_fails_when_run_as_non_admin_user( with (patch.object(installer_mod, "get_argument_parser") as mock_get_arg_parser,): with pytest.raises(SystemExit): # WHEN - start_windows_installer( + win_installer.start_windows_installer( farm_id=parsed_args.farm_id, fleet_id=parsed_args.fleet_id, region=parsed_args.region, - worker_agent_program=Path(sysconfig.get_path("scripts")), install_service=parsed_args.install_service, start_service=parsed_args.service_start, confirm=parsed_args.confirmed, @@ -93,75 +82,104 @@ def test_start_windows_installer_fails_when_run_as_non_admin_user( is_user_an_admin.assert_called_once() -class MockPyWinTypesError(PyWinTypesError): - def __init__(self, winerror): - self.winerror = winerror +class TestCreateLocalQueueUserGroup: + """Tests for the create_local_queue_user_group function""" + @pytest.fixture + def group_name(self) -> str: + return "test_group" -@pytest.fixture -def group_name(): - return "test_group" + @pytest.fixture(autouse=True) + def mock_NetLocalGroupAdd(self) -> typing.Generator[MagicMock, None, None]: + with patch.object(win_installer.win32net, "NetLocalGroupAdd") as m: + yield m + def test_creates_group(self, group_name: str, mock_NetLocalGroupAdd: MagicMock): + # WHEN + win_installer.create_local_queue_user_group(group_name) -def test_group_creation_failure(group_name): - with patch("win32net.NetLocalGroupGetInfo", side_effect=MockPyWinTypesError(2220)), patch( - "win32net.NetLocalGroupAdd", side_effect=Exception("Test Failure") - ), patch("logging.error") as mock_log_error: - with pytest.raises(Exception): - ensure_local_queue_user_group_exists(group_name) - mock_log_error.assert_called_with( - f"Failed to create group {group_name}. Error: Test Failure" + # THEN + mock_NetLocalGroupAdd.assert_called_once_with( + None, + 1, + { + "name": group_name, + "comment": ANY, + }, ) + def test_raises_on_group_creation_failure( + self, + group_name: str, + mock_NetLocalGroupAdd: MagicMock, + caplog: pytest.LogCaptureFixture, + ): + # GIVEN + mock_NetLocalGroupAdd.side_effect = Exception("Test Failure") + + with pytest.raises(Exception) as raised_exc: + # WHEN + win_installer.create_local_queue_user_group(group_name) -def test_unexpected_error_code_handling(group_name): - with patch("win32net.NetLocalGroupGetInfo", side_effect=MockPyWinTypesError(9999)), patch( - "win32net.NetLocalGroupAdd" - ) as mock_group_add, patch("logging.error"): - with pytest.raises(PyWinTypesError): - ensure_local_queue_user_group_exists(group_name) - mock_group_add.assert_not_called() + # THEN + assert raised_exc.value is mock_NetLocalGroupAdd.side_effect + assert f"Failed to create group {group_name}. Error: Test Failure" in caplog.text -def test_ensure_local_agent_user_raises_exception_on_creation_failure(): - username = "testuser" - password = "password123" - error_message = "System error" - with patch( - "deadline_worker_agent.installer.win_installer.check_user_existence", return_value=False - ), patch("win32net.NetUserAdd") as mocked_net_user_add, patch( - "deadline_worker_agent.installer.win_installer.logging.error" - ) as mocked_logging_error: - mocked_net_user_add.side_effect = Exception(error_message) +class TestCreateLocalAgentUser: + """Tests for the create_local_agent_user function""" - with pytest.raises(Exception): - ensure_local_agent_user(username, password) + @pytest.fixture + def username(self) -> str: + return "testuser" - mocked_logging_error.assert_called_once_with( - f"Failed to create user '{username}'. Error: {error_message}" - ) + @pytest.fixture + def password(self) -> str: + return "password123" + @pytest.fixture(autouse=True) + def mock_NetUserAdd(self) -> typing.Generator[MagicMock, None, None]: + with patch.object(win_installer.win32net, "NetUserAdd") as m: + yield m -@patch("win32net.NetUserAdd") -def test_ensure_local_agent_user_logs_info_if_user_exists(mock_net_user_add: MagicMock): - username = "existinguser" - password = "password123" - with patch( - "deadline_worker_agent.installer.win_installer.check_user_existence", return_value=True - ), patch("deadline_worker_agent.installer.win_installer.logging.info") as mocked_logging_info: - ensure_local_agent_user(username, password) - mock_net_user_add.assert_not_called() - mocked_logging_info.assert_called_once_with(f"Agent User {username} already exists") + def test_raises_on_creation_failure( + self, + username: str, + password: str, + mock_NetUserAdd: MagicMock, + ): + # GIVEN + error_message = "System error" + mock_NetUserAdd.side_effect = Exception(error_message) + with ( + patch( + "deadline_worker_agent.installer.win_installer.check_account_existence", + return_value=False, + ), + patch( + "deadline_worker_agent.installer.win_installer.logging.error" + ) as mocked_logging_error, + pytest.raises(Exception) as raised_exc, + ): + # WHEN + win_installer.create_local_agent_user(username, password) + # THEN + assert raised_exc.value is mock_NetUserAdd.side_effect + mocked_logging_error.assert_called_once_with( + f"Failed to create user '{username}'. Error: {error_message}" + ) -def test_ensure_local_agent_user_correct_parameters_passed_to_netuseradd(): - username = "newuser" - password = "password123" - with patch( - "deadline_worker_agent.installer.win_installer.check_user_existence", return_value=False - ), patch("win32net.NetUserAdd") as mocked_net_user_add: - ensure_local_agent_user(username, password) + def test_creates_user( + self, + username: str, + password: str, + mock_NetUserAdd: MagicMock, + ): + # WHEN + win_installer.create_local_agent_user(username, password) + # THEN expected_user_info = { "name": username, "password": password, @@ -171,8 +189,117 @@ def test_ensure_local_agent_user_correct_parameters_passed_to_netuseradd(): "flags": win32netcon.UF_DONT_EXPIRE_PASSWD, "script_path": None, } + mock_NetUserAdd.assert_called_once_with(None, 1, expected_user_info) + + +class TestEnsureUserProfileExists: + """Tests for ensure_user_profile_exists function""" + + @pytest.fixture + def username(self) -> str: + return "testuser" + + @pytest.fixture + def password(self) -> str: + return "password123" + + @pytest.fixture(autouse=True) + def mock_LogonUser(self) -> typing.Generator[MagicMock, None, None]: + with patch.object(win_installer.win32security, "LogonUser") as m: + yield m + + @pytest.fixture(autouse=True) + def mock_LoadUserProfile(self) -> typing.Generator[MagicMock, None, None]: + with patch.object(win_installer.win32profile, "LoadUserProfile") as m: + yield m + + @pytest.fixture(autouse=True) + def mock_UnloadUserProfile(self) -> typing.Generator[MagicMock, None, None]: + with patch.object(win_installer.win32profile, "UnloadUserProfile") as m: + yield m + + @pytest.fixture(autouse=True) + def mock_CloseHandle(self) -> typing.Generator[MagicMock, None, None]: + with patch.object(win_installer.win32api, "CloseHandle") as m: + yield m + + def test_loads_user_profile( + self, + username: str, + password: str, + mock_LogonUser: MagicMock, + mock_LoadUserProfile: MagicMock, + mock_UnloadUserProfile: MagicMock, + mock_CloseHandle: MagicMock, + ): + # WHEN + win_installer.ensure_user_profile_exists(username, password) + + # THEN + mock_LogonUser.assert_called_once_with( + Username=username, + LogonType=win32security.LOGON32_LOGON_NETWORK_CLEARTEXT, + LogonProvider=win32security.LOGON32_PROVIDER_DEFAULT, + Password=password, + Domain=None, + ) + mock_LoadUserProfile.assert_called_once_with( + mock_LogonUser.return_value, + { + "UserName": username, + "Flags": win32profile.PI_NOUI, + "ProfilePath": None, + }, + ) + mock_UnloadUserProfile.assert_called_once_with( + mock_LogonUser.return_value, mock_LoadUserProfile.return_value + ) + mock_CloseHandle.assert_called_once_with(mock_LogonUser.return_value) + + def test_raises_on_logon_user_failure( + self, + username: str, + password: str, + mock_LogonUser: MagicMock, + mock_UnloadUserProfile: MagicMock, + mock_CloseHandle: MagicMock, + caplog: pytest.LogCaptureFixture, + ): + # GIVEN + mock_LogonUser.side_effect = Exception("Failed") + + with pytest.raises(Exception) as raised_exc: + # WHEN + win_installer.ensure_user_profile_exists(username, password) - mocked_net_user_add.assert_called_once_with(None, 1, expected_user_info) + # THEN + assert raised_exc.value is mock_LogonUser.side_effect + assert f"Failed to load user profile for '{username}'" in caplog.text + mock_UnloadUserProfile.assert_not_called() + mock_CloseHandle.assert_not_called() + + def test_raises_on_load_user_profile_failure( + self, + username: str, + password: str, + mock_LogonUser: MagicMock, + mock_LoadUserProfile: MagicMock, + mock_UnloadUserProfile: MagicMock, + mock_CloseHandle: MagicMock, + caplog: pytest.LogCaptureFixture, + ): + # GIVEN + mock_LoadUserProfile.side_effect = Exception("Failed") + + with pytest.raises(Exception) as raised_exc: + # WHEN + win_installer.ensure_user_profile_exists(username, password) + + # THEN + assert raised_exc.value is mock_LoadUserProfile.side_effect + assert f"Failed to load user profile for '{username}'" in caplog.text + mock_UnloadUserProfile.assert_not_called() + mock_CloseHandle.assert_called_once_with(mock_LogonUser.return_value) @patch("deadline_worker_agent.installer.win_installer.secrets.choice") @@ -183,7 +310,7 @@ def test_generate_password(mock_choice): mock_choice.side_effect = characters # When - password = generate_password(password_length) + password = win_installer.generate_password(password_length) # Then expected_password = "".join(characters) @@ -191,22 +318,26 @@ def test_generate_password(mock_choice): def test_validate_deadline_id(): - assert validate_deadline_id("deadline", "deadline-123e4567e89b12d3a456426655441234") + assert win_installer.validate_deadline_id( + "deadline", "deadline-123e4567e89b12d3a456426655441234" + ) def test_non_valid_deadline_id1(): - assert not validate_deadline_id("deadline", "deadline-123") + assert not win_installer.validate_deadline_id("deadline", "deadline-123") def test_non_valid_deadline_id_with_wrong_prefix(): - assert not validate_deadline_id("deadline", "line-123e4567e89b12d3a456426655441234") + assert not win_installer.validate_deadline_id( + "deadline", "line-123e4567e89b12d3a456426655441234" + ) class TestInstallService: """Test cases for the install_service() function""" @pytest.fixture(autouse=True) - def mock_configure_service_failure_actions(self) -> Generator[Mock, None, None]: + def mock_configure_service_failure_actions(self) -> typing.Generator[Mock, None, None]: with patch.object( win_installer, "configure_service_failure_actions", new_callable=Mock ) as mock_configure_service_failure_actions: @@ -432,7 +563,7 @@ class TestConfigureServiceFailureActions: """Test cases for configure_service_failure_actions()""" @pytest.fixture(autouse=True) - def mock_win32_service(self) -> Generator[Mock, None, None]: + def mock_win32_service(self) -> typing.Generator[Mock, None, None]: with patch.object(win_installer, "win32service", new_callable=Mock) as mock_win32_service: yield mock_win32_service @@ -453,7 +584,7 @@ def mock_change_service_config2(self, mock_win32_service: Mock) -> Mock: return mock_win32_service.ChangeServiceConfig2 @pytest.fixture - def mock_logging_debug(self) -> Generator[Mock, None, None]: + def mock_logging_debug(self) -> typing.Generator[Mock, None, None]: with patch.object(win_installer.logging, "debug") as mock_logging_debug: yield mock_logging_debug