Skip to content

fix: restrict unpublished SharePoint Online pages to owners/editors (#3645) - #4437

Merged
Jan-Kazlouski-elastic merged 9 commits into
mainfrom
fix/3645-spo-unpublished-page-acl
Sep 14, 2026
Merged

Jan-Kazlouski-elastic merged 9 commits into
mainfrom
fix/3645-spo-unpublished-page-acl

Conversation

@Jan-Kazlouski-elastic

Copy link
Copy Markdown
Contributor

Summary

With DLS enabled, unpublished SharePoint Online pages were still returned to users with view access. SharePoint does not update a page's ACLs when it is unpublished, so the connector kept the old view-only permissions on the indexed document.

This detects draft vs published state from the page version string and restricts _allow_access_control on unpublished pages to owners/editors only. A published field is also exposed on site_page documents.

Closes #3645

Changes

  • Detect published state from OData__UIVersionString (major version = published, minor version = draft)
  • For unpublished pages, restrict ACLs to owners/editors:
    • unique per-page permissions: only edit-or-higher role members
    • inherited site permissions: use a site editors ACL set instead of the full site ACL
  • Add require_edit_access to _get_access_control_from_role_assignment
  • Add EDIT_ITEM_MASK / EDIT_ROLE_TYPES constants
  • Add published field on site_page documents
  • Debug log when an unpublished page ACL is restricted

Testing

  • Unit tests: version parsing, edit-only role filtering, unpublished/published ACL behavior (unique + inherited permissions). 209 passed in test_sharepoint_online.py.
  • E2E: not run locally (requires live SharePoint Online tenant with DLS).

Notes for reviewers

SharePoint Online does not update a page's ACLs when it is unpublished, so
the connector kept granting view-only users access to draft/unpublished
pages under DLS. Detect the published state from the page version
(major.minor) and, for unpublished pages, only grant access to
owners/editors: view-only members are excluded from both unique per-page
role assignments and inherited site permissions. Also expose a `published`
field on site page documents.

Closes #3645
@Jan-Kazlouski-elastic
Jan-Kazlouski-elastic marked this pull request as draft September 3, 2026 15:24
@Jan-Kazlouski-elastic Jan-Kazlouski-elastic self-assigned this Sep 8, 2026
Resolve NOTICE.txt conflict by taking main's version.

try:
return int(minor) == 0
except ValueError:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why True?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch
I wanted it to be less restrictive initially, but after thinking about it more, changing this to False makes more sense. If we can't parse the version, we shouldn't assume the page is published. Same as with _grants_access.

An unparsable minor version in OData__UIVersionString is no evidence
that a site page is published, so restrict its ACL to owners/editors
instead of assuming the page is live. Missing or empty version strings
still default to published, since an absent field would otherwise
restrict every page on the site.
@Jan-Kazlouski-elastic

Copy link
Copy Markdown
Contributor Author

@erikcurrin-elastic Your comment is addressed. Could you please re-review?

@Jan-Kazlouski-elastic
Jan-Kazlouski-elastic merged commit bc3389c into main Sep 14, 2026
2 checks passed
@Jan-Kazlouski-elastic
Jan-Kazlouski-elastic deleted the fix/3645-spo-unpublished-page-acl branch September 14, 2026 17:00
@github-actions

Copy link
Copy Markdown

💔 Failed to create backport PR(s)

Status Branch Result
✅ 9.4 #4480
✅ 9.5 #4481
❌ 8.19 Commit could not be cherrypicked due to conflicts

Successful backport PRs will be merged automatically after passing CI.

To backport manually run:
backport --pr 4437 --autoMerge --autoMergeMethod squash

Jan-Kazlouski-elastic added a commit that referenced this pull request Sep 15, 2026
…tors (#3645) (#4437) (#4480)

Backports the following commits to 9.4:
- fix: restrict unpublished SharePoint Online pages to owners/editors
(#3645) (#4437)

---------

Co-authored-by: Jan-Kazlouski-elastic <jan.kazlouski@elastic.co>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Jan-Kazlouski-elastic added a commit that referenced this pull request Sep 15, 2026
…tors (#3645) (#4437) (#4481)

Backports the following commits to 9.5:
- fix: restrict unpublished SharePoint Online pages to owners/editors
(#3645) (#4437)

---------

Co-authored-by: Jan-Kazlouski-elastic <jan.kazlouski@elastic.co>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Jan-Kazlouski-elastic added a commit that referenced this pull request Sep 17, 2026
…itors (#3645) (#4437) (#4491)

Backported from #4437

Backports the following commits to 8.19:
- fix: restrict unpublished SharePoint Online pages to owners/editors
(#3645) (#4437)

## Backport notes
- The `8.19` branch uses the monolithic
`connectors/sources/sharepoint_online.py` layout (not the
`sharepoint/sharepoint_online/` package on `main`). Changes were
manually adapted; behaviour matches #4437.

## Test plan
- [ ] `pytest tests/sources/test_sharepoint_online.py`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[SPO] Unpublished pages appear in search results

3 participants