From bfd17846b795e8e25117f66c0dc9520afbce982f Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com> Date: Thu, 18 Nov 2021 11:20:49 +0100 Subject: [PATCH] V9: Fix granular permissions for user groups (#11577) * Fixes the treepicker for granular permissions to use a select callback instead of submit with the full selection * Introduced UserGroup2NodeDto table, to allow users to save empty arrays of permissions * Cleanup * Fixed null issue in audit logging * Fixed migration * Fixed GetDeleteClauses * Fixes for SqlCE, do not run multiple comments on one sql request * Align behavior between content>permissions and usergroups>granularPpermissions - It is now possible to save default values in content>permissions like in usergroups>granularPpermissions - It is now possible to differentiate between we save an empty collection and we remove the granular permissions in content>permissions * Fix comments Co-authored-by: Andy Butland Co-authored-by: Bjarke Berg --- .../Handlers/AuditNotificationsHandler.cs | 2 +- .../UserGroupPermissionsSave.cs | 10 +- .../Persistence/Constants-DatabaseSchema.cs | 1 + .../Services/UserServiceExtensions.cs | 6 +- .../Install/DatabaseSchemaCreator.cs | 3 +- .../Migrations/Upgrade/UmbracoPlan.cs | 4 + .../Upgrade/V_9_2_0/AddUserGroup2NodeTable.cs | 30 +++++ .../Persistence/Dtos/UserGroup2NodeDto.cs | 23 ++++ .../Implement/ContentTypeRepositoryBase.cs | 1 + .../Implement/DocumentRepository.cs | 1 + .../Repositories/Implement/MediaRepository.cs | 1 + .../Implement/MemberGroupRepository.cs | 1 + .../Implement/MemberRepository.cs | 1 + .../Implement/PermissionRepository.cs | 112 ++++++++++++------ .../Implement/TemplateRepository.cs | 1 + .../Implement/UserGroupRepository.cs | 1 + .../Services/Implement/UserService.cs | 2 +- .../Controllers/ContentController.cs | 25 ++-- .../content/content.rights.controller.js | 16 +-- .../src/views/users/group.controller.js | 5 +- 20 files changed, 170 insertions(+), 76 deletions(-) create mode 100644 src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_2_0/AddUserGroup2NodeTable.cs create mode 100644 src/Umbraco.Infrastructure/Persistence/Dtos/UserGroup2NodeDto.cs diff --git a/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs b/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs index ed7fdb971f..267823300d 100644 --- a/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs +++ b/src/Umbraco.Core/Handlers/AuditNotificationsHandler.cs @@ -226,7 +226,7 @@ namespace Umbraco.Cms.Core.Handlers foreach (var perm in perms) { var group = _userService.GetUserGroupById(perm.UserGroupId); - var assigned = string.Join(", ", perm.AssignedPermissions); + var assigned = string.Join(", ", perm?.AssignedPermissions ?? Array.Empty()); var entity = _entityService.Get(perm.EntityId); _auditService.Write(performingUser.Id, $"User \"{performingUser.Name}\" {FormatEmail(performingUser)}", PerformingIp, diff --git a/src/Umbraco.Core/Models/ContentEditing/UserGroupPermissionsSave.cs b/src/Umbraco.Core/Models/ContentEditing/UserGroupPermissionsSave.cs index ae5b4805a7..551cbbc0ee 100644 --- a/src/Umbraco.Core/Models/ContentEditing/UserGroupPermissionsSave.cs +++ b/src/Umbraco.Core/Models/ContentEditing/UserGroupPermissionsSave.cs @@ -10,7 +10,7 @@ namespace Umbraco.Cms.Core.Models.ContentEditing /// Used to assign user group permissions to a content node /// [DataContract(Name = "contentPermission", Namespace = "")] - public class UserGroupPermissionsSave : IValidatableObject + public class UserGroupPermissionsSave { public UserGroupPermissionsSave() { @@ -28,13 +28,5 @@ namespace Umbraco.Cms.Core.Models.ContentEditing /// [DataMember(Name = "permissions")] public IDictionary> AssignedPermissions { get; set; } - - public IEnumerable Validate(ValidationContext validationContext) - { - if (AssignedPermissions.SelectMany(x => x.Value).Any(x => x.IsNullOrWhiteSpace())) - { - yield return new ValidationResult("A permission value cannot be null or empty", new[] { "Permissions" }); - } - } } } diff --git a/src/Umbraco.Core/Persistence/Constants-DatabaseSchema.cs b/src/Umbraco.Core/Persistence/Constants-DatabaseSchema.cs index 34c6284993..680eee5ba2 100644 --- a/src/Umbraco.Core/Persistence/Constants-DatabaseSchema.cs +++ b/src/Umbraco.Core/Persistence/Constants-DatabaseSchema.cs @@ -52,6 +52,7 @@ namespace Umbraco.Cms.Core public const string User2UserGroup = TableNamePrefix + "User2UserGroup"; public const string User2NodeNotify = TableNamePrefix + "User2NodeNotify"; public const string UserGroup2App = TableNamePrefix + "UserGroup2App"; + public const string UserGroup2Node = TableNamePrefix + "UserGroup2Node"; public const string UserGroup2NodePermission = TableNamePrefix + "UserGroup2NodePermission"; public const string ExternalLogin = TableNamePrefix + "ExternalLogin"; public const string ExternalLoginToken = TableNamePrefix + "ExternalLoginToken"; diff --git a/src/Umbraco.Core/Services/UserServiceExtensions.cs b/src/Umbraco.Core/Services/UserServiceExtensions.cs index a1f6c09d5c..57d09077fc 100644 --- a/src/Umbraco.Core/Services/UserServiceExtensions.cs +++ b/src/Umbraco.Core/Services/UserServiceExtensions.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Globalization; using System.Linq; @@ -59,7 +59,7 @@ namespace Umbraco.Extensions /// public static void RemoveUserGroupPermissions(this IUserService userService, int groupId, params int[] entityIds) { - userService.ReplaceUserGroupPermissions(groupId, new char[] {}, entityIds); + userService.ReplaceUserGroupPermissions(groupId, null, entityIds); } /// @@ -69,7 +69,7 @@ namespace Umbraco.Extensions /// public static void RemoveUserGroupPermissions(this IUserService userService, int groupId) { - userService.ReplaceUserGroupPermissions(groupId, new char[] { }); + userService.ReplaceUserGroupPermissions(groupId, null); } diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseSchemaCreator.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseSchemaCreator.cs index c20c157604..d9ebb8bf75 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseSchemaCreator.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseSchemaCreator.cs @@ -77,7 +77,8 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install typeof(DocumentCultureVariationDto), typeof(ContentScheduleDto), typeof(LogViewerQueryDto), - typeof(ContentVersionCleanupPolicyDto) + typeof(ContentVersionCleanupPolicyDto), + typeof(UserGroup2NodeDto) }; private readonly IUmbracoDatabase _database; diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs index 76a67fd330..836981e73b 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs @@ -14,6 +14,7 @@ using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_8_7_0; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_8_9_0; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_0_0; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_1_0; +using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_2_0; using Umbraco.Extensions; namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade @@ -263,6 +264,9 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade // TO 9.1.0 To("{8BAF5E6C-DCB7-41AE-824F-4215AE4F1F98}"); + + // TO 9.2.0 + To("{0571C395-8F0B-44E9-8E3F-47BDD08D817B}"); } } } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_2_0/AddUserGroup2NodeTable.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_2_0/AddUserGroup2NodeTable.cs new file mode 100644 index 0000000000..41692825d3 --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_2_0/AddUserGroup2NodeTable.cs @@ -0,0 +1,30 @@ +using System.Linq; +using Umbraco.Cms.Core; +using Umbraco.Cms.Infrastructure.Persistence.Dtos; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_2_0 +{ + class AddUserGroup2NodeTable : MigrationBase + { + public AddUserGroup2NodeTable(IMigrationContext context) + : base(context) { } + + protected override void Migrate() + { + var tables = SqlSyntax.GetTablesInSchema(Context.Database); + if (!tables.InvariantContains(UserGroup2NodeDto.TableName)) + { + Create.Table().Do(); + } + + // Insert if there exists specific permissions today. Can't do it directly in db in any nice way. + var allData = Database.Fetch(); + var toInsert = allData.Select(x => new UserGroup2NodeDto() { NodeId = x.NodeId, UserGroupId = x.UserGroupId }).Distinct( + new DelegateEqualityComparer( + (x, y) => x.NodeId == y.NodeId && x.UserGroupId == y.UserGroupId, + x => x.NodeId.GetHashCode() + x.UserGroupId.GetHashCode())).ToArray(); + Database.InsertBulk(toInsert); + } + } +} diff --git a/src/Umbraco.Infrastructure/Persistence/Dtos/UserGroup2NodeDto.cs b/src/Umbraco.Infrastructure/Persistence/Dtos/UserGroup2NodeDto.cs new file mode 100644 index 0000000000..ad172c846c --- /dev/null +++ b/src/Umbraco.Infrastructure/Persistence/Dtos/UserGroup2NodeDto.cs @@ -0,0 +1,23 @@ +using NPoco; +using Umbraco.Cms.Core; +using Umbraco.Cms.Infrastructure.Persistence.DatabaseAnnotations; + +namespace Umbraco.Cms.Infrastructure.Persistence.Dtos +{ + [TableName(TableName)] + [ExplicitColumns] + internal class UserGroup2NodeDto + { + public const string TableName = Constants.DatabaseSchema.Tables.UserGroup2Node; + + [Column("userGroupId")] + [PrimaryKeyColumn(AutoIncrement = false, Name = "PK_" + TableName, OnColumns = "userGroupId, nodeId")] + [ForeignKey(typeof(UserGroupDto))] + public int UserGroupId { get; set; } + + [Column("nodeId")] + [ForeignKey(typeof(NodeDto))] + [Index(IndexTypes.NonClustered, Name = "IX_" + TableName + "_nodeId")] + public int NodeId { get; set; } + } +} diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs index d7be081fe1..38e8a99d08 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs @@ -1383,6 +1383,7 @@ WHERE {Cms.Core.Constants.DatabaseSchema.Tables.Content}.nodeId IN (@ids) AND cm var list = new List { "DELETE FROM umbracoUser2NodeNotify WHERE nodeId = @id", + "DELETE FROM umbracoUserGroup2Node WHERE nodeId = @id", "DELETE FROM umbracoUserGroup2NodePermission WHERE nodeId = @id", "DELETE FROM cmsTagRelationship WHERE nodeId = @id", "DELETE FROM cmsContentTypeAllowedContentType WHERE Id = @id", diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs index 12e094cd7c..24043ced98 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/DocumentRepository.cs @@ -233,6 +233,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.ContentSchedule + " WHERE nodeId = @id", "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.RedirectUrl + " WHERE contentKey IN (SELECT uniqueId FROM " + Cms.Core.Constants.DatabaseSchema.Tables.Node + " WHERE id = @id)", "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.User2NodeNotify + " WHERE nodeId = @id", + "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.UserGroup2Node + " WHERE nodeId = @id", "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.UserGroup2NodePermission + " WHERE nodeId = @id", "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.UserStartNode + " WHERE startNode = @id", "UPDATE " + Cms.Core.Constants.DatabaseSchema.Tables.UserGroup + " SET startContentId = NULL WHERE startContentId = @id", diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs index cc188787ba..29960a4044 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MediaRepository.cs @@ -165,6 +165,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement var list = new List { "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.User2NodeNotify + " WHERE nodeId = @id", + "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.UserGroup2Node + " WHERE nodeId = @id", "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.UserGroup2NodePermission + " WHERE nodeId = @id", "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.UserStartNode + " WHERE startNode = @id", "UPDATE " + Cms.Core.Constants.DatabaseSchema.Tables.UserGroup + " SET startContentId = NULL WHERE startContentId = @id", diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberGroupRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberGroupRepository.cs index 6563bbde97..33c1e67831 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberGroupRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberGroupRepository.cs @@ -82,6 +82,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement var list = new[] { "DELETE FROM umbracoUser2NodeNotify WHERE nodeId = @id", + "DELETE FROM umbracoUserGroup2Node WHERE nodeId = @id", "DELETE FROM umbracoUserGroup2NodePermission WHERE nodeId = @id", "DELETE FROM umbracoRelation WHERE parentId = @id", "DELETE FROM umbracoRelation WHERE childId = @id", diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs index 4031971ddc..cf382db164 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/MemberRepository.cs @@ -565,6 +565,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement var list = new List { "DELETE FROM umbracoUser2NodeNotify WHERE nodeId = @id", + "DELETE FROM umbracoUserGroup2Node WHERE nodeId = @id", "DELETE FROM umbracoUserGroup2NodePermission WHERE nodeId = @id", "DELETE FROM umbracoRelation WHERE parentId = @id", "DELETE FROM umbracoRelation WHERE childId = @id", diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PermissionRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PermissionRepository.cs index a1cfee69a9..2b38d55212 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PermissionRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/PermissionRepository.cs @@ -55,8 +55,10 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement { Sql sql = Sql() .SelectAll() - .From() - .Where(dto => group.Contains(dto.UserGroupId)); + .From() + .LeftJoin().On( + (left, right) => left.NodeId == right.NodeId && left.UserGroupId == right.UserGroupId) + .Where(dto => group.Contains(dto.UserGroupId)); List permissions = AmbientScope.Database.Fetch(sql); @@ -73,8 +75,10 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement { Sql sql = Sql() .SelectAll() - .From() - .Where(dto => + .From() + .LeftJoin().On( + (left, right) => left.NodeId == right.NodeId && left.UserGroupId == right.UserGroupId) + .Where(dto => groupIds.Contains(dto.UserGroupId) && group.Contains(dto.NodeId)); List permissions = @@ -98,9 +102,12 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement { Sql sql = Sql() .SelectAll() - .From() - .Where(dto => entityIds.Contains(dto.NodeId)) - .OrderBy(dto => dto.NodeId); + .From() + .LeftJoin() + .On((left, right) => + left.NodeId == right.NodeId && left.UserGroupId == right.UserGroupId) + .Where(dto => entityIds.Contains(dto.NodeId)) + .OrderBy(dto => dto.NodeId); List result = AmbientScope.Database.Fetch(sql); return ConvertToPermissionList(result); @@ -115,9 +122,12 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement { Sql sql = Sql() .SelectAll() - .From() - .Where(dto => dto.NodeId == entityId) - .OrderBy(dto => dto.NodeId); + .From() + .LeftJoin() + .On((left, right) => + left.NodeId == right.NodeId && left.UserGroupId == right.UserGroupId) + .Where(dto => dto.NodeId == entityId) + .OrderBy(dto => dto.NodeId); List result = AmbientScope.Database.Fetch(sql); return ConvertToPermissionList(result); @@ -127,7 +137,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement /// Assigns the same permission set for a single group to any number of entities /// /// - /// + /// The permissions to assign or null to remove the connection between group and entityIds /// /// /// This will first clear the permissions for this user and entities and recreate them @@ -141,26 +151,37 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement IUmbracoDatabase db = AmbientScope.Database; - var sql = - "DELETE FROM umbracoUserGroup2NodePermission WHERE userGroupId = @groupId AND nodeId in (@nodeIds)"; foreach (IEnumerable group in entityIds.InGroupsOf(Constants.Sql.MaxParameterCount)) { - db.Execute(sql, new { groupId, nodeIds = group }); + + db.Execute("DELETE FROM umbracoUserGroup2Node WHERE userGroupId = @groupId AND nodeId in (@nodeIds)", + new { groupId, nodeIds = group }); + + db.Execute("DELETE FROM umbracoUserGroup2NodePermission WHERE userGroupId = @groupId AND nodeId in (@nodeIds)", + new { groupId, nodeIds = group }); } - var toInsert = new List(); - foreach (var p in permissions) + + if (permissions is not null) { + var toInsert = new List(); + var toInsertPermissions = new List(); + foreach (var e in entityIds) { - toInsert.Add(new UserGroup2NodePermissionDto + toInsert.Add(new UserGroup2NodeDto() { NodeId = e, UserGroupId = groupId }); + foreach (var p in permissions) { - NodeId = e, Permission = p.ToString(CultureInfo.InvariantCulture), UserGroupId = groupId - }); + toInsertPermissions.Add(new UserGroup2NodePermissionDto + { + NodeId = e, Permission = p.ToString(CultureInfo.InvariantCulture), UserGroupId = groupId + }); + } } - } - db.BulkInsertRecords(toInsert); + db.BulkInsertRecords(toInsert); + db.BulkInsertRecords(toInsertPermissions); + } } /// @@ -173,17 +194,23 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement { IUmbracoDatabase db = AmbientScope.Database; - var sql = - "DELETE FROM umbracoUserGroup2NodePermission WHERE userGroupId = @groupId AND permission=@permission AND nodeId in (@entityIds)"; - db.Execute(sql, + db.Execute("DELETE FROM umbracoUserGroup2Node WHERE userGroupId = @groupId AND nodeId in (@entityIds)", + new { groupId, entityIds }); + db.Execute("DELETE FROM umbracoUserGroup2NodePermission WHERE userGroupId = @groupId AND permission=@permission AND nodeId in (@entityIds)", new { groupId, permission = permission.ToString(CultureInfo.InvariantCulture), entityIds }); + UserGroup2NodeDto[] actionsPermissions = entityIds.Select(id => new UserGroup2NodeDto + { + NodeId = id, UserGroupId = groupId + }).ToArray(); + UserGroup2NodePermissionDto[] actions = entityIds.Select(id => new UserGroup2NodePermissionDto { NodeId = id, Permission = permission.ToString(CultureInfo.InvariantCulture), UserGroupId = groupId }).ToArray(); db.BulkInsertRecords(actions); + db.BulkInsertRecords(actionsPermissions); } /// @@ -197,9 +224,12 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement IUmbracoDatabase db = AmbientScope.Database; var groupIdsA = groupIds.ToArray(); - const string sql = - "DELETE FROM umbracoUserGroup2NodePermission WHERE nodeId = @nodeId AND permission = @permission AND userGroupId in (@groupIds)"; - db.Execute(sql, + db.Execute("DELETE FROM umbracoUserGroup2Node WHERE nodeId = @nodeId AND userGroupId in (@groupIds)", + new { + nodeId = entity.Id, + groupIds = groupIdsA + }); + db.Execute("DELETE FROM umbracoUserGroup2NodePermission WHERE nodeId = @nodeId AND permission = @permission AND userGroupId in (@groupIds)", new { nodeId = entity.Id, @@ -207,12 +237,18 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement groupIds = groupIdsA }); - UserGroup2NodePermissionDto[] actions = groupIdsA.Select(id => new UserGroup2NodePermissionDto + UserGroup2NodePermissionDto[] actionsPermissions = groupIdsA.Select(id => new UserGroup2NodePermissionDto { NodeId = entity.Id, Permission = permission.ToString(CultureInfo.InvariantCulture), UserGroupId = id }).ToArray(); + UserGroup2NodeDto[] actions = groupIdsA.Select(id => new UserGroup2NodeDto + { + NodeId = entity.Id, UserGroupId = id + }).ToArray(); + db.BulkInsertRecords(actions); + db.BulkInsertRecords(actionsPermissions); } /// @@ -227,15 +263,21 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement { IUmbracoDatabase db = AmbientScope.Database; - const string sql = "DELETE FROM umbracoUserGroup2NodePermission WHERE nodeId = @nodeId"; - db.Execute(sql, new { nodeId = permissionSet.EntityId }); + db.Execute("DELETE FROM umbracoUserGroup2Node WHERE nodeId = @nodeId", new { nodeId = permissionSet.EntityId }); + db.Execute("DELETE FROM umbracoUserGroup2NodePermission WHERE nodeId = @nodeId", new { nodeId = permissionSet.EntityId }); - var toInsert = new List(); + var toInsert = new List(); + var toInsertPermissions = new List(); foreach (EntityPermission entityPermission in permissionSet.PermissionsSet) { + toInsert.Add(new UserGroup2NodeDto + { + NodeId = permissionSet.EntityId, + UserGroupId = entityPermission.UserGroupId + }); foreach (var permission in entityPermission.AssignedPermissions) { - toInsert.Add(new UserGroup2NodePermissionDto + toInsertPermissions.Add(new UserGroup2NodePermissionDto { NodeId = permissionSet.EntityId, Permission = permission, @@ -245,6 +287,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement } db.BulkInsertRecords(toInsert); + db.BulkInsertRecords(toInsertPermissions); } /// @@ -282,7 +325,10 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement foreach (IGrouping permission in userGroupPermissions) { var perms = permission.Select(x => x.Permission).Distinct().ToArray(); - permissions.Add(new EntityPermission(permission.Key, np.Key, perms)); + + // perms can contain null if there are no permissions assigned, but the node is chosen in the UI. + permissions.Add(new EntityPermission(permission.Key, np.Key, + perms.Where(x => x is not null).ToArray())); } } diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TemplateRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TemplateRepository.cs index 52ecd1f779..6d2c02484a 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TemplateRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/TemplateRepository.cs @@ -131,6 +131,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement var list = new List { "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.User2NodeNotify + " WHERE nodeId = @id", + "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.UserGroup2Node + " WHERE nodeId = @id", "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.UserGroup2NodePermission + " WHERE nodeId = @id", "UPDATE " + Cms.Core.Constants.DatabaseSchema.Tables.DocumentVersion + " SET templateId = NULL WHERE templateId = @id", "DELETE FROM " + Cms.Core.Constants.DatabaseSchema.Tables.DocumentType + " WHERE templateNodeId = @id", diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs index a8c6334416..d6be3cf730 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserGroupRepository.cs @@ -286,6 +286,7 @@ namespace Umbraco.Cms.Infrastructure.Persistence.Repositories.Implement { "DELETE FROM umbracoUser2UserGroup WHERE userGroupId = @id", "DELETE FROM umbracoUserGroup2App WHERE userGroupId = @id", + "DELETE FROM umbracoUserGroup2Node WHERE userGroupId = @id", "DELETE FROM umbracoUserGroup2NodePermission WHERE userGroupId = @id", "DELETE FROM umbracoUserGroup WHERE id = @id" }; diff --git a/src/Umbraco.Infrastructure/Services/Implement/UserService.cs b/src/Umbraco.Infrastructure/Services/Implement/UserService.cs index b9fceeffec..c64a91a598 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/UserService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/UserService.cs @@ -713,7 +713,7 @@ namespace Umbraco.Cms.Core.Services.Implement _userGroupRepository.ReplaceGroupPermissions(groupId, permissions, entityIds); scope.Complete(); - var assigned = permissions.Select(p => p.ToString(CultureInfo.InvariantCulture)).ToArray(); + var assigned = permissions?.Select(p => p.ToString(CultureInfo.InvariantCulture)).ToArray(); var entityPermissions = entityIds.Select(x => new EntityPermission(groupId, x, assigned)).ToArray(); scope.Notifications.Publish(new AssignedUserGroupPermissionsNotification(entityPermissions, evtMsgs)); } diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs index 70678545d9..7c01fd19c1 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs @@ -164,27 +164,16 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers IEnumerable groupPermissions; if (saveModel.AssignedPermissions.TryGetValue(userGroup.Id, out groupPermissions)) { - //create a string collection of the assigned letters - var groupPermissionCodes = groupPermissions.ToArray(); - - //check if there are no permissions assigned for this group save model, if that is the case we want to reset the permissions - //for this group/node which will go back to the defaults - if (groupPermissionCodes.Length == 0) + if (groupPermissions is null) { _userService.RemoveUserGroupPermissions(userGroup.Id, content.Id); + continue; } - //check if they are the defaults, if so we should just remove them if they exist since it's more overhead having them stored - else if (userGroup.Permissions.UnsortedSequenceEqual(groupPermissionCodes)) - { - //only remove them if they are actually currently assigned - if (contentPermissions.ContainsKey(userGroup.Id)) - { - //remove these permissions from this node for this group since the ones being assigned are the same as the defaults - _userService.RemoveUserGroupPermissions(userGroup.Id, content.Id); - } - } - //if they are different we need to update, otherwise there's nothing to update - else if (contentPermissions.ContainsKey(userGroup.Id) == false || contentPermissions[userGroup.Id].AssignedPermissions.UnsortedSequenceEqual(groupPermissionCodes) == false) + + // Create a string collection of the assigned letters + var groupPermissionCodes = groupPermissions.ToArray(); + // Check if they are the defaults, if so we should just remove them if they exist since it's more overhead having them stored + if (contentPermissions.ContainsKey(userGroup.Id) == false || contentPermissions[userGroup.Id].AssignedPermissions.UnsortedSequenceEqual(groupPermissionCodes) == false) { _userService.ReplaceUserGroupPermissions(userGroup.Id, groupPermissionCodes.Select(x => x[0]), content.Id); diff --git a/src/Umbraco.Web.UI.Client/src/views/content/content.rights.controller.js b/src/Umbraco.Web.UI.Client/src/views/content/content.rights.controller.js index 2fb032d301..f29c25d525 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/content.rights.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/content/content.rights.controller.js @@ -11,7 +11,7 @@ vm.removedUserGroups = []; vm.viewState = "manageGroups"; vm.labels = {}; - + vm.setViewSate = setViewSate; vm.editPermissions = editPermissions; vm.setPermissions = setPermissions; @@ -24,7 +24,7 @@ function onInit() { vm.loading = true; contentResource.getDetailedPermissions($scope.currentNode.id).then(function (userGroups) { - initData(userGroups); + initData(userGroups); vm.loading = false; currentForm = angularHelper.getCurrentForm($scope); }); @@ -91,7 +91,7 @@ } function setPermissions(group) { - assignGroupPermissions(group); + assignGroupPermissions(group); setViewSate("manageGroups"); $scope.dialog.confirmDiscardChanges = true; } @@ -116,13 +116,16 @@ setViewSate("manageGroups"); } - function formatSaveModel(permissionsSave, groupCollection) { - groupCollection.forEach(function (g) { + function formatSaveModel(permissionsSave, selectedUserGroups, removedUserGroups) { + selectedUserGroups.forEach(function (g) { permissionsSave[g.id] = []; g.allowedPermissions.forEach(function (p) { permissionsSave[g.id].push(p.permissionCode); }); }); + removedUserGroups.forEach(function (g) { + permissionsSave[g.id] = null; + }); } function save() { @@ -134,8 +137,7 @@ //this is a dictionary that we need to populate var permissionsSave = {}; //format the selectedUserGroups, then the removedUserGroups since we want to pass data from both collections up - formatSaveModel(permissionsSave, vm.selectedUserGroups); - formatSaveModel(permissionsSave, vm.removedUserGroups); + formatSaveModel(permissionsSave, vm.selectedUserGroups, vm.removedUserGroups); var saveModel = { contentId: $scope.currentNode.id, diff --git a/src/Umbraco.Web.UI.Client/src/views/users/group.controller.js b/src/Umbraco.Web.UI.Client/src/views/users/group.controller.js index fd5cfc4e4f..e79ffe8bd1 100644 --- a/src/Umbraco.Web.UI.Client/src/views/users/group.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/users/group.controller.js @@ -219,9 +219,8 @@ section: "content", treeAlias: "content", hideSubmitButton: true, - submit: function (model) { - if (model.selection) { - var node = model.selection[0]; + select: function (node) { + if (node) { //check if this is already in our selection var found = _.find(vm.userGroup.assignedPermissions, function (i) { return i.id === node.id;