mirror of
https://github.com/domainaware/parsedmarc.git
synced 2026-07-16 13:34:56 +00:00
MS Graph: case-insensitive auth_method + ClientAssertion support (#809)
* draft fix MS Graph * Add ClientAssertion auth method support for MS Graph in cli.py MSGraphConnection/AuthMethod (via mailsuite) already supported ClientAssertion, but cli.py never parsed a client_assertion config value or passed it through, so it was unusable from the CLI. Wire config_msgraph.client_assertion through _parse_config and the MSGraphConnection call, and document the auth method (including its short-lived-JWT caveat vs. Certificate/ClientSecret for watch mode). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: MISAPOR LAB <misapor@lab.misapor.pl> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
MISAPOR LAB
Claude Sonnet 5
parent
a6241d537e
commit
fa8faa6b39
+14
-3
@@ -218,8 +218,8 @@ The full set of configuration options are:
|
||||
- `password` - str: The IMAP password
|
||||
- `msgraph`
|
||||
- `auth_method` - str: Authentication method, valid types are
|
||||
`UsernamePassword`, `DeviceCode`, `ClientSecret`, or `Certificate`
|
||||
(Default: `UsernamePassword`).
|
||||
`UsernamePassword`, `DeviceCode`, `ClientSecret`, `Certificate`, or
|
||||
`ClientAssertion` (Default: `UsernamePassword`).
|
||||
- `user` - str: The M365 user, required when the auth method is
|
||||
UsernamePassword
|
||||
- `password` - str: The user password, required when the auth
|
||||
@@ -231,6 +231,17 @@ The full set of configuration options are:
|
||||
`Certificate`
|
||||
- `certificate_password` - str: Optional password for the
|
||||
certificate file when using `Certificate` auth
|
||||
- `client_assertion` - str: A signed JWT client assertion, required
|
||||
when the auth method is `ClientAssertion`
|
||||
|
||||
:::{note}
|
||||
`client_assertion` is a static value, so it is only usable for as
|
||||
long as the JWT it contains remains unexpired (assertions are
|
||||
typically short-lived, on the order of minutes). It is best suited
|
||||
to short one-shot runs; for long-running `watch` mode, prefer
|
||||
`Certificate` or `ClientSecret` auth, which can refresh
|
||||
indefinitely.
|
||||
:::
|
||||
- `tenant_id` - str: The Azure AD tenant ID. This is required
|
||||
for all auth methods except UsernamePassword.
|
||||
- `mailbox` - str: The mailbox name. This defaults to the
|
||||
@@ -269,7 +280,7 @@ The full set of configuration options are:
|
||||
```
|
||||
|
||||
The same application permission and mailbox scoping guidance
|
||||
applies to the `Certificate` auth method.
|
||||
applies to the `Certificate` and `ClientAssertion` auth methods.
|
||||
|
||||
:::
|
||||
- `elasticsearch`
|
||||
|
||||
+34
-1
@@ -82,6 +82,26 @@ class ConfigurationError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
def _normalize_graph_auth_method(value: str) -> str:
|
||||
"""Return the canonical :class:`AuthMethod` member name for *value*.
|
||||
|
||||
Matching is case-insensitive so config values like ``certificate`` are
|
||||
accepted alongside the canonical ``Certificate``.
|
||||
|
||||
Raises:
|
||||
ConfigurationError: When *value* does not match a known auth method.
|
||||
"""
|
||||
value_lower = value.lower()
|
||||
for method in AuthMethod:
|
||||
if method.name.lower() == value_lower:
|
||||
return method.name
|
||||
raise ConfigurationError(
|
||||
"Invalid msgraph auth_method: {0!r}. Valid values are: {1}".format(
|
||||
value, ", ".join(m.name for m in AuthMethod)
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def _str_to_list(s):
|
||||
"""Converts a comma separated string to a list"""
|
||||
_list = s.split(",")
|
||||
@@ -635,7 +655,9 @@ def _parse_config(config: ConfigParser, opts):
|
||||
)
|
||||
opts.graph_auth_method = AuthMethod.UsernamePassword.name
|
||||
else:
|
||||
opts.graph_auth_method = graph_config["auth_method"]
|
||||
opts.graph_auth_method = _normalize_graph_auth_method(
|
||||
graph_config["auth_method"]
|
||||
)
|
||||
|
||||
if opts.graph_auth_method == AuthMethod.UsernamePassword.name:
|
||||
if "user" in graph_config:
|
||||
@@ -689,6 +711,14 @@ def _parse_config(config: ConfigParser, opts):
|
||||
if "certificate_password" in graph_config:
|
||||
opts.graph_certificate_password = graph_config["certificate_password"]
|
||||
|
||||
if opts.graph_auth_method == AuthMethod.ClientAssertion.name:
|
||||
if "client_assertion" in graph_config:
|
||||
opts.graph_client_assertion = graph_config["client_assertion"]
|
||||
else:
|
||||
raise ConfigurationError(
|
||||
"client_assertion setting missing from the msgraph config section"
|
||||
)
|
||||
|
||||
if "client_id" in graph_config:
|
||||
opts.graph_client_id = graph_config["client_id"]
|
||||
else:
|
||||
@@ -1922,6 +1952,7 @@ def _main():
|
||||
graph_client_secret=None,
|
||||
graph_certificate_path=None,
|
||||
graph_certificate_password=None,
|
||||
graph_client_assertion=None,
|
||||
graph_tenant_id=None,
|
||||
graph_mailbox=None,
|
||||
graph_allow_unencrypted_storage=False,
|
||||
@@ -2360,6 +2391,7 @@ def _main():
|
||||
if opts.graph_client_id:
|
||||
try:
|
||||
mailbox = opts.graph_mailbox or opts.graph_user
|
||||
logger.info("Connecting to Microsoft Graph mailbox %s", mailbox)
|
||||
mailbox_connection = MSGraphConnection(
|
||||
auth_method=opts.graph_auth_method,
|
||||
mailbox=mailbox,
|
||||
@@ -2368,6 +2400,7 @@ def _main():
|
||||
client_secret=opts.graph_client_secret,
|
||||
certificate_path=opts.graph_certificate_path,
|
||||
certificate_password=opts.graph_certificate_password,
|
||||
client_assertion=opts.graph_client_assertion,
|
||||
username=opts.graph_user,
|
||||
password=opts.graph_password,
|
||||
token_file=opts.graph_token_file,
|
||||
|
||||
@@ -1431,6 +1431,272 @@ certificate_path = /tmp/msgraph-cert.pem
|
||||
mock_graph_connection.assert_not_called()
|
||||
mock_get_mailbox_reports.assert_not_called()
|
||||
|
||||
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
|
||||
@patch("parsedmarc.cli.MSGraphConnection")
|
||||
def testCliPassesMsGraphClientAssertionAuthSettings(
|
||||
self, mock_graph_connection, mock_get_mailbox_reports
|
||||
):
|
||||
mock_graph_connection.return_value = object()
|
||||
mock_get_mailbox_reports.return_value = {
|
||||
"aggregate_reports": [],
|
||||
"failure_reports": [],
|
||||
"smtp_tls_reports": [],
|
||||
}
|
||||
|
||||
config_text = """[general]
|
||||
silent = true
|
||||
|
||||
[msgraph]
|
||||
auth_method = ClientAssertion
|
||||
client_id = client-id
|
||||
client_assertion = signed-jwt-assertion
|
||||
tenant_id = tenant-id
|
||||
mailbox = shared@example.com
|
||||
"""
|
||||
|
||||
with tempfile.NamedTemporaryFile("w", suffix=".ini", delete=False) as cfg:
|
||||
cfg.write(config_text)
|
||||
cfg_path = cfg.name
|
||||
self.addCleanup(lambda: os.path.exists(cfg_path) and os.remove(cfg_path))
|
||||
|
||||
with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path]):
|
||||
parsedmarc.cli._main()
|
||||
|
||||
self.assertEqual(
|
||||
mock_graph_connection.call_args.kwargs.get("auth_method"),
|
||||
"ClientAssertion",
|
||||
)
|
||||
self.assertEqual(
|
||||
mock_graph_connection.call_args.kwargs.get("client_assertion"),
|
||||
"signed-jwt-assertion",
|
||||
)
|
||||
self.assertEqual(
|
||||
mock_graph_connection.call_args.kwargs.get("tenant_id"), "tenant-id"
|
||||
)
|
||||
self.assertEqual(
|
||||
mock_graph_connection.call_args.kwargs.get("mailbox"),
|
||||
"shared@example.com",
|
||||
)
|
||||
|
||||
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
|
||||
@patch("parsedmarc.cli.MSGraphConnection")
|
||||
@patch("parsedmarc.cli.logger")
|
||||
def testCliRequiresMsGraphClientAssertionForClientAssertionAuth(
|
||||
self, mock_logger, mock_graph_connection, mock_get_mailbox_reports
|
||||
):
|
||||
config_text = """[general]
|
||||
silent = true
|
||||
|
||||
[msgraph]
|
||||
auth_method = ClientAssertion
|
||||
client_id = client-id
|
||||
tenant_id = tenant-id
|
||||
mailbox = shared@example.com
|
||||
"""
|
||||
|
||||
with tempfile.NamedTemporaryFile("w", suffix=".ini", delete=False) as cfg:
|
||||
cfg.write(config_text)
|
||||
cfg_path = cfg.name
|
||||
self.addCleanup(lambda: os.path.exists(cfg_path) and os.remove(cfg_path))
|
||||
|
||||
with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path]):
|
||||
with self.assertRaises(SystemExit) as system_exit:
|
||||
parsedmarc.cli._main()
|
||||
|
||||
self.assertEqual(system_exit.exception.code, -1)
|
||||
mock_logger.critical.assert_called_once_with(
|
||||
"client_assertion setting missing from the msgraph config section"
|
||||
)
|
||||
mock_graph_connection.assert_not_called()
|
||||
mock_get_mailbox_reports.assert_not_called()
|
||||
|
||||
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
|
||||
@patch("parsedmarc.cli.MSGraphConnection")
|
||||
@patch("parsedmarc.cli.logger")
|
||||
def testCliRequiresMsGraphTenantIdForClientAssertionAuth(
|
||||
self, mock_logger, mock_graph_connection, mock_get_mailbox_reports
|
||||
):
|
||||
config_text = """[general]
|
||||
silent = true
|
||||
|
||||
[msgraph]
|
||||
auth_method = ClientAssertion
|
||||
client_id = client-id
|
||||
client_assertion = signed-jwt-assertion
|
||||
mailbox = shared@example.com
|
||||
"""
|
||||
|
||||
with tempfile.NamedTemporaryFile("w", suffix=".ini", delete=False) as cfg:
|
||||
cfg.write(config_text)
|
||||
cfg_path = cfg.name
|
||||
self.addCleanup(lambda: os.path.exists(cfg_path) and os.remove(cfg_path))
|
||||
|
||||
with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path]):
|
||||
with self.assertRaises(SystemExit) as system_exit:
|
||||
parsedmarc.cli._main()
|
||||
|
||||
self.assertEqual(system_exit.exception.code, -1)
|
||||
mock_logger.critical.assert_called_once_with(
|
||||
"tenant_id setting missing from the msgraph config section"
|
||||
)
|
||||
mock_graph_connection.assert_not_called()
|
||||
mock_get_mailbox_reports.assert_not_called()
|
||||
|
||||
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
|
||||
@patch("parsedmarc.cli.MSGraphConnection")
|
||||
@patch("parsedmarc.cli.logger")
|
||||
def testCliRequiresMsGraphMailboxForClientAssertionAuth(
|
||||
self, mock_logger, mock_graph_connection, mock_get_mailbox_reports
|
||||
):
|
||||
config_text = """[general]
|
||||
silent = true
|
||||
|
||||
[msgraph]
|
||||
auth_method = ClientAssertion
|
||||
client_id = client-id
|
||||
client_assertion = signed-jwt-assertion
|
||||
tenant_id = tenant-id
|
||||
"""
|
||||
|
||||
with tempfile.NamedTemporaryFile("w", suffix=".ini", delete=False) as cfg:
|
||||
cfg.write(config_text)
|
||||
cfg_path = cfg.name
|
||||
self.addCleanup(lambda: os.path.exists(cfg_path) and os.remove(cfg_path))
|
||||
|
||||
with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path]):
|
||||
with self.assertRaises(SystemExit) as system_exit:
|
||||
parsedmarc.cli._main()
|
||||
|
||||
self.assertEqual(system_exit.exception.code, -1)
|
||||
mock_logger.critical.assert_called_once_with(
|
||||
"mailbox setting missing from the msgraph config section"
|
||||
)
|
||||
mock_graph_connection.assert_not_called()
|
||||
mock_get_mailbox_reports.assert_not_called()
|
||||
|
||||
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
|
||||
@patch("parsedmarc.cli.MSGraphConnection")
|
||||
def testCliAcceptsLowercaseMsGraphCertificateAuthMethod(
|
||||
self, mock_graph_connection, mock_get_mailbox_reports
|
||||
):
|
||||
"""auth_method values are case-insensitive (e.g. ``certificate``)."""
|
||||
mock_graph_connection.return_value = object()
|
||||
mock_get_mailbox_reports.return_value = {
|
||||
"aggregate_reports": [],
|
||||
"failure_reports": [],
|
||||
"smtp_tls_reports": [],
|
||||
}
|
||||
|
||||
config_text = """[general]
|
||||
silent = true
|
||||
|
||||
[msgraph]
|
||||
auth_method = certificate
|
||||
client_id = client-id
|
||||
tenant_id = tenant-id
|
||||
mailbox = shared@example.com
|
||||
certificate_path = /tmp/msgraph-cert.pem
|
||||
certificate_password = cert-pass
|
||||
"""
|
||||
|
||||
with tempfile.NamedTemporaryFile("w", suffix=".ini", delete=False) as cfg:
|
||||
cfg.write(config_text)
|
||||
cfg_path = cfg.name
|
||||
self.addCleanup(lambda: os.path.exists(cfg_path) and os.remove(cfg_path))
|
||||
|
||||
with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path]):
|
||||
parsedmarc.cli._main()
|
||||
|
||||
self.assertEqual(
|
||||
mock_graph_connection.call_args.kwargs.get("auth_method"), "Certificate"
|
||||
)
|
||||
self.assertEqual(
|
||||
mock_graph_connection.call_args.kwargs.get("mailbox"),
|
||||
"shared@example.com",
|
||||
)
|
||||
self.assertEqual(
|
||||
mock_graph_connection.call_args.kwargs.get("certificate_path"),
|
||||
"/tmp/msgraph-cert.pem",
|
||||
)
|
||||
self.assertEqual(
|
||||
mock_graph_connection.call_args.kwargs.get("certificate_password"),
|
||||
"cert-pass",
|
||||
)
|
||||
|
||||
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
|
||||
@patch("parsedmarc.cli.MSGraphConnection")
|
||||
@patch("parsedmarc.cli.logger")
|
||||
def testCliRejectsInvalidMsGraphAuthMethod(
|
||||
self, mock_logger, mock_graph_connection, mock_get_mailbox_reports
|
||||
):
|
||||
config_text = """[general]
|
||||
silent = true
|
||||
|
||||
[msgraph]
|
||||
auth_method = NotARealMethod
|
||||
client_id = client-id
|
||||
tenant_id = tenant-id
|
||||
mailbox = shared@example.com
|
||||
certificate_path = /tmp/msgraph-cert.pem
|
||||
"""
|
||||
|
||||
with tempfile.NamedTemporaryFile("w", suffix=".ini", delete=False) as cfg:
|
||||
cfg.write(config_text)
|
||||
cfg_path = cfg.name
|
||||
self.addCleanup(lambda: os.path.exists(cfg_path) and os.remove(cfg_path))
|
||||
|
||||
with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path]):
|
||||
with self.assertRaises(SystemExit) as system_exit:
|
||||
parsedmarc.cli._main()
|
||||
|
||||
self.assertEqual(system_exit.exception.code, -1)
|
||||
mock_logger.critical.assert_called_once()
|
||||
critical_message = mock_logger.critical.call_args.args[0]
|
||||
self.assertIn("Invalid msgraph auth_method", critical_message)
|
||||
self.assertIn("NotARealMethod", critical_message)
|
||||
mock_graph_connection.assert_not_called()
|
||||
mock_get_mailbox_reports.assert_not_called()
|
||||
|
||||
@patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
|
||||
@patch("parsedmarc.cli.MSGraphConnection")
|
||||
def testCliLogsMsGraphConnectionAttempt(
|
||||
self, mock_graph_connection, mock_get_mailbox_reports
|
||||
):
|
||||
"""An INFO log is emitted when parsedmarc starts a Graph connection."""
|
||||
mock_graph_connection.return_value = object()
|
||||
mock_get_mailbox_reports.return_value = {
|
||||
"aggregate_reports": [],
|
||||
"failure_reports": [],
|
||||
"smtp_tls_reports": [],
|
||||
}
|
||||
|
||||
config_text = """[general]
|
||||
silent = true
|
||||
|
||||
[msgraph]
|
||||
auth_method = Certificate
|
||||
client_id = client-id
|
||||
tenant_id = tenant-id
|
||||
mailbox = shared@example.com
|
||||
certificate_path = /tmp/msgraph-cert.pem
|
||||
"""
|
||||
|
||||
with tempfile.NamedTemporaryFile("w", suffix=".ini", delete=False) as cfg:
|
||||
cfg.write(config_text)
|
||||
cfg_path = cfg.name
|
||||
self.addCleanup(lambda: os.path.exists(cfg_path) and os.remove(cfg_path))
|
||||
|
||||
with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path, "--verbose"]):
|
||||
with self.assertLogs("parsedmarc.log", level="INFO") as cm:
|
||||
parsedmarc.cli._main()
|
||||
|
||||
self.assertTrue(
|
||||
any(
|
||||
"Connecting to Microsoft Graph mailbox shared@example.com" in message
|
||||
for message in cm.output
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
class TestSighupReload(unittest.TestCase):
|
||||
"""Tests for SIGHUP-driven configuration reload in watch mode."""
|
||||
|
||||
Reference in New Issue
Block a user