diff --git a/src/Umbraco.Core/Models/UserExtensions.cs b/src/Umbraco.Core/Models/UserExtensions.cs index 82e4935616..ea61228864 100644 --- a/src/Umbraco.Core/Models/UserExtensions.cs +++ b/src/Umbraco.Core/Models/UserExtensions.cs @@ -9,6 +9,7 @@ using Umbraco.Core.Composing; using Umbraco.Core.Models.Entities; using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; +using Umbraco.Core.Security; namespace Umbraco.Core.Models { @@ -146,68 +147,46 @@ namespace Umbraco.Core.Models internal static bool HasContentRootAccess(this IUser user, IEntityService entityService) { - return HasPathAccess(Constants.System.Root.ToInvariantString(), user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); + return ContentPermissionsHelper.HasPathAccess(Constants.System.Root.ToInvariantString(), user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); } internal static bool HasContentBinAccess(this IUser user, IEntityService entityService) { - return HasPathAccess(Constants.System.RecycleBinContent.ToInvariantString(), user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); + return ContentPermissionsHelper.HasPathAccess(Constants.System.RecycleBinContent.ToInvariantString(), user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); } internal static bool HasMediaRootAccess(this IUser user, IEntityService entityService) { - return HasPathAccess(Constants.System.Root.ToInvariantString(), user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); + return ContentPermissionsHelper.HasPathAccess(Constants.System.Root.ToInvariantString(), user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); } internal static bool HasMediaBinAccess(this IUser user, IEntityService entityService) { - return HasPathAccess(Constants.System.RecycleBinMedia.ToInvariantString(), user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); + return ContentPermissionsHelper.HasPathAccess(Constants.System.RecycleBinMedia.ToInvariantString(), user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); } internal static bool HasPathAccess(this IUser user, IContent content, IEntityService entityService) { - return HasPathAccess(content.Path, user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); + if (content == null) throw new ArgumentNullException(nameof(content)); + return ContentPermissionsHelper.HasPathAccess(content.Path, user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); } internal static bool HasPathAccess(this IUser user, IMedia media, IEntityService entityService) { - return HasPathAccess(media.Path, user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); + if (media == null) throw new ArgumentNullException(nameof(media)); + return ContentPermissionsHelper.HasPathAccess(media.Path, user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); } - internal static bool HasPathAccess(this IUser user, IUmbracoEntity entity, IEntityService entityService, int recycleBinId) + internal static bool HasContentPathAccess(this IUser user, IUmbracoEntity entity, IEntityService entityService) { - switch (recycleBinId) - { - case Constants.System.RecycleBinMedia: - return HasPathAccess(entity.Path, user.CalculateMediaStartNodeIds(entityService), recycleBinId); - case Constants.System.RecycleBinContent: - return HasPathAccess(entity.Path, user.CalculateContentStartNodeIds(entityService), recycleBinId); - default: - throw new NotSupportedException("Path access is only determined on content or media"); - } + if (entity == null) throw new ArgumentNullException(nameof(entity)); + return ContentPermissionsHelper.HasPathAccess(entity.Path, user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); } - internal static bool HasPathAccess(string path, int[] startNodeIds, int recycleBinId) + internal static bool HasMediaPathAccess(this IUser user, IUmbracoEntity entity, IEntityService entityService) { - if (string.IsNullOrWhiteSpace(path)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(path)); - - // check for no access - if (startNodeIds.Length == 0) - return false; - - // check for root access - if (startNodeIds.Contains(Constants.System.Root)) - return true; - - var formattedPath = string.Concat(",", path, ","); - - // only users with root access have access to the recycle bin, - // if the above check didn't pass then access is denied - if (formattedPath.Contains(string.Concat(",", recycleBinId, ","))) - return false; - - // check for a start node in the path - return startNodeIds.Any(x => formattedPath.Contains(string.Concat(",", x, ","))); + if (entity == null) throw new ArgumentNullException(nameof(entity)); + return ContentPermissionsHelper.HasPathAccess(entity.Path, user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); } internal static bool IsInBranchOfStartNode(this IUser user, IUmbracoEntity entity, IEntityService entityService, int recycleBinId, out bool hasPathAccess) @@ -215,58 +194,14 @@ namespace Umbraco.Core.Models switch (recycleBinId) { case Constants.System.RecycleBinMedia: - return IsInBranchOfStartNode(entity.Path, user.CalculateMediaStartNodeIds(entityService), user.GetMediaStartNodePaths(entityService), out hasPathAccess); + return ContentPermissionsHelper.IsInBranchOfStartNode(entity.Path, user.CalculateMediaStartNodeIds(entityService), user.GetMediaStartNodePaths(entityService), out hasPathAccess); case Constants.System.RecycleBinContent: - return IsInBranchOfStartNode(entity.Path, user.CalculateContentStartNodeIds(entityService), user.GetContentStartNodePaths(entityService), out hasPathAccess); + return ContentPermissionsHelper.IsInBranchOfStartNode(entity.Path, user.CalculateContentStartNodeIds(entityService), user.GetContentStartNodePaths(entityService), out hasPathAccess); default: throw new NotSupportedException("Path access is only determined on content or media"); } } - internal static bool IsInBranchOfStartNode(string path, int[] startNodeIds, string[] startNodePaths, out bool hasPathAccess) - { - if (string.IsNullOrWhiteSpace(path)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(path)); - - hasPathAccess = false; - - // check for no access - if (startNodeIds.Length == 0) - return false; - - // check for root access - if (startNodeIds.Contains(Constants.System.Root)) - { - hasPathAccess = true; - return true; - } - - //is it self? - var self = startNodePaths.Any(x => x == path); - if (self) - { - hasPathAccess = true; - return true; - } - - //is it ancestor? - var ancestor = startNodePaths.Any(x => x.StartsWith(path)); - if (ancestor) - { - //hasPathAccess = false; - return true; - } - - //is it descendant? - var descendant = startNodePaths.Any(x => path.StartsWith(x)); - if (descendant) - { - hasPathAccess = true; - return true; - } - - return false; - } - /// /// Determines whether this user has access to view sensitive data /// diff --git a/src/Umbraco.Core/Security/ContentPermissionsHelper.cs b/src/Umbraco.Core/Security/ContentPermissionsHelper.cs new file mode 100644 index 0000000000..1a329fcdcb --- /dev/null +++ b/src/Umbraco.Core/Security/ContentPermissionsHelper.cs @@ -0,0 +1,263 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using Umbraco.Core.Models; +using Umbraco.Core.Models.Entities; +using Umbraco.Core.Models.Membership; +using Umbraco.Core.Services; + +namespace Umbraco.Core.Security +{ + internal class ContentPermissionsHelper + { + public enum ContentAccess + { + Granted, + Denied, + NotFound + } + + public static ContentAccess CheckPermissions( + IContent content, + IUser user, + IUserService userService, + IEntityService entityService, + params char[] permissionsToCheck) + { + if (user == null) throw new ArgumentNullException("user"); + if (userService == null) throw new ArgumentNullException("userService"); + if (entityService == null) throw new ArgumentNullException("entityService"); + + if (content == null) return ContentAccess.NotFound; + + var hasPathAccess = user.HasPathAccess(content, entityService); + + if (hasPathAccess == false) + return ContentAccess.Denied; + + if (permissionsToCheck == null || permissionsToCheck.Length == 0) + return ContentAccess.Granted; + + //get the implicit/inherited permissions for the user for this path + return CheckPermissionsPath(content.Path, user, userService, permissionsToCheck) + ? ContentAccess.Granted + : ContentAccess.Denied; + } + + public static ContentAccess CheckPermissions( + IUmbracoEntity entity, + IUser user, + IUserService userService, + IEntityService entityService, + params char[] permissionsToCheck) + { + if (user == null) throw new ArgumentNullException("user"); + if (userService == null) throw new ArgumentNullException("userService"); + if (entityService == null) throw new ArgumentNullException("entityService"); + + if (entity == null) return ContentAccess.NotFound; + + var hasPathAccess = user.HasContentPathAccess(entity, entityService); + + if (hasPathAccess == false) + return ContentAccess.Denied; + + if (permissionsToCheck == null || permissionsToCheck.Length == 0) + return ContentAccess.Granted; + + //get the implicit/inherited permissions for the user for this path + return CheckPermissionsPath(entity.Path, user, userService, permissionsToCheck) + ? ContentAccess.Granted + : ContentAccess.Denied; + } + + /// + /// Checks if the user has access to the specified node and permissions set + /// + /// + /// + /// + /// + /// The item resolved if one was found for the id + /// + /// + public static ContentAccess CheckPermissions( + int nodeId, + IUser user, + IUserService userService, + IEntityService entityService, + out IUmbracoEntity entity, + params char[] permissionsToCheck) + { + if (user == null) throw new ArgumentNullException("user"); + if (userService == null) throw new ArgumentNullException("userService"); + if (entityService == null) throw new ArgumentNullException("entityService"); + + bool? hasPathAccess = null; + entity = null; + + if (nodeId == Constants.System.Root) + hasPathAccess = user.HasContentRootAccess(entityService); + else if (nodeId == Constants.System.RecycleBinContent) + hasPathAccess = user.HasContentBinAccess(entityService); + + if (hasPathAccess.HasValue) + return hasPathAccess.Value ? ContentAccess.Granted : ContentAccess.Denied; + + entity = entityService.Get(nodeId, UmbracoObjectTypes.Document); + if (entity == null) return ContentAccess.NotFound; + hasPathAccess = user.HasContentPathAccess(entity, entityService); + + if (hasPathAccess == false) + return ContentAccess.Denied; + + if (permissionsToCheck == null || permissionsToCheck.Length == 0) + return ContentAccess.Granted; + + //get the implicit/inherited permissions for the user for this path + return CheckPermissionsPath(entity.Path, user, userService, permissionsToCheck) + ? ContentAccess.Granted + : ContentAccess.Denied; + } + + /// + /// Checks if the user has access to the specified node and permissions set + /// + /// + /// + /// + /// + /// + /// The item resolved if one was found for the id + /// + /// + public static ContentAccess CheckPermissions( + int nodeId, + IUser user, + IUserService userService, + IContentService contentService, + IEntityService entityService, + out IContent contentItem, + params char[] permissionsToCheck) + { + if (user == null) throw new ArgumentNullException("user"); + if (userService == null) throw new ArgumentNullException("userService"); + if (contentService == null) throw new ArgumentNullException("contentService"); + if (entityService == null) throw new ArgumentNullException("entityService"); + + bool? hasPathAccess = null; + contentItem = null; + + if (nodeId == Constants.System.Root) + hasPathAccess = user.HasContentRootAccess(entityService); + else if (nodeId == Constants.System.RecycleBinContent) + hasPathAccess = user.HasContentBinAccess(entityService); + + if (hasPathAccess.HasValue) + return hasPathAccess.Value ? ContentAccess.Granted : ContentAccess.Denied; + + contentItem = contentService.GetById(nodeId); + if (contentItem == null) return ContentAccess.NotFound; + hasPathAccess = user.HasPathAccess(contentItem, entityService); + + if (hasPathAccess == false) + return ContentAccess.Denied; + + if (permissionsToCheck == null || permissionsToCheck.Length == 0) + return ContentAccess.Granted; + + //get the implicit/inherited permissions for the user for this path + return CheckPermissionsPath(contentItem.Path, user, userService, permissionsToCheck) + ? ContentAccess.Granted + : ContentAccess.Denied; + } + + private static bool CheckPermissionsPath(string path, IUser user, IUserService userService, params char[] permissionsToCheck) + { + //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 permission = userService.GetPermissionsForPath(user, path); + + var allowed = true; + foreach (var p in permissionsToCheck) + { + if (permission == null + || permission.GetAllPermissions().Contains(p.ToString(CultureInfo.InvariantCulture)) == false) + { + allowed = false; + } + } + return allowed; + } + + public static bool HasPathAccess(string path, int[] startNodeIds, int recycleBinId) + { + if (string.IsNullOrWhiteSpace(path)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(path)); + + // check for no access + if (startNodeIds.Length == 0) + return false; + + // check for root access + if (startNodeIds.Contains(Constants.System.Root)) + return true; + + var formattedPath = string.Concat(",", path, ","); + + // only users with root access have access to the recycle bin, + // if the above check didn't pass then access is denied + if (formattedPath.Contains(string.Concat(",", recycleBinId, ","))) + return false; + + // check for a start node in the path + return startNodeIds.Any(x => formattedPath.Contains(string.Concat(",", x, ","))); + } + + internal static bool IsInBranchOfStartNode(string path, int[] startNodeIds, string[] startNodePaths, out bool hasPathAccess) + { + if (string.IsNullOrWhiteSpace(path)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(path)); + + hasPathAccess = false; + + // check for no access + if (startNodeIds.Length == 0) + return false; + + // check for root access + if (startNodeIds.Contains(Constants.System.Root)) + { + hasPathAccess = true; + return true; + } + + //is it self? + var self = startNodePaths.Any(x => x == path); + if (self) + { + hasPathAccess = true; + return true; + } + + //is it ancestor? + var ancestor = startNodePaths.Any(x => x.StartsWith(path)); + if (ancestor) + { + //hasPathAccess = false; + return true; + } + + //is it descendant? + var descendant = startNodePaths.Any(x => path.StartsWith(x)); + if (descendant) + { + hasPathAccess = true; + return true; + } + + return false; + } + } +} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 579e96b122..18d05d307d 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -1287,6 +1287,7 @@ + diff --git a/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs b/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs index 21aa739e23..f55a4e593b 100644 --- a/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs +++ b/src/Umbraco.Tests/Web/Controllers/ContentControllerUnitTests.cs @@ -5,6 +5,7 @@ using NUnit.Framework; using Umbraco.Core.Models; using Umbraco.Core.Models.Entities; using Umbraco.Core.Models.Membership; +using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Web.Editors; @@ -33,14 +34,14 @@ namespace Umbraco.Tests.Web.Controllers var userService = userServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, 1234); + var result = ContentPermissionsHelper.CheckPermissions(1234, user, userService, contentService, entityService, out var foundContent); //assert - Assert.IsTrue(result); + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Granted, result); } [Test] - public void Throws_Exception_When_No_Content_Found() + public void No_Content_Found() { //arrange var userMock = new Mock(); @@ -60,8 +61,11 @@ namespace Umbraco.Tests.Web.Controllers var entityServiceMock = new Mock(); var entityService = entityServiceMock.Object; - //act/assert - Assert.Throws(() => ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, 1234, new[] { 'F' })); + //act + var result = ContentPermissionsHelper.CheckPermissions(1234, user, userService, contentService, entityService, out var foundContent, new[] { 'F' }); + + //assert + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.NotFound, result); } [Test] @@ -89,10 +93,10 @@ namespace Umbraco.Tests.Web.Controllers var entityService = entityServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, 1234, new[] { 'F' }); + var result = ContentPermissionsHelper.CheckPermissions(1234, user, userService, contentService, entityService, out var foundContent, new[] { 'F' }); //assert - Assert.IsFalse(result); + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Denied, result); } [Test] @@ -120,10 +124,10 @@ namespace Umbraco.Tests.Web.Controllers var entityService = entityServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, 1234, new[] { 'F' }); + var result = ContentPermissionsHelper.CheckPermissions(1234, user, userService, contentService, entityService, out var foundContent, new[] { 'F' }); //assert - Assert.IsFalse(result); + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Denied, result); } [Test] @@ -152,10 +156,10 @@ namespace Umbraco.Tests.Web.Controllers var entityService = entityServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, 1234, new[] { 'F' }); + var result = ContentPermissionsHelper.CheckPermissions(1234, user, userService, contentService, entityService, out var foundContent, new[] { 'F' }); //assert - Assert.IsTrue(result); + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Granted, result); } [Test] @@ -174,10 +178,10 @@ namespace Umbraco.Tests.Web.Controllers var entityService = entityServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -1); + var result = ContentPermissionsHelper.CheckPermissions(-1, user, userService, contentService, entityService, out var foundContent); //assert - Assert.IsTrue(result); + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Granted, result); } [Test] @@ -196,10 +200,10 @@ namespace Umbraco.Tests.Web.Controllers var entityService = entityServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -20); + var result = ContentPermissionsHelper.CheckPermissions(-20, user, userService, contentService, entityService, out var foundContent); //assert - Assert.IsTrue(result); + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Granted, result); } [Test] @@ -220,10 +224,10 @@ namespace Umbraco.Tests.Web.Controllers var entityService = entityServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -20); + var result = ContentPermissionsHelper.CheckPermissions(-20, user, userService, contentService, entityService, out var foundContent); //assert - Assert.IsFalse(result); + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Denied, result); } [Test] @@ -244,10 +248,10 @@ namespace Umbraco.Tests.Web.Controllers var entityService = entityServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -1); + var result = ContentPermissionsHelper.CheckPermissions(-1, user, userService, contentService, entityService, out var foundContent); //assert - Assert.IsFalse(result); + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Denied, result); } [Test] @@ -274,10 +278,10 @@ namespace Umbraco.Tests.Web.Controllers //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -1, new[] { 'A' }); + var result = ContentPermissionsHelper.CheckPermissions(-1, user, userService, contentService, entityService, out var foundContent, new[] { 'A' }); //assert - Assert.IsTrue(result); + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Granted, result); } [Test] @@ -302,10 +306,10 @@ namespace Umbraco.Tests.Web.Controllers var contentService = contentServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -1, new[] { 'B' }); + var result = ContentPermissionsHelper.CheckPermissions(-1, user, userService, contentService, entityService, out var foundContent, new[] { 'B' }); //assert - Assert.IsFalse(result); + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Denied, result); } [Test] @@ -332,10 +336,10 @@ namespace Umbraco.Tests.Web.Controllers var contentService = contentServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -20, new[] { 'A' }); + var result = ContentPermissionsHelper.CheckPermissions(-20, user, userService, contentService, entityService, out var foundContent, new[] { 'A' }); //assert - Assert.IsTrue(result); + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Granted, result); } [Test] @@ -360,10 +364,10 @@ namespace Umbraco.Tests.Web.Controllers var contentService = contentServiceMock.Object; //act - var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, entityService, -20, new[] { 'B' }); + var result = ContentPermissionsHelper.CheckPermissions(-20, user, userService, contentService, entityService, out var foundContent, new[] { 'B' }); //assert - Assert.IsFalse(result); + Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Denied, result); } } diff --git a/src/Umbraco.Web.UI.Client/src/views/content/overlays/publishdescendants.controller.js b/src/Umbraco.Web.UI.Client/src/views/content/overlays/publishdescendants.controller.js index f7455df3bd..c2ffbced04 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/overlays/publishdescendants.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/content/overlays/publishdescendants.controller.js @@ -6,7 +6,6 @@ var vm = this; vm.changeSelection = changeSelection; - vm.includeUnpublishedChanged = includeUnpublishedChanged; function onInit() { diff --git a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml index d9241ff561..aa5b03f020 100644 --- a/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml +++ b/src/Umbraco.Web.UI/Umbraco/config/lang/en_us.xml @@ -1158,6 +1158,10 @@ To manage your website, simply open the Umbraco back office and start adding con If you just want to setup simple protection using a single login and password + Insufficient user permissions to publish all descendant documents + @@ -1165,7 +1169,7 @@ To manage your website, simply open the Umbraco back office and start adding con %0% could not be published because the item has expired. ]]> Insufficient user permissions, could not complete the operation Cancelled Operation was cancelled by a 3rd party add-in - Publishing was cancelled by a 3rd party add-in Property type already exists Property type created DataType: %1%]]> @@ -1286,10 +1289,11 @@ To manage your website, simply open the Umbraco back office and start adding con Stylesheet saved without any errors Datatype saved Dictionary item saved - Publishing failed because the parent page isn't published Content published and is visible on the website + %0% documents published and visible on the website %0% published and visible on the website + %0% documents published for languages %1% and visible on the website Content saved Remember to publish to make changes visible A schedule for publishing has been updated diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 2e545cfc76..39e7053518 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -38,7 +38,8 @@ using Umbraco.Web.Actions; using Umbraco.Web.ContentApps; using Umbraco.Web.Editors.Binders; using Umbraco.Web.Editors.Filters; - +using Umbraco.Core.Models.Entities; +using Umbraco.Core.Security; namespace Umbraco.Web.Editors { @@ -738,6 +739,27 @@ namespace Umbraco.Web.Editors case ContentSaveAction.PublishNew: { var publishStatus = PublishInternal(contentItem, out wasCancelled, out var successfulCultures); + //global notifications + AddMessageForPublishStatus(new[] { publishStatus }, globalNotifications, successfulCultures); + //variant specific notifications + foreach (var c in successfulCultures) + AddMessageForPublishStatus(new[] { publishStatus }, notifications.GetOrCreate(c), successfulCultures); + } + break; + case ContentSaveAction.PublishWithDescendants: + case ContentSaveAction.PublishWithDescendantsNew: + { + if (!ValidatePublishBranchPermissions(contentItem, out var noAccess)) + { + globalNotifications.AddErrorNotification( + Services.TextService.Localize("publish"), + Services.TextService.Localize("publish/invalidPublishBranchPermissions")); + wasCancelled = false; + break; + } + + var publishStatus = PublishBranchInternal(contentItem, false, out wasCancelled, out var successfulCultures); + //global notifications AddMessageForPublishStatus(publishStatus, globalNotifications, successfulCultures); //variant specific notifications @@ -745,30 +767,25 @@ namespace Umbraco.Web.Editors AddMessageForPublishStatus(publishStatus, notifications.GetOrCreate(c), successfulCultures); } break; - case ContentSaveAction.PublishWithDescendants: - case ContentSaveAction.PublishWithDescendantsNew: - { - var publishStatus = PublishBranchInternal(contentItem, false, out wasCancelled, out var successfulCultures); - - //TODO: Deal with the multiple result - ////global notifications - //AddMessageForPublishStatus(publishStatus, globalNotifications, successfulCultures); - ////variant specific notifications - //foreach (var c in successfulCultures) - // AddMessageForPublishStatus(publishStatus, notifications.GetOrCreate(c), successfulCultures); - } - break; case ContentSaveAction.PublishWithDescendantsForce: case ContentSaveAction.PublishWithDescendantsForceNew: { + if (!ValidatePublishBranchPermissions(contentItem, out var noAccess)) + { + globalNotifications.AddErrorNotification( + Services.TextService.Localize("publish"), + Services.TextService.Localize("publish/invalidPublishBranchPermissions")); + wasCancelled = false; + break; + } + var publishStatus = PublishBranchInternal(contentItem, true, out wasCancelled, out var successfulCultures); - //TODO: Deal with the multiple result - ////global notifications - //AddMessageForPublishStatus(publishStatus, globalNotifications, successfulCultures); - ////variant specific notifications - //foreach (var c in successfulCultures) - // AddMessageForPublishStatus(publishStatus, notifications.GetOrCreate(c), successfulCultures); + //global notifications + AddMessageForPublishStatus(publishStatus, globalNotifications, successfulCultures); + //variant specific notifications + foreach (var c in successfulCultures) + AddMessageForPublishStatus(publishStatus, notifications.GetOrCreate(c), successfulCultures); } break; default: @@ -1039,6 +1056,40 @@ namespace Umbraco.Web.Editors notifications.GetOrCreate(culture).AddSuccessNotification(header, msg); } + /// + /// The user must have publish access to all descendant nodes of the content item in order to continue + /// + /// + /// + private bool ValidatePublishBranchPermissions(ContentItemSave contentItem, out IReadOnlyList noAccess) + { + var denied = new List(); + var page = 0; + const int pageSize = 500; + var total = long.MaxValue; + while (page * pageSize < total) + { + var descendants = Services.EntityService.GetPagedDescendants(contentItem.Id, UmbracoObjectTypes.Document, page++, pageSize, out total, + //order by shallowest to deepest, this allows us to check permissions from top to bottom so we can exit + //early if a permission higher up fails + "path", Direction.Ascending); + + foreach (var c in descendants) + { + //if this item's path has already been denied or if the user doesn't have access to it, add to the deny list + if (denied.Any(x => c.Path.StartsWith($"{x.Path},")) + || (ContentPermissionsHelper.CheckPermissions(c, + Security.CurrentUser, Services.UserService, Services.EntityService, + ActionPublish.ActionLetter) == ContentPermissionsHelper.ContentAccess.Denied)) + { + denied.Add(c); + } + } + } + noAccess = denied; + return denied.Count == 0; + } + private IEnumerable PublishBranchInternal(ContentItemSave contentItem, bool force, out bool wasCancelled, out string[] successfulCultures) { @@ -1269,7 +1320,7 @@ namespace Umbraco.Web.Editors if (publishResult.Success == false) { var notificationModel = new SimpleNotificationModel(); - AddMessageForPublishStatus(publishResult, notificationModel); + AddMessageForPublishStatus(new [] { publishResult }, notificationModel); return Request.CreateValidationErrorResponse(notificationModel); } @@ -1817,152 +1868,124 @@ namespace Umbraco.Web.Editors /// /// This is null when dealing with invariant content, else it's the cultures that were succesfully published /// - private void AddMessageForPublishStatus(PublishResult status, INotificationModel display, string[] successfulCultures = null) + private void AddMessageForPublishStatus(IEnumerable statuses, INotificationModel display, string[] successfulCultures = null) { - switch (status.Result) + //Put the statuses into groups, each group results in a different message + var statusGroup = statuses.GroupBy(x => { - case PublishResultType.SuccessPublish: - case PublishResultType.SuccessPublishAlready: - case PublishResultType.SuccessPublishCulture: - if (successfulCultures == null) - { - display.AddSuccessNotification( - Services.TextService.Localize("speechBubbles/editContentPublishedHeader"), - Services.TextService.Localize("speechBubbles/editContentPublishedText")); - } - else - { - foreach (var c in successfulCultures) + switch (x.Result) + { + case PublishResultType.SuccessPublish: + case PublishResultType.SuccessPublishAlready: + case PublishResultType.SuccessPublishCulture: + //these 3 belong to a single group + return PublishResultType.SuccessPublish; + case PublishResultType.FailedPublishAwaitingRelease: + case PublishResultType.FailedPublishCultureAwaitingRelease: + //these 2 belong to a single group + return PublishResultType.FailedPublishAwaitingRelease; + case PublishResultType.FailedPublishHasExpired: + case PublishResultType.FailedPublishCultureHasExpired: + //these 2 belong to a single group + return PublishResultType.FailedPublishHasExpired; + case PublishResultType.FailedPublishPathNotPublished: + case PublishResultType.FailedPublishCancelledByEvent: + case PublishResultType.FailedPublishIsTrashed: + case PublishResultType.FailedPublishContentInvalid: + case PublishResultType.FailedPublishMandatoryCultureMissing: + //the rest that we are looking for each belong in their own group + return x.Result; + default: + throw new IndexOutOfRangeException($"{x.Result}\" was not expected."); + } + }); + + foreach (var status in statusGroup) + { + switch (status.Key) + { + case PublishResultType.SuccessPublish: + var itemCount = status.Count(); + if (successfulCultures == null) { display.AddSuccessNotification( Services.TextService.Localize("speechBubbles/editContentPublishedHeader"), - Services.TextService.Localize("speechBubbles/editVariantPublishedText", new[] { _allLangs.Value[c].CultureName })); + itemCount > 1 + ? Services.TextService.Localize("speechBubbles/editContentPublishedText") + : Services.TextService.Localize("speechBubbles/editMultiContentPublishedText", new[] { itemCount.ToInvariantString() })); } - } - break; - case PublishResultType.FailedPublishPathNotPublished: - display.AddWarningNotification( + else + { + foreach (var c in successfulCultures) + { + display.AddSuccessNotification( + Services.TextService.Localize("speechBubbles/editContentPublishedHeader"), + itemCount > 1 + ? Services.TextService.Localize("speechBubbles/editMultiVariantPublishedText", new[] { itemCount.ToInvariantString(), _allLangs.Value[c].CultureName }) + : Services.TextService.Localize("speechBubbles/editVariantPublishedText", new[] { _allLangs.Value[c].CultureName })); + } + } + break; + case PublishResultType.FailedPublishPathNotPublished: + { + var names = string.Join(", ", status.Select(x => $"{x.Content.Name} ({x.Content.Id})")); + display.AddWarningNotification( + Services.TextService.Localize("publish"), + Services.TextService.Localize("publish/contentPublishedFailedByParent", + new[] { names }).Trim()); + } + break; + case PublishResultType.FailedPublishCancelledByEvent: + { + var names = string.Join(", ", status.Select(x => $"{x.Content.Name} ({x.Content.Id})")); + AddCancelMessage(display, message: "publish/contentPublishedFailedByEvent", messageParams: new[] { names }); + } + break; + case PublishResultType.FailedPublishAwaitingRelease: + { + var names = string.Join(", ", status.Select(x => $"{x.Content.Name} ({x.Content.Id})")); + display.AddWarningNotification( + Services.TextService.Localize("publish"), + Services.TextService.Localize("publish/contentPublishedFailedAwaitingRelease", + new[] { names }).Trim()); + } + break; + case PublishResultType.FailedPublishHasExpired: + { + var names = string.Join(", ", status.Select(x => $"{x.Content.Name} ({x.Content.Id})")); + display.AddWarningNotification( + Services.TextService.Localize("publish"), + Services.TextService.Localize("publish/contentPublishedFailedExpired", + new[] { names }).Trim()); + } + break; + case PublishResultType.FailedPublishIsTrashed: + { + var names = string.Join(", ", status.Select(x => $"{x.Content.Name} ({x.Content.Id})")); + display.AddWarningNotification( + Services.TextService.Localize("publish"), + Services.TextService.Localize("publish/contentPublishedFailedIsTrashed", + new[] { names }).Trim()); + } + break; + case PublishResultType.FailedPublishContentInvalid: + { + var names = string.Join(", ", status.Select(x => $"{x.Content.Name} ({x.Content.Id})")); + display.AddWarningNotification( + Services.TextService.Localize("publish"), + Services.TextService.Localize("publish/contentPublishedFailedInvalid", + new[] { names }).Trim()); + } + break; + case PublishResultType.FailedPublishMandatoryCultureMissing: + display.AddWarningNotification( Services.TextService.Localize("publish"), - Services.TextService.Localize("publish/contentPublishedFailedByParent", - new[] { $"{status.Content.Name} ({status.Content.Id})" }).Trim()); - break; - case PublishResultType.FailedPublishCancelledByEvent: - AddCancelMessage(display, "publish", "speechBubbles/contentPublishedFailedByEvent"); - break; - case PublishResultType.FailedPublishAwaitingRelease: - case PublishResultType.FailedPublishCultureAwaitingRelease: - //TODO: We'll need to deal with variants here eventually - display.AddWarningNotification( - Services.TextService.Localize("publish"), - Services.TextService.Localize("publish/contentPublishedFailedAwaitingRelease", - new[] { $"{status.Content.Name} ({status.Content.Id})" }).Trim()); - break; - case PublishResultType.FailedPublishHasExpired: - case PublishResultType.FailedPublishCultureHasExpired: - //TODO: We'll need to deal with variants here eventually - display.AddWarningNotification( - Services.TextService.Localize("publish"), - Services.TextService.Localize("publish/contentPublishedFailedExpired", - new[] { $"{status.Content.Name} ({status.Content.Id})", }).Trim()); - break; - case PublishResultType.FailedPublishIsTrashed: - display.AddWarningNotification( - Services.TextService.Localize("publish"), - "publish/contentPublishedFailedIsTrashed"); // fixme properly localize, these keys are missing from lang files! - break; - case PublishResultType.FailedPublishContentInvalid: - display.AddWarningNotification( - Services.TextService.Localize("publish"), - Services.TextService.Localize("publish/contentPublishedFailedInvalid", - new[] - { - $"{status.Content.Name} ({status.Content.Id})", - string.Join(",", status.InvalidProperties.Select(x => x.Alias)) - }).Trim()); - break; - case PublishResultType.FailedPublishMandatoryCultureMissing: - display.AddWarningNotification( - Services.TextService.Localize("publish"), - "publish/contentPublishedFailedByCulture"); // fixme properly localize, these keys are missing from lang files! - break; - default: - throw new IndexOutOfRangeException($"PublishedResultType \"{status.Result}\" was not expected."); - } - } - - /// - /// Performs a permissions check for the user to check if it has access to the node based on - /// start node and/or permissions for the node - /// - /// The storage to add the content item to so it can be reused - /// - /// - /// - /// - /// The content to lookup, if the contentItem is not specified - /// - /// Specifies the already resolved content item to check against - /// - internal static bool CheckPermissions( - IDictionary storage, - IUser user, - IUserService userService, - IContentService contentService, - IEntityService entityService, - int nodeId, - char[] permissionsToCheck = null, - IContent contentItem = null) - { - if (storage == null) throw new ArgumentNullException("storage"); - if (user == null) throw new ArgumentNullException("user"); - if (userService == null) throw new ArgumentNullException("userService"); - if (contentService == null) throw new ArgumentNullException("contentService"); - if (entityService == null) throw new ArgumentNullException("entityService"); - - if (contentItem == null && nodeId != Constants.System.Root && nodeId != Constants.System.RecycleBinContent) - { - contentItem = contentService.GetById(nodeId); - //put the content item into storage so it can be retreived - // in the controller (saves a lookup) - storage[typeof(IContent).ToString()] = contentItem; - } - - if (contentItem == null && nodeId != Constants.System.Root && nodeId != Constants.System.RecycleBinContent) - { - throw new HttpResponseException(HttpStatusCode.NotFound); - } - - var hasPathAccess = (nodeId == Constants.System.Root) - ? user.HasContentRootAccess(entityService) - : (nodeId == Constants.System.RecycleBinContent) - ? user.HasContentBinAccess(entityService) - : user.HasPathAccess(contentItem, entityService); - - if (hasPathAccess == false) - { - return false; - } - - if (permissionsToCheck == null || permissionsToCheck.Length == 0) - { - return true; - } - - //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.GetAllPermissions().Contains(p.ToString(CultureInfo.InvariantCulture)) == false) - { - allowed = false; + "publish/contentPublishedFailedByCulture"); // fixme properly localize, these keys are missing from lang files! + break; + default: + throw new IndexOutOfRangeException($"PublishedResultType \"{status.Key}\" was not expected."); } } - return allowed; } /// diff --git a/src/Umbraco.Web/Editors/ContentControllerBase.cs b/src/Umbraco.Web/Editors/ContentControllerBase.cs index fd58a8a236..ce88c7c935 100644 --- a/src/Umbraco.Web/Editors/ContentControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentControllerBase.cs @@ -148,7 +148,9 @@ namespace Umbraco.Web.Editors string header = "speechBubbles/operationCancelledHeader", string message = "speechBubbles/operationCancelledText", bool localizeHeader = true, - bool localizeMessage = true) + bool localizeMessage = true, + string[] headerParams = null, + string[] messageParams = null) { //if there's already a default event message, don't add our default one //fixme inject @@ -156,8 +158,8 @@ namespace Umbraco.Web.Editors if (msgs != null && msgs.GetAll().Any(x => x.IsDefaultEventMessage)) return; display.AddWarningNotification( - localizeHeader ? Services.TextService.Localize(header) : header, - localizeMessage ? Services.TextService.Localize(message): message); + localizeHeader ? Services.TextService.Localize(header, headerParams) : header, + localizeMessage ? Services.TextService.Localize(message, messageParams): message); } } } diff --git a/src/Umbraco.Web/Editors/Filters/ContentSaveValidationAttribute.cs b/src/Umbraco.Web/Editors/Filters/ContentSaveValidationAttribute.cs index 7aae0926ad..789dd1fdf2 100644 --- a/src/Umbraco.Web/Editors/Filters/ContentSaveValidationAttribute.cs +++ b/src/Umbraco.Web/Editors/Filters/ContentSaveValidationAttribute.cs @@ -3,11 +3,13 @@ using System.Collections.Generic; using System.Linq; using System.Net; using System.Net.Http; +using System.Web.Http; using System.Web.Http.Controllers; using System.Web.Http.Filters; using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Models; +using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Web.Actions; using Umbraco.Web.Composing; @@ -15,7 +17,6 @@ using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Security; using Umbraco.Web.WebApi; - namespace Umbraco.Web.Editors.Filters { /// @@ -100,6 +101,8 @@ namespace Umbraco.Web.Editors.Filters contentIdToCheck = contentToCheck.Id; break; case ContentSaveAction.Publish: + case ContentSaveAction.PublishWithDescendants: + case ContentSaveAction.PublishWithDescendantsForce: permissionToCheck.Add(ActionPublish.ActionLetter); contentToCheck = contentItem.PersistedContent; contentIdToCheck = contentToCheck.Id; @@ -146,6 +149,8 @@ namespace Umbraco.Web.Editors.Filters } break; case ContentSaveAction.PublishNew: + case ContentSaveAction.PublishWithDescendantsNew: + case ContentSaveAction.PublishWithDescendantsForceNew: //Publish new requires both ActionNew AND ActionPublish //TODO: Shoudn't publish also require ActionUpdate since it will definitely perform an update to publish but maybe that's just implied @@ -182,17 +187,34 @@ namespace Umbraco.Web.Editors.Filters throw new ArgumentOutOfRangeException(); } - if (ContentController.CheckPermissions( - actionContext.Request.Properties, - _security.CurrentUser, - _userService, _contentService, _entityService, - contentIdToCheck, permissionToCheck.ToArray(), contentToCheck) == false) + ContentPermissionsHelper.ContentAccess accessResult; + if (contentToCheck != null) { - actionContext.Response = actionContext.Request.CreateUserNoAccessResponse(); - return false; + //store the content item in request cache so it can be resolved in the controller without re-looking it up + actionContext.Request.Properties[typeof(IContent).ToString()] = contentItem; + + accessResult = ContentPermissionsHelper.CheckPermissions( + contentToCheck, _security.CurrentUser, + _userService, _entityService, permissionToCheck.ToArray()); + } + else + { + accessResult = ContentPermissionsHelper.CheckPermissions( + contentIdToCheck, _security.CurrentUser, + _userService, _contentService, _entityService, + out contentToCheck, + permissionToCheck.ToArray()); + if (contentToCheck != null) + { + //store the content item in request cache so it can be resolved in the controller without re-looking it up + actionContext.Request.Properties[typeof(IContent).ToString()] = contentToCheck; + } } - return true; + if (accessResult == ContentPermissionsHelper.ContentAccess.NotFound) + throw new HttpResponseException(HttpStatusCode.NotFound); + + return accessResult == ContentPermissionsHelper.ContentAccess.Granted; } } } diff --git a/src/Umbraco.Web/Editors/MediaController.cs b/src/Umbraco.Web/Editors/MediaController.cs index f61cbc5952..a97e181a5e 100644 --- a/src/Umbraco.Web/Editors/MediaController.cs +++ b/src/Umbraco.Web/Editors/MediaController.cs @@ -720,8 +720,7 @@ namespace Umbraco.Web.Editors if (saveResult == false) { AddCancelMessage(tempFiles, - message: Services.TextService.Localize("speechBubbles/operationCancelledText") + " -- " + mediaItemName, - localizeMessage: false); + message: Services.TextService.Localize("speechBubbles/operationCancelledText") + " -- " + mediaItemName); } else { diff --git a/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs b/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs index e0e9a8ed55..320580aaf9 100644 --- a/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs +++ b/src/Umbraco.Web/Editors/UserEditorAuthorizationHelper.cs @@ -3,6 +3,7 @@ using System.Linq; using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; +using Umbraco.Core.Security; using Umbraco.Core.Services; namespace Umbraco.Web.Editors @@ -113,7 +114,7 @@ namespace Umbraco.Web.Editors { if (contentId == Constants.System.Root) { - var hasAccess = UserExtensions.HasPathAccess("-1", currentUser.CalculateContentStartNodeIds(_entityService), Constants.System.RecycleBinContent); + var hasAccess = ContentPermissionsHelper.HasPathAccess("-1", currentUser.CalculateContentStartNodeIds(_entityService), Constants.System.RecycleBinContent); if (hasAccess == false) return Attempt.Fail("The current user does not have access to the content root"); } @@ -134,7 +135,7 @@ namespace Umbraco.Web.Editors { if (mediaId == Constants.System.Root) { - var hasAccess = UserExtensions.HasPathAccess("-1", currentUser.CalculateMediaStartNodeIds(_entityService), Constants.System.RecycleBinMedia); + var hasAccess = ContentPermissionsHelper.HasPathAccess("-1", currentUser.CalculateMediaStartNodeIds(_entityService), Constants.System.RecycleBinMedia); if (hasAccess == false) return Attempt.Fail("The current user does not have access to the media root"); } diff --git a/src/Umbraco.Web/Trees/ContentTreeController.cs b/src/Umbraco.Web/Trees/ContentTreeController.cs index 04c356a4fd..ddb7170508 100644 --- a/src/Umbraco.Web/Trees/ContentTreeController.cs +++ b/src/Umbraco.Web/Trees/ContentTreeController.cs @@ -65,38 +65,36 @@ namespace Umbraco.Web.Trees queryStrings, hasChildren); - // entity is either a container, or a document + // set container style if it is one if (isContainer) { node.AdditionalData.Add("isContainer", true); node.SetContainerStyle(); } + + var documentEntity = (IDocumentEntitySlim)entity; + + if (!documentEntity.Variations.VariesByCulture()) + { + if (!documentEntity.Published) + node.SetNotPublishedStyle(); + else if (documentEntity.Edited) + node.SetHasPendingVersionStyle(); + } else { - var documentEntity = (IDocumentEntitySlim) entity; - - if (!documentEntity.Variations.VariesByCulture()) + if (!culture.IsNullOrWhiteSpace()) { - if (!documentEntity.Published) + if (!documentEntity.Published || !documentEntity.PublishedCultures.Contains(culture)) node.SetNotPublishedStyle(); - else if (documentEntity.Edited) + else if (documentEntity.EditedCultures.Contains(culture)) node.SetHasPendingVersionStyle(); } - else - { - if (!culture.IsNullOrWhiteSpace()) - { - if (!documentEntity.PublishedCultures.Contains(culture)) - node.SetNotPublishedStyle(); - else if (documentEntity.EditedCultures.Contains(culture)) - node.SetHasPendingVersionStyle(); - } - } - - node.AdditionalData.Add("variesByCulture", documentEntity.Variations.VariesByCulture()); - node.AdditionalData.Add("contentType", documentEntity.ContentTypeAlias); } + node.AdditionalData.Add("variesByCulture", documentEntity.Variations.VariesByCulture()); + node.AdditionalData.Add("contentType", documentEntity.ContentTypeAlias); + if (Services.PublicAccessService.IsProtected(entity.Path)) node.SetProtectedStyle(); @@ -161,7 +159,7 @@ namespace Umbraco.Web.Trees } //if the user has no path access for this node, all they can do is refresh - if (Security.CurrentUser.HasPathAccess(item, Services.EntityService, RecycleBinId) == false) + if (!Security.CurrentUser.HasContentPathAccess(item, Services.EntityService)) { var menu = new MenuItemCollection(); menu.Items.Add(new RefreshNode(Services.TextService, true)); diff --git a/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs b/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs index 0354e62ca7..e5bb15948f 100644 --- a/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs +++ b/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs @@ -232,7 +232,9 @@ namespace Umbraco.Web.Trees protected bool HasPathAccess(IUmbracoEntity entity, FormDataCollection queryStrings) { if (entity == null) return false; - return Security.CurrentUser.HasPathAccess(entity, Services.EntityService, RecycleBinId); + return RecycleBinId == Constants.System.RecycleBinContent + ? Security.CurrentUser.HasContentPathAccess(entity, Services.EntityService) + : Security.CurrentUser.HasMediaPathAccess(entity, Services.EntityService); } /// diff --git a/src/Umbraco.Web/Trees/MediaTreeController.cs b/src/Umbraco.Web/Trees/MediaTreeController.cs index 0292a907fc..61146db246 100644 --- a/src/Umbraco.Web/Trees/MediaTreeController.cs +++ b/src/Umbraco.Web/Trees/MediaTreeController.cs @@ -112,7 +112,7 @@ namespace Umbraco.Web.Trees } //if the user has no path access for this node, all they can do is refresh - if (Security.CurrentUser.HasPathAccess(item, Services.EntityService, RecycleBinId) == false) + if (!Security.CurrentUser.HasMediaPathAccess(item, Services.EntityService)) { menu.Items.Add(new RefreshNode(Services.TextService, true)); return menu; diff --git a/src/Umbraco.Web/UI/Pages/UmbracoEnsuredPage.cs b/src/Umbraco.Web/UI/Pages/UmbracoEnsuredPage.cs index 9f13939bac..f2edcaea35 100644 --- a/src/Umbraco.Web/UI/Pages/UmbracoEnsuredPage.cs +++ b/src/Umbraco.Web/UI/Pages/UmbracoEnsuredPage.cs @@ -54,7 +54,9 @@ namespace Umbraco.Web.UI.Pages var entity = entityId == Constants.System.Root ? EntitySlim.Root : Services.EntityService.Get(entityId, objectType); - var hasAccess = Security.CurrentUser.HasPathAccess(entity, Services.EntityService, objectType == UmbracoObjectTypes.Document ? Constants.System.RecycleBinContent : Constants.System.RecycleBinMedia); + var hasAccess = objectType == UmbracoObjectTypes.Document + ? Security.CurrentUser.HasContentPathAccess(entity, Services.EntityService) + : Security.CurrentUser.HasMediaPathAccess(entity, Services.EntityService); if (hasAccess == false) throw new AuthorizationException($"The current user doesn't have access to the path '{entity.Path}'"); diff --git a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs index 5d71992b74..1601a23bc1 100644 --- a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs @@ -9,6 +9,8 @@ using Umbraco.Web.Editors; using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Web.Actions; +using Umbraco.Core.Security; +using System.Net; namespace Umbraco.Web.WebApi.Filters { @@ -108,25 +110,27 @@ namespace Umbraco.Web.WebApi.Filters nodeId = _nodeId.Value; } - if (ContentController.CheckPermissions( - actionContext.Request.Properties, - //fixme: inject? we can't because this is an attribute but we could provide ctors and empty ctors that pass in the required services + var permissionResult = ContentPermissionsHelper.CheckPermissions(nodeId, Current.UmbracoContext.Security.CurrentUser, Current.Services.UserService, Current.Services.ContentService, Current.Services.EntityService, - nodeId, _permissionToCheck.HasValue ? new[]{_permissionToCheck.Value}: null)) - { - base.OnActionExecuting(actionContext); - } - else - { + out var contentItem, + _permissionToCheck.HasValue ? new[] { _permissionToCheck.Value } : null); + + if (permissionResult == ContentPermissionsHelper.ContentAccess.NotFound) + throw new HttpResponseException(HttpStatusCode.NotFound); + + if (permissionResult == ContentPermissionsHelper.ContentAccess.Denied) throw new HttpResponseException(actionContext.Request.CreateUserNoAccessResponse()); + + if (contentItem != null) + { + //store the content item in request cache so it can be resolved in the controller without re-looking it up + actionContext.Request.Properties[typeof(IContent).ToString()] = contentItem; } + base.OnActionExecuting(actionContext); } - - - } } diff --git a/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingMediaAttribute.cs b/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingMediaAttribute.cs index deaf7628e0..4a4c5b98a0 100644 --- a/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingMediaAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingMediaAttribute.cs @@ -8,6 +8,7 @@ using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; using Umbraco.Core.Composing; +using Umbraco.Core.Security; namespace Umbraco.Web.WebApi.Filters { @@ -79,7 +80,7 @@ namespace Umbraco.Web.WebApi.Filters var toRemove = new List(); foreach (dynamic item in items) { - var hasPathAccess = (item != null && UserExtensions.HasPathAccess(item.Path, GetUserStartNodes(user), RecycleBinId)); + var hasPathAccess = (item != null && ContentPermissionsHelper.HasPathAccess(item.Path, GetUserStartNodes(user), RecycleBinId)); if (hasPathAccess == false) { toRemove.Add(item);