From 496ecf5c9a6770765181a3a5a45dfa39fca3f77d Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 15 Nov 2018 15:24:09 +1100 Subject: [PATCH] Cleans up how the content path permissions are checked, integrates the bulk publishing into the controller, adds security check to ensure the user can publish the whole branch, updates the UI messaging so they are grouped by publish statuses and deals with multiple publish status (i.e. in bulk), fixes some content tree issues --- src/Umbraco.Core/Models/UserExtensions.cs | 99 +---- .../Security/ContentPermissionsHelper.cs | 263 ++++++++++++++ src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../Controllers/ContentControllerUnitTests.cs | 58 +-- .../overlays/publishdescendants.controller.js | 1 - .../Umbraco/config/lang/en_us.xml | 10 +- src/Umbraco.Web/Editors/ContentController.cs | 339 ++++++++++-------- .../Editors/ContentControllerBase.cs | 8 +- .../Filters/ContentSaveValidationAttribute.cs | 40 ++- src/Umbraco.Web/Editors/MediaController.cs | 3 +- .../Editors/UserEditorAuthorizationHelper.cs | 5 +- .../Trees/ContentTreeController.cs | 38 +- .../Trees/ContentTreeControllerBase.cs | 4 +- src/Umbraco.Web/Trees/MediaTreeController.cs | 2 +- .../UI/Pages/UmbracoEnsuredPage.cs | 4 +- ...EnsureUserPermissionForContentAttribute.cs | 28 +- .../FilterAllowedOutgoingMediaAttribute.cs | 3 +- 17 files changed, 583 insertions(+), 323 deletions(-) create mode 100644 src/Umbraco.Core/Security/ContentPermissionsHelper.cs 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);