From 20254f0bbc11d749239554876b2f97e82eb05c7c Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Tue, 5 Aug 2025 09:53:39 +0200 Subject: [PATCH] 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 <175728472+Copilot@users.noreply.github.com> * Revert previous update. * Applied previous update correctly. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Tree/SiblingsDocumentTreeController.cs | 7 +- .../Media/Tree/SiblingsMediaTreeController.cs | 7 +- .../Tree/EntityTreeControllerBase.cs | 17 ++- .../Tree/UserStartNodeTreeControllerBase.cs | 20 ++- .../Entities/IUserStartNodeEntitiesService.cs | 24 +++- .../Entities/UserStartNodeEntitiesService.cs | 91 +++++++++--- .../Repositories/IEntityRepository.cs | 3 +- .../Services/DateTypeServiceExtensions.cs | 7 +- src/Umbraco.Core/Services/EntityService.cs | 2 + src/Umbraco.Core/Services/IEntityService.cs | 2 + .../Implement/EntityRepository.cs | 30 +++- ...iceMediaTests.SiblingUserAccessEntities.cs | 131 ++++++++++++++++++ ...iesServiceTests.ChildUserAccessEntities.cs | 2 +- ...sServiceTests.SiblingUserAccessEntities.cs | 131 ++++++++++++++++++ .../UserStartNodeEntitiesServiceTests.cs | 4 +- .../Services/EntityServiceTests.cs | 59 ++++++-- .../Umbraco.Tests.Integration.csproj | 6 + 17 files changed, 492 insertions(+), 51 deletions(-) create mode 100644 tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceMediaTests.SiblingUserAccessEntities.cs create mode 100644 tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceTests.SiblingUserAccessEntities.cs diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Document/Tree/SiblingsDocumentTreeController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Document/Tree/SiblingsDocumentTreeController.cs index 3fec79bf36..70445d525f 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Document/Tree/SiblingsDocumentTreeController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Document/Tree/SiblingsDocumentTreeController.cs @@ -35,6 +35,9 @@ public class SiblingsDocumentTreeController : DocumentTreeControllerBase [HttpGet("siblings")] [MapToApiVersion("1.0")] [ProducesResponseType(typeof(IEnumerable), StatusCodes.Status200OK)] - public Task>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after) - => GetSiblings(target, before, after); + public Task>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after, Guid? dataTypeId = null) + { + IgnoreUserStartNodesForDataType(dataTypeId); + return GetSiblings(target, before, after); + } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Media/Tree/SiblingsMediaTreeController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Media/Tree/SiblingsMediaTreeController.cs index f5708fa638..4cc855fa6d 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Media/Tree/SiblingsMediaTreeController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Media/Tree/SiblingsMediaTreeController.cs @@ -24,6 +24,9 @@ public class SiblingsMediaTreeController : MediaTreeControllerBase [HttpGet("siblings")] [ProducesResponseType(typeof(IEnumerable), StatusCodes.Status200OK)] - public Task>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after) - => GetSiblings(target, before, after); + public Task>> Siblings(CancellationToken cancellationToken, Guid target, int before, int after, Guid? dataTypeId = null) + { + IgnoreUserStartNodesForDataType(dataTypeId); + return GetSiblings(target, before, after); + } } diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Tree/EntityTreeControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/Tree/EntityTreeControllerBase.cs index 13bbe9bc2b..9b35fb2923 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Tree/EntityTreeControllerBase.cs +++ b/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 @@ public abstract class EntityTreeControllerBase : ManagementApiControllerB protected Task>> 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>>(NotFound()); @@ -110,7 +110,8 @@ public abstract class EntityTreeControllerBase : ManagementApiControllerB .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 @@ public abstract class EntityTreeControllerBase : ManagementApiControllerB 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(); diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Tree/UserStartNodeTreeControllerBase.cs b/src/Umbraco.Cms.Api.Management/Controllers/Tree/UserStartNodeTreeControllerBase.cs index 8c15708f1f..505330bdbd 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Tree/UserStartNodeTreeControllerBase.cs +++ b/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 @@ public abstract class UserStartNodeTreeControllerBase : EntityTreeControl return CalculateAccessMap(() => userAccessEntities, out _); } + protected override IEntitySlim[] GetSiblingEntities(Guid target, int before, int after) + { + if (UserHasRootAccess() || IgnoreUserStartNodes()) + { + return base.GetSiblingEntities(target, before, after); + } + + IEnumerable userAccessEntities = _userStartNodeEntitiesService.SiblingUserAccessEntities( + ItemObjectType, + UserStartNodePaths, + target, + before, + after, + ItemOrdering); + + return CalculateAccessMap(() => userAccessEntities, out _); + } + protected override TItem[] MapTreeItemViewModels(Guid? parentKey, IEntitySlim[] entities) { if (UserHasRootAccess() || IgnoreUserStartNodes()) diff --git a/src/Umbraco.Cms.Api.Management/Services/Entities/IUserStartNodeEntitiesService.cs b/src/Umbraco.Cms.Api.Management/Services/Entities/IUserStartNodeEntitiesService.cs index 2753bd29b8..5fea446bec 100644 --- a/src/Umbraco.Cms.Api.Management/Services/Entities/IUserStartNodeEntitiesService.cs +++ b/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 @@ public interface IUserStartNodeEntitiesService /// IEnumerable ChildUserAccessEntities(IEnumerable candidateChildren, string[] userStartNodePaths); + /// + /// Calculates the applicable sibling entities for a given object type for users without root access. + /// + /// The object type. + /// The calculated start node paths for the user. + /// The key of the target. + /// The number of applicable siblings to retrieve before the target. + /// The number of applicable siblings to retrieve after the target. + /// The ordering to apply when fetching and paginating the children. + /// A list of sibling entities applicable for the user. + /// + /// 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. + /// + IEnumerable SiblingUserAccessEntities( + UmbracoObjectTypes umbracoObjectType, + string[] userStartNodePaths, + Guid targetKey, + int before, + int after, + Ordering ordering) => []; + /// /// Calculates the access level of a collection of entities for users without root access. /// diff --git a/src/Umbraco.Cms.Api.Management/Services/Entities/UserStartNodeEntitiesService.cs b/src/Umbraco.Cms.Api.Management/Services/Entities/UserStartNodeEntitiesService.cs index c811d2c9c7..6d53d2ef38 100644 --- a/src/Umbraco.Cms.Api.Management/Services/Entities/UserStartNodeEntitiesService.cs +++ b/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 class UserStartNodeEntitiesService : IUserStartNodeEntitiesService /// public IEnumerable 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(); - // 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 class UserStartNodeEntitiesService : IUserStartNodeEntitiesService .ToArray(); } + /// public IEnumerable ChildUserAccessEntities(UmbracoObjectTypes umbracoObjectType, string[] userStartNodePaths, Guid parentKey, int skip, int take, Ordering ordering, out long totalItems) { Attempt parentIdAttempt = _idKeyMap.GetIdForKey(parentKey, umbracoObjectType); @@ -83,40 +84,46 @@ public class UserStartNodeEntitiesService : IUserStartNodeEntitiesService 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 query = _scopeProvider.CreateQuery().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(); + } + /// public IEnumerable ChildUserAccessEntities(IEnumerable 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 class UserStartNodeEntitiesService : IUserStartNodeEntitiesService return null; }).WhereNotNull().ToArray(); + /// + public IEnumerable SiblingUserAccessEntities(UmbracoObjectTypes umbracoObjectType, string[] userStartNodePaths, Guid targetKey, int before, int after, Ordering ordering) + { + Attempt 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 query = _scopeProvider.CreateQuery().Where(x => allowedSiblingIds.Contains(x.Id)); + siblings = _entityService.GetSiblings(targetKey, umbracoObjectType, before, after, query, ordering).ToArray(); + return ChildUserAccessEntities(siblings, userStartNodePaths); + } + /// public IEnumerable UserAccessEntities(IEnumerable 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(); diff --git a/src/Umbraco.Core/Persistence/Repositories/IEntityRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IEntityRepository.cs index cdf05ca5aa..31b9c53983 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IEntityRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IEntityRepository.cs @@ -26,9 +26,10 @@ public interface IEntityRepository : IRepository /// The key of the target entity whose siblings are to be retrieved. /// The number of siblings to retrieve before the target entity. /// The number of siblings to retrieve after the target entity. + /// An optional filter to apply to the result set. /// The ordering to apply to the siblings. /// Enumerable of sibling entities. - IEnumerable GetSiblings(Guid objectType, Guid targetKey, int before, int after, Ordering ordering) => []; + IEnumerable GetSiblings(Guid objectType, Guid targetKey, int before, int after, IQuery? filter, Ordering ordering) => []; /// /// Gets entities for a query diff --git a/src/Umbraco.Core/Services/DateTypeServiceExtensions.cs b/src/Umbraco.Core/Services/DateTypeServiceExtensions.cs index ac7e8550b2..b0cd6af6dc 100644 --- a/src/Umbraco.Core/Services/DateTypeServiceExtensions.cs +++ b/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) diff --git a/src/Umbraco.Core/Services/EntityService.cs b/src/Umbraco.Core/Services/EntityService.cs index 0a284bdebf..f9408398e1 100644 --- a/src/Umbraco.Core/Services/EntityService.cs +++ b/src/Umbraco.Core/Services/EntityService.cs @@ -324,6 +324,7 @@ public class EntityService : RepositoryService, IEntityService UmbracoObjectTypes objectType, int before, int after, + IQuery? filter = null, Ordering? ordering = null) { if (before < 0) @@ -345,6 +346,7 @@ public class EntityService : RepositoryService, IEntityService key, before, after, + filter, ordering); scope.Complete(); diff --git a/src/Umbraco.Core/Services/IEntityService.cs b/src/Umbraco.Core/Services/IEntityService.cs index 6652062ac0..1c979f4e4f 100644 --- a/src/Umbraco.Core/Services/IEntityService.cs +++ b/src/Umbraco.Core/Services/IEntityService.cs @@ -177,6 +177,7 @@ public interface IEntityService /// The object type key of the entities. /// The number of siblings to retrieve before the target entity. Needs to be greater or equal to 0. /// The number of siblings to retrieve after the target entity. Needs to be greater or equal to 0. + /// An optional filter to apply to the result set. /// The ordering to apply to the siblings. /// Enumerable of sibling entities. IEnumerable GetSiblings( @@ -184,6 +185,7 @@ public interface IEntityService UmbracoObjectTypes objectType, int before, int after, + IQuery? filter = null, Ordering? ordering = null) => []; /// diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs index c39545f6be..6e7349bf81 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/EntityRepository.cs @@ -146,7 +146,13 @@ internal sealed class EntityRepository : RepositoryBase, IEntityRepositoryExtend } /// - public IEnumerable GetSiblings(Guid objectType, Guid targetKey, int before, int after, Ordering ordering) + public IEnumerable GetSiblings( + Guid objectType, + Guid targetKey, + int before, + int after, + IQuery? filter, + Ordering ordering) { // Ideally we don't want to have to do a second query for the parent ID, but the siblings query is already messy enough // without us also having to do a nested query for the parent ID too. @@ -167,6 +173,23 @@ internal sealed class EntityRepository : RepositoryBase, IEntityRepositoryExtend .From() .Where(x => x.ParentId == parentId && x.Trashed == false); + // Apply the filter if provided. Note that in doing this, we'll add more parameters to the query, so need to track + // how many so we can offset the parameter indexes for the "before" and "after" values added later. + int beforeAfterParameterIndexOffset = 0; + if (filter != null) + { + foreach (Tuple filterClause in filter.GetWhereClauses()) + { + rowNumberSql.Where(filterClause.Item1, filterClause.Item2); + + // We need to offset by one for each non-array parameter in the filter clause. + // If a query is created using Contains or some other set based operation, we'll get both the array and the + // items in the array provided in the where clauses. It's only the latter that count for applying parameters + // to the SQL statement, and hence we should only offset by them. + beforeAfterParameterIndexOffset += filterClause.Item2.Count(x => !x.GetType().IsArray); + } + } + // Find the specific row number of the target node. // We need this to determine the bounds of the row numbers to select. Sql targetRowSql = Sql() @@ -180,11 +203,12 @@ internal sealed class EntityRepository : RepositoryBase, IEntityRepositoryExtend IEnumerable afterArguments = targetRowSql.Arguments.Concat([after]); // Select the UniqueId of nodes which row number is within the specified range of the target node's row number. + const int BeforeAfterParameterIndex = 3; Sql? mainSql = Sql() .Select("UniqueId") .From().AppendSubQuery(rowNumberSql, "NumberedNodes") - .Where($"rn >= ({targetRowSql.SQL}) - @3", beforeArguments.ToArray()) - .Where($"rn <= ({targetRowSql.SQL}) + @3", afterArguments.ToArray()) + .Where($"rn >= ({targetRowSql.SQL}) - @{BeforeAfterParameterIndex + beforeAfterParameterIndexOffset}", beforeArguments.ToArray()) + .Where($"rn <= ({targetRowSql.SQL}) + @{BeforeAfterParameterIndex + beforeAfterParameterIndexOffset}", afterArguments.ToArray()) .OrderBy("rn"); List? keys = Database.Fetch(mainSql); diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceMediaTests.SiblingUserAccessEntities.cs b/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceMediaTests.SiblingUserAccessEntities.cs new file mode 100644 index 0000000000..e481b1c0ac --- /dev/null +++ b/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceMediaTests.SiblingUserAccessEntities.cs @@ -0,0 +1,131 @@ +using NUnit.Framework; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Tests.Integration.ManagementApi.Services; + +public partial class UserStartNodeEntitiesServiceMediaTests +{ + [Test] + public async Task SiblingUserAccessEntities_WithStartNodeOfTargetParent_YieldsAll_AsAllowed() + { + var mediaStartNodePaths = await CreateUserAndGetStartNodePaths(_mediaByName["1"].Id); + + var siblings = UserStartNodeEntitiesService + .SiblingUserAccessEntities( + UmbracoObjectTypes.Media, + mediaStartNodePaths, + _mediaByName["1-5"].Key, + 2, + 2, + BySortOrder) + .ToArray(); + + Assert.AreEqual(5, siblings.Length); + Assert.Multiple(() => + { + for (int i = 0; i < 4; i++) + { + Assert.AreEqual(_mediaByName[$"1-{i + 3}"].Key, siblings[i].Entity.Key); + Assert.IsTrue(siblings[i].HasAccess); + } + }); + } + + [Test] + public async Task SiblingUserAccessEntities_WithStartNodeOfTargetParentAndTarget_YieldsOnlyTarget_AsAllowed() + { + // See notes on ChildUserAccessEntities_ChildAndGrandchildAsStartNode_AllowsOnlyGrandchild. + + var contentStartNodePaths = await CreateUserAndGetStartNodePaths(_mediaByName["1"].Id, _mediaByName["1-5"].Id); + + var siblings = UserStartNodeEntitiesService + .SiblingUserAccessEntities( + UmbracoObjectTypes.Media, + contentStartNodePaths, + _mediaByName["1-5"].Key, + 2, + 2, + BySortOrder) + .ToArray(); + + Assert.AreEqual(1, siblings.Length); + Assert.Multiple(() => + { + Assert.AreEqual(_mediaByName[$"1-5"].Key, siblings[0].Entity.Key); + Assert.IsTrue(siblings[0].HasAccess); + }); + } + + [Test] + public async Task SiblingUserAccessEntities_WithStartNodeOfTarget_YieldsOnlyTarget_AsAllowed() + { + var contentStartNodePaths = await CreateUserAndGetStartNodePaths(_mediaByName["1-5"].Id); + + var siblings = UserStartNodeEntitiesService + .SiblingUserAccessEntities( + UmbracoObjectTypes.Media, + contentStartNodePaths, + _mediaByName["1-5"].Key, + 2, + 2, + BySortOrder) + .ToArray(); + + Assert.AreEqual(1, siblings.Length); + Assert.Multiple(() => + { + Assert.AreEqual(_mediaByName[$"1-5"].Key, siblings[0].Entity.Key); + Assert.IsTrue(siblings[0].HasAccess); + }); + } + + [Test] + public async Task SiblingUserAccessEntities_WithStartsNodesOfTargetAndSiblings_YieldsOnlyPermitted_AsAllowed() + { + var mediaStartNodePaths = await CreateUserAndGetStartNodePaths(_mediaByName["1-3"].Id, _mediaByName["1-5"].Id, _mediaByName["1-7"].Id); + + var siblings = UserStartNodeEntitiesService + .SiblingUserAccessEntities( + UmbracoObjectTypes.Media, + mediaStartNodePaths, + _mediaByName["1-5"].Key, + 2, + 2, + BySortOrder) + .ToArray(); + + Assert.AreEqual(3, siblings.Length); + Assert.Multiple(() => + { + Assert.AreEqual(_mediaByName[$"1-3"].Key, siblings[0].Entity.Key); + Assert.IsTrue(siblings[0].HasAccess); + Assert.AreEqual(_mediaByName[$"1-5"].Key, siblings[1].Entity.Key); + Assert.IsTrue(siblings[1].HasAccess); + Assert.AreEqual(_mediaByName[$"1-7"].Key, siblings[2].Entity.Key); + Assert.IsTrue(siblings[2].HasAccess); + }); + } + + [Test] + public async Task SiblingUserAccessEntities_WithStartsNodesOfTargetGrandchild_YieldsTarget_AsNotAllowed() + { + var mediaStartNodePaths = await CreateUserAndGetStartNodePaths(_mediaByName["1-5-1"].Id); + + var siblings = UserStartNodeEntitiesService + .SiblingUserAccessEntities( + UmbracoObjectTypes.Media, + mediaStartNodePaths, + _mediaByName["1-5"].Key, + 2, + 2, + BySortOrder) + .ToArray(); + + Assert.AreEqual(1, siblings.Length); + Assert.Multiple(() => + { + Assert.AreEqual(_mediaByName[$"1-5"].Key, siblings[0].Entity.Key); + Assert.IsFalse(siblings[0].HasAccess); + }); + } +} diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceTests.ChildUserAccessEntities.cs b/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceTests.ChildUserAccessEntities.cs index 2272b60bbd..c64b61810f 100644 --- a/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceTests.ChildUserAccessEntities.cs +++ b/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceTests.ChildUserAccessEntities.cs @@ -1,4 +1,4 @@ -using NUnit.Framework; +using NUnit.Framework; using Umbraco.Cms.Core.Models; namespace Umbraco.Cms.Tests.Integration.ManagementApi.Services; diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceTests.SiblingUserAccessEntities.cs b/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceTests.SiblingUserAccessEntities.cs new file mode 100644 index 0000000000..839a0edc02 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceTests.SiblingUserAccessEntities.cs @@ -0,0 +1,131 @@ +using NUnit.Framework; +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Tests.Integration.ManagementApi.Services; + +public partial class UserStartNodeEntitiesServiceTests +{ + [Test] + public async Task SiblingUserAccessEntities_WithStartNodeOfTargetParent_YieldsAll_AsAllowed() + { + var contentStartNodePaths = await CreateUserAndGetStartNodePaths(_contentByName["1"].Id); + + var siblings = UserStartNodeEntitiesService + .SiblingUserAccessEntities( + UmbracoObjectTypes.Document, + contentStartNodePaths, + _contentByName["1-5"].Key, + 2, + 2, + BySortOrder) + .ToArray(); + + Assert.AreEqual(5, siblings.Length); + Assert.Multiple(() => + { + for (int i = 0; i < 4; i++) + { + Assert.AreEqual(_contentByName[$"1-{i + 3}"].Key, siblings[i].Entity.Key); + Assert.IsTrue(siblings[i].HasAccess); + } + }); + } + + [Test] + public async Task SiblingUserAccessEntities_WithStartNodeOfTargetParentAndTarget_YieldsOnlyTarget_AsAllowed() + { + // See notes on ChildUserAccessEntities_ChildAndGrandchildAsStartNode_AllowsOnlyGrandchild. + + var contentStartNodePaths = await CreateUserAndGetStartNodePaths(_contentByName["1"].Id, _contentByName["1-5"].Id); + + var siblings = UserStartNodeEntitiesService + .SiblingUserAccessEntities( + UmbracoObjectTypes.Document, + contentStartNodePaths, + _contentByName["1-5"].Key, + 2, + 2, + BySortOrder) + .ToArray(); + + Assert.AreEqual(1, siblings.Length); + Assert.Multiple(() => + { + Assert.AreEqual(_contentByName[$"1-5"].Key, siblings[0].Entity.Key); + Assert.IsTrue(siblings[0].HasAccess); + }); + } + + [Test] + public async Task SiblingUserAccessEntities_WithStartNodeOfTarget_YieldsOnlyTarget_AsAllowed() + { + var contentStartNodePaths = await CreateUserAndGetStartNodePaths(_contentByName["1-5"].Id); + + var siblings = UserStartNodeEntitiesService + .SiblingUserAccessEntities( + UmbracoObjectTypes.Document, + contentStartNodePaths, + _contentByName["1-5"].Key, + 2, + 2, + BySortOrder) + .ToArray(); + + Assert.AreEqual(1, siblings.Length); + Assert.Multiple(() => + { + Assert.AreEqual(_contentByName[$"1-5"].Key, siblings[0].Entity.Key); + Assert.IsTrue(siblings[0].HasAccess); + }); + } + + [Test] + public async Task SiblingUserAccessEntities_WithStartsNodesOfTargetAndSiblings_YieldsOnlyPermitted_AsAllowed() + { + var contentStartNodePaths = await CreateUserAndGetStartNodePaths(_contentByName["1-3"].Id, _contentByName["1-5"].Id, _contentByName["1-7"].Id); + + var siblings = UserStartNodeEntitiesService + .SiblingUserAccessEntities( + UmbracoObjectTypes.Document, + contentStartNodePaths, + _contentByName["1-5"].Key, + 2, + 2, + BySortOrder) + .ToArray(); + + Assert.AreEqual(3, siblings.Length); + Assert.Multiple(() => + { + Assert.AreEqual(_contentByName[$"1-3"].Key, siblings[0].Entity.Key); + Assert.IsTrue(siblings[0].HasAccess); + Assert.AreEqual(_contentByName[$"1-5"].Key, siblings[1].Entity.Key); + Assert.IsTrue(siblings[1].HasAccess); + Assert.AreEqual(_contentByName[$"1-7"].Key, siblings[2].Entity.Key); + Assert.IsTrue(siblings[2].HasAccess); + }); + } + + [Test] + public async Task SiblingUserAccessEntities_WithStartNodesOfTargetChild_YieldsTarget_AsNotAllowed() + { + var contentStartNodePaths = await CreateUserAndGetStartNodePaths(_contentByName["1-5-1"].Id); + + var siblings = UserStartNodeEntitiesService + .SiblingUserAccessEntities( + UmbracoObjectTypes.Document, + contentStartNodePaths, + _contentByName["1-5"].Key, + 2, + 2, + BySortOrder) + .ToArray(); + + Assert.AreEqual(1, siblings.Length); + Assert.Multiple(() => + { + Assert.AreEqual(_contentByName[$"1-5"].Key, siblings[0].Entity.Key); + Assert.IsFalse(siblings[0].HasAccess); + }); + } +} diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceTests.cs b/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceTests.cs index 5cc6bff35d..761c56ce13 100644 --- a/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/ManagementApi/Services/UserStartNodeEntitiesServiceTests.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; using Umbraco.Cms.Api.Management.Services.Entities; using Umbraco.Cms.Core; @@ -32,7 +32,7 @@ public partial class UserStartNodeEntitiesServiceTests : UmbracoIntegrationTest private IUserStartNodeEntitiesService UserStartNodeEntitiesService => GetRequiredService(); - protected readonly Ordering BySortOrder = Ordering.By("sortOrder"); + protected static readonly Ordering BySortOrder = Ordering.By("sortOrder"); protected override void ConfigureTestServices(IServiceCollection services) { diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs index d1031e3b2c..0db7203ad9 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/EntityServiceTests.cs @@ -5,6 +5,7 @@ using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Entities; +using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.ContentTypeEditing; using Umbraco.Cms.Infrastructure.Persistence; @@ -936,9 +937,9 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest { var children = CreateSiblingsTestData(); - var taget = children[1]; + var target = children[1]; - var result = EntityService.GetSiblings(taget.Key, UmbracoObjectTypes.Document, 1, 1).ToArray(); + var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 1, 1).ToArray(); Assert.AreEqual(3, result.Length); Assert.IsTrue(result[0].Key == children[0].Key); Assert.IsTrue(result[1].Key == children[1].Key); @@ -953,8 +954,8 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest var trash = children[1]; ContentService.MoveToRecycleBin(trash); - var taget = children[2]; - var result = EntityService.GetSiblings(taget.Key, UmbracoObjectTypes.Document, 1, 1).ToArray(); + var target = children[2]; + var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 1, 1).ToArray(); Assert.AreEqual(3, result.Length); Assert.IsFalse(result.Any(x => x.Key == trash.Key)); Assert.IsTrue(result[0].Key == children[0].Key); @@ -962,6 +963,44 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest Assert.IsTrue(result[2].Key == children[3].Key); } + [Test] + public void EntityService_Siblings_SkipsFilteredEntities_UsingFilterWithSet() + { + var children = CreateSiblingsTestData(); + + // Apply a filter that excludes the child at index 1. We'd expect to not get this, but + // get still get one previous sibling, i.e. the entity at index 0. + Guid[] keysToExclude = [children[1].Key]; + IQuery filter = ScopeProvider.CreateQuery().Where(x => !keysToExclude.Contains(x.Key)); + + var target = children[2]; + var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 1, 1, filter).ToArray(); + Assert.AreEqual(3, result.Length); + Assert.IsFalse(result.Any(x => x.Key == keysToExclude[0])); + Assert.IsTrue(result[0].Key == children[0].Key); + Assert.IsTrue(result[1].Key == children[2].Key); + Assert.IsTrue(result[2].Key == children[3].Key); + } + + [Test] + public void EntityService_Siblings_SkipsFilteredEntities_UsingFilterWithoutSet() + { + var children = CreateSiblingsTestData(); + + // Apply a filter that excludes the child at index 1. We'd expect to not get this, but + // get still get one previous sibling, i.e. the entity at index 0. + var keyToExclude = children[1].Key; + IQuery filter = ScopeProvider.CreateQuery().Where(x => x.Key != keyToExclude); + + var target = children[2]; + var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 1, 1, filter).ToArray(); + Assert.AreEqual(3, result.Length); + Assert.IsFalse(result.Any(x => x.Key == keyToExclude)); + Assert.IsTrue(result[0].Key == children[0].Key); + Assert.IsTrue(result[1].Key == children[2].Key); + Assert.IsTrue(result[2].Key == children[3].Key); + } + [Test] public void EntityService_Siblings_RespectsOrdering() { @@ -970,8 +1009,8 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest // Order the children by name to ensure the ordering works when differing from the default sort order, the name is a GUID. children = children.OrderBy(x => x.Name).ToList(); - var taget = children[1]; - var result = EntityService.GetSiblings(taget.Key, UmbracoObjectTypes.Document, 1, 1, Ordering.By(nameof(NodeDto.Text))).ToArray(); + var target = children[1]; + var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 1, 1, ordering: Ordering.By(nameof(NodeDto.Text))).ToArray(); Assert.AreEqual(3, result.Length); Assert.IsTrue(result[0].Key == children[0].Key); Assert.IsTrue(result[1].Key == children[1].Key); @@ -983,8 +1022,8 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest { var children = CreateSiblingsTestData(); - var taget = children[1]; - var result = EntityService.GetSiblings(taget.Key, UmbracoObjectTypes.Document, 100, 1).ToArray(); + var target = children[1]; + var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 100, 1).ToArray(); Assert.AreEqual(3, result.Length); Assert.IsTrue(result[0].Key == children[0].Key); Assert.IsTrue(result[1].Key == children[1].Key); @@ -996,8 +1035,8 @@ internal sealed class EntityServiceTests : UmbracoIntegrationTest { var children = CreateSiblingsTestData(); - var taget = children[^2]; - var result = EntityService.GetSiblings(taget.Key, UmbracoObjectTypes.Document, 1, 100).ToArray(); + var target = children[^2]; + var result = EntityService.GetSiblings(target.Key, UmbracoObjectTypes.Document, 1, 100).ToArray(); Assert.AreEqual(3, result.Length); Assert.IsTrue(result[^1].Key == children[^1].Key); Assert.IsTrue(result[^2].Key == children[^2].Key); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj b/tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj index 4bb3bcb5c6..935828e92d 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj +++ b/tests/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj @@ -286,12 +286,18 @@ UserStartNodeEntitiesServiceTests.cs + + UserStartNodeEntitiesServiceTests.cs + UserStartNodeEntitiesServiceMediaTests.cs UserStartNodeEntitiesServiceMediaTests.cs + + UserStartNodeEntitiesServiceMediaTests.cs + ContentBlueprintEditingServiceTests.cs