From fb5c3d21028e81d89b2dc495c9394ece458b5c04 Mon Sep 17 00:00:00 2001 From: Mark Bishop Date: Tue, 3 Dec 2024 15:14:09 -0500 Subject: [PATCH 01/10] Print configuration path on load --- login_duo/login_duo.c | 2 ++ pam_duo/pam_duo.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/login_duo/login_duo.c b/login_duo/login_duo.c index 644bd1e..8389fec 100644 --- a/login_duo/login_duo.c +++ b/login_duo/login_duo.c @@ -142,6 +142,8 @@ do_auth(struct login_ctx *ctx, const char *cmd) config = ctx->config ? ctx->config : DUO_CONF; flags = 0; + duo_syslog(LOG_INFO, "Loading config file %s\n", config); + duo_config_default(&cfg); /* Load our private config. */ diff --git a/pam_duo/pam_duo.c b/pam_duo/pam_duo.c index 0240b3a..d3a2aa0 100644 --- a/pam_duo/pam_duo.c +++ b/pam_duo/pam_duo.c @@ -132,6 +132,8 @@ pam_sm_authenticate(pam_handle_t *pamh, int pam_flags, /* Parse configuration */ config = DUO_CONF; + duo_syslog(LOG_INFO, "Loading config file %s", + config); if(parse_argv(&config, argc, argv) == 0) { return (PAM_SERVICE_ERR); } From f4cd91e9654911765230708dec10ec0345bb5d85 Mon Sep 17 00:00:00 2001 From: Mark Bishop Date: Tue, 3 Dec 2024 17:56:27 -0500 Subject: [PATCH 02/10] Fixing tests --- tests/common_suites.py | 174 ++++++++++++++++++++++++---------------- tests/test_login_duo.py | 148 +++++++++++++++++----------------- tests/test_pam_duo.py | 66 ++++++++------- 3 files changed, 215 insertions(+), 173 deletions(-) diff --git a/tests/common_suites.py b/tests/common_suites.py index f35ba02..0112f77 100644 --- a/tests/common_suites.py +++ b/tests/common_suites.py @@ -7,6 +7,7 @@ # common_suites.py # +import re import os import subprocess import time @@ -49,6 +50,7 @@ EOF = pexpect.EOF PROMPT = pexpect.EOF + def fips_available(): returncode = subprocess.call( [os.path.join(TESTDIR, "is_fips_supported.sh")], @@ -58,6 +60,24 @@ def fips_available(): class CommonTestCase(unittest.TestCase): + def assertRegexSomeline(self, result: list[str], regex: str): + found = False + for line in result: + if re.search(regex, line): + found = True + break + + self.assertTrue(found, f"Regex '{regex}' not found in any lines of {result}") + + def assertSomeline(self, result: list[str], s: str): + found = False + for line in result: + if line == s: + found = True + break + + self.assertTrue(found, f"Line '{s}' not found in any lines of {result}") + def call_binary(self, *args, **kwargs): raise NotImplementedError @@ -68,8 +88,8 @@ class Configuration(CommonTestCase): def test_missing_config_file(self): """Missing conf file""" result = self.call_binary(["-d", "-c", "/nonexistent", "true"]) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Couldn't open /nonexistent: No such file or directory", ) @@ -78,8 +98,8 @@ def test_bad_permissions_on_conf_file(self): with TempConfig(TESTCONF) as temp: os.chmod(temp.name, 0o644) result = self.call_binary(["-d", "-c", temp.name, "true"]) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], "{name} must be readable only by user '.*'".format(name=temp.name), ) @@ -92,24 +112,24 @@ def test_bad_configuration_files(self): ]: with TempConfig(config) as temp: result = self.call_binary(["-d", "-c", temp.name, "true"]) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], "Missing host, ikey, or skey in {name}".format(name=temp.name), ) def test_corrupt_configuration_file_failsafe(self): with TempConfig(BAD_CORRUPT_CONF) as temp: result = self.call_binary(["-d", "-c", temp.name, "true"]) - self.assertRegex( - result["stderr"][0], "Parse error in {name}".format(name=temp.name) + self.assertRegexSomeline( + result["stderr"], "Parse error in {name}".format(name=temp.name) ) self.assertEqual(result["returncode"], 0) def test_corrupt_configuration_file_failsecure(self): with TempConfig(BAD_CORRUPT_SECURE_CONF) as temp: result = self.call_binary(["-d", "-c", temp.name, "true"]) - self.assertRegex( - result["stderr"][0], "Parse error in {name}".format(name=temp.name) + self.assertRegexSomeline( + result["stderr"], "Parse error in {name}".format(name=temp.name) ) self.assertEqual(result["returncode"], 1) @@ -120,8 +140,8 @@ def test_mockduo_down(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "whatever", "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Failsafe Duo login for 'whatever'.*: Couldn't connect to .*", ) @@ -133,8 +153,8 @@ def test_down_fail_secure(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "whatever", "true"] ) - self.assertRegex( - result["stderr"][0], r"Couldn't open Duo API handle for .*" + self.assertRegexSomeline( + result["stderr"], r"Couldn't open Duo API handle for .*" ) self.assertEqual(result["returncode"], 1) @@ -150,8 +170,8 @@ def test_invalid_cert(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "whatever", "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"{failmode} Duo login for .* Couldn't connect to .*: certificate verify failed".format( failmode=config.failmode_as_prefix() ), @@ -165,8 +185,8 @@ def test_self_signed_with_noverify(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "preauth-allow", "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'preauth-allow'.*: preauth-allowed", ) @@ -182,8 +202,8 @@ def test_wrong_hostname(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "whatever", "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"{failmode} Duo login for .*: Couldn't connect to .*: Certificate name validation failed".format( failmode=config.failmode_as_prefix() ), @@ -197,8 +217,8 @@ def test_failsecure(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "whatever", "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Failsecure Duo login for .*: Couldn't connect to .*: Certificate name validation failed", ) @@ -208,8 +228,8 @@ def test_noverify(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "preauth-allow", "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'preauth-allow'.*: preauth-allowed", ) @@ -225,8 +245,8 @@ def test_http_server_abort_errors(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", code, "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Aborted Duo login for '{code}'.*: HTTP {code}".format( code=code ), @@ -239,8 +259,8 @@ def test_http_server_failmode_errors(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", code, "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"{failmode} Duo login for '{code}'.*: HTTP {code}".format( failmode=config.failmode_as_prefix(), code=code ), @@ -253,8 +273,8 @@ def test_http_server_invalid_credentials_error(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", code, "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"{failmode} Duo login for '{code}'.*: Invalid ikey or skey".format( failmode=config.failmode_as_prefix(), code=code ), @@ -266,8 +286,8 @@ def test_with_bad_keys(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "whatever", "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"{failmode} Duo login for .*: Invalid ikey or skey".format( failmode=config.failmode_as_prefix() ), @@ -286,8 +306,8 @@ def check_preauth_state(self, user, message, prefix=None): result = self.call_binary( ["-d", "-c", temp.name, "-f", user, "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"{prefix} Duo login for '{user}'.*{message}".format( prefix=prefix if prefix else config.failmode_as_prefix(), user=user, @@ -343,7 +363,7 @@ def test_preauth_allow_retry_after_date(self): execution_time = time.time() - start_time # 3.x seconds executed twice self.assertGreater(execution_time, 6) - + def test_preauth_allow_rate_limited(self): start_time = time.time() self.check_preauth_state( @@ -363,8 +383,8 @@ def check_host_reporting(self, host): result = self.call_binary( ["-d", "-c", temp.name, "-f", "preauth-allow", "-h", host, "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'preauth-allow' from {host}: preauth-allowed".format( host=host ), @@ -391,8 +411,8 @@ def test_with_no_http_proxy(self): ["-d", "-c", temp.name, "-f", "preauth-allow", "true"], env={}, ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'preauth-allow'.*: preauth-allowed", ) @@ -402,8 +422,8 @@ def test_with_broadcast_proxy(self): ["-d", "-c", temp.name, "-f", "preauth-allow", "true"], env={"http_proxy": "0.0.0.0"}, ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'preauth-allow'.*: preauth-allowed", ) @@ -412,8 +432,8 @@ def test_with_broadcast_proxy(self): ["-d", "-c", temp.name, "-f", "preauth-allow", "true"], env={"http_proxy": "0.0.0.0"}, ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Failsafe Duo login for .*: Couldn't connect to localhost:4443:.*", ) @@ -428,8 +448,8 @@ def test_getting_hostname(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "hostname", "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Aborted Duo login for 'hostname': correct hostname", ) if config.get("failmode", None) == "secure": @@ -449,8 +469,8 @@ def test_fips_login(self): ["-d", "-c", temp.name, "-f", "preauth-allow", "true"], timeout=10, ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'preauth-allow'.*: preauth-allowed", ) @@ -462,8 +482,8 @@ def test_fips_unavailable(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "preauth-allow", "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], "FIPS mode flag specified, but OpenSSL not built with FIPS support. Failing the auth.", ) @@ -478,8 +498,8 @@ def test_failmode_preauth_fail(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "auth_timeout", "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Error in Duo login for 'auth_timeout': HTTP 500", ) @@ -488,8 +508,8 @@ def test_failopen_report(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "failopen", "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Aborted Duo login for 'failopen': correct failmode", ) @@ -498,8 +518,8 @@ def test_failclosed_report(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "failclosed", "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Aborted Duo login for 'failclosed': correct failmode", ) @@ -508,8 +528,8 @@ def test_enroll(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "enroll", "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"User enrollment required", ) @@ -537,8 +557,8 @@ def test_fallback_and_uid(self): }, timeout=15, ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'preauth-allow'.*: preauth-allowed", ) @@ -550,8 +570,8 @@ def test_ssh_connection_host(self): "SSH_CONNECTION": "1.2.3.4", }, ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r" Skipped Duo login for 'preauth-allow'", ) @@ -560,8 +580,8 @@ def test_configuration_with_extra_space(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "preauth-allow", "true"] ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'preauth-allow'.*: preauth-allowed", ) @@ -577,9 +597,23 @@ class Interactive(CommonTestCase): "Passcode or option (1-4): ", ] + def remove_header_lines(self, s): + return "\r\n".join(s.split("\r\n")[1:]) + def assertOutputEqual(self, output, expected): processed_output = [line for line in output.split("\r\n") if line != ""] - self.assertListEqual(processed_output, expected) + for index, line in enumerate(processed_output): + if line == expected[index]: + continue + + if re.search(expected[index], line): + continue + + self.fail( + "Line {index} does not match\nExpected: {expected}\nActual: {actual}".format( + index=index, expected=expected[index], actual=line + ) + ) def run(self, result=None): with MockDuo(NORMAL_CERT): @@ -594,7 +628,8 @@ def three_failed_inputs(self, config): process.expect(CommonSuites.Interactive.PROMPT_REGEX, timeout=10), 0 ) self.assertOutputEqual( - process.match.group(0), CommonSuites.Interactive.PROMPT_TEXT + self.remove_header_lines(process.match.group(0)), + CommonSuites.Interactive.PROMPT_TEXT, ) process.sendline(b"123456") self.assertEqual( @@ -631,7 +666,7 @@ def three_failed_inputs(self, config): self.assertOutputEqual( process.before, [ - "A"*256, + "A" * 256, "[3] Error in Duo login for 'foobar'", ], ) @@ -645,7 +680,8 @@ def menu_options(self, config): process.expect(CommonSuites.Interactive.PROMPT_REGEX, timeout=10), 0 ) self.assertOutputEqual( - process.match.group(0), CommonSuites.Interactive.PROMPT_TEXT + self.remove_header_lines(process.match.group(0)), + CommonSuites.Interactive.PROMPT_TEXT, ) process.sendline(b"3") self.assertEqual( @@ -747,7 +783,7 @@ def test_basic_invalid_json(self): result = self.call_binary( ["-d", "-c", temp.name, "-f", "bad-json", "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"invalid JSON response", ) diff --git a/tests/test_login_duo.py b/tests/test_login_duo.py index 3f8fdeb..8a962bd 100755 --- a/tests/test_login_duo.py +++ b/tests/test_login_duo.py @@ -16,7 +16,7 @@ from tempfile import NamedTemporaryFile import pexpect -from common_suites import NORMAL_CERT, CommonSuites, fips_available, EOF +from common_suites import NORMAL_CERT, CommonSuites, CommonTestCase, fips_available, EOF from config import ( MOCKDUO_ADMINS_NO_USERS, MOCKDUO_AUTOPUSH, @@ -208,13 +208,16 @@ def call_binary(self, *args, **kwargs): return login_duo(*args, **kwargs) -class TestLoginDuoConfig(unittest.TestCase): - @unittest.skipIf(sys.platform == "sunos5", "Solaris ignores empty quotes in argument. Probably a difference in the getopt call") +class TestLoginDuoConfig(CommonTestCase): + @unittest.skipIf( + sys.platform == "sunos5", + "Solaris ignores empty quotes in argument. Probably a difference in the getopt call", + ) def test_empty_args(self): """Test to see how login_duo handles an empty string argument (we do need a valid argument also)""" result = login_duo(["", "-h"]) - self.assertRegex( - result["stderr"][0], ".*login_duo: option requires an argument.*" + self.assertRegexSomeline( + result["stderr"], ".*login_duo: option requires an argument.*" ) self.assertEqual( result["stderr"][1], @@ -225,8 +228,8 @@ def test_empty_args(self): def test_help_output(self): """Basic help output""" result = login_duo(["-h"]) - self.assertRegex( - result["stderr"][0], ".*login_duo: option requires an argument.*" + self.assertRegexSomeline( + result["stderr"], ".*login_duo: option requires an argument.*" ) self.assertEqual( result["stderr"][1], @@ -237,7 +240,7 @@ def test_help_output(self): def test_version_output(self): """Check version output""" result = login_duo(["-v"]) - self.assertRegex(result["stderr"][0], "login_duo \\d+\\.\\d+.\\d+") + self.assertRegexSomeline(result["stderr"], "login_duo \\d+\\.\\d+.\\d+") class TestLoginDuoEnv(CommonSuites.Env): @@ -245,7 +248,7 @@ def call_binary(self, *args, **kwargs): return login_duo(*args) -class TestLoginDuoSpecificEnv(unittest.TestCase): +class TestLoginDuoSpecificEnv(CommonTestCase): def run(self, result=None): with MockDuo(NORMAL_CERT): return super(TestLoginDuoSpecificEnv, self).run(result) @@ -259,8 +262,8 @@ def test_missing_uid(self): }, preload_script=os.path.join(TESTDIR, "login_duo.py"), ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Who are you?", ) @@ -299,7 +302,7 @@ def test_env_factor(self): self.assertEqual(process.expect("SUCCESS", timeout=10), 0) -class TestLoginDuoUIDMismatch(unittest.TestCase): +class TestLoginDuoUIDMismatch(CommonTestCase): def run(self, result=None): with MockDuo(NORMAL_CERT): return super(TestLoginDuoUIDMismatch, self).run(result) @@ -314,8 +317,8 @@ def test_nonroot(self): }, preload_script=os.path.join(TESTDIR, "login_duo.py"), ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Only root may specify -c or -f", ) @@ -324,8 +327,8 @@ def test_sync(self): result = login_duo( ["-d", "-c", temp.name, "-f", "whatever", "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Successful Duo login for 'whatever'", ) @@ -340,8 +343,8 @@ def test_unprivileged(self): preload_script=os.path.join(TESTDIR, "login_duo.py"), timeout=10, ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"couldn't drop privileges:", ) @@ -357,13 +360,13 @@ def test_privsep_user_not_found(self): preload_script=os.path.join(TESTDIR, "login_duo.py"), timeout=10, ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"User .* not found", ) -class TestLoginDuoTimeout(unittest.TestCase): +class TestLoginDuoTimeout(CommonTestCase): def run(self, result=None): with MockDuo(NORMAL_CERT): return super(TestLoginDuoTimeout, self).run(result) @@ -379,10 +382,11 @@ def test_connection_timeout(self): preload_script=os.path.join(TESTDIR, "login_duo.py"), timeout=10, ) - for line in result["stderr"][:3]: + for line in result["stderr"][2:5]: self.assertEqual(line, "Attempting connection") - self.assertRegex( - result["stderr"][3], + + self.assertRegexSomeline( + result["stderr"], r"Failsafe Duo login for 'timeout': Couldn't connect to localhost:4443: Failed to connect", ) @@ -395,9 +399,7 @@ def run(self, result=None): def test_default_shell(self): """Test that we fallback to /bin/sh if there is no shell specified for the user""" with TempConfig(MOCKDUO_AUTOPUSH) as temp: - env = { - "UID": "1015" - } + env = {"UID": "1015"} if sys.platform != "sunos5": # Solaris doesn't like it when you mess with the PS1 env["PS1"] = "$" @@ -420,7 +422,7 @@ def test_shell_as_command(self): self.assertEqual(process.expect("-c echo SUCCESS", timeout=10), 0) -class TestLoginDuoGroups(unittest.TestCase): +class TestLoginDuoGroups(CommonTestCase): def run(self, result=None): with MockDuo(NORMAL_CERT): return super(TestLoginDuoGroups, self).run(result) @@ -435,8 +437,8 @@ def test_users_only_match_users(self): }, preload_script=os.path.join(TESTDIR, "groups.py"), ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'preauth-allow': preauth-allowed", ) @@ -450,8 +452,8 @@ def test_users_or_admins_match_users(self): }, preload_script=os.path.join(TESTDIR, "groups.py"), ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'preauth-allow': preauth-allowed", ) @@ -464,8 +466,8 @@ def test_admins_and_not_users_match_admins(self): }, preload_script=os.path.join(TESTDIR, "groups.py"), ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'preauth-allow': preauth-allowed", ) @@ -476,8 +478,8 @@ def test_users_bypass(self): env={"UID": "1004"}, preload_script=os.path.join(TESTDIR, "groups.py"), ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"User preauth-allow bypassed Duo 2FA due to user's UNIX group", ) @@ -487,7 +489,7 @@ def call_binary(self, *args, **kwargs): return login_duo_interactive(*args, **kwargs) -class TestLoginDuoGECOS(unittest.TestCase): +class TestLoginDuoGECOS(CommonTestCase): def run(self, result=None): with MockDuo(NORMAL_CERT): return super(TestLoginDuoGECOS, self).run(result) @@ -499,8 +501,8 @@ def test_gecos_field_unparsed(self): env={"UID": "1010"}, preload_script=os.path.join(TESTDIR, "login_duo.py"), ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Successful Duo login for '1/2/3/4/5/gecos_user_gecos_field6'", ) @@ -511,12 +513,12 @@ def test_deprecated_gecos_parsed_flag(self): env={"UID": "1010"}, preload_script=os.path.join(TESTDIR, "login_duo.py"), ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"The gecos_parsed configuration item for Duo Unix is deprecated and no longer has any effect. Use gecos_delim and gecos_username_pos instead", ) - self.assertRegex( - result["stderr"][1], + self.assertRegexSomeline( + result["stderr"], "Skipped Duo login for 'gecos/6': gecos/6", ) @@ -527,8 +529,8 @@ def test_gecos_delimiter_default_position_6(self): env={"UID": "1012"}, preload_script=os.path.join(TESTDIR, "login_duo.py"), ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], "Skipped Duo login for 'gecos_user_gecos_field6': gecos-user-gecos-field6-allowed", ) @@ -539,8 +541,8 @@ def test_gecos_delimiter_slash_position_3(self): env={"UID": "1011"}, preload_script=os.path.join(TESTDIR, "login_duo.py"), ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'gecos_user_gecos_field3': gecos-user-gecos-field3-allowed", ) @@ -551,8 +553,8 @@ def test_gecos_parsing_error(self): env={"UID": "1012"}, preload_script=os.path.join(TESTDIR, "login_duo.py"), ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Could not parse GECOS field", ) @@ -563,8 +565,8 @@ def test_gecos_empty(self): env={"UID": "1016"}, preload_script=os.path.join(TESTDIR, "login_duo.py"), ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Empty GECOS field", ) @@ -573,16 +575,16 @@ def test_gecos_invalid_delimiter_length(self): result = login_duo( ["-d", "-c", temp.name, "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Invalid character option length. Character fields must be 1 character long: ',,'", ) - self.assertRegex( - result["stderr"][1], + self.assertRegexSomeline( + result["stderr"], r"Invalid login_duo option: 'gecos_delim'", ) - self.assertRegex( - result["stderr"][2], + self.assertRegexSomeline( + result["stderr"], r"Parse error in {config}, line \d+".format(config=temp.name), ) @@ -595,18 +597,18 @@ def test_invalid_delimiter_value(self): result = login_duo( ["-d", "-c", temp.name, "true"], ) - self.assertEqual( - result["stderr"][0], + self.assertSomeline( + result["stderr"], "Invalid gecos_delim '{delim}' (delimiter must be punctuation other than ':')".format( delim=config["gecos_delim"] ), ) - self.assertRegex( - result["stderr"][1], + self.assertRegexSomeline( + result["stderr"], r"Invalid login_duo option: 'gecos_delim'", ) - self.assertRegex( - result["stderr"][2], + self.assertRegexSomeline( + result["stderr"], r"Parse error in {config}, line \d+".format(config=temp.name), ) @@ -616,15 +618,15 @@ def test_invalid_delimiter_value_whitespace(self): ["-d", "-c", temp.name, "true"], ) self.assertEqual( - result["stderr"][0], + result["stderr"][2], "Invalid character option length. Character fields must be 1 character long: ''", ) - self.assertRegex( - result["stderr"][1], + self.assertRegexSomeline( + result["stderr"], r"Invalid login_duo option: 'gecos_delim'", ) - self.assertRegex( - result["stderr"][2], + self.assertRegexSomeline( + result["stderr"], r"Parse error in {config}, line \d+".format(config=temp.name), ) @@ -634,15 +636,15 @@ def test_invalid_pos_value(self): ["-d", "-c", temp.name, "true"], ) self.assertEqual( - result["stderr"][0], + result["stderr"][2], "Gecos position starts at 1", ) - self.assertRegex( - result["stderr"][1], + self.assertRegexSomeline( + result["stderr"], r"Invalid login_duo option: 'gecos_username_pos'", ) - self.assertRegex( - result["stderr"][2], + self.assertRegexSomeline( + result["stderr"], r"Parse error in {config}, line \d+".format(config=temp.name), ) diff --git a/tests/test_pam_duo.py b/tests/test_pam_duo.py index 75ff093..3ff80c6 100755 --- a/tests/test_pam_duo.py +++ b/tests/test_pam_duo.py @@ -16,7 +16,7 @@ import sys import pexpect -from common_suites import NORMAL_CERT, CommonSuites, EOF +from common_suites import NORMAL_CERT, CommonSuites, EOF, CommonTestCase from config import ( MOCKDUO_CONF, MOCKDUO_GECOS_DEFAULT_DELIM_6_POS, @@ -133,11 +133,11 @@ def pam_duo(args, env={}, timeout=10): @unittest.skipIf(sys.platform == "sunos5", SOLARIS_ISSUE) -class TestPamDuoHelp(unittest.TestCase): +class TestPamDuoHelp(CommonTestCase): def test_help(self): result = pam_duo(["-h"]) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Usage: .*/tests/testpam.py \[-d\] \[-c config\] \[-f user\] \[-h host\]", ) @@ -209,7 +209,7 @@ def call_binary(self, *args, **kwargs): @unittest.skipIf(sys.platform == "sunos5", SOLARIS_ISSUE) -class TestPamPrompts(unittest.TestCase): +class TestPamPrompts(CommonTestCase): def run(self, result=None): with MockDuo(NORMAL_CERT): return super(TestPamPrompts, self).run(result) @@ -217,11 +217,15 @@ def run(self, result=None): def test_max_prompts_equals_one(self): with TempConfig(MOCKDUO_PROMPTS_1) as temp: result = pam_duo(["-d", "-f", "pam_prompt", "-c", temp.name, "true"]) - self.assertRegex(result["stderr"][0], "Failed Duo login for 'pam_prompt'") - self.assertRegex( - result["stdout"][0], "Autopushing login request to phone..." + self.assertRegexSomeline( + result["stderr"], "Failed Duo login for 'pam_prompt'" + ) + self.assertRegexSomeline( + result["stdout"], "Autopushing login request to phone..." + ) + self.assertRegexSomeline( + result["stdout"], "Invalid passcode, please try again." ) - self.assertRegex(result["stdout"][1], "Invalid passcode, please try again.") def test_max_prompts_equals_maximum(self): with TempConfig(MOCKDUO_PROMPTS_DEFAULT) as temp: @@ -318,7 +322,7 @@ def test_invalid_argument(self): @unittest.skipIf(sys.platform == "sunos5", SOLARIS_ISSUE) -class TestPamGECOS(unittest.TestCase): +class TestPamGECOS(CommonTestCase): def run(self, result=None): with MockDuo(NORMAL_CERT): return super(TestPamGECOS, self).run(result) @@ -328,8 +332,8 @@ def test_gecos_field_unparsed(self): result = pam_duo( ["-d", "-c", temp.name, "-f", "fullgecos", "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'full_gecos_field': full-gecos-field", ) @@ -338,12 +342,12 @@ def test_deprecated_gecos_parsed_flag(self): result = pam_duo( ["-d", "-c", temp.name, "-f", "gecos/6", "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"The gecos_parsed configuration item for Duo Unix is deprecated and no longer has any effect. Use gecos_delim and gecos_username_pos instead", ) - self.assertRegex( - result["stderr"][1], + self.assertRegexSomeline( + result["stderr"], "Skipped Duo login for 'gecos/6': gecos/6", ) @@ -352,8 +356,8 @@ def test_gecos_delimiter_default_position_6(self): result = pam_duo( ["-d", "-c", temp.name, "-f", "gecos,6", "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], "Skipped Duo login for 'gecos_user_gecos_field6': gecos-user-gecos-field6-allowed", ) @@ -362,8 +366,8 @@ def test_gecos_delimiter_slash_position_3(self): result = pam_duo( ["-d", "-c", temp.name, "-f", "gecos/3", "true"], ) - self.assertRegex( - result["stderr"][0], + self.assertRegexSomeline( + result["stderr"], r"Skipped Duo login for 'gecos_user_gecos_field3': gecos-user-gecos-field3-allowed", ) @@ -395,17 +399,17 @@ def test_invalid_delimiter_value_colon(self): ["-d", "-c", temp.name, "true"], ) self.assertEqual( - result["stderr"][0], + result["stderr"][1], "Invalid gecos_delim '{delim}' (delimiter must be punctuation other than ':')".format( delim=config["gecos_delim"] ), ) - self.assertRegex( - result["stderr"][1], + self.assertRegexSomeline( + result["stderr"], r"Invalid pam_duo option: 'gecos_delim'", ) - self.assertRegex( - result["stderr"][2], + self.assertRegexSomeline( + result["stderr"], r"Parse error in {config}, line \d+".format(config=temp.name), ) @@ -415,15 +419,15 @@ def test_invalid_delimiter_value_whitespace(self): ["-d", "-c", temp.name, "true"], ) self.assertEqual( - result["stderr"][0], + result["stderr"][1], "Invalid character option length. Character fields must be 1 character long: ''", ) self.assertRegex( - result["stderr"][1], + result["stderr"][2], r"Invalid pam_duo option: 'gecos_delim'", ) self.assertRegex( - result["stderr"][2], + result["stderr"][3], r"Parse error in {config}, line \d+".format(config=temp.name), ) @@ -433,15 +437,15 @@ def test_invalid_pos_value(self): ["-d", "-c", temp.name, "true"], ) self.assertEqual( - result["stderr"][0], + result["stderr"][1], "Gecos position starts at 1", ) self.assertRegex( - result["stderr"][1], + result["stderr"][2], r"Invalid pam_duo option: 'gecos_username_pos'", ) self.assertRegex( - result["stderr"][2], + result["stderr"][3], r"Parse error in {config}, line \d+".format(config=temp.name), ) From 69e6a78af864d92f6ac02c0fd2be92e8fc72d31e Mon Sep 17 00:00:00 2001 From: Mark Bishop Date: Wed, 4 Dec 2024 11:32:45 -0500 Subject: [PATCH 03/10] Remove stray newline --- login_duo/login_duo.c | 2 +- tests/test_login_duo.py | 10 +++++----- tests/test_pam_duo.py | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/login_duo/login_duo.c b/login_duo/login_duo.c index 8389fec..5f2cc87 100644 --- a/login_duo/login_duo.c +++ b/login_duo/login_duo.c @@ -142,7 +142,7 @@ do_auth(struct login_ctx *ctx, const char *cmd) config = ctx->config ? ctx->config : DUO_CONF; flags = 0; - duo_syslog(LOG_INFO, "Loading config file %s\n", config); + duo_syslog(LOG_INFO, "Loading config file %s", config); duo_config_default(&cfg); diff --git a/tests/test_login_duo.py b/tests/test_login_duo.py index 8a962bd..950b142 100755 --- a/tests/test_login_duo.py +++ b/tests/test_login_duo.py @@ -382,7 +382,7 @@ def test_connection_timeout(self): preload_script=os.path.join(TESTDIR, "login_duo.py"), timeout=10, ) - for line in result["stderr"][2:5]: + for line in result["stderr"][1:4]: self.assertEqual(line, "Attempting connection") self.assertRegexSomeline( @@ -617,8 +617,8 @@ def test_invalid_delimiter_value_whitespace(self): result = login_duo( ["-d", "-c", temp.name, "true"], ) - self.assertEqual( - result["stderr"][2], + self.assertRegexSomeline( + result["stderr"], "Invalid character option length. Character fields must be 1 character long: ''", ) self.assertRegexSomeline( @@ -635,8 +635,8 @@ def test_invalid_pos_value(self): result = login_duo( ["-d", "-c", temp.name, "true"], ) - self.assertEqual( - result["stderr"][2], + self.assertRegexSomeline( + result["stderr"], "Gecos position starts at 1", ) self.assertRegexSomeline( diff --git a/tests/test_pam_duo.py b/tests/test_pam_duo.py index 3ff80c6..65ed0ce 100755 --- a/tests/test_pam_duo.py +++ b/tests/test_pam_duo.py @@ -437,15 +437,15 @@ def test_invalid_pos_value(self): ["-d", "-c", temp.name, "true"], ) self.assertEqual( - result["stderr"][1], + result["stderr"][0], "Gecos position starts at 1", ) self.assertRegex( - result["stderr"][2], + result["stderr"][1], r"Invalid pam_duo option: 'gecos_username_pos'", ) self.assertRegex( - result["stderr"][3], + result["stderr"][2], r"Parse error in {config}, line \d+".format(config=temp.name), ) From 7c2626ec5c6b11142b08d005fc9cae67fe75ef12 Mon Sep 17 00:00:00 2001 From: Mark Bishop Date: Wed, 4 Dec 2024 14:54:55 -0500 Subject: [PATCH 04/10] More fixes --- tests/test_pam_duo.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_pam_duo.py b/tests/test_pam_duo.py index 65ed0ce..28fd565 100755 --- a/tests/test_pam_duo.py +++ b/tests/test_pam_duo.py @@ -398,8 +398,8 @@ def test_invalid_delimiter_value_colon(self): result = pam_duo( ["-d", "-c", temp.name, "true"], ) - self.assertEqual( - result["stderr"][1], + self.assertRegexSomeline( + result["stderr"], "Invalid gecos_delim '{delim}' (delimiter must be punctuation other than ':')".format( delim=config["gecos_delim"] ), @@ -419,15 +419,15 @@ def test_invalid_delimiter_value_whitespace(self): ["-d", "-c", temp.name, "true"], ) self.assertEqual( - result["stderr"][1], + result["stderr"][0], "Invalid character option length. Character fields must be 1 character long: ''", ) self.assertRegex( - result["stderr"][2], + result["stderr"][1], r"Invalid pam_duo option: 'gecos_delim'", ) self.assertRegex( - result["stderr"][3], + result["stderr"][2], r"Parse error in {config}, line \d+".format(config=temp.name), ) From e14ccafc013d6e38f90fabc2959b7d920d7f1467 Mon Sep 17 00:00:00 2001 From: Mark Bishop Date: Wed, 4 Dec 2024 15:44:44 -0500 Subject: [PATCH 05/10] Try this fix --- tests/common_suites.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/common_suites.py b/tests/common_suites.py index 0112f77..6b7dd65 100644 --- a/tests/common_suites.py +++ b/tests/common_suites.py @@ -587,6 +587,9 @@ def test_configuration_with_extra_space(self): class Interactive(CommonTestCase): PROMPT_REGEX = ".* or option \\(1-4\\): $" + INITIAL_TEXT = [ + ".*Loading config file .*", + ] PROMPT_TEXT = [ "Duo login for foobar", "Choose or lose:", @@ -598,7 +601,7 @@ class Interactive(CommonTestCase): ] def remove_header_lines(self, s): - return "\r\n".join(s.split("\r\n")[1:]) + return s def assertOutputEqual(self, output, expected): processed_output = [line for line in output.split("\r\n") if line != ""] @@ -629,7 +632,7 @@ def three_failed_inputs(self, config): ) self.assertOutputEqual( self.remove_header_lines(process.match.group(0)), - CommonSuites.Interactive.PROMPT_TEXT, + self.INITIAL_TEXT + CommonSuites.Interactive.PROMPT_TEXT, ) process.sendline(b"123456") self.assertEqual( @@ -681,7 +684,7 @@ def menu_options(self, config): ) self.assertOutputEqual( self.remove_header_lines(process.match.group(0)), - CommonSuites.Interactive.PROMPT_TEXT, + self.INITIAL_TEXT + CommonSuites.Interactive.PROMPT_TEXT, ) process.sendline(b"3") self.assertEqual( From 24bdbec23bed14d697643528be6a6a95d6c6a0f9 Mon Sep 17 00:00:00 2001 From: Mark Bishop Date: Wed, 4 Dec 2024 15:53:48 -0500 Subject: [PATCH 06/10] PAM interactive doesn't have initial text --- tests/test_pam_duo.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_pam_duo.py b/tests/test_pam_duo.py index 28fd565..faea43b 100755 --- a/tests/test_pam_duo.py +++ b/tests/test_pam_duo.py @@ -279,6 +279,8 @@ def call_binary(self, *args): @unittest.skipIf(sys.platform == "sunos5", SOLARIS_ISSUE) class TestPamDuoInteractive(CommonSuites.Interactive): + INITIAL_TEXT = [] + def call_binary(self, *args, **kwargs): return pam_duo_interactive(*args, **kwargs) From 429ad1e923b8c39ca38a86474b247e856854906d Mon Sep 17 00:00:00 2001 From: Mark Bishop Date: Wed, 4 Dec 2024 15:59:22 -0500 Subject: [PATCH 07/10] Switch to someline --- tests/test_pam_duo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pam_duo.py b/tests/test_pam_duo.py index faea43b..5978a0e 100755 --- a/tests/test_pam_duo.py +++ b/tests/test_pam_duo.py @@ -400,7 +400,7 @@ def test_invalid_delimiter_value_colon(self): result = pam_duo( ["-d", "-c", temp.name, "true"], ) - self.assertRegexSomeline( + self.assertSomeline( result["stderr"], "Invalid gecos_delim '{delim}' (delimiter must be punctuation other than ':')".format( delim=config["gecos_delim"] From 7ab27626d08dccb961a46504ae1b4508b125aa4e Mon Sep 17 00:00:00 2001 From: Mark Bishop Date: Mon, 9 Dec 2024 14:25:03 -0500 Subject: [PATCH 08/10] Switch to assertIn --- tests/common_suites.py | 9 --------- tests/test_login_duo.py | 4 ++-- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/tests/common_suites.py b/tests/common_suites.py index 6b7dd65..6ef5af3 100644 --- a/tests/common_suites.py +++ b/tests/common_suites.py @@ -69,15 +69,6 @@ def assertRegexSomeline(self, result: list[str], regex: str): self.assertTrue(found, f"Regex '{regex}' not found in any lines of {result}") - def assertSomeline(self, result: list[str], s: str): - found = False - for line in result: - if line == s: - found = True - break - - self.assertTrue(found, f"Line '{s}' not found in any lines of {result}") - def call_binary(self, *args, **kwargs): raise NotImplementedError diff --git a/tests/test_login_duo.py b/tests/test_login_duo.py index 950b142..5b03f58 100755 --- a/tests/test_login_duo.py +++ b/tests/test_login_duo.py @@ -597,11 +597,11 @@ def test_invalid_delimiter_value(self): result = login_duo( ["-d", "-c", temp.name, "true"], ) - self.assertSomeline( - result["stderr"], + self.assertIn( "Invalid gecos_delim '{delim}' (delimiter must be punctuation other than ':')".format( delim=config["gecos_delim"] ), + result["stderr"], ) self.assertRegexSomeline( result["stderr"], From 4e03d1e656eff1bf04f93609b1ad3ff9b365802f Mon Sep 17 00:00:00 2001 From: Mark Bishop Date: Tue, 10 Dec 2024 10:48:23 -0500 Subject: [PATCH 09/10] Fixup --- tests/test_pam_duo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pam_duo.py b/tests/test_pam_duo.py index 5978a0e..7e213a6 100755 --- a/tests/test_pam_duo.py +++ b/tests/test_pam_duo.py @@ -400,7 +400,7 @@ def test_invalid_delimiter_value_colon(self): result = pam_duo( ["-d", "-c", temp.name, "true"], ) - self.assertSomeline( + self.assertIn( result["stderr"], "Invalid gecos_delim '{delim}' (delimiter must be punctuation other than ':')".format( delim=config["gecos_delim"] From 86c2b87bdc06c306f9a40432853f5b02a0617dc4 Mon Sep 17 00:00:00 2001 From: Mark Bishop Date: Tue, 10 Dec 2024 10:55:15 -0500 Subject: [PATCH 10/10] Fixup --- tests/test_pam_duo.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pam_duo.py b/tests/test_pam_duo.py index 7e213a6..0347597 100755 --- a/tests/test_pam_duo.py +++ b/tests/test_pam_duo.py @@ -401,10 +401,10 @@ def test_invalid_delimiter_value_colon(self): ["-d", "-c", temp.name, "true"], ) self.assertIn( - result["stderr"], "Invalid gecos_delim '{delim}' (delimiter must be punctuation other than ':')".format( delim=config["gecos_delim"] ), + result["stderr"], ) self.assertRegexSomeline( result["stderr"],