From 444fb90f933be3bcfb8ec315897a8cc30d981084 Mon Sep 17 00:00:00 2001
From: Mike Salvatore <mike.s.salvatore@gmail.com>
Date: Fri, 17 Sep 2021 11:30:32 -0400
Subject: [PATCH] Agent: Return single AuthOptions from get_auth_options()

The test suite was overly complicated for get_auth_options(), which
indicated that, perhaps, the function itself was overly complicated.
Previously, it accepted a list of Credentials and returned a list of
AuthOptions. Now, it accepts a single Credentials object and returns a
single AuthOptions object. This simpler interface allowed the test suite
to be easier to read, while adding negligible complexity to
PowerShellExploiter._exploit_host()
---
 monkey/infection_monkey/exploit/powershell.py |  4 +-
 .../exploit/powershell_utils/auth_options.py  | 18 ++----
 .../powershell_utils/test_auth_options.py     | 56 +++++++++----------
 3 files changed, 35 insertions(+), 43 deletions(-)

diff --git a/monkey/infection_monkey/exploit/powershell.py b/monkey/infection_monkey/exploit/powershell.py
index 7b4aaec66..f2883bb63 100644
--- a/monkey/infection_monkey/exploit/powershell.py
+++ b/monkey/infection_monkey/exploit/powershell.py
@@ -47,7 +47,7 @@ class PowerShellExploiter(HostExploiter):
 
     def _exploit_host(self):
         try:
-            is_https = self._is_client_using_https()
+            use_ssl = self._is_client_using_https()
         except PowerShellRemotingDisabledError as e:
             logging.info(e)
             return False
@@ -59,7 +59,7 @@ class PowerShellExploiter(HostExploiter):
             self._config.exploit_ntlm_hash_list,
             is_windows_os(),
         )
-        auth_options = get_auth_options(credentials, is_https)
+        auth_options = [get_auth_options(creds, use_ssl) for creds in credentials]
 
         self._client = self._authenticate_via_brute_force(credentials, auth_options)
         if not self._client:
diff --git a/monkey/infection_monkey/exploit/powershell_utils/auth_options.py b/monkey/infection_monkey/exploit/powershell_utils/auth_options.py
index ad770de3c..973c7ec76 100644
--- a/monkey/infection_monkey/exploit/powershell_utils/auth_options.py
+++ b/monkey/infection_monkey/exploit/powershell_utils/auth_options.py
@@ -1,5 +1,4 @@
 from dataclasses import dataclass
-from typing import List
 
 from infection_monkey.exploit.powershell_utils.credentials import Credentials
 
@@ -16,15 +15,10 @@ class AuthOptions:
     ssl: bool
 
 
-def get_auth_options(credentials: List[Credentials], use_ssl: bool) -> List[AuthOptions]:
-    auth_options = []
+def get_auth_options(credentials: Credentials, use_ssl: bool) -> AuthOptions:
+    # Passwordless login only works with SSL false, AUTH_BASIC and ENCRYPTION_NEVER
+    ssl = False if credentials.secret == "" else use_ssl
+    auth_type = AUTH_BASIC if credentials.secret == "" else AUTH_NEGOTIATE
+    encryption = ENCRYPTION_NEVER if credentials.secret == "" else ENCRYPTION_AUTO
 
-    for creds in credentials:
-        # Passwordless login only works with SSL false, AUTH_BASIC and ENCRYPTION_NEVER
-        ssl = False if creds.secret == "" else use_ssl
-        auth_type = AUTH_BASIC if creds.secret == "" else AUTH_NEGOTIATE
-        encryption = ENCRYPTION_NEVER if creds.secret == "" else ENCRYPTION_AUTO
-
-        auth_options.append(AuthOptions(auth_type, encryption, ssl))
-
-    return auth_options
+    return AuthOptions(auth_type, encryption, ssl)
diff --git a/monkey/tests/unit_tests/infection_monkey/exploit/powershell_utils/test_auth_options.py b/monkey/tests/unit_tests/infection_monkey/exploit/powershell_utils/test_auth_options.py
index d19fffbd0..0aa770ea6 100644
--- a/monkey/tests/unit_tests/infection_monkey/exploit/powershell_utils/test_auth_options.py
+++ b/monkey/tests/unit_tests/infection_monkey/exploit/powershell_utils/test_auth_options.py
@@ -8,80 +8,78 @@ from infection_monkey.exploit.powershell_utils.auth_options import (
 )
 from infection_monkey.exploit.powershell_utils.credentials import Credentials, SecretType
 
