From 31c928d6fce05937837a8a6276e631b706e8cebc Mon Sep 17 00:00:00 2001 From: Kili Date: Tue, 14 Jul 2026 19:34:56 +0200 Subject: [PATCH] Refresh Microsoft Graph docs: national clouds, examples, troubleshooting (#826) * Send report summary via Microsoft Graph; make Graph failures observable Two related fixes shipped together: Send via Graph: the periodic DMARC summary email can now be sent through the already-authenticated Microsoft Graph mailbox connection (MSGraphConnection.send_message(), /users/{mailbox}/sendMail) instead of only SMTP. Triggered when [msgraph] is configured and [smtp] has a `to` value but no `host` -- SMTP is always preferred when `host` is set, with no automatic fallback to Graph on SMTP failure. Reuses the same connection used for reading; no new send-only config mode. email_results()'s SMTP behavior is unchanged; a new email_results_via_msgraph() shares its content-building logic via a new _build_report_email_content() helper. Graph's sendMail always sends as the authenticated mailbox, so [smtp] from is ignored on this path -- documented, along with the required Mail.Send permissions and a caveat that delegated auth flows (UsernamePassword/DeviceCode) don't currently request that scope, so app-only auth is the supported path for sending. Tracks #472. Observable Graph failures: MSGraphConnection construction, mailbox fetch, message send, and --watch failures now catch ClientAuthenticationError/APIError/httpx.HTTPError specifically and log one clear ERROR line naming the mailbox, tenant, auth method, and the Graph request-id/client-request-id when available, instead of a bare "MS Graph Error"/"Mailbox Error" with no context. Full traceback still preserved at --debug. --watch previously had no Graph-specific error handling at all -- a Graph error there crashed with a raw uncaught traceback; it now exits the same way as the other three sites. No new config options. Co-Authored-By: Claude Sonnet 5 * Refresh Microsoft Graph docs: national clouds, examples, troubleshooting The [msgraph] docs were accurate but missed guidance the community has been asking for: - graph_url now lists the actual national/sovereign-cloud endpoint values (GCC High, DoD, China/21Vianet), with an explicit warning that setting it alone is not sufficient -- the Entra ID auth endpoint isn't independently configurable in parsedmarc or mailsuite, so it always hits the global login.microsoftonline.com. - A minimal working [msgraph] example for every auth method (UsernamePassword, DeviceCode, ClientSecret, Certificate, ClientAssertion) -- previously only Certificate had one, entangled with the SMTP-sending example. - A reading-permission matrix alongside the existing sending one, so every auth method x own/shared-mailbox combination is explicit in one place for both directions. - An accurate note on the parsedmarc-named token cache: it's a deliberate backward-compatibility choice from the 9.11.0 mailsuite extraction (mailsuite's own default cache name differs), not a migration users need to act on. - A troubleshooting table for four error scenarios, verified against source rather than assumed: admin consent and folder-resolution failures are still live and documented with real fixes; the event-loop and ISO-timestamp errors are historical, already fixed below this project's dependency/version floor. Co-Authored-By: Claude Sonnet 5 --------- Co-authored-by: Claude Sonnet 5 --- CHANGELOG.md | 7 + docs/source/usage.md | 171 +++++++++++++++++- parsedmarc/__init__.py | 101 +++++++++-- parsedmarc/cli.py | 157 ++++++++++++++--- tests/test_cli.py | 381 +++++++++++++++++++++++++++++++++++++++++ tests/test_init.py | 67 +++++++- 6 files changed, 842 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5effba5..5606ad0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ ### Changes - **Migrated the Elasticsearch output to the elasticsearch-py 8.x client** ([#806](https://github.com/domainaware/parsedmarc/issues/806)): the `elasticsearch-dsl` dependency is gone (the DSL is bundled in the client as `elasticsearch.dsl` since 8.18), and the new pin `elasticsearch>=8.18,<9` no longer forces `urllib3<2` — installs can now resolve urllib3 2.x. **Elasticsearch 7.x servers are no longer supported** (the 8.x client supports ES 8.x and 9.x servers). **OpenSearch users who were pointing the `[elasticsearch]` config section at an OpenSearch cluster must switch to the `[opensearch]` section** (the 8.x client's product check rejects OpenSearch). Also removed the dead ES 6-era `published_policy.fo` index migration in `migrate_indexes()` (unreachable via the 8.x client; the function remains as a no-op for API compatibility). +- **The periodic summary email can now be sent via Microsoft Graph** (tracking [#472](https://github.com/domainaware/parsedmarc/issues/472)). Previously the summary email required `[smtp] host`, forcing M365 tenants that block legacy SMTP AUTH to stand up a separate SMTP relay just to send from the same mailbox they already read reports from. When `[smtp] host` is omitted and `[msgraph]` is configured, the summary is now sent through the same already-authenticated Graph mailbox connection (`/users/{mailbox}/sendMail`), saved to Sent Items. SMTP is preferred whenever `[smtp] host` is set, with no fallback to Graph on SMTP failure. `[smtp] host`/`user`/`password`/`from` are now conditionally required — only when `host` is present; `to` keeps its existing requirement either way. A new public `email_results_via_msgraph()` function shares its subject/message/zip-building logic with the existing `email_results()` via an extracted `_build_report_email_content()` helper, so both transports stay in lockstep. Note `[smtp] from` has no effect on the Graph path — the message's `From` is always the `[msgraph]` mailbox — and sending requires the Graph `Mail.Send` permission (`Mail.Send.Shared` for a shared mailbox under delegated auth); see the "Sending the summary email via Microsoft Graph" docs section for the full permission matrix. +- **Microsoft Graph connection/fetch/send failures now log a single clear ERROR line** instead of a bare `logger.exception()` that hid the actual Azure/Graph error. The line identifies the mailbox, tenant ID, and auth method, and includes the Graph `request-id`/`client-request-id` when available (from the OData inner error or, failing that, the raw response headers) — details that matter when contacting Microsoft support. The full traceback is still preserved under `--debug`. +- **Refreshed the `[msgraph]` documentation**: national/sovereign-cloud `graph_url` values with an explicit warning that the Entra ID auth endpoint isn't independently configurable, a minimal example config for every auth method, a reading-permission matrix alongside the existing sending one, an accurate note on the `parsedmarc`-named token cache (no migration needed — it's a deliberate backward-compatibility choice from the 9.11.0 `mailsuite` extraction, not something users have to act on), and a troubleshooting table distinguishing still-live failure modes (admin consent, uninitialized-mailbox folder resolution) from historical ones already fixed at this project's dependency floor (`Event loop is closed`, invalid ISO timestamps). + +### Bug fixes + +- **`--watch` no longer crashes with a raw uncaught traceback on a Microsoft Graph error.** The continuous-mode loop previously caught only `FileExistsError`/`ParserError`; a Graph auth, API, or transport error during a long-running watch — arguably the most likely real-world failure point, since that's where token/certificate expiry actually surfaces — crashed uncaught. It now gets the same single formatted ERROR line as the other Graph call sites and exits cleanly. ## 10.2.2 diff --git a/docs/source/usage.md b/docs/source/usage.md index 7a59150..71e96f0 100644 --- a/docs/source/usage.md +++ b/docs/source/usage.md @@ -249,12 +249,44 @@ The full set of configuration options are: could be a shared mailbox if the user has access to the mailbox - `graph_url` - str: Microsoft Graph URL. Allows for use of National Clouds (ex Azure Gov) (Default: https://graph.microsoft.com) + + :::{warning} + Setting `graph_url` alone is **not** sufficient for a national/sovereign + cloud tenant. It only changes the Microsoft Graph API root; the + Microsoft Entra ID (Azure AD) token endpoint used for authentication is + not currently configurable in parsedmarc or `mailsuite`, and always + defaults to the global `https://login.microsoftonline.com`. A true + national-cloud deployment also needs its own Entra ID endpoint, so + `graph_url` by itself only helps if your tenant is registered in the + global cloud but you specifically need to reach one of these Graph API + roots (per [Microsoft's national cloud deployment docs][ms-graph-clouds]): + + | National cloud | Microsoft Graph URL | Entra ID endpoint (not configurable here) | + |---|---|---| + | Global (default) | `https://graph.microsoft.com` | `https://login.microsoftonline.com` | + | US Government L4 (GCC High) | `https://graph.microsoft.us` | `https://login.microsoftonline.us` | + | US Government L5 (DoD) | `https://dod-graph.microsoft.us` | `https://login.microsoftonline.us` | + | China, operated by 21Vianet | `https://microsoftgraph.chinacloudapi.cn` | `https://login.chinacloudapi.cn` | + + [ms-graph-clouds]: https://learn.microsoft.com/en-us/graph/deployments + ::: - `token_file` - str: Path to save the token file (Default: `.token`) - `allow_unencrypted_storage` - bool: Allows the Azure Identity module to fall back to unencrypted token cache (Default: `False`). Even if enabled, the cache will always try encrypted storage first. + :::{note} + `token_file` stores the serialized authentication record; the + underlying MSAL persistent token cache is separately named + `parsedmarc`, not `mailsuite`'s own default cache name. This is + deliberate: the Graph mailbox backend used to live directly in + parsedmarc, and moved into the `mailsuite` dependency in parsedmarc + 9.11.0. Explicitly keeping the `parsedmarc` cache name means tokens + cached before that move keep working after upgrading — there is + nothing to migrate and no action needed on your part. + ::: + :::{note} You must create an app registration in Azure AD and have an admin grant the Microsoft Graph `Mail.ReadWrite` @@ -263,6 +295,11 @@ The full set of configuration options are: username, you must grant the app `Mail.ReadWrite.Shared`. ::: + | Auth method | Reading (own mailbox) | Reading (shared mailbox) | + |---|---|---| + | `UsernamePassword` / `DeviceCode` (delegated) | `Mail.ReadWrite` | `Mail.ReadWrite.Shared` | + | `ClientSecret` / `Certificate` / `ClientAssertion` (application) | `Mail.ReadWrite` (application), scoped via `New-ApplicationAccessPolicy` | same as own mailbox — app-only access is scoped by policy, not by permission name | + :::{tip} **Troubleshooting connections.** Run with `--verbose` to log a redacted connection summary (auth method, tenant, client ID, @@ -273,6 +310,11 @@ The full set of configuration options are: an Exchange Online-side one), Microsoft Graph SDK requests, and `httpx` HTTP request lines. Secret values (passwords, client secrets, certificate passwords) are never written to logs. + Connection, mailbox fetch, message send, and `--watch` failures + each log a single ERROR line naming the mailbox, tenant, auth + method, and the Graph `request-id`/`client-request-id` when + available — worth quoting verbatim when contacting Microsoft + support. ::: :::{warning} @@ -295,6 +337,120 @@ The full set of configuration options are: applies to the `Certificate` and `ClientAssertion` auth methods. ::: + + **Sending the summary email via Microsoft Graph.** + When `[msgraph]` is configured and `[smtp]` has a `to` value but no + `host`, the periodic summary email is sent through the same + already-authenticated Graph mailbox connection used for reading + (`/users/{mailbox}/sendMail`), and a copy is saved to Sent Items. + When `[smtp] host` is set, SMTP is used regardless of whether + `[msgraph]` is also configured — SMTP is always preferred, with no + automatic fallback to Graph on SMTP failure. + + Example config combining `[msgraph]` with a `[smtp]` section that + only sets `to`/`subject` (no `host`): + + ```ini + [msgraph] + auth_method = Certificate + client_id = ... + tenant_id = ... + mailbox = dmarc-reports@example.com + certificate_path = /path/to/cert.pem + + [smtp] + to = admin@example.com + subject = DMARC Summary + ``` + + Required Microsoft Graph permissions, in addition to the + reading-related permissions documented above: + + | Auth method | Reading | Sending (own mailbox) | Sending (shared mailbox) | + |---|---|---|---| + | `UsernamePassword` / `DeviceCode` (delegated) | `Mail.ReadWrite` (+`.Shared` for shared) | `Mail.Send` | `Mail.Send.Shared` | + | `ClientSecret` / `Certificate` / `ClientAssertion` (application) | `Mail.ReadWrite` (application) | `Mail.Send` (application) | `Mail.Send` (application), scoped via `New-ApplicationAccessPolicy` | + + :::{warning} + Graph-based sending is only confirmed to work with the app-only + auth methods (`ClientSecret`, `Certificate`, `ClientAssertion`). + The delegated auth methods (`UsernamePassword`, `DeviceCode`) + currently request only the `Mail.ReadWrite`(`.Shared`) scope when + authenticating, not `Mail.Send` — so a delegated connection's + access token will not carry `Mail.Send` even if an administrator + has granted it, and `/sendMail` calls are expected to fail with an + access-denied error regardless of what's granted in Azure AD. Use + an app-only auth method if you need Graph-based sending. + ::: + + **Minimal example configs.** Each auth method needs a different + minimum set of keys. These read-only examples omit `[smtp]`; see + above for adding Graph-based sending on top of any of them. + + `UsernamePassword` (delegated, own mailbox): + ```ini + [msgraph] + auth_method = UsernamePassword + client_id = ... + client_secret = ... + user = dmarc-reports@example.com + password = ... + ``` + + `DeviceCode` (delegated, interactive sign-in on first run — `user` + is the account that signs in; `mailbox` is the shared mailbox it + reads, and only needs to differ from `user` to request + `Mail.ReadWrite.Shared` instead of plain `Mail.ReadWrite`): + ```ini + [msgraph] + auth_method = DeviceCode + client_id = ... + tenant_id = ... + user = signing-in-user@example.com + mailbox = dmarc-reports@example.com + ``` + + `ClientSecret` (app-only): + ```ini + [msgraph] + auth_method = ClientSecret + client_id = ... + tenant_id = ... + client_secret = ... + mailbox = dmarc-reports@example.com + ``` + + `Certificate` (app-only): + ```ini + [msgraph] + auth_method = Certificate + client_id = ... + tenant_id = ... + certificate_path = /path/to/cert.pem + mailbox = dmarc-reports@example.com + ``` + + `ClientAssertion` (app-only, short-lived JWT — see the note above + about its unsuitability for `watch` mode): + ```ini + [msgraph] + auth_method = ClientAssertion + client_id = ... + tenant_id = ... + client_assertion = ... + mailbox = dmarc-reports@example.com + ``` + + :::{tip} + **Troubleshooting.** + + | Error | Cause | Fix | + |---|---|---| + | *"...needs permission to access resources in your organization that only an admin can grant"* / "Admin consent required" | A delegated auth method (`UsernamePassword`, `DeviceCode`) is authenticating with a scope (`Mail.ReadWrite` or `Mail.ReadWrite.Shared`) the tenant admin hasn't consented to yet. | Have an Entra ID admin grant consent: Azure Portal → **Enterprise Applications** → *your app* → **Permissions** → **Grant admin consent**, or `az ad app permission admin-consent --id `. This is separate from the `New-ApplicationAccessPolicy` step above, which only applies to app-only auth. | + | `ErrorItemNotFound: ... Default folder Root not found` | `mailsuite` can resolve the well-known folders (`Inbox`, `Archive`, `Drafts`, `Sent Items`, `Deleted Items`, `Junk Email`) even when a mailbox's folder hierarchy hasn't fully provisioned, but a **custom, non-well-known** `reports_folder` name still fails to resolve on such a mailbox. | Point `reports_folder` at (or under) one of the six well-known folder names above, or sign into the (shared) mailbox once via Outlook/OWA to force Exchange to provision it, then retry. | + | `RuntimeError: Event loop is closed` | Historical bug, fixed in `mailsuite` 2.0.2. Not reachable with the `mailsuite>=2.2.2` this project requires. | Confirm your installed `mailsuite` version is current (`pip show mailsuite`); upgrade if it's somehow pinned below 2.0.2. | + | Invalid/rejected timestamp in the `since`/`receivedDateTime` filter | Historical bug (parsedmarc [#706](https://github.com/domainaware/parsedmarc/pull/706)/[#708](https://github.com/domainaware/parsedmarc/pull/708)): older versions appended a spurious `Z` to an already-UTC-offset ISO timestamp. Fixed since parsedmarc 9.5.1/9.5.5. | Upgrade parsedmarc if you're on a version older than 9.5.5. | + ::: - `elasticsearch` - `hosts` - str: A comma separated list of hostnames and ports or URLs (e.g. `127.0.0.1:9200` or @@ -371,14 +527,21 @@ The full set of configuration options are: - `aggregate_topic` - str: The Kafka topic for aggregate reports - `failure_topic` - str: The Kafka topic for failure reports - `smtp` - - `host` - str: The SMTP hostname + - `host` - str: The SMTP hostname. Required unless `[msgraph]` is + configured, in which case omitting it sends the summary via + Microsoft Graph instead — see "Sending the summary email via + Microsoft Graph" above. - `port` - int: The SMTP port (Default: `25`) - `ssl` - bool: Require SSL/TLS instead of using STARTTLS - `skip_certificate_verification` - bool: Skip certificate verification (not recommended) - - `user` - str: the SMTP username - - `password` - str: the SMTP password - - `from` - str: The From header to use in the email + - `user` - str: the SMTP username. SMTP-only; not used when sending + via Microsoft Graph. + - `password` - str: the SMTP password. SMTP-only; not used when + sending via Microsoft Graph. + - `from` - str: The From header to use in the email. SMTP-only. + When sent via Microsoft Graph, the message's `From` is always the + `[msgraph]` mailbox — `[smtp] from` has no effect. - `to` - list: A list of email addresses to send to - `subject` - str: The Subject header to use in the email (Default: `parsedmarc report`) diff --git a/parsedmarc/__init__.py b/parsedmarc/__init__.py index 2607c51..981a54f 100644 --- a/parsedmarc/__init__.py +++ b/parsedmarc/__init__.py @@ -2807,6 +2807,37 @@ def get_report_zip(results: ParsingResults) -> bytes: return storage.getvalue() +def _build_report_email_content( + results: ParsingResults, + *, + subject: str | None = None, + attachment_filename: str | None = None, + message: str | None = None, +) -> tuple[str, str, list[tuple[str, bytes]]]: + """Builds the subject, plain-text body, and zip attachment shared by + every report-summary email transport. + + Returns: + A ``(subject, plain_message, attachments)`` tuple. + """ + date_string = datetime.now().strftime("%Y-%m-%d") + if attachment_filename: + if not attachment_filename.lower().endswith(".zip"): + attachment_filename += ".zip" + filename = attachment_filename + else: + filename = "DMARC-{0}.zip".format(date_string) + + if subject is None: + subject = "DMARC results for {0}".format(date_string) + if message is None: + message = "DMARC results for {0}".format(date_string) + zip_bytes = get_report_zip(results) + attachments = [(filename, zip_bytes)] + + return subject, message, attachments + + def email_results( results: ParsingResults, host: str, @@ -2844,22 +2875,14 @@ def email_results( message (str): Override the default plain text body """ logger.debug("Emailing report") - date_string = datetime.now().strftime("%Y-%m-%d") - if attachment_filename: - if not attachment_filename.lower().endswith(".zip"): - attachment_filename += ".zip" - filename = attachment_filename - else: - filename = "DMARC-{0}.zip".format(date_string) - assert isinstance(mail_to, list) - if subject is None: - subject = "DMARC results for {0}".format(date_string) - if message is None: - message = "DMARC results for {0}".format(date_string) - zip_bytes = get_report_zip(results) - attachments = [(filename, zip_bytes)] + subject, message, attachments = _build_report_email_content( + results, + subject=subject, + attachment_filename=attachment_filename, + message=message, + ) send_email( host, @@ -2878,6 +2901,56 @@ def email_results( ) +def email_results_via_msgraph( + results: ParsingResults, + connection: MSGraphConnection, + mail_to: list[str], + *, + mail_cc: list[str] | None = None, + mail_bcc: list[str] | None = None, + subject: str | None = None, + attachment_filename: str | None = None, + message: str | None = None, +) -> None: + """ + Emails parsing results as a zip file via an already-authenticated + Microsoft Graph mailbox connection (``/users/{mailbox}/sendMail``), + saving a copy to Sent Items. + + Args: + results (dict): Parsing results + connection (MSGraphConnection): An already-authenticated Microsoft + Graph mailbox connection + mail_to (list): A list of addresses to mail to + mail_cc (list): A list of addresses to CC + mail_bcc (list): A list addresses to BCC + subject (str): Overrides the default message subject + attachment_filename (str): Override the default attachment filename + message (str): Override the default plain text body + """ + logger.debug("Emailing report via Microsoft Graph") + + subject, message, attachments = _build_report_email_content( + results, + subject=subject, + attachment_filename=attachment_filename, + message=message, + ) + + # Graph derives the From header from the authenticated mailbox and + # ignores message_from; it's still passed for API parity with + # send_message()'s signature. + connection.send_message( + message_from=connection.mailbox_name or "", + message_to=mail_to, + message_cc=mail_cc, + message_bcc=mail_bcc, + subject=subject, + attachments=attachments, + plain_message=message, + ) + + # Backward-compatible aliases parse_forensic_report = parse_failure_report parsed_forensic_reports_to_csv_rows = parsed_failure_reports_to_csv_rows diff --git a/parsedmarc/cli.py b/parsedmarc/cli.py index 578a3e6..14ccd73 100644 --- a/parsedmarc/cli.py +++ b/parsedmarc/cli.py @@ -17,7 +17,10 @@ from glob import glob from multiprocessing import Pipe, Process from ssl import CERT_NONE, create_default_context +import httpx import yaml +from azure.core.exceptions import ClientAuthenticationError +from kiota_abstractions.api_error import APIError from tqdm import tqdm from parsedmarc import ( @@ -28,6 +31,7 @@ from parsedmarc import ( __version__, elastic, email_results, + email_results_via_msgraph, gelf, get_dmarc_reports_from_mailbox, get_dmarc_reports_from_mbox, @@ -108,6 +112,63 @@ def _str_to_list(s): return list(map(lambda i: i.lstrip(), _list)) +def _msgraph_request_id_suffix(error: Exception) -> str: + """Returns ``" (request-id=..., client-request-id=...)"`` with only + the ids that are actually present, or ``""`` if neither is + available. Never raises.""" + try: + inner_error = getattr(getattr(error, "error", None), "inner_error", None) + request_id = getattr(inner_error, "request_id", None) + client_request_id = getattr(inner_error, "client_request_id", None) + if not request_id: + headers = getattr(error, "response_headers", None) or {} + request_id = headers.get("request-id") + parts = [] + if request_id: + parts.append("request-id={0}".format(request_id)) + if client_request_id: + parts.append("client-request-id={0}".format(client_request_id)) + if not parts: + return "" + return " ({0})".format(", ".join(parts)) + except Exception: + return "" + + +def _log_msgraph_failure( + error: Exception, + *, + stage: str, + mailbox: str | None, + tenant_id: str | None, + auth_method: str | None, +) -> None: + """Logs a single clear ERROR line for a Microsoft Graph connection, + fetch, send, or watch failure, identifying the mailbox/tenant/auth + method and the Graph request-id/client-request-id when available. + The full traceback is preserved at --debug via a follow-up DEBUG + record. Never calls exit() - the call site keeps its own exit(1).""" + if isinstance(error, APIError): + detail = getattr(error, "primary_message", None) or error.message or str(error) + detail = " ".join(str(detail).split()) + summary = "{0} status={1}: {2}".format( + type(error).__name__, error.response_status_code, detail + ) + else: + summary = "{0}: {1}".format(type(error).__name__, " ".join(str(error).split())) + + logger.error( + "Microsoft Graph %s failed (mailbox=%s, tenant_id=%s, auth_method=%s): %s%s", + stage, + mailbox, + tenant_id, + auth_method, + summary, + _msgraph_request_id_suffix(error), + ) + logger.debug("Microsoft Graph %s failure details:", stage, exc_info=True) + + def _expand_path(p: str) -> str: """Expand ``~`` and ``$VAR`` references in a file path.""" return os.path.expanduser(os.path.expandvars(p)) @@ -959,6 +1020,27 @@ def _parse_config(config: ConfigParser, opts): smtp_config = config["smtp"] if "host" in smtp_config: opts.smtp_host = smtp_config["host"] + if "user" in smtp_config: + opts.smtp_user = smtp_config["user"] + else: + raise ConfigurationError( + "user setting missing from the smtp config section" + ) + if "password" in smtp_config: + opts.smtp_password = smtp_config["password"] + else: + raise ConfigurationError( + "password setting missing from the smtp config section" + ) + if "from" in smtp_config: + opts.smtp_from = smtp_config["from"] + else: + logger.critical("from setting missing from the smtp config section") + elif getattr(opts, "graph_client_id", None): + # host is SMTP-only; when [msgraph] is configured, the + # summary email is sent via the same Graph mailbox connection + # instead, so host/user/password/from are not required here. + pass else: raise ConfigurationError( "host setting missing from the smtp config section" @@ -970,22 +1052,6 @@ def _parse_config(config: ConfigParser, opts): if "skip_certificate_verification" in smtp_config: smtp_verify = bool(smtp_config.getboolean("skip_certificate_verification")) opts.smtp_skip_certificate_verification = smtp_verify - if "user" in smtp_config: - opts.smtp_user = smtp_config["user"] - else: - raise ConfigurationError( - "user setting missing from the smtp config section" - ) - if "password" in smtp_config: - opts.smtp_password = smtp_config["password"] - else: - raise ConfigurationError( - "password setting missing from the smtp config section" - ) - if "from" in smtp_config: - opts.smtp_from = smtp_config["from"] - else: - logger.critical("from setting missing from the smtp config section") if "to" in smtp_config: opts.smtp_to = _str_to_list(smtp_config["to"]) else: @@ -2392,6 +2458,7 @@ def _main(): smtp_tls_reports += reports["smtp_tls_reports"] mailbox_connection = None + msgraph_connection: MSGraphConnection | None = None mailbox_batch_size_value = 10 mailbox_check_timeout_value = 30 normalize_timespan_threshold_hours_value = 24.0 @@ -2488,7 +2555,17 @@ def _main(): "Microsoft Graph connection initialized in %.2f seconds", time.monotonic() - connect_start, ) + msgraph_connection = mailbox_connection + except (ClientAuthenticationError, APIError, httpx.HTTPError) as error: + _log_msgraph_failure( + error, + stage="connection", + mailbox=opts.graph_mailbox or opts.graph_user, + tenant_id=opts.graph_tenant_id, + auth_method=opts.graph_auth_method, + ) + exit(1) except Exception: logger.exception("MS Graph Error") exit(1) @@ -2570,6 +2647,15 @@ def _main(): failure_reports += reports["failure_reports"] smtp_tls_reports += reports["smtp_tls_reports"] + except (ClientAuthenticationError, APIError, httpx.HTTPError) as error: + _log_msgraph_failure( + error, + stage="mailbox fetch", + mailbox=opts.graph_mailbox or opts.graph_user, + tenant_id=opts.graph_tenant_id, + auth_method=opts.graph_auth_method, + ) + exit(1) except Exception: logger.exception("Mailbox Error") exit(1) @@ -2586,17 +2672,17 @@ def _main(): logger.error(error.__str__()) exit(1) + smtp_to_value = ( + list(opts.smtp_to) + if isinstance(opts.smtp_to, list) + else _str_to_list(str(opts.smtp_to)) + ) if opts.smtp_host: try: verify = True if opts.smtp_skip_certificate_verification: verify = False smtp_port_value = int(opts.smtp_port) if opts.smtp_port is not None else 25 - smtp_to_value = ( - list(opts.smtp_to) - if isinstance(opts.smtp_to, list) - else _str_to_list(str(opts.smtp_to)) - ) email_results( parsing_results, opts.smtp_host, @@ -2612,6 +2698,26 @@ def _main(): except Exception: logger.exception("Failed to email results") exit(1) + elif msgraph_connection is not None and smtp_to_value: + try: + email_results_via_msgraph( + parsing_results, + msgraph_connection, + smtp_to_value, + subject=opts.smtp_subject, + ) + except (ClientAuthenticationError, APIError, httpx.HTTPError) as error: + _log_msgraph_failure( + error, + stage="message send", + mailbox=opts.graph_mailbox or opts.graph_user, + tenant_id=opts.graph_tenant_id, + auth_method=opts.graph_auth_method, + ) + exit(1) + except Exception: + logger.exception("Failed to email results via Microsoft Graph") + exit(1) if mailbox_connection and opts.mailbox_watch: logger.info("Watching for email - Ctrl-C once to quit, twice to force") @@ -2656,6 +2762,15 @@ def _main(): except ParserError as error: logger.error(error.__str__()) exit(1) + except (ClientAuthenticationError, APIError, httpx.HTTPError) as error: + _log_msgraph_failure( + error, + stage="mailbox watch", + mailbox=opts.graph_mailbox or opts.graph_user, + tenant_id=opts.graph_tenant_id, + auth_method=opts.graph_auth_method, + ) + exit(1) # Prioritize shutdown over reload if both flags are set (e.g. # SIGHUP followed by SIGTERM). atexit closes output clients. diff --git a/tests/test_cli.py b/tests/test_cli.py index b3789ba..8b5dad8 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9,12 +9,19 @@ import signal import sys import tempfile import unittest +import zipfile from configparser import ConfigParser from tempfile import NamedTemporaryFile from types import SimpleNamespace from typing import cast from unittest.mock import MagicMock, patch +import httpx +from azure.core.exceptions import ClientAuthenticationError +from msgraph.generated.models.o_data_errors.inner_error import InnerError +from msgraph.generated.models.o_data_errors.main_error import MainError +from msgraph.generated.models.o_data_errors.o_data_error import ODataError + import parsedmarc import parsedmarc.cli import parsedmarc.elastic @@ -1828,6 +1835,380 @@ client_assertion = s3cret-signed-jwt-assertion self.assertEqual(logging.getLogger(name).level, logging.WARNING, name) +class TestMSGraphEmailResults(unittest.TestCase): + """#472: the periodic summary email is sent via the same + already-authenticated Microsoft Graph mailbox connection when + [smtp] host is not configured but [msgraph] is, so M365 tenants that + block legacy SMTP AUTH can still receive the summary from the + mailbox they already read reports from. SMTP is preferred when + [smtp] host is set.""" + + CERT_CONFIG = """[general] +silent = true + +[msgraph] +auth_method = Certificate +client_id = client-id-1234 +tenant_id = tenant-id-5678 +mailbox = shared@example.com +certificate_path = /tmp/msgraph-cert.pem +certificate_password = s3cret-cert-pass +""" + + def setUp(self): + # _configure_dependency_logging mutates process-global loggers; + # snapshot and restore their levels and handlers so these tests + # don't leak state into the rest of the suite. + saved = {} + for name in parsedmarc.cli._DEPENDENCY_LOGGERS: + dep = logging.getLogger(name) + saved[name] = (dep.level, list(dep.handlers), dep.propagate) + + def restore(): + for name, (level, handlers, propagate) in saved.items(): + dep = logging.getLogger(name) + dep.setLevel(level) + dep.handlers = handlers + dep.propagate = propagate + + self.addCleanup(restore) + + def _write_config(self, config_text): + 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)) + return cfg_path + + def _run_main(self, cfg_path, *cli_args): + with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path, *cli_args]): + parsedmarc.cli._main() + + @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox") + @patch("parsedmarc.cli.MSGraphConnection") + def testCliSendsSummaryEmailViaMsGraphWhenNoSmtpHost( + self, mock_graph_connection, mock_get_mailbox_reports + ): + """[smtp] with only to/subject (no host) plus [msgraph] sends the + summary via Microsoft Graph's sendMail.""" + mock_get_mailbox_reports.return_value = { + "aggregate_reports": [], + "failure_reports": [], + "smtp_tls_reports": [], + } + config_text = ( + self.CERT_CONFIG + + """ +[smtp] +to = admin@example.com +subject = DMARC Summary +""" + ) + cfg_path = self._write_config(config_text) + self._run_main(cfg_path) + + send_message = mock_graph_connection.return_value.send_message + send_message.assert_called_once() + call_kwargs = send_message.call_args.kwargs + self.assertEqual(call_kwargs["message_to"], ["admin@example.com"]) + self.assertEqual(call_kwargs["subject"], "DMARC Summary") + + filename, payload = call_kwargs["attachments"][0] + self.assertRegex(filename, r"^DMARC-\d{4}-\d{2}-\d{2}\.zip$") + with zipfile.ZipFile(io.BytesIO(payload)) as zf: + self.assertIsNone(zf.testzip()) + + @patch("parsedmarc.cli.email_results") + @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox") + @patch("parsedmarc.cli.MSGraphConnection") + def testCliPrefersSmtpWhenBothSmtpAndMsGraphConfigured( + self, mock_graph_connection, mock_get_mailbox_reports, mock_email_results + ): + """SMTP is preferred over Microsoft Graph when [smtp] host is + set, even with [msgraph] also configured — no fallback, no + dual-send.""" + mock_get_mailbox_reports.return_value = { + "aggregate_reports": [], + "failure_reports": [], + "smtp_tls_reports": [], + } + config_text = ( + self.CERT_CONFIG + + """ +[smtp] +host = smtp.example.com +user = smtp-user +password = smtp-password +from = dmarc@example.com +to = admin@example.com +""" + ) + cfg_path = self._write_config(config_text) + self._run_main(cfg_path) + + mock_email_results.assert_called_once() + call_args = mock_email_results.call_args + self.assertEqual(call_args.args[1], "smtp.example.com") + self.assertEqual(call_args.args[2], "dmarc@example.com") + self.assertEqual(call_args.args[3], ["admin@example.com"]) + self.assertEqual(call_args.kwargs["username"], "smtp-user") + self.assertEqual(call_args.kwargs["password"], "smtp-password") + mock_graph_connection.return_value.send_message.assert_not_called() + + @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox") + @patch("parsedmarc.cli.MSGraphConnection") + def testCliSkipsGraphSendWithoutSmtpSection( + self, mock_graph_connection, mock_get_mailbox_reports + ): + """A [msgraph]-only, read-only config (no [smtp] section at all) + sends nothing — unchanged behavior for existing reading-only + users.""" + mock_get_mailbox_reports.return_value = { + "aggregate_reports": [], + "failure_reports": [], + "smtp_tls_reports": [], + } + cfg_path = self._write_config(self.CERT_CONFIG) + self._run_main(cfg_path) + + mock_graph_connection.return_value.send_message.assert_not_called() + + @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox") + @patch("parsedmarc.cli.MSGraphConnection") + @patch("parsedmarc.cli.logger") + def testCliSmtpWithoutHostRequiresMsGraph( + self, mock_logger, mock_graph_connection, mock_get_mailbox_reports + ): + """[smtp] with to but no host, and no [msgraph] configured at + all, still fails config parsing exactly as it did before this + feature — host is only optional when a Graph connection can + send instead.""" + config_text = """[general] +silent = true + +[smtp] +to = admin@example.com +""" + cfg_path = self._write_config(config_text) + + with self.assertRaises(SystemExit) as system_exit: + self._run_main(cfg_path) + + self.assertEqual(system_exit.exception.code, -1) + mock_logger.critical.assert_called_once_with( + "host setting missing from the smtp 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 testCliLogsMsGraphSendFailure( + self, mock_graph_connection, mock_get_mailbox_reports + ): + """A Graph sendMail failure gets the same single-ERROR-line + treatment as connection/fetch failures.""" + mock_get_mailbox_reports.return_value = { + "aggregate_reports": [], + "failure_reports": [], + "smtp_tls_reports": [], + } + mock_graph_connection.return_value.send_message.side_effect = ODataError( + response_status_code=403, + error=MainError( + message="Access is denied", + inner_error=InnerError(request_id="rid-1", client_request_id="crid-1"), + ), + ) + config_text = ( + self.CERT_CONFIG + + """ +[smtp] +to = admin@example.com +subject = DMARC Summary +""" + ) + cfg_path = self._write_config(config_text) + + with self.assertLogs("parsedmarc.log", level="ERROR") as cm: + with self.assertRaises(SystemExit) as system_exit: + self._run_main(cfg_path) + + self.assertEqual(system_exit.exception.code, 1) + output = "\n".join(cm.output) + self.assertIn("Microsoft Graph message send failed", output) + self.assertIn("mailbox=", output) + self.assertIn("tenant_id=", output) + self.assertIn("auth_method=Certificate", output) + self.assertIn("status=403", output) + self.assertIn("request-id=rid-1", output) + self.assertIn("client-request-id=crid-1", output) + + +class TestMSGraphFailureLogging(unittest.TestCase): + """Microsoft Graph connection/fetch/watch failures log a single + clear ERROR line identifying the mailbox/tenant/auth method and + the Graph request-id/client-request-id when available, instead of + a bare logger.exception() that hides the actual error.""" + + CERT_CONFIG = """[general] +silent = true + +[msgraph] +auth_method = Certificate +client_id = client-id-1234 +tenant_id = tenant-id-5678 +mailbox = shared@example.com +certificate_path = /tmp/msgraph-cert.pem +certificate_password = s3cret-cert-pass +""" + + def setUp(self): + saved = {} + for name in parsedmarc.cli._DEPENDENCY_LOGGERS: + dep = logging.getLogger(name) + saved[name] = (dep.level, list(dep.handlers), dep.propagate) + + def restore(): + for name, (level, handlers, propagate) in saved.items(): + dep = logging.getLogger(name) + dep.setLevel(level) + dep.handlers = handlers + dep.propagate = propagate + + self.addCleanup(restore) + + def _write_config(self, config_text): + 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)) + return cfg_path + + def _run_main(self, cfg_path, *cli_args): + with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path, *cli_args]): + parsedmarc.cli._main() + + @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox") + @patch("parsedmarc.cli.MSGraphConnection") + def testCliLogsMsGraphConnectionAuthFailureContext( + self, mock_graph_connection, mock_get_mailbox_reports + ): + """An auth failure during connection construction logs the + redacted context plus the actionable AADSTS code verbatim.""" + mock_graph_connection.side_effect = ClientAuthenticationError( + "AADSTS7000215: Invalid client secret" + ) + cfg_path = self._write_config(self.CERT_CONFIG) + + with self.assertLogs("parsedmarc.log", level="ERROR") as cm: + with self.assertRaises(SystemExit) as system_exit: + self._run_main(cfg_path) + + self.assertEqual(system_exit.exception.code, 1) + output = "\n".join(cm.output) + self.assertIn("Microsoft Graph connection failed", output) + self.assertIn("mailbox=shared@example.com", output) + self.assertIn("tenant_id=tenant-id-5678", output) + self.assertIn("auth_method=Certificate", output) + self.assertIn("AADSTS7000215", output) + mock_get_mailbox_reports.assert_not_called() + + @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox") + @patch("parsedmarc.cli.MSGraphConnection") + def testCliLogsMsGraphMailboxFetchFailureWithRequestId( + self, mock_graph_connection, mock_get_mailbox_reports + ): + """A mailbox-fetch failure (the most common real-world auth + failure point, since app-only auth defers token acquisition to + first use) surfaces both OData inner-error request ids.""" + mock_get_mailbox_reports.side_effect = ODataError( + response_status_code=503, + error=MainError( + message="Service unavailable", + inner_error=InnerError(request_id="rid-2", client_request_id="crid-2"), + ), + ) + cfg_path = self._write_config(self.CERT_CONFIG) + + with self.assertLogs("parsedmarc.log", level="ERROR") as cm: + with self.assertRaises(SystemExit) as system_exit: + self._run_main(cfg_path) + + self.assertEqual(system_exit.exception.code, 1) + output = "\n".join(cm.output) + self.assertIn("Microsoft Graph mailbox fetch failed", output) + self.assertIn("request-id=rid-2", output) + self.assertIn("client-request-id=crid-2", output) + + @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox") + @patch("parsedmarc.cli.MSGraphConnection") + def testCliMsGraphErrorFallsBackToResponseHeaderRequestId( + self, mock_graph_connection, mock_get_mailbox_reports + ): + """When the response body didn't deserialize into a proper OData + inner error, the request-id still surfaces from the raw + response headers.""" + mock_get_mailbox_reports.side_effect = ODataError( + response_status_code=503, + response_headers={"request-id": "hdr-rid"}, + error=None, + ) + cfg_path = self._write_config(self.CERT_CONFIG) + + with self.assertLogs("parsedmarc.log", level="ERROR") as cm: + with self.assertRaises(SystemExit): + self._run_main(cfg_path) + + output = "\n".join(cm.output) + self.assertIn("request-id=hdr-rid", output) + + @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox") + @patch("parsedmarc.cli.MSGraphConnection") + def testCliMsGraphErrorOmitsRequestIdWhenAbsent( + self, mock_graph_connection, mock_get_mailbox_reports + ): + """When neither an inner error nor a response header carries a + request id, the ERROR line simply omits the suffix rather than + printing an empty/misleading id.""" + mock_get_mailbox_reports.side_effect = ODataError(response_status_code=500) + cfg_path = self._write_config(self.CERT_CONFIG) + + with self.assertLogs("parsedmarc.log", level="ERROR") as cm: + with self.assertRaises(SystemExit) as system_exit: + self._run_main(cfg_path) + + self.assertEqual(system_exit.exception.code, 1) + output = "\n".join(cm.output) + self.assertNotIn("request-id=", output) + + @patch("parsedmarc.cli.watch_inbox", side_effect=httpx.ConnectError("dns failure")) + @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox") + @patch("parsedmarc.cli.MSGraphConnection") + def testWatchModeLogsMsGraphErrorAndExits( + self, mock_graph_connection, mock_get_mailbox_reports, mock_watch_inbox + ): + """Before this fix, --watch had no catch-all at all for Graph + errors, so a token/cert expiry mid-watch crashed with a raw + uncaught traceback. It now gets the same single ERROR line.""" + mock_get_mailbox_reports.return_value = { + "aggregate_reports": [], + "failure_reports": [], + "smtp_tls_reports": [], + } + config_text = self.CERT_CONFIG + "\n[mailbox]\nwatch = true\n" + cfg_path = self._write_config(config_text) + + with self.assertLogs("parsedmarc.log", level="ERROR") as cm: + with self.assertRaises(SystemExit) as system_exit: + self._run_main(cfg_path) + + self.assertEqual(system_exit.exception.code, 1) + output = "\n".join(cm.output) + self.assertIn("Microsoft Graph mailbox watch failed", output) + self.assertIn("ConnectError", output) + + class TestSighupReload(unittest.TestCase): """Tests for SIGHUP-driven configuration reload in watch mode.""" diff --git a/tests/test_init.py b/tests/test_init.py index 6ee8d0b..f01df39 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -19,13 +19,18 @@ from pathlib import Path from shutil import rmtree from tempfile import NamedTemporaryFile, mkdtemp from typing import BinaryIO, cast -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch from lxml import etree # type: ignore[import-untyped] import parsedmarc -from parsedmarc.mail import MaildirConnection -from parsedmarc.types import AggregateReport, FailureReport, SMTPTLSReport +from parsedmarc.mail import MaildirConnection, MSGraphConnection +from parsedmarc.types import ( + AggregateReport, + FailureReport, + ParsingResults, + SMTPTLSReport, +) # Detect if running in GitHub Actions to skip DNS lookups OFFLINE_MODE = os.environ.get("GITHUB_ACTIONS", "false").lower() == "true" @@ -2825,6 +2830,62 @@ class TestEmailResultsErrorBranches(unittest.TestCase): ) +class TestEmailResultsViaMsGraph(unittest.TestCase): + """email_results_via_msgraph() shares its + subject/message/attachment-building logic with email_results() via the + extracted _build_report_email_content() helper, so both transports stay + in lockstep instead of drifting into two different sets of defaults.""" + + @staticmethod + def _results() -> ParsingResults: + return { + "aggregate_reports": [], + "failure_reports": [], + "smtp_tls_reports": [], + } + + def testEmailResultsViaMsGraphBuildsSameContentAsEmailResults(self): + connection = MagicMock(spec=MSGraphConnection, mailbox_name="mb@example.com") + results = self._results() + + parsedmarc.email_results_via_msgraph(results, connection, ["admin@example.com"]) + + connection.send_message.assert_called_once() + graph_kwargs = connection.send_message.call_args.kwargs + + with patch("parsedmarc.send_email") as mock_send_email: + parsedmarc.email_results( + results, + host="smtp.example.com", + mail_from="from@example.com", + mail_to=["admin@example.com"], + ) + mock_send_email.assert_called_once() + smtp_kwargs = mock_send_email.call_args.kwargs + + self.assertEqual(graph_kwargs["subject"], smtp_kwargs["subject"]) + self.assertEqual(graph_kwargs["plain_message"], smtp_kwargs["plain_message"]) + self.assertEqual( + graph_kwargs["attachments"][0][0], + smtp_kwargs["attachments"][0][0], + ) + self.assertEqual(graph_kwargs["message_to"], ["admin@example.com"]) + self.assertEqual(graph_kwargs["message_from"], "mb@example.com") + + def testEmailResultsViaMsGraphAppendsZipExtension(self): + connection = MagicMock(spec=MSGraphConnection, mailbox_name="mb@example.com") + + parsedmarc.email_results_via_msgraph( + self._results(), + connection, + ["admin@example.com"], + attachment_filename="report", + ) + + graph_kwargs = connection.send_message.call_args.kwargs + self.assertEqual(graph_kwargs["attachments"][0][0], "report.zip") + + class TestAppendJson(unittest.TestCase): """append_json writes new files cleanly and merges into existing JSON arrays without breaking valid JSON."""