From c3606b31ce8cc487dde756ce76201e659e7e720c Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 28 May 2020 15:20:02 +0200 Subject: [PATCH] https://dev.azure.com/umbraco/D-Team%20Tracker/_workitems/edit/6587 - Clean up and fixed issue with Ambiguous actions --- .../Controllers/CodeFileController.cs | 79 +++++++++---------- .../Controllers/DataTypeController.cs | 14 ++-- ...rmineAmbiguousActionByPassingParameters.cs | 51 ++++++++++++ .../Controllers/LanguageController.cs | 2 + .../Controllers/RelationTypeController.cs | 4 +- ...alidateAngularAntiForgeryTokenAttribute.cs | 1 - ...racoApiBehaviorApplicationModelProvider.cs | 2 + 7 files changed, 103 insertions(+), 50 deletions(-) create mode 100644 src/Umbraco.Web.BackOffice/Controllers/DetermineAmbiguousActionByPassingParameters.cs diff --git a/src/Umbraco.Web.BackOffice/Controllers/CodeFileController.cs b/src/Umbraco.Web.BackOffice/Controllers/CodeFileController.cs index 63a7330c29..05c0513bed 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/CodeFileController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/CodeFileController.cs @@ -24,7 +24,7 @@ using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Editors; -namespace Umbraco.Web.BackOffice.Controllers + namespace Umbraco.Web.BackOffice.Controllers { // TODO: Put some exception filters in our webapi to return 404 instead of 500 when we throw ArgumentNullException // ref: https://www.exceptionnotfound.net/the-asp-net-web-api-exception-handling-pipeline-a-guided-tour/ @@ -70,7 +70,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// Will return a simple 200 if file creation succeeds [ValidationFilter] - public IActionResult PostCreate(string type, CodeFileDisplay display) + public ActionResult PostCreate(string type, CodeFileDisplay display) { if (display == null) throw new ArgumentNullException("display"); if (string.IsNullOrWhiteSpace(type)) throw new ArgumentException("Value cannot be null or whitespace.", "type"); @@ -108,7 +108,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// The name of the container/folder /// [HttpPost] - public IActionResult PostCreateContainer(string type, string parentId, string name) + public ActionResult PostCreateContainer(string type, string parentId, string name) { if (string.IsNullOrWhiteSpace(type)) throw new ArgumentException("Value cannot be null or whitespace.", "type"); if (string.IsNullOrWhiteSpace(parentId)) throw new ArgumentException("Value cannot be null or whitespace.", "parentId"); @@ -154,11 +154,11 @@ namespace Umbraco.Web.BackOffice.Controllers } - return Ok(new CodeFileDisplay + return new CodeFileDisplay { VirtualPath = virtualPath, Path = Url.GetTreePathFromFilePath(virtualPath) - }); + }; } /// @@ -167,7 +167,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// This is a string but will be 'scripts' 'partialViews', 'partialViewMacros' or 'stylesheets' /// The filename or urlencoded path of the file to open /// The file and its contents from the virtualPath - public IActionResult GetByPath(string type, string virtualPath) + public ActionResult GetByPath(string type, string virtualPath) { if (string.IsNullOrWhiteSpace(type)) throw new ArgumentException("Value cannot be null or whitespace.", "type"); if (string.IsNullOrWhiteSpace(virtualPath)) throw new ArgumentException("Value cannot be null or whitespace.", "virtualPath"); @@ -176,49 +176,49 @@ namespace Umbraco.Web.BackOffice.Controllers switch (type) { - case Core.Constants.Trees.PartialViews: + case Constants.Trees.PartialViews: var view = _fileService.GetPartialView(virtualPath); if (view != null) { var display = _umbracoMapper.Map(view); - display.FileType = Core.Constants.Trees.PartialViews; + display.FileType = Constants.Trees.PartialViews; display.Path = Url.GetTreePathFromFilePath(view.Path); display.Id = System.Web.HttpUtility.UrlEncode(view.Path); - return Ok(display); + return display; } break; - case Core.Constants.Trees.PartialViewMacros: + case Constants.Trees.PartialViewMacros: var viewMacro = _fileService.GetPartialViewMacro(virtualPath); if (viewMacro != null) { var display = _umbracoMapper.Map(viewMacro); - display.FileType = Core.Constants.Trees.PartialViewMacros; + display.FileType = Constants.Trees.PartialViewMacros; display.Path = Url.GetTreePathFromFilePath(viewMacro.Path); display.Id = System.Web.HttpUtility.UrlEncode(viewMacro.Path); - return Ok(display); + return display; } break; - case Core.Constants.Trees.Scripts: + case Constants.Trees.Scripts: var script = _fileService.GetScriptByName(virtualPath); if (script != null) { var display = _umbracoMapper.Map(script); - display.FileType = Core.Constants.Trees.Scripts; + display.FileType = Constants.Trees.Scripts; display.Path = Url.GetTreePathFromFilePath(script.Path); display.Id = System.Web.HttpUtility.UrlEncode(script.Path); - return Ok(display); + return display; } break; - case Core.Constants.Trees.Stylesheets: + case Constants.Trees.Stylesheets: var stylesheet = _fileService.GetStylesheetByName(virtualPath); if (stylesheet != null) { var display = _umbracoMapper.Map(stylesheet); - display.FileType = Core.Constants.Trees.Stylesheets; + display.FileType = Constants.Trees.Stylesheets; display.Path = Url.GetTreePathFromFilePath(stylesheet.Path); display.Id = System.Web.HttpUtility.UrlEncode(stylesheet.Path); - return Ok(display); + return display; } break; } @@ -263,12 +263,12 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// /// - public IActionResult GetScaffold(string type, string id, string snippetName = null) + public ActionResult GetScaffold(string type, string id, string snippetName = null) { if (string.IsNullOrWhiteSpace(type)) throw new ArgumentException("Value cannot be null or whitespace.", "type"); if (string.IsNullOrWhiteSpace(id)) throw new ArgumentException("Value cannot be null or whitespace.", "id"); - CodeFileDisplay codeFileDisplay; + CodeFileDisplay codeFileDisplay; switch (type) { @@ -308,7 +308,7 @@ namespace Umbraco.Web.BackOffice.Controllers codeFileDisplay.VirtualPath = codeFileDisplay.VirtualPath.TrimStart("~"); codeFileDisplay.FileType = type; - return new OkObjectResult(codeFileDisplay); + return codeFileDisplay; } /// @@ -328,8 +328,8 @@ namespace Umbraco.Web.BackOffice.Controllers var currentUser = _umbracoContextAccessor.GetRequiredUmbracoContext().Security.CurrentUser; switch (type) { - case Core.Constants.Trees.PartialViews: - if (IsDirectory(virtualPath, Core.Constants.SystemDirectories.PartialViews)) + case Constants.Trees.PartialViews: + if (IsDirectory(virtualPath, Constants.SystemDirectories.PartialViews)) { _fileService.DeletePartialViewFolder(virtualPath); return Ok(); @@ -340,8 +340,8 @@ namespace Umbraco.Web.BackOffice.Controllers } return new UmbracoProblemResult("No Partial View or folder found with the specified path", HttpStatusCode.NotFound); - case Core.Constants.Trees.PartialViewMacros: - if (IsDirectory(virtualPath, Core.Constants.SystemDirectories.MacroPartials)) + case Constants.Trees.PartialViewMacros: + if (IsDirectory(virtualPath, Constants.SystemDirectories.MacroPartials)) { _fileService.DeletePartialViewMacroFolder(virtualPath); return Ok(); @@ -352,7 +352,7 @@ namespace Umbraco.Web.BackOffice.Controllers } return new UmbracoProblemResult("No Partial View Macro or folder found with the specified path", HttpStatusCode.NotFound); - case Core.Constants.Trees.Scripts: + case Constants.Trees.Scripts: if (IsDirectory(virtualPath, _globalSettings.UmbracoScriptsPath)) { _fileService.DeleteScriptFolder(virtualPath); @@ -364,7 +364,7 @@ namespace Umbraco.Web.BackOffice.Controllers return Ok(); } return new UmbracoProblemResult("No Script or folder found with the specified path", HttpStatusCode.NotFound); - case Core.Constants.Trees.Stylesheets: + case Constants.Trees.Stylesheets: if (IsDirectory(virtualPath, _globalSettings.UmbracoCssPath)) { _fileService.DeleteStyleSheetFolder(virtualPath); @@ -379,8 +379,6 @@ namespace Umbraco.Web.BackOffice.Controllers default: return NotFound(); } - - throw new HttpResponseException(HttpStatusCode.NotFound); } /// @@ -388,10 +386,11 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// /// The updated CodeFileDisplay model - public IActionResult PostSave(CodeFileDisplay display) + public ActionResult PostSave(CodeFileDisplay display) { if (display == null) throw new ArgumentNullException("display"); + TryValidateModel(display); if (ModelState.IsValid == false) { return ValidationProblem(ModelState); @@ -399,14 +398,14 @@ namespace Umbraco.Web.BackOffice.Controllers switch (display.FileType) { - case Core.Constants.Trees.PartialViews: + case Constants.Trees.PartialViews: var partialViewResult = CreateOrUpdatePartialView(display); if (partialViewResult.Success) { display = _umbracoMapper.Map(partialViewResult.Result, display); display.Path = Url.GetTreePathFromFilePath(partialViewResult.Result.Path); display.Id = System.Web.HttpUtility.UrlEncode(partialViewResult.Result.Path); - return Ok(display); + return display; } display.AddErrorNotification( @@ -414,14 +413,14 @@ namespace Umbraco.Web.BackOffice.Controllers _localizedTextService.Localize("speechBubbles/partialViewErrorText")); break; - case Core.Constants.Trees.PartialViewMacros: + case Constants.Trees.PartialViewMacros: var partialViewMacroResult = CreateOrUpdatePartialViewMacro(display); if (partialViewMacroResult.Success) { display = _umbracoMapper.Map(partialViewMacroResult.Result, display); display.Path = Url.GetTreePathFromFilePath(partialViewMacroResult.Result.Path); display.Id = System.Web.HttpUtility.UrlEncode(partialViewMacroResult.Result.Path); - return Ok(display); + return display; } display.AddErrorNotification( @@ -429,31 +428,31 @@ namespace Umbraco.Web.BackOffice.Controllers _localizedTextService.Localize("speechBubbles/partialViewErrorText")); break; - case Core.Constants.Trees.Scripts: + case Constants.Trees.Scripts: var scriptResult = CreateOrUpdateScript(display); display = _umbracoMapper.Map(scriptResult, display); display.Path = Url.GetTreePathFromFilePath(scriptResult.Path); display.Id = System.Web.HttpUtility.UrlEncode(scriptResult.Path); - return Ok(display); + return display; //display.AddErrorNotification( // _localizedTextService.Localize("speechBubbles/partialViewErrorHeader"), // _localizedTextService.Localize("speechBubbles/partialViewErrorText")); - case Core.Constants.Trees.Stylesheets: + case Constants.Trees.Stylesheets: var stylesheetResult = CreateOrUpdateStylesheet(display); display = _umbracoMapper.Map(stylesheetResult, display); display.Path = Url.GetTreePathFromFilePath(stylesheetResult.Path); display.Id = System.Web.HttpUtility.UrlEncode(stylesheetResult.Path); - return Ok(display); + return display; default: - throw new HttpResponseException(HttpStatusCode.NotFound); + return NotFound(); } - return Ok(display); + return display; } /// diff --git a/src/Umbraco.Web.BackOffice/Controllers/DataTypeController.cs b/src/Umbraco.Web.BackOffice/Controllers/DataTypeController.cs index 8cc5d884bd..bdd1435b34 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/DataTypeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/DataTypeController.cs @@ -8,8 +8,6 @@ using Umbraco.Core.Models; using Umbraco.Core.PropertyEditors; using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; -using Umbraco.Web.WebApi.Filters; -using System.Net.Http; using System.Text; using Constants = Umbraco.Core.Constants; using Umbraco.Core.Mapping; @@ -18,10 +16,10 @@ using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Exceptions; +using Umbraco.Web.Editors; -namespace Umbraco.Web.Editors +namespace Umbraco.Web.BackOffice.Controllers { - /// /// The API controller used for editing data types /// @@ -68,7 +66,6 @@ namespace Umbraco.Web.Editors _umbracoContextAccessor = umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor)); } - /// /// Gets data type by name /// @@ -85,6 +82,7 @@ namespace Umbraco.Web.Editors /// /// /// + [DetermineAmbiguousActionByPassingParameters] public DataTypeDisplay GetById(int id) { var dataType = _dataTypeService.GetDataType(id); @@ -100,6 +98,7 @@ namespace Umbraco.Web.Editors /// /// /// + [DetermineAmbiguousActionByPassingParameters] public DataTypeDisplay GetById(Guid id) { var dataType = _dataTypeService.GetDataType(id); @@ -115,6 +114,7 @@ namespace Umbraco.Web.Editors /// /// /// + [DetermineAmbiguousActionByPassingParameters] public DataTypeDisplay GetById(Udi id) { var guidUdi = id as GuidUdi; @@ -265,7 +265,7 @@ namespace Umbraco.Web.Editors /// /// [TypeFilter(typeof(DataTypeValidateAttribute))] - public IActionResult PostSave(DataTypeSave dataType) + public ActionResult PostSave(DataTypeSave dataType) { //If we've made it here, then everything has been wired up and validated by the attribute @@ -297,7 +297,7 @@ namespace Umbraco.Web.Editors // map back to display model, and return var display = _umbracoMapper.Map(dataType.PersistedDataType); display.AddSuccessNotification(_localizedTextService.Localize("speechBubbles/dataTypeSaved"), ""); - return Ok(display); + return display; } /// diff --git a/src/Umbraco.Web.BackOffice/Controllers/DetermineAmbiguousActionByPassingParameters.cs b/src/Umbraco.Web.BackOffice/Controllers/DetermineAmbiguousActionByPassingParameters.cs new file mode 100644 index 0000000000..a1f7782ea6 --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Controllers/DetermineAmbiguousActionByPassingParameters.cs @@ -0,0 +1,51 @@ +using System; +using System.Linq; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.ActionConstraints; +using Microsoft.AspNetCore.Routing; +using Umbraco.Core; + +namespace Umbraco.Web.BackOffice.Controllers +{ + public class DetermineAmbiguousActionByPassingParameters : ActionMethodSelectorAttribute + { + public override bool IsValidForRequest(RouteContext routeContext, ActionDescriptor action) + { + var parameters = action.Parameters; + if (parameters.Any()) + { + var canUse = true; + foreach (var parameterDescriptor in parameters) + { + var value = routeContext.HttpContext.Request.Query[parameterDescriptor.Name]; + + if (parameterDescriptor.ParameterType == typeof(Udi)) + { + canUse &= UdiParser.TryParse(value, out _); + } + else if (parameterDescriptor.ParameterType == typeof(int)) + { + canUse &= int.TryParse(value, out _); + } + else if (parameterDescriptor.ParameterType == typeof(Guid)) + { + canUse &= Guid.TryParse(value, out _); + } + else if (parameterDescriptor.ParameterType == typeof(string)) + { + canUse = true; + } + else + { + canUse = false; + } + } + + return canUse; + } + + + return true; + } + } +} diff --git a/src/Umbraco.Web.BackOffice/Controllers/LanguageController.cs b/src/Umbraco.Web.BackOffice/Controllers/LanguageController.cs index 8038e7841a..91a99c9182 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/LanguageController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/LanguageController.cs @@ -9,6 +9,7 @@ using Umbraco.Core.Mapping; using Umbraco.Core.Models; using Umbraco.Core.Services; using Umbraco.Web.BackOffice.Filters; +using Umbraco.Web.BackOffice.Validation; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Exceptions; using Umbraco.Web.Editors; @@ -20,6 +21,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// Backoffice controller supporting the dashboard for language administration. /// [PluginController("UmbracoApi")] + //[PrefixlessBodyModelValidator] public class LanguageController : UmbracoAuthorizedJsonController { private readonly ILocalizationService _localizationService; diff --git a/src/Umbraco.Web.BackOffice/Controllers/RelationTypeController.cs b/src/Umbraco.Web.BackOffice/Controllers/RelationTypeController.cs index a0e0c1cb94..fe43944779 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/RelationTypeController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/RelationTypeController.cs @@ -108,7 +108,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// The relation type to create. /// A containing the persisted relation type's ID. - public IActionResult PostCreate(RelationTypeSave relationType) + public int PostCreate(RelationTypeSave relationType) { var relationTypePersisted = new RelationType(relationType.Name, relationType.Name.ToSafeAlias(_shortStringHelper, true), relationType.IsBidirectional, relationType.ChildObjectType, relationType.ParentObjectType); @@ -116,7 +116,7 @@ namespace Umbraco.Web.BackOffice.Controllers { _relationService.Save(relationTypePersisted); - return Ok(relationTypePersisted.Id); + return relationTypePersisted.Id; } catch (Exception ex) { diff --git a/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs index d2e22dc623..b8f035077b 100644 --- a/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/ValidateAngularAntiForgeryTokenAttribute.cs @@ -109,7 +109,6 @@ namespace Umbraco.Web.BackOffice.Filters _logger.Error(ex, "Could not validate XSRF token"); return false; } - return true; } } } diff --git a/src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs b/src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs index e332c89536..bbd9d5207a 100644 --- a/src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs +++ b/src/Umbraco.Web.Common/ApplicationModels/UmbracoApiBehaviorApplicationModelProvider.cs @@ -69,6 +69,8 @@ namespace Umbraco.Web.Common.ApplicationModels if (!IsUmbracoApiController(controller)) continue; + + foreach (var action in controller.Actions) { foreach (var convention in ActionModelConventions)