diff --git a/src/Umbraco.Core/Models/Membership/UserGroupExtensions.cs b/src/Umbraco.Core/Models/Membership/UserGroupExtensions.cs index 3903fe405b..b1d0189c56 100644 --- a/src/Umbraco.Core/Models/Membership/UserGroupExtensions.cs +++ b/src/Umbraco.Core/Models/Membership/UserGroupExtensions.cs @@ -15,6 +15,12 @@ namespace Umbraco.Core.Models.Membership return new ReadOnlyUserGroup(group.Id, group.Name, group.Icon, group.StartContentId, group.StartMediaId, group.Alias, group.AllowedSections, group.Permissions); } + public static bool IsSystemUserGroup(this IUserGroup group) => + IsSystemUserGroup(group.Alias); + + public static bool IsSystemUserGroup(this IReadOnlyUserGroup group) => + IsSystemUserGroup(group.Alias); + public static IReadOnlyUserGroup ToReadOnlyGroup(this UserGroupDto group) { return new ReadOnlyUserGroup(group.Id, group.Name, group.Icon, @@ -22,5 +28,12 @@ namespace Umbraco.Core.Models.Membership group.UserGroup2AppDtos.Select(x => x.AppAlias).ToArray(), group.DefaultPermissions == null ? Enumerable.Empty() : group.DefaultPermissions.ToCharArray().Select(x => x.ToString())); } + + private static bool IsSystemUserGroup(this string groupAlias) + { + return groupAlias == Constants.Security.AdminGroupAlias + || groupAlias == Constants.Security.SensitiveDataGroupAlias + || groupAlias == Constants.Security.TranslatorGroupAlias; + } } } diff --git a/src/Umbraco.Web.UI.Client/src/views/users/views/groups/groups.controller.js b/src/Umbraco.Web.UI.Client/src/views/users/views/groups/groups.controller.js index 1e51d4585e..b21859f5c4 100644 --- a/src/Umbraco.Web.UI.Client/src/views/users/views/groups/groups.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/users/views/groups/groups.controller.js @@ -81,7 +81,7 @@ } // Disallow selection of the admin/translators group, the checkbox is not visible in the UI, but clicking(and thus selecting) is still possible. // Currently selection can only be used for deleting, and the Controller will also disallow deleting the admin group. - if (userGroup.alias === "admin" || userGroup.alias === "translator") + if (userGroup.isSystemUserGroup) return; listViewHelper.selectHandler(userGroup, $index, vm.userGroups, vm.selection, $event); diff --git a/src/Umbraco.Web.UI.Client/src/views/users/views/groups/groups.html b/src/Umbraco.Web.UI.Client/src/views/users/views/groups/groups.html index 5d1496d90f..4d252a3ae0 100644 --- a/src/Umbraco.Web.UI.Client/src/views/users/views/groups/groups.html +++ b/src/Umbraco.Web.UI.Client/src/views/users/views/groups/groups.html @@ -86,7 +86,7 @@
+ ng-class="{'-selected': group.selected, '-selectable': group.hasAccess && !group.isSystemUserGroup}">
diff --git a/src/Umbraco.Web/Editors/Filters/UserGroupValidateAttribute.cs b/src/Umbraco.Web/Editors/Filters/UserGroupValidateAttribute.cs index 7d465a9ddc..78cd8e6a4d 100644 --- a/src/Umbraco.Web/Editors/Filters/UserGroupValidateAttribute.cs +++ b/src/Umbraco.Web/Editors/Filters/UserGroupValidateAttribute.cs @@ -51,16 +51,11 @@ namespace Umbraco.Web.Editors.Filters return; } - if (persisted.Alias != userGroupSave.Alias) + if (persisted.Alias != userGroupSave.Alias && persisted.IsSystemUserGroup()) { - if (persisted.Alias == Constants.Security.AdminGroupAlias - || persisted.Alias == Constants.Security.SensitiveDataGroupAlias - || persisted.Alias == Constants.Security.TranslatorGroupAlias) - { - var message = $"User group with alias: {persisted.Alias} cannot be changed"; - actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.BadRequest, message); - return; - } + var message = $"User group with alias: {persisted.Alias} cannot be changed"; + actionContext.Response = actionContext.Request.CreateErrorResponse(HttpStatusCode.BadRequest, message); + return; } //map the model to the persisted instance diff --git a/src/Umbraco.Web/Editors/UserGroupsController.cs b/src/Umbraco.Web/Editors/UserGroupsController.cs index 1b64722735..b081ca6137 100644 --- a/src/Umbraco.Web/Editors/UserGroupsController.cs +++ b/src/Umbraco.Web/Editors/UserGroupsController.cs @@ -154,8 +154,8 @@ namespace Umbraco.Web.Editors public HttpResponseMessage PostDeleteUserGroups([FromUri] int[] userGroupIds) { var userGroups = Services.UserService.GetAllUserGroups(userGroupIds) - //never delete the admin group or translators group - .Where(x => x.Alias != Constants.Security.AdminGroupAlias && x.Alias != Constants.Security.TranslatorGroupAlias) + //never delete the admin group, sensitive data or translators group + .Where(x => !x.IsSystemUserGroup()) .ToArray(); foreach (var userGroup in userGroups) { diff --git a/src/Umbraco.Web/Models/Mapping/UserMapDefinition.cs b/src/Umbraco.Web/Models/Mapping/UserMapDefinition.cs index abbb67c4aa..88960fb189 100644 --- a/src/Umbraco.Web/Models/Mapping/UserMapDefinition.cs +++ b/src/Umbraco.Web/Models/Mapping/UserMapDefinition.cs @@ -147,7 +147,9 @@ namespace Umbraco.Web.Models.Mapping target.Name = source.Name; target.ParentId = -1; target.Path = "-1," + source.Id; - MapUserGroupBasic(target, source.Alias, source.AllowedSections, source.StartContentId, source.StartMediaId, context); + target.IsSystemUserGroup = source.IsSystemUserGroup(); + + MapUserGroupBasic(target, source.AllowedSections, source.StartContentId, source.StartMediaId, context); } // Umbraco.Code.MapAll -ContentStartNode -MediaStartNode -Sections -Notifications @@ -162,7 +164,9 @@ namespace Umbraco.Web.Models.Mapping target.ParentId = -1; target.Path = "-1," + source.Id; target.UserCount = source.UserCount; - MapUserGroupBasic(target, source.Alias, source.AllowedSections, source.StartContentId, source.StartMediaId, context); + target.IsSystemUserGroup = source.IsSystemUserGroup(); + + MapUserGroupBasic(target, source.AllowedSections, source.StartContentId, source.StartMediaId, context); } // Umbraco.Code.MapAll -Udi -Trashed -AdditionalData -AssignedPermissions @@ -198,7 +202,7 @@ namespace Umbraco.Web.Models.Mapping } // Umbraco.Code.MapAll -ContentStartNode -MediaStartNode -Sections -Notifications -Udi - // Umbraco.Code.MapAll -Trashed -AdditionalData -Users -AssignedPermissions -IsSystemUserGroup + // Umbraco.Code.MapAll -Trashed -AdditionalData -Users -AssignedPermissions private void Map(IUserGroup source, UserGroupDisplay target, MapperContext context) { target.Alias = source.Alias; @@ -210,8 +214,9 @@ namespace Umbraco.Web.Models.Mapping target.ParentId = -1; target.Path = "-1," + source.Id; target.UserCount = source.UserCount; + target.IsSystemUserGroup = source.IsSystemUserGroup(); - MapUserGroupBasic(target, source.Alias, source.AllowedSections, source.StartContentId, source.StartMediaId, context); + MapUserGroupBasic(target, source.AllowedSections, source.StartContentId, source.StartMediaId, context); //Important! Currently we are never mapping to multiple UserGroupDisplay objects but if we start doing that // this will cause an N+1 and we'll need to change how this works. @@ -334,12 +339,8 @@ namespace Umbraco.Web.Models.Mapping // helpers - private void MapUserGroupBasic(UserGroupBasic target, string alias, IEnumerable sourceAllowedSections, int? sourceStartContentId, int? sourceStartMediaId, MapperContext context) + private void MapUserGroupBasic(UserGroupBasic target, IEnumerable sourceAllowedSections, int? sourceStartContentId, int? sourceStartMediaId, MapperContext context) { - target.IsSystemUserGroup = alias == Constants.Security.AdminGroupAlias - || alias == Constants.Security.TranslatorGroupAlias - || alias == Constants.Security.SensitiveDataGroupAlias; - var allSections = _sectionService.GetSections(); target.Sections = context.MapEnumerable(allSections.Where(x => sourceAllowedSections.Contains(x.Alias)));