diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 72939c12d9..d45c556d37 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -398,6 +398,7 @@ + diff --git a/src/Umbraco.Tests/Web/Mvc/HtmlHelperExtensionMethodsTests.cs b/src/Umbraco.Tests/Web/Mvc/HtmlHelperExtensionMethodsTests.cs index e7e4f363c1..7be0717c7e 100644 --- a/src/Umbraco.Tests/Web/Mvc/HtmlHelperExtensionMethodsTests.cs +++ b/src/Umbraco.Tests/Web/Mvc/HtmlHelperExtensionMethodsTests.cs @@ -4,7 +4,8 @@ using Umbraco.Web; namespace Umbraco.Tests.Web.Mvc { - [TestFixture] + + [TestFixture] public class HtmlHelperExtensionMethodsTests { [SetUp] diff --git a/src/Umbraco.Tests/Web/Mvc/ValidateUmbracoFormRouteStringAttributeTests.cs b/src/Umbraco.Tests/Web/Mvc/ValidateUmbracoFormRouteStringAttributeTests.cs new file mode 100644 index 0000000000..a942f846a0 --- /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 = UmbracoHelper.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 d4734e5d24..b08fde081a 100644 --- a/src/Umbraco.Web/Mvc/HttpUmbracoFormRouteStringException.cs +++ b/src/Umbraco.Web/Mvc/HttpUmbracoFormRouteStringException.cs @@ -1,5 +1,6 @@ using System; using System.Net; +using System.Runtime.Serialization; using System.Web; namespace Umbraco.Web.Mvc @@ -11,6 +12,14 @@ namespace Umbraco.Web.Mvc [Serializable] public sealed class HttpUmbracoFormRouteStringException : HttpException { + /// Initializes a new instance of the class. + /// + /// The that holds the serialized object data about the exception being thrown. + /// The that holds the contextual information about the source or destination. + private HttpUmbracoFormRouteStringException(SerializationInfo info, StreamingContext context) + : base(info, context) + { } + /// /// Initializes a new instance of the class. /// @@ -19,5 +28,14 @@ namespace Umbraco.Web.Mvc : base(message) { } + /// + /// Initializes a new instance of the class. + /// + /// The error message displayed to the client when the exception is thrown. + /// The , if any, that threw the current exception. + 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 f3d0cc0e27..8d929197e1 100644 --- a/src/Umbraco.Web/Mvc/ValidateUmbracoFormRouteStringAttribute.cs +++ b/src/Umbraco.Web/Mvc/ValidateUmbracoFormRouteStringAttribute.cs @@ -1,15 +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 { @@ -26,27 +32,31 @@ 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 Umbraco request data is invalid."); + throw new HttpUmbracoFormRouteStringException("The required request field \"ufprt\" is not present."); } if (!UmbracoHelper.DecryptAndValidateEncryptedRouteString(ufprt, out var additionalDataParts)) { - throw new HttpUmbracoFormRouteStringException("The required Umbraco request data is invalid."); + throw new HttpUmbracoFormRouteStringException("The Umbraco form request route string could not be decrypted."); } - if (!additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Controller].InvariantEquals(filterContext.ActionDescriptor.ControllerDescriptor.ControllerName) || - !additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Action].InvariantEquals(filterContext.ActionDescriptor.ActionName) || - (!additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Area].IsNullOrWhiteSpace() && !additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Area].InvariantEquals(filterContext.RouteData.DataTokens["area"]?.ToString()))) + 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 required Umbraco request data is invalid."); + 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 c00d37f216..5320b6085a 100644 --- a/src/Umbraco.Web/UmbracoHelper.cs +++ b/src/Umbraco.Web/UmbracoHelper.cs @@ -1656,7 +1656,7 @@ namespace Umbraco.Web { decryptedString = ufprt.DecryptWithMachineKey(); } - catch (FormatException) + catch (Exception ex) when (ex is FormatException || ex is ArgumentException) { LogHelper.Warn("A value was detected in the ufprt parameter but Umbraco could not decrypt the string"); parts = null;