diff --git a/app/connectors_service/NOTICE.txt b/app/connectors_service/NOTICE.txt index 474507ed8..065edcb54 100644 --- a/app/connectors_service/NOTICE.txt +++ b/app/connectors_service/NOTICE.txt @@ -4475,7 +4475,7 @@ SOFTWARE. greenlet -3.5.5 +3.5.6 MIT AND PSF-2.0 The following files are derived from Stackless Python and are subject to the same license as Stackless Python: diff --git a/app/connectors_service/connectors/sources/sharepoint/sharepoint_online/constants.py b/app/connectors_service/connectors/sources/sharepoint/sharepoint_online/constants.py index 13ef69838..e85c696c5 100644 --- a/app/connectors_service/connectors/sources/sharepoint/sharepoint_online/constants.py +++ b/app/connectors_service/connectors/sources/sharepoint/sharepoint_online/constants.py @@ -49,6 +49,7 @@ # See also: https://learn.microsoft.com/en-us/previous-versions/office/sharepoint-csom/ee536458(v=office.15) VIEW_ITEM_MASK = 0x1 # View items in lists, documents in document libraries, and Web discussion comments. VIEW_PAGE_MASK = 0x20000 # View pages in a Site. +EDIT_ITEM_MASK = 0x4 # Edit items in lists, edit documents in document libraries, and customize Web Part Pages in document libraries. # See https://github.com/pnp/pnpcore/blob/dev/src/sdk/PnP.Core/Model/SharePoint/Core/Public/Enums/RoleType.cs # See also: https://learn.microsoft.com/en-us/dotnet/api/microsoft.sharepoint.client.roletype?view=sharepoint-csom @@ -69,6 +70,14 @@ REVIEWER, SYSTEM, ] +# Excludes READER and REVIEWER (view-only). +EDIT_ROLE_TYPES = [ + CONTRIBUTOR, + WEB_DESIGNER, + ADMINISTRATOR, + EDITOR, + SYSTEM, +] # $expand param has a max of 20: see: https://developer.microsoft.com/en-us/graph/known-issues/?search=expand SPO_MAX_EXPAND_SIZE = 20 diff --git a/app/connectors_service/connectors/sources/sharepoint/sharepoint_online/datasource.py b/app/connectors_service/connectors/sources/sharepoint/sharepoint_online/datasource.py index 36c13b336..eb9f5367c 100644 --- a/app/connectors_service/connectors/sources/sharepoint/sharepoint_online/datasource.py +++ b/app/connectors_service/connectors/sources/sharepoint/sharepoint_online/datasource.py @@ -30,6 +30,8 @@ ) from connectors.sources.sharepoint.sharepoint_online.constants import ( CURSOR_SITE_DRIVE_KEY, + EDIT_ITEM_MASK, + EDIT_ROLE_TYPES, EXCLUDED_SHAREPOINT_LIST_NAMES, MAX_DOCUMENT_SIZE, SPO_API_MAX_BATCH_SIZE, @@ -56,6 +58,23 @@ from connectors.utils import html_to_text, iterable_batches_generator +def _is_page_published(version_string): + """True when the page version is a major release (e.g. "3.0").""" + if not version_string: + return True + + _, _, minor = str(version_string).partition(".") + + if not minor: + return True + + try: + return int(minor) == 0 + except ValueError: + # An unparsable minor version is no evidence of a published page + return False + + class SharepointOnlineDataSource(BaseDataSource): """Sharepoint Online""" @@ -314,17 +333,23 @@ async def _site_access_control(self, site): [ "user":spo-admin" ] + - list: site members with edit-or-higher access, plus site-admins + [ + "user:spo-admin", + "user:spo-editor" + ] """ self._logger.debug(f"Looking at site: {site['id']} with url {site['webUrl']}") if not self._dls_enabled(): - return [], [] + return [], [], [] def _is_site_admin(user): return user.get("IsSiteAdmin", False) access_control = set() site_admins_access_control = set() + editors_access_control = set() try: async for role_assignment in self.client.site_role_assignments( @@ -336,6 +361,12 @@ def _is_site_admin(user): await self._get_access_control_from_role_assignment(role_assignment) ) + editors_access_control.update( + await self._get_access_control_from_role_assignment( + role_assignment, require_edit_access=True + ) + ) + if _is_site_admin(member): # These are likely in the "Owners" group for the site site_admins_access_control |= member_access_control @@ -360,7 +391,13 @@ def _is_site_admin(user): ) raise PermissionsMissing(msg) from e - return list(access_control), list(site_admins_access_control) + editors_access_control |= site_admins_access_control + + return ( + list(access_control), + list(site_admins_access_control), + list(editors_access_control), + ) def _dls_enabled(self): if self._features is None: @@ -605,6 +642,7 @@ async def get_docs(self, filtering=None): ( site_access_control, site_admin_access_control, + site_editors_access_control, ) = await self._site_access_control(site) yield ( @@ -676,7 +714,9 @@ async def get_docs(self, filtering=None): yield list_item, download_func # Sync site pages - async for site_page in self.site_pages(site, site_access_control): + async for site_page in self.site_pages( + site, site_access_control, site_editors_access_control + ): # Always include site admins in site page access controls site_page = self._decorate_with_access_control( site_page, site_admin_access_control @@ -708,6 +748,7 @@ async def get_docs_incrementally(self, sync_cursor, filtering=None): ( site_access_control, site_admin_access_control, + site_editors_access_control, ) = await self._site_access_control(site) yield ( @@ -789,7 +830,10 @@ async def get_docs_incrementally(self, sync_cursor, filtering=None): # Sync site pages async for site_page in self.site_pages( - site, site_access_control, check_timestamp=True + site, + site_access_control, + site_editors_access_control, + check_timestamp=True, ): # Always include site admins in site page access controls site_page = self._decorate_with_access_control( @@ -1122,11 +1166,14 @@ async def site_lists(self, site, site_access_control, check_timestamp=False): yield site_list - async def _get_access_control_from_role_assignment(self, role_assignment): + async def _get_access_control_from_role_assignment( + self, role_assignment, require_edit_access=False + ): """Extracts access control from a role assignment. Args: role_assignment (dict): dictionary representing a role assignment. + require_edit_access (bool): when True, return only members with edit access. Returns: access_control (list): list of usernames and dynamic group ids, which have the role assigned. @@ -1135,7 +1182,7 @@ async def _get_access_control_from_role_assignment(self, role_assignment): If any role is assigned to a user this means at least "read" access. """ - def _has_limited_access(role_assignment): + def _grants_access(role_assignment): bindings = role_assignment.get("RoleDefinitionBindings", []) # If there is no permission information, default to restrict access @@ -1143,9 +1190,8 @@ def _has_limited_access(role_assignment): self._logger.debug( f"No RoleDefinitionBindings found for '{role_assignment.get('odata.id')}'" ) - return True + return False - # if any binding grants view access, this role assignment's member has view access for binding in bindings: # full explanation of the bit-math: https://stackoverflow.com/questions/51897160/how-to-parse-getusereffectivepermissions-sharepoint-response-in-java # this approach was confirmed as valid by a Microsoft Sr. Support Escalation Engineer @@ -1153,18 +1199,21 @@ def _has_limited_access(role_assignment): nested_get_from_dict(binding, ["BasePermissions", "Low"], "0") # pyright: ignore ) role_type_kind = binding.get("RoleTypeKind", 0) - if ( + if require_edit_access: + if (base_permission_low & EDIT_ITEM_MASK) or ( + role_type_kind in EDIT_ROLE_TYPES + ): + return True + elif ( (base_permission_low & VIEW_ITEM_MASK) or (base_permission_low & VIEW_PAGE_MASK) or (role_type_kind in VIEW_ROLE_TYPES) ): - return False + return True - return ( - True # no evidence of view access was found, so assuming limited access - ) + return False - if _has_limited_access(role_assignment): + if not _grants_access(role_assignment): return [] access_control = [] @@ -1189,7 +1238,13 @@ def _has_limited_access(role_assignment): return access_control - async def site_pages(self, site, site_access_control, check_timestamp=False): + async def site_pages( + self, + site, + site_access_control, + site_editors_access_control, + check_timestamp=False, + ): site_id = site["id"] url = site["webUrl"] async for site_page in self.client.site_pages(url): @@ -1206,6 +1261,13 @@ async def site_pages(self, site, site_access_control, check_timestamp=False): site_page["_id"] = f"{site_id}-site_page-{site_page['Id']}" site_page["object_type"] = "site_page" + published = _is_page_published(site_page.get("OData__UIVersionString")) + site_page["published"] = published + if not published: + self._logger.debug( + f"Unpublished site page '{site_page['_id']}'; restricting ACL to owners/editors." + ) + has_unique_role_assignments = False # ignore parent site permissions and use unique per page permissions ("unique permissions" means breaking the inheritance to the parent site) @@ -1233,7 +1295,8 @@ async def site_pages(self, site, site_access_control, check_timestamp=False): ): page_access_control.extend( await self._get_access_control_from_role_assignment( - role_assignment + role_assignment, + require_edit_access=not published, ) ) @@ -1241,10 +1304,14 @@ async def site_pages(self, site, site_access_control, check_timestamp=False): site_page, page_access_control ) - # set parent site access control if not has_unique_role_assignments: + inherited_access_control = ( + site_access_control + if published + else site_editors_access_control + ) site_page = self._decorate_with_access_control( - site_page, site_access_control + site_page, inherited_access_control ) for html_field in [ diff --git a/app/connectors_service/tests/sources/test_sharepoint_online.py b/app/connectors_service/tests/sources/test_sharepoint_online.py index 98cbba41a..473e8476c 100644 --- a/app/connectors_service/tests/sources/test_sharepoint_online.py +++ b/app/connectors_service/tests/sources/test_sharepoint_online.py @@ -49,6 +49,9 @@ EXCLUDED_SHAREPOINT_LIST_NAMES, WILDCARD, ) +from connectors.sources.sharepoint.sharepoint_online.datasource import ( + _is_page_published, +) from connectors.sources.sharepoint.sharepoint_online.utils import ( DeltaLinkExpired, SyncCursorEmpty, @@ -153,6 +156,18 @@ } ] +EDIT_BINDING = [ + { + "BasePermissions": {"High": "432", "Low": "1011030767"}, + "Description": "Can add, edit and delete lists; can view, add, update and delete list items and documents.", + "Hidden": False, + "Id": 1073741827, + "Name": "Edit", + "Order": 48, + "RoleTypeKind": 6, + } +] + SAMPLE_DRIVE_PERMISSIONS = [ { "id": "3", @@ -2563,7 +2578,7 @@ async def test_get_docs_with_access_control(self, patch_sharepoint_client): async with create_spo_source(use_document_level_security=True) as source: source._site_access_control = AsyncMock( - return_value=(expected_access_control, []) + return_value=(expected_access_control, [], expected_access_control) ) results = [] @@ -2673,7 +2688,7 @@ async def test_get_docs_incrementally_with_empty_cursor( @freeze_time(iso_utc()) async def test_get_docs_incrementally(self, patch_sharepoint_client): async with create_spo_source() as source: - source._site_access_control = AsyncMock(return_value=([], [])) + source._site_access_control = AsyncMock(return_value=([], [], [])) # mock cache lookup source.site_group_users = AsyncMock(return_value=self.site_group_users) @@ -3194,7 +3209,7 @@ async def test_drive_items_permissions_when_fetch_drive_item_permissions_enabled async with create_spo_source(use_document_level_security=True) as source: source._site_access_control = AsyncMock( - return_value=(site_access_controls, []) + return_value=(site_access_controls, [], site_access_controls) ) results = [] @@ -3243,7 +3258,11 @@ async def test_site_page_permissions_when_fetch_drive_item_permissions_enabled( async with create_spo_source(use_document_level_security=True) as source: source._site_access_control = AsyncMock( - return_value=([], admin_site_access_controls) + return_value=( + [], + admin_site_access_controls, + admin_site_access_controls, + ) ) results = [] @@ -3707,7 +3726,7 @@ async def test_site_access_control(self, patch_sharepoint_client): site = {"id": 1, "webUrl": "some url"} - access_control, _ = await source._site_access_control(site) + access_control, _, _ = await source._site_access_control(site) two_users = 2 two_groups = 2 @@ -4248,6 +4267,244 @@ async def test_get_access_control_from_role_assignment( identity in access_control for identity in expected_access_control ) + @pytest.mark.parametrize( + "role_assignment, expected_access_control", + [ + ( + # Reader (view-only) is excluded when edit access is required + { + "Member": { + "odata.type": "SP.User", + "UserPrincipalName": USER_ONE_EMAIL, + }, + "RoleDefinitionBindings": READ_BINDING, + }, + [], + ), + ( + # "Reader" role type (view-only) is excluded + { + "Member": { + "odata.type": "SP.User", + "UserPrincipalName": USER_ONE_EMAIL, + }, + "RoleDefinitionBindings": [{"RoleTypeKind": 2}], + }, + [], + ), + ( + # View item mask only is excluded when edit access is required + { + "Member": { + "odata.type": "SP.User", + "UserPrincipalName": USER_ONE_EMAIL, + }, + "RoleDefinitionBindings": [{"BasePermissions": {"Low": "1"}}], + }, + [], + ), + ( + # Editor binding grants edit access + { + "Member": { + "odata.type": "SP.User", + "UserPrincipalName": USER_ONE_EMAIL, + }, + "RoleDefinitionBindings": EDIT_BINDING, + }, + [_prefix_user(USER_ONE_EMAIL)], + ), + ( + # "Contributor" role type grants edit access + { + "Member": { + "odata.type": "SP.User", + "UserPrincipalName": USER_ONE_EMAIL, + }, + "RoleDefinitionBindings": [{"RoleTypeKind": 3}], + }, + [_prefix_user(USER_ONE_EMAIL)], + ), + ( + # Edit item mask only grants edit access + { + "Member": { + "odata.type": "SP.User", + "UserPrincipalName": USER_ONE_EMAIL, + }, + "RoleDefinitionBindings": [{"BasePermissions": {"Low": "4"}}], + }, + [_prefix_user(USER_ONE_EMAIL)], + ), + ], + ) + @pytest.mark.asyncio + async def test_get_access_control_from_role_assignment_require_edit_access( + self, role_assignment, expected_access_control + ): + async with create_spo_source() as source: + access_control = await source._get_access_control_from_role_assignment( + role_assignment, require_edit_access=True + ) + + assert len(access_control) == len(expected_access_control) + assert all( + identity in access_control for identity in expected_access_control + ) + + @pytest.mark.parametrize( + "version_string, expected_published", + [ + ("1.0", True), + ("3.0", True), + ("12.0", True), + ("0.1", False), + ("3.1", False), + ("2.5", False), + # Missing/undeterminable version defaults to published + (None, True), + ("", True), + ("5", True), + ("not-a-version", True), + # An unparsable minor version is not evidence of a published page + ("3.foo", False), + ("1.x", False), + ], + ) + def test_is_page_published(self, version_string, expected_published): + assert _is_page_published(version_string) == expected_published + + @pytest.mark.asyncio + @patch( + "connectors.sources.sharepoint.sharepoint_online.datasource.ACCESS_CONTROL", + ALLOW_ACCESS_CONTROL_PATCHED, + ) + async def test_site_pages_unpublished_page_with_unique_permissions_excludes_viewers( + self, patch_sharepoint_client + ): + viewer = { + "Member": {"odata.type": "SP.User", "UserPrincipalName": USER_ONE_EMAIL}, + "RoleDefinitionBindings": READ_BINDING, + } + editor = { + "Member": {"odata.type": "SP.User", "UserPrincipalName": USER_TWO_EMAIL}, + "RoleDefinitionBindings": EDIT_BINDING, + } + + async with create_spo_source(use_document_level_security=True) as source: + source.client.site_pages = AsyncIterator( + [ + { + "Id": "4", + "Modified": self.day_ago, + "OData__UIVersionString": "2.1", + } + ] + ) + source.client.site_page_has_unique_role_assignments = AsyncMock( + return_value=True + ) + source.client.site_page_role_assignments = AsyncIterator([viewer, editor]) + + pages = [ + page + async for page in source.site_pages( + {"id": "1", "webUrl": "http://fakesharepoint.com/site"}, + site_access_control=[], + site_editors_access_control=[], + ) + ] + + assert len(pages) == 1 + page = pages[0] + assert page["published"] is False + access_control = page[ALLOW_ACCESS_CONTROL_PATCHED] + assert _prefix_user(USER_TWO_EMAIL) in access_control + assert _prefix_user(USER_ONE_EMAIL) not in access_control + + @pytest.mark.asyncio + @patch( + "connectors.sources.sharepoint.sharepoint_online.datasource.ACCESS_CONTROL", + ALLOW_ACCESS_CONTROL_PATCHED, + ) + async def test_site_pages_published_page_with_unique_permissions_includes_viewers( + self, patch_sharepoint_client + ): + viewer = { + "Member": {"odata.type": "SP.User", "UserPrincipalName": USER_ONE_EMAIL}, + "RoleDefinitionBindings": READ_BINDING, + } + + async with create_spo_source(use_document_level_security=True) as source: + source.client.site_pages = AsyncIterator( + [ + { + "Id": "4", + "Modified": self.day_ago, + "OData__UIVersionString": "2.0", + } + ] + ) + source.client.site_page_has_unique_role_assignments = AsyncMock( + return_value=True + ) + source.client.site_page_role_assignments = AsyncIterator([viewer]) + + pages = [ + page + async for page in source.site_pages( + {"id": "1", "webUrl": "http://fakesharepoint.com/site"}, + site_access_control=[], + site_editors_access_control=[], + ) + ] + + assert len(pages) == 1 + page = pages[0] + assert page["published"] is True + assert _prefix_user(USER_ONE_EMAIL) in page[ALLOW_ACCESS_CONTROL_PATCHED] + + @pytest.mark.asyncio + @patch( + "connectors.sources.sharepoint.sharepoint_online.datasource.ACCESS_CONTROL", + ALLOW_ACCESS_CONTROL_PATCHED, + ) + async def test_site_pages_unpublished_inherited_page_uses_editors_access_control( + self, patch_sharepoint_client + ): + site_access_control = [_prefix_user("viewer")] + site_editors_access_control = [_prefix_user("editor")] + + async with create_spo_source(use_document_level_security=True) as source: + source.client.site_pages = AsyncIterator( + [ + { + "Id": "4", + "Modified": self.day_ago, + "OData__UIVersionString": "2.1", + } + ] + ) + source.client.site_page_has_unique_role_assignments = AsyncMock( + return_value=False + ) + + pages = [ + page + async for page in source.site_pages( + {"id": "1", "webUrl": "http://fakesharepoint.com/site"}, + site_access_control=site_access_control, + site_editors_access_control=site_editors_access_control, + ) + ] + + assert len(pages) == 1 + page = pages[0] + assert page["published"] is False + access_control = page[ALLOW_ACCESS_CONTROL_PATCHED] + assert _prefix_user("editor") in access_control + assert _prefix_user("viewer") not in access_control + @pytest.mark.parametrize( "raw_login_name, expected_login_name", [