yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Cherry picks ValidateUmbracoFormRouteStringAttribute implementation and fixes up some logic

Browse Source
This commit is contained in:
Shannon
2019-07-16 23:03:26 +10:00
parent 2aaca865e7
commit d52420183e
12 changed files with 81 additions and 259 deletions
Download Patch File Download Diff File Expand all files Collapse all files

157
src/Umbraco.Tests/Security/UmbracoAntiForgeryAdditionalDataProviderTests.cs
View File

@@ -1,157 +0,0 @@
using System.Collections.Specialized;
using System.Web;
using System.Web.Helpers;
using Moq;
using Newtonsoft.Json;
using NUnit.Framework;
using Umbraco.Core;
using Umbraco.Tests.TestHelpers;
using Umbraco.Web.Mvc;
using Umbraco.Web.Security;
namespace Umbraco.Tests.Security
{
[TestFixture]
public class UmbracoAntiForgeryAdditionalDataProviderTests
{
[Test]
public void Test_Wrapped_Non_BeginUmbracoForm()
{
var wrapped = Mock.Of<IAntiForgeryAdditionalDataProvider>(x => x.GetAdditionalData(It.IsAny<HttpContextBase>()) == "custom");
var provider = new UmbracoAntiForgeryAdditionalDataProvider(wrapped);
var httpContextFactory = new FakeHttpContextFactory("/hello/world");
var data = provider.GetAdditionalData(httpContextFactory.HttpContext);
Assert.IsTrue(data.DetectIsJson());
var json = JsonConvert.DeserializeObject<UmbracoAntiForgeryAdditionalDataProvider.AdditionalData>(data);
Assert.AreEqual(null, json.Ufprt);
Assert.IsTrue(json.Stamp != default);
Assert.AreEqual("custom", json.WrappedValue);
}
[Test]
public void Null_Wrapped_Non_BeginUmbracoForm()
{
var provider = new UmbracoAntiForgeryAdditionalDataProvider(null);
var httpContextFactory = new FakeHttpContextFactory("/hello/world");
var data = provider.GetAdditionalData(httpContextFactory.HttpContext);
Assert.IsTrue(data.DetectIsJson());
var json = JsonConvert.DeserializeObject<UmbracoAntiForgeryAdditionalDataProvider.AdditionalData>(data);
Assert.AreEqual(null, json.Ufprt);
Assert.IsTrue(json.Stamp != default);
Assert.AreEqual("default", json.WrappedValue);
}
[Test]
public void Validate_Non_Json()
{
var provider = new UmbracoAntiForgeryAdditionalDataProvider(null);
var httpContextFactory = new FakeHttpContextFactory("/hello/world");
var isValid = provider.ValidateAdditionalData(httpContextFactory.HttpContext, "hello");
Assert.IsFalse(isValid);
}
[Test]
public void Validate_Invalid_Json()
{
var provider = new UmbracoAntiForgeryAdditionalDataProvider(null);
var httpContextFactory = new FakeHttpContextFactory("/hello/world");
var isValid = provider.ValidateAdditionalData(httpContextFactory.HttpContext, "{'Stamp': '0'}");
Assert.IsFalse(isValid);
isValid = provider.ValidateAdditionalData(httpContextFactory.HttpContext, "{'Stamp': ''}");
Assert.IsFalse(isValid);
isValid = provider.ValidateAdditionalData(httpContextFactory.HttpContext, "{'hello': 'world'}");
Assert.IsFalse(isValid);
}
[Test]
public void Validate_No_Request_Ufprt()
{
var provider = new UmbracoAntiForgeryAdditionalDataProvider(null);
var httpContextFactory = new FakeHttpContextFactory("/hello/world");
//there is a ufprt in the additional data, but not in the request
var isValid = provider.ValidateAdditionalData(httpContextFactory.HttpContext, "{'Stamp': '636970328040070330', 'WrappedValue': 'default', 'Ufprt': 'ASBVDFDFDFDF'}");
Assert.IsFalse(isValid);
}
[Test]
public void Validate_No_AdditionalData_Ufprt()
{
var provider = new UmbracoAntiForgeryAdditionalDataProvider(null);
var httpContextFactory = new FakeHttpContextFactory("/hello/world");
var requestMock = Mock.Get(httpContextFactory.HttpContext.Request);
requestMock.SetupGet(x => x["ufprt"]).Returns("ABCDEFG");
//there is a ufprt in the additional data, but not in the request
var isValid = provider.ValidateAdditionalData(httpContextFactory.HttpContext, "{'Stamp': '636970328040070330', 'WrappedValue': 'default', 'Ufprt': ''}");
Assert.IsFalse(isValid);
}
[Test]
public void Validate_No_AdditionalData_Or_Request_Ufprt()
{
var provider = new UmbracoAntiForgeryAdditionalDataProvider(null);
var httpContextFactory = new FakeHttpContextFactory("/hello/world");
//there is a ufprt in the additional data, but not in the request
var isValid = provider.ValidateAdditionalData(httpContextFactory.HttpContext, "{'Stamp': '636970328040070330', 'WrappedValue': 'default', 'Ufprt': ''}");
Assert.IsTrue(isValid);
}
[Test]
public void Validate_Request_And_AdditionalData_Ufprt()
{
var provider = new UmbracoAntiForgeryAdditionalDataProvider(null);
var routeParams1 = $"{RenderRouteHandler.ReservedAdditionalKeys.Controller}={HttpUtility.UrlEncode("Test")}&{RenderRouteHandler.ReservedAdditionalKeys.Action}={HttpUtility.UrlEncode("Index")}&{RenderRouteHandler.ReservedAdditionalKeys.Area}=Umbraco";
var routeParams2 = $"{RenderRouteHandler.ReservedAdditionalKeys.Controller}={HttpUtility.UrlEncode("Test")}&{RenderRouteHandler.ReservedAdditionalKeys.Action}={HttpUtility.UrlEncode("Index")}&{RenderRouteHandler.ReservedAdditionalKeys.Area}=Umbraco";
var httpContextFactory = new FakeHttpContextFactory("/hello/world");
var requestMock = Mock.Get(httpContextFactory.HttpContext.Request);
requestMock.SetupGet(x => x["ufprt"]).Returns(routeParams1.EncryptWithMachineKey());
var isValid = provider.ValidateAdditionalData(httpContextFactory.HttpContext, "{'Stamp': '636970328040070330', 'WrappedValue': 'default', 'Ufprt': '" + routeParams2.EncryptWithMachineKey() + "'}");
Assert.IsTrue(isValid);
routeParams2 = $"{RenderRouteHandler.ReservedAdditionalKeys.Controller}={HttpUtility.UrlEncode("Invalid")}&{RenderRouteHandler.ReservedAdditionalKeys.Action}={HttpUtility.UrlEncode("Index")}&{RenderRouteHandler.ReservedAdditionalKeys.Area}=Umbraco";
isValid = provider.ValidateAdditionalData(httpContextFactory.HttpContext, "{'Stamp': '636970328040070330', 'WrappedValue': 'default', 'Ufprt': '" + routeParams2.EncryptWithMachineKey() + "'}");
Assert.IsFalse(isValid);
}
[Test]
public void Validate_Wrapped_Request_And_AdditionalData_Ufprt()
{
var wrapped = Mock.Of<IAntiForgeryAdditionalDataProvider>(x => x.ValidateAdditionalData(It.IsAny<HttpContextBase>(), "custom") == true);
var provider = new UmbracoAntiForgeryAdditionalDataProvider(wrapped);
var routeParams1 = $"{RenderRouteHandler.ReservedAdditionalKeys.Controller}={HttpUtility.UrlEncode("Test")}&{RenderRouteHandler.ReservedAdditionalKeys.Action}={HttpUtility.UrlEncode("Index")}&{RenderRouteHandler.ReservedAdditionalKeys.Area}=Umbraco";
var routeParams2 = $"{RenderRouteHandler.ReservedAdditionalKeys.Controller}={HttpUtility.UrlEncode("Test")}&{RenderRouteHandler.ReservedAdditionalKeys.Action}={HttpUtility.UrlEncode("Index")}&{RenderRouteHandler.ReservedAdditionalKeys.Area}=Umbraco";
var httpContextFactory = new FakeHttpContextFactory("/hello/world");
var requestMock = Mock.Get(httpContextFactory.HttpContext.Request);
requestMock.SetupGet(x => x["ufprt"]).Returns(routeParams1.EncryptWithMachineKey());
var isValid = provider.ValidateAdditionalData(httpContextFactory.HttpContext, "{'Stamp': '636970328040070330', 'WrappedValue': 'default', 'Ufprt': '" + routeParams2.EncryptWithMachineKey() + "'}");
Assert.IsFalse(isValid);
isValid = provider.ValidateAdditionalData(httpContextFactory.HttpContext, "{'Stamp': '636970328040070330', 'WrappedValue': 'custom', 'Ufprt': '" + routeParams2.EncryptWithMachineKey() + "'}");
Assert.IsTrue(isValid);
routeParams2 = $"{RenderRouteHandler.ReservedAdditionalKeys.Controller}={HttpUtility.UrlEncode("Invalid")}&{RenderRouteHandler.ReservedAdditionalKeys.Action}={HttpUtility.UrlEncode("Index")}&{RenderRouteHandler.ReservedAdditionalKeys.Area}=Umbraco";
isValid = provider.ValidateAdditionalData(httpContextFactory.HttpContext, "{'Stamp': '636970328040070330', 'WrappedValue': 'default', 'Ufprt': '" + routeParams2.EncryptWithMachineKey() + "'}");
Assert.IsFalse(isValid);
}
}
}

