Added user start node restrictions to sibling endpoints (#19839)

* Added user start node restrictions to sibling endpoints.

* Further integration tests.

* Tidy up.

* Apply suggestions from code review

Co-authored-by: Copilot <[email protected]>

* Revert previous update.

* Applied previous update correctly.

---------

Co-authored-by: Copilot <[email protected]>

Author: AndyButland

src/Umbraco.Cms.Api.Management/Controllers/Document/Tree/SiblingsDocumentTreeController.cs

@@ -35,6 +35,9 @@ public SiblingsDocumentTreeController(
     [HttpGet("siblings")]
     [MapToApiVersion("1.0")]
     [ProducesResponseType(typeof(IEnumerable<DocumentTreeItemResponseModel>), StatusCodes.Status200OK)]
-    public Task<ActionResult<IEnumerable<DocumentTreeItemResponseModel>>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after)
-        => GetSiblings(target, before, after);
+    public Task<ActionResult<IEnumerable<DocumentTreeItemResponseModel>>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after, Guid? dataTypeId = null)
+    {
+        IgnoreUserStartNodesForDataType(dataTypeId);
+        return GetSiblings(target, before, after);
+    }
 }

src/Umbraco.Cms.Api.Management/Controllers/Media/Tree/SiblingsMediaTreeController.cs

@@ -24,6 +24,9 @@ public SiblingsMediaTreeController(
 
     [HttpGet("siblings")]
     [ProducesResponseType(typeof(IEnumerable<MediaTreeItemResponseModel>), StatusCodes.Status200OK)]
-    public Task<ActionResult<IEnumerable<MediaTreeItemResponseModel>>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after)
-        => GetSiblings(target, before, after);
+    public Task<ActionResult<IEnumerable<MediaTreeItemResponseModel>>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after, Guid? dataTypeId = null)
+    {
+        IgnoreUserStartNodesForDataType(dataTypeId);
+        return GetSiblings(target, before, after);
+    }
 }

src/Umbraco.Cms.Api.Management/Controllers/Tree/EntityTreeControllerBase.cs

@@ -1,4 +1,4 @@
-using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc;
 using Umbraco.Cms.Api.Common.ViewModels.Pagination;
 using Umbraco.Cms.Api.Management.ViewModels;
 using Umbraco.Cms.Api.Management.ViewModels.Tree;
