diff --git a/src/Umbraco.Core/Editors/UserEditorAuthorizationHelper.cs b/src/Umbraco.Core/Editors/UserEditorAuthorizationHelper.cs index 53faa69835..7cb97ec57c 100644 --- a/src/Umbraco.Core/Editors/UserEditorAuthorizationHelper.cs +++ b/src/Umbraco.Core/Editors/UserEditorAuthorizationHelper.cs @@ -79,6 +79,18 @@ namespace Umbraco.Cms.Core.Editors if (userGroupAliases != null) { var savingGroupAliases = userGroupAliases.ToArray(); + var existingGroupAliases = savingUser == null + ? new string[0] + : savingUser.Groups.Select(x => x.Alias).ToArray(); + + var addedGroupAliases = savingGroupAliases.Except(existingGroupAliases); + + // As we know the current user is not admin, it is only allowed to use groups that the user do have themselves. + var savingGroupAliasesNotAllowed = addedGroupAliases.Except(currentUser.Groups.Select(x=>x.Alias)).ToArray(); + if (savingGroupAliasesNotAllowed.Any()) + { + return Attempt.Fail("Cannot assign the group(s) '" + string.Join(", ", savingGroupAliasesNotAllowed) + "', the current user is not part of them or admin"); + } //only validate any groups that have changed. //a non-admin user can remove groups and add groups that they have access to @@ -94,9 +106,7 @@ namespace Umbraco.Cms.Core.Editors if (userGroupsChanged) { // d) A user cannot assign a group to another user that they do not belong to - var currentUserGroups = currentUser.Groups.Select(x => x.Alias).ToArray(); - foreach (var group in newGroups) { if (currentUserGroups.Contains(group) == false) diff --git a/src/Umbraco.Core/Models/Membership/UserGroup.cs b/src/Umbraco.Core/Models/Membership/UserGroup.cs index 0c5f4a7d66..fd173caa73 100644 --- a/src/Umbraco.Core/Models/Membership/UserGroup.cs +++ b/src/Umbraco.Core/Models/Membership/UserGroup.cs @@ -21,7 +21,7 @@ namespace Umbraco.Cms.Core.Models.Membership private string _icon; private string _name; private IEnumerable _permissions; - private readonly List _sectionCollection; + private List _sectionCollection; //Custom comparer for enumerable private static readonly DelegateEqualityComparer> StringEnumerableComparer = @@ -104,7 +104,10 @@ namespace Umbraco.Cms.Core.Models.Membership set => SetPropertyValueAndDetectChanges(value, ref _permissions, nameof(Permissions), StringEnumerableComparer); } - public IEnumerable AllowedSections => _sectionCollection; + public IEnumerable AllowedSections + { + get => _sectionCollection; + } public void RemoveAllowedSection(string sectionAlias) { @@ -124,5 +127,16 @@ namespace Umbraco.Cms.Core.Models.Membership } public int UserCount { get; } + + protected override void PerformDeepClone(object clone) + { + + base.PerformDeepClone(clone); + + var clonedEntity = (UserGroup)clone; + + //manually clone the start node props + clonedEntity._sectionCollection = new List(_sectionCollection); + } } } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Models/UserGroupTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Models/UserGroupTests.cs new file mode 100644 index 0000000000..eccd20c182 --- /dev/null +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Models/UserGroupTests.cs @@ -0,0 +1,65 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Reflection; +using Newtonsoft.Json; +using NUnit.Framework; +using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Common.Builders.Extensions; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Models +{ + [TestFixture] + public class UserGroupTests + { + private UserGroupBuilder _builder; + + [SetUp] + public void SetUp() => _builder = new UserGroupBuilder(); + + [Test] + public void Can_Deep_Clone() + { + IUserGroup item = Build(); + + var clone = (IUserGroup)item.DeepClone(); + + var x = clone.Equals(item); + Assert.AreNotSame(clone, item); + Assert.AreEqual(clone, item); + + Assert.AreEqual(clone.AllowedSections.Count(), item.AllowedSections.Count()); + Assert.AreNotSame(clone.AllowedSections, item.AllowedSections); + + // Verify normal properties with reflection + PropertyInfo[] allProps = clone.GetType().GetProperties(); + foreach (PropertyInfo propertyInfo in allProps) + { + Assert.AreEqual(propertyInfo.GetValue(clone, null), propertyInfo.GetValue(item, null)); + } + } + + [Test] + public void Can_Serialize_Without_Error() + { + IUserGroup item = Build(); + + var json = JsonConvert.SerializeObject(item); + Debug.Print(json); + } + + private IUserGroup Build() => + _builder + .WithId(3) + .WithAllowedSections(new List(){"A", "B"}) + .Build(); + + + } + + +} diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Editors/UserEditorAuthorizationHelperTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Editors/UserEditorAuthorizationHelperTests.cs index ebdbaf08c0..94064ddaba 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Editors/UserEditorAuthorizationHelperTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Editors/UserEditorAuthorizationHelperTests.cs @@ -109,6 +109,36 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Editors Assert.IsTrue(result.Success); } + [Test] + [TestCase(Constants.Security.AdminGroupAlias, Constants.Security.AdminGroupAlias, ExpectedResult = true)] + [TestCase(Constants.Security.AdminGroupAlias, "SomethingElse", ExpectedResult = true)] + [TestCase(Constants.Security.EditorGroupAlias, Constants.Security.AdminGroupAlias, ExpectedResult = false)] + [TestCase(Constants.Security.EditorGroupAlias, "SomethingElse", ExpectedResult = false)] + [TestCase(Constants.Security.EditorGroupAlias, Constants.Security.EditorGroupAlias, ExpectedResult = true)] + public bool Can_only_add_user_groups_you_are_part_of_yourself_unless_you_are_admin(string groupAlias, string groupToAdd) + { + var currentUser = Mock.Of(user => user.Groups == new[] + { + new ReadOnlyUserGroup(1, "CurrentUser", "icon-user", null, null, groupAlias, new string[0], new string[0]) + }); + IUser savingUser = null; // This means it is a new created user + + var contentService = new Mock(); + var mediaService = new Mock(); + var userService = new Mock(); + var entityService = new Mock(); + + var authHelper = new UserEditorAuthorizationHelper( + contentService.Object, + mediaService.Object, + entityService.Object, + AppCaches.Disabled); + + var result = authHelper.IsAuthorized(currentUser, savingUser, new int[0], new int[0], new[] { groupToAdd }); + + return result.Success; + } + [Test] public void Can_Add_Another_Content_Start_Node_On_User_With_Access() { diff --git a/src/Umbraco.Web.BackOffice/Controllers/UserGroupEditorAuthorizationHelper.cs b/src/Umbraco.Web.BackOffice/Controllers/UserGroupEditorAuthorizationHelper.cs index 0d90dba120..ea74b47e87 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/UserGroupEditorAuthorizationHelper.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/UserGroupEditorAuthorizationHelper.cs @@ -78,18 +78,15 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers /// /// Authorize that the user is not adding a section to the group that they don't have access to /// - /// - /// - /// - /// - public Attempt AuthorizeSectionChanges(IUser currentUser, - IEnumerable currentAllowedSections, + public Attempt AuthorizeSectionChanges( + IUser currentUser, + IEnumerable existingSections, IEnumerable proposedAllowedSections) { if (currentUser.IsAdmin()) return Attempt.Succeed(); - var sectionsAdded = currentAllowedSections.Except(proposedAllowedSections).ToArray(); + var sectionsAdded = proposedAllowedSections.Except(existingSections).ToArray(); var sectionAccessMissing = sectionsAdded.Except(currentUser.AllowedSections).ToArray(); return sectionAccessMissing.Length > 0 ? Attempt.Fail("Current user doesn't have access to add these sections " + string.Join(", ", sectionAccessMissing)) diff --git a/src/Umbraco.Web.BackOffice/Controllers/UserGroupsController.cs b/src/Umbraco.Web.BackOffice/Controllers/UserGroupsController.cs index ea6c3d9c60..b663eff6c7 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/UserGroupsController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/UserGroupsController.cs @@ -34,9 +34,14 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers private readonly IShortStringHelper _shortStringHelper; private readonly AppCaches _appCaches; - public UserGroupsController(IUserService userService, IContentService contentService, - IEntityService entityService, IMediaService mediaService, IBackOfficeSecurityAccessor backofficeSecurityAccessor, - UmbracoMapper umbracoMapper, ILocalizedTextService localizedTextService, + public UserGroupsController( + IUserService userService, + IContentService contentService, + IEntityService entityService, + IMediaService mediaService, + IBackOfficeSecurityAccessor backofficeSecurityAccessor, + UmbracoMapper umbracoMapper, + ILocalizedTextService localizedTextService, IShortStringHelper shortStringHelper, AppCaches appCaches) { @@ -66,7 +71,8 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers return Unauthorized(isAuthorized.Result); //if sections were added we need to check that the current user has access to that section - isAuthorized = authHelper.AuthorizeSectionChanges(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, + isAuthorized = authHelper.AuthorizeSectionChanges( + _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, userGroupSave.PersistedUserGroup.AllowedSections, userGroupSave.Sections); if (isAuthorized == false) @@ -82,7 +88,10 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers return Unauthorized(isAuthorized.Result); //need to ensure current user is in a group if not an admin to avoid a 401 - EnsureNonAdminUserIsInSavedUserGroup(userGroupSave); + EnsureNonAdminUserIsInSavedUserGroup(userGroupSave); + + //map the model to the persisted instance + _umbracoMapper.Map(userGroupSave, userGroupSave.PersistedUserGroup); //save the group _userService.Save(userGroupSave.PersistedUserGroup, userGroupSave.Users.ToArray()); diff --git a/src/Umbraco.Web.BackOffice/Filters/UserGroupValidateAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/UserGroupValidateAttribute.cs index 1bcfbfe0f9..7edf790fb3 100644 --- a/src/Umbraco.Web.BackOffice/Filters/UserGroupValidateAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/UserGroupValidateAttribute.cs @@ -6,6 +6,7 @@ using Umbraco.Cms.Core.Mapping; using Umbraco.Cms.Core.Models.ContentEditing; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Strings; using Umbraco.Cms.Web.BackOffice.ActionResults; using Umbraco.Cms.Web.Common.ActionsResults; using Umbraco.Extensions; @@ -20,15 +21,15 @@ namespace Umbraco.Cms.Web.BackOffice.Filters private class UserGroupValidateFilter : IActionFilter { - private readonly UmbracoMapper _umbracoMapper; + private readonly IShortStringHelper _shortStringHelper; private readonly IUserService _userService; public UserGroupValidateFilter( IUserService userService, - UmbracoMapper umbracoMapper) + IShortStringHelper shortStringHelper) { _userService = userService ?? throw new ArgumentNullException(nameof(userService)); - _umbracoMapper = umbracoMapper ?? throw new ArgumentNullException(nameof(umbracoMapper)); + _shortStringHelper = shortStringHelper ?? throw new ArgumentNullException(nameof(shortStringHelper)); } public void OnActionExecuting(ActionExecutingContext context) @@ -58,13 +59,9 @@ namespace Umbraco.Cms.Web.BackOffice.Filters return; } - //map the model to the persisted instance - _umbracoMapper.Map(userGroupSave, persisted); break; case ContentSaveAction.SaveNew: - //create the persisted model from mapping the saved model - persisted = _umbracoMapper.Map(userGroupSave); - ((UserGroup) persisted).ResetIdentity(); + persisted = new UserGroup(_shortStringHelper); break; default: context.Result = diff --git a/src/Umbraco.Web.BackOffice/Security/PasswordChanger.cs b/src/Umbraco.Web.BackOffice/Security/PasswordChanger.cs index 7cfbcf5e81..a0e7b788d8 100644 --- a/src/Umbraco.Web.BackOffice/Security/PasswordChanger.cs +++ b/src/Umbraco.Web.BackOffice/Security/PasswordChanger.cs @@ -64,6 +64,11 @@ namespace Umbraco.Cms.Web.BackOffice.Security return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("The current user is not authorized", new[] { "value" }) }); } + if (!currentUser.IsAdmin() && savingUser.IsAdmin()) + { + return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("The current user cannot change the password for the specified user", new[] { "resetPassword" }) }); + } + //ok, we should be able to reset it var resetToken = await userMgr.GeneratePasswordResetTokenAsync(backOfficeIdentityUser); diff --git a/src/Umbraco.Web.UI.Client/package-lock.json b/src/Umbraco.Web.UI.Client/package-lock.json index fc6376bfcf..2542dbe03f 100644 --- a/src/Umbraco.Web.UI.Client/package-lock.json +++ b/src/Umbraco.Web.UI.Client/package-lock.json @@ -2734,7 +2734,7 @@ "cli-color": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/cli-color/-/cli-color-1.4.0.tgz", - "integrity": "sha1-fRBzj0hSaCT4/n2lGFfLD1cv4B8=", + "integrity": "sha512-xu6RvQqqrWEo6MPR1eixqGPywhYBHRs653F9jfXB2Hx4jdM/3WxiNE1vppRmxtMIfl16SFYTpYlrnqH/HsK/2w==", "dev": true, "requires": { "ansi-regex": "^2.1.1", @@ -2968,7 +2968,7 @@ "color": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/color/-/color-3.0.0.tgz", - "integrity": "sha1-2SC0Mo1TSjrIKV1o971LpsQnvpo=", + "integrity": "sha512-jCpd5+s0s0t7p3pHQKpnJ0TpQKKdleP71LWcA0aqiljpiuAkOSUFN/dyH8ZwF0hRmFlrIuRhufds1QyEP9EB+w==", "dev": true, "requires": { "color-convert": "^1.9.1", @@ -3859,7 +3859,7 @@ "diagnostics": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/diagnostics/-/diagnostics-1.1.1.tgz", - "integrity": "sha1-yrasM99wydmnJ0kK5DrJladpsio=", + "integrity": "sha512-8wn1PmdunLJ9Tqbx+Fx/ZEuHfJf4NKSN2ZBj7SJC/OWRWha843+WsTjqMe1B5E3p28jqBlp+mJ2fPVxPyNgYKQ==", "dev": true, "requires": { "colorspace": "1.1.x", @@ -3954,7 +3954,7 @@ "domhandler": { "version": "2.4.2", "resolved": "https://registry.npmjs.org/domhandler/-/domhandler-2.4.2.tgz", - "integrity": "sha1-iAUJfpM9ZehVRvcm1g9euItE+AM=", + "integrity": "sha512-JiK04h0Ht5u/80fdLMCEmV4zkNh2BcoMFBmZ/91WtYZ8qVXSKjiw7fXMgFPnHcSZgOo3XdinHvmnDUeMf5R4wA==", "dev": true, "requires": { "domelementtype": "1" @@ -4229,7 +4229,7 @@ "debug": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/debug/-/debug-3.1.0.tgz", - "integrity": "sha512-OX8XqP7/1a9cqkxYw2yXss15f26NKWBpDXQd0/uK/KPqdQhxbPa994hnzjcE2VqQpDslf55723cKPUOGSmMY3g==", + "integrity": "sha1-W7WgZyYotkFJVmuhaBnmFRjGcmE=", "dev": true, "requires": { "ms": "2.0.0" @@ -4406,7 +4406,7 @@ "source-map": { "version": "0.6.1", "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.6.1.tgz", - "integrity": "sha1-dHIq8y6WFOnCh6jQu95IteLxomM=", + "integrity": "sha512-UjgapumWlbMhkBgzT7Ykc5YXUT46F0iKu8SGXq0bcwP5dz/h0Plj6enJqjz1Zbq2l5WaqYnrVbwWOWMyF3F47g==", "dev": true, "optional": true } @@ -8814,7 +8814,7 @@ "kuler": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/kuler/-/kuler-1.0.1.tgz", - "integrity": "sha1-73x4TzbJ+24W3TFQ0VJneysCKKY=", + "integrity": "sha512-J9nVUucG1p/skKul6DU3PUZrhs0LPulNaeUOox0IyXDi8S4CztTHs1gQphhuZmzXG7VOQSf6NJfKuzteQLv9gQ==", "dev": true, "requires": { "colornames": "^1.1.1" @@ -9386,7 +9386,7 @@ "memoizee": { "version": "0.4.14", "resolved": "https://registry.npmjs.org/memoizee/-/memoizee-0.4.14.tgz", - "integrity": "sha1-B6APIEaZ+alcLZ53IYJxx81hDVc=", + "integrity": "sha512-/SWFvWegAIYAO4NQMpcX+gcra0yEZu4OntmUdrBaWrJncxOqAziGFlHxc7yjKVK2uu3lpPW27P27wkR82wA8mg==", "dev": true, "requires": { "d": "1", @@ -9511,7 +9511,7 @@ "minimize": { "version": "2.2.0", "resolved": "https://registry.npmjs.org/minimize/-/minimize-2.2.0.tgz", - "integrity": "sha1-ixZ28wBR2FmNdDZGvRJpCwdNpMM=", + "integrity": "sha512-IxR2XMbw9pXCxApkdD9BTcH2U4XlXhbeySUrv71rmMS9XDA8BVXEsIuFu24LtwCfBgfbL7Fuh8/ZzkO5DaTLlQ==", "dev": true, "requires": { "argh": "^0.1.4", @@ -9536,7 +9536,7 @@ "is-extendable": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/is-extendable/-/is-extendable-1.0.1.tgz", - "integrity": "sha1-p0cPnkJnM9gb2B4RVSZOOjUHyrQ=", + "integrity": "sha512-arnXMxT1hhoKo9k1LZdmlNyJdDDfy2v0fXjFlmok4+i8ul/6WlbVge9bhM74OpNPQPMGUToDtz+KXa1PneJxOA==", "dev": true, "requires": { "is-plain-object": "^2.0.4" @@ -15048,7 +15048,7 @@ "debug": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/debug/-/debug-3.1.0.tgz", - "integrity": "sha512-OX8XqP7/1a9cqkxYw2yXss15f26NKWBpDXQd0/uK/KPqdQhxbPa994hnzjcE2VqQpDslf55723cKPUOGSmMY3g==", + "integrity": "sha1-W7WgZyYotkFJVmuhaBnmFRjGcmE=", "dev": true, "requires": { "ms": "2.0.0" @@ -15646,7 +15646,7 @@ "text-hex": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/text-hex/-/text-hex-1.0.0.tgz", - "integrity": "sha1-adycGxdEbueakr9biEu0uRJ1BvU=", + "integrity": "sha512-uuVGNWzgJ4yhRaNSiubPY7OjISw4sw4E5Uv0wbjp+OzcbmVU/rsT8ujgcXJhn9ypzsgr5vlzpPqP+MBBKcGvbg==", "dev": true }, "text-table": { @@ -15738,7 +15738,7 @@ "timers-ext": { "version": "0.1.7", "resolved": "https://registry.npmjs.org/timers-ext/-/timers-ext-0.1.7.tgz", - "integrity": "sha1-b1ethXjgej+5+R2Th9ZWR1VeJcY=", + "integrity": "sha512-b85NUNzTSdodShTIbky6ZF02e8STtVVfD+fu4aXXShEELpozH+bCpJLYMPZbsABN2wDH7fJpqIoXxJpzbf0NqQ==", "dev": true, "requires": { "es5-ext": "~0.10.46", diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbfileupload.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbfileupload.directive.js index 3581aed9e0..60882a372f 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbfileupload.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbfileupload.directive.js @@ -20,11 +20,7 @@ function umbFileUpload() { el.val(''); }); - el.on('drag dragstart dragend dragover dragenter dragleave drop', function (e) { - e.preventDefault(); - e.stopPropagation(); - }) - .on('dragover dragenter', function () { + el.on('dragover dragenter', function () { scope.$emit("isDragover", { value: true }); }) .on('dragleave dragend drop', function () { diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/dialogs/config.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/dialogs/config.controller.js index 30c89d5f2e..7330d69b2f 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/dialogs/config.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/dialogs/config.controller.js @@ -16,7 +16,25 @@ function ConfigController($scope) { $scope.model.close(); } } - + + vm.showEmptyState = false; + vm.showConfig = false; + vm.showStyles = false; + + $scope.$watchCollection('model.config', onWatch); + $scope.$watchCollection('model.styles', onWatch); + + function onWatch() { + + vm.showConfig = $scope.model.config && + ($scope.model.config.length > 0 || Object.keys($scope.model.config).length > 0); + vm.showStyles = $scope.model.styles && + ($scope.model.styles.length > 0 || Object.keys($scope.model.styles).length > 0); + + vm.showEmptyState = vm.showConfig === false && vm.showStyles === false; + + } + } angular.module("umbraco").controller("Umbraco.PropertyEditors.GridPrevalueEditor.ConfigController", ConfigController); diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/dialogs/config.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/dialogs/config.html index a7cabd1636..88585b76fa 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/dialogs/config.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/dialogs/config.html @@ -13,11 +13,11 @@ - + No further configuration available - +
@@ -29,7 +29,7 @@ - +
@@ -52,7 +52,7 @@ shortcut="esc" action="vm.close()"> -