From a0e3ca601ee975ed152685f41ac647bb3f5d0ff4 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Wed, 9 Apr 2025 10:57:50 +0200 Subject: [PATCH] Aggregate document permissions for current user in API response (#18721) * Create integration test verifying existing behaviour. * Aggregate permissions per document for the current user response. * Refactoring following Codescene warnings. --- .../Factories/UserPresentationFactory.cs | 81 +++++++- .../Builders/UserGroupBuilder.cs | 5 +- .../UserGroupPresentationFactoryTests.cs | 20 +- .../Factories/UserPresentationFactoryTests.cs | 191 ++++++++++++++++++ 4 files changed, 280 insertions(+), 17 deletions(-) create mode 100644 tests/Umbraco.Tests.Integration/ManagementApi/Factories/UserPresentationFactoryTests.cs diff --git a/src/Umbraco.Cms.Api.Management/Factories/UserPresentationFactory.cs b/src/Umbraco.Cms.Api.Management/Factories/UserPresentationFactory.cs index e1ee38d51a..d76d2cb5c0 100644 --- a/src/Umbraco.Cms.Api.Management/Factories/UserPresentationFactory.cs +++ b/src/Umbraco.Cms.Api.Management/Factories/UserPresentationFactory.cs @@ -1,13 +1,17 @@ -using Umbraco.Cms.Api.Management.Routing; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; +using Umbraco.Cms.Api.Management.Routing; using Umbraco.Cms.Api.Management.Security; using Umbraco.Cms.Api.Management.ViewModels; using Umbraco.Cms.Api.Management.ViewModels.User; using Umbraco.Cms.Api.Management.ViewModels.User.Current; -using Umbraco.Cms.Core; using Umbraco.Cms.Api.Management.ViewModels.User.Item; +using Umbraco.Cms.Api.Management.ViewModels.UserGroup; +using Umbraco.Cms.Api.Management.ViewModels.UserGroup.Permissions; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Mail; using Umbraco.Cms.Core.Media; @@ -20,7 +24,6 @@ namespace Umbraco.Cms.Api.Management.Factories; public class UserPresentationFactory : IUserPresentationFactory { - private readonly IEntityService _entityService; private readonly AppCaches _appCaches; private readonly MediaFileManager _mediaFileManager; @@ -31,7 +34,10 @@ public class UserPresentationFactory : IUserPresentationFactory private readonly IPasswordConfigurationPresentationFactory _passwordConfigurationPresentationFactory; private readonly IBackOfficeExternalLoginProviders _externalLoginProviders; private readonly SecuritySettings _securitySettings; + private readonly IUserService _userService; + private readonly IContentService _contentService; + [Obsolete("Please use the constructor taking all parameters. Scheduled for removal in Umbraco 17.")] public UserPresentationFactory( IEntityService entityService, AppCaches appCaches, @@ -43,6 +49,35 @@ public class UserPresentationFactory : IUserPresentationFactory IPasswordConfigurationPresentationFactory passwordConfigurationPresentationFactory, IOptionsSnapshot securitySettings, IBackOfficeExternalLoginProviders externalLoginProviders) + : this( + entityService, + appCaches, + mediaFileManager, + imageUrlGenerator, + userGroupPresentationFactory, + absoluteUrlBuilder, + emailSender, + passwordConfigurationPresentationFactory, + securitySettings, + externalLoginProviders, + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + public UserPresentationFactory( + IEntityService entityService, + AppCaches appCaches, + MediaFileManager mediaFileManager, + IImageUrlGenerator imageUrlGenerator, + IUserGroupPresentationFactory userGroupPresentationFactory, + IAbsoluteUrlBuilder absoluteUrlBuilder, + IEmailSender emailSender, + IPasswordConfigurationPresentationFactory passwordConfigurationPresentationFactory, + IOptionsSnapshot securitySettings, + IBackOfficeExternalLoginProviders externalLoginProviders, + IUserService userService, + IContentService contentService) { _entityService = entityService; _appCaches = appCaches; @@ -54,6 +89,8 @@ public class UserPresentationFactory : IUserPresentationFactory _externalLoginProviders = externalLoginProviders; _securitySettings = securitySettings.Value; _absoluteUrlBuilder = absoluteUrlBuilder; + _userService = userService; + _contentService = contentService; } public UserResponseModel CreateResponseModel(IUser user) @@ -195,7 +232,7 @@ public class UserPresentationFactory : IUserPresentationFactory var contentStartNodeIds = user.CalculateContentStartNodeIds(_entityService, _appCaches); var documentStartNodeKeys = GetKeysFromIds(contentStartNodeIds, UmbracoObjectTypes.Document); - var permissions = presentationGroups.SelectMany(x => x.Permissions).ToHashSet(); + var permissions = GetAggregatedGranularPermissions(user, presentationGroups); var fallbackPermissions = presentationGroups.SelectMany(x => x.FallbackPermissions).ToHashSet(); var hasAccessToAllLanguages = presentationGroups.Any(x => x.HasAccessToAllLanguages); @@ -225,6 +262,42 @@ public class UserPresentationFactory : IUserPresentationFactory }); } + private HashSet GetAggregatedGranularPermissions(IUser user, IEnumerable presentationGroups) + { + var permissions = presentationGroups.SelectMany(x => x.Permissions).ToHashSet(); + + // The raw permission data consists of several permissions for each document. We want to aggregate this server-side so + // we return one set of aggregate permissions per document that the client will use. + + // Get the unique document keys that have granular permissions. + IEnumerable documentKeysWithGranularPermissions = permissions + .Where(x => x is DocumentPermissionPresentationModel) + .Cast() + .Select(x => x.Document.Id) + .Distinct(); + + var aggregatedPermissions = new HashSet(); + foreach (Guid documentKey in documentKeysWithGranularPermissions) + { + // Retrieve the path of the document. + var path = _contentService.GetById(documentKey)?.Path; + if (string.IsNullOrEmpty(path)) + { + continue; + } + + // With the path we can call the same logic as used server-side for authorizing access to resources. + EntityPermissionSet permissionsForPath = _userService.GetPermissionsForPath(user, path); + aggregatedPermissions.Add(new DocumentPermissionPresentationModel + { + Document = new ReferenceByIdModel(documentKey), + Verbs = permissionsForPath.GetAllPermissions() + }); + } + + return aggregatedPermissions; + } + public async Task CreateCalculatedUserStartNodesResponseModelAsync(IUser user) { var mediaStartNodeIds = user.CalculateMediaStartNodeIds(_entityService, _appCaches); diff --git a/tests/Umbraco.Tests.Common/Builders/UserGroupBuilder.cs b/tests/Umbraco.Tests.Common/Builders/UserGroupBuilder.cs index 2d78198649..2ae6af8c37 100644 --- a/tests/Umbraco.Tests.Common/Builders/UserGroupBuilder.cs +++ b/tests/Umbraco.Tests.Common/Builders/UserGroupBuilder.cs @@ -159,7 +159,7 @@ public class UserGroupBuilder Key = key, StartContentId = startContentId, StartMediaId = startMediaId, - Permissions = _permissions + Permissions = _permissions, }; BuildAllowedSections(userGroup); @@ -169,7 +169,6 @@ public class UserGroupBuilder return userGroup; } - private void BuildAllowedSections(UserGroup userGroup) { foreach (var section in _allowedSections) @@ -204,6 +203,6 @@ public class UserGroupBuilder .WithAlias(alias + suffix) .WithName(name + suffix) .WithPermissions(permissions ?? new[] { "A", "B", "C" }.ToHashSet()) - .WithAllowedSections(allowedSections ?? new[] { "content", "media" }) + .WithAllowedSections(allowedSections ?? ["content", "media"]) .Build(); } diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/Factories/UserGroupPresentationFactoryTests.cs b/tests/Umbraco.Tests.Integration/ManagementApi/Factories/UserGroupPresentationFactoryTests.cs index 8a27922276..3ae7769ee6 100644 --- a/tests/Umbraco.Tests.Integration/ManagementApi/Factories/UserGroupPresentationFactoryTests.cs +++ b/tests/Umbraco.Tests.Integration/ManagementApi/Factories/UserGroupPresentationFactoryTests.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; using Umbraco.Cms.Api.Management.Factories; using Umbraco.Cms.Api.Management.Mapping.Permissions; @@ -34,15 +34,15 @@ public class UserGroupPresentationFactoryTests : UmbracoIntegrationTest services.AddTransient(); services.AddSingleton(); services.AddSingleton(); - services.AddSingleton(x=>x.GetRequiredService()); - services.AddSingleton(x=>x.GetRequiredService()); + services.AddSingleton(x => x.GetRequiredService()); + services.AddSingleton(x => x.GetRequiredService()); } [Test] public async Task Can_Map_Create_Model_And_Create() { - var updateModel = new CreateUserGroupRequestModel() + var createModel = new CreateUserGroupRequestModel() { Alias = "testAlias", FallbackPermissions = new HashSet(), @@ -53,7 +53,7 @@ public class UserGroupPresentationFactoryTests : UmbracoIntegrationTest Permissions = new HashSet() }; - var attempt = await UserGroupPresentationFactory.CreateAsync(updateModel); + var attempt = await UserGroupPresentationFactory.CreateAsync(createModel); Assert.IsTrue(attempt.Success); var userGroupCreateAttempt = await UserGroupService.CreateAsync(attempt.Result, Constants.Security.SuperUserKey); @@ -71,7 +71,7 @@ public class UserGroupPresentationFactoryTests : UmbracoIntegrationTest [Test] public async Task Cannot_Create_UserGroup_With_Unexisting_Document_Reference() { - var updateModel = new CreateUserGroupRequestModel() + var createModel = new CreateUserGroupRequestModel() { Alias = "testAlias", FallbackPermissions = new HashSet(), @@ -89,7 +89,7 @@ public class UserGroupPresentationFactoryTests : UmbracoIntegrationTest } }; - var attempt = await UserGroupPresentationFactory.CreateAsync(updateModel); + var attempt = await UserGroupPresentationFactory.CreateAsync(createModel); Assert.IsTrue(attempt.Success); var userGroupCreateAttempt = await UserGroupService.CreateAsync(attempt.Result, Constants.Security.SuperUserKey); @@ -102,11 +102,11 @@ public class UserGroupPresentationFactoryTests : UmbracoIntegrationTest } [Test] - public async Task Can_Create_Usergroup_With_Empty_Granluar_Permissions_For_Document() + public async Task Can_Create_Usergroup_With_Empty_Granular_Permissions_For_Document() { var contentKey = await CreateContent(); - var updateModel = new CreateUserGroupRequestModel() + var createModel = new CreateUserGroupRequestModel() { Alias = "testAlias", FallbackPermissions = new HashSet(), @@ -124,7 +124,7 @@ public class UserGroupPresentationFactoryTests : UmbracoIntegrationTest } }; - var attempt = await UserGroupPresentationFactory.CreateAsync(updateModel); + var attempt = await UserGroupPresentationFactory.CreateAsync(createModel); Assert.IsTrue(attempt.Success); var userGroupCreateAttempt = await UserGroupService.CreateAsync(attempt.Result, Constants.Security.SuperUserKey); diff --git a/tests/Umbraco.Tests.Integration/ManagementApi/Factories/UserPresentationFactoryTests.cs b/tests/Umbraco.Tests.Integration/ManagementApi/Factories/UserPresentationFactoryTests.cs new file mode 100644 index 0000000000..f5f0c93fba --- /dev/null +++ b/tests/Umbraco.Tests.Integration/ManagementApi/Factories/UserPresentationFactoryTests.cs @@ -0,0 +1,191 @@ +using Microsoft.Extensions.DependencyInjection; +using NUnit.Framework; +using Umbraco.Cms.Api.Management.Factories; +using Umbraco.Cms.Api.Management.Mapping.Permissions; +using Umbraco.Cms.Api.Management.Routing; +using Umbraco.Cms.Api.Management.ViewModels.UserGroup.Permissions; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Models.Membership.Permissions; +using Umbraco.Cms.Core.Routing; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Infrastructure.Persistence.Mappers; +using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Common.Builders.Extensions; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.ManagementApi.Factories; + +[TestFixture] +[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] +public class UserPresentationFactoryTests : UmbracoIntegrationTestWithContent +{ + public IUserPresentationFactory UserPresentationFactory => GetRequiredService(); + + public IUserGroupService UserGroupService => GetRequiredService(); + + public IUserService UserService => GetRequiredService(); + + public ILanguageService LanguageService => GetRequiredService(); + + public IMediaService MediaService => GetRequiredService(); + + protected override void ConfigureTestServices(IServiceCollection services) + { + services.AddTransient(); + services.AddTransient(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(x => x.GetRequiredService()); + services.AddSingleton(x => x.GetRequiredService()); + } + + [Test] + public async Task Can_Create_Current_User_Response_Model() + { + var daLanguage = new LanguageBuilder() + .WithCultureInfo("da-DK") + .Build(); + await LanguageService.CreateAsync(daLanguage, Constants.Security.SuperUserKey); + var enUsLanguage = await LanguageService.GetAsync("en-US"); + var daDkLanguage = await LanguageService.GetAsync("da-DK"); + + var rootContentKey = Guid.Parse(TextpageKey); + var subPageContentKey = Guid.Parse(SubPageKey); + var subPage2ContentKey = Guid.Parse(SubPage2Key); + + var rootMediaFolder = MediaService.CreateMedia("Pictures Folder", Constants.System.Root, "Folder"); + MediaService.Save(rootMediaFolder); + + var groupOne = await CreateUserGroup( + "Group One", + "groupOne", + [enUsLanguage.Id], + ["A", "B", "C"], + [ + new DocumentGranularPermission + { + Key = rootContentKey, + Permission = "A", + }, + new DocumentGranularPermission + { + Key = rootContentKey, + Permission = "E", + }, + new DocumentGranularPermission + { + Key = subPageContentKey, + Permission = "F", + }, + new DocumentGranularPermission + { + Key = subPage2ContentKey, + Permission = "F", + } + ], + rootMediaFolder.Id); + var groupTwo = await CreateUserGroup( + "Group Two", + "groupTwo", + [daDkLanguage.Id], + ["A", "B", "D"], + [ + new DocumentGranularPermission + { + Key = subPage2ContentKey, + Permission = "G", + }, + new DocumentGranularPermission + { + Key = subPage2ContentKey, + Permission = "H", + } + ], + rootMediaFolder.Id); + + var user = await CreateUser([groupOne.Key, groupTwo.Key]); + + var model = await UserPresentationFactory.CreateCurrentUserResponseModelAsync(user); + + Assert.AreEqual(user.Key, model.Id); + Assert.AreEqual("test@test.com", model.Email); + Assert.AreEqual("Test User", model.Name); + Assert.AreEqual("test@test.com", model.UserName); + Assert.AreEqual(2, model.UserGroupIds.Count); + Assert.IsTrue(model.UserGroupIds.Select(x => x.Id).ContainsAll([groupOne.Key, groupTwo.Key])); + Assert.IsFalse(model.HasAccessToAllLanguages); + Assert.AreEqual(2, model.Languages.Count()); + Assert.IsTrue(model.Languages.ContainsAll(["en-US", "da-DK"])); + Assert.IsTrue(model.HasDocumentRootAccess); + Assert.AreEqual(0, model.DocumentStartNodeIds.Count); + Assert.IsFalse(model.HasMediaRootAccess); + Assert.AreEqual(1, model.MediaStartNodeIds.Count); + Assert.AreEqual(rootMediaFolder.Key, model.MediaStartNodeIds.First().Id); + Assert.IsFalse(model.HasAccessToSensitiveData); + Assert.AreEqual(4, model.FallbackPermissions.Count); + Assert.IsTrue(model.FallbackPermissions.ContainsAll(["A", "B", "C", "D"])); + + // When aggregated, we expect one permission per document (we have several granular permissions assigned, for three unique documents). + Assert.AreEqual(3, model.Permissions.Count); + + // User has two user groups, one of which provides granular permissions for the root content item. + // As such we expect the aggregated permissions to be the union of the specific permissions coming from the user group with them assigned to the document, + // and the fallback permissions from the other. + var rootContentPermissions = model.Permissions.Cast().Single(x => x.Document.Id == rootContentKey); + Assert.AreEqual(4, rootContentPermissions.Verbs.Count); + Assert.IsTrue(rootContentPermissions.Verbs.ContainsAll(["A", "B", "D", "E"])); + + // The sub-page and it's parent have specific granular permissions from one user group. + // So we expect the aggregated permissions to include those from the sub-page and the other user's groups fallback permissions. + var subPageContentPermissions = model.Permissions.Cast().Single(x => x.Document.Id == subPageContentKey); + Assert.AreEqual(4, subPageContentPermissions.Verbs.Count); + Assert.IsTrue(subPageContentPermissions.Verbs.ContainsAll(["A", "B", "D", "F"])); + + // Both user groups provide granular permissions for the second sub-page content item. + // Here we expect the aggregated permissions to be the union of the granular permissions on the document from both user groups. + var subPage2ContentPermissions = model.Permissions.Cast().Single(x => x.Document.Id == subPage2ContentKey); + Assert.AreEqual(3, subPage2ContentPermissions.Verbs.Count); + Assert.IsTrue(subPage2ContentPermissions.Verbs.ContainsAll(["F", "G", "H"])); + } + + private async Task CreateUserGroup( + string name, + string alias, + int[] allowedLanguages, + string[] permissions, + DocumentGranularPermission[] granularPermissions, + int startMediaId) + { + var userGroup = new UserGroupBuilder() + .WithName(name) + .WithAlias(alias) + .WithAllowedLanguages(allowedLanguages) + .WithStartMediaId(startMediaId) + .WithPermissions(permissions.ToHashSet()) + .WithGranularPermissions(granularPermissions) + .Build(); + var createUserGroupResult = await UserGroupService.CreateAsync(userGroup, Constants.Security.SuperUserKey); + Assert.IsTrue(createUserGroupResult.Success); + return userGroup; + } + + private async Task CreateUser(Guid[] userGroupKeys) + { + var createUserAttempt = await UserService.CreateAsync(Constants.Security.SuperUserKey, new UserCreateModel + { + Email = "test@test.com", + Name = "Test User", + UserName = "test@test.com", + UserGroupKeys = userGroupKeys.ToHashSet(), + }); + Assert.IsTrue(createUserAttempt.Success); + + return await UserService.GetAsync(createUserAttempt.Result.CreatedUser.Key); + } +}