diff --git a/src/Umbraco.Tests/Controllers/WebApiEditors/EnsureUserPermissionForContentAttributeTests.cs b/src/Umbraco.Tests/Controllers/WebApiEditors/EnsureUserPermissionForContentAttributeTests.cs index 4a89181276..5f24e12e21 100644 --- a/src/Umbraco.Tests/Controllers/WebApiEditors/EnsureUserPermissionForContentAttributeTests.cs +++ b/src/Umbraco.Tests/Controllers/WebApiEditors/EnsureUserPermissionForContentAttributeTests.cs @@ -14,6 +14,7 @@ using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; using Umbraco.Tests.TestHelpers; using Umbraco.Web; +using Umbraco.Web.Editors; using Umbraco.Web.WebApi; using Umbraco.Web.WebApi.Filters; using umbraco.presentation.channels.businesslogic; @@ -21,7 +22,7 @@ using umbraco.presentation.channels.businesslogic; namespace Umbraco.Tests.Controllers.WebApiEditors { [TestFixture] - public class EnsureUserPermissionForContentAttributeTests + public class ContentControllerUnitTests { [Test] public void Does_Not_Throw_Exception_When_Access_Allowed_By_Path() @@ -37,12 +38,9 @@ namespace Umbraco.Tests.Controllers.WebApiEditors var userService = MockRepository.GenerateStub(); var permissions = new List(); userService.Stub(x => x.GetPermissions(user, 1234)).Return(permissions); - var ctx = new HttpActionContext(); - ctx.ActionArguments.Add("id", 1234); - var attribute = new EnsureUserPermissionForContentAttribute(1234); //act - var result = attribute.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); //assert Assert.IsTrue(result); @@ -62,11 +60,9 @@ namespace Umbraco.Tests.Controllers.WebApiEditors var userService = MockRepository.GenerateStub(); var permissions = new List(); userService.Stub(x => x.GetPermissions(user, 1234)).Return(permissions); - var ctx = new HttpActionContext(); - var attribute = new EnsureUserPermissionForContentAttribute(1234); //act/assert - Assert.Throws(() => attribute.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F')); + Assert.Throws(() => ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F')); } [Test] @@ -83,11 +79,9 @@ namespace Umbraco.Tests.Controllers.WebApiEditors var userService = MockRepository.GenerateStub(); var permissions = new List(); userService.Stub(x => x.GetPermissions(user, 1234)).Return(permissions); - var ctx = new HttpActionContext(); - var attribute = new EnsureUserPermissionForContentAttribute(1234); //act - var result = attribute.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); //assert Assert.IsFalse(result); @@ -110,12 +104,9 @@ namespace Umbraco.Tests.Controllers.WebApiEditors new EntityPermission(9, 1234, new string[]{ "A", "B", "C" }) }; userService.Stub(x => x.GetPermissions(user, 1234)).Return(permissions); - var ctx = new HttpActionContext(); - var attribute = new EnsureUserPermissionForContentAttribute(1234); - //act - var result = attribute.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); //assert Assert.IsFalse(result); @@ -138,12 +129,9 @@ namespace Umbraco.Tests.Controllers.WebApiEditors new EntityPermission(9, 1234, new string[]{ "A", "F", "C" }) }; userService.Stub(x => x.GetPermissions(user, 1234)).Return(permissions); - var ctx = new HttpActionContext(); - ctx.ActionArguments.Add("id", 1234); - var attribute = new EnsureUserPermissionForContentAttribute(1234); //act - var result = attribute.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); //assert Assert.IsTrue(result); @@ -163,12 +151,9 @@ namespace Umbraco.Tests.Controllers.WebApiEditors var userService = MockRepository.GenerateStub(); var permissions = new List(); userService.Stub(x => x.GetPermissions(user, 1234)).Return(permissions); - var ctx = new HttpActionContext(); - ctx.ActionArguments.Add("id", 1234); - var attribute = new EnsureUserPermissionForContentAttribute(1234); //act - var result = attribute.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); + var result = ContentController.CheckPermissions(new Dictionary(), user, userService, contentService, 1234, 'F'); //assert Assert.IsTrue(result); diff --git a/src/Umbraco.Web.UI.Client/src/common/services/util.service.js b/src/Umbraco.Web.UI.Client/src/common/services/util.service.js index 51749ec24f..e5393069c9 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/util.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/util.service.js @@ -294,7 +294,7 @@ function umbDataFormatter() { _.each(tab.properties, function (prop) { //don't include the custom generic tab properties - if (tab.id !== 0 && !prop.alias.startsWith("_umb_")) { + if (!prop.alias.startsWith("_umb_")) { saveModel.properties.push({ id: prop.id, alias: prop.alias, diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index 2780547bae..84aa14cf4f 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Linq; using System.Net; using System.Net.Http; @@ -25,6 +26,7 @@ using Umbraco.Web.WebApi.Filters; using umbraco; using Umbraco.Core.Models; using Umbraco.Core.Dynamics; +using umbraco.BusinessLogic.Actions; using Constants = Umbraco.Core.Constants; namespace Umbraco.Web.Editors @@ -78,12 +80,7 @@ namespace Umbraco.Web.Editors [EnsureUserPermissionForContent("id")] public ContentItemDisplay GetById(int id) { - //with the filter applied we need to check if the content has already been looked up - - var foundContent = !Request.Properties.ContainsKey("contentItem") - ? Services.ContentService.GetById(id) - : (IContent) Request.Properties["contentItem"]; - + var foundContent = GetEntityFromRequest(() => Services.ContentService.GetById(id)); if (foundContent == null) { HandleContentNotFound(id); @@ -171,20 +168,31 @@ namespace Umbraco.Web.Editors [ModelBinder(typeof(ContentItemBinder))] ContentItemSave contentItem) { + //We now need to validate that the user is allowed to be doing what they are doing + if (CheckPermissions( + Request.Properties, + Security.CurrentUser, + Services.UserService, + Services.ContentService, + contentItem.Id, + (int) contentItem.Action >= 2 ? ActionPublish.Instance.Letter : ActionSave.Instance.Letter, + contentItem.PersistedContent) == false) + { + throw new HttpResponseException(HttpStatusCode.Unauthorized); + } + //If we've reached here it means: // * Our model has been bound // * and validated // * any file attachments have been saved to their temporary location for us to use // * we have a reference to the DTO object and the persisted object - + UpdateName(contentItem); //TODO: We need to support 'send to publish' //TODO: We'll need to save the new template, publishat, etc... values here - - //TODO: We need to check the user's permissions to see if they are allowed to do this! - + MapPropertyValues(contentItem); //We need to manually check the validation results here because: @@ -392,5 +400,58 @@ namespace Umbraco.Web.Editors } } + /// + /// 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, + IUserService userService, + IContentService contentService, + int nodeId, + char permissionToCheck, + IContent contentItem = null) + { + if (contentItem == null) + { + contentItem = contentService.GetById(nodeId); + } + + if (contentItem == null) + { + throw new HttpResponseException(HttpStatusCode.NotFound); + } + + //put the content item into storage so it can be retreived + // in the controller (saves a lookup) + storage[typeof(IContent).ToString()] = contentItem; + + var hasPathAccess = user.HasPathAccess(contentItem); + + if (hasPathAccess == false) + { + return false; + } + + var permission = userService.GetPermissions(user, nodeId).FirstOrDefault(); + if (permission == null || permission.AssignedPermissions.Contains(permissionToCheck.ToString(CultureInfo.InvariantCulture))) + { + return true; + } + else + { + return false; + } + } + } } \ No newline at end of file diff --git a/src/Umbraco.Web/Editors/ContentControllerBase.cs b/src/Umbraco.Web/Editors/ContentControllerBase.cs index 2cf7121e50..c465edc758 100644 --- a/src/Umbraco.Web/Editors/ContentControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentControllerBase.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; using System.Net; using System.Net.Http; @@ -117,5 +118,24 @@ namespace Umbraco.Web.Editors } } + /// + /// A helper method to attempt to get the instance from the request storage if it can be found there, + /// otherwise gets it from the callback specified + /// + /// + /// + /// + /// + /// This is useful for when filters have alraedy looked up a persisted entity and we don't want to have + /// to look it up again. + /// + protected TPersisted GetEntityFromRequest(Func getFromService) + where TPersisted : IContentBase + { + return Request.Properties.ContainsKey(typeof (TPersisted).ToString()) == false + ? getFromService() + : (TPersisted) Request.Properties[typeof (TPersisted).ToString()]; + } + } } \ No newline at end of file diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentSaveAction.cs b/src/Umbraco.Web/Models/ContentEditing/ContentSaveAction.cs index 91ac6e4fe4..337c97e12b 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentSaveAction.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentSaveAction.cs @@ -8,21 +8,21 @@ /// /// Saves the content item, no publish /// - Save, - - /// - /// Saves and publishes the content item - /// - Publish, + Save = 0, /// /// Saves a new content item /// - SaveNew, + SaveNew = 1, + + /// + /// Saves and publishes the content item + /// + Publish = 2, /// /// Saves an publishes a new content item /// - PublishNew + PublishNew = 3 } } \ No newline at end of file diff --git a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs index 0996a29bf6..65ef7ba3ed 100644 --- a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForContentAttribute.cs @@ -10,6 +10,8 @@ using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; +using Umbraco.Web.Editors; +using Umbraco.Web.Models.ContentEditing; using umbraco.BusinessLogic.Actions; namespace Umbraco.Web.WebApi.Filters @@ -70,14 +72,14 @@ namespace Umbraco.Web.WebApi.Filters throw new InvalidOperationException("No argument found for the current action with the name: " + _paramName); } - nodeId = (int) actionContext.ActionArguments[_paramName]; + nodeId = (int)actionContext.ActionArguments[_paramName]; } else { nodeId = _nodeId.Value; } - if (CheckPermissions( + if (ContentController.CheckPermissions( actionContext.Request.Properties, UmbracoContext.Current.Security.CurrentUser, ApplicationContext.Current.Services.UserService, @@ -92,35 +94,7 @@ namespace Umbraco.Web.WebApi.Filters } - internal bool CheckPermissions(IDictionary storage, IUser user, IUserService userService, IContentService contentService, int nodeId, char permissionToCheck) - { - var contentItem = contentService.GetById(nodeId); - if (contentItem == null) - { - throw new HttpResponseException(System.Net.HttpStatusCode.NotFound); - } - - //put the content item into storage so it can be retreived - // in the controller (saves a lookup) - storage.Add(typeof(IContent).ToString(), contentItem); - - var hasPathAccess = user.HasPathAccess(contentItem); - - if (hasPathAccess == false) - { - return false; - } - - var permission = userService.GetPermissions(user, nodeId).FirstOrDefault(); - if (permission == null || permission.AssignedPermissions.Contains(permissionToCheck.ToString(CultureInfo.InvariantCulture))) - { - return true; - } - else - { - return false; - } - } + } } \ No newline at end of file