[9.4] fix(outlook): prefix content document identities so DLS can match them (#4291) - #4312
Merged
Merged
Conversation
#4291) ## Closes #4290 With DLS enabled, the Outlook connector writes its two sides in different dialects. Access control documents grant **prefixed** identities, while every content document is decorated with the **raw** SMTP address: ``` ACL doc grants : ["user_id:60e8a9c1-...", "name:Alex Wilber", "email:alex.wilber@gmail.com"] content doc has : ["alex.wilber@gmail.com"] overlap : NONE ``` DLS filters `_allow_access_control` with a `terms` query, so the intersection being empty means the owner of a mailbox retrieves **none** of their own documents. This is pre-existing since DLS was added and affects all five `_decorate_with_access_control` call sites (mails, attachments, contacts, tasks, calendars). This routes the mailbox address through `_prefix_email` at every call site. Casing is normalised inside that helper rather than at the call sites, because both sides of the comparison already share it, so the invariant holds by construction instead of by convention. Two smaller things came along with it, both load-bearing rather than drive-by: - **Casing.** A `terms` query is case-sensitive, and a mailbox's primary SMTP address is not guaranteed to match the casing the directory reports for the same person. Prefixing alone would leave that as a second way to silently match nothing. - **`None` identities.** A mailbox with no address prefixes to `None`, and `_decorate_with_access_control` would previously store that null in the terms list. `es_access_control_query` already filters `None` on the access control side; this makes the content side agree. ### What this does not fix Access control documents take the address from the directory (Graph's `mail` on cloud, the LDAP `mail` attribute on server), while content documents take it from the mailbox (`primary_smtp_address`). Those are normally the same address, and after this change they also agree on casing, but they are not guaranteed to be equal: a tenant using aliases or proxy addresses can have a directory `mail` that differs from the mailbox's primary SMTP address, and for those users the terms query still will not match. Covering that properly means granting every proxy address a mailbox answers to, which is a larger change and deserves its own issue. Recording it here so it is a known limitation rather than a surprise. ## Upgrade note The incorrect values are stored on the content documents themselves, not only in the query template, so deploying this is not sufficient on its own. Affected deployments need a **full content sync** to rewrite `_allow_access_control` on every document; an access control sync alone will not do it. This differs from #4005, where only the stored query was wrong and re-running the access control sync was enough. Worth knowing alongside that: documents indexed before DLS was switched on carry no `_allow_access_control` field, and the `must_not exists` clause in `DLS_QUERY` makes those visible to everyone. That is existing framework behaviour rather than anything this PR changes, but it does mean enabling DLS never retroactively protects already-indexed content. ## Verification Beyond unit tests, I confirmed the bug and the fix end to end against a real Elasticsearch 8.19.0. Both documents were produced by driving the connector itself rather than written by hand, the index was created with the framework's own mappings, and the mustache template stored on the access control document was rendered by Elasticsearch and then executed. Before: | Document | main / 9.6 | 8.19 | | --- | --- | --- | | as the connector writes it today | **HIDDEN** | **HIDDEN** | | same document, `email:` prefix applied | VISIBLE | VISIBLE | | document with no `_allow_access_control` | VISIBLE | VISIBLE | After, on this branch, the first row is `VISIBLE` in both configurations. One thing worth knowing if you reproduce this: the two versions have to be paired correctly or you will get a misleading result. `main` queries `_allow_access_control.keyword` and applies no mappings, so dynamic mapping supplies `.keyword`; 8.19 queries `_allow_access_control.enum` and applies `Mappings.default_text_fields_mappings`, whose dynamic template supplies `.enum`. Each is internally consistent, and both fail for the same reason, which is what makes the prefix the sole cause. ## Checklists #### Pre-Review Checklist - [x] 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 - [x] 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`) - [x] For bugfixes: backport safely to all minor branches still receiving patch releases - [x] Considered corresponding documentation changes **Tests.** Six new tests. The three `test_content_documents_are_reachable_*` ones assert the invariant directly — that the identities on content documents intersect the identities the access control document grants — for the cloud path, the server/LDAP path, and mismatched address casing. This is the dual-side assertion the Outlook suite was missing: it asserted the access control side with `assert len(acl) == 1` and never looked at `_allow_access_control` on content documents, which is why this survived. Jira, Confluence and OneDrive already assert both sides. I checked the new tests are load-bearing by reverting only the source change and re-running them: ``` FAILED test_content_documents_are_reachable_by_their_owner FAILED test_content_documents_are_reachable_by_their_owner_on_server FAILED test_content_documents_are_reachable_whatever_the_address_casing FAILED test_decorate_with_access_control_drops_unknown_identities FAILED test_prefix_email_normalises_casing[Alex.Wilber@Example.COM-...] ``` Full suite on this branch: `2301 passed`, coverage 92.19%. **Local run.** `make autoformat lint test PYTHON=python3.11`. Ruff is clean. `typecheck` reports 13 pyright `unknown import symbol` errors in `kibana.py`, `service_cli.py`, `agent/`, `cli/`, `es/`, `protocol/` and `services/` — all pre-existing on `main` and none in files this PR touches. #### Changes Requiring Extra Attention - [x] Security-related changes (encryption, TLS, SSRF, etc) This changes which documents a user can retrieve under Document Level Security, so it deserves a careful read. The change only ever *narrows* nothing and *widens* access from "nobody sees anything" to "the mailbox owner sees their own mailbox" — content documents still carry exactly one identity, the mailbox they came from, which is the same value as before with a prefix applied. No document becomes visible to anyone other than its own mailbox owner. Worth flagging explicitly: documents with no `_allow_access_control` field are visible to everyone by design (the `must_not exists` clause in `DLS_QUERY`). That is unchanged here, and Outlook always sets the field when DLS is on. ## Related Pull Requests * #4287 — the other bugfix from the same investigation ## Release Note Fixed Document Level Security for the Outlook connector, where content documents were indexed with identities that did not match the ones granted by the access control documents, so no synced document was retrievable by the user who owned it. Co-authored-by: Cursor <cursoragent@cursor.com>
entsearchmachine
approved these changes
Jul 31, 2026
11 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.4: