Merge pull request #5728 from umbraco/v8/feature/AB-1085

V8: https://umbraco.visualstudio.com/D-Team%20Tracker/_workitems/edit/108…

src/Umbraco.Tests/Security/UmbracoAntiForgeryAdditionalDataProviderTests.cs

@@ -0,0 +1,157 @@
+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);
+        }
+    }
+}

src/Umbraco.Web/HtmlHelperRenderExtensions.cs

@@ -223,6 +223,13 @@ namespace Umbraco.Web
                 _method = method;
 			    _controllerName = controllerName;
                 _encryptedString = UrlHelperRenderExtensions.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;
             }
 
             private readonly ViewContext _viewContext;

src/Umbraco.Web/Mvc/RenderRouteHandler.cs

@@ -19,7 +19,7 @@ namespace Umbraco.Web.Mvc
     public class RenderRouteHandler : IRouteHandler
     {
         // Define reserved dictionary keys for controller, action and area specified in route additional values data
-        private static class ReservedAdditionalKeys
+        internal static class ReservedAdditionalKeys
         {
             internal const string Controller = "c";
             internal const string Action = "a";
@@ -134,36 +134,7 @@ namespace Umbraco.Web.Mvc
                     return null;
             }
 
-
-            string decryptedString;
-            try
-            {
-                decryptedString = encodedVal.DecryptWithMachineKey();
-            }
-            catch (FormatException)
-            {
-                Current.Logger.Warn<RenderRouteHandler>("A value was detected in the ufprt parameter but Umbraco could not decrypt the string");
-                return null;
-            }
-
-            var parsedQueryString = HttpUtility.ParseQueryString(decryptedString);
-            var decodedParts = new Dictionary<string, string>();
-
-            foreach (var key in parsedQueryString.AllKeys)
-            {
-                decodedParts[key] = parsedQueryString[key];
-            }
-
-            //validate all required keys exist
-
-            //the controller
-            if (decodedParts.All(x => x.Key != ReservedAdditionalKeys.Controller))
-                return null;
-            //the action
-            if (decodedParts.All(x => x.Key != ReservedAdditionalKeys.Action))
-                return null;
-            //the area
-            if (decodedParts.All(x => x.Key != ReservedAdditionalKeys.Area))
+            if (!UmbracoHelper.DecryptAndValidateEncryptedRouteString(encodedVal, out var decodedParts))
                 return null;
 
             foreach (var item in decodedParts.Where(x => new[] {
@@ -417,7 +388,7 @@ namespace Umbraco.Web.Mvc
 
             return new UmbracoMvcHandler(requestContext);
         }
-        
+
         private SessionStateBehavior GetSessionStateBehavior(RequestContext requestContext, string controllerName)
         {
             return _controllerFactory.GetControllerSessionBehavior(requestContext, controllerName);

src/Umbraco.Web/Runtime/WebFinalComponent.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Linq;
+using System.Web.Helpers;
 using System.Web.Http;
 using System.Web.Mvc;
 using System.Web.Routing;
@@ -8,6 +9,7 @@ using Umbraco.Core.Composing;
 using Umbraco.Core.Configuration;
 using Umbraco.Web.Install;
 using Umbraco.Web.Mvc;
+using Umbraco.Web.Security;
 using Umbraco.Web.WebApi;
 
 namespace Umbraco.Web.Runtime
@@ -34,6 +36,8 @@ namespace Umbraco.Web.Runtime
 
             // ensure WebAPI is initialized, after everything
             GlobalConfiguration.Configuration.EnsureInitialized();
+
+            AntiForgeryConfig.AdditionalDataProvider = new UmbracoAntiForgeryAdditionalDataProvider(AntiForgeryConfig.AdditionalDataProvider);
         }
 
         public void Terminate()

src/Umbraco.Web/Security/UmbracoAntiForgeryAdditionalDataProvider.cs

@@ -0,0 +1,92 @@
+using System;
+using Umbraco.Web.Mvc;
+using Umbraco.Core;
+using System.Web.Helpers;
+using System.Web;
+using Newtonsoft.Json;
+using Umbraco.Web.Composing;
+
+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; }
+        }
+
+    }
+}

src/Umbraco.Web/UmbracoHelper.cs

@@ -8,7 +8,10 @@ using Umbraco.Core.Dictionary;
 using Umbraco.Core.Models;
 using Umbraco.Core.Models.PublishedContent;
 using Umbraco.Core.Xml;
+using Umbraco.Web.Composing;
+using Umbraco.Web.Mvc;
 using Umbraco.Web.Security;
+using Constants = Umbraco.Core.Constants;
 
 namespace Umbraco.Web
 {
@@ -228,7 +231,7 @@ namespace Umbraco.Web
 
         #endregion
 
-       
+
 
         #region Member/Content/Media from Udi
 
@@ -495,7 +498,7 @@ namespace Umbraco.Web
         /// <returns>The existing contents corresponding to the identifiers.</returns>
         /// <remarks>If an identifier does not match an existing content, it will be missing in the returned value.</remarks>
         public IEnumerable<IPublishedContent> Content(IEnumerable<Udi> ids)
-        {          
+        {
             return ids.Select(id => ContentQuery.Content(id)).WhereNotNull();
         }
 
@@ -810,10 +813,42 @@ namespace Umbraco.Web
 
         #endregion
 
-       
 
-        
+        internal static bool DecryptAndValidateEncryptedRouteString(string ufprt, out IDictionary<string, string> parts)
+        {
+            string decryptedString;
+            try
+            {
+                decryptedString = ufprt.DecryptWithMachineKey();
+            }
+            catch (FormatException)
+            {
+                Current.Logger.Warn(typeof(UmbracoHelper), "A value was detected in the ufprt parameter but Umbraco could not decrypt the string");
+                parts = null;
+                return false;
+            }
 
-        
+            var parsedQueryString = HttpUtility.ParseQueryString(decryptedString);
+            parts = new Dictionary<string, string>();
+
+            foreach (var key in parsedQueryString.AllKeys)
+            {
+                parts[key] = parsedQueryString[key];
+            }
+
+            //validate all required keys exist
+
+            //the controller
+            if (parts.All(x => x.Key != RenderRouteHandler.ReservedAdditionalKeys.Controller))
+                return false;
+            //the action
+            if (parts.All(x => x.Key != RenderRouteHandler.ReservedAdditionalKeys.Action))
+                return false;
+            //the area
+            if (parts.All(x => x.Key != RenderRouteHandler.ReservedAdditionalKeys.Area))
+                return false;
+
+            return true;
+        }
     }
 }