@@ -46,7 +46,7 @@ protected Task<ActionResult<PagedViewModel<TItem>>> GetChildren(Guid parentId, i
 
     protected Task<ActionResult<IEnumerable<TItem>>> GetSiblings(Guid target, int before, int after)
     {
-        IEntitySlim[] siblings = EntityService.GetSiblings(target, ItemObjectType, before, after, ItemOrdering).ToArray();
+        IEntitySlim[] siblings = GetSiblingEntities(target, before, after);
         if (siblings.Length == 0)
         {
             return Task.FromResult<ActionResult<IEnumerable<TItem>>>(NotFound());
@@ -110,7 +110,8 @@ protected virtual IEntitySlim[] GetPagedRootEntities(int skip, int take, out lon
             .ToArray();
 
     protected virtual IEntitySlim[] GetPagedChildEntities(Guid parentKey, int skip, int take, out long totalItems) =>
-        EntityService.GetPagedChildren(
+        EntityService
+            .GetPagedChildren(
                 parentKey,
                 ItemObjectType,
                 skip,
@@ -119,6 +120,16 @@ protected virtual IEntitySlim[] GetPagedChildEntities(Guid parentKey, int skip,
                 ordering: ItemOrdering)
             .ToArray();
 
+    protected virtual IEntitySlim[] GetSiblingEntities(Guid target, int before, int after) =>
+        EntityService
+            .GetSiblings(
+                target,
+                ItemObjectType,
+                before,
+                after,
+                ordering: ItemOrdering)
+        .ToArray();
+
     protected virtual TItem[] MapTreeItemViewModels(Guid? parentKey, IEntitySlim[] entities)
         => entities.Select(entity => MapTreeItemViewModel(parentKey, entity)).ToArray();
 

src/Umbraco.Cms.Api.Management/Controllers/Tree/UserStartNodeTreeControllerBase.cs

@@ -1,4 +1,4 @@
-using Umbraco.Cms.Api.Management.Models.Entities;
+using Umbraco.Cms.Api.Management.Models.Entities;
 using Umbraco.Cms.Api.Management.Services.Entities;
 using Umbraco.Cms.Api.Management.ViewModels.Tree;
 using Umbraco.Cms.Core;
@@ -59,6 +59,24 @@ protected override IEntitySlim[] GetPagedChildEntities(Guid parentKey, int skip,
         return CalculateAccessMap(() => userAccessEntities, out _);
     }
 
+    protected override IEntitySlim[] GetSiblingEntities(Guid target, int before, int after)
+    {
+        if (UserHasRootAccess() || IgnoreUserStartNodes())
+        {
+            return base.GetSiblingEntities(target, before, after);
+        }
+
+        IEnumerable<UserAccessEntity> userAccessEntities = _userStartNodeEntitiesService.SiblingUserAccessEntities(
+            ItemObjectType,
+            UserStartNodePaths,
+            target,
+            before,
+            after,
+            ItemOrdering);
+
+        return CalculateAccessMap(() => userAccessEntities, out _);
+    }
+
     protected override TItem[] MapTreeItemViewModels(Guid? parentKey, IEntitySlim[] entities)
     {
         if (UserHasRootAccess() || IgnoreUserStartNodes())

src/Umbraco.Cms.Api.Management/Services/Entities/IUserStartNodeEntitiesService.cs

@@ -1,4 +1,4 @@
-using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.Entities;
 using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Api.Management.Models.Entities;
@@ -64,6 +64,28 @@ IEnumerable<UserAccessEntity> ChildUserAccessEntities(
     /// </remarks>
     IEnumerable<UserAccessEntity> ChildUserAccessEntities(IEnumerable<IEntitySlim> candidateChildren, string[] userStartNodePaths);
 
+    /// <summary>
+    /// Calculates the applicable sibling entities for a given object type for users without root access.
+    /// </summary>
+    /// <param name="umbracoObjectType">The object type.</param>
+    /// <param name="userStartNodePaths">The calculated start node paths for the user.</param>
+    /// <param name="targetKey">The key of the target.</param>
+    /// <param name="before">The number of applicable siblings to retrieve before the target.</param>
+    /// <param name="after">The number of applicable siblings to retrieve after the target.</param>
+    /// <param name="ordering">The ordering to apply when fetching and paginating the children.</param>
+    /// <returns>A list of sibling entities applicable for the user.</returns>
+    /// <remarks>
+    /// The returned entities may include entities that outside of the user start node scope, but are needed to
+    /// for browsing to the actual user start nodes. These entities will be marked as "no access" entities.
+    /// </remarks>
+    IEnumerable<UserAccessEntity> SiblingUserAccessEntities(
+        UmbracoObjectTypes umbracoObjectType,
+        string[] userStartNodePaths,
+        Guid targetKey,
+        int before,
+        int after,
+        Ordering ordering) => [];
+
     /// <summary>
     /// Calculates the access level of a collection of entities for users without root access.
     /// </summary>

src/Umbraco.Cms.Api.Management/Services/Entities/UserStartNodeEntitiesService.cs

@@ -1,4 +1,4 @@
-using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.DependencyInjection;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.Entities;
@@ -36,17 +36,17 @@ public UserStartNodeEntitiesService(IEntityService entityService, ICoreScopeProv
     /// <inheritdoc />
     public IEnumerable<UserAccessEntity> RootUserAccessEntities(UmbracoObjectTypes umbracoObjectType, int[] userStartNodeIds)
     {
-        // root entities for users without root access should include:
+        // Root entities for users without root access should include:
         // - the start nodes that are actual root entities (level == 1)
         // - the root level ancestors to the rest of the start nodes (required for browsing to the actual start nodes - will be marked as "no access")
         IEntitySlim[] userStartEntities = userStartNodeIds.Any()
             ? _entityService.GetAll(umbracoObjectType, userStartNodeIds).ToArray()
             : Array.Empty<IEntitySlim>();
 
-        // find the start nodes that are at root level (level == 1)
+        // Find the start nodes that are at root level (level == 1).
         IEntitySlim[] allowedTopmostEntities = userStartEntities.Where(entity => entity.Level == 1).ToArray();
 
-        // find the root level ancestors of the rest of the start nodes, and add those as well
+        // Find the root level ancestors of the rest of the start nodes, and add those as well.
         var nonAllowedTopmostEntityIds = userStartEntities.Except(allowedTopmostEntities)
             .Select(entity => int.TryParse(entity.Path.Split(Constants.CharArrays.Comma).Skip(1).FirstOrDefault(), out var id) ? id : 0)
             .Where(id => id > 0)
@@ -63,6 +63,7 @@ public IEnumerable<UserAccessEntity> RootUserAccessEntities(UmbracoObjectTypes u
             .ToArray();
     }
 
+    /// <inheritdoc/>
     public IEnumerable<UserAccessEntity> ChildUserAccessEntities(UmbracoObjectTypes umbracoObjectType, string[] userStartNodePaths, Guid parentKey, int skip, int take, Ordering ordering, out long totalItems)
     {
         Attempt<int> parentIdAttempt = _idKeyMap.GetIdForKey(parentKey, umbracoObjectType);
@@ -83,40 +84,46 @@ public IEnumerable<UserAccessEntity> ChildUserAccessEntities(UmbracoObjectTypes
         IEntitySlim[] children;
         if (userStartNodePaths.Any(path => $"{parent.Path},".StartsWith($"{path},")))
         {
-            // the requested parent is one of the user start nodes (or a descendant of one), all children are by definition allowed
+            // The requested parent is one of the user start nodes (or a descendant of one), all children are by definition allowed.
             children = _entityService.GetPagedChildren(parentKey, umbracoObjectType, skip, take, out totalItems, ordering: ordering).ToArray();
             return ChildUserAccessEntities(children, userStartNodePaths);
         }
 
-        // if one or more of the user start nodes are descendants of the requested parent, find the "next child IDs" in those user start node paths
-        // - e.g. given the user start node path "-1,2,3,4,5", if the requested parent ID is 3, the "next child ID" is 4.
-        var userStartNodePathIds = userStartNodePaths.Select(path => path.Split(Constants.CharArrays.Comma).Select(int.Parse).ToArray()).ToArray();
-        var allowedChildIds = userStartNodePathIds
-            .Where(ids => ids.Contains(parentId))
-            // given the previous checks, the parent ID can never be the last in the user start node path, so this is safe
-            .Select(ids => ids[ids.IndexOf(parentId) + 1])
-            .Distinct()
-            .ToArray();
+        int[] allowedChildIds = GetAllowedIds(userStartNodePaths, parentId);
 
         totalItems = allowedChildIds.Length;
         if (allowedChildIds.Length == 0)
         {
-            // the requested parent is outside the scope of any user start nodes
+            // The requested parent is outside the scope of any user start nodes.
             return [];
         }
 
-        // even though we know the IDs of the allowed child entities to fetch, we still use a Query to yield correctly sorted children
+        // Even though we know the IDs of the allowed child entities to fetch, we still use a Query to yield correctly sorted children.
         IQuery<IUmbracoEntity> query = _scopeProvider.CreateQuery<IUmbracoEntity>().Where(x => allowedChildIds.Contains(x.Id));
         children = _entityService.GetPagedChildren(parentKey, umbracoObjectType, skip, take, out totalItems, query, ordering).ToArray();
         return ChildUserAccessEntities(children, userStartNodePaths);
     }
 
+    private static int[] GetAllowedIds(string[] userStartNodePaths, int parentId)
+    {
+        // If one or more of the user start nodes are descendants of the requested parent, find the "next child IDs" in those user start node paths
+        // that are the final entries in the path.
+        // E.g. given the user start node path "-1,2,3,4,5", if the requested parent ID is 3, the "next child ID" is 4.
+        var userStartNodePathIds = userStartNodePaths.Select(path => path.Split(Constants.CharArrays.Comma).Select(int.Parse).ToArray()).ToArray();
+        return userStartNodePathIds
+            .Where(ids => ids.Contains(parentId))
+            .Select(ids => ids[ids.IndexOf(parentId) + 1]) // Given the previous checks, the parent ID can never be the last in the user start node path, so this is safe
+            .Distinct()
+            .ToArray();
+    }
+
     /// <inheritdoc />
     public IEnumerable<UserAccessEntity> ChildUserAccessEntities(IEnumerable<IEntitySlim> candidateChildren, string[] userStartNodePaths)
-        // child entities for users without root access should include:
+
+        // Child or sibling entities for users without root access should include:
         // - children that are descendant-or-self of a user start node
         // - children that are ancestors of a user start node (required for browsing to the actual start nodes - will be marked as "no access")
-        // all other candidate children should be discarded
+        // All other candidate children should be discarded.
         => candidateChildren.Select(child =>
         {
             // is descendant-or-self of a start node?
@@ -134,9 +141,55 @@ public IEnumerable<UserAccessEntity> ChildUserAccessEntities(IEnumerable<IEntity
             return null;
         }).WhereNotNull().ToArray();
 
+    /// <inheritdoc />
+    public IEnumerable<UserAccessEntity> SiblingUserAccessEntities(UmbracoObjectTypes umbracoObjectType, string[] userStartNodePaths, Guid targetKey, int before, int after, Ordering ordering)
+    {
+        Attempt<int> targetIdAttempt = _idKeyMap.GetIdForKey(targetKey, umbracoObjectType);
+        if (targetIdAttempt.Success is false)
+        {
+            return [];
+        }
+
+        var targetId = targetIdAttempt.Result;
+        IEntitySlim? target = _entityService.Get(targetId);
+        if (target is null)
+        {
+            return [];
+        }
+
+        IEntitySlim[] siblings;
+
+        IEntitySlim? targetParent = _entityService.Get(target.ParentId);
+        if (targetParent is null) // Even if the parent is the root, we still expect to get a value here.
+        {
+            return [];
+        }
+
+        if (userStartNodePaths.Any(path => $"{targetParent?.Path},".StartsWith($"{path},")))
+        {
+            // The requested parent of the target is one of the user start nodes (or a descendant of one), all siblings are by definition allowed.
+            siblings = _entityService.GetSiblings(targetKey, umbracoObjectType, before, after, ordering: ordering).ToArray();
+            return ChildUserAccessEntities(siblings, userStartNodePaths);
+        }
+
+        int[] allowedSiblingIds = GetAllowedIds(userStartNodePaths, targetParent.Id);
+
+        if (allowedSiblingIds.Length == 0)
+        {
+            // The requested target is outside the scope of any user start nodes.
+            return [];
+        }
+
+        // Even though we know the IDs of the allowed sibling entities to fetch, we still use a Query to yield correctly sorted children.
+        IQuery<IUmbracoEntity> query = _scopeProvider.CreateQuery<IUmbracoEntity>().Where(x => allowedSiblingIds.Contains(x.Id));
+        siblings = _entityService.GetSiblings(targetKey, umbracoObjectType, before, after, query, ordering).ToArray();
+        return ChildUserAccessEntities(siblings, userStartNodePaths);
+    }
+
     /// <inheritdoc />
     public IEnumerable<UserAccessEntity> UserAccessEntities(IEnumerable<IEntitySlim> entities, string[] userStartNodePaths)
-        // entities for users without root access should include:
+
+        // Entities for users without root access should include:
         // - entities that are descendant-or-self of a user start node as regular entities
         // - all other entities as "no access" entities
         => entities.Select(entity => new UserAccessEntity(entity, IsDescendantOrSelf(entity, userStartNodePaths))).ToArray();

src/Umbraco.Core/Persistence/Repositories/IEntityRepository.cs

@@ -26,9 +26,10 @@ public interface IEntityRepository : IRepository
     /// <param name="targetKey">The key of the target entity whose siblings are to be retrieved.</param>
     /// <param name="before">The number of siblings to retrieve before the target entity.</param>
     /// <param name="after">The number of siblings to retrieve after the target entity.</param>
+    /// <param name="filter">An optional filter to apply to the result set.</param>
     /// <param name="ordering">The ordering to apply to the siblings.</param>
     /// <returns>Enumerable of sibling entities.</returns>
-    IEnumerable<IEntitySlim> GetSiblings(Guid objectType, Guid targetKey, int before, int after, Ordering ordering) => [];
+    IEnumerable<IEntitySlim> GetSiblings(Guid objectType, Guid targetKey, int before, int after, IQuery<IUmbracoEntity>? filter, Ordering ordering) => [];
 
     /// <summary>
     ///     Gets entities for a query

src/Umbraco.Core/Services/DateTypeServiceExtensions.cs

@@ -1,4 +1,4 @@
-using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.PropertyEditors;
 using Umbraco.Cms.Core.Services;
 
@@ -8,11 +8,6 @@ public static class DateTypeServiceExtensions
 {
     public static bool IsDataTypeIgnoringUserStartNodes(this IDataTypeService dataTypeService, Guid key)
     {
-        if (DataTypeExtensions.IsBuildInDataType(key))
-        {
-            return false; // built in ones can never be ignoring start nodes
-        }
-
         IDataType? dataType = dataTypeService.GetAsync(key).GetAwaiter().GetResult();
 
         if (dataType != null && dataType.ConfigurationObject is IIgnoreUserStartNodesConfig ignoreStartNodesConfig)

src/Umbraco.Core/Services/EntityService.cs

@@ -324,6 +324,7 @@ public IEnumerable<IEntitySlim> GetSiblings(
         UmbracoObjectTypes objectType,
         int before,
         int after,
+        IQuery<IUmbracoEntity>? filter = null,
         Ordering? ordering = null)
     {
         if (before < 0)
@@ -345,6 +346,7 @@ public IEnumerable<IEntitySlim> GetSiblings(
             key,
             before,
             after,
+            filter,
             ordering);
 
         scope.Complete();

src/Umbraco.Core/Services/IEntityService.cs

@@ -177,13 +177,15 @@ IEnumerable<IEntitySlim> GetAll<T>(params Guid[] keys)
     /// <param name="objectType">The object type key of the entities.</param>
     /// <param name="before">The number of siblings to retrieve before the target entity. Needs to be greater or equal to 0.</param>
     /// <param name="after">The number of siblings to retrieve after the target entity. Needs to be greater or equal to 0.</param>
+    /// <param name="filter">An optional filter to apply to the result set.</param>
     /// <param name="ordering">The ordering to apply to the siblings.</param>
     /// <returns>Enumerable of sibling entities.</returns>
     IEnumerable<IEntitySlim> GetSiblings(
         Guid key,
         UmbracoObjectTypes objectType,
         int before,
         int after,
+        IQuery<IUmbracoEntity>? filter = null,
         Ordering? ordering = null) => [];
 
     /// <summary>