diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 4f4a83dc26..f41ff1dd07 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -267,6 +267,7 @@ + diff --git a/src/Umbraco.Tests/Web/Mvc/HtmlHelperExtensionMethodsTests.cs b/src/Umbraco.Tests/Web/Mvc/HtmlHelperExtensionMethodsTests.cs index a301dfbd04..1a4220c83b 100644 --- a/src/Umbraco.Tests/Web/Mvc/HtmlHelperExtensionMethodsTests.cs +++ b/src/Umbraco.Tests/Web/Mvc/HtmlHelperExtensionMethodsTests.cs @@ -4,6 +4,7 @@ using Umbraco.Web; namespace Umbraco.Tests.Web.Mvc { + [TestFixture] public class HtmlHelperExtensionMethodsTests { diff --git a/src/Umbraco.Tests/Web/Mvc/ValidateUmbracoFormRouteStringAttributeTests.cs b/src/Umbraco.Tests/Web/Mvc/ValidateUmbracoFormRouteStringAttributeTests.cs new file mode 100644 index 0000000000..d4c3b7c887 --- /dev/null +++ b/src/Umbraco.Tests/Web/Mvc/ValidateUmbracoFormRouteStringAttributeTests.cs @@ -0,0 +1,37 @@ +using NUnit.Framework; +using Umbraco.Web; +using Umbraco.Web.Mvc; + +namespace Umbraco.Tests.Web.Mvc +{ + [TestFixture] + public class ValidateUmbracoFormRouteStringAttributeTests + { + [Test] + public void Validate_Route_String() + { + var attribute = new ValidateUmbracoFormRouteStringAttribute(); + + Assert.Throws(() => attribute.ValidateRouteString(null, null, null, null)); + + const string ControllerName = "Test"; + const string ControllerAction = "Index"; + const string Area = "MyArea"; + var validUfprt = UrlHelperRenderExtensions.CreateEncryptedRouteString(ControllerName, ControllerAction, Area); + + var invalidUfprt = validUfprt + "z"; + Assert.Throws(() => attribute.ValidateRouteString(invalidUfprt, null, null, null)); + + Assert.Throws(() => attribute.ValidateRouteString(validUfprt, ControllerName, ControllerAction, "doesntMatch")); + Assert.Throws(() => attribute.ValidateRouteString(validUfprt, ControllerName, ControllerAction, null)); + Assert.Throws(() => attribute.ValidateRouteString(validUfprt, ControllerName, "doesntMatch", Area)); + Assert.Throws(() => attribute.ValidateRouteString(validUfprt, ControllerName, null, Area)); + Assert.Throws(() => attribute.ValidateRouteString(validUfprt, "doesntMatch", ControllerAction, Area)); + Assert.Throws(() => attribute.ValidateRouteString(validUfprt, null, ControllerAction, Area)); + + Assert.DoesNotThrow(() => attribute.ValidateRouteString(validUfprt, ControllerName, ControllerAction, Area)); + Assert.DoesNotThrow(() => attribute.ValidateRouteString(validUfprt, ControllerName.ToLowerInvariant(), ControllerAction.ToLowerInvariant(), Area.ToLowerInvariant())); + } + + } +} diff --git a/src/Umbraco.Web/Mvc/HttpUmbracoFormRouteStringException.cs b/src/Umbraco.Web/Mvc/HttpUmbracoFormRouteStringException.cs index 8e0b50b11c..b08fde081a 100644 --- a/src/Umbraco.Web/Mvc/HttpUmbracoFormRouteStringException.cs +++ b/src/Umbraco.Web/Mvc/HttpUmbracoFormRouteStringException.cs @@ -1,23 +1,17 @@ using System; +using System.Net; using System.Runtime.Serialization; using System.Web; namespace Umbraco.Web.Mvc { /// - /// Describes an exception that occurred during the processing of an Umbraco form route string. + /// Exception that occurs when an Umbraco form route string is invalid /// /// [Serializable] public sealed class HttpUmbracoFormRouteStringException : HttpException { - /// - /// Initializes a new instance of the class. - /// - public HttpUmbracoFormRouteStringException() - { } - - /// /// Initializes a new instance of the class. /// /// The that holds the serialized object data about the exception being thrown. @@ -42,5 +36,6 @@ namespace Umbraco.Web.Mvc public HttpUmbracoFormRouteStringException(string message, Exception innerException) : base(message, innerException) { } + } } diff --git a/src/Umbraco.Web/Mvc/ValidateUmbracoFormRouteStringAttribute.cs b/src/Umbraco.Web/Mvc/ValidateUmbracoFormRouteStringAttribute.cs index 2be89f05a6..8d929197e1 100644 --- a/src/Umbraco.Web/Mvc/ValidateUmbracoFormRouteStringAttribute.cs +++ b/src/Umbraco.Web/Mvc/ValidateUmbracoFormRouteStringAttribute.cs @@ -1,14 +1,21 @@ using System; +using System.Net; +using System.Net.Http; using System.Web.Mvc; using Umbraco.Core; namespace Umbraco.Web.Mvc { /// - /// Represents an attribute that is used to prevent an invalid Umbraco form request route string on a request. + /// Attribute used to check that the request contains a valid Umbraco form request string. /// /// /// + /// + /// Applying this attribute/filter to a or SurfaceController Action will ensure that the Action can only be executed + /// when it is routed to from within Umbraco, typically when rendering a form with BegingUmbracoForm. It will mean that the natural MVC route for this Action + /// will fail with a . + /// [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] public sealed class ValidateUmbracoFormRouteStringAttribute : FilterAttribute, IAuthorizationFilter { @@ -25,11 +32,14 @@ namespace Umbraco.Web.Mvc public void OnAuthorization(AuthorizationContext filterContext) { if (filterContext == null) - { throw new ArgumentNullException(nameof(filterContext)); - } var ufprt = filterContext.HttpContext.Request["ufprt"]; + ValidateRouteString(ufprt, filterContext.ActionDescriptor?.ControllerDescriptor.ControllerName, filterContext.ActionDescriptor?.ActionName, filterContext.RouteData?.DataTokens["area"]?.ToString()); + } + + public void ValidateRouteString(string ufprt, string currentController, string currentAction, string currentArea) + { if (ufprt.IsNullOrWhiteSpace()) { throw new HttpUmbracoFormRouteStringException("The required request field \"ufprt\" is not present."); @@ -40,12 +50,13 @@ namespace Umbraco.Web.Mvc throw new HttpUmbracoFormRouteStringException("The Umbraco form request route string could not be decrypted."); } - if (additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Controller] != filterContext.ActionDescriptor.ControllerDescriptor.ControllerName || - additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Action] != filterContext.ActionDescriptor.ActionName || - additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Area].NullOrWhiteSpaceAsNull() != filterContext.RouteData.DataTokens["area"]?.ToString().NullOrWhiteSpaceAsNull()) + if (!additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Controller].InvariantEquals(currentController) || + !additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Action].InvariantEquals(currentAction) || + (!additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Area].IsNullOrWhiteSpace() && !additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Area].InvariantEquals(currentArea))) { throw new HttpUmbracoFormRouteStringException("The provided Umbraco form request route string was meant for a different controller and action."); } + } } } diff --git a/src/Umbraco.Web/UmbracoHelper.cs b/src/Umbraco.Web/UmbracoHelper.cs index bf017c73cc..367d90a504 100644 --- a/src/Umbraco.Web/UmbracoHelper.cs +++ b/src/Umbraco.Web/UmbracoHelper.cs @@ -820,7 +820,7 @@ namespace Umbraco.Web { decryptedString = ufprt.DecryptWithMachineKey(); } - catch (FormatException) + catch (Exception ex) when (ex is FormatException || ex is ArgumentException) { Current.Logger.Warn(typeof(UmbracoHelper), "A value was detected in the ufprt parameter but Umbraco could not decrypt the string"); parts = null;