From 15afe89930f99f92346a4b96806c4c68d76bdbae Mon Sep 17 00:00:00 2001
From: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>
Date: Wed, 30 Oct 2024 10:15:38 -0500
Subject: [PATCH] fix: non-user-friendly error when trying to install the
 worker agent as domain user (#457)

Signed-off-by: Josh Usiskin <56369778+jusiskin@users.noreply.github.com>
---
 .../installer/__init__.py                     | 11 ++++--
 .../installer/win_installer.py                | 25 ++++++++++++++
 test/unit/install/test_windows_installer.py   | 34 +++++++++++++++++++
 3 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/src/deadline_worker_agent/installer/__init__.py b/src/deadline_worker_agent/installer/__init__.py
index 97778607..22e0604c 100644
--- a/src/deadline_worker_agent/installer/__init__.py
+++ b/src/deadline_worker_agent/installer/__init__.py
@@ -12,7 +12,10 @@
 
 
 if sys.platform == "win32":
-    from deadline_worker_agent.installer.win_installer import start_windows_installer
+    from deadline_worker_agent.installer.win_installer import (
+        start_windows_installer,
+        InstallerFailedException,
+    )
 
 
 INSTALLER_PATH = {
@@ -100,7 +103,11 @@ def install() -> None:
         if args.windows_job_user:
             installer_args.update(windows_job_user=args.windows_job_user)
 
-        start_windows_installer(**installer_args)
+        try:
+            start_windows_installer(**installer_args)
+        except InstallerFailedException as e:
+            print(f"ERROR: {e}")
+            sys.exit(1)
     else:
         cmd = [
             "sudo",
diff --git a/src/deadline_worker_agent/installer/win_installer.py b/src/deadline_worker_agent/installer/win_installer.py
index b0e5be87..5ab92ab5 100644
--- a/src/deadline_worker_agent/installer/win_installer.py
+++ b/src/deadline_worker_agent/installer/win_installer.py
@@ -66,6 +66,21 @@ def print_banner():
     )
 
 
+def is_domain_user(username: str) -> bool:
+    # There are two formats for specifying domain users:
+    #
+    # 1. User Principal Name (UPN), e.g:
+    #
+    #       <USERNAME>@<DOMAIN>
+    #
+    # 2. Down-Level Logon Name, e.g:
+    #
+    #       <DOMAIN>\<USERNAME>
+    #
+    # See https://learn.microsoft.com/en-us/windows/win32/secauthn/user-name-formats
+    return "\\" in username or "@" in username
+
+
 def check_account_existence(account_name: str) -> bool:
     """
     Checks if an account exists on the system by attempting to resolve the account's SID.
@@ -846,16 +861,26 @@ def print_helping_info_and_exit():
         logging.error(f"Not a valid value for Fleet id: {fleet_id}")
         print_helping_info_and_exit()
 
+    # Validate that the --user argument is not a domain user. The installer does not currently support this.
+    if is_domain_user(user_name):
+        raise InstallerFailedException(
+            "running worker agent as a domain user is not currently supported. You can "
+            "have jobs run as a domain user by configuring the queue job run user to specify a "
+            "domain user account."
+        )
+
     # Check that user has Administrator privileges
     if not shell.IsUserAnAdmin():
         logging.error(f"User does not have Administrator privileges: {os.environ['USERNAME']}")
         print_helping_info_and_exit()
 
+    # Validate that if a windows job user override is specified, that the user exists
     if windows_job_user is not None and not check_account_existence(windows_job_user):
         raise InstallerFailedException(
             f"Account {windows_job_user} provided for argument windows-job-user does not exist. "
             "Please create the account before proceeding."
         )
+    # Validate that if a windows job user override is specified, that it is not the same as the worker agent user
     elif windows_job_user is not None and users_equal(windows_job_user, user_name):
         raise InstallerFailedException(
             f"Argument for windows-job-user cannot be the same as the worker agent user: {user_name}. "
diff --git a/test/unit/install/test_windows_installer.py b/test/unit/install/test_windows_installer.py
index 1809c2a0..8a370d63 100644
--- a/test/unit/install/test_windows_installer.py
+++ b/test/unit/install/test_windows_installer.py
@@ -121,6 +121,40 @@ def test_start_windows_installer_fails_when_windows_job_user_is_agent_user(
     )
 
 
+# Override the "user" fixture, which feeds into the parsed_kwargs fixture
+# with Valid domain username formats. For valid domain user formats, see:
+# https://learn.microsoft.com/en-us/windows/win32/secauthn/user-name-formats
+@pytest.mark.parametrize(
+    argnames="user",
+    argvalues=(
+        pytest.param("user@domain", id="user principal name"),
+        pytest.param(r"domain\username", id="down-level logon name"),
+    ),
+)
+# No job user override
+@pytest.mark.parametrize(
+    argnames="windows_job_user",
+    argvalues=(None,),
+)
+def test_start_windows_installer_fails_on_domain_worker_user(
+    parsed_kwargs: dict,
+) -> None:
+    # GIVEN
+    with (
+        patch.object(shell, "IsUserAnAdmin", return_value=True),
+        pytest.raises(win_installer.InstallerFailedException) as raise_ctx,
+    ):
+        # WHEN
+        win_installer.start_windows_installer(**parsed_kwargs)
+
+    # THEN
+    assert str(raise_ctx.value) == (
+        "running worker agent as a domain user is not currently supported. You can "
+        "have jobs run as a domain user by configuring the queue job run user to specify a "
+        "domain user account."
+    )
+
+
 class TestCreateLocalQueueUserGroup:
     """Tests for the create_local_queue_user_group function"""