From 8033c6807fd3b15151ae2a0026ea450d91c2a3ba Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 22 Aug 2017 13:06:56 +1000 Subject: [PATCH] Ensures admin can access anything --- .../Editors/UserEditorAuthorizationHelper.cs | 114 ++++++++++++++++++ .../UserGroupAuthorizationAttribute.cs | 6 + src/Umbraco.Web/Editors/UsersController.cs | 101 ---------------- src/Umbraco.Web/Umbraco.Web.csproj | 1 + 4 files changed, 121 insertions(+), 101 deletions(-) create mode 100644 src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs diff --git a/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs b/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs new file mode 100644 index 0000000000..ce3be3eb55 --- /dev/null +++ b/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs @@ -0,0 +1,114 @@ +using System.Collections.Generic; +using System.Linq; +using Umbraco.Core; +using Umbraco.Core.Models; +using Umbraco.Core.Models.Membership; +using Umbraco.Core.Services; + +namespace Umbraco.Web.Editors +{ + internal class UserEditorAuthorizationHelper + { + private readonly IContentService _contentService; + private readonly IMediaService _mediaService; + private readonly IUserService _userService; + private readonly IEntityService _entityService; + + public UserEditorAuthorizationHelper(IContentService contentService, IMediaService mediaService, IUserService userService, IEntityService entityService) + { + _contentService = contentService; + _mediaService = mediaService; + _userService = userService; + _entityService = entityService; + } + + /// + /// Checks if the current user has access to save the user data + /// + /// The current user trying to save user data + /// The user instance being saved (can be null if it's a new user) + /// The start content ids of the user being saved (can be null or empty) + /// The start media ids of the user being saved (can be null or empty) + /// The user aliases of the user being saved (can be null or empty) + /// + public Attempt AuthorizeActions(IUser currentUser, + IUser savingUser, + IEnumerable startContentIds, IEnumerable startMediaIds, + IEnumerable userGroupAliases) + { + var currentIsAdmin = currentUser.IsAdmin(); + + // a1) An admin can edit anything + if (currentIsAdmin) + return Attempt.Succeed(); + + // a2) A non-admin cannot save an admin + + if (savingUser != null) + { + if (savingUser.IsAdmin()) + return Attempt.Fail("The current user is not an administrator"); + } + + // b0) A user cannot set a start node on another user that they don't have access to + + var pathResult = AuthorizePath(currentUser, startContentIds, startMediaIds); + if (pathResult == false) + return pathResult; + + if (userGroupAliases != null) + { + var userGroups = _userService.GetUserGroupsByAlias(userGroupAliases.ToArray()).ToArray(); + + // b1) 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; + } + + // c) 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)); + } + } + + return Attempt.Succeed(); + } + + private Attempt AuthorizePath(IUser currentUser, IEnumerable startContentIds, IEnumerable startMediaIds) + { + if (startContentIds != null) + { + foreach (var contentId in startContentIds) + { + var content = _contentService.GetById(contentId); + if (content == null) continue; + var hasAccess = currentUser.HasPathAccess(content, _entityService); + if (hasAccess == false) + return Attempt.Fail("The current user does not have access to the content path " + content.Path); + } + } + + if (startMediaIds != null) + { + foreach (var mediaId in startMediaIds) + { + var media = _mediaService.GetById(mediaId); + if (media == null) continue; + var hasAccess = currentUser.HasPathAccess(media, _entityService); + if (hasAccess == false) + return Attempt.Fail("The current user does not have access to the media path " + media.Path); + } + } + + return Attempt.Succeed(); + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Editors/UserGroupAuthorizationAttribute.cs b/src/Umbraco.Web/Editors/UserGroupAuthorizationAttribute.cs index 7810d62962..70ec52a0bb 100644 --- a/src/Umbraco.Web/Editors/UserGroupAuthorizationAttribute.cs +++ b/src/Umbraco.Web/Editors/UserGroupAuthorizationAttribute.cs @@ -4,6 +4,7 @@ using System.Net.Http; using System.Web.Http; using System.Web.Http.Controllers; using Umbraco.Core; +using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; namespace Umbraco.Web.Editors @@ -41,6 +42,11 @@ namespace Umbraco.Web.Editors protected override bool IsAuthorized(HttpActionContext actionContext) { var currentUser = GetUmbracoContext().Security.CurrentUser; + + //admins can access any group + if (currentUser.IsAdmin()) + return base.IsAuthorized(actionContext); + var queryString = actionContext.Request .GetQueryNameValuePairs(); diff --git a/src/Umbraco.Web/Editors/UsersController.cs b/src/Umbraco.Web/Editors/UsersController.cs index e595edc7de..d304112cb5 100644 --- a/src/Umbraco.Web/Editors/UsersController.cs +++ b/src/Umbraco.Web/Editors/UsersController.cs @@ -672,105 +672,4 @@ namespace Umbraco.Web.Editors } } - - internal class UserEditorAuthorizationHelper - { - private readonly IContentService _contentService; - private readonly IMediaService _mediaService; - private readonly IUserService _userService; - private readonly IEntityService _entityService; - - public UserEditorAuthorizationHelper(IContentService contentService, IMediaService mediaService, IUserService userService, IEntityService entityService) - { - _contentService = contentService; - _mediaService = mediaService; - _userService = userService; - _entityService = entityService; - } - - /// - /// Checks if the current user has access to save the user data - /// - /// The current user trying to save user data - /// The user instance being saved (can be null if it's a new user) - /// The start content ids of the user being saved (can be null or empty) - /// The start media ids of the user being saved (can be null or empty) - /// The user aliases of the user being saved (can be null or empty) - /// - public Attempt AuthorizeActions(IUser currentUser, - IUser savingUser, - IEnumerable startContentIds, IEnumerable startMediaIds, - IEnumerable userGroupAliases) - { - // a) A non-admin cannot save an admin - - if (savingUser != null) - { - var currentIsAdmin = currentUser.IsAdmin(); - var savingIsAdmin = savingUser.IsAdmin(); - if (currentIsAdmin == false && savingIsAdmin) - return Attempt.Fail("The current user is not an administrator"); - } - - // b0) A user cannot set a start node on another user that they don't have access to - - var pathResult = AuthorizePath(currentUser, startContentIds, startMediaIds); - if (pathResult == false) - return pathResult; - - if (userGroupAliases != null) - { - var userGroups = _userService.GetUserGroupsByAlias(userGroupAliases.ToArray()).ToArray(); - - // b1) 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; - } - - // c) 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)); - } - } - - return Attempt.Succeed(); - } - - private Attempt AuthorizePath(IUser currentUser, IEnumerable startContentIds, IEnumerable startMediaIds) - { - if (startContentIds != null) - { - foreach (var contentId in startContentIds) - { - var content = _contentService.GetById(contentId); - if (content == null) continue; - var hasAccess = currentUser.HasPathAccess(content, _entityService); - if (hasAccess == false) - return Attempt.Fail("The current user does not have access to the content path " + content.Path); - } - } - - if (startMediaIds != null) - { - foreach (var mediaId in startMediaIds) - { - var media = _mediaService.GetById(mediaId); - if (media == null) continue; - var hasAccess = currentUser.HasPathAccess(media, _entityService); - if (hasAccess == false) - return Attempt.Fail("The current user does not have access to the media path " + media.Path); - } - } - - return Attempt.Succeed(); - } - } } \ No newline at end of file diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index cb4037b83a..aa9957b329 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -328,6 +328,7 @@ +