-CREDENTIALS = [
-    Credentials("user1", "password1", SecretType.PASSWORD),
-    Credentials("user2", "", SecretType.PASSWORD),
-    Credentials("user3", None, SecretType.CACHED),
-]
+CREDENTIALS_WITH_PASSWORD = Credentials("user1", "password1", SecretType.PASSWORD)
+CREDENTIALS_EMPTY_PASSWORD = Credentials("user2", "", SecretType.PASSWORD)
+CREDENTIALS_NONE_PASSWORD = Credentials("user3", None, SecretType.CACHED)
 
 
 def test_get_auth_options__ssl_true_with_password():
-    auth_options = get_auth_options(CREDENTIALS, use_ssl=True)
+    auth_options = get_auth_options(CREDENTIALS_WITH_PASSWORD, use_ssl=True)
 
-    assert auth_options[0].ssl
+    assert auth_options.ssl
 
 
 def test_get_auth_options__ssl_true_empty_password():
-    auth_options = get_auth_options(CREDENTIALS, use_ssl=True)
+    auth_options = get_auth_options(CREDENTIALS_EMPTY_PASSWORD, use_ssl=True)
 
-    assert not auth_options[1].ssl
+    assert not auth_options.ssl
 
 
 def test_get_auth_options__ssl_true_none_password():
-    auth_options = get_auth_options(CREDENTIALS, use_ssl=True)
+    auth_options = get_auth_options(CREDENTIALS_NONE_PASSWORD, use_ssl=True)
 
-    assert auth_options[2].ssl
+    assert auth_options.ssl
 
 
 def test_get_auth_options__ssl_false_with_password():
-    auth_options = get_auth_options(CREDENTIALS, use_ssl=False)
+    auth_options = get_auth_options(CREDENTIALS_WITH_PASSWORD, use_ssl=False)
 
-    assert not auth_options[0].ssl
+    assert not auth_options.ssl
 
 
 def test_get_auth_options__ssl_false_empty_password():
-    auth_options = get_auth_options(CREDENTIALS, use_ssl=False)
+    auth_options = get_auth_options(CREDENTIALS_EMPTY_PASSWORD, use_ssl=False)
 
-    assert not auth_options[1].ssl
+    assert not auth_options.ssl
 
 
 def test_get_auth_options__ssl_false_none_password():
-    auth_options = get_auth_options(CREDENTIALS, use_ssl=False)
+    auth_options = get_auth_options(CREDENTIALS_NONE_PASSWORD, use_ssl=False)
 
-    assert not auth_options[2].ssl
+    assert not auth_options.ssl
 
 
 def test_get_auth_options__auth_type_with_password():
-    auth_options = get_auth_options(CREDENTIALS, use_ssl=False)
+    auth_options = get_auth_options(CREDENTIALS_WITH_PASSWORD, use_ssl=False)
 
-    assert auth_options[0].auth_type == AUTH_NEGOTIATE
+    assert auth_options.auth_type == AUTH_NEGOTIATE
 
 
 def test_get_auth_options__auth_type_empty_password():
-    auth_options = get_auth_options(CREDENTIALS, use_ssl=False)
+    auth_options = get_auth_options(CREDENTIALS_EMPTY_PASSWORD, use_ssl=False)
 
-    assert auth_options[1].auth_type == AUTH_BASIC
+    assert auth_options.auth_type == AUTH_BASIC
 
 
 def test_get_auth_options__auth_type_none_password():
-    auth_options = get_auth_options(CREDENTIALS, use_ssl=False)
+    auth_options = get_auth_options(CREDENTIALS_NONE_PASSWORD, use_ssl=False)
 
-    assert auth_options[2].auth_type == AUTH_NEGOTIATE
+    assert auth_options.auth_type == AUTH_NEGOTIATE
 
 
 def test_get_auth_options__encryption_with_password():
-    auth_options = get_auth_options(CREDENTIALS, use_ssl=False)
+    auth_options = get_auth_options(CREDENTIALS_WITH_PASSWORD, use_ssl=False)
 
-    assert auth_options[0].encryption == ENCRYPTION_AUTO
+    assert auth_options.encryption == ENCRYPTION_AUTO
 
 
 def test_get_auth_options__encryption_empty_password():
-    auth_options = get_auth_options(CREDENTIALS, use_ssl=False)
+    auth_options = get_auth_options(CREDENTIALS_EMPTY_PASSWORD, use_ssl=False)
 
-    assert auth_options[1].encryption == ENCRYPTION_NEVER
+    assert auth_options.encryption == ENCRYPTION_NEVER
 
 
 def test_get_auth_options__encryption_none_password():
-    auth_options = get_auth_options(CREDENTIALS, use_ssl=False)
+    auth_options = get_auth_options(CREDENTIALS_NONE_PASSWORD, use_ssl=False)
 
-    assert auth_options[2].encryption == ENCRYPTION_AUTO
+    assert auth_options.encryption == ENCRYPTION_AUTO