diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/usergroups.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/usergroups.resource.js index 90d9e58736..a0901ce0db 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/usergroups.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/usergroups.resource.js @@ -50,12 +50,21 @@ } - function getUserGroups() { + function getUserGroups(args) { + + if (!args) { + args = { onlyCurrentUserGroups: true }; + } + if (args.onlyCurrentUserGroups === undefined || args.onlyCurrentUserGroups === null) { + args.onlyCurrentUserGroups = true; + } + return umbRequestHelper.resourcePromise( $http.get( umbRequestHelper.getApiUrl( "userGroupsApiBaseUrl", - "GetUserGroups")), + "GetUserGroups", + args)), "Failed to retrieve user groups"); } 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 e7331de240..92d70765b0 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 @@ -1,7 +1,7 @@ (function () { "use strict"; - function UserGroupsController($scope, $timeout, $location, userGroupsResource, formHelper, localizationService) { + function UserGroupsController($scope, $timeout, $location, userService, userGroupsResource, formHelper, localizationService) { var vm = this; @@ -14,14 +14,21 @@ vm.selectUserGroup = selectUserGroup; vm.deleteUserGroups = deleteUserGroups; + var currentUser = null; + function onInit() { vm.loading = true; - // Get usergroups - userGroupsResource.getUserGroups().then(function (userGroups) { - vm.userGroups = userGroups; - vm.loading = false; + userService.getCurrentUser().then(function(user) { + currentUser = user; + // Get usergroups + userGroupsResource.getUserGroups({ onlyCurrentUserGroups: false }).then(function (userGroups) { + vm.userGroups = _.map(userGroups, function (ug) { + return { group: ug, isMember: user.userGroups.indexOf(ug.alias) !== -1} + }); + vm.loading = false; + }); }); } @@ -34,22 +41,31 @@ } function clickUserGroup(userGroup) { + + if (currentUser.userGroups.indexOf(userGroup.group.alias) === -1) { + return; + } + if (vm.selection.length > 0) { selectUserGroup(userGroup, vm.selection); } else { - goToUserGroup(userGroup.id); + goToUserGroup(userGroup.group.id); } } function selectUserGroup(userGroup, selection, event) { + if (currentUser.userGroups.indexOf(userGroup.group.alias) === -1) { + return; + } + if (userGroup.selected) { - var index = selection.indexOf(userGroup.id); + var index = selection.indexOf(userGroup.group.id); selection.splice(index, 1); userGroup.selected = false; } else { userGroup.selected = true; - vm.selection.push(userGroup.id); + vm.selection.push(userGroup.group.id); } if(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 7a1cd100bb..4c54ce88b1 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 @@ -67,22 +67,38 @@ -
- -
- \ No newline at end of file diff --git a/src/Umbraco.Web.UI.Client/src/views/users/views/users/users.controller.js b/src/Umbraco.Web.UI.Client/src/views/users/views/users/users.controller.js index 4df493cfe6..3ea6c46580 100644 --- a/src/Umbraco.Web.UI.Client/src/views/users/views/users/users.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/users/views/users/users.controller.js @@ -106,7 +106,7 @@ getUsers(); // Get user groups - userGroupsResource.getUserGroups().then(function (userGroups) { + userGroupsResource.getUserGroups({ onlyCurrentUserGroups: false}).then(function (userGroups) { vm.userGroups = userGroups; }); diff --git a/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs b/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs index 745108c7b8..b81ec7b038 100644 --- a/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs +++ b/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs @@ -59,24 +59,42 @@ namespace Umbraco.Web.Editors if (userGroupAliases != null) { - var userGroups = _userService.GetUserGroupsByAlias(userGroupAliases.ToArray()).ToArray(); + var savingGroupAliases = userGroupAliases.ToArray(); - // d) A user cannot assign a group to another user that grants them access to a start node they don't have access to - foreach (var group in userGroups) - { - pathResult = AuthorizePath(currentUser, - group.StartContentId.HasValue ? new[] { group.StartContentId.Value } : null, - group.StartMediaId.HasValue ? new[] { group.StartMediaId.Value } : null); - if (pathResult == false) - return pathResult; - } + //only validate any groups that have changed. + //a non-admin user can remove groups and add groups that they have access to + //but they cannot add a group that they do not have access to or that grants them + //path or section access that they don't have access to. - // e) A user cannot set a section on another user that they don't have access to - var allGroupSections = userGroups.SelectMany(x => x.AllowedSections).Distinct(); - var missingSectionAccess = allGroupSections.Except(currentUser.AllowedSections).ToArray(); - if (missingSectionAccess.Length > 0) + var newGroups = savingUser == null + ? savingGroupAliases + : savingGroupAliases.Except(savingUser.Groups.Select(x => x.Alias)).ToArray(); + + var userGroupsChanged = savingUser != null && newGroups.Length > 0; + + if (userGroupsChanged) { - return Attempt.Fail("The current user does not have access to sections " + string.Join(",", missingSectionAccess)); + var userGroups = _userService.GetUserGroupsByAlias(newGroups).ToArray(); + + //TODO: pretty sure d + e can be done by just checking if the newGroups being added are groups that the current user is a member of + + // d) A user cannot assign a group to another user that grants them access to a start node they don't have access to + foreach (var group in userGroups) + { + pathResult = AuthorizePath(currentUser, + group.StartContentId.HasValue ? new[] { group.StartContentId.Value } : null, + group.StartMediaId.HasValue ? new[] { group.StartMediaId.Value } : null); + if (pathResult == false) + return pathResult; + } + + // e) A user cannot set a section on another user that they don't have access to + var allGroupSections = userGroups.SelectMany(x => x.AllowedSections).Distinct(); + var missingSectionAccess = allGroupSections.Except(currentUser.AllowedSections).ToArray(); + if (missingSectionAccess.Length > 0) + { + return Attempt.Fail("The current user does not have access to sections " + string.Join(",", missingSectionAccess)); + } } } diff --git a/src/Umbraco.Web/Editors/UserGroupsController.cs b/src/Umbraco.Web/Editors/UserGroupsController.cs index 9db73a82a0..33b1d9dc08 100644 --- a/src/Umbraco.Web/Editors/UserGroupsController.cs +++ b/src/Umbraco.Web/Editors/UserGroupsController.cs @@ -92,13 +92,20 @@ namespace Umbraco.Web.Editors /// Returns all user groups /// /// - public IEnumerable GetUserGroups() + public IEnumerable GetUserGroups(bool onlyCurrentUserGroups = true) { - var allGroups = Mapper.Map, IEnumerable>(Services.UserService.GetAllUserGroups()); + var allGroups = Mapper.Map, IEnumerable>(Services.UserService.GetAllUserGroups()) + .ToList(); - //if admin, return all groups - if (Security.CurrentUser.IsAdmin()) + var isAdmin = Security.CurrentUser.IsAdmin(); + if (isAdmin) return allGroups; + + if (onlyCurrentUserGroups == false) + { + //this user is not an admin so in that case we need to exlude all admin users + allGroups.RemoveAt(allGroups.IndexOf(allGroups.Find(basic => basic.Alias == Constants.Security.AdminGroupAlias))); return allGroups; + } //we cannot return user groups that this user does not have access to var currentUserGroups = Security.CurrentUser.Groups.Select(x => x.Alias).ToArray(); diff --git a/src/Umbraco.Web/Models/ContentEditing/UserDetail.cs b/src/Umbraco.Web/Models/ContentEditing/UserDetail.cs index f613065eee..75e1462ec3 100644 --- a/src/Umbraco.Web/Models/ContentEditing/UserDetail.cs +++ b/src/Umbraco.Web/Models/ContentEditing/UserDetail.cs @@ -27,10 +27,14 @@ namespace Umbraco.Web.Models.ContentEditing public string EmailHash { get; set; } [Obsolete("This should not be used it exists for legacy reasons only, use user groups instead, it will be removed in future versions")] - [EditorBrowsable(EditorBrowsableState.Never)] + [EditorBrowsable(EditorBrowsableState.Never)] [ReadOnly(true)] [DataMember(Name = "userType")] - public string UserType { get; set; } + public string UserType { get; set; } + + [ReadOnly(true)] + [DataMember(Name = "userGroups")] + public string[] UserGroups { get; set; } /// /// Gets/sets the number of seconds for the user's auth ticket to expire diff --git a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs index bd25c5da07..7f4fb222ca 100644 --- a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs @@ -321,12 +321,15 @@ namespace Umbraco.Web.Models.Mapping detail => detail.EmailHash, opt => opt.MapFrom(user => user.Email.ToLowerInvariant().Trim().GenerateHash())) .ForMember(detail => detail.SecondsUntilTimeout, opt => opt.Ignore()) + .ForMember(detail => detail.UserGroups, opt => opt.Ignore()) .AfterMap((user, detail) => { //we need to map the legacy UserType //the best we can do here is to return the user's first user group as a IUserType object //but we should attempt to return any group that is the built in ones first var groups = user.Groups.ToArray(); + detail.UserGroups = user.Groups.Select(x => x.Alias).ToArray(); + if (groups.Length == 0) { //In backwards compatibility land, a user type cannot be null! so we need to return a fake one.