diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index 659a98ea07..c392011eca 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -1338,7 +1338,6 @@ internal partial class UserService : RepositoryService, IUserService includedUserGroupAliases = userGroupKeyConversionAttempt.Result.ToArray(); } - if (mergedFilter.NameFilters is not null) { foreach (var nameFilter in mergedFilter.NameFilters) @@ -1357,17 +1356,19 @@ internal partial class UserService : RepositoryService, IUserService } else { - includeUserStates = new HashSet(filter.IncludeUserStates!); - includeUserStates.IntersectWith(baseFilter.IncludeUserStates); + includeUserStates = new HashSet(baseFilter.IncludeUserStates); + if (filter.IncludeUserStates is not null && filter.IncludeUserStates.Contains(UserState.All) is false) + { + includeUserStates.IntersectWith(filter.IncludeUserStates); + } // This means that we've only chosen to include a user state that is not allowed, so we'll return an empty result - if(includeUserStates.Count == 0) + if (includeUserStates.Count == 0) { return Attempt.SucceedWithStatus(UserOperationStatus.Success, new PagedModel()); } } - PaginationHelper.ConvertSkipTakeToPaging(skip, take, out long pageNumber, out int pageSize); Expression> orderByExpression = GetOrderByExpression(orderBy); diff --git a/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml b/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml index 918b3ea4fe..7cf519f60f 100644 --- a/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml +++ b/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml @@ -78,6 +78,13 @@ lib/net9.0/Umbraco.Tests.Integration.dll true + + CP0002 + M:Umbraco.Cms.Tests.Integration.Umbraco.Core.Services.UserServiceCrudTests.Cannot_Request_Disabled_If_Hidden(Umbraco.Cms.Core.Models.Membership.UserState) + lib/net9.0/Umbraco.Tests.Integration.dll + lib/net9.0/Umbraco.Tests.Integration.dll + true + CP0002 M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.ContentPublishingServiceTests.Publish_Branch_Does_Not_Publish_Unpublished_Children_Unless_Explicitly_Instructed_To(System.Boolean) diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Filter.cs b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Filter.cs index 71b557d4d2..58d79bb2a2 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Filter.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Core/Services/UserServiceCrudTests.Filter.cs @@ -1,4 +1,4 @@ -using NUnit.Framework; +using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Models; @@ -10,12 +10,14 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.Services; public partial class UserServiceCrudTests { - [Test] - [TestCase(UserState.Disabled)] - [TestCase(UserState.All)] - public async Task Cannot_Request_Disabled_If_Hidden(UserState includeState) + [TestCase(null, 1)] // Requesting no filter, will just get the admin user but not the created and disabled one. + // - verifies fix for https://github.com/umbraco/Umbraco-CMS/issues/18812 + [TestCase(UserState.Inactive, 1)] // Requesting inactive, will just get the admin user but not the created and disabled one. + [TestCase(UserState.Disabled, 0)] // Requesting disabled, won't get any as admin user isn't disabled and, whilst the created one is, disabled users are hidden. + [TestCase(UserState.All, 1)] // Requesting all, will just get the admin user but not the created and disabled one. + public async Task Cannot_Request_Disabled_If_Hidden(UserState? includeState, int expectedCount) { - var userService = CreateUserService(new SecuritySettings {HideDisabledUsersInBackOffice = true}); + var userService = CreateUserService(new SecuritySettings { HideDisabledUsersInBackOffice = true }); var editorGroup = await UserGroupService.GetAsync(Constants.Security.EditorGroupKey); var createModel = new UserCreateModel @@ -23,21 +25,25 @@ public partial class UserServiceCrudTests UserName = "editor@mail.com", Email = "editor@mail.com", Name = "Editor", - UserGroupKeys = new HashSet { editorGroup.Key } + UserGroupKeys = new HashSet { editorGroup.Key }, }; var createAttempt = await userService.CreateAsync(Constants.Security.SuperUserKey, createModel, true); Assert.IsTrue(createAttempt.Success); var disableStatus = - await userService.DisableAsync(Constants.Security.SuperUserKey, new HashSet{ createAttempt.Result.CreatedUser!.Key }); + await userService.DisableAsync(Constants.Security.SuperUserKey, new HashSet { createAttempt.Result.CreatedUser!.Key }); Assert.AreEqual(UserOperationStatus.Success, disableStatus); - var filter = new UserFilter {IncludeUserStates = new HashSet {includeState}}; + var filter = new UserFilter(); + if (includeState.HasValue) + { + filter.IncludeUserStates = new HashSet { includeState.Value }; + } var filterAttempt = await userService.FilterAsync(Constants.Security.SuperUserKey, filter, 0, 1000); Assert.IsTrue(filterAttempt.Success); - Assert.AreEqual(0, filterAttempt.Result.Items.Count()); + Assert.AreEqual(expectedCount, filterAttempt.Result.Items.Count()); } [Test]