diff --git a/src/Umbraco.Core/Models/UserExtensions.cs b/src/Umbraco.Core/Models/UserExtensions.cs index 295b764834..6a3ea16ff7 100644 --- a/src/Umbraco.Core/Models/UserExtensions.cs +++ b/src/Umbraco.Core/Models/UserExtensions.cs @@ -92,46 +92,46 @@ namespace Umbraco.Core.Models public static bool HasContentRootAccess(this IUser user, IEntityService entityService) { - return ContentPermissionsHelper.HasPathAccess(Constants.System.RootString, user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); + return ContentPermissions.HasPathAccess(Constants.System.RootString, user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); } public static bool HasContentBinAccess(this IUser user, IEntityService entityService) { - return ContentPermissionsHelper.HasPathAccess(Constants.System.RecycleBinContentString, user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); + return ContentPermissions.HasPathAccess(Constants.System.RecycleBinContentString, user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); } public static bool HasMediaRootAccess(this IUser user, IEntityService entityService) { - return ContentPermissionsHelper.HasPathAccess(Constants.System.RootString, user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); + return ContentPermissions.HasPathAccess(Constants.System.RootString, user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); } public static bool HasMediaBinAccess(this IUser user, IEntityService entityService) { - return ContentPermissionsHelper.HasPathAccess(Constants.System.RecycleBinMediaString, user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); + return ContentPermissions.HasPathAccess(Constants.System.RecycleBinMediaString, user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); } public static bool HasPathAccess(this IUser user, IContent content, IEntityService entityService) { if (content == null) throw new ArgumentNullException(nameof(content)); - return ContentPermissionsHelper.HasPathAccess(content.Path, user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); + return ContentPermissions.HasPathAccess(content.Path, user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); } public static bool HasPathAccess(this IUser user, IMedia media, IEntityService entityService) { if (media == null) throw new ArgumentNullException(nameof(media)); - return ContentPermissionsHelper.HasPathAccess(media.Path, user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); + return ContentPermissions.HasPathAccess(media.Path, user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); } public static bool HasContentPathAccess(this IUser user, IUmbracoEntity entity, IEntityService entityService) { if (entity == null) throw new ArgumentNullException(nameof(entity)); - return ContentPermissionsHelper.HasPathAccess(entity.Path, user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); + return ContentPermissions.HasPathAccess(entity.Path, user.CalculateContentStartNodeIds(entityService), Constants.System.RecycleBinContent); } public static bool HasMediaPathAccess(this IUser user, IUmbracoEntity entity, IEntityService entityService) { if (entity == null) throw new ArgumentNullException(nameof(entity)); - return ContentPermissionsHelper.HasPathAccess(entity.Path, user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); + return ContentPermissions.HasPathAccess(entity.Path, user.CalculateMediaStartNodeIds(entityService), Constants.System.RecycleBinMedia); } /// diff --git a/src/Umbraco.Core/Security/ContentPermissionsHelper.cs b/src/Umbraco.Core/Security/ContentPermissions.cs similarity index 69% rename from src/Umbraco.Core/Security/ContentPermissionsHelper.cs rename to src/Umbraco.Core/Security/ContentPermissions.cs index 7f0a5c3c24..b598897133 100644 --- a/src/Umbraco.Core/Security/ContentPermissionsHelper.cs +++ b/src/Umbraco.Core/Security/ContentPermissions.cs @@ -2,8 +2,6 @@ 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; @@ -11,8 +9,16 @@ using Umbraco.Core.Services; namespace Umbraco.Core.Security { - public class ContentPermissionsHelper + + /// + /// Checks user access to content + /// + public class ContentPermissions { + private readonly IUserService _userService; + private readonly IContentService _contentService; + private readonly IEntityService _entityService; + public enum ContentAccess { Granted, @@ -20,56 +26,68 @@ namespace Umbraco.Core.Security NotFound } - public static ContentAccess CheckPermissions( + public ContentPermissions( + IUserService userService, + IContentService contentService, + IEntityService entityService) + { + _userService = userService; + _contentService = contentService; + _entityService = entityService; + } + + public ContentAccess CheckPermissions( IContent content, IUser user, - IUserService userService, - IEntityService entityService, - params char[] permissionsToCheck) + char permissionToCheck) => CheckPermissions(content, user, new[] { permissionToCheck }); + + public ContentAccess CheckPermissions( + IContent content, + IUser user, + IReadOnlyList permissionsToCheck) { - if (user == null) throw new ArgumentNullException("user"); - if (userService == null) throw new ArgumentNullException("userService"); - if (entityService == null) throw new ArgumentNullException("entityService"); + if (user == null) throw new ArgumentNullException(nameof(user)); if (content == null) return ContentAccess.NotFound; - var hasPathAccess = user.HasPathAccess(content, entityService); + var hasPathAccess = user.HasPathAccess(content, _entityService); if (hasPathAccess == false) return ContentAccess.Denied; - if (permissionsToCheck == null || permissionsToCheck.Length == 0) + if (permissionsToCheck == null || permissionsToCheck.Count == 0) return ContentAccess.Granted; //get the implicit/inherited permissions for the user for this path - return CheckPermissionsPath(content.Path, user, userService, permissionsToCheck) + return CheckPermissionsPath(content.Path, user) ? ContentAccess.Granted : ContentAccess.Denied; } - public static ContentAccess CheckPermissions( + public ContentAccess CheckPermissions( IUmbracoEntity entity, IUser user, - IUserService userService, - IEntityService entityService, - params char[] permissionsToCheck) + char permissionToCheck) => CheckPermissions(entity, user, new[] { permissionToCheck }); + + public ContentAccess CheckPermissions( + IUmbracoEntity entity, + IUser user, + IReadOnlyList permissionsToCheck) { - if (user == null) throw new ArgumentNullException("user"); - if (userService == null) throw new ArgumentNullException("userService"); - if (entityService == null) throw new ArgumentNullException("entityService"); + if (user == null) throw new ArgumentNullException(nameof(user)); if (entity == null) return ContentAccess.NotFound; - var hasPathAccess = user.HasContentPathAccess(entity, entityService); + var hasPathAccess = user.HasContentPathAccess(entity, _entityService); if (hasPathAccess == false) return ContentAccess.Denied; - if (permissionsToCheck == null || permissionsToCheck.Length == 0) + if (permissionsToCheck == null || permissionsToCheck.Count == 0) return ContentAccess.Granted; //get the implicit/inherited permissions for the user for this path - return CheckPermissionsPath(entity.Path, user, userService, permissionsToCheck) + return CheckPermissionsPath(entity.Path, user) ? ContentAccess.Granted : ContentAccess.Denied; } @@ -84,41 +102,42 @@ namespace Umbraco.Core.Security /// The item resolved if one was found for the id /// /// - public static ContentAccess CheckPermissions( + public ContentAccess CheckPermissions( int nodeId, IUser user, - IUserService userService, - IEntityService entityService, out IUmbracoEntity entity, - params char[] permissionsToCheck) + IReadOnlyList permissionsToCheck = null) { - if (user == null) throw new ArgumentNullException("user"); - if (userService == null) throw new ArgumentNullException("userService"); - if (entityService == null) throw new ArgumentNullException("entityService"); + if (user == null) throw new ArgumentNullException(nameof(user)); + + if (permissionsToCheck == null) + { + permissionsToCheck = Array.Empty(); + } bool? hasPathAccess = null; entity = null; if (nodeId == Constants.System.Root) - hasPathAccess = user.HasContentRootAccess(entityService); + hasPathAccess = user.HasContentRootAccess(_entityService); else if (nodeId == Constants.System.RecycleBinContent) - hasPathAccess = user.HasContentBinAccess(entityService); + hasPathAccess = user.HasContentBinAccess(_entityService); if (hasPathAccess.HasValue) return hasPathAccess.Value ? ContentAccess.Granted : ContentAccess.Denied; - entity = entityService.Get(nodeId, UmbracoObjectTypes.Document); + entity = _entityService.Get(nodeId, UmbracoObjectTypes.Document); if (entity == null) return ContentAccess.NotFound; - hasPathAccess = user.HasContentPathAccess(entity, entityService); + hasPathAccess = user.HasContentPathAccess(entity, _entityService); if (hasPathAccess == false) return ContentAccess.Denied; - if (permissionsToCheck == null || permissionsToCheck.Length == 0) + if (permissionsToCheck == null || permissionsToCheck.Count == 0) return ContentAccess.Granted; //get the implicit/inherited permissions for the user for this path - return CheckPermissionsPath(entity.Path, user, userService, permissionsToCheck) + return CheckPermissionsPath(entity.Path, user, permissionsToCheck) ? ContentAccess.Granted : ContentAccess.Denied; } @@ -134,52 +153,56 @@ namespace Umbraco.Core.Security /// The item resolved if one was found for the id /// /// - public static ContentAccess CheckPermissions( + public ContentAccess CheckPermissions( int nodeId, - IUser user, - IUserService userService, - IContentService contentService, - IEntityService entityService, + IUser user, out IContent contentItem, - params char[] permissionsToCheck) + IReadOnlyList permissionsToCheck = null) { - 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 (user == null) throw new ArgumentNullException(nameof(user)); + + if (permissionsToCheck == null) + { + permissionsToCheck = Array.Empty(); + } bool? hasPathAccess = null; contentItem = null; if (nodeId == Constants.System.Root) - hasPathAccess = user.HasContentRootAccess(entityService); + hasPathAccess = user.HasContentRootAccess(_entityService); else if (nodeId == Constants.System.RecycleBinContent) - hasPathAccess = user.HasContentBinAccess(entityService); + hasPathAccess = user.HasContentBinAccess(_entityService); if (hasPathAccess.HasValue) return hasPathAccess.Value ? ContentAccess.Granted : ContentAccess.Denied; - contentItem = contentService.GetById(nodeId); + contentItem = _contentService.GetById(nodeId); if (contentItem == null) return ContentAccess.NotFound; - hasPathAccess = user.HasPathAccess(contentItem, entityService); + hasPathAccess = user.HasPathAccess(contentItem, _entityService); if (hasPathAccess == false) return ContentAccess.Denied; - if (permissionsToCheck == null || permissionsToCheck.Length == 0) + if (permissionsToCheck == null || permissionsToCheck.Count == 0) return ContentAccess.Granted; //get the implicit/inherited permissions for the user for this path - return CheckPermissionsPath(contentItem.Path, user, userService, permissionsToCheck) + return CheckPermissionsPath(contentItem.Path, user, permissionsToCheck) ? ContentAccess.Granted : ContentAccess.Denied; } - private static bool CheckPermissionsPath(string path, IUser user, IUserService userService, params char[] permissionsToCheck) + private bool CheckPermissionsPath(string path, IUser user, IReadOnlyList permissionsToCheck = null) { + if (permissionsToCheck == null) + { + permissionsToCheck = Array.Empty(); + } + //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 permission = _userService.GetPermissionsForPath(user, path); var allowed = true; foreach (var p in permissionsToCheck) diff --git a/src/Umbraco.Core/Security/MediaPermissions.cs b/src/Umbraco.Core/Security/MediaPermissions.cs new file mode 100644 index 0000000000..65da95ec02 --- /dev/null +++ b/src/Umbraco.Core/Security/MediaPermissions.cs @@ -0,0 +1,75 @@ +using System; +using System.Collections.Generic; +using Umbraco.Core.Models; +using Umbraco.Core.Models.Membership; +using Umbraco.Core.Services; + +namespace Umbraco.Core.Security +{ + /// + /// Checks user access to media + /// + public class MediaPermissions + { + private readonly IMediaService _mediaService; + private readonly IEntityService _entityService; + + public enum MediaAccess + { + Granted, + Denied, + NotFound + } + + public MediaPermissions(IMediaService mediaService, IEntityService entityService) + { + _mediaService = mediaService; + _entityService = entityService; + } + + /// + /// 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 content to lookup, if the contentItem is not specified + /// + public MediaAccess CheckPermissions(IUser user, int nodeId, out IMedia media) + { + if (user == null) throw new ArgumentNullException(nameof(user)); + + media = null; + + if (nodeId != Constants.System.Root && nodeId != Constants.System.RecycleBinMedia) + { + media = _mediaService.GetById(nodeId); + } + + if (media == null && nodeId != Constants.System.Root && nodeId != Constants.System.RecycleBinMedia) + { + return MediaAccess.NotFound; + } + + var hasPathAccess = (nodeId == Constants.System.Root) + ? user.HasMediaRootAccess(_entityService) + : (nodeId == Constants.System.RecycleBinMedia) + ? user.HasMediaBinAccess(_entityService) + : user.HasPathAccess(media, _entityService); + + return hasPathAccess ? MediaAccess.Granted : MediaAccess.Denied; + } + + public MediaAccess CheckPermissions(IMedia media, IUser user) + { + if (user == null) throw new ArgumentNullException(nameof(user)); + + if (media == null) return MediaAccess.NotFound; + + var hasPathAccess = user.HasPathAccess(media, _entityService); + + return hasPathAccess ? MediaAccess.Granted : MediaAccess.Denied; + } + } +} diff --git a/src/Umbraco.Infrastructure/Editors/UserEditorAuthorizationHelper.cs b/src/Umbraco.Infrastructure/Editors/UserEditorAuthorizationHelper.cs index d19f4d954c..d8a5f675b2 100644 --- a/src/Umbraco.Infrastructure/Editors/UserEditorAuthorizationHelper.cs +++ b/src/Umbraco.Infrastructure/Editors/UserEditorAuthorizationHelper.cs @@ -112,7 +112,7 @@ namespace Umbraco.Web.Editors { if (contentId == Constants.System.Root) { - var hasAccess = ContentPermissionsHelper.HasPathAccess("-1", currentUser.CalculateContentStartNodeIds(_entityService), Constants.System.RecycleBinContent); + var hasAccess = ContentPermissions.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"); } @@ -133,7 +133,7 @@ namespace Umbraco.Web.Editors { if (mediaId == Constants.System.Root) { - var hasAccess = ContentPermissionsHelper.HasPathAccess("-1", currentUser.CalculateMediaStartNodeIds(_entityService), Constants.System.RecycleBinMedia); + var hasAccess = ContentPermissions.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.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerUnitTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/ContentPermissionsTests.cs similarity index 70% rename from src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerUnitTests.cs rename to src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/ContentPermissionsTests.cs index 3df5f76da5..8a7b26c6cf 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/ContentControllerUnitTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/ContentPermissionsTests.cs @@ -8,10 +8,10 @@ using Umbraco.Core.Services; using Umbraco.Tests.Common.Builders; using Umbraco.Tests.Common.Builders.Extensions; -namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers +namespace Umbraco.Tests.UnitTests.Umbraco.Core.Security { [TestFixture] - public class ContentControllerUnitTests + public class ContentPermissionsTests { [Test] public void Access_Allowed_By_Path() @@ -28,12 +28,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var entityService = entityServiceMock.Object; var userServiceMock = new Mock(); var userService = userServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(1234, user, userService, contentService, entityService, out var foundContent); + var result = contentPermissions.CheckPermissions(1234, user, out IContent foundContent); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Granted, result); + Assert.AreEqual(ContentPermissions.ContentAccess.Granted, result); } [Test] @@ -54,12 +55,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var userService = userServiceMock.Object; var entityServiceMock = new Mock(); var entityService = entityServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(1234, user, userService, contentService, entityService, out var foundContent, new[] { 'F' }); + var result = contentPermissions.CheckPermissions(1234, user, out IContent foundContent, new[] { 'F' }); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.NotFound, result); + Assert.AreEqual(ContentPermissions.ContentAccess.NotFound, result); } [Test] @@ -82,12 +84,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers entityServiceMock.Setup(x => x.GetAllPaths(It.IsAny(), It.IsAny())) .Returns(new[] { Mock.Of(entity => entity.Id == 9876 && entity.Path == "-1,9876") }); var entityService = entityServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(1234, user, userService, contentService, entityService, out var foundContent, new[] { 'F' }); + var result = contentPermissions.CheckPermissions(1234, user, out IContent foundContent, new[] { 'F' }); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Denied, result); + Assert.AreEqual(ContentPermissions.ContentAccess.Denied, result); } [Test] @@ -111,12 +114,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var userService = userServiceMock.Object; var entityServiceMock = new Mock(); var entityService = entityServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(1234, user, userService, contentService, entityService, out var foundContent, new[] { 'F' }); + var result = contentPermissions.CheckPermissions(1234, user, out IContent foundContent, new[] { 'F' }); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Denied, result); + Assert.AreEqual(ContentPermissions.ContentAccess.Denied, result); } [Test] @@ -140,12 +144,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var userService = userServiceMock.Object; var entityServiceMock = new Mock(); var entityService = entityServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(1234, user, userService, contentService, entityService, out var foundContent, new[] { 'F' }); + var result = contentPermissions.CheckPermissions(1234, user, out IContent foundContent, new[] { 'F' }); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Granted, result); + Assert.AreEqual(ContentPermissions.ContentAccess.Granted, result); } [Test] @@ -159,12 +164,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var userService = userServiceMock.Object; var entityServiceMock = new Mock(); var entityService = entityServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(-1, user, userService, contentService, entityService, out var foundContent); + var result = contentPermissions.CheckPermissions(-1, user, out IContent _); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Granted, result); + Assert.AreEqual(ContentPermissions.ContentAccess.Granted, result); } [Test] @@ -178,12 +184,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var userService = userServiceMock.Object; var entityServiceMock = new Mock(); var entityService = entityServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(-20, user, userService, contentService, entityService, out var foundContent); + var result = contentPermissions.CheckPermissions(-20, user, out IContent _); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Granted, result); + Assert.AreEqual(ContentPermissions.ContentAccess.Granted, result); } [Test] @@ -199,12 +206,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers entityServiceMock.Setup(x => x.GetAllPaths(It.IsAny(), It.IsAny())) .Returns(new[] { Mock.Of(entity => entity.Id == 1234 && entity.Path == "-1,1234") }); var entityService = entityServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(-20, user, userService, contentService, entityService, out var foundContent); + var result = contentPermissions.CheckPermissions(-20, user, out IContent foundContent); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Denied, result); + Assert.AreEqual(ContentPermissions.ContentAccess.Denied, result); } [Test] @@ -221,12 +229,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers entityServiceMock.Setup(x => x.GetAllPaths(It.IsAny(), It.IsAny())) .Returns(new[] { Mock.Of(entity => entity.Id == 1234 && entity.Path == "-1,1234") }); var entityService = entityServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(-1, user, userService, contentService, entityService, out var foundContent); + var result = contentPermissions.CheckPermissions(-1, user, out IContent foundContent); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Denied, result); + Assert.AreEqual(ContentPermissions.ContentAccess.Denied, result); } [Test] @@ -247,12 +256,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var userService = userServiceMock.Object; var entityServiceMock = new Mock(); var entityService = entityServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(-1, user, userService, contentService, entityService, out var foundContent, new[] { 'A' }); + var result = contentPermissions.CheckPermissions(-1, user, out IContent foundContent, new[] { 'A' }); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Granted, result); + Assert.AreEqual(ContentPermissions.ContentAccess.Granted, result); } [Test] @@ -273,12 +283,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var entityService = entityServiceMock.Object; var contentServiceMock = new Mock(); var contentService = contentServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(-1, user, userService, contentService, entityService, out var foundContent, new[] { 'B' }); + var result = contentPermissions.CheckPermissions(-1, user, out IContent foundContent, new[] { 'B' }); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Denied, result); + Assert.AreEqual(ContentPermissions.ContentAccess.Denied, result); } [Test] @@ -300,12 +311,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var entityService = entityServiceMock.Object; var contentServiceMock = new Mock(); var contentService = contentServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(-20, user, userService, contentService, entityService, out var foundContent, new[] { 'A' }); + var result = contentPermissions.CheckPermissions(-20, user, out IContent foundContent, new[] { 'A' }); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Granted, result); + Assert.AreEqual(ContentPermissions.ContentAccess.Granted, result); } [Test] @@ -326,12 +338,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var entityService = entityServiceMock.Object; var contentServiceMock = new Mock(); var contentService = contentServiceMock.Object; + var contentPermissions = new ContentPermissions(userService, contentService, entityService); //act - var result = ContentPermissionsHelper.CheckPermissions(-20, user, userService, contentService, entityService, out var foundContent, new[] { 'B' }); + var result = contentPermissions.CheckPermissions(-20, user, out IContent foundContent, new[] { 'B' }); //assert - Assert.AreEqual(ContentPermissionsHelper.ContentAccess.Denied, result); + Assert.AreEqual(ContentPermissions.ContentAccess.Denied, result); } private IUser CreateUser(int id = 0, int? startContentId = null, bool withUserGroup = true) @@ -340,77 +353,15 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers .WithId(id) .WithStartContentIds(startContentId.HasValue ? new[] { startContentId.Value } : new int[0]); if (withUserGroup) - { builder = builder .AddUserGroup() .WithId(1) .WithName("admin") .WithAlias("admin") .Done(); - } return builder.Build(); } } - //NOTE: The below self hosted stuff does work so need to get some tests written. Some are not possible atm because - // of the legacy SQL calls like checking permissions. - - //[TestFixture] - //public class ContentControllerHostedTests : BaseRoutingTest - //{ - - // protected override DatabaseBehavior DatabaseTestBehavior - // { - // get { return DatabaseBehavior.NoDatabasePerFixture; } - // } - - // public override void TearDown() - // { - // base.TearDown(); - // UmbracoAuthorizeAttribute.Enable = true; - // UmbracoApplicationAuthorizeAttribute.Enable = true; - // } - - // /// - // /// Tests to ensure that the response filter works so that any items the user - // /// doesn't have access to are removed - // /// - // [Test] - // public async void Get_By_Ids_Response_Filtered() - // { - // UmbracoAuthorizeAttribute.Enable = false; - // UmbracoApplicationAuthorizeAttribute.Enable = false; - - // var baseUrl = string.Format("http://{0}:9876", Environment.MachineName); - // var url = baseUrl + "/api/Content/GetByIds?ids=1&ids=2"; - - // var routingCtx = GetRoutingContext(url, 1234, null, true); - - // var config = new HttpSelfHostConfiguration(baseUrl); - // using (var server = new HttpSelfHostServer(config)) - // { - // var route = config.Routes.MapHttpRoute("test", "api/Content/GetByIds", - // new - // { - // controller = "Content", - // action = "GetByIds", - // id = RouteParameter.Optional - // }); - // route.DataTokens["Namespaces"] = new string[] { "Umbraco.Web.Editors" }; - - // var client = new HttpClient(server); - - // var request = new HttpRequestMessage - // { - // RequestUri = new Uri(url), - // Method = HttpMethod.Get - // }; - - // var result = await client.SendAsync(request); - // } - - // } - - //} } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MediaControllerUnitTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/MediaPermissionsTests.cs similarity index 73% rename from src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MediaControllerUnitTests.cs rename to src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/MediaPermissionsTests.cs index 2f692ade85..c9b0324f06 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MediaControllerUnitTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Security/MediaPermissionsTests.cs @@ -1,19 +1,17 @@ -using System.Collections.Generic; -using Moq; +using Moq; 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.Tests.Common.Builders; using Umbraco.Tests.Common.Builders.Extensions; -using Umbraco.Web.BackOffice.Controllers; -using Umbraco.Web.Common.Exceptions; -namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers +namespace Umbraco.Tests.UnitTests.Umbraco.Core.Security { [TestFixture] - public class MediaControllerUnitTests + public class MediaPermissionsTests { [Test] public void Access_Allowed_By_Path() @@ -28,16 +26,17 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var mediaService = mediaServiceMock.Object; var entityServiceMock = new Mock(); var entityService = entityServiceMock.Object; + var mediaPermissions = new MediaPermissions(mediaService, entityService); //act - var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, 1234); + var result = mediaPermissions.CheckPermissions(user, 1234, out _); //assert - Assert.IsTrue(result); + Assert.AreEqual(MediaPermissions.MediaAccess.Granted, result); } [Test] - public void Throws_Exception_When_No_Media_Found() + public void Returns_Not_Found_When_No_Media_Found() { //arrange var user = CreateUser(id: 9); @@ -49,9 +48,11 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var mediaService = mediaServiceMock.Object; var entityServiceMock = new Mock(); var entityService = entityServiceMock.Object; + var mediaPermissions = new MediaPermissions(mediaService, entityService); //act/assert - Assert.Throws(() => MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, 1234)); + var result = mediaPermissions.CheckPermissions(user, 1234, out _); + Assert.AreEqual(MediaPermissions.MediaAccess.NotFound, result); } [Test] @@ -69,12 +70,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers entityServiceMock.Setup(x => x.GetAllPaths(It.IsAny(), It.IsAny())) .Returns(new[] { Mock.Of(entity => entity.Id == 9876 && entity.Path == "-1,9876") }); var entityService = entityServiceMock.Object; + var mediaPermissions = new MediaPermissions(mediaService, entityService); //act - var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, 1234); + var result = mediaPermissions.CheckPermissions(user, 1234, out _); //assert - Assert.IsFalse(result); + Assert.AreEqual(MediaPermissions.MediaAccess.Denied, result); } [Test] @@ -86,12 +88,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var mediaService = mediaServiceMock.Object; var entityServiceMock = new Mock(); var entityService = entityServiceMock.Object; + var mediaPermissions = new MediaPermissions(mediaService, entityService); //act - var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, -1); + var result = mediaPermissions.CheckPermissions(user, -1, out _); //assert - Assert.IsTrue(result); + Assert.AreEqual(MediaPermissions.MediaAccess.Granted, result); } [Test] @@ -105,12 +108,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers entityServiceMock.Setup(x => x.GetAllPaths(It.IsAny(), It.IsAny())) .Returns(new[] { Mock.Of(entity => entity.Id == 1234 && entity.Path == "-1,1234") }); var entityService = entityServiceMock.Object; + var mediaPermissions = new MediaPermissions(mediaService, entityService); //act - var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, -1); + var result = mediaPermissions.CheckPermissions(user, -1, out _); //assert - Assert.IsFalse(result); + Assert.AreEqual(MediaPermissions.MediaAccess.Denied, result); } [Test] @@ -122,12 +126,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers var mediaService = mediaServiceMock.Object; var entityServiceMock = new Mock(); var entityService = entityServiceMock.Object; + var mediaPermissions = new MediaPermissions(mediaService, entityService); //act - var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, -21); + var result = mediaPermissions.CheckPermissions(user, -21, out _); //assert - Assert.IsTrue(result); + Assert.AreEqual(MediaPermissions.MediaAccess.Granted, result); } [Test] @@ -141,12 +146,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers entityServiceMock.Setup(x => x.GetAllPaths(It.IsAny(), It.IsAny())) .Returns(new[] { Mock.Of(entity => entity.Id == 1234 && entity.Path == "-1,1234") }); var entityService = entityServiceMock.Object; + var mediaPermissions = new MediaPermissions(mediaService, entityService); //act - var result = MediaController.CheckPermissions(new Dictionary(), user, mediaService, entityService, -21); + var result = mediaPermissions.CheckPermissions(user, -21, out _); //assert - Assert.IsFalse(result); + Assert.AreEqual(MediaPermissions.MediaAccess.Denied, result); } private IUser CreateUser(int id = 0, int? startMediaId = null) diff --git a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionPublishBranchHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionPublishBranchHandler.cs new file mode 100644 index 0000000000..f172bfdea0 --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionPublishBranchHandler.cs @@ -0,0 +1,70 @@ +using Microsoft.AspNetCore.Authorization; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Umbraco.Core; +using Umbraco.Core.Models; +using Umbraco.Core.Models.Entities; +using Umbraco.Core.Security; +using Umbraco.Core.Services; + +namespace Umbraco.Web.BackOffice.Authorization +{ + /// + /// The user must have access to all descendant nodes of the content item in order to continue + /// + public class ContentPermissionPublishBranchHandler : AuthorizationHandler + { + private readonly IEntityService _entityService; + private readonly ContentPermissions _contentPermissions; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; + + public ContentPermissionPublishBranchHandler( + IEntityService entityService, + ContentPermissions contentPermissions, + IBackOfficeSecurityAccessor backOfficeSecurityAccessor) + { + _entityService = entityService; + _contentPermissions = contentPermissions; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; + } + + protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, ContentPermissionsPublishBranchRequirement requirement, IContent resource) + { + var denied = new List(); + var page = 0; + const int pageSize = 500; + var total = long.MaxValue; + while (page * pageSize < total) + { + var descendants = _entityService.GetPagedDescendants(resource.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 + ordering: Ordering.By("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},")) + || (_contentPermissions.CheckPermissions(c, + _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser, + requirement.Permission) == ContentPermissions.ContentAccess.Denied)) + { + denied.Add(c); + } + } + } + + if (denied.Count == 0) + { + context.Succeed(requirement); + } + else + { + context.Fail(); + } + + return Task.CompletedTask; + } + } +} diff --git a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionQueryStringHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionQueryStringHandler.cs index b6e99d9320..fa10d5477b 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionQueryStringHandler.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionQueryStringHandler.cs @@ -11,45 +11,6 @@ using Umbraco.Core.Services; namespace Umbraco.Web.BackOffice.Authorization { - /// - /// Used to authorize if the user has the correct permission access to the content for the specified - /// - public class ContentPermissionResourceHandler : AuthorizationHandler - { - private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; - private readonly IEntityService _entityService; - private readonly IUserService _userService; - - public ContentPermissionResourceHandler( - IBackOfficeSecurityAccessor backofficeSecurityAccessor, - IEntityService entityService, - IUserService userService) - { - _backofficeSecurityAccessor = backofficeSecurityAccessor; - _entityService = entityService; - _userService = userService; - } - - protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, ContentPermissionResourceRequirement requirement, IContent resource) - { - var permissionResult = ContentPermissionsHelper.CheckPermissions(resource, - _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, - _userService, - _entityService, - new[] { requirement.PermissionToCheck }); - - if (permissionResult == ContentPermissionsHelper.ContentAccess.Denied) - { - context.Fail(); - } - else - { - context.Succeed(requirement); - } - - return Task.CompletedTask; - } - } /// /// Used to authorize if the user has the correct permission access to the content for the content id specified in a query string @@ -59,21 +20,18 @@ namespace Umbraco.Web.BackOffice.Authorization private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; private readonly IHttpContextAccessor _httpContextAccessor; private readonly IEntityService _entityService; - private readonly IUserService _userService; - private readonly IContentService _contentService; + private readonly ContentPermissions _contentPermissions; public ContentPermissionQueryStringHandler( IBackOfficeSecurityAccessor backofficeSecurityAccessor, IHttpContextAccessor httpContextAccessor, IEntityService entityService, - IUserService userService, - IContentService contentService) + ContentPermissions contentPermissions) { _backofficeSecurityAccessor = backofficeSecurityAccessor; _httpContextAccessor = httpContextAccessor; _entityService = entityService; - _userService = userService; - _contentService = contentService; + _contentPermissions = contentPermissions; } protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, ContentPermissionsQueryStringRequirement requirement) @@ -118,20 +76,17 @@ namespace Umbraco.Web.BackOffice.Authorization nodeId = requirement.NodeId.Value; } - var permissionResult = ContentPermissionsHelper.CheckPermissions(nodeId, - _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, - _userService, - _contentService, - _entityService, - out var contentItem, + var permissionResult = _contentPermissions.CheckPermissions(nodeId, + _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, + out IContent contentItem, new[] { requirement.PermissionToCheck }); - if (permissionResult == ContentPermissionsHelper.ContentAccess.NotFound) + if (permissionResult == ContentPermissions.ContentAccess.NotFound) { return null; } - if (permissionResult == ContentPermissionsHelper.ContentAccess.Denied) + if (permissionResult == ContentPermissions.ContentAccess.Denied) { context.Fail(); } diff --git a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionResourceHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionResourceHandler.cs new file mode 100644 index 0000000000..8172eab2a7 --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionResourceHandler.cs @@ -0,0 +1,43 @@ +using Microsoft.AspNetCore.Authorization; +using System.Threading.Tasks; +using Umbraco.Core.Models; +using Umbraco.Core.Security; +using Umbraco.Core.Services; + +namespace Umbraco.Web.BackOffice.Authorization +{ + /// + /// Used to authorize if the user has the correct permission access to the content for the specified + /// + public class ContentPermissionResourceHandler : AuthorizationHandler + { + private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; + private readonly ContentPermissions _contentPermissions; + + public ContentPermissionResourceHandler( + IBackOfficeSecurityAccessor backofficeSecurityAccessor, + ContentPermissions contentPermissions) + { + _backofficeSecurityAccessor = backofficeSecurityAccessor; + _contentPermissions = contentPermissions; + } + + protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, ContentPermissionResourceRequirement requirement, IContent resource) + { + var permissionResult = _contentPermissions.CheckPermissions(resource, + _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, + requirement.PermissionsToCheck); + + if (permissionResult == ContentPermissions.ContentAccess.Denied) + { + context.Fail(); + } + else + { + context.Succeed(requirement); + } + + return Task.CompletedTask; + } + } +} diff --git a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionResourceRequirement.cs b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionResourceRequirement.cs index 6781f45ab5..f430ffcd60 100644 --- a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionResourceRequirement.cs +++ b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionResourceRequirement.cs @@ -1,7 +1,10 @@ using Microsoft.AspNetCore.Authorization; +using System.Collections.Generic; +using Umbraco.Web.Actions; namespace Umbraco.Web.BackOffice.Authorization { + /// /// An authorization requirement for /// @@ -13,9 +16,21 @@ namespace Umbraco.Web.BackOffice.Authorization /// public ContentPermissionResourceRequirement(char permissionToCheck) { - PermissionToCheck = permissionToCheck; + PermissionsToCheck = new List { permissionToCheck }; } - public char PermissionToCheck { get; } + public ContentPermissionResourceRequirement(IReadOnlyList permissionToCheck) + { + PermissionsToCheck = permissionToCheck; + } + + public ContentPermissionResourceRequirement(int nodeId, IReadOnlyList permissionToCheck) + { + NodeId = nodeId; + PermissionsToCheck = permissionToCheck; + } + + public int? NodeId { get; } + public IReadOnlyList PermissionsToCheck { get; } } } diff --git a/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsPublishBranchRequirement.cs b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsPublishBranchRequirement.cs new file mode 100644 index 0000000000..6c56b828bc --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Authorization/ContentPermissionsPublishBranchRequirement.cs @@ -0,0 +1,17 @@ +using Microsoft.AspNetCore.Authorization; + +namespace Umbraco.Web.BackOffice.Authorization +{ + /// + /// Authorization requirement for + /// + public class ContentPermissionsPublishBranchRequirement : IAuthorizationRequirement + { + public ContentPermissionsPublishBranchRequirement(char permission) + { + Permission = permission; + } + + public char Permission { get; } + } +} diff --git a/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionQueryStringHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionQueryStringHandler.cs new file mode 100644 index 0000000000..dc3cdf3090 --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionQueryStringHandler.cs @@ -0,0 +1,94 @@ +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Primitives; +using System; +using System.Threading.Tasks; +using Umbraco.Core; +using Umbraco.Core.Models; +using Umbraco.Core.Security; +using Umbraco.Core.Services; + +namespace Umbraco.Web.BackOffice.Authorization +{ + public class MediaPermissionQueryStringHandler : AuthorizationHandler + { + private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; + private readonly IHttpContextAccessor _httpContextAccessor; + private readonly MediaPermissions _mediaPermissions; + private readonly IEntityService _entityService; + + public MediaPermissionQueryStringHandler( + IBackOfficeSecurityAccessor backofficeSecurityAccessor, + IHttpContextAccessor httpContextAccessor, + MediaPermissions mediaPermissions) + { + _backofficeSecurityAccessor = backofficeSecurityAccessor; + _httpContextAccessor = httpContextAccessor; + _mediaPermissions = mediaPermissions; + } + + protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, MediaPermissionsQueryStringRequirement requirement) + { + StringValues routeVal; + foreach (var qs in requirement.QueryStringNames) + { + if (_httpContextAccessor.HttpContext.Request.Query.TryGetValue(qs, out routeVal)) + { + break; + } + } + + if (routeVal.Count == 0) + { + throw new InvalidOperationException("No argument found for the current action with by names " + string.Join(", ", requirement.QueryStringNames)); + } + + int nodeId; + + var argument = routeVal.ToString(); + // if the argument is an int, it will parse and can be assigned to nodeId + // if might be a udi, so check that next + // otherwise treat it as a guid - unlikely we ever get here + if (int.TryParse(argument, out int parsedId)) + { + nodeId = parsedId; + } + else if (UdiParser.TryParse(argument, true, out var udi)) + { + nodeId = _entityService.GetId(udi).Result; + } + else + { + Guid.TryParse(argument, out Guid key); + nodeId = _entityService.GetId(key, UmbracoObjectTypes.Document).Result; + } + + var permissionResult = _mediaPermissions.CheckPermissions( + _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, + nodeId, + out var mediaItem); + + if (permissionResult == MediaPermissions.MediaAccess.NotFound) + { + return null; + } + + if (permissionResult == MediaPermissions.MediaAccess.Denied) + { + context.Fail(); + } + else + { + context.Succeed(requirement); + } + + if (mediaItem != null) + { + //store the content item in request cache so it can be resolved in the controller without re-looking it up + _httpContextAccessor.HttpContext.Items[typeof(IMedia).ToString()] = mediaItem; + } + + return Task.CompletedTask; + } + } +} diff --git a/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionResourceHandler.cs b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionResourceHandler.cs new file mode 100644 index 0000000000..074cb4e51b --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionResourceHandler.cs @@ -0,0 +1,54 @@ +using Microsoft.AspNetCore.Authorization; +using System.Threading.Tasks; +using Umbraco.Core.Models; +using Umbraco.Core.Security; + +namespace Umbraco.Web.BackOffice.Authorization +{ + /// + /// Used to authorize if the user has the correct permission access to the content for the specified + /// + public class MediaPermissionResourceHandler : AuthorizationHandler + { + private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; + private readonly MediaPermissions _mediaPermissions; + + public MediaPermissionResourceHandler( + IBackOfficeSecurityAccessor backofficeSecurityAccessor, + MediaPermissions mediaPermissions) + { + _backofficeSecurityAccessor = backofficeSecurityAccessor; + _mediaPermissions = mediaPermissions; + } + + protected override Task HandleRequirementAsync(AuthorizationHandlerContext context, MediaPermissionResourceRequirement requirement, IMedia resource) + { + var permissionResult = MediaPermissions.MediaAccess.NotFound; + + if (resource != null) + { + permissionResult = _mediaPermissions.CheckPermissions( + resource, + _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser); + } + else if (requirement.NodeId.HasValue) + { + permissionResult = _mediaPermissions.CheckPermissions( + _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, + requirement.NodeId.Value, + out _); + } + + if (permissionResult == MediaPermissions.MediaAccess.Denied) + { + context.Fail(); + } + else + { + context.Succeed(requirement); + } + + return Task.CompletedTask; + } + } +} diff --git a/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionResourceRequirement.cs b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionResourceRequirement.cs new file mode 100644 index 0000000000..5721a57d62 --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionResourceRequirement.cs @@ -0,0 +1,21 @@ +using Microsoft.AspNetCore.Authorization; + +namespace Umbraco.Web.BackOffice.Authorization +{ + /// + /// An authorization requirement for + /// + public class MediaPermissionResourceRequirement : IAuthorizationRequirement + { + public MediaPermissionResourceRequirement() + { + } + + public MediaPermissionResourceRequirement(int nodeId) + { + NodeId = nodeId; + } + + public int? NodeId { get; } + } +} diff --git a/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsQueryStringRequirement.cs b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsQueryStringRequirement.cs new file mode 100644 index 0000000000..75bb78cea1 --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Authorization/MediaPermissionsQueryStringRequirement.cs @@ -0,0 +1,14 @@ +using Microsoft.AspNetCore.Authorization; + +namespace Umbraco.Web.BackOffice.Authorization +{ + public class MediaPermissionsQueryStringRequirement : IAuthorizationRequirement + { + public MediaPermissionsQueryStringRequirement(string[] paramNames) + { + QueryStringNames = paramNames; + } + + public string[] QueryStringNames { get; } + } +} diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs index 06f6d3b052..ba5016251d 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ContentController.cs @@ -638,9 +638,9 @@ namespace Umbraco.Web.BackOffice.Controllers /// [FileUploadCleanupFilter] [ContentSaveValidation] - public ContentItemDisplay PostSaveBlueprint([ModelBinder(typeof(BlueprintItemBinder))] ContentItemSave contentItem) + public async Task PostSaveBlueprint([ModelBinder(typeof(BlueprintItemBinder))] ContentItemSave contentItem) { - var contentItemDisplay = PostSaveInternal(contentItem, + var contentItemDisplay = await PostSaveInternal(contentItem, content => { EnsureUniqueName(content.Name, content, "Name"); @@ -666,9 +666,9 @@ namespace Umbraco.Web.BackOffice.Controllers [FileUploadCleanupFilter] [ContentSaveValidation] [TypeFilter(typeof(OutgoingEditorModelEventAttribute))] - public ContentItemDisplay PostSave([ModelBinder(typeof(ContentItemBinder))] ContentItemSave contentItem) + public async Task PostSave([ModelBinder(typeof(ContentItemBinder))] ContentItemSave contentItem) { - var contentItemDisplay = PostSaveInternal( + var contentItemDisplay = await PostSaveInternal( contentItem, content => _contentService.Save(contentItem.PersistedContent, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id), MapToDisplay); @@ -676,7 +676,7 @@ namespace Umbraco.Web.BackOffice.Controllers return contentItemDisplay; } - private ContentItemDisplay PostSaveInternal(ContentItemSave contentItem, Func saveMethod, Func mapToDisplay) + private async Task PostSaveInternal(ContentItemSave contentItem, Func saveMethod, Func mapToDisplay) { //Recent versions of IE/Edge may send in the full client side file path instead of just the file name. //To ensure similar behavior across all browsers no matter what they do - we strip the FileName property of all @@ -811,7 +811,7 @@ namespace Umbraco.Web.BackOffice.Controllers case ContentSaveAction.PublishWithDescendants: case ContentSaveAction.PublishWithDescendantsNew: { - if (!ValidatePublishBranchPermissions(contentItem, out var noAccess)) + if (!await ValidatePublishBranchPermissionsAsync(contentItem)) { globalNotifications.AddErrorNotification( _localizedTextService.Localize("publish"), @@ -827,7 +827,7 @@ namespace Umbraco.Web.BackOffice.Controllers case ContentSaveAction.PublishWithDescendantsForce: case ContentSaveAction.PublishWithDescendantsForceNew: { - if (!ValidatePublishBranchPermissions(contentItem, out var noAccess)) + if (!await ValidatePublishBranchPermissionsAsync(contentItem)) { globalNotifications.AddErrorNotification( _localizedTextService.Localize("publish"), @@ -1198,33 +1198,12 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// /// - private bool ValidatePublishBranchPermissions(ContentItemSave contentItem, out IReadOnlyList noAccess) + private async Task ValidatePublishBranchPermissionsAsync(ContentItemSave contentItem) { - var denied = new List(); - var page = 0; - const int pageSize = 500; - var total = long.MaxValue; - while (page * pageSize < total) - { - var descendants = _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 - ordering: Ordering.By("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, - _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, _userService, _entityService, - ActionPublish.ActionLetter) == ContentPermissionsHelper.ContentAccess.Denied)) - { - denied.Add(c); - } - } - } - noAccess = denied; - return denied.Count == 0; + // Authorize... + var requirement = new ContentPermissionsPublishBranchRequirement(ActionPublish.ActionLetter); + var authorizationResult = await _authorizationService.AuthorizeAsync(User, contentItem.PersistedContent, requirement); + return authorizationResult.Succeeded; } private IEnumerable PublishBranchInternal(ContentItemSave contentItem, bool force, string cultureForInvariantErrors, diff --git a/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs b/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs index 4c68b7dfa6..f98d09bd84 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MediaController.cs @@ -42,6 +42,7 @@ using Umbraco.Web.Models.ContentEditing; using Constants = Umbraco.Core.Constants; using Microsoft.AspNetCore.Authorization; using Umbraco.Web.Common.Authorization; +using Umbraco.Web.BackOffice.Authorization; namespace Umbraco.Web.BackOffice.Controllers { @@ -67,6 +68,7 @@ namespace Umbraco.Web.BackOffice.Controllers private readonly IRelationService _relationService; private readonly IImageUrlGenerator _imageUrlGenerator; private readonly IJsonSerializer _serializer; + private readonly IAuthorizationService _authorizationService; private readonly ILogger _logger; public MediaController( @@ -89,7 +91,8 @@ namespace Umbraco.Web.BackOffice.Controllers IMediaFileSystem mediaFileSystem, IHostingEnvironment hostingEnvironment, IImageUrlGenerator imageUrlGenerator, - IJsonSerializer serializer) + IJsonSerializer serializer, + IAuthorizationService authorizationService) : base(cultureDictionary, loggerFactory, shortStringHelper, eventMessages, localizedTextService, serializer) { _shortStringHelper = shortStringHelper; @@ -110,6 +113,7 @@ namespace Umbraco.Web.BackOffice.Controllers _logger = loggerFactory.CreateLogger(); _imageUrlGenerator = imageUrlGenerator; _serializer = serializer; + _authorizationService = authorizationService; } /// @@ -167,7 +171,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// [TypeFilter(typeof(OutgoingEditorModelEventAttribute))] - [EnsureUserPermissionForMedia("id")] + [Authorize(Policy = AuthorizationPolicies.MediaPermissionPathById)] [DetermineAmbiguousActionByPassingParameters] public MediaItemDisplay GetById(int id) { @@ -188,7 +192,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// [TypeFilter(typeof(OutgoingEditorModelEventAttribute))] - [EnsureUserPermissionForMedia("id")] + [Authorize(Policy = AuthorizationPolicies.MediaPermissionPathById)] [DetermineAmbiguousActionByPassingParameters] public MediaItemDisplay GetById(Guid id) { @@ -209,7 +213,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// [TypeFilter(typeof(OutgoingEditorModelEventAttribute))] - [EnsureUserPermissionForMedia("id")] + [Authorize(Policy = AuthorizationPolicies.MediaPermissionPathById)] [DetermineAmbiguousActionByPassingParameters] public MediaItemDisplay GetById(Udi id) { @@ -432,7 +436,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// /// - [EnsureUserPermissionForMedia("id")] + [Authorize(Policy = AuthorizationPolicies.MediaPermissionPathById)] [HttpPost] public IActionResult DeleteById(int id) { @@ -473,9 +477,16 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// /// - [EnsureUserPermissionForMedia("move.Id")] - public IActionResult PostMove(MoveOrCopy move) + public async Task PostMove(MoveOrCopy move) { + // Authorize... + var requirement = new MediaPermissionResourceRequirement(); + var authorizationResult = await _authorizationService.AuthorizeAsync(User, _mediaService.GetById(move.Id), requirement); + if (!authorizationResult.Succeeded) + { + return Forbid(); + } + var toMove = ValidateMoveOrCopy(move); var destinationParentID = move.ParentId; var sourceParentID = toMove.ParentId; @@ -610,8 +621,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// /// - [EnsureUserPermissionForMedia("sorted.ParentId")] - public IActionResult PostSort(ContentSortOrder sorted) + public async Task PostSort(ContentSortOrder sorted) { if (sorted == null) { @@ -624,14 +634,21 @@ namespace Umbraco.Web.BackOffice.Controllers return Ok(); } - var mediaService = _mediaService; + // Authorize... + var requirement = new MediaPermissionResourceRequirement(); + var authorizationResult = await _authorizationService.AuthorizeAsync(User, _mediaService.GetById(sorted.ParentId), requirement); + if (!authorizationResult.Succeeded) + { + return Forbid(); + } + var sortedMedia = new List(); try { - sortedMedia.AddRange(sorted.IdSortOrder.Select(mediaService.GetById)); + sortedMedia.AddRange(sorted.IdSortOrder.Select(_mediaService.GetById)); // Save Media with new sort order and update content xml in db accordingly - if (mediaService.Sort(sortedMedia) == false) + if (_mediaService.Sort(sortedMedia) == false) { _logger.LogWarning("Media sorting failed, this was probably caused by an event being cancelled"); throw HttpResponseException.CreateValidationErrorResponse("Media sorting failed, this was probably caused by an event being cancelled"); @@ -645,19 +662,16 @@ namespace Umbraco.Web.BackOffice.Controllers } } - public ActionResult PostAddFolder(PostedFolder folder) + public async Task> PostAddFolder(PostedFolder folder) { - var parentId = GetParentIdAsInt(folder.ParentId, validatePermissions:true); + var parentId = await GetParentIdAsIntAsync(folder.ParentId, validatePermissions:true); if (!parentId.HasValue) { return NotFound("The passed id doesn't exist"); } - - var mediaService = _mediaService; - - var f = mediaService.CreateMedia(folder.Name, parentId.Value, Constants.Conventions.MediaTypes.Folder); - mediaService.Save(f, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id); + var f = _mediaService.CreateMedia(folder.Name, parentId.Value, Constants.Conventions.MediaTypes.Folder); + _mediaService.Save(f, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id); return _umbracoMapper.Map(f); } @@ -682,13 +696,13 @@ namespace Umbraco.Web.BackOffice.Controllers } //get the string json from the request - var parentId = GetParentIdAsInt(currentFolder, validatePermissions: true); + var parentId = await GetParentIdAsIntAsync(currentFolder, validatePermissions: true); if (!parentId.HasValue) { return NotFound("The passed id doesn't exist"); } var tempFiles = new PostedFiles(); - var mediaService = _mediaService; + //in case we pass a path with a folder in it, we will create it and upload media to it. if (!string.IsNullOrEmpty(path)) @@ -706,18 +720,18 @@ namespace Umbraco.Web.BackOffice.Controllers { //look for matching folder folderMediaItem = - mediaService.GetRootMedia().FirstOrDefault(x => x.Name == folderName && x.ContentType.Alias == Constants.Conventions.MediaTypes.Folder); + _mediaService.GetRootMedia().FirstOrDefault(x => x.Name == folderName && x.ContentType.Alias == Constants.Conventions.MediaTypes.Folder); if (folderMediaItem == null) { //if null, create a folder - folderMediaItem = mediaService.CreateMedia(folderName, -1, Constants.Conventions.MediaTypes.Folder); - mediaService.Save(folderMediaItem); + folderMediaItem = _mediaService.CreateMedia(folderName, -1, Constants.Conventions.MediaTypes.Folder); + _mediaService.Save(folderMediaItem); } } else { //get current parent - var mediaRoot = mediaService.GetById(parentId.Value); + var mediaRoot = _mediaService.GetById(parentId.Value); //if the media root is null, something went wrong, we'll abort if (mediaRoot == null) @@ -731,8 +745,8 @@ namespace Umbraco.Web.BackOffice.Controllers if (folderMediaItem == null) { //if null, create a folder - folderMediaItem = mediaService.CreateMedia(folderName, mediaRoot, Constants.Conventions.MediaTypes.Folder); - mediaService.Save(folderMediaItem); + folderMediaItem = _mediaService.CreateMedia(folderName, mediaRoot, Constants.Conventions.MediaTypes.Folder); + _mediaService.Save(folderMediaItem); } } //set the media root to the folder id so uploaded files will end there. @@ -765,7 +779,7 @@ namespace Umbraco.Web.BackOffice.Controllers var mediaItemName = fileName.ToFriendlyName(); - var f = mediaService.CreateMedia(mediaItemName, parentId.Value, mediaType, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id); + var f = _mediaService.CreateMedia(mediaItemName, parentId.Value, mediaType, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id); await using (var stream = formFile.OpenReadStream()) @@ -774,7 +788,7 @@ namespace Umbraco.Web.BackOffice.Controllers } - var saveResult = mediaService.Save(f, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id); + var saveResult = _mediaService.Save(f, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id); if (saveResult == false) { AddCancelMessage(tempFiles, @@ -830,7 +844,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// and if that check fails an unauthorized exception will occur /// /// - private int? GetParentIdAsInt(string parentId, bool validatePermissions) + private async Task GetParentIdAsIntAsync(string parentId, bool validatePermissions) { int intParentId; @@ -863,22 +877,23 @@ namespace Umbraco.Web.BackOffice.Controllers } } + // Authorize... //ensure the user has access to this folder by parent id! - if (validatePermissions && CheckPermissions( - new Dictionary(), - _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, - _mediaService, - _entityService, - intParentId) == false) + if (validatePermissions) { - throw new HttpResponseException( + var requirement = new MediaPermissionResourceRequirement(); + var authorizationResult = await _authorizationService.AuthorizeAsync(User, _mediaService.GetById(intParentId), requirement); + if (!authorizationResult.Succeeded) + { + throw new HttpResponseException( HttpStatusCode.Forbidden, new SimpleNotificationModel(new BackOfficeNotification( _localizedTextService.Localize("speechBubbles/operationFailedHeader"), _localizedTextService.Localize("speechBubbles/invalidUserPermissionsText"), NotificationStyle.Warning))); + } } - + return intParentId; } @@ -894,8 +909,8 @@ namespace Umbraco.Web.BackOffice.Controllers throw new HttpResponseException(HttpStatusCode.NotFound); } - var mediaService = _mediaService; - var toMove = mediaService.GetById(model.Id); + + var toMove = _mediaService.GetById(model.Id); if (toMove == null) { throw new HttpResponseException(HttpStatusCode.NotFound); @@ -914,7 +929,7 @@ namespace Umbraco.Web.BackOffice.Controllers } else { - var parent = mediaService.GetById(model.ParentId); + var parent = _mediaService.GetById(model.ParentId); if (parent == null) { throw new HttpResponseException(HttpStatusCode.NotFound); @@ -942,45 +957,7 @@ namespace Umbraco.Web.BackOffice.Controllers return toMove; } - /// - /// 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, setting this ignores the nodeId - /// - internal static bool CheckPermissions(IDictionary storage, IUser user, IMediaService mediaService, IEntityService entityService, int nodeId, IMedia media = null) - { - if (storage == null) throw new ArgumentNullException("storage"); - if (user == null) throw new ArgumentNullException("user"); - if (mediaService == null) throw new ArgumentNullException("mediaService"); - if (entityService == null) throw new ArgumentNullException("entityService"); - - if (media == null && nodeId != Constants.System.Root && nodeId != Constants.System.RecycleBinMedia) - { - media = mediaService.GetById(nodeId); - //put the content item into storage so it can be retrieved - // in the controller (saves a lookup) - storage[typeof(IMedia).ToString()] = media; - } - - if (media == null && nodeId != Constants.System.Root && nodeId != Constants.System.RecycleBinMedia) - { - throw new HttpResponseException(HttpStatusCode.NotFound); - } - - var hasPathAccess = (nodeId == Constants.System.Root) - ? user.HasMediaRootAccess(entityService) - : (nodeId == Constants.System.RecycleBinMedia) - ? user.HasMediaBinAccess(entityService) - : user.HasPathAccess(media, entityService); - - return hasPathAccess; - } + public PagedResult GetPagedReferences(int id, string entityType, int pageNumber = 1, int pageSize = 100) { diff --git a/src/Umbraco.Web.BackOffice/Extensions/BackOfficeServiceCollectionExtensions.cs b/src/Umbraco.Web.BackOffice/Extensions/BackOfficeServiceCollectionExtensions.cs index 8e3eb96660..2aadaa3fd6 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/BackOfficeServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/BackOfficeServiceCollectionExtensions.cs @@ -128,10 +128,22 @@ namespace Umbraco.Extensions services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); services.AddAuthorization(options => { + // these are the query strings we will check for media ids when permission checking + var mediaPermissionQueryStrings = new[] { "id" }; + + options.AddPolicy(AuthorizationPolicies.MediaPermissionPathById, policy => + { + policy.AuthenticationSchemes.Add(Constants.Security.BackOfficeAuthenticationType); + policy.Requirements.Add(new MediaPermissionsQueryStringRequirement(mediaPermissionQueryStrings)); + }); + // these are the query strings we will check for content ids when permission checking var contentPermissionQueryStrings = new[] { "id", "contentId" }; diff --git a/src/Umbraco.Web.BackOffice/Filters/ContentSaveValidationAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/ContentSaveValidationAttribute.cs index 2553232185..fba9fa9b35 100644 --- a/src/Umbraco.Web.BackOffice/Filters/ContentSaveValidationAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/ContentSaveValidationAttribute.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Infrastructure; @@ -10,6 +12,7 @@ using Umbraco.Core.Models; using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Web.Actions; +using Umbraco.Web.BackOffice.Authorization; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Security; @@ -27,11 +30,13 @@ namespace Umbraco.Web.BackOffice.Filters } - private sealed class ContentSaveValidationFilter : IActionFilter + private sealed class ContentSaveValidationFilter : IAsyncActionFilter { private readonly IContentService _contentService; private readonly IEntityService _entityService; private readonly IPropertyValidationService _propertyValidationService; + private readonly ContentPermissions _contentPermissions; + private readonly IAuthorizationService _authorizationService; private readonly ILoggerFactory _loggerFactory; private readonly ILocalizedTextService _textService; private readonly IUserService _userService; @@ -45,7 +50,9 @@ namespace Umbraco.Web.BackOffice.Filters IContentService contentService, IUserService userService, IEntityService entityService, - IPropertyValidationService propertyValidationService) + IPropertyValidationService propertyValidationService, + ContentPermissions contentPermissions, + IAuthorizationService authorizationService) { _loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory)); _backofficeSecurityAccessor = backofficeSecurityAccessor ?? throw new ArgumentNullException(nameof(backofficeSecurityAccessor)); @@ -54,16 +61,28 @@ namespace Umbraco.Web.BackOffice.Filters _userService = userService ?? throw new ArgumentNullException(nameof(userService)); _entityService = entityService ?? throw new ArgumentNullException(nameof(entityService)); _propertyValidationService = propertyValidationService ?? throw new ArgumentNullException(nameof(propertyValidationService)); + _contentPermissions = contentPermissions; + _authorizationService = authorizationService; } - public void OnActionExecuting(ActionExecutingContext context) + public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) + { + // on executing... + await OnActionExecutingAsync(context); + + await next(); //need to pass the execution to next + + // on executed... + } + + private async Task OnActionExecutingAsync(ActionExecutingContext context) { var model = (ContentItemSave) context.ActionArguments["contentItem"]; var contentItemValidator = new ContentSaveModelValidator(_loggerFactory.CreateLogger(), _backofficeSecurityAccessor.BackOfficeSecurity, _textService, _propertyValidationService); if (!ValidateAtLeastOneVariantIsBeingSaved(model, context)) return; if (!contentItemValidator.ValidateExistingContent(model, context)) return; - if (!ValidateUserAccess(model, context, _backofficeSecurityAccessor.BackOfficeSecurity)) return; + if (!await ValidateUserAccessAsync(model, context, _backofficeSecurityAccessor.BackOfficeSecurity)) return; //validate for each variant that is being updated foreach (var variant in model.Variants.Where(x => x.Save)) @@ -74,9 +93,6 @@ namespace Umbraco.Web.BackOffice.Filters } } - public void OnActionExecuted(ActionExecutedContext context) - { - } /// /// If there are no variants tagged for Saving, then this is an invalid request @@ -84,7 +100,8 @@ namespace Umbraco.Web.BackOffice.Filters /// /// /// - private bool ValidateAtLeastOneVariantIsBeingSaved(ContentItemSave contentItem, + private bool ValidateAtLeastOneVariantIsBeingSaved( + ContentItemSave contentItem, ActionExecutingContext actionContext) { if (!contentItem.Variants.Any(x => x.Save)) @@ -102,7 +119,9 @@ namespace Umbraco.Web.BackOffice.Filters /// /// /// - private bool ValidateUserAccess(ContentItemSave contentItem, ActionExecutingContext actionContext, + private async Task ValidateUserAccessAsync( + ContentItemSave contentItem, + ActionExecutingContext actionContext, IBackOfficeSecurity backofficeSecurity) { // We now need to validate that the user is allowed to be doing what they are doing. @@ -210,42 +229,22 @@ namespace Umbraco.Web.BackOffice.Filters throw new ArgumentOutOfRangeException(); } - ContentPermissionsHelper.ContentAccess accessResult; - if (contentToCheck != null) - { - //store the content item in request cache so it can be resolved in the controller without re-looking it up - actionContext.HttpContext.Items[typeof(IContent).ToString()] = contentItem; - accessResult = ContentPermissionsHelper.CheckPermissions( - contentToCheck, backofficeSecurity.CurrentUser, - _userService, _entityService, permissionToCheck.ToArray()); - } - else - { - accessResult = ContentPermissionsHelper.CheckPermissions( - contentIdToCheck, backofficeSecurity.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.HttpContext.Items[typeof(IContent).ToString()] = contentToCheck; - } - } + var requirement = contentToCheck == null + ? new ContentPermissionResourceRequirement(contentIdToCheck, permissionToCheck) + : new ContentPermissionResourceRequirement(permissionToCheck); - if (accessResult == ContentPermissionsHelper.ContentAccess.NotFound) - { - actionContext.Result = new NotFoundResult(); - } - - if (accessResult != ContentPermissionsHelper.ContentAccess.Granted) + var authorizationResult = await _authorizationService.AuthorizeAsync(actionContext.HttpContext.User, contentToCheck, requirement); + if (!authorizationResult.Succeeded) { actionContext.Result = new ForbidResult(); + return false; } - return accessResult == ContentPermissionsHelper.ContentAccess.Granted; + return true; } + + } } } diff --git a/src/Umbraco.Web.BackOffice/Filters/EnsureUserPermissionForMediaAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/EnsureUserPermissionForMediaAttribute.cs index cabc865d40..607d77cd21 100644 --- a/src/Umbraco.Web.BackOffice/Filters/EnsureUserPermissionForMediaAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/EnsureUserPermissionForMediaAttribute.cs @@ -1,182 +1,184 @@ -using System; -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.Filters; -using Umbraco.Core; -using Umbraco.Core.Models; -using Umbraco.Core.Security; -using Umbraco.Core.Services; -using Umbraco.Web.BackOffice.Controllers; -using Umbraco.Web.Common.Exceptions; -using Umbraco.Web.Editors; -using Umbraco.Web.Security; +//using System; +//using Microsoft.AspNetCore.Mvc; +//using Microsoft.AspNetCore.Mvc.Filters; +//using Umbraco.Core; +//using Umbraco.Core.Models; +//using Umbraco.Core.Security; +//using Umbraco.Core.Services; +//using Umbraco.Web.BackOffice.Controllers; +//using Umbraco.Web.Common.Exceptions; +//using Umbraco.Web.Editors; +//using Umbraco.Web.Security; -namespace Umbraco.Web.BackOffice.Filters -{ - /// - /// Auth filter to check if the current user has access to the content item - /// - /// - /// Since media doesn't have permissions, this simply checks start node access - /// - internal sealed class EnsureUserPermissionForMediaAttribute : TypeFilterAttribute - { - public EnsureUserPermissionForMediaAttribute(int nodeId) - : base(typeof(EnsureUserPermissionForMediaFilter)) - { - Arguments = new object[] - { - nodeId - }; - } +//namespace Umbraco.Web.BackOffice.Filters +//{ +// /// +// /// Auth filter to check if the current user has access to the content item +// /// +// /// +// /// Since media doesn't have permissions, this simply checks start node access +// /// +// internal sealed class EnsureUserPermissionForMediaAttribute : TypeFilterAttribute +// { +// // TODO: This needs to be an authorization policy - public EnsureUserPermissionForMediaAttribute(string paramName) - : base(typeof(EnsureUserPermissionForMediaFilter)) - { - Arguments = new object[] - { - paramName - }; - } - private sealed class EnsureUserPermissionForMediaFilter : IActionFilter - { - private readonly int? _nodeId; - private readonly string _paramName; - private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; - private readonly IEntityService _entityService; - private readonly IMediaService _mediaService; +// public EnsureUserPermissionForMediaAttribute(int nodeId) +// : base(typeof(EnsureUserPermissionForMediaFilter)) +// { +// Arguments = new object[] +// { +// nodeId +// }; +// } - /// - /// This constructor will only be able to test the start node access - /// - public EnsureUserPermissionForMediaFilter( - IBackOfficeSecurityAccessor backofficeSecurityAccessor, - IEntityService entityService, - IMediaService mediaService, - int nodeId) - :this(backofficeSecurityAccessor, entityService, mediaService, nodeId, null) - { - _nodeId = nodeId; - } +// public EnsureUserPermissionForMediaAttribute(string paramName) +// : base(typeof(EnsureUserPermissionForMediaFilter)) +// { +// Arguments = new object[] +// { +// paramName +// }; +// } +// private sealed class EnsureUserPermissionForMediaFilter : IActionFilter +// { +// private readonly int? _nodeId; +// private readonly string _paramName; +// private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; +// private readonly IEntityService _entityService; +// private readonly IMediaService _mediaService; - public EnsureUserPermissionForMediaFilter( - IBackOfficeSecurityAccessor backofficeSecurityAccessor, - IEntityService entityService, - IMediaService mediaService, - string paramName) - :this(backofficeSecurityAccessor, entityService, mediaService,null, paramName) - { - if (paramName == null) throw new ArgumentNullException(nameof(paramName)); - if (string.IsNullOrEmpty(paramName)) - throw new ArgumentException("Value can't be empty.", nameof(paramName)); - } +// /// +// /// This constructor will only be able to test the start node access +// /// +// public EnsureUserPermissionForMediaFilter( +// IBackOfficeSecurityAccessor backofficeSecurityAccessor, +// IEntityService entityService, +// IMediaService mediaService, +// int nodeId) +// :this(backofficeSecurityAccessor, entityService, mediaService, nodeId, null) +// { +// _nodeId = nodeId; +// } - private EnsureUserPermissionForMediaFilter( - IBackOfficeSecurityAccessor backofficeSecurityAccessor, - IEntityService entityService, - IMediaService mediaService, - int? nodeId, string paramName) - { - _backofficeSecurityAccessor = backofficeSecurityAccessor ?? throw new ArgumentNullException(nameof(backofficeSecurityAccessor)); - _entityService = entityService ?? throw new ArgumentNullException(nameof(entityService)); - _mediaService = mediaService ?? throw new ArgumentNullException(nameof(mediaService)); +// public EnsureUserPermissionForMediaFilter( +// IBackOfficeSecurityAccessor backofficeSecurityAccessor, +// IEntityService entityService, +// IMediaService mediaService, +// string paramName) +// :this(backofficeSecurityAccessor, entityService, mediaService,null, paramName) +// { +// if (paramName == null) throw new ArgumentNullException(nameof(paramName)); +// if (string.IsNullOrEmpty(paramName)) +// throw new ArgumentException("Value can't be empty.", nameof(paramName)); +// } - _paramName = paramName; +// private EnsureUserPermissionForMediaFilter( +// IBackOfficeSecurityAccessor backofficeSecurityAccessor, +// IEntityService entityService, +// IMediaService mediaService, +// int? nodeId, string paramName) +// { +// _backofficeSecurityAccessor = backofficeSecurityAccessor ?? throw new ArgumentNullException(nameof(backofficeSecurityAccessor)); +// _entityService = entityService ?? throw new ArgumentNullException(nameof(entityService)); +// _mediaService = mediaService ?? throw new ArgumentNullException(nameof(mediaService)); - if (nodeId.HasValue) - { - _nodeId = nodeId.Value; - } - } +// _paramName = paramName; - private int GetNodeIdFromParameter(object parameterValue) - { - if (parameterValue is int) - { - return (int) parameterValue; - } +// if (nodeId.HasValue) +// { +// _nodeId = nodeId.Value; +// } +// } - var guidId = Guid.Empty; - if (parameterValue is Guid) - { - guidId = (Guid) parameterValue; - } - else if (parameterValue is GuidUdi) - { - guidId = ((GuidUdi) parameterValue).Guid; - } +// private int GetNodeIdFromParameter(object parameterValue) +// { +// if (parameterValue is int) +// { +// return (int) parameterValue; +// } - if (guidId != Guid.Empty) - { - var found = _entityService.GetId(guidId, UmbracoObjectTypes.Media); - if (found) - return found.Result; - } +// var guidId = Guid.Empty; +// if (parameterValue is Guid) +// { +// guidId = (Guid) parameterValue; +// } +// else if (parameterValue is GuidUdi) +// { +// guidId = ((GuidUdi) parameterValue).Guid; +// } - throw new InvalidOperationException("The id type: " + parameterValue.GetType() + - " is not a supported id"); - } +// if (guidId != Guid.Empty) +// { +// var found = _entityService.GetId(guidId, UmbracoObjectTypes.Media); +// if (found) +// return found.Result; +// } - public void OnActionExecuting(ActionExecutingContext context) - { - if (_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser == null) - { - throw new HttpResponseException(System.Net.HttpStatusCode.Unauthorized); - } +// throw new InvalidOperationException("The id type: " + parameterValue.GetType() + +// " is not a supported id"); +// } - int nodeId; - if (_nodeId.HasValue == false) - { - var parts = _paramName.Split(new[] { '.' }, StringSplitOptions.RemoveEmptyEntries); +// public void OnActionExecuting(ActionExecutingContext context) +// { +// if (_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser == null) +// { +// throw new HttpResponseException(System.Net.HttpStatusCode.Unauthorized); +// } - if (context.ActionArguments[parts[0]] == null) - { - throw new InvalidOperationException("No argument found for the current action with the name: " + - _paramName); - } +// int nodeId; +// if (_nodeId.HasValue == false) +// { +// var parts = _paramName.Split(new[] { '.' }, StringSplitOptions.RemoveEmptyEntries); - if (parts.Length == 1) - { - nodeId = GetNodeIdFromParameter(context.ActionArguments[parts[0]]); - } - else - { - //now we need to see if we can get the property of whatever object it is - var pType = context.ActionArguments[parts[0]].GetType(); - var prop = pType.GetProperty(parts[1]); - if (prop == null) - { - throw new InvalidOperationException( - "No argument found for the current action with the name: " + _paramName); - } +// if (context.ActionArguments[parts[0]] == null) +// { +// throw new InvalidOperationException("No argument found for the current action with the name: " + +// _paramName); +// } - nodeId = GetNodeIdFromParameter(prop.GetValue(context.ActionArguments[parts[0]])); - } - } - else - { - nodeId = _nodeId.Value; - } +// if (parts.Length == 1) +// { +// nodeId = GetNodeIdFromParameter(context.ActionArguments[parts[0]]); +// } +// else +// { +// //now we need to see if we can get the property of whatever object it is +// var pType = context.ActionArguments[parts[0]].GetType(); +// var prop = pType.GetProperty(parts[1]); +// if (prop == null) +// { +// throw new InvalidOperationException( +// "No argument found for the current action with the name: " + _paramName); +// } - if (MediaController.CheckPermissions( - context.HttpContext.Items, - _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, - _mediaService, - _entityService, - nodeId)) - { +// nodeId = GetNodeIdFromParameter(prop.GetValue(context.ActionArguments[parts[0]])); +// } +// } +// else +// { +// nodeId = _nodeId.Value; +// } - } - else - { - throw new HttpResponseException(System.Net.HttpStatusCode.Unauthorized); - } - } +// if (MediaController.CheckPermissions( +// context.HttpContext.Items, +// _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, +// _mediaService, +// _entityService, +// nodeId)) +// { - public void OnActionExecuted(ActionExecutedContext context) - { +// } +// else +// { +// throw new HttpResponseException(System.Net.HttpStatusCode.Unauthorized); +// } +// } - } +// public void OnActionExecuted(ActionExecutedContext context) +// { - } - } -} +// } + +// } +// } +//} diff --git a/src/Umbraco.Web.BackOffice/Filters/FilterAllowedOutgoingMediaAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/FilterAllowedOutgoingMediaAttribute.cs index 679fcdad6b..c7a7a56f83 100644 --- a/src/Umbraco.Web.BackOffice/Filters/FilterAllowedOutgoingMediaAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/FilterAllowedOutgoingMediaAttribute.cs @@ -87,7 +87,7 @@ namespace Umbraco.Web.BackOffice.Filters var toRemove = new List(); foreach (dynamic item in items) { - var hasPathAccess = (item != null && ContentPermissionsHelper.HasPathAccess(item.Path, GetUserStartNodes(user), RecycleBinId)); + var hasPathAccess = (item != null && ContentPermissions.HasPathAccess(item.Path, GetUserStartNodes(user), RecycleBinId)); if (hasPathAccess == false) { toRemove.Add(item); diff --git a/src/Umbraco.Web.BackOffice/Filters/MediaItemSaveValidationAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/MediaItemSaveValidationAttribute.cs index fa1ee568f0..afedf44211 100644 --- a/src/Umbraco.Web.BackOffice/Filters/MediaItemSaveValidationAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/MediaItemSaveValidationAttribute.cs @@ -1,4 +1,6 @@ using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Extensions.Logging; @@ -6,9 +8,8 @@ using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Core.Security; using Umbraco.Core.Services; -using Umbraco.Web.BackOffice.Controllers; +using Umbraco.Web.BackOffice.Authorization; using Umbraco.Web.Models.ContentEditing; -using Umbraco.Web.Security; namespace Umbraco.Web.BackOffice.Filters { @@ -21,12 +22,10 @@ namespace Umbraco.Web.BackOffice.Filters { } - private sealed class MediaItemSaveValidationFilter : IActionFilter + private sealed class MediaItemSaveValidationFilter : IAsyncActionFilter { - private readonly IEntityService _entityService; private readonly IPropertyValidationService _propertyValidationService; - - + private readonly IAuthorizationService _authorizationService; private readonly IMediaService _mediaService; private readonly ILocalizedTextService _textService; private readonly ILoggerFactory _loggerFactory; @@ -37,23 +36,33 @@ namespace Umbraco.Web.BackOffice.Filters IBackOfficeSecurityAccessor backofficeSecurityAccessor, ILocalizedTextService textService, IMediaService mediaService, - IEntityService entityService, - IPropertyValidationService propertyValidationService) + IPropertyValidationService propertyValidationService, + IAuthorizationService authorizationService) { _loggerFactory = loggerFactory ?? throw new ArgumentNullException(nameof(loggerFactory)); _backofficeSecurityAccessor = backofficeSecurityAccessor ?? throw new ArgumentNullException(nameof(backofficeSecurityAccessor)); _textService = textService ?? throw new ArgumentNullException(nameof(textService)); _mediaService = mediaService ?? throw new ArgumentNullException(nameof(mediaService)); - _entityService = entityService ?? throw new ArgumentNullException(nameof(entityService)); _propertyValidationService = propertyValidationService ?? throw new ArgumentNullException(nameof(propertyValidationService)); + _authorizationService = authorizationService ?? throw new ArgumentNullException(nameof(authorizationService)); } - public void OnActionExecuting(ActionExecutingContext context) + public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) + { + // on executing... + await OnActionExecutingAsync(context); + + await next(); //need to pass the execution to next + + // on executed... + } + + private async Task OnActionExecutingAsync(ActionExecutingContext context) { var model = (MediaItemSave) context.ActionArguments["contentItem"]; var contentItemValidator = new MediaSaveModelValidator(_loggerFactory.CreateLogger(), _backofficeSecurityAccessor.BackOfficeSecurity, _textService, _propertyValidationService); - if (ValidateUserAccess(model, context)) + if (await ValidateUserAccessAsync(model, context)) { //now do each validation step if (contentItemValidator.ValidateExistingContent(model, context)) @@ -68,7 +77,7 @@ namespace Umbraco.Web.BackOffice.Filters /// /// /// - private bool ValidateUserAccess(MediaItemSave mediaItem, ActionExecutingContext actionContext) + private async Task ValidateUserAccessAsync(MediaItemSave mediaItem, ActionExecutingContext actionContext) { //We now need to validate that the user is allowed to be doing what they are doing. //Then if it is new, we need to lookup those permissions on the parent. @@ -100,11 +109,12 @@ namespace Umbraco.Web.BackOffice.Filters return false; } - if (MediaController.CheckPermissions( - actionContext.HttpContext.Items, - _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, - _mediaService, _entityService, - contentIdToCheck, contentToCheck) == false) + var requirement = contentToCheck == null + ? new MediaPermissionResourceRequirement(contentIdToCheck) + : new MediaPermissionResourceRequirement(); + + var authorizationResult = await _authorizationService.AuthorizeAsync(actionContext.HttpContext.User, contentToCheck, requirement); + if (!authorizationResult.Succeeded) { actionContext.Result = new ForbidResult(); return false; @@ -112,11 +122,6 @@ namespace Umbraco.Web.BackOffice.Filters return true; } - - public void OnActionExecuted(ActionExecutedContext context) - { - - } } } } diff --git a/src/Umbraco.Web.BackOffice/Trees/ContentTreeControllerBase.cs b/src/Umbraco.Web.BackOffice/Trees/ContentTreeControllerBase.cs index 36b9f7f5de..53a6f02a79 100644 --- a/src/Umbraco.Web.BackOffice/Trees/ContentTreeControllerBase.cs +++ b/src/Umbraco.Web.BackOffice/Trees/ContentTreeControllerBase.cs @@ -123,7 +123,7 @@ namespace Umbraco.Web.BackOffice.Trees internal TreeNode GetSingleTreeNodeWithAccessCheck(IEntitySlim e, string parentId, FormCollection queryStrings, int[] startNodeIds, string[] startNodePaths) { - var entityIsAncestorOfStartNodes = ContentPermissionsHelper.IsInBranchOfStartNode(e.Path, startNodeIds, startNodePaths, out var hasPathAccess); + var entityIsAncestorOfStartNodes = ContentPermissions.IsInBranchOfStartNode(e.Path, startNodeIds, startNodePaths, out var hasPathAccess); var ignoreUserStartNodes = IgnoreUserStartNodes(queryStrings); if (ignoreUserStartNodes == false && entityIsAncestorOfStartNodes == false) return null; diff --git a/src/Umbraco.Web.Common/Authorization/AuthorizationPolicies.cs b/src/Umbraco.Web.Common/Authorization/AuthorizationPolicies.cs index 644ee4dd71..335dc5397b 100644 --- a/src/Umbraco.Web.Common/Authorization/AuthorizationPolicies.cs +++ b/src/Umbraco.Web.Common/Authorization/AuthorizationPolicies.cs @@ -23,6 +23,8 @@ public const string ContentPermissionBrowseById = nameof(ContentPermissionBrowseById); public const string ContentPermissionDeleteById = nameof(ContentPermissionDeleteById); + public const string MediaPermissionPathById = nameof(MediaPermissionPathById); + // Single section access public const string SectionAccessContent = nameof(SectionAccessContent); diff --git a/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs b/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs index 8acea23289..a6c13bf50b 100644 --- a/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs +++ b/src/Umbraco.Web.Common/Runtime/AspNetCoreComposer.cs @@ -96,6 +96,9 @@ namespace Umbraco.Web.Common.Runtime composition.Services.AddUnique(); composition.Services.AddUnique(); composition.Services.AddUnique(factory => new LegacyPasswordSecurity()); + + composition.Services.AddUnique(); + composition.Services.AddUnique(); } } }