1
src/Umbraco.Tests/Umbraco.Tests.csproj
View File

@@ -197,7 +197,6 @@
<Compile Include="Packaging\PackageExtractionTests.cs" />
<Compile Include="Persistence\Repositories\SimilarNodeNameTests.cs" />
<Compile Include="PublishedContent\StronglyTypedModels\Home.cs" />
<Compile Include="Security\UmbracoAntiForgeryAdditionalDataProviderTests.cs" />
<Compile Include="Services\AuditServiceTests.cs" />
<Compile Include="Services\ConsentServiceTests.cs" />
<Compile Include="Services\MemberGroupServiceTests.cs" />

1
src/Umbraco.Web/Controllers/UmbLoginController.cs
View File

@@ -12,6 +12,7 @@ namespace Umbraco.Web.Controllers
{
[HttpPost]
[ValidateAntiForgeryToken]
[ValidateUmbracoFormRouteString]
public ActionResult HandleLogin([Bind(Prefix = "loginModel")]LoginModel model)
{
if (ModelState.IsValid == false)

1
src/Umbraco.Web/Controllers/UmbLoginStatusController.cs
View File

@@ -13,6 +13,7 @@ namespace Umbraco.Web.Controllers
{
[HttpPost]
[ValidateAntiForgeryToken]
[ValidateUmbracoFormRouteString]
public ActionResult HandleLogout([Bind(Prefix = "logoutModel")]PostRedirectModel model)
{
if (ModelState.IsValid == false)

1
src/Umbraco.Web/Controllers/UmbProfileController.cs
View File

@@ -16,6 +16,7 @@ namespace Umbraco.Web.Controllers
{
[HttpPost]
[ValidateAntiForgeryToken]
[ValidateUmbracoFormRouteString]
public ActionResult HandleUpdateProfile([Bind(Prefix = "profileModel")] ProfileModel model)
{
var provider = global::Umbraco.Core.Security.MembershipProviderExtensions.GetMembersMembershipProvider();

1
src/Umbraco.Web/Controllers/UmbRegisterController.cs
View File

@@ -11,6 +11,7 @@ namespace Umbraco.Web.Controllers
{
[HttpPost]
[ValidateAntiForgeryToken]
[ValidateUmbracoFormRouteString]
public ActionResult HandleRegisterMember([Bind(Prefix = "registerModel")]RegisterModel model)
{
if (ModelState.IsValid == false)

7
src/Umbraco.Web/HtmlHelperRenderExtensions.cs
View File

@@ -293,13 +293,6 @@ namespace Umbraco.Web
_controllerName = controllerName;
_encryptedString = UmbracoHelper.CreateEncryptedRouteString(controllerName, controllerAction, area, additionalRouteVals);
//For UmbracoForm's we want to add our routing string to the httpcontext items in the case where anti-forgery tokens are used.
//In which case our custom UmbracoAntiForgeryAdditionalDataProvider will kick in and validate the values in the request against
//the values that will be appended to the token. This essentially means that when anti-forgery tokens are used with UmbracoForm's forms,
//that each token is unique to the controller/action/area instead of the default ASP.Net implementation which is that the token is unique
//per user.
_viewContext.HttpContext.Items["ufprt"] = _encryptedString;
}

23
src/Umbraco.Web/Mvc/HttpUmbracoFormRouteStringException.cs Normal file
View File

@@ -0,0 +1,23 @@
using System;
using System.Net;
using System.Web;
namespace Umbraco.Web.Mvc
{
/// <summary>
/// Exception that occurs when an Umbraco form route string is invalid
/// </summary>
/// <seealso cref="System.Web.HttpException" />
[Serializable]
public sealed class HttpUmbracoFormRouteStringException : HttpException
{
/// <summary>
/// Initializes a new instance of the <see cref="HttpUmbracoFormRouteStringException" /> class.
/// </summary>
/// <param name="message">The error message displayed to the client when the exception is thrown.</param>
public HttpUmbracoFormRouteStringException(string message)
: base(message)
{ }
}
}

52
src/Umbraco.Web/Mvc/ValidateUmbracoFormRouteStringAttribute.cs Normal file
View File

@@ -0,0 +1,52 @@
using System;
using System.Net;
using System.Web.Mvc;
using Umbraco.Core;
namespace Umbraco.Web.Mvc
{
/// <summary>
/// Represents an attribute that is used to prevent an invalid Umbraco form request route string on a request.
/// </summary>
/// <seealso cref="System.Web.Mvc.FilterAttribute" />
/// <seealso cref="System.Web.Mvc.IAuthorizationFilter" />
[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)]
public sealed class ValidateUmbracoFormRouteStringAttribute : FilterAttribute, IAuthorizationFilter
{
/// <summary>
/// Called when authorization is required.
/// </summary>
/// <param name="filterContext">The filter context.</param>
/// <exception cref="ArgumentNullException">filterContext</exception>
/// <exception cref="Umbraco.Web.Mvc.HttpUmbracoFormRouteStringException">The required request field \"ufprt\" is not present.
/// or
/// The Umbraco form request route string could not be decrypted.
/// or
/// The provided Umbraco form request route string was meant for a different controller and action.</exception>
public void OnAuthorization(AuthorizationContext filterContext)
{
if (filterContext == null)
{
throw new ArgumentNullException(nameof(filterContext));
}
var ufprt = filterContext.HttpContext.Request["ufprt"];
if (ufprt.IsNullOrWhiteSpace())
{
throw new HttpUmbracoFormRouteStringException("The required Umbraco request data is invalid.");
}
if (!UmbracoHelper.DecryptAndValidateEncryptedRouteString(ufprt, out var additionalDataParts))
{
throw new HttpUmbracoFormRouteStringException("The required Umbraco request data is invalid.");
}
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())))
{
throw new HttpUmbracoFormRouteStringException("The required Umbraco request data is invalid.");
}
}
}
}

91
src/Umbraco.Web/Security/UmbracoAntiForgeryAdditionalDataProvider.cs
View File

@@ -1,91 +0,0 @@
using System;
using Umbraco.Web.Mvc;
using Umbraco.Core;
using System.Web.Helpers;
using System.Web;
using Newtonsoft.Json;
namespace Umbraco.Web.Security
{
/// <summary>
/// A custom <see cref="IAntiForgeryAdditionalDataProvider"/> to create a unique antiforgery token/validator per form created with BeginUmbracoForm
/// </summary>
public class UmbracoAntiForgeryAdditionalDataProvider : IAntiForgeryAdditionalDataProvider
{
private readonly IAntiForgeryAdditionalDataProvider _defaultProvider;
/// <summary>
/// Constructor, allows wrapping a default provider
/// </summary>
/// <param name="defaultProvider"></param>
public UmbracoAntiForgeryAdditionalDataProvider(IAntiForgeryAdditionalDataProvider defaultProvider)
{
_defaultProvider = defaultProvider;
}
public string GetAdditionalData(HttpContextBase context)
{
return JsonConvert.SerializeObject(new AdditionalData
{
Stamp = DateTime.UtcNow.Ticks,
//this value will be here if this is a BeginUmbracoForms form
Ufprt = context.Items["ufprt"]?.ToString(),
//if there was a wrapped provider, add it's value to the json, else just a static value
WrappedValue = _defaultProvider?.GetAdditionalData(context) ?? "default"
});
}
public bool ValidateAdditionalData(HttpContextBase context, string additionalData)
{
if (!additionalData.DetectIsJson())
return false; //must be json
AdditionalData json;
try
{
json = JsonConvert.DeserializeObject<AdditionalData>(additionalData);
}
catch
{
return false; //couldn't parse
}
if (json.Stamp == default) return false;
//if there was a wrapped provider, validate it, else validate the static value
var validateWrapped = _defaultProvider?.ValidateAdditionalData(context, json.WrappedValue) ?? json.WrappedValue == "default";
if (!validateWrapped)
return false;
var ufprtRequest = context.Request["ufprt"]?.ToString();
//if the custom BeginUmbracoForms route value is not there, then it's nothing more to validate
if (ufprtRequest.IsNullOrWhiteSpace() && json.Ufprt.IsNullOrWhiteSpace())
return true;
//if one or the other is null then something is wrong
if (!ufprtRequest.IsNullOrWhiteSpace() && json.Ufprt.IsNullOrWhiteSpace()) return false;
if (ufprtRequest.IsNullOrWhiteSpace() && !json.Ufprt.IsNullOrWhiteSpace()) return false;
if (!UmbracoHelper.DecryptAndValidateEncryptedRouteString(json.Ufprt, out var additionalDataParts))
return false;
if (!UmbracoHelper.DecryptAndValidateEncryptedRouteString(ufprtRequest, out var requestParts))
return false;
//ensure they all match
return additionalDataParts.Count == requestParts.Count
&& additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Controller] == requestParts[RenderRouteHandler.ReservedAdditionalKeys.Controller]
&& additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Action] == requestParts[RenderRouteHandler.ReservedAdditionalKeys.Action]
&& additionalDataParts[RenderRouteHandler.ReservedAdditionalKeys.Area] == requestParts[RenderRouteHandler.ReservedAdditionalKeys.Area];
}
internal class AdditionalData
{
public string Ufprt { get; set; }
public long Stamp { get; set; }
public string WrappedValue { get; set; }
}
}
}

3
src/Umbraco.Web/Umbraco.Web.csproj
View File

@@ -307,7 +307,8 @@
<Compile Include="Cache\TemplateCacheRefresher.cs" />
<Compile Include="Cache\UnpublishedPageCacheRefresher.cs" />
<Compile Include="Cache\UserCacheRefresher.cs" />
<Compile Include="Security\UmbracoAntiForgeryAdditionalDataProvider.cs" />
<Compile Include="Mvc\HttpUmbracoFormRouteStringException.cs" />
<Compile Include="Mvc\ValidateUmbracoFormRouteStringAttribute.cs" />
<Compile Include="Editors\PreviewController.cs" />
<Compile Include="Editors\BackOfficeAssetsController.cs" />
<Compile Include="Features\DisabledFeatures.cs" />

2
src/Umbraco.Web/WebBootManager.cs
View File

@@ -190,8 +190,6 @@ namespace Umbraco.Web
base.Complete(afterComplete);
AntiForgeryConfig.AdditionalDataProvider = new UmbracoAntiForgeryAdditionalDataProvider(AntiForgeryConfig.AdditionalDataProvider);
//Now, startup all of our legacy startup handler
ApplicationEventsResolver.Current.InstantiateLegacyStartupHandlers();