From 7571dab4cc7034f087656f2c3d494f9b7d455fe5 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 29 Aug 2017 15:13:07 +1000 Subject: [PATCH] Updates the user auth helper and adds unit tests for it's scenarios --- src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + .../UserEditorAuthorizationHelperTests.cs | 225 ++++++++++++++++++ .../Editors/UserEditorAuthorizationHelper.cs | 28 +-- 3 files changed, 235 insertions(+), 19 deletions(-) create mode 100644 src/Umbraco.Tests/Web/Controllers/UserEditorAuthorizationHelperTests.cs diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 186f44f8e4..4874f8a74a 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -196,6 +196,7 @@ + diff --git a/src/Umbraco.Tests/Web/Controllers/UserEditorAuthorizationHelperTests.cs b/src/Umbraco.Tests/Web/Controllers/UserEditorAuthorizationHelperTests.cs new file mode 100644 index 0000000000..a97f36fd4d --- /dev/null +++ b/src/Umbraco.Tests/Web/Controllers/UserEditorAuthorizationHelperTests.cs @@ -0,0 +1,225 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Moq; +using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Models; +using Umbraco.Core.Models.EntityBase; +using Umbraco.Core.Models.Membership; +using Umbraco.Core.Services; +using Umbraco.Web.Editors; + +namespace Umbraco.Tests.Web.Controllers +{ + [TestFixture] + public class UserEditorAuthorizationHelperTests + { + [Test] + public void Admin_Is_Authorized() + { + var currentUser = GetAdminUser(); + var savingUser = Mock.Of(); + + var contentService = new Mock(); + var mediaService = new Mock(); + var userService = new Mock(); + var entityService = new Mock(); + + var authHelper = new UserEditorAuthorizationHelper( + contentService.Object, + mediaService.Object, + userService.Object, + entityService.Object); + + var result = authHelper.IsAuthorized(currentUser, savingUser, new int[0], new int[0], new string[0]); + + Assert.IsTrue(result.Success); + } + + [Test] + public void Non_Admin_Cannot_Save_Admin() + { + var currentUser = Mock.Of(); + var savingUser = GetAdminUser(); + + var contentService = new Mock(); + var mediaService = new Mock(); + var userService = new Mock(); + var entityService = new Mock(); + + var authHelper = new UserEditorAuthorizationHelper( + contentService.Object, + mediaService.Object, + userService.Object, + entityService.Object); + + var result = authHelper.IsAuthorized(currentUser, savingUser, new int[0], new int[0], new string[0]); + + Assert.IsFalse(result.Success); + } + + [Test] + public void Cannot_Grant_Group_Membership_Without_Being_A_Member() + { + var currentUser = Mock.Of(user => user.Groups == new[] + { + new ReadOnlyUserGroup(1, "Test", "icon-user", null, null, "test", new string[0], new string[0]) + }); + var savingUser = Mock.Of(); + + var contentService = new Mock(); + var mediaService = new Mock(); + var userService = new Mock(); + var entityService = new Mock(); + + var authHelper = new UserEditorAuthorizationHelper( + contentService.Object, + mediaService.Object, + userService.Object, + entityService.Object); + + var result = authHelper.IsAuthorized(currentUser, savingUser, new int[0], new int[0], new[] {"FunGroup"}); + + Assert.IsFalse(result.Success); + } + + [Test] + public void Can_Grant_Group_Membership_With_Being_A_Member() + { + var currentUser = Mock.Of(user => user.Groups == new[] + { + new ReadOnlyUserGroup(1, "Test", "icon-user", null, null, "test", new string[0], new string[0]) + }); + var savingUser = Mock.Of(); + + var contentService = new Mock(); + var mediaService = new Mock(); + var userService = new Mock(); + var entityService = new Mock(); + + var authHelper = new UserEditorAuthorizationHelper( + contentService.Object, + mediaService.Object, + userService.Object, + entityService.Object); + + var result = authHelper.IsAuthorized(currentUser, savingUser, new int[0], new int[0], new[] { "test" }); + + Assert.IsTrue(result.Success); + } + + [Test] + public void Cannot_Grant_Content_Start_Node_On_User_Without_Access() + { + var currentUser = Mock.Of(user => user.StartContentIds == new[]{9876}); + var savingUser = Mock.Of(); + + var contentService = new Mock(); + contentService.Setup(x => x.GetById(It.IsAny())).Returns(Mock.Of(content => content.Path == "-1,1234")); + var mediaService = new Mock(); + var userService = new Mock(); + var entityService = new Mock(); + entityService.Setup(service => service.GetAllPaths(It.IsAny(), It.IsAny())) + .Returns(new[] { new EntityPath() { Path = "-1,9876", Id = 9876 } }); + + var authHelper = new UserEditorAuthorizationHelper( + contentService.Object, + mediaService.Object, + userService.Object, + entityService.Object); + + var result = authHelper.IsAuthorized(currentUser, savingUser, new []{1234}, new int[0], new string[0]); + + Assert.IsFalse(result.Success); + } + + [Test] + public void Can_Grant_Content_Start_Node_On_User_With_Access() + { + var currentUser = Mock.Of(user => user.StartContentIds == new[] { 9876 }); + var savingUser = Mock.Of(); + + var contentService = new Mock(); + contentService.Setup(x => x.GetById(It.IsAny())) + .Returns(Mock.Of(content => content.Path == "-1,9876,5555")); + var mediaService = new Mock(); + var userService = new Mock(); + var entityService = new Mock(); + entityService.Setup(service => service.GetAllPaths(It.IsAny(), It.IsAny())) + .Returns(new[] { new EntityPath() { Path = "-1,9876", Id = 9876 } }); + + var authHelper = new UserEditorAuthorizationHelper( + contentService.Object, + mediaService.Object, + userService.Object, + entityService.Object); + + var result = authHelper.IsAuthorized(currentUser, savingUser, new[] { 5555 }, new int[0], new string[0]); + + Assert.IsTrue(result.Success); + } + + [Test] + public void Cannot_Grant_Media_Start_Node_On_User_Without_Access() + { + var currentUser = Mock.Of(user => user.StartMediaIds == new[] { 9876 }); + var savingUser = Mock.Of(); + + var contentService = new Mock(); + var mediaService = new Mock(); + mediaService.Setup(x => x.GetById(It.IsAny())).Returns(Mock.Of(content => content.Path == "-1,1234")); + var userService = new Mock(); + var entityService = new Mock(); + entityService.Setup(service => service.GetAllPaths(It.IsAny(), It.IsAny())) + .Returns(new[] { new EntityPath() { Path = "-1,9876", Id = 9876 } }); + + var authHelper = new UserEditorAuthorizationHelper( + contentService.Object, + mediaService.Object, + userService.Object, + entityService.Object); + + var result = authHelper.IsAuthorized(currentUser, savingUser, new int[0], new[] {1234}, new string[0]); + + Assert.IsFalse(result.Success); + } + + [Test] + public void Can_Grant_Media_Start_Node_On_User_With_Access() + { + var currentUser = Mock.Of(user => user.StartMediaIds == new[] { 9876 }); + var savingUser = Mock.Of(); + + var contentService = new Mock(); + var mediaService = new Mock(); + mediaService.Setup(x => x.GetById(It.IsAny())) + .Returns(Mock.Of(content => content.Path == "-1,9876,5555")); + var userService = new Mock(); + var entityService = new Mock(); + entityService.Setup(service => service.GetAllPaths(It.IsAny(), It.IsAny())) + .Returns(new[] { new EntityPath() { Path = "-1,9876", Id = 9876 } }); + + var authHelper = new UserEditorAuthorizationHelper( + contentService.Object, + mediaService.Object, + userService.Object, + entityService.Object); + + var result = authHelper.IsAuthorized(currentUser, savingUser, new int[0], new[] { 5555 }, new string[0]); + + Assert.IsTrue(result.Success); + } + + private IUser GetAdminUser() + { + var admin = Mock.Of(user => user.Groups == new[] + { + new ReadOnlyUserGroup(1, "Admin", "icon-user", null, null, Constants.Security.AdminGroupAlias, new string[0], new string[0]) + }); + return admin; + } + } +} diff --git a/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs b/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs index 6fcbd80b39..b5e24411e4 100644 --- a/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs +++ b/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs @@ -74,27 +74,17 @@ namespace Umbraco.Web.Editors if (userGroupsChanged) { - var userGroups = _userService.GetUserGroupsByAlias(newGroups).ToArray(); + // d) A user cannot assign a group to another user that they do not belong to - //TODO: pretty sure d + e can be done by just checking if the newGroups being added are groups that the current user is a member of - - // d) A user cannot assign a group to another user that grants them access to a start node they don't have access to - foreach (var group in userGroups) + var currentUserGroups = currentUser.Groups.Select(x => x.Alias).ToArray(); + + foreach (var group in newGroups) { - pathResult = AuthorizePath(currentUser, - group.StartContentId.HasValue ? new[] { group.StartContentId.Value } : null, - group.StartMediaId.HasValue ? new[] { group.StartMediaId.Value } : null); - if (pathResult == false) - return pathResult; - } - - // e) A user cannot set a section on another user that they don't have access to - var allGroupSections = userGroups.SelectMany(x => x.AllowedSections).Distinct(); - var missingSectionAccess = allGroupSections.Except(currentUser.AllowedSections).ToArray(); - if (missingSectionAccess.Length > 0) - { - return Attempt.Fail("The current user does not have access to sections " + string.Join(",", missingSectionAccess)); - } + if (currentUserGroups.Contains(group) == false) + { + return Attempt.Fail("Cannot assign the group " + group + ", the current user is not a member"); + } + } } }