From 7c9f5eda9dd18c7fd752bc45f284f22e2e9a57d8 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 3 Sep 2013 16:35:36 +1000 Subject: [PATCH] Fixes up more permissions checks, refactors MediaController.PostAddFile to use the correct WebAPI usage. Fixes issue with UmbracoClientManager throwing errors because the treeProps didn't include all called props. --- src/Umbraco.Core/Models/ContentBase.cs | 3 +- src/Umbraco.Core/Models/ContentExtensions.cs | 29 +----- .../Application/UmbracoClientManager.js | 2 +- src/Umbraco.Web/Editors/MediaController.cs | 90 +++++++++++++------ .../Models/ContentEditing/EntityBasic.cs | 2 +- .../Models/Mapping/ContentModelMapper.cs | 2 +- .../WebApi/Binders/ContentItemBaseBinder.cs | 3 +- .../EnsureUserPermissionForMediaAttribute.cs | 33 ++++++- .../FilterAllowedOutgoingContentAttribute.cs | 11 +-- .../FilterAllowedOutgoingMediaAttribute.cs | 7 +- 10 files changed, 118 insertions(+), 64 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index 738a896155..de586b1716 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -372,7 +372,7 @@ namespace Umbraco.Core.Models /// Sets the value of a Property /// /// Alias of the PropertyType - /// Value to set for the Property + /// Value to set for the Property public virtual void SetPropertyValue(string propertyTypeAlias, HttpPostedFile value) { ContentExtensions.SetValue(this, propertyTypeAlias, value); @@ -393,6 +393,7 @@ namespace Umbraco.Core.Models /// /// Alias of the PropertyType /// Value to set for the Property + [Obsolete("There is no reason for this overload since HttpPostedFileWrapper inherits from HttpPostedFileBase")] public virtual void SetPropertyValue(string propertyTypeAlias, HttpPostedFileWrapper value) { ContentExtensions.SetValue(this, propertyTypeAlias, value); diff --git a/src/Umbraco.Core/Models/ContentExtensions.cs b/src/Umbraco.Core/Models/ContentExtensions.cs index 6026171a31..ba0511cda4 100644 --- a/src/Umbraco.Core/Models/ContentExtensions.cs +++ b/src/Umbraco.Core/Models/ContentExtensions.cs @@ -365,20 +365,7 @@ namespace Umbraco.Core.Models /// The containing the file that will be uploaded public static void SetValue(this IContentBase content, string propertyTypeAlias, HttpPostedFile value) { - // Ensure we get the filename without the path in IE in intranet mode - // http://stackoverflow.com/questions/382464/httppostedfile-filename-different-from-ie - var fileName = value.FileName; - if (fileName.LastIndexOf(@"\") > 0) - fileName = fileName.Substring(fileName.LastIndexOf(@"\") + 1); - - var name = - IOHelper.SafeFileName( - fileName.Substring(fileName.LastIndexOf(IOHelper.DirSepChar) + 1, - fileName.Length - fileName.LastIndexOf(IOHelper.DirSepChar) - 1) - .ToLower()); - - if (string.IsNullOrEmpty(name) == false) - SetFileOnContent(content, propertyTypeAlias, name, value.InputStream); + SetValue(content, propertyTypeAlias, (HttpPostedFileBase)new HttpPostedFileWrapper(value)); } /// @@ -387,20 +374,12 @@ namespace Umbraco.Core.Models /// to add property value to /// Alias of the property to save the value on /// The containing the file that will be uploaded + [Obsolete("There is no reason for this overload since HttpPostedFileWrapper inherits from HttpPostedFileBase")] public static void SetValue(this IContentBase content, string propertyTypeAlias, HttpPostedFileWrapper value) { - // Ensure we get the filename without the path in IE in intranet mode - // http://stackoverflow.com/questions/382464/httppostedfile-filename-different-from-ie - var fileName = value.FileName; - if (fileName.LastIndexOf(@"\") > 0) - fileName = fileName.Substring(fileName.LastIndexOf(@"\") + 1); - - var name = IOHelper.SafeFileName(fileName); - - if (string.IsNullOrEmpty(name) == false) - SetFileOnContent(content, propertyTypeAlias, name, value.InputStream); + SetValue(content, propertyTypeAlias, (HttpPostedFileBase)value); } - + /// /// Sets and uploads the file from a as the property value /// diff --git a/src/Umbraco.Web.UI/umbraco_client/Application/UmbracoClientManager.js b/src/Umbraco.Web.UI/umbraco_client/Application/UmbracoClientManager.js index 651d63162b..5d2ec4d268 100644 --- a/src/Umbraco.Web.UI/umbraco_client/Application/UmbracoClientManager.js +++ b/src/Umbraco.Web.UI/umbraco_client/Application/UmbracoClientManager.js @@ -57,7 +57,7 @@ Umbraco.Sys.registerNamespace("Umbraco.Application"); || this.mainWindow().jQuery(".umbTree").UmbracoTreeAPI() == null) { //creates a false tree with all the public tree params set to a false method. var tmpTree = {}; - var treeProps = ["init", "setRecycleBinNodeId", "clearTreeCache", "toggleEditMode", "refreshTree", "rebuildTree", "saveTreeState", "syncTree", "childNodeCreated", "moveNode", "copyNode", "findNode", "selectNode", "reloadActionNode", "getActionNode", "setActiveTreeType", "getNodeDef"]; + var treeProps = ["init", "setRecycleBinNodeId", "clearTreeCache", "toggleEditMode", "refreshTree", "rebuildTree", "saveTreeState", "syncTree", "childNodeCreated", "moveNode", "copyNode", "findNode", "selectNode", "reloadActionNode", "getActionNode", "setActiveTreeType", "getNodeDef", "addEventHandler", "removeEventHandler"]; for (var p in treeProps) { tmpTree[treeProps[p]] = function() { return false; }; } diff --git a/src/Umbraco.Web/Editors/MediaController.cs b/src/Umbraco.Web/Editors/MediaController.cs index c5aa02b049..c2e33a3a9f 100644 --- a/src/Umbraco.Web/Editors/MediaController.cs +++ b/src/Umbraco.Web/Editors/MediaController.cs @@ -1,11 +1,16 @@ using System; using System.Collections.Generic; +using System.IO; using System.Net; using System.Net.Http; +using System.Net.Http.Formatting; +using System.Threading.Tasks; +using System.Web; using System.Web.Http; using System.Web.Http.ModelBinding; using AutoMapper; using Umbraco.Core; +using Umbraco.Core.IO; using Umbraco.Core.Logging; using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; @@ -89,6 +94,7 @@ namespace Umbraco.Web.Editors /// /// /// + [FilterAllowedOutgoingMedia(typeof(IEnumerable))] public IEnumerable GetByIds([FromUri]int[] ids) { var foundMedia = ((MediaService)Services.MediaService).GetByIds(ids); @@ -99,6 +105,7 @@ namespace Umbraco.Web.Editors /// /// Returns the root media objects /// + [FilterAllowedOutgoingMedia(typeof(IEnumerable>))] public IEnumerable> GetRootMedia() { //TODO: Add permissions check! @@ -110,11 +117,11 @@ namespace Umbraco.Web.Editors /// /// Returns the child media objects /// + [FilterAllowedOutgoingMedia(typeof(IEnumerable>))] public IEnumerable> GetChildren(int parentId) { //TODO: Change this to be like content with paged params - //TODO: filter results based on permissions! - + return Services.MediaService.GetChildren(parentId) .Select(Mapper.Map>); } @@ -185,6 +192,7 @@ namespace Umbraco.Web.Editors /// /// /// + [EnsureUserPermissionForMedia("sorted.ParentId")] public HttpResponseMessage PostSort(ContentSortOrder sorted) { if (sorted == null) @@ -219,7 +227,7 @@ namespace Umbraco.Web.Editors } } - //shorthand to use with the media dialog + [EnsureUserPermissionForMedia("folder.ParentId")] public MediaItemDisplay PostAddFolder(EntityBasic folder) { var mediaService = ApplicationContext.Services.MediaService; @@ -229,32 +237,64 @@ namespace Umbraco.Web.Editors return Mapper.Map(f); } - //short hand to use with the uploader in the media dialog - public HttpResponseMessage PostAddFile() + /// + /// Used to submit a media file + /// + /// + /// + /// We cannot validate this request with attributes (nicely) due to the nature of the multi-part for data. + /// + public async Task PostAddFile() { - var context = UmbracoContext.HttpContext; - if(context.Request.Files.Count > 0) + if (Request.Content.IsMimeMultipartContent() == false) { - if (context.Request.Form.Count > 0) - { - int parentId; - if (int.TryParse(context.Request.Form[0], out parentId)) - { - var file = context.Request.Files[0]; - var name = file.FileName; - - var mediaService = base.ApplicationContext.Services.MediaService; - var f = mediaService.CreateMedia(name, parentId, Constants.Conventions.MediaTypes.Image); - f.SetValue(Constants.Conventions.Media.File, file); - mediaService.Save(f); - - return new HttpResponseMessage(HttpStatusCode.OK); - } - } - + throw new HttpResponseException(HttpStatusCode.UnsupportedMediaType); } - return new HttpResponseMessage(HttpStatusCode.NotFound); + var root = IOHelper.MapPath("~/App_Data/TEMP/FileUploads"); + //ensure it exists + Directory.CreateDirectory(root); + var provider = new MultipartFormDataStreamProvider(root); + + var result = await Request.Content.ReadAsMultipartAsync(provider); + + //must have a file + if (result.FileData.Count == 0) + { + return new HttpResponseMessage(HttpStatusCode.NotFound); + } + + //get the string json from the request + int parentId; + if (int.TryParse(result.FormData["currentFolder"], out parentId) == false) + { + throw new HttpResponseException( + new HttpResponseMessage(HttpStatusCode.BadRequest) + { + ReasonPhrase = "The request was not formatted correctly, the currentFolder is not an integer" + }); + } + + //get the files + foreach (var file in result.FileData) + { + var fileName = file.Headers.ContentDisposition.FileName.Trim(new[] { '\"' }); + + var mediaService = ApplicationContext.Services.MediaService; + var f = mediaService.CreateMedia(fileName, parentId, Constants.Conventions.MediaTypes.Image); + + using (var fs = System.IO.File.OpenRead(file.LocalFileName)) + { + f.SetValue(Constants.Conventions.Media.File, fileName, fs); + } + + mediaService.Save(f); + + //now we can remove the temp file + System.IO.File.Delete(file.LocalFileName); + } + + return new HttpResponseMessage(HttpStatusCode.OK); } diff --git a/src/Umbraco.Web/Models/ContentEditing/EntityBasic.cs b/src/Umbraco.Web/Models/ContentEditing/EntityBasic.cs index c9ee842339..b5799e986d 100644 --- a/src/Umbraco.Web/Models/ContentEditing/EntityBasic.cs +++ b/src/Umbraco.Web/Models/ContentEditing/EntityBasic.cs @@ -31,6 +31,6 @@ namespace Umbraco.Web.Models.ContentEditing /// For now we'll exclude this from the json results, this is needed for permissions check filtering /// [IgnoreDataMember] - internal string Path { get; set; } + public string Path { get; set; } } } diff --git a/src/Umbraco.Web/Models/Mapping/ContentModelMapper.cs b/src/Umbraco.Web/Models/Mapping/ContentModelMapper.cs index a73dd5cb49..533ea5b227 100644 --- a/src/Umbraco.Web/Models/Mapping/ContentModelMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/ContentModelMapper.cs @@ -26,7 +26,7 @@ namespace Umbraco.Web.Models.Mapping //FROM IContent TO ContentItemDisplay config.CreateMap() - .ForMember( + .ForMember( dto => dto.Owner, expression => expression.ResolveUsing>()) .ForMember( diff --git a/src/Umbraco.Web/WebApi/Binders/ContentItemBaseBinder.cs b/src/Umbraco.Web/WebApi/Binders/ContentItemBaseBinder.cs index e7805adaaa..d1e843b6ef 100644 --- a/src/Umbraco.Web/WebApi/Binders/ContentItemBaseBinder.cs +++ b/src/Umbraco.Web/WebApi/Binders/ContentItemBaseBinder.cs @@ -16,6 +16,7 @@ using System.Web.ModelBinding; using Newtonsoft.Json; using Newtonsoft.Json.Serialization; using Umbraco.Core; +using Umbraco.Core.IO; using Umbraco.Core.Models; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Security; @@ -54,7 +55,7 @@ namespace Umbraco.Web.WebApi.Binders throw new HttpResponseException(HttpStatusCode.UnsupportedMediaType); } - var root = HttpContext.Current.Server.MapPath("~/App_Data/TEMP/FileUploads"); + var root = IOHelper.MapPath("~/App_Data/TEMP/FileUploads"); //ensure it exists Directory.CreateDirectory(root); var provider = new MultipartFormDataStreamProvider(root); diff --git a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForMediaAttribute.cs b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForMediaAttribute.cs index d85fe10fea..cb0f07f796 100644 --- a/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForMediaAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/EnsureUserPermissionForMediaAttribute.cs @@ -22,6 +22,14 @@ namespace Umbraco.Web.WebApi.Filters { private readonly int? _nodeId; private readonly string _paramName; + private DictionarySource _source; + + public enum DictionarySource + { + ActionArguments, + RequestForm, + RequestQueryString + } /// /// This constructor will only be able to test the start node access @@ -34,7 +42,15 @@ namespace Umbraco.Web.WebApi.Filters public EnsureUserPermissionForMediaAttribute(string paramName) { Mandate.ParameterNotNullOrEmpty(paramName, "paramName"); - _paramName = paramName; + _paramName = paramName; + _source = DictionarySource.ActionArguments; + } + + public EnsureUserPermissionForMediaAttribute(string paramName, DictionarySource source) + { + Mandate.ParameterNotNullOrEmpty(paramName, "paramName"); + _paramName = paramName; + _source = source; } public override bool AllowMultiple @@ -94,6 +110,21 @@ namespace Umbraco.Web.WebApi.Filters } + //private object GetValueFromSource(HttpActionContext actionContext, string key) + //{ + // switch (_source) + // { + // case DictionarySource.ActionArguments: + // return actionContext.ActionArguments[key]; + // case DictionarySource.RequestForm: + // return actionContext.Request.Properties + // case DictionarySource.RequestQueryString: + // break; + // default: + // throw new ArgumentOutOfRangeException(); + // } + //} + } diff --git a/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingContentAttribute.cs b/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingContentAttribute.cs index 903f819f9b..dda78dc4d2 100644 --- a/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingContentAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingContentAttribute.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using System.Collections.Generic; using System.Globalization; using System.Linq; @@ -35,25 +36,25 @@ namespace Umbraco.Web.WebApi.Filters { } - protected override void FilterItems(IUser user, List items) + protected override void FilterItems(IUser user, IList items) { base.FilterItems(user, items); FilterBasedOnPermissions(items, user, ApplicationContext.Current.Services.UserService); } - - internal void FilterBasedOnPermissions(List items, IUser user, IUserService userService) + + internal void FilterBasedOnPermissions(IList items, IUser user, IUserService userService) { var length = items.Count; var ids = new List(); for (var i = 0; i < length; i++) { - ids.Add(items[i].Id); + ids.Add(((dynamic)items[i]).Id); } //get all the permissions for these nodes in one call var permissions = userService.GetPermissions(user, ids.ToArray()).ToArray(); var toRemove = new List(); - foreach(var item in items) + foreach(dynamic item in items) { var nodePermission = permissions.Where(x => x.EntityId == item.Id).ToArray(); //if there are no permissions for this id, then remove the item diff --git a/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingMediaAttribute.cs b/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingMediaAttribute.cs index 130ee28fb4..5c0d141e82 100644 --- a/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingMediaAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/FilterAllowedOutgoingMediaAttribute.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using System.Collections.Generic; using System.Linq; using System.Net.Http; @@ -66,15 +67,15 @@ namespace Umbraco.Web.WebApi.Filters base.OnActionExecuted(actionExecutedContext); } - protected virtual void FilterItems(IUser user, List items) + protected virtual void FilterItems(IUser user, IList items) { FilterBasedOnStartNode(items, user); } - internal void FilterBasedOnStartNode(List items, IUser user) + internal void FilterBasedOnStartNode(IList items, IUser user) { var toRemove = new List(); - foreach (var item in items) + foreach (dynamic item in items) { var hasPathAccess = UserExtensions.HasPathAccess(item.Path, user.StartContentId, Constants.System.RecycleBinContent); if (!hasPathAccess)