From 53bce239c36072b541d9ae529a3d12c02ba3e76e Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 11 Jul 2017 19:19:38 +1000 Subject: [PATCH 01/11] Gets implicit/inherited permissions 'working', well at least with one test, now need to test the rest. --- .../Models/Membership/EntityPermission.cs | 16 + .../Models/Membership/IReadOnlyUserGroup.cs | 8 + .../Models/Membership/ReadOnlyUserGroup.cs | 12 +- .../Models/Membership/UserGroup.cs | 2 +- .../Models/Membership/UserGroupExtensions.cs | 9 +- .../Repositories/ContentRepository.cs | 34 +- .../Repositories/PermissionRepository.cs | 119 ++++--- src/Umbraco.Core/Services/IUserService.cs | 24 +- src/Umbraco.Core/Services/UserService.cs | 290 ++++++++++++------ .../Repositories/ContentRepositoryTest.cs | 20 +- .../Services/ContentServiceTests.cs | 172 +++++++++++ .../Services/UserServiceTests.cs | 117 ++++--- .../umbraco/dialogs/cruds.aspx.cs | 2 +- src/umbraco.businesslogic/User.cs | 3 +- 14 files changed, 585 insertions(+), 243 deletions(-) diff --git a/src/Umbraco.Core/Models/Membership/EntityPermission.cs b/src/Umbraco.Core/Models/Membership/EntityPermission.cs index 15bdb4cbe9..a1fa57d274 100644 --- a/src/Umbraco.Core/Models/Membership/EntityPermission.cs +++ b/src/Umbraco.Core/Models/Membership/EntityPermission.cs @@ -11,6 +11,14 @@ namespace Umbraco.Core.Models.Membership { EntityId = entityId; AssignedPermissions = assignedPermissions; + IsDefaultPermissions = false; + } + + public EntityPermission(int entityId, string[] assignedPermissions, bool isDefaultPermissions) + { + EntityId = entityId; + AssignedPermissions = assignedPermissions; + IsDefaultPermissions = isDefaultPermissions; } public int EntityId { get; private set; } @@ -19,6 +27,14 @@ namespace Umbraco.Core.Models.Membership /// The assigned permissions for the user/entity combo /// public string[] AssignedPermissions { get; private set; } + + /// + /// True if the permissions assigned to this object are the group's default permissions and not explicitly defined permissions + /// + /// + /// This will be the case when looking up entity permissions and falling back to the default permissions + /// + public bool IsDefaultPermissions { get; private set; } /// /// Adds additional permissions to an existing instance of diff --git a/src/Umbraco.Core/Models/Membership/IReadOnlyUserGroup.cs b/src/Umbraco.Core/Models/Membership/IReadOnlyUserGroup.cs index beffc5862d..deebc03401 100644 --- a/src/Umbraco.Core/Models/Membership/IReadOnlyUserGroup.cs +++ b/src/Umbraco.Core/Models/Membership/IReadOnlyUserGroup.cs @@ -18,6 +18,14 @@ namespace Umbraco.Core.Models.Membership /// string Alias { get; } + /// + /// The set of default permissions + /// + /// + /// By default each permission is simply a single char but we've made this an enumerable{string} to support a more flexible permissions structure in the future. + /// + IEnumerable Permissions { get; set; } + IEnumerable AllowedSections { get; } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/Membership/ReadOnlyUserGroup.cs b/src/Umbraco.Core/Models/Membership/ReadOnlyUserGroup.cs index b2a7349e81..c1b8d3997f 100644 --- a/src/Umbraco.Core/Models/Membership/ReadOnlyUserGroup.cs +++ b/src/Umbraco.Core/Models/Membership/ReadOnlyUserGroup.cs @@ -5,13 +5,15 @@ namespace Umbraco.Core.Models.Membership { public class ReadOnlyUserGroup : IReadOnlyUserGroup, IEquatable { - public ReadOnlyUserGroup(int id, string name, string icon, int? startContentId, int? startMediaId, string @alias, IEnumerable allowedSections) + public ReadOnlyUserGroup(int id, string name, string icon, int? startContentId, int? startMediaId, string @alias, + IEnumerable allowedSections, IEnumerable permissions) { Name = name; Icon = icon; Id = id; Alias = alias; AllowedSections = allowedSections; + Permissions = permissions; //Zero is invalid and will be treated as Null StartContentId = startContentId == 0 ? null : startContentId; @@ -24,6 +26,14 @@ namespace Umbraco.Core.Models.Membership public int? StartContentId { get; private set; } public int? StartMediaId { get; private set; } public string Alias { get; private set; } + + /// + /// The set of default permissions + /// + /// + /// By default each permission is simply a single char but we've made this an enumerable{string} to support a more flexible permissions structure in the future. + /// + public IEnumerable Permissions { get; set; } public IEnumerable AllowedSections { get; private set; } public bool Equals(ReadOnlyUserGroup other) diff --git a/src/Umbraco.Core/Models/Membership/UserGroup.cs b/src/Umbraco.Core/Models/Membership/UserGroup.cs index 29afc86fc8..012e9b6cc2 100644 --- a/src/Umbraco.Core/Models/Membership/UserGroup.cs +++ b/src/Umbraco.Core/Models/Membership/UserGroup.cs @@ -12,7 +12,7 @@ namespace Umbraco.Core.Models.Membership /// [Serializable] [DataContract(IsReference = true)] - internal class UserGroup : Entity, IUserGroup + internal class UserGroup : Entity, IUserGroup, IReadOnlyUserGroup { private int? _startContentId; private int? _startMediaId; diff --git a/src/Umbraco.Core/Models/Membership/UserGroupExtensions.cs b/src/Umbraco.Core/Models/Membership/UserGroupExtensions.cs index d7625f0fd8..34f4b805f9 100644 --- a/src/Umbraco.Core/Models/Membership/UserGroupExtensions.cs +++ b/src/Umbraco.Core/Models/Membership/UserGroupExtensions.cs @@ -7,12 +7,17 @@ namespace Umbraco.Core.Models.Membership { public static IReadOnlyUserGroup ToReadOnlyGroup(this IUserGroup group) { - return new ReadOnlyUserGroup(group.Id, group.Name, group.Icon, group.StartContentId, group.StartMediaId, group.Alias, group.AllowedSections); + //this will generally always be the case + var readonlyGroup = group as IReadOnlyUserGroup; + if (readonlyGroup != null) return readonlyGroup; + + //otherwise create one + return new ReadOnlyUserGroup(group.Id, group.Name, group.Icon, group.StartContentId, group.StartMediaId, group.Alias, group.AllowedSections, group.Permissions); } public static IReadOnlyUserGroup ToReadOnlyGroup(this UserGroupDto group) { - return new ReadOnlyUserGroup(group.Id, group.Name, group.Icon, group.StartContentId, group.StartMediaId, group.Alias, group.UserGroup2AppDtos.Select(x => x.AppAlias).ToArray()); + return new ReadOnlyUserGroup(group.Id, group.Name, group.Icon, group.StartContentId, group.StartMediaId, group.Alias, group.UserGroup2AppDtos.Select(x => x.AppAlias).ToArray(), group.DefaultPermissions.ToCharArray().Select(x => x.ToString())); } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 499f958449..f07e9c8385 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -470,24 +470,24 @@ namespace Umbraco.Core.Persistence.Repositories entity.SortOrder = sortOrder; entity.Level = level; - //Assign the same permissions to it as the parent node - // http://issues.umbraco.org/issue/U4-2161 - var permissionsRepo = new PermissionRepository(UnitOfWork, _cacheHelper, SqlSyntax); - var parentPermissions = permissionsRepo.GetPermissionsForEntity(entity.ParentId).ToArray(); - //if there are parent permissions then assign them, otherwise leave null and permissions will become the - // user's default permissions. - if (parentPermissions.Any()) - { - var userGroupPermissions = ( - from perm in parentPermissions - from p in perm.AssignedPermissions - select new EntityPermissionSet.UserGroupPermission(perm.UserGroupId, p)).ToList(); + ////Assign the same permissions to it as the parent node + //// http://issues.umbraco.org/issue/U4-2161 + //var permissionsRepo = new PermissionRepository(UnitOfWork, _cacheHelper, SqlSyntax); + //var parentPermissions = permissionsRepo.GetPermissionsForEntity(entity.ParentId).ToArray(); + ////if there are parent permissions then assign them, otherwise leave null and permissions will become the + //// user's default permissions. + //if (parentPermissions.Length > 0) + //{ + // var userGroupPermissions = ( + // from perm in parentPermissions + // from p in perm.AssignedPermissions + // select new EntityPermissionSet.UserGroupPermission(perm.UserGroupId, p)).ToList(); - permissionsRepo.ReplaceEntityPermissions(new EntityPermissionSet(entity.Id, userGroupPermissions)); - //flag the entity's permissions changed flag so we can track those changes. - //Currently only used for the cache refreshers to detect if we should refresh all user permissions cache. - ((Content)entity).PermissionsChanged = true; - } + // permissionsRepo.ReplaceEntityPermissions(new EntityPermissionSet(entity.Id, userGroupPermissions)); + // //flag the entity's permissions changed flag so we can track those changes. + // //Currently only used for the cache refreshers to detect if we should refresh all user permissions cache. + // ((Content)entity).PermissionsChanged = true; + //} //Create the Content specific data - cmsContent var contentDto = dto.ContentVersionDto.ContentDto; diff --git a/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs b/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs index ca9045507b..5ae4efad45 100644 --- a/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs @@ -35,82 +35,68 @@ namespace Umbraco.Core.Persistence.Repositories } /// - /// Returns permissions for a given group for any number of nodes + /// Returns explicitly defined permissions for a user group for any number of nodes /// /// /// /// public IEnumerable GetPermissionsForEntities(int groupId, params int[] entityIds) { - var entityIdKey = GetEntityIdKey(entityIds); - return _runtimeCache.GetCacheItem>( - string.Format("{0}{1}{2}", - CacheKeys.UserGroupPermissionsCacheKey, - groupId, - entityIdKey), - () => - { - var whereCriteria = GetPermissionsForEntitiesCriteria(groupId, entityIds); - var sql = new Sql(); - sql.Select("*") - .From() - .Where(whereCriteria); - var result = _unitOfWork.Database.Fetch(sql).ToArray(); - // ToArray() to ensure it's all fetched from the db once - return ConvertToPermissionList(result); - }, - GetCacheTimeout(), - priority: GetCachePriority()); - } - - private static string GetEntityIdKey(int[] entityIds) - { - return string.Join(",", entityIds.Select(x => x.ToString(CultureInfo.InvariantCulture))); - } - - private string GetPermissionsForEntitiesCriteria(int groupId, params int[] entityIds) - { - var whereBuilder = new StringBuilder(); - whereBuilder.Append(_sqlSyntax.GetQuotedColumnName("userGroupId")); - whereBuilder.Append("="); - whereBuilder.Append(groupId); - - if (entityIds.Any()) + //var whereCriteria = GetPermissionsForEntitiesCriteria(groupId, entityIds); + var result = new List(); + //iterate in groups of 2000 since we don't want to exceed the max SQL param count + foreach (var groupOfIds in entityIds.InGroupsOf(2000)) { - whereBuilder.Append(" AND "); + var ids = groupOfIds; - //where nodeId = @nodeId1 OR nodeId = @nodeId2, etc... - whereBuilder.Append("("); - for (var index = 0; index < entityIds.Length; index++) - { - var entityId = entityIds[index]; - whereBuilder.Append(_sqlSyntax.GetQuotedColumnName("nodeId")); - whereBuilder.Append("="); - whereBuilder.Append(entityId); - if (index < entityIds.Length - 1) - { - whereBuilder.Append(" OR "); - } - } - - whereBuilder.Append(")"); - } - - return whereBuilder.ToString(); + var sql = new Sql(); + sql.Select("*") + .From(_sqlSyntax) + .Where(dto => dto.UserGroupId == groupId && ids.Contains(dto.NodeId), _sqlSyntax); + var permissions = _unitOfWork.Database.Fetch(sql); + result.AddRange(ConvertToPermissionList(permissions)); + } + return result; + + + //var entityIdKey = GetEntityIdKey(entityIds); + + //return _runtimeCache.GetCacheItem>( + // string.Format("{0}{1}{2}", + // CacheKeys.UserGroupPermissionsCacheKey, + // groupId, + // entityIdKey), + // () => + // { + // var whereCriteria = GetPermissionsForEntitiesCriteria(groupId, entityIds); + // var sql = new Sql(); + // sql.Select("*") + // .From() + // .Where(whereCriteria); + // var result = _unitOfWork.Database.Fetch(sql); + // return ConvertToPermissionList(result); + // }, + // GetCacheTimeout(), + // priority: GetCachePriority()); } - private static TimeSpan GetCacheTimeout() - { - //Since this cache can be quite large (http://issues.umbraco.org/issue/U4-2161) we will only have this exist in cache for 20 minutes, - // then it will refresh from the database. - return new TimeSpan(0, 20, 0); - } + //private static string GetEntityIdKey(int[] entityIds) + //{ + // return string.Join(",", entityIds.Select(x => x.ToString(CultureInfo.InvariantCulture))); + //} - private static CacheItemPriority GetCachePriority() - { - //Since this cache can be quite large (http://issues.umbraco.org/issue/U4-2161) we will make this priority below average - return CacheItemPriority.BelowNormal; - } + //private static TimeSpan GetCacheTimeout() + //{ + // //Since this cache can be quite large (http://issues.umbraco.org/issue/U4-2161) we will only have this exist in cache for 20 minutes, + // // then it will refresh from the database. + // return new TimeSpan(0, 20, 0); + //} + + //private static CacheItemPriority GetCachePriority() + //{ + // //Since this cache can be quite large (http://issues.umbraco.org/issue/U4-2161) we will make this priority below average + // return CacheItemPriority.BelowNormal; + //} private static IEnumerable ConvertToPermissionList(IEnumerable result) { @@ -142,8 +128,7 @@ namespace Umbraco.Core.Persistence.Repositories .Where(dto => dto.NodeId == entityId) .OrderBy(dto => dto.NodeId); - var result = _unitOfWork.Database.Fetch(sql).ToArray(); - // ToArray() to ensure it's all fetched from the db once + var result = _unitOfWork.Database.Fetch(sql); return ConvertToPermissionList(result); } diff --git a/src/Umbraco.Core/Services/IUserService.cs b/src/Umbraco.Core/Services/IUserService.cs index 3ac485bf2b..b55048ef4d 100644 --- a/src/Umbraco.Core/Services/IUserService.cs +++ b/src/Umbraco.Core/Services/IUserService.cs @@ -92,63 +92,63 @@ namespace Umbraco.Core.Services /// User to retrieve permissions for /// Specifiying nothing will return all user permissions for all nodes /// An enumerable list of + /// + /// > This is the ONLY one for permissions from 7.6! + /// IEnumerable GetPermissions(IUser user, params int[] nodeIds); /// /// Get permissions set for a group and optional node Ids /// /// Group to retrieve permissions for - /// + /// /// Flag indicating if we want to get just the permissions directly assigned for the group and path, /// or fall back to the group's default permissions when nothing is directly assigned /// /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of - IEnumerable GetPermissions(string groupAlias, bool directlyAssignedOnly, params int[] nodeIds); + IEnumerable GetPermissions(string groupAlias, bool fallbackToDefaultPermissions, params int[] nodeIds); /// /// Get permissions set for a group and optional node Ids /// /// Group to retrieve permissions for - /// + /// /// Flag indicating if we want to get just the permissions directly assigned for the group and path, /// or fall back to the group's default permissions when nothing is directly assigned /// /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of - IEnumerable GetPermissions(IUserGroup group, bool directlyAssignedOnly, params int[] nodeIds); + IEnumerable GetPermissions(IUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds); /// /// Gets the permissions for the provided user and path /// /// User to check permissions for /// Path to check permissions for - /// String indicating permissions for provided user and path - string GetPermissionsForPath(IUser user, string path); + EntityPermissionSet GetPermissionsForPath(IUser user, string path); /// /// Gets the permissions for the provided group and path /// /// Group alias to check permissions for /// Path to check permissions for - /// + /// /// Flag indicating if we want to get just the permissions directly assigned for the group and path, /// or fall back to the group's default permissions when nothing is directly assigned /// - /// String indicating permissions for provided user and path - string GetPermissionsForPath(string groupAlias, string path, bool directlyAssignedOnly = true); + EntityPermission GetPermissionsForPath(string groupAlias, string path, bool fallbackToDefaultPermissions = false); /// /// Gets the permissions for the provided group and path /// /// Group to check permissions for /// Path to check permissions for - /// + /// /// Flag indicating if we want to get just the permissions directly assigned for the group and path, /// or fall back to the group's default permissions when nothing is directly assigned /// - /// String indicating permissions for provided user and path - string GetPermissionsForPath(IUserGroup group, string path, bool directlyAssignedOnly = true); + EntityPermission GetPermissionsForPath(IUserGroup group, string path, bool fallbackToDefaultPermissions = false); /// /// Replaces the same permission set for a single group to any number of entities diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index f7cf263e91..b2a1ddec58 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -121,7 +121,7 @@ namespace Umbraco.Core.Services Language = GlobalSettings.DefaultUILanguage, Name = username, RawPasswordValue = passwordValue, - Username = username, + Username = username, IsLockedOut = false, IsApproved = isApproved }; @@ -208,11 +208,11 @@ namespace Umbraco.Core.Services public void Delete(IUser membershipUser) { //disable - membershipUser.IsApproved = false; - + membershipUser.IsApproved = false; + Save(membershipUser); - } - + } + /// /// This is simply a helper method which essentially just wraps the MembershipProvider's ChangePassword method /// @@ -616,12 +616,12 @@ namespace Umbraco.Core.Services { return repository.GetAllNotInGroup(groupId); } - } - + } + #endregion - + #region Implementation of IUserService - + /// /// Gets an IProfile by User Id. /// @@ -706,8 +706,8 @@ namespace Umbraco.Core.Services repository.AssignGroupPermission(groupId, permission, entityIds); uow.Commit(); } - } - + } + /// /// Gets all UserGroups or those specified as parameters /// @@ -720,8 +720,8 @@ namespace Umbraco.Core.Services var repository = RepositoryFactory.CreateUserGroupRepository(uow); return repository.GetAll(ids).OrderBy(x => x.Name); } - } - + } + public IEnumerable GetUserGroupsByAlias(params string[] aliases) { if (aliases.Length == 0) return Enumerable.Empty(); @@ -788,8 +788,8 @@ namespace Umbraco.Core.Services } var repository = RepositoryFactory.CreateUserGroupRepository(uow); - repository.AddOrUpdateGroupWithUsers(userGroup, userIds); - + repository.AddOrUpdateGroupWithUsers(userGroup, userIds); + uow.Commit(); if (raiseEvents) @@ -838,8 +838,8 @@ namespace Umbraco.Core.Services uow.Commit(); //TODO: Events? } - } - + } + /// /// Get permissions set for a user and node Id /// @@ -848,16 +848,19 @@ namespace Umbraco.Core.Services /// An enumerable list of public IEnumerable GetPermissions(IUser user, params int[] nodeIds) { + if (nodeIds.Length == 0) + return Enumerable.Empty(); + var result = new List(); + foreach (var group in user.Groups) { - //TODO: This may perform horribly :/ - foreach (var permission in GetPermissions(group.Alias, false, nodeIds)) + foreach (var permission in GetPermissions(group, true, nodeIds)) { AddOrAmendPermissionList(result, permission); } - } - + } + return result; } @@ -865,19 +868,57 @@ namespace Umbraco.Core.Services /// Get permissions set for a group and node Id /// /// Group to retrieve permissions for - /// + /// /// Flag indicating if we want to get just the permissions directly assigned for the group and path, /// or fall back to the group's default permissions when nothing is directly assigned /// /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of - public IEnumerable GetPermissions(string groupAlias, bool directlyAssignedOnly, params int[] nodeIds) - { + public IEnumerable GetPermissions(string groupAlias, bool fallbackToDefaultPermissions, params int[] nodeIds) + { + if (nodeIds.Length == 0) + return Enumerable.Empty(); + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateUserGroupRepository(uow); var group = repository.Get(groupAlias); - return GetPermissionsInternal(repository, group, directlyAssignedOnly, nodeIds); + if (group == null) throw new InvalidOperationException("No group found with alias " + groupAlias); + return GetPermissionsInternal(repository, group.ToReadOnlyGroup(), fallbackToDefaultPermissions, nodeIds); + } + } + + private IEnumerable GetPermissions(IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) + { + if (group == null) throw new ArgumentNullException("group"); + if (nodeIds.Length == 0) + return Enumerable.Empty(); + + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) + { + var repository = RepositoryFactory.CreateUserGroupRepository(uow); + return GetPermissionsInternal(repository, group, fallbackToDefaultPermissions, nodeIds); + } + } + + private IEnumerable GetPermissions(int groupId, bool fallbackToDefaultPermissions, params int[] nodeIds) + { + if (nodeIds.Length == 0) + return Enumerable.Empty(); + + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) + { + var repository = RepositoryFactory.CreateUserGroupRepository(uow); + + if (fallbackToDefaultPermissions == false) + { + //if fallbackToDefaultPermissions is false, we don't have to lookup the group + return repository.GetPermissionsForEntities(groupId, nodeIds); + } + + var group = repository.Get(groupId); + if (group == null) throw new InvalidOperationException("No group found with id " + groupId); + return GetPermissionsInternal(repository, group.ToReadOnlyGroup(), true, nodeIds); } } @@ -885,35 +926,40 @@ namespace Umbraco.Core.Services /// Get permissions set for a group and optional node Ids /// /// Group to retrieve permissions for - /// + /// /// Flag indicating if we want to get just the permissions directly assigned for the group and path, /// or fall back to the group's default permissions when nothing is directly assigned /// /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of - public IEnumerable GetPermissions(IUserGroup group, bool directlyAssignedOnly, params int[] nodeIds) + public IEnumerable GetPermissions(IUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) { if (group == null) throw new ArgumentNullException("group"); using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateUserGroupRepository(uow); - return GetPermissionsInternal(repository, group, directlyAssignedOnly, nodeIds); + return GetPermissionsInternal(repository, group.ToReadOnlyGroup(), fallbackToDefaultPermissions, nodeIds); } - } - - private IEnumerable GetPermissionsInternal(IUserGroupRepository repository, IUserGroup group, bool directlyAssignedOnly, params int[] nodeIds) + } + + private static IEnumerable GetPermissionsInternal(IUserGroupRepository repository, IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) { + if (group == null) throw new ArgumentNullException("group"); + + if (nodeIds.Length == 0) + return Enumerable.Empty(); + var explicitPermissions = repository.GetPermissionsForEntities(group.Id, nodeIds); var result = new List(explicitPermissions); // If requested, and no permissions are assigned to a particular node, then we will fill in those permissions with the group's defaults - if (directlyAssignedOnly == false) + if (fallbackToDefaultPermissions) { - var missingIds = nodeIds.Except(result.Select(x => x.EntityId)).ToList(); - if (missingIds.Any()) + var missingIds = nodeIds.Except(result.Select(x => x.EntityId)).ToArray(); + if (missingIds.Length > 0) { result.AddRange(missingIds - .Select(i => new EntityPermission(i, group.Permissions.ToArray()))); + .Select(i => new EntityPermission(i, group.Permissions.ToArray(), isDefaultPermissions: true))); } } return result; @@ -946,91 +992,163 @@ namespace Umbraco.Core.Services /// /// User to check permissions for /// Path to check permissions for - /// String indicating permissions for provided user and path - public string GetPermissionsForPath(IUser user, string path) - { - var assignedPermissions = GetPermissionsForGroupsAndPath(user.Groups.Select(x => x.Alias), path); - return GetAggregatePermissions(assignedPermissions); + public EntityPermissionSet GetPermissionsForPath(IUser user, string path) + { + var nodeIds = GetIdsFromPath(path); + + if (nodeIds.Length == 0) + return null; + + var permissionsByGroup = GetPermissionsForGroupsAndPath(user.Groups, nodeIds); + + // not sure this will ever happen, it shouldn't since this should return defaults, but maybe those are empty? + if (permissionsByGroup.Count == 0) + return null; + + var entityId = nodeIds[0]; + + var groupPermissions = new List(); + foreach (var entityPermission in permissionsByGroup) + { + var groupId = entityPermission.Key; + foreach (var assignedPermission in entityPermission.Value.AssignedPermissions) + { + groupPermissions.Add(new EntityPermissionSet.UserGroupPermission(groupId, assignedPermission)); + } + } + + var permissionSet = new EntityPermissionSet(entityId, groupPermissions); + return permissionSet; } /// /// Retrieves the permissions assigned to each group for a given path /// /// List of groups associated with the user - /// Path to check permissions for - /// List of strings indicating permissions for each groups - private IEnumerable GetPermissionsForGroupsAndPath(IEnumerable groups, string path) + /// Path to check permissions for + /// A dictionary of group ids and their associated node permissions + private IDictionary GetPermissionsForGroupsAndPath(IEnumerable groups, int[] pathIds) { return groups - .Select(g => GetPermissionsForPath(g, path, directlyAssignedOnly: false)) - .ToList(); - } - - /// - /// Aggregates a set of permissions strings to return a unique permissions string containing the most permissive set - /// - /// List of permission strings - /// Single permission string - private static string GetAggregatePermissions(IEnumerable assignedPermissions) - { - return string.Join(string.Empty, assignedPermissions - .SelectMany(s => s.ToCharArray()) - .Distinct()); - } - + .Select(g => new + { + group = g.Id, + permissions = GetPermissionsForPath(g, pathIds, fallbackToDefaultPermissions: true) + }) + .ToDictionary(x => x.group, x => x.permissions); + } + /// /// Gets the permissions for the provided group and path /// /// User to check permissions for /// Path to check permissions for - /// + /// /// Flag indicating if we want to get just the permissions directly assigned for the group and path, /// or fall back to the group's default permissions when nothing is directly assigned /// /// String indicating permissions for provided user and path - public string GetPermissionsForPath(string groupAlias, string path, bool directlyAssignedOnly = true) + public EntityPermission GetPermissionsForPath(string groupAlias, string path, bool fallbackToDefaultPermissions = false) { - var nodeId = GetNodeIdFromPath(path); - var permission = GetPermissions(groupAlias, directlyAssignedOnly, nodeId) - .FirstOrDefault(); - return permission != null - ? string.Join(string.Empty, permission.AssignedPermissions) - : string.Empty; - } - + var nodeIds = GetIdsFromPath(path); + //get permissions for all nodes in the path + var permissions = GetPermissions(groupAlias, fallbackToDefaultPermissions, nodeIds); + return GetPermissionsForPath(permissions, nodeIds, fallbackToDefaultPermissions); + } + /// /// Gets the permissions for the provided group and path /// /// Group to check permissions for /// Path to check permissions for - /// + /// /// Flag indicating if we want to get just the permissions directly assigned for the group and path, /// or fall back to the group's default permissions when nothing is directly assigned /// /// String indicating permissions for provided user and path - public string GetPermissionsForPath(IUserGroup group, string path, bool directlyAssignedOnly = true) + public EntityPermission GetPermissionsForPath(IUserGroup group, string path, bool fallbackToDefaultPermissions = false) { - var nodeId = GetNodeIdFromPath(path); - var permission = GetPermissions(group, directlyAssignedOnly, nodeId) - .FirstOrDefault(); - return permission != null - ? string.Join(string.Empty, permission.AssignedPermissions) - : string.Empty; + var nodeIds = GetIdsFromPath(path); + return GetPermissionsForPath(group.ToReadOnlyGroup(), nodeIds, fallbackToDefaultPermissions); } + private EntityPermission GetPermissionsForPath(IReadOnlyUserGroup group, int[] pathIds, bool fallbackToDefaultPermissions = false) + { + //get permissions for all nodes in the path + var permissions = GetPermissions(group, fallbackToDefaultPermissions, pathIds); + return GetPermissionsForPath(permissions, pathIds, fallbackToDefaultPermissions); + } + + //private EntityPermission GetPermissionsForPath(int groupId, string path, bool fallbackToDefaultPermissions = false) + //{ + // var nodeIds = GetIdsFromPath(path); + // //get permissions for all nodes in the path + // var permissions = GetPermissions(groupId, fallbackToDefaultPermissions, nodeIds); + // return GetPermissionsForPath(permissions, groupId, nodeIds, fallbackToDefaultPermissions); + //} + + /// + /// Returns the permissions for the path ids + /// + /// + /// Must be ordered deepest to shallowest (right to left) + /// + /// + private static EntityPermission GetPermissionsForPath( + IEnumerable pathPermissions, + int[] pathIds, + bool fallbackToDefaultPermissions = false) + { + //get permissions for all nodes in the path + var permissionsByEntityId = pathPermissions.ToDictionary(x => x.EntityId, x => x); + + //then the permissions assigned to the path will be the 'deepest' node found that has permissions + foreach (var id in pathIds) + { + EntityPermission permission; + if (permissionsByEntityId.TryGetValue(id, out permission)) + { + //don't return the default permissions if that is the one assigned here (we'll do that below if nothing was found) + if (permission.IsDefaultPermissions == false) + return permission; + } + } + + //if we've made it here it means that no implicit/inherited permissions were found so we return the defaults if that is specified + if (fallbackToDefaultPermissions == false) + return null; + + return permissionsByEntityId[pathIds[0]]; + } /// - /// Parses a path to find the lowermost node id + /// Convert a path to node ids in the order from right to left (deepest to shallowest) /// - /// Path as string - /// Node id - private static int GetNodeIdFromPath(string path) + /// + /// + private int[] GetIdsFromPath(string path) { - return path.Contains(",") - ? int.Parse(path.Substring(path.LastIndexOf(",", StringComparison.Ordinal) + 1)) - : int.Parse(path); - } - + var nodeIds = path.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries) + .Select(x => x.TryConvertTo()) + .Where(x => x.Success) + .Select(x => x.Result) + .Reverse() + .ToArray(); + return nodeIds; + } + + ///// + ///// Parses a path to find the lowermost node id + ///// + ///// Path as string + ///// Node id + //private static int GetNodeIdFromPath(string path) + //{ + // return path.Contains(",") + // ? int.Parse(path.Substring(path.LastIndexOf(",", StringComparison.Ordinal) + 1)) + // : int.Parse(path); + //} + /// /// Checks in a set of permissions associated with a user for those related to a given nodeId /// diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs index a3e5510941..2b9e0b1ed3 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs @@ -135,11 +135,11 @@ namespace Umbraco.Tests.Persistence.Repositories using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) { var hasPropertiesContentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage"); - content1 = MockedContent.CreateSimpleContent(hasPropertiesContentType); - + content1 = MockedContent.CreateSimpleContent(hasPropertiesContentType); + contentTypeRepository.AddOrUpdate(hasPropertiesContentType); - repository.AddOrUpdate(content1); - unitOfWork.Commit(); + repository.AddOrUpdate(content1); + unitOfWork.Commit(); } var versionDtos = new List(); @@ -167,7 +167,7 @@ namespace Umbraco.Tests.Persistence.Repositories VersionId = version, WriterUserId = 0, UpdateDate = versionDate, - TemplateId = content1.Template == null || content1.Template.Id <= 0 ? null : (int?) content1.Template.Id + TemplateId = content1.Template == null || content1.Template.Id <= 0 ? null : (int?)content1.Template.Id }); } @@ -188,7 +188,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.AreEqual(contentItem.Version, versionDtos.Single(x => x.Id == versionDtos.Max(y => y.Id)).VersionId); var allVersions = repository.GetAllVersions(content[0].Id); - var allKnownVersions = versionDtos.Select(x => x.VersionId).Union(new[]{ content1.Version }).ToArray(); + var allKnownVersions = versionDtos.Select(x => x.VersionId).Union(new[] { content1.Version }).ToArray(); Assert.IsTrue(allKnownVersions.ContainsAll(allVersions.Select(x => x.Version))); Assert.IsTrue(allVersions.Select(x => x.Version).ContainsAll(allKnownVersions)); } @@ -547,9 +547,9 @@ namespace Umbraco.Tests.Persistence.Repositories var userGroup = MockedUserGroup.CreateUserGroup("1"); repository.AddOrUpdate(userGroup); unitOfWork.Commit(); - } - - ContentTypeRepository contentTypeRepository; + } + + ContentTypeRepository contentTypeRepository; using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) { var contentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage"); @@ -563,7 +563,7 @@ namespace Umbraco.Tests.Persistence.Repositories unitOfWork.Commit(); // Act - repository.AssignEntityPermission(parentPage, 'A', new [] { 1 }); + repository.AssignEntityPermission(parentPage, 'A', new[] { 1 }); var childPage = MockedContent.CreateSimpleContent(contentType, "child", parentPage); repository.AddOrUpdate(childPage); unitOfWork.Commit(); diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index b521172417..93202af102 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -1458,6 +1458,178 @@ namespace Umbraco.Tests.Services Assert.That(contents.Any(), Is.False); } + [Test] + public void Ensures_Permissions_Are_Set_On_Copied_Entity_To_Parent_Without_Permissions() + { + // Arrange + var userGroup = MockedUserGroup.CreateUserGroup("1"); + ServiceContext.UserService.Save(userGroup); + + var contentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage"); + contentType.AllowedContentTypes = new List + { + new ContentTypeSort(new Lazy(() => contentType.Id), 0, contentType.Alias) + }; + ServiceContext.ContentTypeService.Save(contentType); + + var parentPage = MockedContent.CreateSimpleContent(contentType); + ServiceContext.ContentService.Save(parentPage); + ServiceContext.ContentService.AssignContentPermission(parentPage, 'A', new[] { 1 }); + + var childPage = MockedContent.CreateSimpleContent(contentType, "child", parentPage); + ServiceContext.ContentService.Save(childPage); + + //Ok, now copy, what should happen is the childPage will not have any permissions assigned since it's new parent + //doesn't have any assigned + var parentPage2 = MockedContent.CreateSimpleContent(contentType); + ServiceContext.ContentService.Save(parentPage2); + + var copy = ServiceContext.ContentService.Copy(childPage, parentPage2.Id, false, true); + + //Re-get the permissions, since there was none assigned to the new parent, the child shouldn't have any directly assigned + var copyPermissions = ServiceContext.ContentService.GetPermissionsForEntity(copy); + Assert.AreEqual(0, copyPermissions.Count()); + } + + [Test] + public void Ensures_Permissions_Are_Set_On_Copied_Entity_To_Parent_With_Permissions() + { + // Arrange + var userGroup = MockedUserGroup.CreateUserGroup("1"); + ServiceContext.UserService.Save(userGroup); + + var contentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage"); + contentType.AllowedContentTypes = new List + { + new ContentTypeSort(new Lazy(() => contentType.Id), 0, contentType.Alias) + }; + ServiceContext.ContentTypeService.Save(contentType); + + var parentPage = MockedContent.CreateSimpleContent(contentType); + ServiceContext.ContentService.Save(parentPage); + ServiceContext.ContentService.AssignContentPermission(parentPage, 'A', new[] { 1 }); + + var childPage = MockedContent.CreateSimpleContent(contentType, "child", parentPage); + ServiceContext.ContentService.Save(childPage); + + //Ok, now copy, what should happen is the childPage will have it's new parent permissions copied over + var parentPage2 = MockedContent.CreateSimpleContent(contentType); + ServiceContext.ContentService.Save(parentPage2); + ServiceContext.ContentService.AssignContentPermission(parentPage2, 'B', new[] { 1 }); + + var copy = ServiceContext.ContentService.Copy(childPage, parentPage2.Id, false, true); + + //Re-get the permissions, since there was none assigned to the new parent, the child shouldn't have any directly assigned + var copyPermissions = ServiceContext.ContentService.GetPermissionsForEntity(copy); + Assert.AreEqual(1, copyPermissions.Count()); + Assert.AreEqual("B", copyPermissions.Single().AssignedPermissions.First()); + } + + [Test] + public void Ensures_Permissions_Are_Set_On_Copied_Descendants_To_Parent_With_Permissions() + { + // Arrange + var userGroup = MockedUserGroup.CreateUserGroup("1"); + ServiceContext.UserService.Save(userGroup); + + var contentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage"); + contentType.AllowedContentTypes = new List + { + new ContentTypeSort(new Lazy(() => contentType.Id), 0, contentType.Alias) + }; + ServiceContext.ContentTypeService.Save(contentType); + + var parentPage = MockedContent.CreateSimpleContent(contentType); + ServiceContext.ContentService.Save(parentPage); + ServiceContext.ContentService.AssignContentPermission(parentPage, 'A', new[] { 1 }); + + var childPage1 = MockedContent.CreateSimpleContent(contentType, "child1", parentPage); + ServiceContext.ContentService.Save(childPage1); + var childPage2 = MockedContent.CreateSimpleContent(contentType, "child2", childPage1); + ServiceContext.ContentService.Save(childPage2); + var childPage3 = MockedContent.CreateSimpleContent(contentType, "child3", childPage2); + ServiceContext.ContentService.Save(childPage3); + + //Ok, now copy, what should happen is the childPage will have it's new parent permissions copied over + var parentPage2 = MockedContent.CreateSimpleContent(contentType); + ServiceContext.ContentService.Save(parentPage2); + ServiceContext.ContentService.AssignContentPermission(parentPage2, 'B', new[] { 1 }); + + var copy = ServiceContext.ContentService.Copy(childPage1, parentPage2.Id, false, true); + + //Re-get the permissions, since there was none assigned to the new parent, the child shouldn't have any directly assigned + var copyPermissions = ServiceContext.ContentService.GetPermissionsForEntity(copy); + Assert.AreEqual(1, copyPermissions.Count()); + Assert.AreEqual("B", copyPermissions.Single().AssignedPermissions.First()); + + var descendants = ServiceContext.ContentService.GetDescendants(copy).ToArray(); + Assert.AreEqual(2, descendants.Length); + + foreach (var descendant in descendants) + { + var permissions = ServiceContext.ContentService.GetPermissionsForEntity(descendant); + Assert.AreEqual(1, permissions.Count()); + Assert.AreEqual("B", permissions.Single().AssignedPermissions.First()); + } + } + + [Test] + public void Ensures_Permissions_Are_Set_On_Descendants_When_Permissions_Added() + { + // Arrange + var userGroup = MockedUserGroup.CreateUserGroup("1"); + ServiceContext.UserService.Save(userGroup); + + var contentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage"); + contentType.AllowedContentTypes = new List + { + new ContentTypeSort(new Lazy(() => contentType.Id), 0, contentType.Alias) + }; + ServiceContext.ContentTypeService.Save(contentType); + + var parentPage = MockedContent.CreateSimpleContent(contentType); + ServiceContext.ContentService.Save(parentPage); + var childPage1 = MockedContent.CreateSimpleContent(contentType, "child1", parentPage); + ServiceContext.ContentService.Save(childPage1); + var childPage2 = MockedContent.CreateSimpleContent(contentType, "child2", childPage1); + ServiceContext.ContentService.Save(childPage2); + var childPage3 = MockedContent.CreateSimpleContent(contentType, "child3", childPage2); + ServiceContext.ContentService.Save(childPage3); + + //ensure there are no permissions on any node + + var permissions = ServiceContext.ContentService.GetPermissionsForEntity(parentPage); + Assert.AreEqual(0, permissions.Count()); + + var descendants = ServiceContext.ContentService.GetDescendants(parentPage).ToArray(); + Assert.AreEqual(3, descendants.Length); + + foreach (var descendant in descendants) + { + permissions = ServiceContext.ContentService.GetPermissionsForEntity(descendant); + Assert.AreEqual(0, permissions.Count()); + } + + //Ok, now assign permissions to the parent, all descenents should get these right? + ServiceContext.ContentService.AssignContentPermission(parentPage, 'A', new[] { 1 }); + + //re-test + + permissions = ServiceContext.ContentService.GetPermissionsForEntity(parentPage); + Assert.AreEqual(1, permissions.Count()); + Assert.AreEqual("A", permissions.Single().AssignedPermissions.First()); + + descendants = ServiceContext.ContentService.GetDescendants(parentPage).ToArray(); + Assert.AreEqual(3, descendants.Length); + + foreach (var descendant in descendants) + { + permissions = ServiceContext.ContentService.GetPermissionsForEntity(descendant); + Assert.AreEqual(1, permissions.Count()); + Assert.AreEqual("A", permissions.Single().AssignedPermissions.First()); + } + } + [Test] public void Can_Empty_RecycleBin_With_Content_That_Has_All_Related_Data() { diff --git a/src/Umbraco.Tests/Services/UserServiceTests.cs b/src/Umbraco.Tests/Services/UserServiceTests.cs index b0ec03f754..d672229e6b 100644 --- a/src/Umbraco.Tests/Services/UserServiceTests.cs +++ b/src/Umbraco.Tests/Services/UserServiceTests.cs @@ -4,14 +4,10 @@ using System.Security.Cryptography; using System.Text; using NUnit.Framework; using Umbraco.Core.Models.Membership; -using Umbraco.Core.Services; using Umbraco.Tests.TestHelpers.Entities; using umbraco.BusinessLogic.Actions; using Umbraco.Core.Persistence.Querying; -using Umbraco.Core.Services; using Umbraco.Tests.TestHelpers; -using Umbraco.Tests.TestHelpers.Entities; -using umbraco.BusinessLogic.Actions; using Umbraco.Core; using Umbraco.Core.Persistence.DatabaseModelDefinitions; @@ -37,7 +33,7 @@ namespace Umbraco.Tests.Services } [Test] - public void UserService_Get_User_Permissions_For_Unassigned_Permission_Nodes() + public void Get_User_Permissions_For_Unassigned_Permission_Nodes() { // Arrange var userService = ServiceContext.UserService; @@ -55,17 +51,18 @@ namespace Umbraco.Tests.Services ServiceContext.ContentService.Save(content); // Act - var permissions = userService.GetPermissions(user, content.ElementAt(0).Id, content.ElementAt(1).Id, content.ElementAt(2).Id); + var permissions = userService.GetPermissions(user, content[0].Id, content[1].Id, content[2].Id) + .ToArray(); //assert - Assert.AreEqual(3, permissions.Count()); - Assert.AreEqual(17, permissions.ElementAt(0).AssignedPermissions.Count()); - Assert.AreEqual(17, permissions.ElementAt(1).AssignedPermissions.Count()); - Assert.AreEqual(17, permissions.ElementAt(2).AssignedPermissions.Count()); + Assert.AreEqual(3, permissions.Length); + Assert.AreEqual(17, permissions[0].AssignedPermissions.Length); + Assert.AreEqual(17, permissions[1].AssignedPermissions.Length); + Assert.AreEqual(17, permissions[2].AssignedPermissions.Length); } [Test] - public void UserService_Get_User_Permissions_For_Assigned_Permission_Nodes() + public void Get_User_Permissions_For_Assigned_Permission_Nodes() { // Arrange var userService = ServiceContext.UserService; @@ -99,7 +96,7 @@ namespace Umbraco.Tests.Services } [Test] - public void UserService_Get_UserGroup_Assigned_Permissions() + public void Get_UserGroup_Assigned_Permissions() { // Arrange var userService = ServiceContext.UserService; @@ -121,38 +118,6 @@ namespace Umbraco.Tests.Services ServiceContext.ContentService.AssignContentPermission(content.ElementAt(1), ActionDelete.Instance.Letter, new int[] { userGroup.Id }); ServiceContext.ContentService.AssignContentPermission(content.ElementAt(2), ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); - // Act - var permissions = userService.GetPermissions(userGroup, true, content.ElementAt(0).Id, content.ElementAt(1).Id, content.ElementAt(2).Id); - - //assert - Assert.AreEqual(3, permissions.Count()); - Assert.AreEqual(3, permissions.ElementAt(0).AssignedPermissions.Length); - Assert.AreEqual(2, permissions.ElementAt(1).AssignedPermissions.Length); - Assert.AreEqual(1, permissions.ElementAt(2).AssignedPermissions.Length); - } - - [Test] - public void UserService_Get_UserGroup_Assigned_And_Default_Permissions() - { - // Arrange - var userService = ServiceContext.UserService; - var userGroup = CreateTestUserGroup(); - - var contentType = MockedContentTypes.CreateSimpleContentType(); - ServiceContext.ContentTypeService.Save(contentType); - var content = new[] - { - MockedContent.CreateSimpleContent(contentType), - MockedContent.CreateSimpleContent(contentType), - MockedContent.CreateSimpleContent(contentType) - }; - ServiceContext.ContentService.Save(content); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(0), ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(0), ActionDelete.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(0), ActionMove.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(1), ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(1), ActionDelete.Instance.Letter, new int[] { userGroup.Id }); - // Act var permissions = userService.GetPermissions(userGroup, false, content.ElementAt(0).Id, content.ElementAt(1).Id, content.ElementAt(2).Id); @@ -160,7 +125,69 @@ namespace Umbraco.Tests.Services Assert.AreEqual(3, permissions.Count()); Assert.AreEqual(3, permissions.ElementAt(0).AssignedPermissions.Length); Assert.AreEqual(2, permissions.ElementAt(1).AssignedPermissions.Length); - Assert.AreEqual(17, permissions.ElementAt(2).AssignedPermissions.Length); + Assert.AreEqual(1, permissions.ElementAt(2).AssignedPermissions.Length); + } + + [Test] + public void Get_UserGroup_Assigned_And_Default_Permissions() + { + // Arrange + var userService = ServiceContext.UserService; + var userGroup = CreateTestUserGroup(); + + var contentType = MockedContentTypes.CreateSimpleContentType(); + ServiceContext.ContentTypeService.Save(contentType); + var content = new[] + { + MockedContent.CreateSimpleContent(contentType), + MockedContent.CreateSimpleContent(contentType), + MockedContent.CreateSimpleContent(contentType) + }; + ServiceContext.ContentService.Save(content); + ServiceContext.ContentService.AssignContentPermission(content.ElementAt(0), ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content.ElementAt(0), ActionDelete.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content.ElementAt(0), ActionMove.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content.ElementAt(1), ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content.ElementAt(1), ActionDelete.Instance.Letter, new int[] { userGroup.Id }); + + // Act + var permissions = userService.GetPermissions(userGroup, true, content.ElementAt(0).Id, content.ElementAt(1).Id, content.ElementAt(2).Id) + .ToArray(); + + //assert + Assert.AreEqual(3, permissions.Length); + Assert.AreEqual(3, permissions[0].AssignedPermissions.Length); + Assert.AreEqual(2, permissions[1].AssignedPermissions.Length); + Assert.AreEqual(17,permissions[2].AssignedPermissions.Length); + } + + [Test] + public void Get_User_Implicit_Permissions() + { + // Arrange + var userService = ServiceContext.UserService; + var userGroup = CreateTestUserGroup(); + + var contentType = MockedContentTypes.CreateSimpleContentType(); + ServiceContext.ContentTypeService.Save(contentType); + var parent = MockedContent.CreateSimpleContent(contentType); + ServiceContext.ContentService.Save(parent); + var child1 = MockedContent.CreateSimpleContent(contentType, "child1", parent); + ServiceContext.ContentService.Save(child1); + var child2 = MockedContent.CreateSimpleContent(contentType, "child2", child1); + ServiceContext.ContentService.Save(child2); + + ServiceContext.ContentService.AssignContentPermission(parent, ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(parent, ActionDelete.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(parent, ActionMove.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(parent, ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(parent, ActionDelete.Instance.Letter, new int[] { userGroup.Id }); + + // Act + var permissions = userService.GetPermissionsForPath(userGroup, child2.Path); + + //assert + Assert.AreEqual(3, permissions.AssignedPermissions.Length); } [Test] diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/cruds.aspx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/cruds.aspx.cs index aaab898ae2..ee93aec60b 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/cruds.aspx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/cruds.aspx.cs @@ -89,7 +89,7 @@ namespace umbraco.dialogs if (a.CanBePermissionAssigned == false) continue; var permissions = userService.GetPermissionsForPath(group, _node.Path); - if (permissions.Contains(a.Letter)) + if (permissions.AssignedPermissions.Contains(a.Letter.ToString())) { chk.Checked = true; } diff --git a/src/umbraco.businesslogic/User.cs b/src/umbraco.businesslogic/User.cs index fbf226c39f..0f92a4427f 100644 --- a/src/umbraco.businesslogic/User.cs +++ b/src/umbraco.businesslogic/User.cs @@ -571,7 +571,8 @@ namespace umbraco.BusinessLogic if (_lazyId.HasValue) SetupUser(_lazyId.Value); var userService = ApplicationContext.Current.Services.UserService; - return userService.GetPermissionsForPath(UserEntity, path); + return string.Join("", + userService.GetPermissionsForPath(UserEntity, path).PermissionsSet.SelectMany(x => x.Permission)); } /// From d9937fb276c556e2b24517d18a770b891437f9dd Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 12 Jul 2017 14:01:36 +1000 Subject: [PATCH 02/11] Stop trying to build the UI.Client project as part of the sln build --- src/umbraco.sln | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/umbraco.sln b/src/umbraco.sln index df260903ee..a2603aecbe 100644 --- a/src/umbraco.sln +++ b/src/umbraco.sln @@ -1,6 +1,6 @@ Microsoft Visual Studio Solution File, Format Version 12.00 -# Visual Studio 14 -VisualStudioVersion = 14.0.25420.1 +# Visual Studio 15 +VisualStudioVersion = 15.0.26430.15 MinimumVisualStudioVersion = 10.0.40219.1 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Umbraco.Web.UI", "Umbraco.Web.UI\Umbraco.Web.UI.csproj", "{4C4C194C-B5E4-4991-8F87-4373E24CC19F}" EndProject @@ -127,9 +127,7 @@ Global {4C4C194C-B5E4-4991-8F87-4373E24CC19F}.Release|Any CPU.ActiveCfg = Release|Any CPU {4C4C194C-B5E4-4991-8F87-4373E24CC19F}.Release|Any CPU.Build.0 = Release|Any CPU {3819A550-DCEC-4153-91B4-8BA9F7F0B9B4}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {3819A550-DCEC-4153-91B4-8BA9F7F0B9B4}.Debug|Any CPU.Build.0 = Debug|Any CPU {3819A550-DCEC-4153-91B4-8BA9F7F0B9B4}.Release|Any CPU.ActiveCfg = Debug|Any CPU - {3819A550-DCEC-4153-91B4-8BA9F7F0B9B4}.Release|Any CPU.Build.0 = Debug|Any CPU {651E1350-91B6-44B7-BD60-7207006D7003}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {651E1350-91B6-44B7-BD60-7207006D7003}.Debug|Any CPU.Build.0 = Debug|Any CPU {651E1350-91B6-44B7-BD60-7207006D7003}.Release|Any CPU.ActiveCfg = Release|Any CPU From 7115d85440bc4d2d80a72f34df06f355a23d6525 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 12 Jul 2017 14:06:30 +1000 Subject: [PATCH 03/11] Removes unused and unneeded APIs on IUserService, updates code comments to describe implicit vs explicit permissions --- .../Interfaces/IUserGroupRepository.cs | 4 +- .../Repositories/UserGroupRepository.cs | 4 +- src/Umbraco.Core/Services/IUserService.cs | 35 +----- src/Umbraco.Core/Services/UserService.cs | 114 +++--------------- .../umbraco/dialogs/cruds.aspx.cs | 76 +----------- 5 files changed, 31 insertions(+), 202 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs index 3b2663e026..3ccfa89885 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs @@ -26,10 +26,10 @@ namespace Umbraco.Core.Persistence.Repositories void AddOrUpdateGroupWithUsers(IUserGroup userGroup, int[] userIds); /// - /// Gets the group permissions for the specified entities + /// Gets explicilty defined permissions for the group for specified entities /// /// Id of group - /// Array of entity Ids + /// Array of entity Ids, if empty will return permissions for the group for all entities IEnumerable GetPermissionsForEntities(int groupId, params int[] entityIds); /// diff --git a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs index 7a160f7f24..e0717deda4 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs @@ -91,10 +91,10 @@ namespace Umbraco.Core.Persistence.Repositories /// - /// Gets the group permissions for the specified entities + /// Gets explicilty defined permissions for the group for specified entities /// /// Id of group - /// Array of entity Ids + /// Array of entity Ids, if empty will return permissions for the group for all entities public IEnumerable GetPermissionsForEntities(int groupId, params int[] entityIds) { var repo = new PermissionRepository(UnitOfWork, _cacheHelper, SqlSyntax); diff --git a/src/Umbraco.Core/Services/IUserService.cs b/src/Umbraco.Core/Services/IUserService.cs index b55048ef4d..de228412da 100644 --- a/src/Umbraco.Core/Services/IUserService.cs +++ b/src/Umbraco.Core/Services/IUserService.cs @@ -86,31 +86,19 @@ namespace Umbraco.Core.Services void DeleteSectionFromAllUserGroups(string sectionAlias); /// - /// Get permissions set for a user and optional node ids + /// Get explicitly assigned permissions for a user and optional node ids /// /// If no permissions are found for a particular entity then the user's default permissions will be applied /// User to retrieve permissions for /// Specifiying nothing will return all user permissions for all nodes /// An enumerable list of /// - /// > This is the ONLY one for permissions from 7.6! + /// This will return the default permissions for the user's groups for nodes that don't have explicitly defined permissions /// IEnumerable GetPermissions(IUser user, params int[] nodeIds); - + /// - /// Get permissions set for a group and optional node Ids - /// - /// Group to retrieve permissions for - /// - /// Flag indicating if we want to get just the permissions directly assigned for the group and path, - /// or fall back to the group's default permissions when nothing is directly assigned - /// - /// Specifiying nothing will return all permissions for all nodes - /// An enumerable list of - IEnumerable GetPermissions(string groupAlias, bool fallbackToDefaultPermissions, params int[] nodeIds); - - /// - /// Get permissions set for a group and optional node Ids + /// Get explicitly assigned permissions for a group and optional node Ids /// /// Group to retrieve permissions for /// @@ -122,23 +110,12 @@ namespace Umbraco.Core.Services IEnumerable GetPermissions(IUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds); /// - /// Gets the permissions for the provided user and path + /// Gets the implicit/inherited permissions for the user for the given path /// /// User to check permissions for /// Path to check permissions for EntityPermissionSet GetPermissionsForPath(IUser user, string path); - - /// - /// Gets the permissions for the provided group and path - /// - /// Group alias to check permissions for - /// Path to check permissions for - /// - /// Flag indicating if we want to get just the permissions directly assigned for the group and path, - /// or fall back to the group's default permissions when nothing is directly assigned - /// - EntityPermission GetPermissionsForPath(string groupAlias, string path, bool fallbackToDefaultPermissions = false); - + /// /// Gets the permissions for the provided group and path /// diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index b2a1ddec58..841a51456e 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -13,7 +13,6 @@ using Umbraco.Core.Persistence.Querying; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Persistence.UnitOfWork; using Umbraco.Core.Security; -using Umbraco.Core.Persistence.Querying; namespace Umbraco.Core.Services { @@ -841,16 +840,13 @@ namespace Umbraco.Core.Services } /// - /// Get permissions set for a user and node Id + /// Get explicitly assigned permissions for a user and optional node ids /// /// User to retrieve permissions for /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of public IEnumerable GetPermissions(IUser user, params int[] nodeIds) { - if (nodeIds.Length == 0) - return Enumerable.Empty(); - var result = new List(); foreach (var group in user.Groups) @@ -859,35 +855,21 @@ namespace Umbraco.Core.Services { AddOrAmendPermissionList(result, permission); } - } - + } + return result; - } + } /// - /// Get permissions set for a group and node Id + /// Get explicitly assigned permissions for a group and optional node Ids /// - /// Group to retrieve permissions for + /// Group to retrieve permissions for /// /// Flag indicating if we want to get just the permissions directly assigned for the group and path, /// or fall back to the group's default permissions when nothing is directly assigned /// /// Specifiying nothing will return all permissions for all nodes - /// An enumerable list of - public IEnumerable GetPermissions(string groupAlias, bool fallbackToDefaultPermissions, params int[] nodeIds) - { - if (nodeIds.Length == 0) - return Enumerable.Empty(); - - using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) - { - var repository = RepositoryFactory.CreateUserGroupRepository(uow); - var group = repository.Get(groupAlias); - if (group == null) throw new InvalidOperationException("No group found with alias " + groupAlias); - return GetPermissionsInternal(repository, group.ToReadOnlyGroup(), fallbackToDefaultPermissions, nodeIds); - } - } - + /// An enumerable list of private IEnumerable GetPermissions(IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) { if (group == null) throw new ArgumentNullException("group"); @@ -899,31 +881,10 @@ namespace Umbraco.Core.Services var repository = RepositoryFactory.CreateUserGroupRepository(uow); return GetPermissionsInternal(repository, group, fallbackToDefaultPermissions, nodeIds); } - } - - private IEnumerable GetPermissions(int groupId, bool fallbackToDefaultPermissions, params int[] nodeIds) - { - if (nodeIds.Length == 0) - return Enumerable.Empty(); - - using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) - { - var repository = RepositoryFactory.CreateUserGroupRepository(uow); + } - if (fallbackToDefaultPermissions == false) - { - //if fallbackToDefaultPermissions is false, we don't have to lookup the group - return repository.GetPermissionsForEntities(groupId, nodeIds); - } - - var group = repository.Get(groupId); - if (group == null) throw new InvalidOperationException("No group found with id " + groupId); - return GetPermissionsInternal(repository, group.ToReadOnlyGroup(), true, nodeIds); - } - } - /// - /// Get permissions set for a group and optional node Ids + /// Get explicitly assigned permissions for a group and optional node Ids /// /// Group to retrieve permissions for /// @@ -945,10 +906,7 @@ namespace Umbraco.Core.Services private static IEnumerable GetPermissionsInternal(IUserGroupRepository repository, IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) { if (group == null) throw new ArgumentNullException("group"); - - if (nodeIds.Length == 0) - return Enumerable.Empty(); - + var explicitPermissions = repository.GetPermissionsForEntities(group.Id, nodeIds); var result = new List(explicitPermissions); @@ -972,7 +930,7 @@ namespace Umbraco.Core.Services /// /// List of already found permissions /// New permission to aggregate - private void AddOrAmendPermissionList(IList permissions, EntityPermission groupPermission) + private static void AddOrAmendPermissionList(ICollection permissions, EntityPermission groupPermission) { //TODO: Fix the performance of this, we need to use things like HashSet and equality checkers, we are iterating too much @@ -988,7 +946,7 @@ namespace Umbraco.Core.Services } /// - /// Gets the permissions for the provided user and path + /// Gets the implicit/inherited permissions for the user for the given path /// /// User to check permissions for /// Path to check permissions for @@ -997,13 +955,13 @@ namespace Umbraco.Core.Services var nodeIds = GetIdsFromPath(path); if (nodeIds.Length == 0) - return null; + return EntityPermissionSet.Empty(); var permissionsByGroup = GetPermissionsForGroupsAndPath(user.Groups, nodeIds); // not sure this will ever happen, it shouldn't since this should return defaults, but maybe those are empty? if (permissionsByGroup.Count == 0) - return null; + return EntityPermissionSet.Empty(); var entityId = nodeIds[0]; @@ -1036,25 +994,7 @@ namespace Umbraco.Core.Services permissions = GetPermissionsForPath(g, pathIds, fallbackToDefaultPermissions: true) }) .ToDictionary(x => x.group, x => x.permissions); - } - - /// - /// Gets the permissions for the provided group and path - /// - /// User to check permissions for - /// Path to check permissions for - /// - /// Flag indicating if we want to get just the permissions directly assigned for the group and path, - /// or fall back to the group's default permissions when nothing is directly assigned - /// - /// String indicating permissions for provided user and path - public EntityPermission GetPermissionsForPath(string groupAlias, string path, bool fallbackToDefaultPermissions = false) - { - var nodeIds = GetIdsFromPath(path); - //get permissions for all nodes in the path - var permissions = GetPermissions(groupAlias, fallbackToDefaultPermissions, nodeIds); - return GetPermissionsForPath(permissions, nodeIds, fallbackToDefaultPermissions); - } + } /// /// Gets the permissions for the provided group and path @@ -1078,15 +1018,7 @@ namespace Umbraco.Core.Services var permissions = GetPermissions(group, fallbackToDefaultPermissions, pathIds); return GetPermissionsForPath(permissions, pathIds, fallbackToDefaultPermissions); } - - //private EntityPermission GetPermissionsForPath(int groupId, string path, bool fallbackToDefaultPermissions = false) - //{ - // var nodeIds = GetIdsFromPath(path); - // //get permissions for all nodes in the path - // var permissions = GetPermissions(groupId, fallbackToDefaultPermissions, nodeIds); - // return GetPermissionsForPath(permissions, groupId, nodeIds, fallbackToDefaultPermissions); - //} - + /// /// Returns the permissions for the path ids /// @@ -1136,19 +1068,7 @@ namespace Umbraco.Core.Services .ToArray(); return nodeIds; } - - ///// - ///// Parses a path to find the lowermost node id - ///// - ///// Path as string - ///// Node id - //private static int GetNodeIdFromPath(string path) - //{ - // return path.Contains(",") - // ? int.Parse(path.Substring(path.LastIndexOf(",", StringComparison.Ordinal) + 1)) - // : int.Parse(path); - //} - + /// /// Checks in a set of permissions associated with a user for those related to a given nodeId /// diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/cruds.aspx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/cruds.aspx.cs index ee93aec60b..de4c902ee2 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/cruds.aspx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/cruds.aspx.cs @@ -19,7 +19,8 @@ namespace umbraco.dialogs /// /// Summary description for cruds. /// - public partial class cruds : BasePages.UmbracoEnsuredPage + [Obsolete("Remove this for 7.7 release!")] + public class cruds : BasePages.UmbracoEnsuredPage { public cruds() @@ -37,78 +38,9 @@ namespace umbraco.dialogs pane_form.Text = ui.Text("actions", "SetPermissionsForThePage",_node.Text); } - override protected void OnInit(EventArgs e) + protected override void OnInit(EventArgs e) { - base.OnInit(e); - - _node = new CMSNode(Request.GetItemAs("id")); - - var ht = new HtmlTable(); - ht.Attributes.Add("class", "table"); - - var names = new HtmlTableRow(); - - var corner = new HtmlTableCell("th"); - corner.Style.Add("border", "none"); - names.Cells.Add(corner); - - foreach (var a in ActionsResolver.Current.Actions) - { - if (a.CanBePermissionAssigned == false) continue; - - var permissionRow = new HtmlTableRow(); - var label = new HtmlTableCell - { - InnerText = ui.Text("actions", a.Alias) - }; - permissionRow.Cells.Add(label); - _permissions.Add(a.Alias, permissionRow); - } - - ht.Rows.Add(names); - - var userService = ApplicationContext.Current.Services.UserService; - foreach (var group in userService.GetAllUserGroups()) - { - var hc = new HtmlTableCell("th") - { - InnerText = group.Name - }; - hc.Style.Add("text-align", "center"); - hc.Style.Add("border", "none"); - names.Cells.Add(hc); - - foreach (var a in ActionsResolver.Current.Actions) - { - var chk = new CheckBox - { - //Each checkbox is named with the group _ permission alias so we can parse - ID = group.Id + "_" + a.Letter - }; - - if (a.CanBePermissionAssigned == false) continue; - - var permissions = userService.GetPermissionsForPath(group, _node.Path); - if (permissions.AssignedPermissions.Contains(a.Letter.ToString())) - { - chk.Checked = true; - } - - var cell = new HtmlTableCell(); - cell.Style.Add("text-align", "center"); - cell.Controls.Add(chk); - - _permissions[a.Alias].Cells.Add(cell); - } - } - - //add all collected rows - foreach (var perm in _permissions.Values) - { - ht.Rows.Add(perm); - } - - PlaceHolder1.Controls.Add(ht); + throw new NotSupportedException("This cruds.aspx.cs needs to be removed, it is no longer required"); } From 044120ffd2e62fbc6188a719dc3f900cba6810bf Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 13 Jul 2017 00:07:51 +1000 Subject: [PATCH 04/11] Gets inherited permissions working, adds lots of unit tests, few more tests to write now. --- .../Models/Membership/ContentPermissionSet.cs | 54 ++++ .../Models/Membership/EntityPermission.cs | 11 +- .../Models/Membership/EntityPermissionSet.cs | 76 +++--- .../Membership/UserGroupEntityPermission.cs | 16 -- .../ContentBlueprintRepository.cs | 31 +++ .../Repositories/ContentRepository.cs | 75 ++---- .../Interfaces/IContentRepository.cs | 17 +- .../Interfaces/IUserGroupRepository.cs | 10 +- .../Repositories/PermissionRepository.cs | 254 ++++++++++++------ .../Repositories/SimpleGetRepository.cs | 10 +- .../Repositories/UserGroupRepository.cs | 39 ++- src/Umbraco.Core/Services/ContentService.cs | 18 +- .../Services/ContentServiceExtensions.cs | 2 +- src/Umbraco.Core/Services/IContentService.cs | 4 +- src/Umbraco.Core/Services/IUserService.cs | 10 +- src/Umbraco.Core/Services/UserService.cs | 196 +++++++------- src/Umbraco.Core/StringExtensions.cs | 16 ++ src/Umbraco.Core/Umbraco.Core.csproj | 3 +- .../Repositories/ContentRepositoryTest.cs | 43 +-- .../Services/ContentServiceTests.cs | 141 ++-------- .../Services/UserServiceTests.cs | 130 ++++++++- .../Controllers/ContentControllerUnitTests.cs | 41 +-- ...terAllowedOutgoingContentAttributeTests.cs | 8 +- .../Cache/CacheRefresherEventHandler.cs | 7 +- src/Umbraco.Web/Editors/ContentController.cs | 12 +- src/umbraco.businesslogic/User.cs | 2 +- 26 files changed, 725 insertions(+), 501 deletions(-) create mode 100644 src/Umbraco.Core/Models/Membership/ContentPermissionSet.cs delete mode 100644 src/Umbraco.Core/Models/Membership/UserGroupEntityPermission.cs create mode 100644 src/Umbraco.Core/Persistence/Repositories/ContentBlueprintRepository.cs diff --git a/src/Umbraco.Core/Models/Membership/ContentPermissionSet.cs b/src/Umbraco.Core/Models/Membership/ContentPermissionSet.cs new file mode 100644 index 0000000000..1345be2c8c --- /dev/null +++ b/src/Umbraco.Core/Models/Membership/ContentPermissionSet.cs @@ -0,0 +1,54 @@ +using System; +using System.Collections.Generic; +using Umbraco.Core.Models.EntityBase; + +namespace Umbraco.Core.Models.Membership +{ + /// + /// Represents an -> user group & permission key value pair collection + /// + /// + /// This implements purely so it can be used with the repository layer which is why it's explicitly implemented. + /// + public class ContentPermissionSet : EntityPermissionSet, IAggregateRoot + { + private readonly IContent _content; + + public ContentPermissionSet(IContent content, IEnumerable permissionsSet) + : base(content.Id, permissionsSet) + { + _content = content; + } + + public override int EntityId + { + get { return _content.Id; } + } + + #region Explicit implementation of IAggregateRoot + int IEntity.Id + { + get { return EntityId; } + set { throw new NotImplementedException(); } + } + + bool IEntity.HasIdentity + { + get { return EntityId > 0; } + } + + Guid IEntity.Key { get; set; } + + DateTime IEntity.CreateDate { get; set; } + + DateTime IEntity.UpdateDate { get; set; } + + DateTime? IDeletableEntity.DeletedDate { get; set; } + + object IDeepCloneable.DeepClone() + { + throw new NotImplementedException(); + } + #endregion + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Models/Membership/EntityPermission.cs b/src/Umbraco.Core/Models/Membership/EntityPermission.cs index a1fa57d274..c6d9e8350d 100644 --- a/src/Umbraco.Core/Models/Membership/EntityPermission.cs +++ b/src/Umbraco.Core/Models/Membership/EntityPermission.cs @@ -7,21 +7,24 @@ namespace Umbraco.Core.Models.Membership /// public class EntityPermission { - public EntityPermission(int entityId, string[] assignedPermissions) + public EntityPermission(int groupId, int entityId, string[] assignedPermissions) { + UserGroupId = groupId; EntityId = entityId; AssignedPermissions = assignedPermissions; IsDefaultPermissions = false; } - public EntityPermission(int entityId, string[] assignedPermissions, bool isDefaultPermissions) + public EntityPermission(int groupId, int entityId, string[] assignedPermissions, bool isDefaultPermissions) { + UserGroupId = groupId; EntityId = entityId; AssignedPermissions = assignedPermissions; IsDefaultPermissions = isDefaultPermissions; } public int EntityId { get; private set; } + public int UserGroupId { get; private set; } /// /// The assigned permissions for the user/entity combo @@ -50,7 +53,9 @@ namespace Umbraco.Core.Models.Membership AssignedPermissions = newPermissions .Distinct() .ToArray(); - } + } + + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/Membership/EntityPermissionSet.cs b/src/Umbraco.Core/Models/Membership/EntityPermissionSet.cs index d91cd3a352..dd9190ae85 100644 --- a/src/Umbraco.Core/Models/Membership/EntityPermissionSet.cs +++ b/src/Umbraco.Core/Models/Membership/EntityPermissionSet.cs @@ -1,60 +1,56 @@ using System.Collections.Generic; +using System.Linq; namespace Umbraco.Core.Models.Membership { /// /// Represents an entity -> user group & permission key value pair collection - /// + /// public class EntityPermissionSet { /// - /// The entity id with permissions assigned + /// Returns an empty permission set /// - public int EntityId { get; private set; } + /// + public static EntityPermissionSet Empty() + { + return new EntityPermissionSet(-1, new EntityPermission[0]); + } - /// - /// The key/value pairs of user group id & single permission - /// - public IEnumerable PermissionsSet { get; private set; } - - public EntityPermissionSet(int entityId, IEnumerable permissionsSet) + public EntityPermissionSet(int entityId, IEnumerable permissionsSet) { EntityId = entityId; PermissionsSet = permissionsSet; } - public class UserGroupPermission + /// + /// The entity id with permissions assigned + /// + public virtual int EntityId { get; private set; } + + /// + /// The key/value pairs of user group id & single permission + /// + public IEnumerable PermissionsSet { get; private set; } + + /// + /// Returns the aggregte permissions in the permission set + /// + /// + /// + /// This value is only calculated once + /// + public IEnumerable GetAllPermissions() { - public UserGroupPermission(int groupId, string permission) - { - UserGroupId = groupId; - Permission = permission; - } - - public int UserGroupId { get; private set; } - - public string Permission { get; private set; } - - protected bool Equals(UserGroupPermission other) - { - return UserGroupId == other.UserGroupId && string.Equals(Permission, other.Permission); - } - - public override bool Equals(object obj) - { - if (ReferenceEquals(null, obj)) return false; - if (ReferenceEquals(this, obj)) return true; - if (obj.GetType() != this.GetType()) return false; - return Equals((UserGroupPermission) obj); - } - - public override int GetHashCode() - { - unchecked - { - return (UserGroupId * 397) ^ Permission.GetHashCode(); - } - } + return _calculatedPermissions ?? (_calculatedPermissions = + PermissionsSet.SelectMany(x => x.AssignedPermissions).Distinct().ToArray()); } + + private string[] _calculatedPermissions; + + + + + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/Membership/UserGroupEntityPermission.cs b/src/Umbraco.Core/Models/Membership/UserGroupEntityPermission.cs deleted file mode 100644 index 619223f01a..0000000000 --- a/src/Umbraco.Core/Models/Membership/UserGroupEntityPermission.cs +++ /dev/null @@ -1,16 +0,0 @@ -namespace Umbraco.Core.Models.Membership -{ - /// - /// Represents a user group -> entity permission - /// - public class UserGroupEntityPermission : EntityPermission - { - public UserGroupEntityPermission(int groupId, int entityId, string[] assignedPermissions) - : base(entityId, assignedPermissions) - { - UserGroupId = groupId; - } - - public int UserGroupId { get; private set; } - } -} \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentBlueprintRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentBlueprintRepository.cs new file mode 100644 index 0000000000..4c9021d20a --- /dev/null +++ b/src/Umbraco.Core/Persistence/Repositories/ContentBlueprintRepository.cs @@ -0,0 +1,31 @@ +using System; +using Umbraco.Core.Configuration.UmbracoSettings; +using Umbraco.Core.Logging; +using Umbraco.Core.Persistence.SqlSyntax; +using Umbraco.Core.Persistence.UnitOfWork; + +namespace Umbraco.Core.Persistence.Repositories +{ + /// + /// Override the base content repository so we can change the node object type + /// + /// + /// It would be nicer if we could separate most of this down into a smaller version of the ContentRepository class, however to do that + /// requires quite a lot of work since we'd need to re-organize the interhitance quite a lot or create a helper class to perform a lot of the underlying logic. + /// + /// TODO: Create a helper method to contain most of the underlying logic for the ContentRepository + /// + internal class ContentBlueprintRepository : ContentRepository + { + public ContentBlueprintRepository(IScopeUnitOfWork work, CacheHelper cacheHelper, ILogger logger, ISqlSyntaxProvider syntaxProvider, IContentTypeRepository contentTypeRepository, ITemplateRepository templateRepository, ITagRepository tagRepository, IContentSection contentSection) + : base(work, cacheHelper, logger, syntaxProvider, contentTypeRepository, templateRepository, tagRepository, contentSection) + { + } + + protected override Guid NodeObjectTypeId + { + get { return Constants.ObjectTypes.DocumentBlueprintGuid; } + } + + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index f07e9c8385..2c3f126c6e 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -20,28 +20,6 @@ using Umbraco.Core.Persistence.UnitOfWork; namespace Umbraco.Core.Persistence.Repositories { - /// - /// Override the base content repository so we can change the node object type - /// - /// - /// It would be nicer if we could separate most of this down into a smaller version of the ContentRepository class, however to do that - /// requires quite a lot of work since we'd need to re-organize the interhitance quite a lot or create a helper class to perform a lot of the underlying logic. - /// - /// TODO: Create a helper method to conain most of the underlying logic for the ContentRepository - /// - internal class ContentBlueprintRepository : ContentRepository - { - public ContentBlueprintRepository(IScopeUnitOfWork work, CacheHelper cacheHelper, ILogger logger, ISqlSyntaxProvider syntaxProvider, IContentTypeRepository contentTypeRepository, ITemplateRepository templateRepository, ITagRepository tagRepository, IContentSection contentSection) : base(work, cacheHelper, logger, syntaxProvider, contentTypeRepository, templateRepository, tagRepository, contentSection) - { - } - - protected override Guid NodeObjectTypeId - { - get { return Constants.ObjectTypes.DocumentBlueprintGuid; } - } - - } - /// /// Represents a repository for doing CRUD operations for /// @@ -50,9 +28,9 @@ namespace Umbraco.Core.Persistence.Repositories private readonly IContentTypeRepository _contentTypeRepository; private readonly ITemplateRepository _templateRepository; private readonly ITagRepository _tagRepository; - private readonly CacheHelper _cacheHelper; private readonly ContentPreviewRepository _contentPreviewRepository; private readonly ContentXmlRepository _contentXmlRepository; + private readonly PermissionRepository _permissionRepository; public ContentRepository(IScopeUnitOfWork work, CacheHelper cacheHelper, ILogger logger, ISqlSyntaxProvider syntaxProvider, IContentTypeRepository contentTypeRepository, ITemplateRepository templateRepository, ITagRepository tagRepository, IContentSection contentSection) : base(work, cacheHelper, logger, syntaxProvider, contentSection) @@ -63,10 +41,10 @@ namespace Umbraco.Core.Persistence.Repositories _contentTypeRepository = contentTypeRepository; _templateRepository = templateRepository; _tagRepository = tagRepository; - _cacheHelper = cacheHelper; _contentPreviewRepository = new ContentPreviewRepository(work, CacheHelper.NoCache, logger, syntaxProvider); _contentXmlRepository = new ContentXmlRepository(work, CacheHelper.NoCache, logger, syntaxProvider); - + _permissionRepository = new PermissionRepository(UnitOfWork, cacheHelper, Logger, SqlSyntax); + EnsureUniqueNaming = true; } @@ -468,26 +446,7 @@ namespace Umbraco.Core.Persistence.Repositories entity.Id = nodeDto.NodeId; //Set Id on entity to ensure an Id is set entity.Path = nodeDto.Path; entity.SortOrder = sortOrder; - entity.Level = level; - - ////Assign the same permissions to it as the parent node - //// http://issues.umbraco.org/issue/U4-2161 - //var permissionsRepo = new PermissionRepository(UnitOfWork, _cacheHelper, SqlSyntax); - //var parentPermissions = permissionsRepo.GetPermissionsForEntity(entity.ParentId).ToArray(); - ////if there are parent permissions then assign them, otherwise leave null and permissions will become the - //// user's default permissions. - //if (parentPermissions.Length > 0) - //{ - // var userGroupPermissions = ( - // from perm in parentPermissions - // from p in perm.AssignedPermissions - // select new EntityPermissionSet.UserGroupPermission(perm.UserGroupId, p)).ToList(); - - // permissionsRepo.ReplaceEntityPermissions(new EntityPermissionSet(entity.Id, userGroupPermissions)); - // //flag the entity's permissions changed flag so we can track those changes. - // //Currently only used for the cache refreshers to detect if we should refresh all user permissions cache. - // ((Content)entity).PermissionsChanged = true; - //} + entity.Level = level; //Create the Content specific data - cmsContent var contentDto = dto.ContentVersionDto.ContentDto; @@ -863,8 +822,7 @@ order by umbracoNode.{2}, umbracoNode.parentID, umbracoNode.sortOrder", public void ReplaceContentPermissions(EntityPermissionSet permissionSet) { - var repo = new PermissionRepository(UnitOfWork, _cacheHelper, SqlSyntax); - repo.ReplaceEntityPermissions(permissionSet); + _permissionRepository.ReplaceEntityPermissions(permissionSet); } public void ClearPublished(IContent content) @@ -880,21 +838,19 @@ order by umbracoNode.{2}, umbracoNode.parentID, umbracoNode.sortOrder", /// /// public void AssignEntityPermission(IContent entity, char permission, IEnumerable groupIds) - { - var repo = new PermissionRepository(UnitOfWork, _cacheHelper, SqlSyntax); - repo.AssignEntityPermission(entity, permission, groupIds); + { + _permissionRepository.AssignEntityPermission(entity, permission, groupIds); } /// - /// Returns permissions directly assigned to the content item for all user groups + /// Gets the explicit list of permissions for the content item /// /// /// - public IEnumerable GetPermissionsForEntity(int entityId) + public IEnumerable GetPermissionsForEntity(int entityId) { - var repo = new PermissionRepository(UnitOfWork, _cacheHelper, SqlSyntax); - return repo.GetPermissionsForEntity(entityId); - } + return _permissionRepository.GetPermissionsForEntity(entityId); + } /// /// Adds/updates content/published xml @@ -906,6 +862,15 @@ order by umbracoNode.{2}, umbracoNode.parentID, umbracoNode.sortOrder", _contentXmlRepository.AddOrUpdate(new ContentXmlEntity(content, xml)); } + /// + /// Used to add/update a permission for a content item + /// + /// + public void AddOrUpdatePermissions(ContentPermissionSet permission) + { + _permissionRepository.AddOrUpdate(permission); + } + /// /// Used to remove the content xml for a content item /// diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs index 41889de1c4..6f643b3e0b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs @@ -57,11 +57,18 @@ namespace Umbraco.Core.Persistence.Repositories void AssignEntityPermission(IContent entity, char permission, IEnumerable groupIds); /// - /// Gets the list of permissions for the content item + /// Gets the explicit list of permissions for the content item /// /// /// - IEnumerable GetPermissionsForEntity(int entityId); + IEnumerable GetPermissionsForEntity(int entityId); + + ///// + ///// Gets the implicit/inherited list of permissions for the content item + ///// + ///// + ///// + //IEnumerable GetPermissionsForPath(string path); /// /// Used to add/update published xml for the content item @@ -70,6 +77,12 @@ namespace Umbraco.Core.Persistence.Repositories /// void AddOrUpdateContentXml(IContent content, Func xml); + /// + /// Used to add/update a permission for a content item + /// + /// + void AddOrUpdatePermissions(ContentPermissionSet permission); + /// /// Used to remove the content xml for a content item /// diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs index 3ccfa89885..b018bd3c08 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs @@ -32,6 +32,14 @@ namespace Umbraco.Core.Persistence.Repositories /// Array of entity Ids, if empty will return permissions for the group for all entities IEnumerable GetPermissionsForEntities(int groupId, params int[] entityIds); + /// + /// Gets explicilt and default permissions (if requested) permissions for the group for specified entities + /// + /// The group + /// If true will include the group's default permissions if no permissions are explicitly assigned + /// Array of entity Ids, if empty will return permissions for the group for all entities + IEnumerable GetPermissionsForEntities(IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds); + /// /// Replaces the same permission set for a single group to any number of entities /// @@ -47,6 +55,6 @@ namespace Umbraco.Core.Persistence.Repositories /// Permissions as enumerable list of /// Specify the nodes to replace permissions for void AssignGroupPermission(int groupId, char permission, params int[] entityIds); - + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs b/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs index 5ae4efad45..4cad3be7e7 100644 --- a/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs @@ -12,6 +12,9 @@ using Umbraco.Core.Persistence.SqlSyntax; using Umbraco.Core.Persistence.UnitOfWork; using CacheKeys = Umbraco.Core.Cache.CacheKeys; using Umbraco.Core.Cache; +using Umbraco.Core.Logging; +using Umbraco.Core.Models; +using Umbraco.Core.Persistence.Querying; namespace Umbraco.Core.Persistence.Repositories { @@ -19,19 +22,18 @@ namespace Umbraco.Core.Persistence.Repositories /// A repository that exposes functionality to modify assigned permissions to a node /// /// - internal class PermissionRepository + /// + /// This repo implements the base class so that permissions can be queued to be persisted + /// like the normal repository pattern but the standard repository Get commands don't apply and will throw + /// + internal class PermissionRepository : PetaPocoRepositoryBase where TEntity : class, IAggregateRoot { - private readonly IScopeUnitOfWork _unitOfWork; - private readonly IRuntimeCacheProvider _runtimeCache; - private readonly ISqlSyntaxProvider _sqlSyntax; - internal PermissionRepository(IScopeUnitOfWork unitOfWork, CacheHelper cache, ISqlSyntaxProvider sqlSyntax) + public PermissionRepository(IScopeUnitOfWork work, CacheHelper cache, ILogger logger, ISqlSyntaxProvider sqlSyntax) + : base(work, cache, logger, sqlSyntax) { - _unitOfWork = unitOfWork; - //Make this repository use an isolated cache - _runtimeCache = cache.IsolatedRuntimeCache.GetOrCreateCache(); - _sqlSyntax = sqlSyntax; + } /// @@ -41,7 +43,6 @@ namespace Umbraco.Core.Persistence.Repositories /// /// public IEnumerable GetPermissionsForEntities(int groupId, params int[] entityIds) - { //var whereCriteria = GetPermissionsForEntitiesCriteria(groupId, entityIds); var result = new List(); //iterate in groups of 2000 since we don't want to exceed the max SQL param count @@ -51,9 +52,9 @@ namespace Umbraco.Core.Persistence.Repositories var sql = new Sql(); sql.Select("*") - .From(_sqlSyntax) - .Where(dto => dto.UserGroupId == groupId && ids.Contains(dto.NodeId), _sqlSyntax); - var permissions = _unitOfWork.Database.Fetch(sql); + .From(SqlSyntax) + .Where(dto => dto.UserGroupId == groupId && ids.Contains(dto.NodeId), SqlSyntax); + var permissions = UnitOfWork.Database.Fetch(sql); result.AddRange(ConvertToPermissionList(permissions)); } return result; @@ -73,62 +74,62 @@ namespace Umbraco.Core.Persistence.Repositories // sql.Select("*") // .From() // .Where(whereCriteria); - // var result = _unitOfWork.Database.Fetch(sql); + // var result = UnitOfWork.Database.Fetch(sql); // return ConvertToPermissionList(result); // }, // GetCacheTimeout(), // priority: GetCachePriority()); - } - - //private static string GetEntityIdKey(int[] entityIds) - //{ - // return string.Join(",", entityIds.Select(x => x.ToString(CultureInfo.InvariantCulture))); - //} - - //private static TimeSpan GetCacheTimeout() - //{ - // //Since this cache can be quite large (http://issues.umbraco.org/issue/U4-2161) we will only have this exist in cache for 20 minutes, - // // then it will refresh from the database. - // return new TimeSpan(0, 20, 0); - //} - - //private static CacheItemPriority GetCachePriority() - //{ - // //Since this cache can be quite large (http://issues.umbraco.org/issue/U4-2161) we will make this priority below average - // return CacheItemPriority.BelowNormal; - //} - - private static IEnumerable ConvertToPermissionList(IEnumerable result) + } + + //private static string GetEntityIdKey(int[] entityIds) + //{ + // return string.Join(",", entityIds.Select(x => x.ToString(CultureInfo.InvariantCulture))); + //} + + //private static TimeSpan GetCacheTimeout() + //{ + // //Since this cache can be quite large (http://issues.umbraco.org/issue/U4-2161) we will only have this exist in cache for 20 minutes, + // // then it will refresh from the database. + // return new TimeSpan(0, 20, 0); + //} + + //private static CacheItemPriority GetCachePriority() + //{ + // //Since this cache can be quite large (http://issues.umbraco.org/issue/U4-2161) we will make this priority below average + // return CacheItemPriority.BelowNormal; + //} + + /// + /// Returns permissions directly assigned to the content items for all user groups + /// + /// + /// + public IEnumerable GetPermissionsForEntities(int[] entityIds) { - var permissions = new List(); - var nodePermissions = result.GroupBy(x => x.NodeId); - foreach (var np in nodePermissions) - { - var userGroupPermissions = np.GroupBy(x => x.UserGroupId); - foreach (var permission in userGroupPermissions) - { - var perms = permission.Select(x => x.Permission).ToArray(); - permissions.Add(new UserGroupEntityPermission(permission.Key, permission.First().NodeId, perms)); - } - } + var sql = new Sql(); + sql.Select("*") + .From(SqlSyntax) + .Where(dto => entityIds.Contains(dto.NodeId), SqlSyntax) + .OrderBy(dto => dto.NodeId, SqlSyntax); - return permissions; - } + var result = UnitOfWork.Database.Fetch(sql); + return ConvertToPermissionList(result); + } /// /// Returns permissions directly assigned to the content item for all user groups /// /// /// - public IEnumerable GetPermissionsForEntity(int entityId) + public IEnumerable GetPermissionsForEntity(int entityId) { var sql = new Sql(); sql.Select("*") - .From() - .Where(dto => dto.NodeId == entityId) - .OrderBy(dto => dto.NodeId); + .From(SqlSyntax) + .Where(dto => dto.NodeId == entityId, SqlSyntax) + .OrderBy(dto => dto.NodeId, SqlSyntax); - var result = _unitOfWork.Database.Fetch(sql); + var result = UnitOfWork.Database.Fetch(sql); return ConvertToPermissionList(result); } @@ -143,7 +144,7 @@ namespace Umbraco.Core.Persistence.Repositories /// public void ReplacePermissions(int groupId, IEnumerable permissions, params int[] entityIds) { - var db = _unitOfWork.Database; + var db = UnitOfWork.Database; //we need to batch these in groups of 2000 so we don't exceed the max 2100 limit var sql = "DELETE FROM umbracoUserGroup2NodePermission WHERE userGroupId = @groupId AND nodeId in (@nodeIds)"; @@ -166,10 +167,10 @@ namespace Umbraco.Core.Persistence.Repositories } } - _unitOfWork.Database.BulkInsertRecords(toInsert, _sqlSyntax); + UnitOfWork.Database.BulkInsertRecords(toInsert, SqlSyntax); //Raise the event - _unitOfWork.Events.Dispatch(AssignedPermissions, this, new SaveEventArgs(ConvertToPermissionList(toInsert), false)); + UnitOfWork.Events.Dispatch(AssignedPermissions, this, new SaveEventArgs(ConvertToPermissionList(toInsert), false)); } @@ -181,7 +182,7 @@ namespace Umbraco.Core.Persistence.Repositories /// public void AssignPermission(int groupId, char permission, params int[] entityIds) { - var db = _unitOfWork.Database; + var db = UnitOfWork.Database; var sql = "DELETE FROM umbracoUserGroup2NodePermission WHERE userGroupId = @groupId AND permission=@permission AND nodeId in (@entityIds)"; db.Execute(sql, new @@ -198,10 +199,10 @@ namespace Umbraco.Core.Persistence.Repositories UserGroupId = groupId }).ToArray(); - _unitOfWork.Database.BulkInsertRecords(actions, _sqlSyntax); + UnitOfWork.Database.BulkInsertRecords(actions, SqlSyntax); //Raise the event - _unitOfWork.Events.Dispatch(AssignedPermissions, this, new SaveEventArgs(ConvertToPermissionList(actions), false)); + UnitOfWork.Events.Dispatch(AssignedPermissions, this, new SaveEventArgs(ConvertToPermissionList(actions), false)); } @@ -213,7 +214,7 @@ namespace Umbraco.Core.Persistence.Repositories /// public void AssignEntityPermission(TEntity entity, char permission, IEnumerable groupIds) { - var db = _unitOfWork.Database; + var db = UnitOfWork.Database; var sql = "DELETE FROM umbracoUserGroup2NodePermission WHERE nodeId = @nodeId AND permission = @permission AND userGroupId in (@groupIds)"; db.Execute(sql, new @@ -230,15 +231,15 @@ namespace Umbraco.Core.Persistence.Repositories UserGroupId = id }).ToArray(); - _unitOfWork.Database.BulkInsertRecords(actions, _sqlSyntax); + UnitOfWork.Database.BulkInsertRecords(actions, SqlSyntax); //Raise the event - _unitOfWork.Events.Dispatch(AssignedPermissions, this, new SaveEventArgs(ConvertToPermissionList(actions), false)); + UnitOfWork.Events.Dispatch(AssignedPermissions, this, new SaveEventArgs(ConvertToPermissionList(actions), false)); } /// - /// Assigns permissions to an entity for multiple users/permission entries + /// Assigns permissions to an entity for multiple group/permission entries /// /// /// @@ -247,25 +248,118 @@ namespace Umbraco.Core.Persistence.Repositories /// public void ReplaceEntityPermissions(EntityPermissionSet permissionSet) { - var db = _unitOfWork.Database; - var sql = "DELETE FROM umbracoUserGroup2NodePermission WHERE nodeId = @nodeId"; - db.Execute(sql, new { nodeId = permissionSet.EntityId }); - - var actions = permissionSet.PermissionsSet.Select(p => new UserGroup2NodePermissionDto - { - NodeId = permissionSet.EntityId, - Permission = p.Permission, - UserGroupId = p.UserGroupId - }).ToArray(); + var db = UnitOfWork.Database; + var sql = "DELETE FROM umbracoUserGroup2NodePermission WHERE nodeId = @nodeId"; + db.Execute(sql, new { nodeId = permissionSet.EntityId }); + + var toInsert = new List(); + foreach (var entityPermission in permissionSet.PermissionsSet) + { + foreach (var permission in entityPermission.AssignedPermissions) + { + toInsert.Add(new UserGroup2NodePermissionDto + { + NodeId = permissionSet.EntityId, + Permission = permission, + UserGroupId = entityPermission.UserGroupId + }); + } + } + + UnitOfWork.Database.BulkInsertRecords(toInsert, SqlSyntax); + + //Raise the event + UnitOfWork.Events.Dispatch(AssignedPermissions, this, new SaveEventArgs(permissionSet.PermissionsSet, false)); + - _unitOfWork.Database.BulkInsertRecords(actions, _sqlSyntax); - - //Raise the event - _unitOfWork.Events.Dispatch(AssignedPermissions, this, new SaveEventArgs(ConvertToPermissionList(actions), false)); - - } - public static event TypedEventHandler, SaveEventArgs> AssignedPermissions; + + #region Not implemented (don't need to for the purposes of this repo) + protected override ContentPermissionSet PerformGet(int id) + { + throw new NotImplementedException(); + } + + protected override IEnumerable PerformGetAll(params int[] ids) + { + throw new NotImplementedException(); + } + + protected override IEnumerable PerformGetByQuery(IQuery query) + { + throw new NotImplementedException(); + } + + protected override Sql GetBaseQuery(bool isCount) + { + throw new NotImplementedException(); + } + + protected override string GetBaseWhereClause() + { + throw new NotImplementedException(); + } + + protected override IEnumerable GetDeleteClauses() + { + return new List(); + } + + protected override Guid NodeObjectTypeId + { + get { throw new NotImplementedException(); } + } + + protected override void PersistDeletedItem(ContentPermissionSet entity) + { + throw new NotImplementedException(); + } + + #endregion + + /// + /// Used to add or update entity permissions during a content item being updated + /// + /// + protected override void PersistNewItem(ContentPermissionSet entity) + { + //does the same thing as update + PersistUpdatedItem(entity); + } + + /// + /// Used to add or update entity permissions during a content item being updated + /// + /// + protected override void PersistUpdatedItem(ContentPermissionSet entity) + { + var asAggregateRoot = (IAggregateRoot)entity; + if (asAggregateRoot.HasIdentity == false) + { + throw new InvalidOperationException("Cannot create permissions for an entity without an Id"); + } + + ReplaceEntityPermissions(entity); + } + + private static IEnumerable ConvertToPermissionList(IEnumerable result) + { + var permissions = new List(); + var nodePermissions = result.GroupBy(x => x.NodeId); + foreach (var np in nodePermissions) + { + var userGroupPermissions = np.GroupBy(x => x.UserGroupId); + foreach (var permission in userGroupPermissions) + { + var perms = permission.Select(x => x.Permission).ToArray(); + permissions.Add(new EntityPermission(permission.Key, np.Key, perms)); + } + } + + return permissions; + } + + public static event TypedEventHandler, SaveEventArgs> AssignedPermissions; } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/SimpleGetRepository.cs b/src/Umbraco.Core/Persistence/Repositories/SimpleGetRepository.cs index 67c9149422..28e676743f 100644 --- a/src/Umbraco.Core/Persistence/Repositories/SimpleGetRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/SimpleGetRepository.cs @@ -66,7 +66,7 @@ namespace Umbraco.Core.Persistence.Repositories return Database.Fetch(sql).Select(ConvertToEntity); } - protected override sealed IEnumerable PerformGetByQuery(IQuery query) + protected sealed override IEnumerable PerformGetByQuery(IQuery query) { var sqlClause = GetBaseQuery(false); var translator = new SqlTranslator(sqlClause, query); @@ -76,22 +76,22 @@ namespace Umbraco.Core.Persistence.Repositories #region Not implemented and not required - protected override sealed IEnumerable GetDeleteClauses() + protected sealed override IEnumerable GetDeleteClauses() { throw new NotImplementedException(); } - protected override sealed Guid NodeObjectTypeId + protected sealed override Guid NodeObjectTypeId { get { throw new NotImplementedException(); } } - protected override sealed void PersistNewItem(TEntity entity) + protected sealed override void PersistNewItem(TEntity entity) { throw new NotImplementedException(); } - protected override sealed void PersistUpdatedItem(TEntity entity) + protected sealed override void PersistUpdatedItem(TEntity entity) { throw new NotImplementedException(); } diff --git a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs index e0717deda4..bd2b8390cd 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs @@ -22,12 +22,14 @@ namespace Umbraco.Core.Persistence.Repositories { private readonly CacheHelper _cacheHelper; private readonly UserGroupWithUsersRepository _userGroupWithUsersRepository; + private readonly PermissionRepository _permissionRepository; public UserGroupRepository(IScopeUnitOfWork work, CacheHelper cacheHelper, ILogger logger, ISqlSyntaxProvider sqlSyntax) : base(work, cacheHelper, logger, sqlSyntax) { _cacheHelper = cacheHelper; _userGroupWithUsersRepository = new UserGroupWithUsersRepository(this, work, cacheHelper, logger, sqlSyntax); + _permissionRepository = new PermissionRepository(work, _cacheHelper, logger, sqlSyntax); } public const string GetByAliasCacheKeyPrefix = "UserGroupRepository_GetByAlias_"; @@ -97,8 +99,33 @@ namespace Umbraco.Core.Persistence.Repositories /// Array of entity Ids, if empty will return permissions for the group for all entities public IEnumerable GetPermissionsForEntities(int groupId, params int[] entityIds) { - var repo = new PermissionRepository(UnitOfWork, _cacheHelper, SqlSyntax); - return repo.GetPermissionsForEntities(groupId, entityIds); + return _permissionRepository.GetPermissionsForEntities(groupId, entityIds); + } + + /// + /// Gets explicilt and default permissions (if requested) permissions for the group for specified entities + /// + /// The group + /// If true will include the group's default permissions if no permissions are explicitly assigned + /// Array of entity Ids, if empty will return permissions for the group for all entities + public IEnumerable GetPermissionsForEntities(IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) + { + if (group == null) throw new ArgumentNullException("group"); + + var explicitPermissions = GetPermissionsForEntities(group.Id, nodeIds); + var result = new List(explicitPermissions); + + // If requested, and no permissions are assigned to a particular node, then we will fill in those permissions with the group's defaults + if (fallbackToDefaultPermissions) + { + var missingIds = nodeIds.Except(result.Select(x => x.EntityId)).ToArray(); + if (missingIds.Length > 0) + { + result.AddRange(missingIds + .Select(i => new EntityPermission(group.Id, i, group.Permissions.ToArray(), isDefaultPermissions: true))); + } + } + return result; } /// @@ -109,8 +136,7 @@ namespace Umbraco.Core.Persistence.Repositories /// Specify the nodes to replace permissions for. If nothing is specified all permissions are removed. public void ReplaceGroupPermissions(int groupId, IEnumerable permissions, params int[] entityIds) { - var repo = new PermissionRepository(UnitOfWork, _cacheHelper, SqlSyntax); - repo.ReplacePermissions(groupId, permissions, entityIds); + _permissionRepository.ReplacePermissions(groupId, permissions, entityIds); } /// @@ -121,9 +147,8 @@ namespace Umbraco.Core.Persistence.Repositories /// Specify the nodes to replace permissions for public void AssignGroupPermission(int groupId, char permission, params int[] entityIds) { - var repo = new PermissionRepository(UnitOfWork, _cacheHelper, SqlSyntax); - repo.AssignPermission(groupId, permission, entityIds); - } + _permissionRepository.AssignPermission(groupId, permission, entityIds); + } #region Overrides of RepositoryBase diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 6e09d5f845..b7649ea382 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -130,11 +130,11 @@ namespace Umbraco.Core.Services } /// - /// Returns permissions directly assigned to the content item for all user groups + /// Returns implicit/inherited permissions assigned to the content item for all user groups /// /// /// - public IEnumerable GetPermissionsForEntity(IContent content) + public IEnumerable GetPermissionsForEntity(IContent content) { using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { @@ -1671,9 +1671,21 @@ namespace Umbraco.Core.Services // Update the create author and last edit author copy.CreatorId = userId; copy.WriterId = userId; + + //get the current permissions, if there are any explicit ones they need to be copied + var currentPermissions = GetPermissionsForEntity(content) + .Where(x => x.IsDefaultPermissions == false).ToArray(); repository.AddOrUpdate(copy); - repository.AddOrUpdatePreviewXml(copy, c => _entitySerializer.Serialize(this, _dataTypeService, _userService, c)); + repository.AddOrUpdatePreviewXml(copy, c => _entitySerializer.Serialize(this, _dataTypeService, _userService, c)); + + //add permissions + if (currentPermissions.Length > 0) + { + var permissionSet = new ContentPermissionSet(copy, currentPermissions); + repository.AddOrUpdatePermissions(permissionSet); + } + uow.Commit(); // todo - this should flush, not commit //Special case for the associated tags diff --git a/src/Umbraco.Core/Services/ContentServiceExtensions.cs b/src/Umbraco.Core/Services/ContentServiceExtensions.cs index 95fe79f9ac..c6dc0274b6 100644 --- a/src/Umbraco.Core/Services/ContentServiceExtensions.cs +++ b/src/Umbraco.Core/Services/ContentServiceExtensions.cs @@ -17,7 +17,7 @@ namespace Umbraco.Core.Services /// public static void RemoveContentPermissions(this IContentService contentService, int contentId) { - contentService.ReplaceContentPermissions(new EntityPermissionSet(contentId, Enumerable.Empty())); + contentService.ReplaceContentPermissions(new EntityPermissionSet(contentId, Enumerable.Empty())); } /// diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index 289af7324b..251bf169ad 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -151,11 +151,11 @@ namespace Umbraco.Core.Services void AssignContentPermission(IContent entity, char permission, IEnumerable groupIds); /// - /// Returns permissions directly assigned to the content item for all user groups + /// Returns implicit/inherited permissions assigned to the content item for all user groups /// /// /// - IEnumerable GetPermissionsForEntity(IContent content); + IEnumerable GetPermissionsForEntity(IContent content); bool SendToPublication(IContent content, int userId = 0); diff --git a/src/Umbraco.Core/Services/IUserService.cs b/src/Umbraco.Core/Services/IUserService.cs index de228412da..c5b282cd66 100644 --- a/src/Umbraco.Core/Services/IUserService.cs +++ b/src/Umbraco.Core/Services/IUserService.cs @@ -102,8 +102,7 @@ namespace Umbraco.Core.Services /// /// Group to retrieve permissions for /// - /// Flag indicating if we want to get just the permissions directly assigned for the group and path, - /// or fall back to the group's default permissions when nothing is directly assigned + /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set /// /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of @@ -114,16 +113,15 @@ namespace Umbraco.Core.Services /// /// User to check permissions for /// Path to check permissions for - EntityPermissionSet GetPermissionsForPath(IUser user, string path); - + EntityPermissionSet GetPermissionsForPath(IUser user, string path); + /// /// Gets the permissions for the provided group and path /// /// Group to check permissions for /// Path to check permissions for /// - /// Flag indicating if we want to get just the permissions directly assigned for the group and path, - /// or fall back to the group's default permissions when nothing is directly assigned + /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set /// EntityPermission GetPermissionsForPath(IUserGroup group, string path, bool fallbackToDefaultPermissions = false); diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index 841a51456e..52e3e82106 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -865,21 +865,18 @@ namespace Umbraco.Core.Services /// /// Group to retrieve permissions for /// - /// Flag indicating if we want to get just the permissions directly assigned for the group and path, - /// or fall back to the group's default permissions when nothing is directly assigned + /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set /// /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of private IEnumerable GetPermissions(IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) { - if (group == null) throw new ArgumentNullException("group"); - if (nodeIds.Length == 0) - return Enumerable.Empty(); - + if (group == null) throw new ArgumentNullException("group"); + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateUserGroupRepository(uow); - return GetPermissionsInternal(repository, group, fallbackToDefaultPermissions, nodeIds); + return repository.GetPermissionsForEntities(group, fallbackToDefaultPermissions, nodeIds); } } @@ -888,8 +885,7 @@ namespace Umbraco.Core.Services /// /// Group to retrieve permissions for /// - /// Flag indicating if we want to get just the permissions directly assigned for the group and path, - /// or fall back to the group's default permissions when nothing is directly assigned + /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set /// /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of @@ -899,28 +895,8 @@ namespace Umbraco.Core.Services using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateUserGroupRepository(uow); - return GetPermissionsInternal(repository, group.ToReadOnlyGroup(), fallbackToDefaultPermissions, nodeIds); + return repository.GetPermissionsForEntities(group.ToReadOnlyGroup(), fallbackToDefaultPermissions, nodeIds); } - } - - private static IEnumerable GetPermissionsInternal(IUserGroupRepository repository, IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) - { - if (group == null) throw new ArgumentNullException("group"); - - var explicitPermissions = repository.GetPermissionsForEntities(group.Id, nodeIds); - var result = new List(explicitPermissions); - - // If requested, and no permissions are assigned to a particular node, then we will fill in those permissions with the group's defaults - if (fallbackToDefaultPermissions) - { - var missingIds = nodeIds.Except(result.Select(x => x.EntityId)).ToArray(); - if (missingIds.Length > 0) - { - result.AddRange(missingIds - .Select(i => new EntityPermission(i, group.Permissions.ToArray(), isDefaultPermissions: true))); - } - } - return result; } /// @@ -952,49 +928,18 @@ namespace Umbraco.Core.Services /// Path to check permissions for public EntityPermissionSet GetPermissionsForPath(IUser user, string path) { - var nodeIds = GetIdsFromPath(path); + var nodeIds = path.GetIdsFromPathReversed(); if (nodeIds.Length == 0) return EntityPermissionSet.Empty(); - var permissionsByGroup = GetPermissionsForGroupsAndPath(user.Groups, nodeIds); - - // not sure this will ever happen, it shouldn't since this should return defaults, but maybe those are empty? - if (permissionsByGroup.Count == 0) - return EntityPermissionSet.Empty(); - - var entityId = nodeIds[0]; - - var groupPermissions = new List(); - foreach (var entityPermission in permissionsByGroup) - { - var groupId = entityPermission.Key; - foreach (var assignedPermission in entityPermission.Value.AssignedPermissions) - { - groupPermissions.Add(new EntityPermissionSet.UserGroupPermission(groupId, assignedPermission)); - } - } - - var permissionSet = new EntityPermissionSet(entityId, groupPermissions); - return permissionSet; - } - - /// - /// Retrieves the permissions assigned to each group for a given path - /// - /// List of groups associated with the user - /// Path to check permissions for - /// A dictionary of group ids and their associated node permissions - private IDictionary GetPermissionsForGroupsAndPath(IEnumerable groups, int[] pathIds) - { - return groups - .Select(g => new - { - group = g.Id, - permissions = GetPermissionsForPath(g, pathIds, fallbackToDefaultPermissions: true) - }) - .ToDictionary(x => x.group, x => x.permissions); - } + //collect all permissions structures for all nodes for all groups belonging to the user + var groupPermissions = user.Groups + .Select(g => GetPermissionsForPath(g, nodeIds, fallbackToDefaultPermissions: true)) + .ToArray(); + + return CalculatePermissionsForPathForUser(groupPermissions, nodeIds); + } /// /// Gets the permissions for the provided group and path @@ -1002,13 +947,12 @@ namespace Umbraco.Core.Services /// Group to check permissions for /// Path to check permissions for /// - /// Flag indicating if we want to get just the permissions directly assigned for the group and path, - /// or fall back to the group's default permissions when nothing is directly assigned + /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set /// /// String indicating permissions for provided user and path public EntityPermission GetPermissionsForPath(IUserGroup group, string path, bool fallbackToDefaultPermissions = false) { - var nodeIds = GetIdsFromPath(path); + var nodeIds = path.GetIdsFromPathReversed(); return GetPermissionsForPath(group.ToReadOnlyGroup(), nodeIds, fallbackToDefaultPermissions); } @@ -1016,17 +960,91 @@ namespace Umbraco.Core.Services { //get permissions for all nodes in the path var permissions = GetPermissions(group, fallbackToDefaultPermissions, pathIds); - return GetPermissionsForPath(permissions, pathIds, fallbackToDefaultPermissions); - } - + return GetPermissionsForPathForGroup(permissions, pathIds, fallbackToDefaultPermissions); + } + /// - /// Returns the permissions for the path ids + /// This performs the calculations for inherited nodes based on this http://issues.umbraco.org/issue/U4-10075#comment=67-40085 /// - /// - /// Must be ordered deepest to shallowest (right to left) - /// + /// + /// /// - private static EntityPermission GetPermissionsForPath( + internal static EntityPermissionSet CalculatePermissionsForPathForUser( + EntityPermission[] groupPermissions, + int[] pathIds) + { + // not sure this will ever happen, it shouldn't since this should return defaults, but maybe those are empty? + if (groupPermissions.Length == 0 || pathIds.Length == 0) + return EntityPermissionSet.Empty(); + + //The actual entity id being looked at (deepest part of the path) + var entityId = pathIds[0]; + + var resultPermissions = new List(); + + //create a grouped by dictionary of another grouped by dictionary + var permissionsByGroup = groupPermissions + .GroupBy(x => x.UserGroupId) + .ToDictionary( + x => x.Key, + x => x.GroupBy(a => a.EntityId).ToDictionary(a => a.Key, a => a.ToArray())); + + //iterate through each group + foreach (var byGroup in permissionsByGroup) + { + var added = false; + + //iterate deepest to shallowest + foreach (var pathId in pathIds) + { + EntityPermission[] permissionsForNodeAndGroup; + if (byGroup.Value.TryGetValue(pathId, out permissionsForNodeAndGroup) == false) + continue; + + //In theory there will only be one EntityPermission in this group + // but there's nothing stopping the logic of this method + // from having more so we deal with it here + foreach (var entityPermission in permissionsForNodeAndGroup) + { + if (entityPermission.IsDefaultPermissions == false) + { + //explicit permision found so we'll append it and move on, of course if there was two explicit permissions + //found for this group the ones after this one wouldn't matter but considering there should only be one per + //group anyways, that is fine. + resultPermissions.Add(entityPermission); + added = true; + break; + } + } + + //if the permission has been added for this group and this branch then we can exit this loop + if (added) + break; + } + + if (added == false && byGroup.Value.Count > 0) + { + //if there was no explicit permissions assigned in this branch for this group, then we will + //add the group's default permissions + resultPermissions.Add(byGroup.Value[entityId][0]); + } + + } + + var permissionSet = new EntityPermissionSet(entityId, resultPermissions); + return permissionSet; + } + + /// + /// Returns the resulting permission set for a group for the path based on all permissions provided for the branch + /// + /// The collective set of permissions provided to calculate the resulting permissions set for the path + /// Must be ordered deepest to shallowest (right to left) + /// + /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set + /// + /// + internal static EntityPermission GetPermissionsForPathForGroup( IEnumerable pathPermissions, int[] pathIds, bool fallbackToDefaultPermissions = false) @@ -1051,24 +1069,8 @@ namespace Umbraco.Core.Services return null; return permissionsByEntityId[pathIds[0]]; - } - - /// - /// Convert a path to node ids in the order from right to left (deepest to shallowest) - /// - /// - /// - private int[] GetIdsFromPath(string path) - { - var nodeIds = path.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries) - .Select(x => x.TryConvertTo()) - .Where(x => x.Success) - .Select(x => x.Result) - .Reverse() - .ToArray(); - return nodeIds; } - + /// /// Checks in a set of permissions associated with a user for those related to a given nodeId /// diff --git a/src/Umbraco.Core/StringExtensions.cs b/src/Umbraco.Core/StringExtensions.cs index dba7f2cfee..0780fe799c 100644 --- a/src/Umbraco.Core/StringExtensions.cs +++ b/src/Umbraco.Core/StringExtensions.cs @@ -41,6 +41,22 @@ namespace Umbraco.Core ToCSharpEscapeChars[escape[0]] = escape[1]; } + /// + /// Convert a path to node ids in the order from right to left (deepest to shallowest) + /// + /// + /// + internal static int[] GetIdsFromPathReversed(this string path) + { + var nodeIds = path.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries) + .Select(x => x.TryConvertTo()) + .Where(x => x.Success) + .Select(x => x.Result) + .Reverse() + .ToArray(); + return nodeIds; + } + /// /// Removes new lines and tabs /// diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 8a635b966c..7269dcd9f6 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -345,6 +345,7 @@ + @@ -456,7 +457,6 @@ - @@ -588,6 +588,7 @@ + diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs index 2b9e0b1ed3..f969488055 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs @@ -533,48 +533,7 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.AreEqual(dateValue, persistedTextpage.GetValue(dateTimePropertyAlias)); Assert.AreEqual(persistedTextpage.GetValue(dateTimePropertyAlias), textpage.GetValue(dateTimePropertyAlias)); } - } - - [Test] - public void Ensures_Permissions_Are_Set_If_Parent_Entity_Permissions_Exist() - { - // Arrange - var provider = new PetaPocoUnitOfWorkProvider(Logger); - var unitOfWork = provider.GetUnitOfWork(); - - using (var repository = CreateUserGroupRepository(unitOfWork)) - { - var userGroup = MockedUserGroup.CreateUserGroup("1"); - repository.AddOrUpdate(userGroup); - unitOfWork.Commit(); - } - - ContentTypeRepository contentTypeRepository; - using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) - { - var contentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage"); - contentType.AllowedContentTypes = new List - { - new ContentTypeSort(new Lazy(() => contentType.Id), 0, contentType.Alias) - }; - var parentPage = MockedContent.CreateSimpleContent(contentType); - contentTypeRepository.AddOrUpdate(contentType); - repository.AddOrUpdate(parentPage); - unitOfWork.Commit(); - - // Act - repository.AssignEntityPermission(parentPage, 'A', new[] { 1 }); - var childPage = MockedContent.CreateSimpleContent(contentType, "child", parentPage); - repository.AddOrUpdate(childPage); - unitOfWork.Commit(); - - // Assert - var permissions = repository.GetPermissionsForEntity(childPage.Id); - Assert.AreEqual(1, permissions.Count()); - Assert.AreEqual("A", permissions.Single().AssignedPermissions.First()); - } - - } + } [Test] public void Can_Perform_Add_On_ContentRepository() diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 93202af102..43f049adf4 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -1459,7 +1459,7 @@ namespace Umbraco.Tests.Services } [Test] - public void Ensures_Permissions_Are_Set_On_Copied_Entity_To_Parent_Without_Permissions() + public void Ensures_Permissions_Are_Retained_For_Copied_Descendants_With_Explicit_Permissions() { // Arrange var userGroup = MockedUserGroup.CreateUserGroup("1"); @@ -1472,27 +1472,28 @@ namespace Umbraco.Tests.Services }; ServiceContext.ContentTypeService.Save(contentType); - var parentPage = MockedContent.CreateSimpleContent(contentType); - ServiceContext.ContentService.Save(parentPage); - ServiceContext.ContentService.AssignContentPermission(parentPage, 'A', new[] { 1 }); + var parentPage = MockedContent.CreateSimpleContent(contentType); + ServiceContext.ContentService.Save(parentPage); var childPage = MockedContent.CreateSimpleContent(contentType, "child", parentPage); - ServiceContext.ContentService.Save(childPage); - - //Ok, now copy, what should happen is the childPage will not have any permissions assigned since it's new parent - //doesn't have any assigned + ServiceContext.ContentService.Save(childPage); + //assign explicit permissions to the child + ServiceContext.ContentService.AssignContentPermission(childPage, 'A', new[] { userGroup.Id }); + + //Ok, now copy, what should happen is the childPage will retain it's own permissions var parentPage2 = MockedContent.CreateSimpleContent(contentType); ServiceContext.ContentService.Save(parentPage2); var copy = ServiceContext.ContentService.Copy(childPage, parentPage2.Id, false, true); - //Re-get the permissions, since there was none assigned to the new parent, the child shouldn't have any directly assigned - var copyPermissions = ServiceContext.ContentService.GetPermissionsForEntity(copy); - Assert.AreEqual(0, copyPermissions.Count()); + //get the permissions and verify + var permissions = ServiceContext.UserService.GetPermissionsForPath(userGroup, copy.Path, fallbackToDefaultPermissions: true); + Assert.AreEqual(1, permissions.AssignedPermissions.Length); + Assert.AreEqual("A", permissions.AssignedPermissions[0]); } [Test] - public void Ensures_Permissions_Are_Set_On_Copied_Entity_To_Parent_With_Permissions() + public void Ensures_Permissions_Are_Inherited_For_Copied_Descendants() { // Arrange var userGroup = MockedUserGroup.CreateUserGroup("1"); @@ -1507,88 +1508,8 @@ namespace Umbraco.Tests.Services var parentPage = MockedContent.CreateSimpleContent(contentType); ServiceContext.ContentService.Save(parentPage); - ServiceContext.ContentService.AssignContentPermission(parentPage, 'A', new[] { 1 }); + ServiceContext.ContentService.AssignContentPermission(parentPage, 'A', new[] { userGroup.Id }); - var childPage = MockedContent.CreateSimpleContent(contentType, "child", parentPage); - ServiceContext.ContentService.Save(childPage); - - //Ok, now copy, what should happen is the childPage will have it's new parent permissions copied over - var parentPage2 = MockedContent.CreateSimpleContent(contentType); - ServiceContext.ContentService.Save(parentPage2); - ServiceContext.ContentService.AssignContentPermission(parentPage2, 'B', new[] { 1 }); - - var copy = ServiceContext.ContentService.Copy(childPage, parentPage2.Id, false, true); - - //Re-get the permissions, since there was none assigned to the new parent, the child shouldn't have any directly assigned - var copyPermissions = ServiceContext.ContentService.GetPermissionsForEntity(copy); - Assert.AreEqual(1, copyPermissions.Count()); - Assert.AreEqual("B", copyPermissions.Single().AssignedPermissions.First()); - } - - [Test] - public void Ensures_Permissions_Are_Set_On_Copied_Descendants_To_Parent_With_Permissions() - { - // Arrange - var userGroup = MockedUserGroup.CreateUserGroup("1"); - ServiceContext.UserService.Save(userGroup); - - var contentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage"); - contentType.AllowedContentTypes = new List - { - new ContentTypeSort(new Lazy(() => contentType.Id), 0, contentType.Alias) - }; - ServiceContext.ContentTypeService.Save(contentType); - - var parentPage = MockedContent.CreateSimpleContent(contentType); - ServiceContext.ContentService.Save(parentPage); - ServiceContext.ContentService.AssignContentPermission(parentPage, 'A', new[] { 1 }); - - var childPage1 = MockedContent.CreateSimpleContent(contentType, "child1", parentPage); - ServiceContext.ContentService.Save(childPage1); - var childPage2 = MockedContent.CreateSimpleContent(contentType, "child2", childPage1); - ServiceContext.ContentService.Save(childPage2); - var childPage3 = MockedContent.CreateSimpleContent(contentType, "child3", childPage2); - ServiceContext.ContentService.Save(childPage3); - - //Ok, now copy, what should happen is the childPage will have it's new parent permissions copied over - var parentPage2 = MockedContent.CreateSimpleContent(contentType); - ServiceContext.ContentService.Save(parentPage2); - ServiceContext.ContentService.AssignContentPermission(parentPage2, 'B', new[] { 1 }); - - var copy = ServiceContext.ContentService.Copy(childPage1, parentPage2.Id, false, true); - - //Re-get the permissions, since there was none assigned to the new parent, the child shouldn't have any directly assigned - var copyPermissions = ServiceContext.ContentService.GetPermissionsForEntity(copy); - Assert.AreEqual(1, copyPermissions.Count()); - Assert.AreEqual("B", copyPermissions.Single().AssignedPermissions.First()); - - var descendants = ServiceContext.ContentService.GetDescendants(copy).ToArray(); - Assert.AreEqual(2, descendants.Length); - - foreach (var descendant in descendants) - { - var permissions = ServiceContext.ContentService.GetPermissionsForEntity(descendant); - Assert.AreEqual(1, permissions.Count()); - Assert.AreEqual("B", permissions.Single().AssignedPermissions.First()); - } - } - - [Test] - public void Ensures_Permissions_Are_Set_On_Descendants_When_Permissions_Added() - { - // Arrange - var userGroup = MockedUserGroup.CreateUserGroup("1"); - ServiceContext.UserService.Save(userGroup); - - var contentType = MockedContentTypes.CreateSimpleContentType("umbTextpage1", "Textpage"); - contentType.AllowedContentTypes = new List - { - new ContentTypeSort(new Lazy(() => contentType.Id), 0, contentType.Alias) - }; - ServiceContext.ContentTypeService.Save(contentType); - - var parentPage = MockedContent.CreateSimpleContent(contentType); - ServiceContext.ContentService.Save(parentPage); var childPage1 = MockedContent.CreateSimpleContent(contentType, "child1", parentPage); ServiceContext.ContentService.Save(childPage1); var childPage2 = MockedContent.CreateSimpleContent(contentType, "child2", childPage1); @@ -1596,37 +1517,33 @@ namespace Umbraco.Tests.Services var childPage3 = MockedContent.CreateSimpleContent(contentType, "child3", childPage2); ServiceContext.ContentService.Save(childPage3); - //ensure there are no permissions on any node - - var permissions = ServiceContext.ContentService.GetPermissionsForEntity(parentPage); - Assert.AreEqual(0, permissions.Count()); - + //Verify that the children have the inherited permissions var descendants = ServiceContext.ContentService.GetDescendants(parentPage).ToArray(); Assert.AreEqual(3, descendants.Length); foreach (var descendant in descendants) { - permissions = ServiceContext.ContentService.GetPermissionsForEntity(descendant); - Assert.AreEqual(0, permissions.Count()); + var permissions = ServiceContext.UserService.GetPermissionsForPath(userGroup, descendant.Path, fallbackToDefaultPermissions:true); + Assert.AreEqual(1, permissions.AssignedPermissions.Length); + Assert.AreEqual("A", permissions.AssignedPermissions[0]); } - - //Ok, now assign permissions to the parent, all descenents should get these right? - ServiceContext.ContentService.AssignContentPermission(parentPage, 'A', new[] { 1 }); - - //re-test - permissions = ServiceContext.ContentService.GetPermissionsForEntity(parentPage); - Assert.AreEqual(1, permissions.Count()); - Assert.AreEqual("A", permissions.Single().AssignedPermissions.First()); + //create a new parent with a new permission structure + var parentPage2 = MockedContent.CreateSimpleContent(contentType); + ServiceContext.ContentService.Save(parentPage2); + ServiceContext.ContentService.AssignContentPermission(parentPage2, 'B', new[] { userGroup.Id }); - descendants = ServiceContext.ContentService.GetDescendants(parentPage).ToArray(); + //Now copy, what should happen is the child pages will now have permissions inherited from the new parent + var copy = ServiceContext.ContentService.Copy(childPage1, parentPage2.Id, false, true); + + descendants = ServiceContext.ContentService.GetDescendants(parentPage2).ToArray(); Assert.AreEqual(3, descendants.Length); foreach (var descendant in descendants) { - permissions = ServiceContext.ContentService.GetPermissionsForEntity(descendant); - Assert.AreEqual(1, permissions.Count()); - Assert.AreEqual("A", permissions.Single().AssignedPermissions.First()); + var permissions = ServiceContext.UserService.GetPermissionsForPath(userGroup, descendant.Path, fallbackToDefaultPermissions: true); + Assert.AreEqual(1, permissions.AssignedPermissions.Length); + Assert.AreEqual("B", permissions.AssignedPermissions[0]); } } diff --git a/src/Umbraco.Tests/Services/UserServiceTests.cs b/src/Umbraco.Tests/Services/UserServiceTests.cs index d672229e6b..ac58f9f734 100644 --- a/src/Umbraco.Tests/Services/UserServiceTests.cs +++ b/src/Umbraco.Tests/Services/UserServiceTests.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using System.Security.Cryptography; using System.Text; @@ -10,6 +11,7 @@ using Umbraco.Core.Persistence.Querying; using Umbraco.Tests.TestHelpers; using Umbraco.Core; using Umbraco.Core.Persistence.DatabaseModelDefinitions; +using Umbraco.Core.Services; namespace Umbraco.Tests.Services { @@ -151,7 +153,7 @@ namespace Umbraco.Tests.Services ServiceContext.ContentService.AssignContentPermission(content.ElementAt(1), ActionDelete.Instance.Letter, new int[] { userGroup.Id }); // Act - var permissions = userService.GetPermissions(userGroup, true, content.ElementAt(0).Id, content.ElementAt(1).Id, content.ElementAt(2).Id) + var permissions = userService.GetPermissions(userGroup, true, content.ElementAt(0).Id, content.ElementAt(1).Id, content.ElementAt(2).Id) .ToArray(); //assert @@ -161,6 +163,132 @@ namespace Umbraco.Tests.Services Assert.AreEqual(17,permissions[2].AssignedPermissions.Length); } + [Test] + + [Test] + public void Calculate_Permissions_For_User_For_Path_1() + { + //see: http://issues.umbraco.org/issue/U4-10075#comment=67-40085 + // for an overview of what this is testing + + const string path = "-1,1,2,3,4"; + var pathIds = path.GetIdsFromPathReversed(); + + const int groupA = 7; + const int groupB = 8; + const int groupC = 9; + + var userGroups = new Dictionary + { + {groupA, new[] {"S", "D", "F"}}, + {groupB, new[] {"S", "D", "G", "K"}}, + {groupC, new[] {"F", "G"}} + }; + + var permissions = new [] + { + new EntityPermission(groupA, 1, userGroups[groupA], isDefaultPermissions:true), + new EntityPermission(groupA, 2, userGroups[groupA], isDefaultPermissions:true), + new EntityPermission(groupA, 3, userGroups[groupA], isDefaultPermissions:true), + new EntityPermission(groupA, 4, userGroups[groupA], isDefaultPermissions:true), + + new EntityPermission(groupB, 1, userGroups[groupB], isDefaultPermissions:true), + new EntityPermission(groupB, 2, new []{"F", "R"}, isDefaultPermissions:false), + new EntityPermission(groupB, 3, userGroups[groupB], isDefaultPermissions:true), + new EntityPermission(groupB, 4, userGroups[groupB], isDefaultPermissions:true), + + new EntityPermission(groupC, 1, userGroups[groupC], isDefaultPermissions:true), + new EntityPermission(groupC, 2, userGroups[groupC], isDefaultPermissions:true), + new EntityPermission(groupC, 3, new []{"Q", "Z"}, isDefaultPermissions:false), + new EntityPermission(groupC, 4, userGroups[groupC], isDefaultPermissions:true), + }; + + //Permissions for Id 4 + var result = UserService.CalculatePermissionsForPathForUser(permissions, pathIds); + Assert.AreEqual(4, result.EntityId); + var allPermissions = result.GetAllPermissions().ToArray(); + Assert.AreEqual(6, allPermissions.Length, string.Join(",", allPermissions)); + Assert.IsTrue(allPermissions.ContainsAll(new[] { "S", "D", "F", "R", "Q", "Z" })); + + //Permissions for Id 3 + result = UserService.CalculatePermissionsForPathForUser(permissions, pathIds.Skip(1).ToArray()); + Assert.AreEqual(3, result.EntityId); + allPermissions = result.GetAllPermissions().ToArray(); + Assert.AreEqual(6, allPermissions.Length, string.Join(",", allPermissions)); + Assert.IsTrue(allPermissions.ContainsAll(new[] { "S", "D", "F", "R", "Q", "Z" })); + + //Permissions for Id 2 + result = UserService.CalculatePermissionsForPathForUser(permissions, pathIds.Skip(2).ToArray()); + Assert.AreEqual(2, result.EntityId); + allPermissions = result.GetAllPermissions().ToArray(); + Assert.AreEqual(5, allPermissions.Length, string.Join(",", allPermissions)); + Assert.IsTrue(allPermissions.ContainsAll(new[] { "S", "D", "F", "G", "R" })); + + //Permissions for Id 1 + result = UserService.CalculatePermissionsForPathForUser(permissions, pathIds.Skip(3).ToArray()); + Assert.AreEqual(1, result.EntityId); + allPermissions = result.GetAllPermissions().ToArray(); + Assert.AreEqual(5, allPermissions.Length, string.Join(",", allPermissions)); + Assert.IsTrue(allPermissions.ContainsAll(new[] { "S", "D", "F", "G", "K" })); + + } + + [Test] + public void Determine_Deepest_Explicit_Permissions_For_Group_For_Path_1() + { + var path = "-1,1,2,3"; + var pathIds = path.GetIdsFromPathReversed(); + var defaults = new[] {"A", "B"}; + var permissions = new List + { + new EntityPermission(9876, 1, defaults, isDefaultPermissions:true), + new EntityPermission(9876, 2, new []{"B","C", "D"}, isDefaultPermissions:false), + new EntityPermission(9876, 3, defaults, isDefaultPermissions:true) + }; + var result = UserService.GetPermissionsForPathForGroup(permissions, pathIds, fallbackToDefaultPermissions: true); + Assert.AreEqual(3, result.AssignedPermissions.Length); + Assert.IsFalse(result.IsDefaultPermissions); + Assert.IsTrue(result.AssignedPermissions.ContainsAll(new[] { "B", "C", "D" })); + Assert.AreEqual(2, result.EntityId); + Assert.AreEqual(9876, result.UserGroupId); + } + + [Test] + public void Determine_Deepest_Explicit_Permissions_For_Group_For_Path_2() + { + var path = "-1,1,2,3"; + var pathIds = path.GetIdsFromPathReversed(); + var defaults = new[] { "A", "B", "C" }; + var permissions = new List + { + new EntityPermission(9876, 1, defaults, isDefaultPermissions:true), + new EntityPermission(9876, 2, defaults, isDefaultPermissions:true), + new EntityPermission(9876, 3, defaults, isDefaultPermissions:true) + }; + var result = UserService.GetPermissionsForPathForGroup(permissions, pathIds, fallbackToDefaultPermissions:false); + Assert.IsNull(result); + } + + [Test] + public void Determine_Deepest_Explicit_Permissions_For_Group_For_Path_3() + { + var path = "-1,1,2,3"; + var pathIds = path.GetIdsFromPathReversed(); + var defaults = new[] { "A", "B" }; + var permissions = new List + { + new EntityPermission(9876, 1, defaults, isDefaultPermissions:true), + new EntityPermission(9876, 2, defaults, isDefaultPermissions:true), + new EntityPermission(9876, 3, defaults, isDefaultPermissions:true) + }; + var result = UserService.GetPermissionsForPathForGroup(permissions, pathIds, fallbackToDefaultPermissions: true); + Assert.AreEqual(2, result.AssignedPermissions.Length); + Assert.IsTrue(result.IsDefaultPermissions); + Assert.IsTrue(result.AssignedPermissions.ContainsAll(defaults)); + Assert.AreEqual(3, result.EntityId); + Assert.AreEqual(9876, result.UserGroupId); + } + [Test] public void Get_User_Implicit_Permissions() { diff --git a/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs b/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs index f4480ca636..bc1f5dfe77 100644 --- a/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs @@ -50,7 +50,8 @@ namespace Umbraco.Tests.Web.Controllers var contentService = contentServiceMock.Object; var userServiceMock = new Mock(); var permissions = new List(); - userServiceMock.Setup(x => x.GetPermissions(user, 1234)).Returns(permissions); + var permissionSet = new EntityPermissionSet(1234, permissions); + userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1,1234,5678")).Returns(permissionSet); var userService = userServiceMock.Object; //act/assert @@ -73,7 +74,8 @@ namespace Umbraco.Tests.Web.Controllers var contentService = contentServiceMock.Object; var userServiceMock = new Mock(); var permissions = new List(); - userServiceMock.Setup(x => x.GetPermissions(user, 1234)).Returns(permissions); + var permissionSet = new EntityPermissionSet(1234, permissions); + userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1,1234")).Returns(permissionSet); var userService = userServiceMock.Object; //act @@ -100,9 +102,10 @@ namespace Umbraco.Tests.Web.Controllers var userServiceMock = new Mock(); var permissions = new List { - new EntityPermission(1234, new string[]{ "A", "B", "C" }) + new EntityPermission(9876, 1234, new string[]{ "A", "B", "C" }) }; - userServiceMock.Setup(x => x.GetPermissions(user, 1234)).Returns(permissions); + var permissionSet = new EntityPermissionSet(1234, permissions); + userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1,1234,5678")).Returns(permissionSet); var userService = userServiceMock.Object; //act @@ -129,9 +132,10 @@ namespace Umbraco.Tests.Web.Controllers var userServiceMock = new Mock(); var permissions = new List { - new EntityPermission(1234, new string[]{ "A", "F", "C" }) + new EntityPermission(9876, 1234, new string[]{ "A", "F", "C" }) }; - userServiceMock.Setup(x => x.GetPermissions(user, 1234)).Returns(permissions); + var permissionSet = new EntityPermissionSet(1234, permissions); + userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1,1234,5678")).Returns(permissionSet); var userService = userServiceMock.Object; //act @@ -217,9 +221,10 @@ namespace Umbraco.Tests.Web.Controllers var userServiceMock = new Mock(); var permissions = new List { - new EntityPermission(1234, new string[]{ "A" }) + new EntityPermission(9876, 1234, new string[]{ "A" }) }; - userServiceMock.Setup(x => x.GetPermissions(user, -1)).Returns(permissions); + var permissionSet = new EntityPermissionSet(1234, permissions); + userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1")).Returns(permissionSet); var userService = userServiceMock.Object; //act @@ -241,9 +246,10 @@ namespace Umbraco.Tests.Web.Controllers var userServiceMock = new Mock(); var permissions = new List { - new EntityPermission(1234, new string[]{ "A" }) + new EntityPermission(9876, 1234, new string[]{ "A" }) }; - userServiceMock.Setup(x => x.GetPermissions(user, -1)).Returns(permissions); + var permissionSet = new EntityPermissionSet(1234, permissions); + userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1")).Returns(permissionSet); var userService = userServiceMock.Object; //act @@ -264,10 +270,12 @@ namespace Umbraco.Tests.Web.Controllers var userServiceMock = new Mock(); var permissions = new List - { - new EntityPermission(1234, new string[]{ "A" }) - }; - userServiceMock.Setup(x => x.GetPermissions(user, -20)).Returns(permissions); + { + new EntityPermission(9876, 1234, new string[]{ "A" }) + }; + var permissionSet = new EntityPermissionSet(-20, permissions); + + userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-20")).Returns(permissionSet); var userService = userServiceMock.Object; //act @@ -289,9 +297,10 @@ namespace Umbraco.Tests.Web.Controllers var userServiceMock = new Mock(); var permissions = new List { - new EntityPermission(1234, new string[]{ "A" }) + new EntityPermission(9876, 1234, new string[]{ "A" }) }; - userServiceMock.Setup(x => x.GetPermissions(user, -20)).Returns(permissions); + var permissionSet = new EntityPermissionSet(1234, permissions); + userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-20")).Returns(permissionSet); var userService = userServiceMock.Object; //act diff --git a/src/Umbraco.Tests/Web/Controllers/FilterAllowedOutgoingContentAttributeTests.cs b/src/Umbraco.Tests/Web/Controllers/FilterAllowedOutgoingContentAttributeTests.cs index 8e84dcf030..49ce356324 100644 --- a/src/Umbraco.Tests/Web/Controllers/FilterAllowedOutgoingContentAttributeTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/FilterAllowedOutgoingContentAttributeTests.cs @@ -112,10 +112,10 @@ namespace Umbraco.Tests.Web.Controllers //we're only assigning 3 nodes browse permissions so that is what we expect as a result var permissions = new List { - new EntityPermission(1, new string[]{ "F" }), - new EntityPermission(2, new string[]{ "F" }), - new EntityPermission(3, new string[]{ "F" }), - new EntityPermission(4, new string[]{ "A" }) + new EntityPermission(9876, 1, new string[]{ "F" }), + new EntityPermission(9876, 2, new string[]{ "F" }), + new EntityPermission(9876, 3, new string[]{ "F" }), + new EntityPermission(9876, 4, new string[]{ "A" }) }; userServiceMock.Setup(x => x.GetPermissions(user, ids)).Returns(permissions); var userService = userServiceMock.Object; diff --git a/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs b/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs index 4b65eb1b3a..8e628204d8 100644 --- a/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs +++ b/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs @@ -599,10 +599,13 @@ namespace Umbraco.Web.Cache #region User/permissions event handlers - static void CacheRefresherEventHandler_AssignedPermissions(PermissionRepository sender, SaveEventArgs e) + static void CacheRefresherEventHandler_AssignedPermissions(PermissionRepository sender, SaveEventArgs e) { var groupIds = e.SavedEntities.Select(x => x.UserGroupId).Distinct(); - groupIds.ForEach(x => DistributedCache.Instance.RefreshUserGroupPermissionsCache(x)); + foreach (var groupId in groupIds) + { + DistributedCache.Instance.RefreshUserGroupPermissionsCache(groupId); + } } static void PermissionDeleted(UserGroupPermission sender, DeleteEventArgs e) diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 8fd2d8f221..f935673317 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -1076,17 +1076,21 @@ namespace Umbraco.Web.Editors return false; } - if (permissionsToCheck == null || permissionsToCheck.Any() == false) + if (permissionsToCheck == null || permissionsToCheck.Length == 0) { return true; } - - var permission = userService.GetPermissions(user, nodeId).FirstOrDefault(); + + //get the implicit/inherited permissions for the user for this path, + //if there is no content item for this id, than just use the id as the path (i.e. -1 or -20) + var path = contentItem != null ? contentItem.Path : nodeId.ToString(); + var permission = userService.GetPermissionsForPath(user, path); var allowed = true; foreach (var p in permissionsToCheck) { - if (permission == null || permission.AssignedPermissions.Contains(p.ToString(CultureInfo.InvariantCulture)) == false) + if (permission == null + || permission.GetAllPermissions().Contains(p.ToString(CultureInfo.InvariantCulture)) == false) { allowed = false; } diff --git a/src/umbraco.businesslogic/User.cs b/src/umbraco.businesslogic/User.cs index 0f92a4427f..dcac5a0caf 100644 --- a/src/umbraco.businesslogic/User.cs +++ b/src/umbraco.businesslogic/User.cs @@ -572,7 +572,7 @@ namespace umbraco.BusinessLogic var userService = ApplicationContext.Current.Services.UserService; return string.Join("", - userService.GetPermissionsForPath(UserEntity, path).PermissionsSet.SelectMany(x => x.Permission)); + userService.GetPermissionsForPath(UserEntity, path).GetAllPermissions()); } /// From 87dcdc596e906926f3062a98521b0c2e34b26ef6 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 13 Jul 2017 01:56:22 +1000 Subject: [PATCH 05/11] Gets last unit tests written for returning all permissions for either user or group when not specifying Ids --- .../Models/Membership/ContentPermissionSet.cs | 2 +- .../Models/Membership/EntityPermission.cs | 41 ++--- .../Membership/EntityPermissionCollection.cs | 34 +++++ .../Models/Membership/EntityPermissionSet.cs | 15 +- .../Repositories/ContentRepository.cs | 2 +- .../Interfaces/IContentRepository.cs | 2 +- .../Interfaces/IUserGroupRepository.cs | 4 +- .../Repositories/PermissionRepository.cs | 93 +++++------- .../Repositories/UserGroupRepository.cs | 13 +- src/Umbraco.Core/Services/ContentService.cs | 8 +- .../Services/ContentServiceExtensions.cs | 2 +- src/Umbraco.Core/Services/IContentService.cs | 2 +- src/Umbraco.Core/Services/IUserService.cs | 10 +- src/Umbraco.Core/Services/UserService.cs | 42 ++--- src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../Services/UserServiceTests.cs | 143 ++++++++++++++---- .../Controllers/ContentControllerUnitTests.cs | 16 +- ...terAllowedOutgoingContentAttributeTests.cs | 2 +- 18 files changed, 262 insertions(+), 170 deletions(-) create mode 100644 src/Umbraco.Core/Models/Membership/EntityPermissionCollection.cs diff --git a/src/Umbraco.Core/Models/Membership/ContentPermissionSet.cs b/src/Umbraco.Core/Models/Membership/ContentPermissionSet.cs index 1345be2c8c..ca0a910c05 100644 --- a/src/Umbraco.Core/Models/Membership/ContentPermissionSet.cs +++ b/src/Umbraco.Core/Models/Membership/ContentPermissionSet.cs @@ -14,7 +14,7 @@ namespace Umbraco.Core.Models.Membership { private readonly IContent _content; - public ContentPermissionSet(IContent content, IEnumerable permissionsSet) + public ContentPermissionSet(IContent content, EntityPermissionCollection permissionsSet) : base(content.Id, permissionsSet) { _content = content; diff --git a/src/Umbraco.Core/Models/Membership/EntityPermission.cs b/src/Umbraco.Core/Models/Membership/EntityPermission.cs index c6d9e8350d..8f609c0e13 100644 --- a/src/Umbraco.Core/Models/Membership/EntityPermission.cs +++ b/src/Umbraco.Core/Models/Membership/EntityPermission.cs @@ -1,11 +1,11 @@ -using System.Linq; +using System; namespace Umbraco.Core.Models.Membership { /// /// Represents an entity permission (defined on the user group and derived to retrieve permissions for a given user) /// - public class EntityPermission + public class EntityPermission : IEquatable { public EntityPermission(int groupId, int entityId, string[] assignedPermissions) { @@ -37,25 +37,30 @@ namespace Umbraco.Core.Models.Membership /// /// This will be the case when looking up entity permissions and falling back to the default permissions /// - public bool IsDefaultPermissions { get; private set; } + public bool IsDefaultPermissions { get; private set; } - /// - /// Adds additional permissions to an existing instance of - /// ensuring that only ones that aren't already assigned are added - /// - /// - public void AddAdditionalPermissions(string[] additionalPermissions) - { - //TODO: Fix the performance of this, we need to use things like HashSet and equality checkers, we are iterating too much + public bool Equals(EntityPermission other) + { + if (ReferenceEquals(null, other)) return false; + if (ReferenceEquals(this, other)) return true; + return EntityId == other.EntityId && UserGroupId == other.UserGroupId; + } - var newPermissions = AssignedPermissions.ToList(); - newPermissions.AddRange(additionalPermissions); - AssignedPermissions = newPermissions - .Distinct() - .ToArray(); - } - + public override bool Equals(object obj) + { + if (ReferenceEquals(null, obj)) return false; + if (ReferenceEquals(this, obj)) return true; + if (obj.GetType() != this.GetType()) return false; + return Equals((EntityPermission) obj); + } + public override int GetHashCode() + { + unchecked + { + return (EntityId * 397) ^ UserGroupId; + } + } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/Membership/EntityPermissionCollection.cs b/src/Umbraco.Core/Models/Membership/EntityPermissionCollection.cs new file mode 100644 index 0000000000..5fca079cfc --- /dev/null +++ b/src/Umbraco.Core/Models/Membership/EntityPermissionCollection.cs @@ -0,0 +1,34 @@ +using System.Collections.Generic; +using System.Linq; + +namespace Umbraco.Core.Models.Membership +{ + /// + /// A of + /// + public class EntityPermissionCollection : HashSet + { + public EntityPermissionCollection() + { + } + + public EntityPermissionCollection(IEnumerable collection) : base(collection) + { + } + + /// + /// Returns the aggregate permissions in the permission set + /// + /// + /// + /// This value is only calculated once + /// + public IEnumerable GetAllPermissions() + { + return _aggregatePermissions ?? (_aggregatePermissions = + this.SelectMany(x => x.AssignedPermissions).Distinct().ToArray()); + } + + private string[] _aggregatePermissions; + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Models/Membership/EntityPermissionSet.cs b/src/Umbraco.Core/Models/Membership/EntityPermissionSet.cs index dd9190ae85..0f35fec990 100644 --- a/src/Umbraco.Core/Models/Membership/EntityPermissionSet.cs +++ b/src/Umbraco.Core/Models/Membership/EntityPermissionSet.cs @@ -14,10 +14,10 @@ namespace Umbraco.Core.Models.Membership /// public static EntityPermissionSet Empty() { - return new EntityPermissionSet(-1, new EntityPermission[0]); + return new EntityPermissionSet(-1, new EntityPermissionCollection()); } - public EntityPermissionSet(int entityId, IEnumerable permissionsSet) + public EntityPermissionSet(int entityId, EntityPermissionCollection permissionsSet) { EntityId = entityId; PermissionsSet = permissionsSet; @@ -31,10 +31,11 @@ namespace Umbraco.Core.Models.Membership /// /// The key/value pairs of user group id & single permission /// - public IEnumerable PermissionsSet { get; private set; } + public EntityPermissionCollection PermissionsSet { get; private set; } + /// - /// Returns the aggregte permissions in the permission set + /// Returns the aggregate permissions in the permission set /// /// /// @@ -42,15 +43,11 @@ namespace Umbraco.Core.Models.Membership /// public IEnumerable GetAllPermissions() { - return _calculatedPermissions ?? (_calculatedPermissions = - PermissionsSet.SelectMany(x => x.AssignedPermissions).Distinct().ToArray()); + return PermissionsSet.GetAllPermissions(); } - private string[] _calculatedPermissions; - - } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 2c3f126c6e..addd0bc925 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -847,7 +847,7 @@ order by umbracoNode.{2}, umbracoNode.parentID, umbracoNode.sortOrder", /// /// /// - public IEnumerable GetPermissionsForEntity(int entityId) + public EntityPermissionCollection GetPermissionsForEntity(int entityId) { return _permissionRepository.GetPermissionsForEntity(entityId); } diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs index 6f643b3e0b..c44c8ac1dc 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IContentRepository.cs @@ -61,7 +61,7 @@ namespace Umbraco.Core.Persistence.Repositories /// /// /// - IEnumerable GetPermissionsForEntity(int entityId); + EntityPermissionCollection GetPermissionsForEntity(int entityId); ///// ///// Gets the implicit/inherited list of permissions for the content item diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs index b018bd3c08..ec5fe79065 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs @@ -30,7 +30,7 @@ namespace Umbraco.Core.Persistence.Repositories /// /// Id of group /// Array of entity Ids, if empty will return permissions for the group for all entities - IEnumerable GetPermissionsForEntities(int groupId, params int[] entityIds); + EntityPermissionCollection GetPermissionsForEntities(int groupId, params int[] entityIds); /// /// Gets explicilt and default permissions (if requested) permissions for the group for specified entities @@ -38,7 +38,7 @@ namespace Umbraco.Core.Persistence.Repositories /// The group /// If true will include the group's default permissions if no permissions are explicitly assigned /// Array of entity Ids, if empty will return permissions for the group for all entities - IEnumerable GetPermissionsForEntities(IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds); + EntityPermissionCollection GetPermissionsForEntities(IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds); /// /// Replaces the same permission set for a single group to any number of entities diff --git a/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs b/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs index 4cad3be7e7..a22b5c5437 100644 --- a/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs @@ -42,62 +42,43 @@ namespace Umbraco.Core.Persistence.Repositories /// /// /// - public IEnumerable GetPermissionsForEntities(int groupId, params int[] entityIds) - //var whereCriteria = GetPermissionsForEntitiesCriteria(groupId, entityIds); - var result = new List(); - //iterate in groups of 2000 since we don't want to exceed the max SQL param count - foreach (var groupOfIds in entityIds.InGroupsOf(2000)) - { - var ids = groupOfIds; + public EntityPermissionCollection GetPermissionsForEntities(int groupId, params int[] entityIds) + { + var result = new EntityPermissionCollection(); - var sql = new Sql(); + if (entityIds.Length == 0) + { + var sql = new Sql(); sql.Select("*") .From(SqlSyntax) - .Where(dto => dto.UserGroupId == groupId && ids.Contains(dto.NodeId), SqlSyntax); + .Where(dto => dto.UserGroupId == groupId, SqlSyntax); var permissions = UnitOfWork.Database.Fetch(sql); - result.AddRange(ConvertToPermissionList(permissions)); - } - return result; - - - //var entityIdKey = GetEntityIdKey(entityIds); - - //return _runtimeCache.GetCacheItem>( - // string.Format("{0}{1}{2}", - // CacheKeys.UserGroupPermissionsCacheKey, - // groupId, - // entityIdKey), - // () => - // { - // var whereCriteria = GetPermissionsForEntitiesCriteria(groupId, entityIds); - // var sql = new Sql(); - // sql.Select("*") - // .From() - // .Where(whereCriteria); - // var result = UnitOfWork.Database.Fetch(sql); - // return ConvertToPermissionList(result); - // }, - // GetCacheTimeout(), - // priority: GetCachePriority()); - } - - //private static string GetEntityIdKey(int[] entityIds) - //{ - // return string.Join(",", entityIds.Select(x => x.ToString(CultureInfo.InvariantCulture))); - //} - - //private static TimeSpan GetCacheTimeout() - //{ - // //Since this cache can be quite large (http://issues.umbraco.org/issue/U4-2161) we will only have this exist in cache for 20 minutes, - // // then it will refresh from the database. - // return new TimeSpan(0, 20, 0); - //} - - //private static CacheItemPriority GetCachePriority() - //{ - // //Since this cache can be quite large (http://issues.umbraco.org/issue/U4-2161) we will make this priority below average - // return CacheItemPriority.BelowNormal; - //} + foreach (var permission in ConvertToPermissionList(permissions)) + { + result.Add(permission); + } + } + else + { + //iterate in groups of 2000 since we don't want to exceed the max SQL param count + foreach (var groupOfIds in entityIds.InGroupsOf(2000)) + { + var ids = groupOfIds; + + var sql = new Sql(); + sql.Select("*") + .From(SqlSyntax) + .Where(dto => dto.UserGroupId == groupId && ids.Contains(dto.NodeId), SqlSyntax); + var permissions = UnitOfWork.Database.Fetch(sql); + foreach (var permission in ConvertToPermissionList(permissions)) + { + result.Add(permission); + } + } + } + + return result; + } /// /// Returns permissions directly assigned to the content items for all user groups @@ -121,7 +102,7 @@ namespace Umbraco.Core.Persistence.Repositories /// /// /// - public IEnumerable GetPermissionsForEntity(int entityId) + public EntityPermissionCollection GetPermissionsForEntity(int entityId) { var sql = new Sql(); sql.Select("*") @@ -343,16 +324,16 @@ namespace Umbraco.Core.Persistence.Repositories ReplaceEntityPermissions(entity); } - private static IEnumerable ConvertToPermissionList(IEnumerable result) + private static EntityPermissionCollection ConvertToPermissionList(IEnumerable result) { - var permissions = new List(); + var permissions = new EntityPermissionCollection(); var nodePermissions = result.GroupBy(x => x.NodeId); foreach (var np in nodePermissions) { var userGroupPermissions = np.GroupBy(x => x.UserGroupId); foreach (var permission in userGroupPermissions) { - var perms = permission.Select(x => x.Permission).ToArray(); + var perms = permission.Select(x => x.Permission).Distinct().ToArray(); permissions.Add(new EntityPermission(permission.Key, np.Key, perms)); } } diff --git a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs index bd2b8390cd..619d896fde 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs @@ -97,7 +97,7 @@ namespace Umbraco.Core.Persistence.Repositories /// /// Id of group /// Array of entity Ids, if empty will return permissions for the group for all entities - public IEnumerable GetPermissionsForEntities(int groupId, params int[] entityIds) + public EntityPermissionCollection GetPermissionsForEntities(int groupId, params int[] entityIds) { return _permissionRepository.GetPermissionsForEntities(groupId, entityIds); } @@ -108,12 +108,12 @@ namespace Umbraco.Core.Persistence.Repositories /// The group /// If true will include the group's default permissions if no permissions are explicitly assigned /// Array of entity Ids, if empty will return permissions for the group for all entities - public IEnumerable GetPermissionsForEntities(IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) + public EntityPermissionCollection GetPermissionsForEntities(IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) { if (group == null) throw new ArgumentNullException("group"); var explicitPermissions = GetPermissionsForEntities(group.Id, nodeIds); - var result = new List(explicitPermissions); + var result = new EntityPermissionCollection(explicitPermissions); // If requested, and no permissions are assigned to a particular node, then we will fill in those permissions with the group's defaults if (fallbackToDefaultPermissions) @@ -121,8 +121,11 @@ namespace Umbraco.Core.Persistence.Repositories var missingIds = nodeIds.Except(result.Select(x => x.EntityId)).ToArray(); if (missingIds.Length > 0) { - result.AddRange(missingIds - .Select(i => new EntityPermission(group.Id, i, group.Permissions.ToArray(), isDefaultPermissions: true))); + foreach (var permission in missingIds + .Select(i => new EntityPermission(group.Id, i, group.Permissions.ToArray(), isDefaultPermissions: true))) + { + result.Add(permission); + } } } return result; diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index b7649ea382..491e03a065 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -134,7 +134,7 @@ namespace Umbraco.Core.Services /// /// /// - public IEnumerable GetPermissionsForEntity(IContent content) + public EntityPermissionCollection GetPermissionsForEntity(IContent content) { using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { @@ -1673,14 +1673,14 @@ namespace Umbraco.Core.Services copy.WriterId = userId; //get the current permissions, if there are any explicit ones they need to be copied - var currentPermissions = GetPermissionsForEntity(content) - .Where(x => x.IsDefaultPermissions == false).ToArray(); + var currentPermissions = GetPermissionsForEntity(content); + currentPermissions.RemoveWhere(p => p.IsDefaultPermissions); repository.AddOrUpdate(copy); repository.AddOrUpdatePreviewXml(copy, c => _entitySerializer.Serialize(this, _dataTypeService, _userService, c)); //add permissions - if (currentPermissions.Length > 0) + if (currentPermissions.Count > 0) { var permissionSet = new ContentPermissionSet(copy, currentPermissions); repository.AddOrUpdatePermissions(permissionSet); diff --git a/src/Umbraco.Core/Services/ContentServiceExtensions.cs b/src/Umbraco.Core/Services/ContentServiceExtensions.cs index c6dc0274b6..b3380cf1bf 100644 --- a/src/Umbraco.Core/Services/ContentServiceExtensions.cs +++ b/src/Umbraco.Core/Services/ContentServiceExtensions.cs @@ -17,7 +17,7 @@ namespace Umbraco.Core.Services /// public static void RemoveContentPermissions(this IContentService contentService, int contentId) { - contentService.ReplaceContentPermissions(new EntityPermissionSet(contentId, Enumerable.Empty())); + contentService.ReplaceContentPermissions(new EntityPermissionSet(contentId, new EntityPermissionCollection())); } /// diff --git a/src/Umbraco.Core/Services/IContentService.cs b/src/Umbraco.Core/Services/IContentService.cs index 251bf169ad..b048046ac0 100644 --- a/src/Umbraco.Core/Services/IContentService.cs +++ b/src/Umbraco.Core/Services/IContentService.cs @@ -155,7 +155,7 @@ namespace Umbraco.Core.Services /// /// /// - IEnumerable GetPermissionsForEntity(IContent content); + EntityPermissionCollection GetPermissionsForEntity(IContent content); bool SendToPublication(IContent content, int userId = 0); diff --git a/src/Umbraco.Core/Services/IUserService.cs b/src/Umbraco.Core/Services/IUserService.cs index c5b282cd66..5f1d16d6bc 100644 --- a/src/Umbraco.Core/Services/IUserService.cs +++ b/src/Umbraco.Core/Services/IUserService.cs @@ -83,8 +83,8 @@ namespace Umbraco.Core.Services /// /// This is useful when an entire section is removed from config /// Alias of the section to remove - void DeleteSectionFromAllUserGroups(string sectionAlias); - + void DeleteSectionFromAllUserGroups(string sectionAlias); + /// /// Get explicitly assigned permissions for a user and optional node ids /// @@ -95,8 +95,8 @@ namespace Umbraco.Core.Services /// /// This will return the default permissions for the user's groups for nodes that don't have explicitly defined permissions /// - IEnumerable GetPermissions(IUser user, params int[] nodeIds); - + EntityPermissionCollection GetPermissions(IUser user, params int[] nodeIds); + /// /// Get explicitly assigned permissions for a group and optional node Ids /// @@ -106,7 +106,7 @@ namespace Umbraco.Core.Services /// /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of - IEnumerable GetPermissions(IUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds); + EntityPermissionCollection GetPermissions(IUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds); /// /// Gets the implicit/inherited permissions for the user for the given path diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index 52e3e82106..149b37cbec 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -6,6 +6,7 @@ using System.Linq.Expressions; using Umbraco.Core.Configuration; using Umbraco.Core.Events; using Umbraco.Core.Logging; +using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; using Umbraco.Core.Persistence; using Umbraco.Core.Persistence.DatabaseModelDefinitions; @@ -845,15 +846,17 @@ namespace Umbraco.Core.Services /// User to retrieve permissions for /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of - public IEnumerable GetPermissions(IUser user, params int[] nodeIds) - { - var result = new List(); + public EntityPermissionCollection GetPermissions(IUser user, params int[] nodeIds) + { + //TODO: we don't need to run this query for each group assigned, we can do this in one query + + var result = new EntityPermissionCollection(); foreach (var group in user.Groups) { foreach (var permission in GetPermissions(group, true, nodeIds)) { - AddOrAmendPermissionList(result, permission); + result.Add(permission); } } @@ -889,7 +892,7 @@ namespace Umbraco.Core.Services /// /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of - public IEnumerable GetPermissions(IUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) + public EntityPermissionCollection GetPermissions(IUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) { if (group == null) throw new ArgumentNullException("group"); using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) @@ -899,28 +902,6 @@ namespace Umbraco.Core.Services } } - /// - /// For an existing list of , takes a new and aggregates it. - /// If a permission for the entity associated with the new permission already exists, it's updated with those permissions to create a distinct, most permissive set. - /// If it doesn't, it's added to the list. - /// - /// List of already found permissions - /// New permission to aggregate - private static void AddOrAmendPermissionList(ICollection permissions, EntityPermission groupPermission) - { - //TODO: Fix the performance of this, we need to use things like HashSet and equality checkers, we are iterating too much - - var existingPermission = permissions.FirstOrDefault(x => x.EntityId == groupPermission.EntityId); - if (existingPermission != null) - { - existingPermission.AddAdditionalPermissions(groupPermission.AssignedPermissions); - } - else - { - permissions.Add(groupPermission); - } - } - /// /// Gets the implicit/inherited permissions for the user for the given path /// @@ -980,7 +961,7 @@ namespace Umbraco.Core.Services //The actual entity id being looked at (deepest part of the path) var entityId = pathIds[0]; - var resultPermissions = new List(); + var resultPermissions = new EntityPermissionCollection(); //create a grouped by dictionary of another grouped by dictionary var permissionsByGroup = groupPermissions @@ -1008,9 +989,8 @@ namespace Umbraco.Core.Services { if (entityPermission.IsDefaultPermissions == false) { - //explicit permision found so we'll append it and move on, of course if there was two explicit permissions - //found for this group the ones after this one wouldn't matter but considering there should only be one per - //group anyways, that is fine. + //explicit permision found so we'll append it and move on, the collection is a hashset anyways + //so only supports adding one element per groupid/contentid resultPermissions.Add(entityPermission); added = true; break; diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 7269dcd9f6..f9b1c7a752 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -346,6 +346,7 @@ + diff --git a/src/Umbraco.Tests/Services/UserServiceTests.cs b/src/Umbraco.Tests/Services/UserServiceTests.cs index ac58f9f734..610a6498c8 100644 --- a/src/Umbraco.Tests/Services/UserServiceTests.cs +++ b/src/Umbraco.Tests/Services/UserServiceTests.cs @@ -80,21 +80,21 @@ namespace Umbraco.Tests.Services MockedContent.CreateSimpleContent(contentType) }; ServiceContext.ContentService.Save(content); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(0), ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(0), ActionDelete.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(0), ActionMove.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(1), ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(1), ActionDelete.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(2), ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[0], ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[0], ActionDelete.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[0], ActionMove.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[1], ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[1], ActionDelete.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[2], ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); // Act - var permissions = userService.GetPermissions(user, content.ElementAt(0).Id, content.ElementAt(1).Id, content.ElementAt(2).Id); + var permissions = userService.GetPermissions(user, content[0].Id, content[1].Id, content[2].Id).ToArray(); //assert - Assert.AreEqual(3, permissions.Count()); - Assert.AreEqual(3, permissions.ElementAt(0).AssignedPermissions.Length); - Assert.AreEqual(2, permissions.ElementAt(1).AssignedPermissions.Length); - Assert.AreEqual(1, permissions.ElementAt(2).AssignedPermissions.Length); + Assert.AreEqual(3, permissions.Length); + Assert.AreEqual(3, permissions[0].AssignedPermissions.Length); + Assert.AreEqual(2, permissions[1].AssignedPermissions.Length); + Assert.AreEqual(1, permissions[2].AssignedPermissions.Length); } [Test] @@ -121,13 +121,13 @@ namespace Umbraco.Tests.Services ServiceContext.ContentService.AssignContentPermission(content.ElementAt(2), ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); // Act - var permissions = userService.GetPermissions(userGroup, false, content.ElementAt(0).Id, content.ElementAt(1).Id, content.ElementAt(2).Id); + var permissions = userService.GetPermissions(userGroup, false, content[0].Id, content[1].Id, content[2].Id).ToArray(); //assert - Assert.AreEqual(3, permissions.Count()); - Assert.AreEqual(3, permissions.ElementAt(0).AssignedPermissions.Length); - Assert.AreEqual(2, permissions.ElementAt(1).AssignedPermissions.Length); - Assert.AreEqual(1, permissions.ElementAt(2).AssignedPermissions.Length); + Assert.AreEqual(3, permissions.Length); + Assert.AreEqual(3, permissions[0].AssignedPermissions.Length); + Assert.AreEqual(2, permissions[1].AssignedPermissions.Length); + Assert.AreEqual(1, permissions[2].AssignedPermissions.Length); } [Test] @@ -146,14 +146,14 @@ namespace Umbraco.Tests.Services MockedContent.CreateSimpleContent(contentType) }; ServiceContext.ContentService.Save(content); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(0), ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(0), ActionDelete.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(0), ActionMove.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(1), ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); - ServiceContext.ContentService.AssignContentPermission(content.ElementAt(1), ActionDelete.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[0], ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[0], ActionDelete.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[0], ActionMove.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[1], ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[1], ActionDelete.Instance.Letter, new int[] { userGroup.Id }); // Act - var permissions = userService.GetPermissions(userGroup, true, content.ElementAt(0).Id, content.ElementAt(1).Id, content.ElementAt(2).Id) + var permissions = userService.GetPermissions(userGroup, true, content[0].Id, content[1].Id, content[2].Id) .ToArray(); //assert @@ -164,9 +164,100 @@ namespace Umbraco.Tests.Services } [Test] + public void Get_All_User_Permissions_For_All_Nodes() + { + // Arrange + var userService = ServiceContext.UserService; + var userGroup1 = CreateTestUserGroup(); + var userGroup2 = CreateTestUserGroup("test2", "Test 2"); + var user = userService.CreateUserWithIdentity("John Doe", "john@umbraco.io"); + + user.AddGroup(userGroup1); + user.AddGroup(userGroup2); + userService.Save(user); + + var contentType = MockedContentTypes.CreateSimpleContentType(); + ServiceContext.ContentTypeService.Save(contentType); + var content = new[] + { + MockedContent.CreateSimpleContent(contentType), + MockedContent.CreateSimpleContent(contentType), + MockedContent.CreateSimpleContent(contentType) + }; + ServiceContext.ContentService.Save(content); + ServiceContext.ContentService.AssignContentPermission(content[0], ActionBrowse.Instance.Letter, new int[] { userGroup1.Id }); + ServiceContext.ContentService.AssignContentPermission(content[0], ActionDelete.Instance.Letter, new int[] { userGroup1.Id }); + ServiceContext.ContentService.AssignContentPermission(content[0], ActionMove.Instance.Letter, new int[] { userGroup2.Id }); + ServiceContext.ContentService.AssignContentPermission(content[1], ActionBrowse.Instance.Letter, new int[] { userGroup1.Id }); + ServiceContext.ContentService.AssignContentPermission(content[1], ActionDelete.Instance.Letter, new int[] { userGroup2.Id }); + ServiceContext.ContentService.AssignContentPermission(content[2], ActionDelete.Instance.Letter, new int[] { userGroup1.Id }); + + // Act + //we don't pass in any nodes so it will return all of them + var result = userService.GetPermissions(user).ToArray(); + var permissions = result + .GroupBy(x => x.EntityId) + .ToDictionary(x => x.Key, x => x.GroupBy(a => a.UserGroupId).ToDictionary(a => a.Key, a => a.ToArray())); + + //assert + Assert.AreEqual(3, permissions.Count); + Assert.IsTrue(permissions.ContainsKey(content[0].Id)); + Assert.IsTrue(permissions[content[0].Id].ContainsKey(userGroup1.Id)); + Assert.IsTrue(permissions[content[0].Id].ContainsKey(userGroup2.Id)); + Assert.AreEqual(2, permissions[content[0].Id][userGroup1.Id].SelectMany(x => x.AssignedPermissions).Count()); + Assert.AreEqual(1, permissions[content[0].Id][userGroup2.Id].SelectMany(x => x.AssignedPermissions).Count()); + Assert.IsTrue(permissions.ContainsKey(content[1].Id)); + Assert.IsTrue(permissions[content[1].Id].ContainsKey(userGroup1.Id)); + Assert.IsTrue(permissions[content[1].Id].ContainsKey(userGroup2.Id)); + Assert.AreEqual(1, permissions[content[1].Id][userGroup1.Id].SelectMany(x => x.AssignedPermissions).Count()); + Assert.AreEqual(1, permissions[content[1].Id][userGroup2.Id].SelectMany(x => x.AssignedPermissions).Count()); + Assert.IsTrue(permissions.ContainsKey(content[2].Id)); + Assert.IsTrue(permissions[content[2].Id].ContainsKey(userGroup1.Id)); + Assert.IsFalse(permissions[content[2].Id].ContainsKey(userGroup2.Id)); + Assert.AreEqual(1, permissions[content[2].Id][userGroup1.Id].SelectMany(x => x.AssignedPermissions).Count()); + } [Test] - public void Calculate_Permissions_For_User_For_Path_1() + public void Get_All_User_Group_Permissions_For_All_Nodes() + { + // Arrange + var userService = ServiceContext.UserService; + var userGroup = CreateTestUserGroup(); + + var contentType = MockedContentTypes.CreateSimpleContentType(); + ServiceContext.ContentTypeService.Save(contentType); + var content = new[] + { + MockedContent.CreateSimpleContent(contentType), + MockedContent.CreateSimpleContent(contentType), + MockedContent.CreateSimpleContent(contentType) + }; + ServiceContext.ContentService.Save(content); + ServiceContext.ContentService.AssignContentPermission(content[0], ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[0], ActionDelete.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[0], ActionMove.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[1], ActionBrowse.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[1], ActionDelete.Instance.Letter, new int[] { userGroup.Id }); + ServiceContext.ContentService.AssignContentPermission(content[2], ActionDelete.Instance.Letter, new int[] { userGroup.Id }); + + // Act + //we don't pass in any nodes so it will return all of them + var permissions = userService.GetPermissions(userGroup, true) + .GroupBy(x => x.EntityId) + .ToDictionary(x => x.Key, x => x); + + //assert + Assert.AreEqual(3, permissions.Count); + Assert.IsTrue(permissions.ContainsKey(content[0].Id)); + Assert.AreEqual(3, permissions[content[0].Id].SelectMany(x => x.AssignedPermissions).Count()); + Assert.IsTrue(permissions.ContainsKey(content[1].Id)); + Assert.AreEqual(2, permissions[content[1].Id].SelectMany(x => x.AssignedPermissions).Count()); + Assert.IsTrue(permissions.ContainsKey(content[2].Id)); + Assert.AreEqual(1, permissions[content[2].Id].SelectMany(x => x.AssignedPermissions).Count()); + } + + [Test] + public void Calculate_Permissions_For_User_For_Path() { //see: http://issues.umbraco.org/issue/U4-10075#comment=67-40085 // for an overview of what this is testing @@ -857,12 +948,12 @@ namespace Umbraco.Tests.Services return user; } - private UserGroup CreateTestUserGroup() + private UserGroup CreateTestUserGroup(string alias = "testGroup", string name = "Test Group") { var userGroup = new UserGroup { - Alias = "testGroup", - Name = "Test Group", + Alias = alias, + Name = name, Permissions = "ABCDEFGHIJ1234567".ToCharArray().Select(x => x.ToString()) }; diff --git a/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs b/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs index bc1f5dfe77..23177cf419 100644 --- a/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs @@ -49,7 +49,7 @@ namespace Umbraco.Tests.Web.Controllers contentServiceMock.Setup(x => x.GetById(0)).Returns(content); var contentService = contentServiceMock.Object; var userServiceMock = new Mock(); - var permissions = new List(); + var permissions = new EntityPermissionCollection(); var permissionSet = new EntityPermissionSet(1234, permissions); userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1,1234,5678")).Returns(permissionSet); var userService = userServiceMock.Object; @@ -73,7 +73,7 @@ namespace Umbraco.Tests.Web.Controllers contentServiceMock.Setup(x => x.GetById(1234)).Returns(content); var contentService = contentServiceMock.Object; var userServiceMock = new Mock(); - var permissions = new List(); + var permissions = new EntityPermissionCollection(); var permissionSet = new EntityPermissionSet(1234, permissions); userServiceMock.Setup(x => x.GetPermissionsForPath(user, "-1,1234")).Returns(permissionSet); var userService = userServiceMock.Object; @@ -100,7 +100,7 @@ namespace Umbraco.Tests.Web.Controllers contentServiceMock.Setup(x => x.GetById(1234)).Returns(content); var contentService = contentServiceMock.Object; var userServiceMock = new Mock(); - var permissions = new List + var permissions = new EntityPermissionCollection { new EntityPermission(9876, 1234, new string[]{ "A", "B", "C" }) }; @@ -130,7 +130,7 @@ namespace Umbraco.Tests.Web.Controllers contentServiceMock.Setup(x => x.GetById(1234)).Returns(content); var contentService = contentServiceMock.Object; var userServiceMock = new Mock(); - var permissions = new List + var permissions = new EntityPermissionCollection { new EntityPermission(9876, 1234, new string[]{ "A", "F", "C" }) }; @@ -219,7 +219,7 @@ namespace Umbraco.Tests.Web.Controllers var user = userMock.Object; var userServiceMock = new Mock(); - var permissions = new List + var permissions = new EntityPermissionCollection { new EntityPermission(9876, 1234, new string[]{ "A" }) }; @@ -244,7 +244,7 @@ namespace Umbraco.Tests.Web.Controllers var user = userMock.Object; var userServiceMock = new Mock(); - var permissions = new List + var permissions = new EntityPermissionCollection { new EntityPermission(9876, 1234, new string[]{ "A" }) }; @@ -269,7 +269,7 @@ namespace Umbraco.Tests.Web.Controllers var user = userMock.Object; var userServiceMock = new Mock(); - var permissions = new List + var permissions = new EntityPermissionCollection { new EntityPermission(9876, 1234, new string[]{ "A" }) }; @@ -295,7 +295,7 @@ namespace Umbraco.Tests.Web.Controllers var user = userMock.Object; var userServiceMock = new Mock(); - var permissions = new List + var permissions = new EntityPermissionCollection { new EntityPermission(9876, 1234, new string[]{ "A" }) }; diff --git a/src/Umbraco.Tests/Web/Controllers/FilterAllowedOutgoingContentAttributeTests.cs b/src/Umbraco.Tests/Web/Controllers/FilterAllowedOutgoingContentAttributeTests.cs index 49ce356324..5afce04a1a 100644 --- a/src/Umbraco.Tests/Web/Controllers/FilterAllowedOutgoingContentAttributeTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/FilterAllowedOutgoingContentAttributeTests.cs @@ -110,7 +110,7 @@ namespace Umbraco.Tests.Web.Controllers var userServiceMock = new Mock(); //we're only assigning 3 nodes browse permissions so that is what we expect as a result - var permissions = new List + var permissions = new EntityPermissionCollection { new EntityPermission(9876, 1, new string[]{ "F" }), new EntityPermission(9876, 2, new string[]{ "F" }), From 76f9e2c22a0348180a3462b19b1643a374f95756 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 13 Jul 2017 10:43:42 +1000 Subject: [PATCH 06/11] Updates user permissions lookup to do one query instead of one query per user group --- .../Interfaces/IUserGroupRepository.cs | 8 +-- .../Repositories/PermissionRepository.cs | 54 +++++++++++-------- .../Repositories/UserGroupRepository.cs | 32 ++++++----- src/Umbraco.Core/Services/UserService.cs | 18 +++---- 4 files changed, 63 insertions(+), 49 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs index ec5fe79065..dd6188e31c 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Interfaces/IUserGroupRepository.cs @@ -28,17 +28,17 @@ namespace Umbraco.Core.Persistence.Repositories /// /// Gets explicilty defined permissions for the group for specified entities /// - /// Id of group + /// /// Array of entity Ids, if empty will return permissions for the group for all entities - EntityPermissionCollection GetPermissionsForEntities(int groupId, params int[] entityIds); + EntityPermissionCollection GetPermissions(int[] groupIds, params int[] entityIds); /// /// Gets explicilt and default permissions (if requested) permissions for the group for specified entities /// - /// The group + /// /// If true will include the group's default permissions if no permissions are explicitly assigned /// Array of entity Ids, if empty will return permissions for the group for all entities - EntityPermissionCollection GetPermissionsForEntities(IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds); + EntityPermissionCollection GetPermissions(IReadOnlyUserGroup[] groups, bool fallbackToDefaultPermissions, params int[] nodeIds); /// /// Replaces the same permission set for a single group to any number of entities diff --git a/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs b/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs index a22b5c5437..7dc751040d 100644 --- a/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs @@ -39,43 +39,53 @@ namespace Umbraco.Core.Persistence.Repositories /// /// Returns explicitly defined permissions for a user group for any number of nodes /// - /// + /// + /// The group ids to lookup permissions for + /// /// /// - public EntityPermissionCollection GetPermissionsForEntities(int groupId, params int[] entityIds) + /// + /// This method will not support passing in more than 2000 group Ids + /// + public EntityPermissionCollection GetPermissionsForEntities(int[] groupIds, params int[] entityIds) { var result = new EntityPermissionCollection(); - if (entityIds.Length == 0) + foreach (var groupOfGroupIds in groupIds.InGroupsOf(2000)) { - var sql = new Sql(); - sql.Select("*") - .From(SqlSyntax) - .Where(dto => dto.UserGroupId == groupId, SqlSyntax); - var permissions = UnitOfWork.Database.Fetch(sql); - foreach (var permission in ConvertToPermissionList(permissions)) - { - result.Add(permission); - } - } - else - { - //iterate in groups of 2000 since we don't want to exceed the max SQL param count - foreach (var groupOfIds in entityIds.InGroupsOf(2000)) - { - var ids = groupOfIds; + //copy local + var localIds = groupOfGroupIds.ToArray(); - var sql = new Sql(); + if (entityIds.Length == 0) + { + var sql = new Sql(); sql.Select("*") .From(SqlSyntax) - .Where(dto => dto.UserGroupId == groupId && ids.Contains(dto.NodeId), SqlSyntax); + .Where(dto => localIds.Contains(dto.UserGroupId), SqlSyntax); var permissions = UnitOfWork.Database.Fetch(sql); foreach (var permission in ConvertToPermissionList(permissions)) { result.Add(permission); } + } + else + { + //iterate in groups of 2000 since we don't want to exceed the max SQL param count + foreach (var groupOfEntityIds in entityIds.InGroupsOf(2000)) + { + var ids = groupOfEntityIds; + var sql = new Sql(); + sql.Select("*") + .From(SqlSyntax) + .Where(dto => localIds.Contains(dto.UserGroupId) && ids.Contains(dto.NodeId), SqlSyntax); + var permissions = UnitOfWork.Database.Fetch(sql); + foreach (var permission in ConvertToPermissionList(permissions)) + { + result.Add(permission); + } + } } - } + } return result; } diff --git a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs index 619d896fde..dec6148bb8 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs @@ -91,42 +91,46 @@ namespace Umbraco.Core.Persistence.Repositories _userGroupWithUsersRepository.AddOrUpdate(new UserGroupWithUsers(userGroup, userIds)); } - + /// /// Gets explicilty defined permissions for the group for specified entities /// - /// Id of group + /// /// Array of entity Ids, if empty will return permissions for the group for all entities - public EntityPermissionCollection GetPermissionsForEntities(int groupId, params int[] entityIds) + public EntityPermissionCollection GetPermissions(int[] groupIds, params int[] entityIds) { - return _permissionRepository.GetPermissionsForEntities(groupId, entityIds); + return _permissionRepository.GetPermissionsForEntities(groupIds, entityIds); } /// /// Gets explicilt and default permissions (if requested) permissions for the group for specified entities /// - /// The group + /// /// If true will include the group's default permissions if no permissions are explicitly assigned /// Array of entity Ids, if empty will return permissions for the group for all entities - public EntityPermissionCollection GetPermissionsForEntities(IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) + public EntityPermissionCollection GetPermissions(IReadOnlyUserGroup[] groups, bool fallbackToDefaultPermissions, params int[] nodeIds) { - if (group == null) throw new ArgumentNullException("group"); + if (groups == null) throw new ArgumentNullException("groups"); - var explicitPermissions = GetPermissionsForEntities(group.Id, nodeIds); + var groupIds = groups.Select(x => x.Id).ToArray(); + var explicitPermissions = GetPermissions(groupIds, nodeIds); var result = new EntityPermissionCollection(explicitPermissions); // If requested, and no permissions are assigned to a particular node, then we will fill in those permissions with the group's defaults if (fallbackToDefaultPermissions) { - var missingIds = nodeIds.Except(result.Select(x => x.EntityId)).ToArray(); - if (missingIds.Length > 0) + foreach (var group in groups) { - foreach (var permission in missingIds - .Select(i => new EntityPermission(group.Id, i, group.Permissions.ToArray(), isDefaultPermissions: true))) + var missingIds = nodeIds.Except(result.Select(x => x.EntityId)).ToArray(); + if (missingIds.Length > 0) { - result.Add(permission); + foreach (var permission in missingIds + .Select(i => new EntityPermission(group.Id, i, group.Permissions.ToArray(), isDefaultPermissions: true))) + { + result.Add(permission); + } } - } + } } return result; } diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index 149b37cbec..0d31e9d281 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -847,19 +847,19 @@ namespace Umbraco.Core.Services /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of public EntityPermissionCollection GetPermissions(IUser user, params int[] nodeIds) - { - //TODO: we don't need to run this query for each group assigned, we can do this in one query - + { var result = new EntityPermissionCollection(); - foreach (var group in user.Groups) + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { - foreach (var permission in GetPermissions(group, true, nodeIds)) + var repository = RepositoryFactory.CreateUserGroupRepository(uow); + + foreach (var permission in repository.GetPermissions(user.Groups.ToArray(), true, nodeIds)) { result.Add(permission); } - } - + } + return result; } @@ -879,7 +879,7 @@ namespace Umbraco.Core.Services using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateUserGroupRepository(uow); - return repository.GetPermissionsForEntities(group, fallbackToDefaultPermissions, nodeIds); + return repository.GetPermissions(new[] { group }, fallbackToDefaultPermissions, nodeIds); } } @@ -898,7 +898,7 @@ namespace Umbraco.Core.Services using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateUserGroupRepository(uow); - return repository.GetPermissionsForEntities(group.ToReadOnlyGroup(), fallbackToDefaultPermissions, nodeIds); + return repository.GetPermissions(new[] { group.ToReadOnlyGroup() }, fallbackToDefaultPermissions, nodeIds); } } From c472463f37d7b39aefb25ef360c1c9639917d44a Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 13 Jul 2017 11:02:34 +1000 Subject: [PATCH 07/11] Removes all user permissions cache refreshing and events and obsoletes assocated clsses --- src/Umbraco.Core/Cache/CacheKeys.cs | 4 +- src/Umbraco.Core/Models/Content.cs | 12 +-- .../Repositories/PermissionRepository.cs | 22 +---- .../Cache/CacheRefresherEventHandler.cs | 92 +------------------ .../Cache/DistributedCacheExtensions.cs | 18 ---- .../Cache/UserGroupCacheRefresher.cs | 18 +--- .../UserGroupPermissionsCacheRefresher.cs | 36 +------- 7 files changed, 18 insertions(+), 184 deletions(-) diff --git a/src/Umbraco.Core/Cache/CacheKeys.cs b/src/Umbraco.Core/Cache/CacheKeys.cs index f41bc92053..c9991ba45a 100644 --- a/src/Umbraco.Core/Cache/CacheKeys.cs +++ b/src/Umbraco.Core/Cache/CacheKeys.cs @@ -55,7 +55,9 @@ namespace Umbraco.Core.Cache [Obsolete("This is no longer used and will be removed from the codebase in the future")] [EditorBrowsable(EditorBrowsableState.Never)] public const string UserCacheKey = "UmbracoUser"; - + + [Obsolete("This is no longer used and will be removed from the codebase in the future")] + [EditorBrowsable(EditorBrowsableState.Never)] public const string UserGroupPermissionsCacheKey = "UmbracoUserGroupPermissions"; [UmbracoWillObsolete("This cache key is only used for legacy business logic caching, remove in v8")] diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index cef06ec4f7..b3fbe4164b 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -87,7 +87,6 @@ namespace Umbraco.Core.Models public readonly PropertyInfo ExpireDateSelector = ExpressionHelper.GetPropertyInfo(x => x.ExpireDate); public readonly PropertyInfo WriterSelector = ExpressionHelper.GetPropertyInfo(x => x.WriterId); public readonly PropertyInfo NodeNameSelector = ExpressionHelper.GetPropertyInfo(x => x.NodeName); - public readonly PropertyInfo PermissionsChangedSelector = ExpressionHelper.GetPropertyInfo(x => x.PermissionsChanged); } /// @@ -196,16 +195,7 @@ namespace Umbraco.Core.Models get { return _nodeName; } set { SetPropertyValueAndDetectChanges(value, ref _nodeName, Ps.Value.NodeNameSelector); } } - - /// - /// Used internally to track if permissions have been changed during the saving process for this entity - /// - [IgnoreDataMember] - internal bool PermissionsChanged - { - get { return _permissionsChanged; } - set { SetPropertyValueAndDetectChanges(value, ref _permissionsChanged, Ps.Value.PermissionsChangedSelector); } - } + /// /// Gets the ContentType used by this content object diff --git a/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs b/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs index 7dc751040d..7dc6996b57 100644 --- a/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs @@ -159,9 +159,6 @@ namespace Umbraco.Core.Persistence.Repositories } UnitOfWork.Database.BulkInsertRecords(toInsert, SqlSyntax); - - //Raise the event - UnitOfWork.Events.Dispatch(AssignedPermissions, this, new SaveEventArgs(ConvertToPermissionList(toInsert), false)); } @@ -191,10 +188,7 @@ namespace Umbraco.Core.Persistence.Repositories }).ToArray(); UnitOfWork.Database.BulkInsertRecords(actions, SqlSyntax); - - //Raise the event - UnitOfWork.Events.Dispatch(AssignedPermissions, this, new SaveEventArgs(ConvertToPermissionList(actions), false)); - + } /// @@ -223,10 +217,7 @@ namespace Umbraco.Core.Persistence.Repositories }).ToArray(); UnitOfWork.Database.BulkInsertRecords(actions, SqlSyntax); - - //Raise the event - UnitOfWork.Events.Dispatch(AssignedPermissions, this, new SaveEventArgs(ConvertToPermissionList(actions), false)); - + } /// @@ -258,11 +249,7 @@ namespace Umbraco.Core.Persistence.Repositories } UnitOfWork.Database.BulkInsertRecords(toInsert, SqlSyntax); - - //Raise the event - UnitOfWork.Events.Dispatch(AssignedPermissions, this, new SaveEventArgs(permissionSet.PermissionsSet, false)); - - + } @@ -350,7 +337,6 @@ namespace Umbraco.Core.Persistence.Repositories return permissions; } - - public static event TypedEventHandler, SaveEventArgs> AssignedPermissions; + } } \ No newline at end of file diff --git a/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs b/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs index 8e628204d8..daf405a562 100644 --- a/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs +++ b/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs @@ -111,22 +111,8 @@ namespace Umbraco.Web.Cache Bind(() => MemberTypeService.Saved += MemberTypeService_Saved, () => MemberTypeService.Saved -= MemberTypeService_Saved); Bind(() => MemberTypeService.Deleted += MemberTypeService_Deleted, - () => MemberTypeService.Deleted -= MemberTypeService_Deleted); - - // bind to permission events - // we should wrap legacy permissions so we can get rid of this - // fixme - the method names here (PermissionNew...) are not supported - // by the event handling mechanism for scopes and deploy, and not sure - // how to fix with the generic repository - Bind(() => Permission.New += PermissionNew, - () => Permission.New -= PermissionNew); - Bind(() => Permission.Updated += PermissionUpdated, - () => Permission.Updated -= PermissionUpdated); - Bind(() => Permission.Deleted += PermissionDeleted, - () => Permission.Deleted -= PermissionDeleted); - Bind(() => PermissionRepository.AssignedPermissions += CacheRefresherEventHandler_AssignedPermissions, - () => PermissionRepository.AssignedPermissions -= CacheRefresherEventHandler_AssignedPermissions); - + () => MemberTypeService.Deleted -= MemberTypeService_Deleted); + // bind to template events Bind(() => FileService.SavedTemplate += FileService_SavedTemplate, () => FileService.SavedTemplate -= FileService_SavedTemplate); @@ -360,13 +346,6 @@ namespace Umbraco.Web.Cache /// static void ContentService_Copied(IContentService sender, CopyEventArgs e) { - //check if permissions have changed - var permissionsChanged = ((Content)e.Copy).WasPropertyDirty("PermissionsChanged"); - if (permissionsChanged) - { - DistributedCache.Instance.RefreshAllUserGroupPermissionsCache(); - } - //run the un-published cache refresher since copied content is not published DistributedCache.Instance.RefreshUnpublishedPageCache(e.Copy); } @@ -388,33 +367,10 @@ namespace Umbraco.Web.Cache /// /// /// When an entity is saved we need to notify other servers about the change in order for the Examine indexes to - /// stay up-to-date for unpublished content. - /// - /// When an entity is created new permissions may be assigned to it based on it's parent, if that is the - /// case then we need to clear all user group permissions cache. + /// stay up-to-date for unpublished content. /// static void ContentService_Saved(IContentService sender, SaveEventArgs e) { - var clearUserGroupPermissions = false; - e.SavedEntities.ForEach(x => - { - //check if it is new - if (x.IsNewEntity()) - { - //check if permissions have changed - var permissionsChanged = ((Content)x).WasPropertyDirty("PermissionsChanged"); - if (permissionsChanged) - { - clearUserGroupPermissions = true; - } - } - }); - - if (clearUserGroupPermissions) - { - DistributedCache.Instance.RefreshAllUserGroupPermissionsCache(); - } - //filter out the entities that have only been saved (not newly published) since // newly published ones will be synced with the published page cache refresher var unpublished = e.SavedEntities.Where(x => x.JustPublished() == false); @@ -598,30 +554,7 @@ namespace Umbraco.Web.Cache #endregion #region User/permissions event handlers - - static void CacheRefresherEventHandler_AssignedPermissions(PermissionRepository sender, SaveEventArgs e) - { - var groupIds = e.SavedEntities.Select(x => x.UserGroupId).Distinct(); - foreach (var groupId in groupIds) - { - DistributedCache.Instance.RefreshUserGroupPermissionsCache(groupId); - } - } - - static void PermissionDeleted(UserGroupPermission sender, DeleteEventArgs e) - { - InvalidateCacheForPermissionsChange(sender); - } - - static void PermissionUpdated(UserGroupPermission sender, SaveEventArgs e) - { - InvalidateCacheForPermissionsChange(sender); - } - - static void PermissionNew(UserGroupPermission sender, NewEventArgs e) - { - InvalidateCacheForPermissionsChange(sender); - } + static void UserService_SavedUser(IUserService sender, SaveEventArgs e) { @@ -642,22 +575,7 @@ namespace Umbraco.Web.Cache { e.DeletedEntities.ForEach(x => DistributedCache.Instance.RemoveUserGroupCache(x.Id)); } - - private static void InvalidateCacheForPermissionsChange(UserGroupPermission sender) - { - if (sender.UserGroup != null) - { - DistributedCache.Instance.RefreshUserGroupPermissionsCache(sender.UserGroup.Id); - } - else if (sender.UserId > -1) - { - DistributedCache.Instance.RefreshUserGroupPermissionsCache(sender.UserId); - } - else if (sender.NodeIds.Any()) - { - DistributedCache.Instance.RefreshAllUserGroupPermissionsCache(); - } - } + #endregion diff --git a/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs b/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs index f8e8b412c5..397410dbd5 100644 --- a/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs +++ b/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs @@ -79,24 +79,6 @@ namespace Umbraco.Web.Cache #endregion - #region User group permissions cache - - public static void RemoveUserGroupPermissionsCache(this DistributedCache dc, int groupId) - { - dc.Remove(DistributedCache.UserGroupPermissionsCacheRefresherGuid, groupId); - } - - public static void RefreshUserGroupPermissionsCache(this DistributedCache dc, int groupId) - { - dc.Refresh(DistributedCache.UserGroupPermissionsCacheRefresherGuid, groupId); - } - - public static void RefreshAllUserGroupPermissionsCache(this DistributedCache dc) - { - dc.RefreshAll(DistributedCache.UserGroupPermissionsCacheRefresherGuid); - } - - #endregion #region Template cache diff --git a/src/Umbraco.Web/Cache/UserGroupCacheRefresher.cs b/src/Umbraco.Web/Cache/UserGroupCacheRefresher.cs index e6447d1f45..84496b61fd 100644 --- a/src/Umbraco.Web/Cache/UserGroupCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/UserGroupCacheRefresher.cs @@ -34,10 +34,6 @@ namespace Umbraco.Web.Cache { userGroupCache.Result.ClearCacheByKeySearch(UserGroupRepository.GetByAliasCacheKeyPrefix); } - if (UserGroupPermissionsCache) - { - UserGroupPermissionsCache.Result.ClearCacheByKeySearch(CacheKeys.UserGroupPermissionsCacheKey); - } base.RefreshAll(); } @@ -56,20 +52,8 @@ namespace Umbraco.Web.Cache userGroupCache.Result.ClearCacheItem(RepositoryBase.GetCacheIdKey(id)); userGroupCache.Result.ClearCacheByKeySearch(UserGroupRepository.GetByAliasCacheKeyPrefix); } - - if (UserGroupPermissionsCache) - { - //TODO: Is this good enough for all users attached to this? - var keyStartsWith = string.Format("{0}{1}", CacheKeys.UserGroupPermissionsCacheKey, id); - UserGroupPermissionsCache.Result.ClearCacheByKeySearch(keyStartsWith); - } - + base.Remove(id); } - - private Attempt UserGroupPermissionsCache - { - get { return ApplicationContext.Current.ApplicationCache.IsolatedRuntimeCache.GetCache(); } - } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Cache/UserGroupPermissionsCacheRefresher.cs b/src/Umbraco.Web/Cache/UserGroupPermissionsCacheRefresher.cs index 8ea1b26f47..405da0b306 100644 --- a/src/Umbraco.Web/Cache/UserGroupPermissionsCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/UserGroupPermissionsCacheRefresher.cs @@ -1,17 +1,13 @@ using System; +using System.ComponentModel; using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Models.Membership; namespace Umbraco.Web.Cache { - /// - /// Used only to invalidate the user permissions cache - /// - /// - /// The UserGroupCacheRefresher will also clear a groups's permissions cache, this refresher is for invalidating only permissions - /// for user groups/content, not the groups themselves. - /// + [Obsolete("This is no longer used and will be removed from the codebase in the future")] + [EditorBrowsable(EditorBrowsableState.Never)] public sealed class UserGroupPermissionsCacheRefresher : CacheRefresherBase { protected override UserGroupPermissionsCacheRefresher Instance @@ -29,30 +25,6 @@ namespace Umbraco.Web.Cache { get { return "User group permissions cache refresher"; } } - - public override void RefreshAll() - { - if (UserGroupPermissionsCache) - UserGroupPermissionsCache.Result.ClearCacheByKeySearch(CacheKeys.UserGroupPermissionsCacheKey); - base.RefreshAll(); - } - - public override void Refresh(int id) - { - Remove(id); - base.Refresh(id); - } - - public override void Remove(int id) - { - if (UserGroupPermissionsCache) - UserGroupPermissionsCache.Result.ClearCacheByKeySearch(string.Format("{0}{1}", CacheKeys.UserGroupPermissionsCacheKey, id)); - base.Remove(id); - } - - private Attempt UserGroupPermissionsCache - { - get { return ApplicationContext.Current.ApplicationCache.IsolatedRuntimeCache.GetCache(); } - } + } } \ No newline at end of file From 69a6a80ffd99d614e38d7c65788bdf0f7d7dc2d1 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 13 Jul 2017 14:16:13 +1000 Subject: [PATCH 08/11] Updates logic and test for getting all node permissions for a user that have explicit permissions defined --- .../Repositories/UserGroupRepository.cs | 20 ++++++---- src/Umbraco.Core/Services/IUserService.cs | 4 +- src/Umbraco.Core/Services/UserService.cs | 11 +---- .../Services/UserServiceTests.cs | 40 +++++++++++++++---- src/umbraco.cms/businesslogic/Permission.cs | 6 +-- 5 files changed, 53 insertions(+), 28 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs index dec6148bb8..137cc0885b 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs @@ -119,16 +119,22 @@ namespace Umbraco.Core.Persistence.Repositories // If requested, and no permissions are assigned to a particular node, then we will fill in those permissions with the group's defaults if (fallbackToDefaultPermissions) { + //if no node ids are passed in, then we need to determine the node ids for the explicit permissions set + nodeIds = nodeIds.Length == 0 + ? explicitPermissions.Select(x => x.EntityId).Distinct().ToArray() + : nodeIds; + + //if there are still no nodeids we can just exit + if (nodeIds.Length == 0) + return result; + foreach (var group in groups) { - var missingIds = nodeIds.Except(result.Select(x => x.EntityId)).ToArray(); - if (missingIds.Length > 0) + foreach (var nodeId in nodeIds) { - foreach (var permission in missingIds - .Select(i => new EntityPermission(group.Id, i, group.Permissions.ToArray(), isDefaultPermissions: true))) - { - result.Add(permission); - } + var defaultPermission = new EntityPermission(group.Id, nodeId, group.Permissions.ToArray(), isDefaultPermissions: true); + //Since this is a hashset, this will not add anything that already exists by group/node combination + result.Add(defaultPermission); } } } diff --git a/src/Umbraco.Core/Services/IUserService.cs b/src/Umbraco.Core/Services/IUserService.cs index 5f1d16d6bc..5ae31f8c0d 100644 --- a/src/Umbraco.Core/Services/IUserService.cs +++ b/src/Umbraco.Core/Services/IUserService.cs @@ -90,10 +90,10 @@ namespace Umbraco.Core.Services /// /// If no permissions are found for a particular entity then the user's default permissions will be applied /// User to retrieve permissions for - /// Specifiying nothing will return all user permissions for all nodes + /// Specifiying nothing will return all user permissions for all nodes that have explicit permissions defined /// An enumerable list of /// - /// This will return the default permissions for the user's groups for nodes that don't have explicitly defined permissions + /// This will return the default permissions for the user's groups for node ids that don't have explicitly defined permissions /// EntityPermissionCollection GetPermissions(IUser user, params int[] nodeIds); diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index 0d31e9d281..24ce233cc2 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -848,19 +848,12 @@ namespace Umbraco.Core.Services /// An enumerable list of public EntityPermissionCollection GetPermissions(IUser user, params int[] nodeIds) { - var result = new EntityPermissionCollection(); - using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateUserGroupRepository(uow); - foreach (var permission in repository.GetPermissions(user.Groups.ToArray(), true, nodeIds)) - { - result.Add(permission); - } - } - - return result; + return repository.GetPermissions(user.Groups.ToArray(), true, nodeIds); + } } /// diff --git a/src/Umbraco.Tests/Services/UserServiceTests.cs b/src/Umbraco.Tests/Services/UserServiceTests.cs index 610a6498c8..cdfff6ff0d 100644 --- a/src/Umbraco.Tests/Services/UserServiceTests.cs +++ b/src/Umbraco.Tests/Services/UserServiceTests.cs @@ -164,16 +164,20 @@ namespace Umbraco.Tests.Services } [Test] - public void Get_All_User_Permissions_For_All_Nodes() + public void Get_All_User_Permissions_For_All_Nodes_With_Explicit_Permission() { // Arrange var userService = ServiceContext.UserService; var userGroup1 = CreateTestUserGroup(); var userGroup2 = CreateTestUserGroup("test2", "Test 2"); - var user = userService.CreateUserWithIdentity("John Doe", "john@umbraco.io"); + var userGroup3 = CreateTestUserGroup("test3", "Test 3"); + var user = userService.CreateUserWithIdentity("John Doe", "john@umbraco.io"); + + var defaultPermissionCount = userGroup3.Permissions.Count(); user.AddGroup(userGroup1); user.AddGroup(userGroup2); + user.AddGroup(userGroup3); userService.Save(user); var contentType = MockedContentTypes.CreateSimpleContentType(); @@ -184,7 +188,8 @@ namespace Umbraco.Tests.Services MockedContent.CreateSimpleContent(contentType), MockedContent.CreateSimpleContent(contentType) }; - ServiceContext.ContentService.Save(content); + ServiceContext.ContentService.Save(content); + //assign permissions - we aren't assigning anything explicit for group3 and nothing explicit for content[2] /w group2 ServiceContext.ContentService.AssignContentPermission(content[0], ActionBrowse.Instance.Letter, new int[] { userGroup1.Id }); ServiceContext.ContentService.AssignContentPermission(content[0], ActionDelete.Instance.Letter, new int[] { userGroup1.Id }); ServiceContext.ContentService.AssignContentPermission(content[0], ActionMove.Instance.Letter, new int[] { userGroup2.Id }); @@ -199,22 +204,43 @@ namespace Umbraco.Tests.Services .GroupBy(x => x.EntityId) .ToDictionary(x => x.Key, x => x.GroupBy(a => a.UserGroupId).ToDictionary(a => a.Key, a => a.ToArray())); - //assert - Assert.AreEqual(3, permissions.Count); - Assert.IsTrue(permissions.ContainsKey(content[0].Id)); + //assert + + //there will be 3 since that is how many content items there are + Assert.AreEqual(3, permissions.Count); + + //test permissions contains content[0] + Assert.IsTrue(permissions.ContainsKey(content[0].Id)); + //test that this permissions set contains permissions for all groups Assert.IsTrue(permissions[content[0].Id].ContainsKey(userGroup1.Id)); Assert.IsTrue(permissions[content[0].Id].ContainsKey(userGroup2.Id)); + Assert.IsTrue(permissions[content[0].Id].ContainsKey(userGroup3.Id)); + //test that the correct number of permissions are returned for each group Assert.AreEqual(2, permissions[content[0].Id][userGroup1.Id].SelectMany(x => x.AssignedPermissions).Count()); Assert.AreEqual(1, permissions[content[0].Id][userGroup2.Id].SelectMany(x => x.AssignedPermissions).Count()); + Assert.AreEqual(defaultPermissionCount, permissions[content[0].Id][userGroup3.Id].SelectMany(x => x.AssignedPermissions).Count()); + + //test permissions contains content[1] Assert.IsTrue(permissions.ContainsKey(content[1].Id)); + //test that this permissions set contains permissions for all groups Assert.IsTrue(permissions[content[1].Id].ContainsKey(userGroup1.Id)); Assert.IsTrue(permissions[content[1].Id].ContainsKey(userGroup2.Id)); + Assert.IsTrue(permissions[content[1].Id].ContainsKey(userGroup3.Id)); + //test that the correct number of permissions are returned for each group Assert.AreEqual(1, permissions[content[1].Id][userGroup1.Id].SelectMany(x => x.AssignedPermissions).Count()); Assert.AreEqual(1, permissions[content[1].Id][userGroup2.Id].SelectMany(x => x.AssignedPermissions).Count()); + Assert.AreEqual(defaultPermissionCount, permissions[content[1].Id][userGroup3.Id].SelectMany(x => x.AssignedPermissions).Count()); + + //test permissions contains content[2] Assert.IsTrue(permissions.ContainsKey(content[2].Id)); + //test that this permissions set contains permissions for all groups Assert.IsTrue(permissions[content[2].Id].ContainsKey(userGroup1.Id)); - Assert.IsFalse(permissions[content[2].Id].ContainsKey(userGroup2.Id)); + Assert.IsTrue(permissions[content[2].Id].ContainsKey(userGroup2.Id)); + Assert.IsTrue(permissions[content[2].Id].ContainsKey(userGroup3.Id)); + //test that the correct number of permissions are returned for each group Assert.AreEqual(1, permissions[content[2].Id][userGroup1.Id].SelectMany(x => x.AssignedPermissions).Count()); + Assert.AreEqual(defaultPermissionCount, permissions[content[2].Id][userGroup2.Id].SelectMany(x => x.AssignedPermissions).Count()); + Assert.AreEqual(defaultPermissionCount, permissions[content[2].Id][userGroup3.Id].SelectMany(x => x.AssignedPermissions).Count()); } [Test] diff --git a/src/umbraco.cms/businesslogic/Permission.cs b/src/umbraco.cms/businesslogic/Permission.cs index 4bbe4604a4..335535c08d 100644 --- a/src/umbraco.cms/businesslogic/Permission.cs +++ b/src/umbraco.cms/businesslogic/Permission.cs @@ -9,15 +9,15 @@ using Umbraco.Core.Events; using umbraco.DataLayer; using umbraco.cms.businesslogic; using System.Collections.Generic; +using System.ComponentModel; using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; using DeleteEventArgs = umbraco.cms.businesslogic.DeleteEventArgs; namespace umbraco.BusinessLogic { - /// - /// Summary description for Permission. - /// + [Obsolete("This is no longer used and will be removed in future versions, use the IUserService to manage permissions for Users")] + [EditorBrowsable(EditorBrowsableState.Never)] public class Permission { From 6e892020024cbbbbeabbf556dcea55604fd7d645 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 13 Jul 2017 14:32:43 +1000 Subject: [PATCH 09/11] Fixes usages of IUserService.GetPermission since before it was doing a FirstOrDefault() since that's just how the old APIs worked, now we need to get all permissions based on groups. Also updated the usages of ToString for char for the permissions to be culture invariant everywhere. --- src/Umbraco.Web/Editors/ContentController.cs | 6 +++--- .../Models/ContentEditing/ContentItemDisplay.cs | 2 +- .../Models/Mapping/ContentModelMapper.cs | 17 +++++++---------- .../FilterAllowedOutgoingContentAttribute.cs | 2 +- .../umbraco/Trees/BaseContentTree.cs | 2 +- .../umbraco/dialogs/moveOrCopy.aspx.cs | 4 ++-- .../umbraco/dialogs/notifications.aspx.cs | 3 ++- .../umbraco/templateControls/Item.cs | 3 ++- src/umbraco.cms/Actions/Action.cs | 3 ++- 9 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index f935673317..b624041992 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -242,7 +242,7 @@ namespace Umbraco.Web.Editors //set a custom path since the tree that renders this has the content type id as the parent content.Path = string.Format("-1,{0},{1}", persistedContent.ContentTypeId, content.Id); - content.AllowedActions = new[] {'A'}; + content.AllowedActions = new[] {"A"}; var excludeProps = new[] {"_umb_urls", "_umb_releasedate", "_umb_expiredate", "_umb_template"}; var propsTab = content.Tabs.Last(); @@ -440,8 +440,8 @@ namespace Umbraco.Web.Editors [HttpGet] public bool HasPermission(string permissionToCheck, int nodeId) { - var p = Services.UserService.GetPermissions(Security.CurrentUser, nodeId).FirstOrDefault(); - if (p != null && p.AssignedPermissions.Contains(permissionToCheck.ToString(CultureInfo.InvariantCulture))) + var p = Services.UserService.GetPermissions(Security.CurrentUser, nodeId).GetAllPermissions(); + if (p.Contains(permissionToCheck.ToString(CultureInfo.InvariantCulture))) { return true; } diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs b/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs index 2615c8774d..c416f30a48 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentItemDisplay.cs @@ -56,7 +56,7 @@ namespace Umbraco.Web.Models.ContentEditing /// Each char represents a button which we can then map on the front-end to the correct actions /// [DataMember(Name = "allowedActions")] - public IEnumerable AllowedActions { get; set; } + public IEnumerable AllowedActions { get; set; } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Models/Mapping/ContentModelMapper.cs b/src/Umbraco.Web/Models/Mapping/ContentModelMapper.cs index a33921b7b9..3b55def135 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentModelMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentModelMapper.cs @@ -135,7 +135,7 @@ namespace Umbraco.Web.Models.Mapping Label = localizedText.Localize("content/releaseDate"), Value = display.ReleaseDate.HasValue ? display.ReleaseDate.Value.ToIsoString() : null, //Not editible for people without publish permission (U4-287) - View = display.AllowedActions.Contains(ActionPublish.Instance.Letter) ? "datepicker" : PropertyEditorResolver.Current.GetByAlias(Constants.PropertyEditors.NoEditAlias).ValueEditor.View, + View = display.AllowedActions.Contains(ActionPublish.Instance.Letter.ToString(CultureInfo.InvariantCulture)) ? "datepicker" : PropertyEditorResolver.Current.GetByAlias(Constants.PropertyEditors.NoEditAlias).ValueEditor.View, Config = new Dictionary { {"offsetTime", "1"} @@ -148,7 +148,7 @@ namespace Umbraco.Web.Models.Mapping Label = localizedText.Localize("content/unpublishDate"), Value = display.ExpireDate.HasValue ? display.ExpireDate.Value.ToIsoString() : null, //Not editible for people without publish permission (U4-287) - View = display.AllowedActions.Contains(ActionPublish.Instance.Letter) ? "datepicker" : PropertyEditorResolver.Current.GetByAlias(Constants.PropertyEditors.NoEditAlias).ValueEditor.View, + View = display.AllowedActions.Contains(ActionPublish.Instance.Letter.ToString(CultureInfo.InvariantCulture)) ? "datepicker" : PropertyEditorResolver.Current.GetByAlias(Constants.PropertyEditors.NoEditAlias).ValueEditor.View, Config = new Dictionary { {"offsetTime", "1"} @@ -213,10 +213,9 @@ namespace Umbraco.Web.Models.Mapping } /// - //TODO: This is horribly inneficient /// Creates the list of action buttons allowed for this user - Publish, Send to publish, save, unpublish returned as the button's 'letter' /// - private class ActionButtonsResolver : ValueResolver> + private class ActionButtonsResolver : ValueResolver> { private readonly Lazy _userService; @@ -225,12 +224,12 @@ namespace Umbraco.Web.Models.Mapping _userService = userService; } - protected override IEnumerable ResolveCore(IContent source) + protected override IEnumerable ResolveCore(IContent source) { if (UmbracoContext.Current == null) { //cannot check permissions without a context - return Enumerable.Empty(); + return Enumerable.Empty(); } var svc = _userService.Value; @@ -242,11 +241,9 @@ namespace Umbraco.Web.Models.Mapping // Here we need to do a special check since this could be new content, in which case we need to get the permissions // from the parent, not the existing one otherwise permissions would be coming from the root since Id is 0. source.HasIdentity ? source.Id : source.ParentId) - .FirstOrDefault(); + .GetAllPermissions(); - return permissions == null - ? Enumerable.Empty() - : permissions.AssignedPermissions.Where(x => x.Length == 1).Select(x => x.ToUpperInvariant()[0]); + return permissions; } } } diff --git a/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingContentAttribute.cs b/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingContentAttribute.cs index ffd6bf362e..6091392321 100644 --- a/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingContentAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingContentAttribute.cs @@ -74,7 +74,7 @@ namespace Umbraco.Web.WebApi.Filters var nodePermission = permissions.Where(x => x.EntityId == Convert.ToInt32(item.Id)).ToArray(); //if there are no permissions for this id then we need to check what the user's default // permissions are. - if (nodePermission.Any() == false) + if (nodePermission.Length == 0) { //var defaultP = user.DefaultPermissions diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseContentTree.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseContentTree.cs index 1eacc072bd..e448d9653a 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseContentTree.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/Trees/BaseContentTree.cs @@ -360,7 +360,7 @@ function openContent(id) { } else if (!this.IsDialog || (this.DialogMode == TreeDialogModes.id)) { - if (CurrentUser.GetPermissions(dd.Path).Contains(ActionUpdate.Instance.Letter.ToString())) + if (CurrentUser.GetPermissions(dd.Path).Contains(ActionUpdate.Instance.Letter.ToString(CultureInfo.InvariantCulture))) { treeElement.Action = String.Format("javascript:openContent({0});", dd.Id); } diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/moveOrCopy.aspx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/moveOrCopy.aspx.cs index 9aed597702..f8fc1d3ea3 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/moveOrCopy.aspx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/moveOrCopy.aspx.cs @@ -137,8 +137,8 @@ namespace umbraco.dialogs private bool CheckPermissions(IContentBase node, IAction currentAction) { var userService = ApplicationContext.Current.Services.UserService; - var currUserPermissions = userService.GetPermissions(UmbracoContext.Current.Security.CurrentUser, node.Id).FirstOrDefault(); - return currUserPermissions != null && currUserPermissions.AssignedPermissions.Contains(currentAction.Letter.ToString()); + var currUserPermissions = userService.GetPermissions(UmbracoContext.Current.Security.CurrentUser, node.Id).GetAllPermissions(); + return currUserPermissions != null && currUserPermissions.Contains(currentAction.Letter.ToString(CultureInfo.InvariantCulture)); } private void HandleDocumentTypeCopy() diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/notifications.aspx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/notifications.aspx.cs index 97dc8d84d3..70137da920 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/notifications.aspx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/dialogs/notifications.aspx.cs @@ -1,5 +1,6 @@ using System; using System.Collections; +using System.Globalization; using System.Web.UI; using System.Web.UI.HtmlControls; using System.Web.UI.WebControls; @@ -49,7 +50,7 @@ namespace umbraco.dialogs { CheckBox c = new CheckBox(); - c.ID = a.Letter.ToString(); + c.ID = a.Letter.ToString(CultureInfo.InvariantCulture); if (base.getUser().GetNotifications(node.Path).IndexOf(a.Letter) > -1) c.Checked = true; diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/templateControls/Item.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/templateControls/Item.cs index 6d0465f693..346008bda3 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/templateControls/Item.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/templateControls/Item.cs @@ -1,6 +1,7 @@ using System; using System.Collections; using System.ComponentModel; +using System.Globalization; using System.Web; using System.Web.UI; using System.Web.UI.WebControls; @@ -304,7 +305,7 @@ namespace umbraco.presentation.templateControls protected virtual bool FieldEditableWithUserPermissions() { BusinessLogic.User u = helper.GetCurrentUmbracoUser(); - return u != null && u.GetPermissions(PageElements["path"].ToString()).Contains(ActionUpdate.Instance.Letter.ToString()); + return u != null && u.GetPermissions(PageElements["path"].ToString()).Contains(ActionUpdate.Instance.Letter.ToString(CultureInfo.InvariantCulture)); } #endregion diff --git a/src/umbraco.cms/Actions/Action.cs b/src/umbraco.cms/Actions/Action.cs index 3a5503d714..81c78ade1b 100644 --- a/src/umbraco.cms/Actions/Action.cs +++ b/src/umbraco.cms/Actions/Action.cs @@ -1,6 +1,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Globalization; using System.Web; using System.Reflection; using Umbraco.Core; @@ -150,7 +151,7 @@ namespace umbraco.BusinessLogic.Actions /// public static string ToString(List actions) { - string[] strMenu = Array.ConvertAll(actions.ToArray(), delegate(IAction a) { return (a.Letter.ToString()); }); + string[] strMenu = Array.ConvertAll(actions.ToArray(), delegate(IAction a) { return (a.Letter.ToString(CultureInfo.InvariantCulture)); }); return string.Join("", strMenu); } From 1643eec62c432434ba37ad0581b0a838ed3d73a1 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 13 Jul 2017 18:47:10 +1000 Subject: [PATCH 10/11] Fixes cache refreshing for user groups since user cache need to be cleared for that too. Updates cache refreshers to use json.net for serializing which is way faster. adds better support for querying multiple user groups and entity ids for permissions, updates tests --- .../Models/Membership/EntityPermissionSet.cs | 6 +- .../Repositories/PermissionRepository.cs | 3 + .../Repositories/UserGroupRepository.cs | 8 +- src/Umbraco.Core/Services/IUserService.cs | 24 ++-- src/Umbraco.Core/Services/UserService.cs | 133 ++++++++++++------ .../Services/UserServiceExtensions.cs | 33 ++++- .../Services/ContentServiceTests.cs | 17 ++- .../Services/UserServiceTests.cs | 3 +- .../Cache/CacheRefresherEventHandler.cs | 27 +++- .../Cache/ContentTypeCacheRefresher.cs | 7 +- .../Cache/DataTypeCacheRefresher.cs | 7 +- .../Cache/DistributedCacheExtensions.cs | 19 +++ src/Umbraco.Web/Cache/MacroCacheRefresher.cs | 13 +- src/Umbraco.Web/Cache/MediaCacheRefresher.cs | 13 +- .../Cache/MemberGroupCacheRefresher.cs | 7 +- .../Cache/UnpublishedPageCacheRefresher.cs | 8 +- .../Cache/UserGroupCacheRefresher.cs | 12 +- .../Models/Mapping/UserModelMapper.cs | 2 +- src/umbraco.cms/businesslogic/Permission.cs | 132 +---------------- 19 files changed, 233 insertions(+), 241 deletions(-) diff --git a/src/Umbraco.Core/Models/Membership/EntityPermissionSet.cs b/src/Umbraco.Core/Models/Membership/EntityPermissionSet.cs index 0f35fec990..c33c4aa315 100644 --- a/src/Umbraco.Core/Models/Membership/EntityPermissionSet.cs +++ b/src/Umbraco.Core/Models/Membership/EntityPermissionSet.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; namespace Umbraco.Core.Models.Membership @@ -8,13 +9,14 @@ namespace Umbraco.Core.Models.Membership /// public class EntityPermissionSet { + private static readonly Lazy EmptyInstance = new Lazy(() => new EntityPermissionSet(-1, new EntityPermissionCollection())); /// /// Returns an empty permission set /// /// public static EntityPermissionSet Empty() { - return new EntityPermissionSet(-1, new EntityPermissionCollection()); + return EmptyInstance.Value; } public EntityPermissionSet(int entityId, EntityPermissionCollection permissionsSet) diff --git a/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs b/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs index 7dc6996b57..edf0274832 100644 --- a/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/PermissionRepository.cs @@ -135,6 +135,9 @@ namespace Umbraco.Core.Persistence.Repositories /// public void ReplacePermissions(int groupId, IEnumerable permissions, params int[] entityIds) { + if (entityIds.Length == 0) + return; + var db = UnitOfWork.Database; //we need to batch these in groups of 2000 so we don't exceed the max 2100 limit diff --git a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs index 137cc0885b..3d291d5e07 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserGroupRepository.cs @@ -132,6 +132,10 @@ namespace Umbraco.Core.Persistence.Repositories { foreach (var nodeId in nodeIds) { + //TODO: We could/should change the EntityPermissionsCollection into a KeyedCollection and they key could be + // a struct of the nodeid + groupid so then we don't actually allocate this class just to check if it's not + // going to be included in the result! + var defaultPermission = new EntityPermission(group.Id, nodeId, group.Permissions.ToArray(), isDefaultPermissions: true); //Since this is a hashset, this will not add anything that already exists by group/node combination result.Add(defaultPermission); @@ -145,8 +149,8 @@ namespace Umbraco.Core.Persistence.Repositories /// Replaces the same permission set for a single group to any number of entities /// /// Id of group - /// Permissions as enumerable list of - /// Specify the nodes to replace permissions for. If nothing is specified all permissions are removed. + /// Permissions as enumerable list of If nothing is specified all permissions are removed. + /// Specify the nodes to replace permissions for. public void ReplaceGroupPermissions(int groupId, IEnumerable permissions, params int[] entityIds) { _permissionRepository.ReplacePermissions(groupId, permissions, entityIds); diff --git a/src/Umbraco.Core/Services/IUserService.cs b/src/Umbraco.Core/Services/IUserService.cs index 5ae31f8c0d..ea56264075 100644 --- a/src/Umbraco.Core/Services/IUserService.cs +++ b/src/Umbraco.Core/Services/IUserService.cs @@ -95,35 +95,35 @@ namespace Umbraco.Core.Services /// /// This will return the default permissions for the user's groups for node ids that don't have explicitly defined permissions /// - EntityPermissionCollection GetPermissions(IUser user, params int[] nodeIds); - + EntityPermissionCollection GetPermissions(IUser user, params int[] nodeIds); + /// - /// Get explicitly assigned permissions for a group and optional node Ids + /// Get explicitly assigned permissions for groups and optional node Ids /// - /// Group to retrieve permissions for + /// /// - /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set + /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set /// /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of - EntityPermissionCollection GetPermissions(IUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds); + EntityPermissionCollection GetPermissions(IUserGroup[] groups, bool fallbackToDefaultPermissions, params int[] nodeIds); /// /// Gets the implicit/inherited permissions for the user for the given path /// /// User to check permissions for /// Path to check permissions for - EntityPermissionSet GetPermissionsForPath(IUser user, string path); - + EntityPermissionSet GetPermissionsForPath(IUser user, string path); + /// - /// Gets the permissions for the provided group and path + /// Gets the permissions for the provided groups and path /// - /// Group to check permissions for + /// /// Path to check permissions for /// - /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set + /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set /// - EntityPermission GetPermissionsForPath(IUserGroup group, string path, bool fallbackToDefaultPermissions = false); + EntityPermissionSet GetPermissionsForPath(IUserGroup[] groups, string path, bool fallbackToDefaultPermissions = false); /// /// Replaces the same permission set for a single group to any number of entities diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index 24ce233cc2..2d5fff536c 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Data.Common; +using System.Globalization; using System.Linq; using System.Linq.Expressions; using Umbraco.Core.Configuration; @@ -680,15 +681,26 @@ namespace Umbraco.Core.Services /// /// If no 'entityIds' are specified all permissions will be removed for the specified group. /// Id of the group - /// Permissions as enumerable list of - /// Specify the nodes to replace permissions for. If nothing is specified all permissions are removed. + /// Permissions as enumerable list of If nothing is specified all permissions are removed. + /// Specify the nodes to replace permissions for. public void ReplaceUserGroupPermissions(int groupId, IEnumerable permissions, params int[] entityIds) { + if (entityIds.Length == 0) + return; + using (var uow = UowProvider.GetUnitOfWork()) { var repository = RepositoryFactory.CreateUserGroupRepository(uow); repository.ReplaceGroupPermissions(groupId, permissions, entityIds); uow.Commit(); + + uow.Events.Dispatch(UserGroupPermissionsAssigned, this, new SaveEventArgs( + entityIds.Select( + x => new EntityPermission( + groupId, + x, + permissions.Select(p => p.ToString(CultureInfo.InvariantCulture)).ToArray())) + .ToArray(), false)); } } @@ -700,11 +712,22 @@ namespace Umbraco.Core.Services /// Specify the nodes to replace permissions for public void AssignUserGroupPermission(int groupId, char permission, params int[] entityIds) { + if (entityIds.Length == 0) + return; + using (var uow = UowProvider.GetUnitOfWork()) { var repository = RepositoryFactory.CreateUserGroupRepository(uow); repository.AssignGroupPermission(groupId, permission, entityIds); uow.Commit(); + + uow.Events.Dispatch(UserGroupPermissionsAssigned, this, new SaveEventArgs( + entityIds.Select( + x => new EntityPermission( + groupId, + x, + new[] {permission.ToString(CultureInfo.InvariantCulture)})) + .ToArray(), false)); } } @@ -853,45 +876,45 @@ namespace Umbraco.Core.Services var repository = RepositoryFactory.CreateUserGroupRepository(uow); return repository.GetPermissions(user.Groups.ToArray(), true, nodeIds); - } + } } /// /// Get explicitly assigned permissions for a group and optional node Ids /// - /// Group to retrieve permissions for - /// - /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set - /// - /// Specifiying nothing will return all permissions for all nodes - /// An enumerable list of - private IEnumerable GetPermissions(IReadOnlyUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) - { - if (group == null) throw new ArgumentNullException("group"); - - using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) - { - var repository = RepositoryFactory.CreateUserGroupRepository(uow); - return repository.GetPermissions(new[] { group }, fallbackToDefaultPermissions, nodeIds); - } - } - - /// - /// Get explicitly assigned permissions for a group and optional node Ids - /// - /// Group to retrieve permissions for + /// Groups to retrieve permissions for /// /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set /// /// Specifiying nothing will return all permissions for all nodes /// An enumerable list of - public EntityPermissionCollection GetPermissions(IUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) + private IEnumerable GetPermissions(IReadOnlyUserGroup[] groups, bool fallbackToDefaultPermissions, params int[] nodeIds) { - if (group == null) throw new ArgumentNullException("group"); + if (groups == null) throw new ArgumentNullException("group"); + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) { var repository = RepositoryFactory.CreateUserGroupRepository(uow); - return repository.GetPermissions(new[] { group.ToReadOnlyGroup() }, fallbackToDefaultPermissions, nodeIds); + return repository.GetPermissions(groups, fallbackToDefaultPermissions, nodeIds); + } + } + + /// + /// Get explicitly assigned permissions for a group and optional node Ids + /// + /// + /// + /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set + /// + /// Specifiying nothing will return all permissions for all nodes + /// An enumerable list of + public EntityPermissionCollection GetPermissions(IUserGroup[] groups, bool fallbackToDefaultPermissions, params int[] nodeIds) + { + if (groups == null) throw new ArgumentNullException("groups"); + using (var uow = UowProvider.GetUnitOfWork(readOnly: true)) + { + var repository = RepositoryFactory.CreateUserGroupRepository(uow); + return repository.GetPermissions(groups.Select(x => x.ToReadOnlyGroup()).ToArray(), fallbackToDefaultPermissions, nodeIds); } } @@ -905,36 +928,47 @@ namespace Umbraco.Core.Services var nodeIds = path.GetIdsFromPathReversed(); if (nodeIds.Length == 0) - return EntityPermissionSet.Empty(); - - //collect all permissions structures for all nodes for all groups belonging to the user - var groupPermissions = user.Groups - .Select(g => GetPermissionsForPath(g, nodeIds, fallbackToDefaultPermissions: true)) - .ToArray(); - + return EntityPermissionSet.Empty(); + + //collect all permissions structures for all nodes for all groups belonging to the user + var groupPermissions = GetPermissionsForPath(user.Groups.ToArray(), nodeIds, fallbackToDefaultPermissions: true).ToArray(); + return CalculatePermissionsForPathForUser(groupPermissions, nodeIds); - } - + } + /// /// Gets the permissions for the provided group and path /// - /// Group to check permissions for + /// /// Path to check permissions for /// - /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set + /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set /// /// String indicating permissions for provided user and path - public EntityPermission GetPermissionsForPath(IUserGroup group, string path, bool fallbackToDefaultPermissions = false) + public EntityPermissionSet GetPermissionsForPath(IUserGroup[] groups, string path, bool fallbackToDefaultPermissions = false) { var nodeIds = path.GetIdsFromPathReversed(); - return GetPermissionsForPath(group.ToReadOnlyGroup(), nodeIds, fallbackToDefaultPermissions); + + if (nodeIds.Length == 0) + return EntityPermissionSet.Empty(); + + //collect all permissions structures for all nodes for all groups + var groupPermissions = GetPermissionsForPath(groups.Select(x => x.ToReadOnlyGroup()).ToArray(), nodeIds, fallbackToDefaultPermissions: true).ToArray(); + + return CalculatePermissionsForPathForUser(groupPermissions, nodeIds); } - private EntityPermission GetPermissionsForPath(IReadOnlyUserGroup group, int[] pathIds, bool fallbackToDefaultPermissions = false) + private EntityPermissionCollection GetPermissionsForPath(IReadOnlyUserGroup[] groups, int[] pathIds, bool fallbackToDefaultPermissions = false) { - //get permissions for all nodes in the path - var permissions = GetPermissions(group, fallbackToDefaultPermissions, pathIds); - return GetPermissionsForPathForGroup(permissions, pathIds, fallbackToDefaultPermissions); + if (pathIds.Length == 0) + return new EntityPermissionCollection(Enumerable.Empty()); + + //get permissions for all nodes in the path by group + var permissions = GetPermissions(groups, fallbackToDefaultPermissions, pathIds) + .GroupBy(x => x.UserGroupId); + + return new EntityPermissionCollection( + permissions.Select(x => GetPermissionsForPathForGroup(x, pathIds, fallbackToDefaultPermissions))); } /// @@ -1011,7 +1045,10 @@ namespace Umbraco.Core.Services /// /// Returns the resulting permission set for a group for the path based on all permissions provided for the branch /// - /// The collective set of permissions provided to calculate the resulting permissions set for the path + /// + /// The collective set of permissions provided to calculate the resulting permissions set for the path + /// based on a single group + /// /// Must be ordered deepest to shallowest (right to left) /// /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set @@ -1122,6 +1159,10 @@ namespace Umbraco.Core.Services /// /// Occurs after Delete /// - public static event TypedEventHandler> DeletedUserGroup; + public static event TypedEventHandler> DeletedUserGroup; + + //TODO: still don't know if we need this yet unless we start caching permissions, but that also means we'll need another + // event on the ContentService since there's a method there to modify node permissions too, or we can proxy events if needed. + internal static event TypedEventHandler> UserGroupPermissionsAssigned; } } diff --git a/src/Umbraco.Core/Services/UserServiceExtensions.cs b/src/Umbraco.Core/Services/UserServiceExtensions.cs index 0492a59d03..e53a24527f 100644 --- a/src/Umbraco.Core/Services/UserServiceExtensions.cs +++ b/src/Umbraco.Core/Services/UserServiceExtensions.cs @@ -4,8 +4,37 @@ using Umbraco.Core.Models.Membership; namespace Umbraco.Core.Services { - internal static class UserServiceExtensions + public static class UserServiceExtensions { + /// + /// Get explicitly assigned permissions for a group and optional node Ids + /// + /// + /// + /// + /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set + /// + /// Specifiying nothing will return all permissions for all nodes + /// An enumerable list of + public static EntityPermissionCollection GetPermissions(this IUserService service, IUserGroup group, bool fallbackToDefaultPermissions, params int[] nodeIds) + { + return service.GetPermissions(new[] {group}, fallbackToDefaultPermissions, nodeIds); + } + + /// + /// Gets the permissions for the provided group and path + /// + /// + /// + /// Path to check permissions for + /// + /// Flag indicating if we want to include the default group permissions for each result if there are not explicit permissions set + /// + public static EntityPermissionSet GetPermissionsForPath(this IUserService service, IUserGroup group, string path, bool fallbackToDefaultPermissions = false) + { + return service.GetPermissionsForPath(new[] { group }, path, fallbackToDefaultPermissions); + } + /// /// Remove all permissions for this user group for all nodes specified /// @@ -36,7 +65,7 @@ namespace Umbraco.Core.Services /// To maintain compatibility we have to check the login name if the provider key lookup fails but otherwise /// we'll store the provider user key in the login column. /// - public static IUser CreateUserMappingForCustomProvider(this IUserService userService, MembershipUser member) + internal static IUser CreateUserMappingForCustomProvider(this IUserService userService, MembershipUser member) { if (member == null) throw new ArgumentNullException("member"); diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index 43f049adf4..69ff951f26 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -1488,8 +1488,9 @@ namespace Umbraco.Tests.Services //get the permissions and verify var permissions = ServiceContext.UserService.GetPermissionsForPath(userGroup, copy.Path, fallbackToDefaultPermissions: true); - Assert.AreEqual(1, permissions.AssignedPermissions.Length); - Assert.AreEqual("A", permissions.AssignedPermissions[0]); + var allPermissions = permissions.GetAllPermissions().ToArray(); + Assert.AreEqual(1, allPermissions.Length); + Assert.AreEqual("A", allPermissions[0]); } [Test] @@ -1523,9 +1524,10 @@ namespace Umbraco.Tests.Services foreach (var descendant in descendants) { - var permissions = ServiceContext.UserService.GetPermissionsForPath(userGroup, descendant.Path, fallbackToDefaultPermissions:true); - Assert.AreEqual(1, permissions.AssignedPermissions.Length); - Assert.AreEqual("A", permissions.AssignedPermissions[0]); + var permissions = ServiceContext.UserService.GetPermissionsForPath(userGroup, descendant.Path, fallbackToDefaultPermissions: true); + var allPermissions = permissions.GetAllPermissions().ToArray(); + Assert.AreEqual(1, allPermissions.Length); + Assert.AreEqual("A", allPermissions[0]); } //create a new parent with a new permission structure @@ -1542,8 +1544,9 @@ namespace Umbraco.Tests.Services foreach (var descendant in descendants) { var permissions = ServiceContext.UserService.GetPermissionsForPath(userGroup, descendant.Path, fallbackToDefaultPermissions: true); - Assert.AreEqual(1, permissions.AssignedPermissions.Length); - Assert.AreEqual("B", permissions.AssignedPermissions[0]); + var allPermissions = permissions.GetAllPermissions().ToArray(); + Assert.AreEqual(1, allPermissions.Length); + Assert.AreEqual("B", allPermissions[0]); } } diff --git a/src/Umbraco.Tests/Services/UserServiceTests.cs b/src/Umbraco.Tests/Services/UserServiceTests.cs index cdfff6ff0d..dcd572fdd7 100644 --- a/src/Umbraco.Tests/Services/UserServiceTests.cs +++ b/src/Umbraco.Tests/Services/UserServiceTests.cs @@ -432,7 +432,8 @@ namespace Umbraco.Tests.Services var permissions = userService.GetPermissionsForPath(userGroup, child2.Path); //assert - Assert.AreEqual(3, permissions.AssignedPermissions.Length); + var allPermissions = permissions.GetAllPermissions().ToArray(); + Assert.AreEqual(3, allPermissions.Length); } [Test] diff --git a/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs b/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs index daf405a562..b4e28fb95b 100644 --- a/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs +++ b/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs @@ -68,6 +68,8 @@ namespace Umbraco.Web.Cache () => UserService.SavedUser -= UserService_SavedUser); Bind(() => UserService.DeletedUser += UserService_DeletedUser, () => UserService.DeletedUser -= UserService_DeletedUser); + Bind(() => UserService.UserGroupPermissionsAssigned += UserService_UserGroupPermissionsAssigned, + () => UserService.UserGroupPermissionsAssigned -= UserService_UserGroupPermissionsAssigned); // bind to dictionary events Bind(() => LocalizationService.DeletedDictionaryItem += LocalizationService_DeletedDictionaryItem, @@ -111,8 +113,10 @@ namespace Umbraco.Web.Cache Bind(() => MemberTypeService.Saved += MemberTypeService_Saved, () => MemberTypeService.Saved -= MemberTypeService_Saved); Bind(() => MemberTypeService.Deleted += MemberTypeService_Deleted, - () => MemberTypeService.Deleted -= MemberTypeService_Deleted); - + () => MemberTypeService.Deleted -= MemberTypeService_Deleted); + + + // bind to template events Bind(() => FileService.SavedTemplate += FileService_SavedTemplate, () => FileService.SavedTemplate -= FileService_SavedTemplate); @@ -548,13 +552,22 @@ namespace Umbraco.Web.Cache static void MemberTypeService_Saved(IMemberTypeService sender, SaveEventArgs e) { e.SavedEntities.ForEach(x => DistributedCache.Instance.RefreshMemberTypeCache(x)); - } - - + } + + #endregion - + #region User/permissions event handlers - + + static void UserService_UserGroupPermissionsAssigned(IUserService sender, SaveEventArgs e) + { + //TODO: Not sure if we need this yet depends if we start caching permissions + //var groupIds = e.SavedEntities.Select(x => x.UserGroupId).Distinct(); + //foreach (var groupId in groupIds) + //{ + // DistributedCache.Instance.RefreshUserGroupPermissionsCache(groupId); + //} + } static void UserService_SavedUser(IUserService sender, SaveEventArgs e) { diff --git a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs index e83050dd26..5e527cdfcb 100644 --- a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Text; using System.Web.Script.Serialization; +using Newtonsoft.Json; using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Models; @@ -34,8 +35,7 @@ namespace Umbraco.Web.Cache /// internal static JsonPayload[] DeserializeFromJsonPayload(string json) { - var serializer = new JavaScriptSerializer(); - var jsonObject = serializer.Deserialize(json); + var jsonObject = JsonConvert.DeserializeObject(json); return jsonObject; } @@ -79,9 +79,8 @@ namespace Umbraco.Web.Cache /// internal static string SerializeToJsonPayload(bool isDeleted, params IContentTypeBase[] contentTypes) { - var serializer = new JavaScriptSerializer(); var items = contentTypes.Select(x => FromContentType(x, isDeleted)).ToArray(); - var json = serializer.Serialize(items); + var json = JsonConvert.SerializeObject(items); return json; } diff --git a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs index 9d8dfc3332..833855a972 100644 --- a/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/DataTypeCacheRefresher.cs @@ -3,6 +3,7 @@ using System.Web.Script.Serialization; using Umbraco.Core; using Umbraco.Core.Cache; using System.Linq; +using Newtonsoft.Json; using Umbraco.Core.Models; using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.PropertyEditors.ValueConverters; @@ -27,8 +28,7 @@ namespace Umbraco.Web.Cache /// private static JsonPayload[] DeserializeFromJsonPayload(string json) { - var serializer = new JavaScriptSerializer(); - var jsonObject = serializer.Deserialize(json); + var jsonObject = JsonConvert.DeserializeObject(json); return jsonObject; } @@ -39,9 +39,8 @@ namespace Umbraco.Web.Cache /// internal static string SerializeToJsonPayload(params IDataTypeDefinition[] dataTypes) { - var serializer = new JavaScriptSerializer(); var items = dataTypes.Select(FromDataTypeDefinition).ToArray(); - var json = serializer.Serialize(items); + var json = JsonConvert.SerializeObject(items); return json; } diff --git a/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs b/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs index 397410dbd5..849cd6c81a 100644 --- a/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs +++ b/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs @@ -79,6 +79,25 @@ namespace Umbraco.Web.Cache #endregion + #region User group permissions cache + + public static void RemoveUserGroupPermissionsCache(this DistributedCache dc, int groupId) + { + dc.Remove(DistributedCache.UserGroupPermissionsCacheRefresherGuid, groupId); + } + + public static void RefreshUserGroupPermissionsCache(this DistributedCache dc, int groupId) + { + //TODO: Not sure if we need this yet depends if we start caching permissions + //dc.Refresh(DistributedCache.UserGroupPermissionsCacheRefresherGuid, groupId); + } + + public static void RefreshAllUserGroupPermissionsCache(this DistributedCache dc) + { + dc.RefreshAll(DistributedCache.UserGroupPermissionsCacheRefresherGuid); + } + + #endregion #region Template cache diff --git a/src/Umbraco.Web/Cache/MacroCacheRefresher.cs b/src/Umbraco.Web/Cache/MacroCacheRefresher.cs index cb62dbeb39..4292a30589 100644 --- a/src/Umbraco.Web/Cache/MacroCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/MacroCacheRefresher.cs @@ -8,6 +8,7 @@ using umbraco; using Umbraco.Core.Persistence.Repositories; using umbraco.interfaces; using System.Linq; +using Newtonsoft.Json; using Macro = umbraco.cms.businesslogic.macro.Macro; namespace Umbraco.Web.Cache @@ -47,8 +48,7 @@ namespace Umbraco.Web.Cache /// private static JsonPayload[] DeserializeFromJsonPayload(string json) { - var serializer = new JavaScriptSerializer(); - var jsonObject = serializer.Deserialize(json); + var jsonObject = JsonConvert.DeserializeObject(json); return jsonObject; } @@ -59,9 +59,8 @@ namespace Umbraco.Web.Cache /// internal static string SerializeToJsonPayload(params Macro[] macros) { - var serializer = new JavaScriptSerializer(); var items = macros.Select(FromMacro).ToArray(); - var json = serializer.Serialize(items); + var json = JsonConvert.SerializeObject(items); return json; } @@ -72,9 +71,8 @@ namespace Umbraco.Web.Cache /// internal static string SerializeToJsonPayload(params IMacro[] macros) { - var serializer = new JavaScriptSerializer(); var items = macros.Select(FromMacro).ToArray(); - var json = serializer.Serialize(items); + var json = JsonConvert.SerializeObject(items); return json; } @@ -85,9 +83,8 @@ namespace Umbraco.Web.Cache /// internal static string SerializeToJsonPayload(params macro[] macros) { - var serializer = new JavaScriptSerializer(); var items = macros.Select(FromMacro).ToArray(); - var json = serializer.Serialize(items); + var json = JsonConvert.SerializeObject(items); return json; } diff --git a/src/Umbraco.Web/Cache/MediaCacheRefresher.cs b/src/Umbraco.Web/Cache/MediaCacheRefresher.cs index fc589b530b..3613da426d 100644 --- a/src/Umbraco.Web/Cache/MediaCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/MediaCacheRefresher.cs @@ -11,6 +11,7 @@ using Umbraco.Core.Models; using Umbraco.Core.Persistence.Repositories; using umbraco.interfaces; using System.Linq; +using Newtonsoft.Json; using Umbraco.Web.PublishedCache.XmlPublishedCache; namespace Umbraco.Web.Cache @@ -32,8 +33,7 @@ namespace Umbraco.Web.Cache /// public static JsonPayload[] DeserializeFromJsonPayload(string json) { - var serializer = new JavaScriptSerializer(); - var jsonObject = serializer.Deserialize(json); + var jsonObject = JsonConvert.DeserializeObject(json); return jsonObject; } @@ -45,34 +45,31 @@ namespace Umbraco.Web.Cache /// internal static string SerializeToJsonPayload(OperationType operation, params IMedia[] media) { - var serializer = new JavaScriptSerializer(); var items = media.Select(x => FromMedia(x, operation)).ToArray(); - var json = serializer.Serialize(items); + var json = JsonConvert.SerializeObject(items); return json; } internal static string SerializeToJsonPayloadForMoving(OperationType operation, MoveEventInfo[] media) { - var serializer = new JavaScriptSerializer(); var items = media.Select(x => new JsonPayload { Id = x.Entity.Id, Operation = operation, Path = x.OriginalPath }).ToArray(); - var json = serializer.Serialize(items); + var json = JsonConvert.SerializeObject(items); return json; } internal static string SerializeToJsonPayloadForPermanentDeletion(params int[] mediaIds) { - var serializer = new JavaScriptSerializer(); var items = mediaIds.Select(x => new JsonPayload { Id = x, Operation = OperationType.Deleted }).ToArray(); - var json = serializer.Serialize(items); + var json = JsonConvert.SerializeObject(items); return json; } diff --git a/src/Umbraco.Web/Cache/MemberGroupCacheRefresher.cs b/src/Umbraco.Web/Cache/MemberGroupCacheRefresher.cs index dc2ba39b9d..2d61b254a0 100644 --- a/src/Umbraco.Web/Cache/MemberGroupCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/MemberGroupCacheRefresher.cs @@ -1,6 +1,7 @@ using System; using System.Linq; using System.Web.Script.Serialization; +using Newtonsoft.Json; using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Models; @@ -20,8 +21,7 @@ namespace Umbraco.Web.Cache /// private static JsonPayload[] DeserializeFromJsonPayload(string json) { - var serializer = new JavaScriptSerializer(); - var jsonObject = serializer.Deserialize(json); + var jsonObject = JsonConvert.DeserializeObject(json); return jsonObject; } @@ -32,9 +32,8 @@ namespace Umbraco.Web.Cache /// internal static string SerializeToJsonPayload(params IMemberGroup[] groups) { - var serializer = new JavaScriptSerializer(); var items = groups.Select(FromMemberGroup).ToArray(); - var json = serializer.Serialize(items); + var json = JsonConvert.SerializeObject(items); return json; } diff --git a/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs b/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs index e352c8108a..2941357f19 100644 --- a/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/UnpublishedPageCacheRefresher.cs @@ -5,7 +5,7 @@ using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Models; using System.Linq; - +using Newtonsoft.Json; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Sync; @@ -40,21 +40,19 @@ namespace Umbraco.Web.Cache /// internal static JsonPayload[] DeserializeFromJsonPayload(string json) { - var serializer = new JavaScriptSerializer(); - var jsonObject = serializer.Deserialize(json); + var jsonObject = JsonConvert.DeserializeObject(json); return jsonObject; } internal static string SerializeToJsonPayloadForPermanentDeletion(params int[] contentIds) { - var serializer = new JavaScriptSerializer(); var items = contentIds.Select(x => new JsonPayload { Id = x, Operation = OperationType.Deleted }).ToArray(); - var json = serializer.Serialize(items); + var json = JsonConvert.SerializeObject(items); return json; } diff --git a/src/Umbraco.Web/Cache/UserGroupCacheRefresher.cs b/src/Umbraco.Web/Cache/UserGroupCacheRefresher.cs index 84496b61fd..5fd1e4dfa4 100644 --- a/src/Umbraco.Web/Cache/UserGroupCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/UserGroupCacheRefresher.cs @@ -9,6 +9,9 @@ namespace Umbraco.Web.Cache /// /// Handles User group cache invalidation/refreshing /// + /// + /// This also needs to clear the user cache since IReadOnlyUserGroup's are attached to IUser objects + /// public sealed class UserGroupCacheRefresher : CacheRefresherBase { protected override UserGroupCacheRefresher Instance @@ -35,6 +38,9 @@ namespace Umbraco.Web.Cache userGroupCache.Result.ClearCacheByKeySearch(UserGroupRepository.GetByAliasCacheKeyPrefix); } + //We'll need to clear all user cache too + ClearAllIsolatedCacheByEntityType(); + base.RefreshAll(); } @@ -52,8 +58,12 @@ namespace Umbraco.Web.Cache userGroupCache.Result.ClearCacheItem(RepositoryBase.GetCacheIdKey(id)); userGroupCache.Result.ClearCacheByKeySearch(UserGroupRepository.GetByAliasCacheKeyPrefix); } - + + //we don't know what user's belong to this group without doing a look up so we'll need to just clear them all + ClearAllIsolatedCacheByEntityType(); + base.Remove(id); } + } } \ No newline at end of file diff --git a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs index 8056724a68..6525e95d66 100644 --- a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs @@ -193,7 +193,7 @@ namespace Umbraco.Web.Models.Mapping //Deal with assigned permissions: - var allContentPermissions = applicationContext.Services.UserService.GetPermissions(group, true) + var allContentPermissions = applicationContext.Services.UserService.GetPermissions(@group, true) .ToDictionary(x => x.EntityId, x => x); var contentEntities = allContentPermissions.Keys.Count == 0 diff --git a/src/umbraco.cms/businesslogic/Permission.cs b/src/umbraco.cms/businesslogic/Permission.cs index 335535c08d..32daa3df50 100644 --- a/src/umbraco.cms/businesslogic/Permission.cs +++ b/src/umbraco.cms/businesslogic/Permission.cs @@ -32,24 +32,8 @@ namespace umbraco.BusinessLogic public static void MakeNew(IUserGroup userGroup, CMSNode node, char permissionKey) { - MakeNew(userGroup, node, permissionKey, true); - } - - private static void MakeNew(IUserGroup userGroup, IEnumerable nodes, char permissionKey, bool raiseEvents) - { - var asArray = nodes.ToArray(); - - ApplicationContext.Current.Services.UserService.AssignUserGroupPermission(userGroup.Id, permissionKey, asArray.Select(x => x.Id).ToArray()); - - if (raiseEvents) - { - OnNew(new UserGroupPermission(userGroup, asArray, new[] { permissionKey }), new NewEventArgs()); - } - } - - private static void MakeNew(IUserGroup userGroup, CMSNode Node, char PermissionKey, bool raiseEvents) - { - MakeNew(userGroup, new[] {Node}, PermissionKey, raiseEvents); + ApplicationContext.Current.Services.UserService.AssignUserGroupPermission( + userGroup.Id, permissionKey, node.Id); } /// @@ -99,17 +83,8 @@ namespace umbraco.BusinessLogic /// /// public static void DeletePermissions(IUserGroup userGroup, CMSNode node) - { - DeletePermissions(userGroup, node, true); - } - - internal static void DeletePermissions(IUserGroup userGroup, CMSNode node, bool raiseEvents) { ApplicationContext.Current.Services.UserService.RemoveUserGroupPermissions(userGroup.Id, node.Id); - if (raiseEvents) - { - OnDeleted(new UserGroupPermission(userGroup, node, null), new DeleteEventArgs()); - } } /// @@ -119,15 +94,13 @@ namespace umbraco.BusinessLogic public static void DeletePermissions(IUserGroup userGroup) { ApplicationContext.Current.Services.UserService.RemoveUserGroupPermissions(userGroup.Id); - - OnDeleted(new UserGroupPermission(userGroup, Enumerable.Empty(), null), new DeleteEventArgs()); + } public static void DeletePermissions(int userGroupId, int[] iNodeIDs) { ApplicationContext.Current.Services.UserService.RemoveUserGroupPermissions(userGroupId, iNodeIDs); - - OnDeleted(new UserGroupPermission(userGroupId, iNodeIDs), new DeleteEventArgs()); + } public static void DeletePermissions(int userGroupId, int iNodeID) { @@ -142,7 +115,6 @@ namespace umbraco.BusinessLogic { ApplicationContext.Current.Services.ContentService.RemoveContentPermissions(node.Id); - OnDeleted(new UserGroupPermission(null, node, null), new DeleteEventArgs()); } public static void UpdateCruds(IUserGroup userGroup, CMSNode node, string permissions) @@ -151,103 +123,9 @@ namespace umbraco.BusinessLogic userGroup.Id, permissions.ToCharArray(), node.Id); - - OnUpdated(new UserGroupPermission(userGroup, node, permissions.ToCharArray()), new SaveEventArgs()); - } - - internal static event TypedEventHandler Deleted; - private static void OnDeleted(UserGroupPermission permission, DeleteEventArgs args) - { - if (Deleted != null) - { - Deleted(permission, args); - } - } - - internal static event TypedEventHandler Updated; - private static void OnUpdated(UserGroupPermission permission, SaveEventArgs args) - { - if (Updated != null) - { - Updated(permission, args); - } - } - - internal static event TypedEventHandler New; - private static void OnNew(UserGroupPermission permission, NewEventArgs args) - { - if (New != null) - { - New(permission, args); - } + } } - internal class UserGroupPermission - { - private int? _userGroupId; - private readonly int[] _nodeIds; - - internal UserGroupPermission(int userGroupId) - { - _userGroupId = userGroupId; - } - - internal UserGroupPermission(int userGroupId, IEnumerable nodeIds) - { - _userGroupId = userGroupId; - _nodeIds = nodeIds.ToArray(); - } - - internal UserGroupPermission(IUserGroup userGroup, CMSNode node, char[] permissionKeys) - { - UserGroup = userGroup; - Nodes = new[] { node }; - PermissionKeys = permissionKeys; - } - - internal UserGroupPermission(IUserGroup userGroup, IEnumerable nodes, char[] permissionKeys) - { - UserGroup = userGroup; - Nodes = nodes; - PermissionKeys = permissionKeys; - } - - internal int UserId - { - get - { - if (_userGroupId.HasValue) - { - return _userGroupId.Value; - } - if (UserGroup != null) - { - return UserGroup.Id; - } - return -1; - } - } - - internal IEnumerable NodeIds - { - get - { - if (_nodeIds != null) - { - return _nodeIds; - } - if (Nodes != null) - { - return Nodes.Select(x => x.Id); - } - return Enumerable.Empty(); - } - } - - internal IUserGroup UserGroup { get; private set; } - internal IEnumerable Nodes { get; private set; } - internal char[] PermissionKeys { get; private set; } - } } \ No newline at end of file From 3ac64f18d24f51c6e50f4638f989a6967a8be520 Mon Sep 17 00:00:00 2001 From: Stephan Date: Mon, 17 Jul 2017 15:08:45 +0200 Subject: [PATCH 11/11] Bugfix UserModelMapper --- .../Persistence/Factories/UserGroupFactory.cs | 2 +- .../Models/Mapping/UserModelMapperTests.cs | 47 +++++++++++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 src/Umbraco.Tests/Models/Mapping/UserModelMapperTests.cs diff --git a/src/Umbraco.Core/Persistence/Factories/UserGroupFactory.cs b/src/Umbraco.Core/Persistence/Factories/UserGroupFactory.cs index 20057db20f..f4ecffca54 100644 --- a/src/Umbraco.Core/Persistence/Factories/UserGroupFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/UserGroupFactory.cs @@ -13,7 +13,7 @@ namespace Umbraco.Core.Persistence.Factories var userGroup = new UserGroup(dto.UserCount, dto.Alias, dto.Name, dto.DefaultPermissions.IsNullOrWhiteSpace() ? Enumerable.Empty() - : dto.DefaultPermissions.ToCharArray().Select(x => x.ToString(CultureInfo.InvariantCulture)), + : dto.DefaultPermissions.ToCharArray().Select(x => x.ToString(CultureInfo.InvariantCulture)).ToList(), dto.Icon); try diff --git a/src/Umbraco.Tests/Models/Mapping/UserModelMapperTests.cs b/src/Umbraco.Tests/Models/Mapping/UserModelMapperTests.cs new file mode 100644 index 0000000000..7f5e975cfc --- /dev/null +++ b/src/Umbraco.Tests/Models/Mapping/UserModelMapperTests.cs @@ -0,0 +1,47 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using AutoMapper; +using Moq; +using Newtonsoft.Json; +using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Models.Membership; +using Umbraco.Tests.TestHelpers; +using Umbraco.Web.Models.ContentEditing; +using Umbraco.Web.Models.Mapping; + +namespace Umbraco.Tests.Models.Mapping +{ + [TestFixture] + [DatabaseTestBehavior(DatabaseBehavior.NewDbFileAndSchemaPerTest)] + public class UserModelMapperTests : BaseDatabaseFactoryTest + { + [Test] + public void Map_UserGroupSave_To_IUserGroup() + { + var userModelMapper = new UserModelMapper(); + Mapper.Initialize(configuration => userModelMapper.ConfigureMappings(configuration, ApplicationContext.Current)); + + var userService = ApplicationContext.Services.UserService; + IUserGroup userGroup = new UserGroup(0, "alias", "name", new List { "c" }, "icon"); + userService.Save(userGroup); + + // userGroup.permissions is System.Collections.Generic.List`1[System.String] + + userGroup = userService.GetUserGroupById(userGroup.Id); + + // userGroup.permissions is System.Linq.Enumerable+WhereSelectArrayIterator`2[System.Char, System.String] + // fixed: now System.Collections.Generic.List`1[System.String] + + const string json = "{\"id\":@@@ID@@@,\"alias\":\"perm1\",\"name\":\"Perm1\",\"icon\":\"icon-users\",\"sections\":[\"content\"],\"users\":[],\"defaultPermissions\":[\"F\",\"C\",\"A\"],\"assignedPermissions\":{},\"startContentId\":-1,\"startMediaId\":-1,\"action\":\"save\",\"parentId\":-1}"; + var userGroupSave = JsonConvert.DeserializeObject(json.Replace("@@@ID@@@", userGroup.Id.ToString())); + + // failed, AutoMapper complained, "Unable to cast object of type 'WhereSelectArrayIterator`2[System.Char,System.String]' to type 'System.Collections.IList'". + // fixmed: added ToList() in UserGroupFactory + Mapper.Map(userGroupSave, userGroup); + } + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 5a9a79db47..3d4a8d7320 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -186,6 +186,7 @@ +