Repository navigation
[9.3] fix(outlook): skip mailbox-less accounts and stop SSL misconfig from aborting sync (#4085) - #4091
Merged
Conversation
…aborting sync (#4085) ## Relates to elastic/sdh-search#1898 Fixes two independent failure modes in the Outlook Server connector that caused an entire sync job to abort when a single account or the SSL configuration was misconfigured. The guiding principle of this branch: **per-account problems skip that account and continue; connection-wide problems fail the sync loudly** (so they cannot silently empty the index). **Issue A — `ErrorNonExistentMailbox` kills the whole sync** When iterating over AD user accounts, `get_mails()` accesses `account.inbox`, which lazily resolves `account.root` via an EWS `GetFolder` call. For AD users whose SMTP address has no associated Exchange mailbox (a valid LDAP entry but no mailbox provisioned), `exchangelib` raises `ErrorNonExistentMailbox`. With no exception handling around the per-account block, this propagated up and terminated the sync, losing all remaining accounts. This is the still-open part of #2931. **Fix:** the per-account block in `get_docs()` is wrapped in `try/except ErrorNonExistentMailbox`. A missing mailbox is specific to a single account, so the account is skipped with a warning and the sync continues with the remaining accounts. **Issue B — `NO_CERTIFICATE_OR_CRL_FOUND` SSL crash** When `ssl_enabled=True` but no certificate is provided, `ssl_ca` is set to `""`. The old code wrote that empty string to the cert file and still selected `RootCAAdapter`. When urllib3 later called `context.load_verify_locations()` on the empty file it raised `NO_CERTIFICATE_OR_CRL_FOUND`, aborting the sync. The error occurs inside the HTTP layer (not inside `cert_verify()`), so `RootCAAdapter`'s own `try/except` never caught it. **Fix:** in `ExchangeUsers.get_user_accounts()`, the cert file is written and `RootCAAdapter` is selected only when `ssl_ca` is actually populated. When SSL is enabled but no certificate is supplied, the connector falls back to `NoVerifyHTTPAdapter` and logs a clear warning instead of crashing. ### Why connection-wide SSL errors are NOT skipped per account An earlier iteration of this branch also caught a custom `SSLFailed` exception inside the per-account loop to "skip" accounts on SSL problems. That approach was removed because it was both ineffective and dangerous: - **Ineffective:** `SSLFailed` is only raised inside `RootCAAdapter.cert_verify`, and `requests`' `cert_verify` does not actually load the certificate — it only records the CA path. A genuinely bad/expired cert fails later, during the TLS handshake, where `exchangelib` catches the underlying `requests.exceptions.SSLError` and re-raises it as `exchangelib.errors.TransportError` (and explicitly does **not** retry it). So `SSLFailed` was never raised in practice and the `except SSLFailed` clause was dead code. - **Dangerous:** an SSL/connection failure affects *every* account, not one. Silently skipping all accounts would produce an empty but "successful" sync, which the framework interprets as "all documents deleted" — wiping previously indexed data. Therefore only `ErrorNonExistentMailbox` (genuinely per-account) is skipped. Connection-wide failures — TLS errors surfaced as `TransportError`, and any other unexpected error — propagate and abort the sync loudly, so the misconfiguration is surfaced to the operator and existing indexed data is preserved. ## Checklists #### Pre-Review Checklist - [ ] this PR does NOT contain credentials of any kind, such as API keys or username/passwords (double check `config.yml.example`) - [x] this PR has a meaningful title - [x] this PR links to all relevant github issues that it fixes or partially addresses - [ ] if there is no GH issue, please create it. Each PR should have a link to an issue - [x] this PR has a thorough description - [x] Covered the changes with automated tests - [x] Tested the changes locally - [x] Added a label for each target release version (example: `v7.13.2`, `v7.14.0`, `v8.0.0`) - [ ] For bugfixes: backport safely to all minor branches still receiving patch releases - [ ] Considered corresponding documentation changes - [ ] Contributed any configuration settings changes to the configuration reference - [ ] if you added or changed Rich Configurable Fields for a Native Connector, you made a corresponding PR in [Kibana](https://github.com/elastic/kibana/blob/main/packages/kbn-search-connectors/types/native_connectors.ts) #### Changes Requiring Extra Attention - [x] Security-related changes (encryption, TLS, SSRF, etc) — the SSL cert fallback behaviour changes: `ssl_enabled=True` with no cert now falls back to `NoVerifyHTTPAdapter` (unverified connections) instead of crashing. A genuinely invalid/expired certificate now fails the sync loudly rather than being silently skipped. Reviewers should confirm the no-cert fallback posture is desired. ## Related Pull Requests * Partially addresses #2931 ## Release Note **Outlook Server connector**: sync jobs no longer abort when an Active Directory user has a valid SMTP address but no associated Exchange mailbox (`ErrorNonExistentMailbox`). The affected account is now skipped with a warning and the sync continues. Additionally, configuring SSL without providing a certificate no longer crashes with `NO_CERTIFICATE_OR_CRL_FOUND`; the connector falls back to unverified connections and logs a clear warning. Genuine certificate/connection errors still fail the sync loudly rather than silently emptying the index. Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor <cursoragent@cursor.com>
entsearchmachine
approved these changes
Jun 23, 2026
7 of 13 tasks
Jan-Kazlouski-elastic
enabled auto-merge (squash)
June 23, 2026 13:07
Jan-Kazlouski-elastic
approved these changes
Jun 23, 2026
Closed
5 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backports the following commits to 9.3: