From c681b4a835f0a6c73a27194ea12e62c74a025415 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 16 May 2020 09:41:54 +0200 Subject: [PATCH 1/5] Migrated HttpQueryStringModelBinder into netcore. --- .../ContentModelBinderTests.cs | 5 +- .../HttpQueryStringModelBinderTests.cs | 90 +++++++++++++++++++ .../Extensions/HttpRequestExtensions.cs | 12 +++ .../HttpQueryStringModelBinder.cs | 51 +++++++++++ .../Filters/HttpQueryStringModelBinder.cs | 1 + 5 files changed, 155 insertions(+), 4 deletions(-) rename src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/{ => ModelBinders}/ContentModelBinderTests.cs (97%) create mode 100644 src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/HttpQueryStringModelBinderTests.cs create mode 100644 src/Umbraco.Web.Common/Extensions/HttpRequestExtensions.cs create mode 100644 src/Umbraco.Web.Common/ModelBinders/HttpQueryStringModelBinder.cs diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ContentModelBinderTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs similarity index 97% rename from src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ContentModelBinderTests.cs rename to src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs index 99a3ea4fe5..0ad8e2c421 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ContentModelBinderTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/ContentModelBinderTests.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Abstractions; @@ -12,7 +11,7 @@ using Umbraco.Core.Models.PublishedContent; using Umbraco.Web.Common.ModelBinders; using Umbraco.Web.Models; -namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common +namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.ModelBinders { [TestFixture] public class ContentModelBinderTests @@ -92,9 +91,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common var httpContext = new DefaultHttpContext(); var routeData = new RouteData(); if (withUmbracoDataToken) - { routeData.DataTokens.Add(Constants.Web.UmbracoDataToken, source); - } var actionContext = new ActionContext(httpContext, routeData, new ActionDescriptor()); var metadataProvider = new EmptyModelMetadataProvider(); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/HttpQueryStringModelBinderTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/HttpQueryStringModelBinderTests.cs new file mode 100644 index 0000000000..6dd3b024b3 --- /dev/null +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.Common/ModelBinders/HttpQueryStringModelBinderTests.cs @@ -0,0 +1,90 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Primitives; +using NUnit.Framework; +using Umbraco.Web.Common.ModelBinders; + +namespace Umbraco.Tests.UnitTests.Umbraco.Web.Common.ModelBinders +{ + [TestFixture] + public class HttpQueryStringModelBinderTests + { + [Test] + public void Binds_Query_To_FormCollection() + { + // Arrange + var bindingContext = CreateBindingContext("?foo=bar&baz=buzz"); + var binder = new HttpQueryStringModelBinder(); + + // Act + binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + + var typedModel = bindingContext.Result.Model as FormCollection; + Assert.IsNotNull(typedModel); + Assert.AreEqual(typedModel["foo"], "bar"); + Assert.AreEqual(typedModel["baz"], "buzz"); + } + + [Test] + public void Sets_Culture_Form_Value_From_Query_If_Provided() + { + // Arrange + var bindingContext = CreateBindingContext("?foo=bar&baz=buzz&culture=en-gb"); + var binder = new HttpQueryStringModelBinder(); + + // Act + binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + + var typedModel = bindingContext.Result.Model as FormCollection; + Assert.IsNotNull(typedModel); + Assert.AreEqual(typedModel["culture"], "en-gb"); + } + + [Test] + public void Sets_Culture_Form_Value_From_Header_If_Not_Provided_In_Query() + { + // Arrange + var bindingContext = CreateBindingContext("?foo=bar&baz=buzz"); + var binder = new HttpQueryStringModelBinder(); + + // Act + binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + + var typedModel = bindingContext.Result.Model as FormCollection; + Assert.IsNotNull(typedModel); + Assert.AreEqual(typedModel["culture"], "en-gb"); + } + + private ModelBindingContext CreateBindingContext(string querystring) + { + var httpContext = new DefaultHttpContext(); + httpContext.Request.QueryString = new QueryString(querystring); + httpContext.Request.Headers.Add("X-UMB-CULTURE", new StringValues("en-gb")); + var routeData = new RouteData(); + var actionContext = new ActionContext(httpContext, routeData, new ActionDescriptor()); + var metadataProvider = new EmptyModelMetadataProvider(); + var routeValueDictionary = new RouteValueDictionary(); + var valueProvider = new RouteValueProvider(BindingSource.Path, routeValueDictionary); + var modelType = typeof(FormCollection); + return new DefaultModelBindingContext + { + ActionContext = actionContext, + ModelMetadata = metadataProvider.GetMetadataForType(modelType), + ModelName = modelType.Name, + ValueProvider = valueProvider, + }; + } + } +} diff --git a/src/Umbraco.Web.Common/Extensions/HttpRequestExtensions.cs b/src/Umbraco.Web.Common/Extensions/HttpRequestExtensions.cs new file mode 100644 index 0000000000..9dab445992 --- /dev/null +++ b/src/Umbraco.Web.Common/Extensions/HttpRequestExtensions.cs @@ -0,0 +1,12 @@ +using Microsoft.AspNetCore.Http; + +namespace Umbraco.Web.Common.Extensions +{ + public static class HttpRequestExtensions + { + internal static string ClientCulture(this HttpRequest request) + { + return request.Headers.TryGetValue("X-UMB-CULTURE", out var values) ? values[0] : null; + } + } +} diff --git a/src/Umbraco.Web.Common/ModelBinders/HttpQueryStringModelBinder.cs b/src/Umbraco.Web.Common/ModelBinders/HttpQueryStringModelBinder.cs new file mode 100644 index 0000000000..eb6a1ab7fb --- /dev/null +++ b/src/Umbraco.Web.Common/ModelBinders/HttpQueryStringModelBinder.cs @@ -0,0 +1,51 @@ +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.Primitives; +using Umbraco.Core; +using Umbraco.Web.Common.Extensions; + +namespace Umbraco.Web.Common.ModelBinders +{ + /// + /// Allows an Action to execute with an arbitrary number of QueryStrings + /// + /// + /// Just like you can POST an arbitrary number of parameters to an Action, you can't GET an arbitrary number + /// but this will allow you to do it. + /// + public sealed class HttpQueryStringModelBinder : IModelBinder + { + public Task BindModelAsync(ModelBindingContext bindingContext) + { + var queryStrings = GetQueryAsDictionary(bindingContext.ActionContext.HttpContext.Request.Query); + var queryStringKeys = queryStrings.Select(kvp => kvp.Key).ToArray(); + if (queryStringKeys.InvariantContains("culture") == false) + { + queryStrings.Add("culture", new StringValues(bindingContext.ActionContext.HttpContext.Request.ClientCulture())); + } + + var formData = new FormCollection(queryStrings); + bindingContext.Result = ModelBindingResult.Success(formData); + return Task.CompletedTask; + } + + private Dictionary GetQueryAsDictionary(IQueryCollection query) + { + var result = new Dictionary(); + if (query == null) + { + return result; + } + + foreach (var item in query) + { + result.Add(item.Key, item.Value); + } + + return result; + } + } +} diff --git a/src/Umbraco.Web/WebApi/Filters/HttpQueryStringModelBinder.cs b/src/Umbraco.Web/WebApi/Filters/HttpQueryStringModelBinder.cs index 4d58e2b512..3b8e4bea3f 100644 --- a/src/Umbraco.Web/WebApi/Filters/HttpQueryStringModelBinder.cs +++ b/src/Umbraco.Web/WebApi/Filters/HttpQueryStringModelBinder.cs @@ -14,6 +14,7 @@ namespace Umbraco.Web.WebApi.Filters /// Just like you can POST an arbitrary number of parameters to an Action, you can't GET an arbitrary number /// but this will allow you to do it /// + /// Migrated to .NET core public sealed class HttpQueryStringModelBinder : IModelBinder { public bool BindModel(HttpActionContext actionContext, ModelBindingContext bindingContext) From b2e12f400c904d77e6ca82f045ce986a4068459a Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 16 May 2020 19:17:08 +0200 Subject: [PATCH 2/5] Migrated OnlyLocalRequestsAttribute into netcore --- .../Umbraco.Tests.UnitTests.csproj | 1 + .../OnlyLocalRequestsAttributeTests.cs | 127 ++++++++++++++++++ .../Filters/OnlyLocalRequestsAttribute.cs | 18 +++ .../Extensions/HttpRequestExtensions.cs | 32 ++++- src/Umbraco.Web/Umbraco.Web.csproj | 2 +- .../Filters/OnlyLocalRequestsAttribute.cs | 1 + 6 files changed, 179 insertions(+), 2 deletions(-) create mode 100644 src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttributeTests.cs create mode 100644 src/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttribute.cs diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Tests.UnitTests.csproj b/src/Umbraco.Tests.UnitTests/Umbraco.Tests.UnitTests.csproj index c5732870f3..67d822fb03 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Tests.UnitTests.csproj +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Tests.UnitTests.csproj @@ -20,6 +20,7 @@ + diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttributeTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttributeTests.cs new file mode 100644 index 0000000000..f00713f6a4 --- /dev/null +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttributeTests.cs @@ -0,0 +1,127 @@ +using System; +using System.Collections.Generic; +using System.Net; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.Routing; +using Microsoft.AspNetCore.Routing; +using Moq; +using NUnit.Framework; +using Umbraco.Web.BackOffice.Filters; + +namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Filters +{ + [TestFixture] + public class OnlyLocalRequestsAttributeTests + { + [Test] + public void Does_Not_Set_Result_When_No_Remote_Address() + { + // Arrange + var context = CreateContext(); + var attribute = new OnlyLocalRequestsAttribute(); + + // Act + attribute.OnActionExecuting(context); + + // Assert + Assert.IsNull(context.Result); + } + + [Test] + public void Does_Not_Set_Result_When_Remote_Address_Is_Null_Ip_Address() + { + // Arrange + var context = CreateContext(remoteIpAddress: "::1"); + var attribute = new OnlyLocalRequestsAttribute(); + + // Act + attribute.OnActionExecuting(context); + + // Assert + Assert.IsNull(context.Result); + } + + [Test] + public void Does_Not_Set_Result_When_Remote_Address_Matches_Local_Address() + { + // Arrange + var context = CreateContext(remoteIpAddress: "100.1.2.3", localIpAddress: "100.1.2.3"); + var attribute = new OnlyLocalRequestsAttribute(); + + // Act + attribute.OnActionExecuting(context); + + // Assert + Assert.IsNull(context.Result); + } + + [Test] + public void Returns_Not_Found_When_Remote_Address_Does_Not_Match_Local_Address() + { + // Arrange + var context = CreateContext(remoteIpAddress: "100.1.2.3", localIpAddress: "100.1.2.2"); + var attribute = new OnlyLocalRequestsAttribute(); + + // Act + attribute.OnActionExecuting(context); + + // Assert + var typedResult = context.Result as NotFoundResult; + Assert.IsNotNull(typedResult); + } + + [Test] + public void Does_Not_Set_Result_When_Remote_Address_Matches_LoopBack_Address() + { + // Arrange + var context = CreateContext(remoteIpAddress: "127.0.0.1", localIpAddress: "::1"); + var attribute = new OnlyLocalRequestsAttribute(); + + // Act + attribute.OnActionExecuting(context); + + // Assert + Assert.IsNull(context.Result); + } + + [Test] + public void Returns_Not_Found_When_Remote_Address_Does_Not_Match_LoopBack_Address() + { + // Arrange + var context = CreateContext(remoteIpAddress: "100.1.2.3", localIpAddress: "::1"); + var attribute = new OnlyLocalRequestsAttribute(); + + // Act + attribute.OnActionExecuting(context); + + // Assert + var typedResult = context.Result as NotFoundResult; + Assert.IsNotNull(typedResult); + } + + private static ActionExecutingContext CreateContext(string remoteIpAddress = null, string localIpAddress = null) + { + var httpContext = new DefaultHttpContext(); + if (!string.IsNullOrEmpty(remoteIpAddress)) + { + httpContext.Connection.RemoteIpAddress = IPAddress.Parse(remoteIpAddress); + } + + if (!string.IsNullOrEmpty(localIpAddress)) + { + httpContext.Connection.LocalIpAddress = IPAddress.Parse(localIpAddress); + } + + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + + return new ActionExecutingContext( + actionContext, + new List(), + new Dictionary(), + new Mock().Object); + } + } +} diff --git a/src/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttribute.cs new file mode 100644 index 0000000000..9ee289a39c --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttribute.cs @@ -0,0 +1,18 @@ +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Web.Common.Extensions; + +namespace Umbraco.Web.BackOffice.Filters +{ + public class OnlyLocalRequestsAttribute : ActionFilterAttribute + { + public override void OnActionExecuting(ActionExecutingContext context) + { + if (!context.HttpContext.Request.IsLocal()) + { + context.Result = new NotFoundResult(); + } + } + } +} diff --git a/src/Umbraco.Web.Common/Extensions/HttpRequestExtensions.cs b/src/Umbraco.Web.Common/Extensions/HttpRequestExtensions.cs index 9dab445992..c346f0ddc9 100644 --- a/src/Umbraco.Web.Common/Extensions/HttpRequestExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/HttpRequestExtensions.cs @@ -1,4 +1,5 @@ -using Microsoft.AspNetCore.Http; +using System.Net; +using Microsoft.AspNetCore.Http; namespace Umbraco.Web.Common.Extensions { @@ -8,5 +9,34 @@ namespace Umbraco.Web.Common.Extensions { return request.Headers.TryGetValue("X-UMB-CULTURE", out var values) ? values[0] : null; } + + /// + /// Determines if a request is local. + /// + /// True if request is local + /// + /// Hat-tip: https://stackoverflow.com/a/41242493/489433 + /// + public static bool IsLocal(this HttpRequest request) + { + var connection = request.HttpContext.Connection; + if (connection.RemoteIpAddress.IsSet()) + { + // We have a remote address set up + return connection.LocalIpAddress.IsSet() + // Is local is same as remote, then we are local + ? connection.RemoteIpAddress.Equals(connection.LocalIpAddress) + // else we are remote if the remote IP address is not a loopback address + : IPAddress.IsLoopback(connection.RemoteIpAddress); + } + + return true; + } + + private static bool IsSet(this IPAddress address) + { + const string NullIpAddress = "::1"; + return address != null && address.ToString() != NullIpAddress; + } } } diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 88833c38de..9fb19447de 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -147,6 +147,7 @@ + @@ -225,7 +226,6 @@ - diff --git a/src/Umbraco.Web/WebApi/Filters/OnlyLocalRequestsAttribute.cs b/src/Umbraco.Web/WebApi/Filters/OnlyLocalRequestsAttribute.cs index 6906519b17..710749b39f 100644 --- a/src/Umbraco.Web/WebApi/Filters/OnlyLocalRequestsAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/OnlyLocalRequestsAttribute.cs @@ -6,6 +6,7 @@ using System.Web.Http.Filters; namespace Umbraco.Web.WebApi.Filters { + // Migrated to .NET Core public class OnlyLocalRequestsAttribute : ActionFilterAttribute { public override void OnActionExecuting(HttpActionContext actionContext) From 60c5c15fdf2886a5350e7d2b020160e977bb6ba4 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sat, 16 May 2020 19:35:45 +0200 Subject: [PATCH 3/5] Migrated ValidationFilterAttribute into netcore --- .../Filters/ValidationFilterAttributeTests.cs | 68 +++++++++++++++++++ .../Filters/ValidationFilterAttribute.cs | 21 ++++++ .../Umbraco.Web.BackOffice.csproj | 6 ++ .../Filters/ValidationFilterAttribute.cs | 1 + 4 files changed, 96 insertions(+) create mode 100644 src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/ValidationFilterAttributeTests.cs create mode 100644 src/Umbraco.Web.BackOffice/Filters/ValidationFilterAttribute.cs diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/ValidationFilterAttributeTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/ValidationFilterAttributeTests.cs new file mode 100644 index 0000000000..51521c48fa --- /dev/null +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/ValidationFilterAttributeTests.cs @@ -0,0 +1,68 @@ +using System; +using System.Collections.Generic; +using System.Net; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.Routing; +using Microsoft.AspNetCore.Routing; +using Moq; +using NUnit.Framework; +using Umbraco.Web.BackOffice.Filters; + +namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Filters +{ + [TestFixture] + public class ValidationFilterAttributeTests + { + [Test] + public void Does_Not_Set_Result_When_No_Errors_In_Model_State() + { + // Arrange + var context = CreateContext(); + var attribute = new ValidationFilterAttribute(); + + // Act + attribute.OnActionExecuting(context); + + // Assert + Assert.IsNull(context.Result); + } + + [Test] + public void Returns_Bad_Request_When_Errors_In_Model_State() + { + // Arrange + var context = CreateContext(withError: true); + var attribute = new ValidationFilterAttribute(); + + // Act + attribute.OnActionExecuting(context); + + // Assert + var typedResult = context.Result as BadRequestObjectResult; + Assert.IsNotNull(typedResult); + } + + private static ActionExecutingContext CreateContext(bool withError = false) + { + var httpContext = new DefaultHttpContext(); + + var modelState = new ModelStateDictionary(); + if (withError) + { + modelState.AddModelError(string.Empty, "Error"); + } + + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor(), modelState); + + return new ActionExecutingContext( + actionContext, + new List(), + new Dictionary(), + new Mock().Object); + } + } +} diff --git a/src/Umbraco.Web.BackOffice/Filters/ValidationFilterAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/ValidationFilterAttribute.cs new file mode 100644 index 0000000000..360cc88e0a --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Filters/ValidationFilterAttribute.cs @@ -0,0 +1,21 @@ +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; + +namespace Umbraco.Web.BackOffice.Filters +{ + /// + /// An action filter used to do basic validation against the model and return a result + /// straight away if it fails. + /// + internal sealed class ValidationFilterAttribute : ActionFilterAttribute + { + public override void OnActionExecuting(ActionExecutingContext context) + { + var modelState = context.ModelState; + if (!context.ModelState.IsValid) + { + context.Result = new BadRequestObjectResult(modelState); + } + } + } +} diff --git a/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj b/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj index 5b031c095e..3f6dd6969c 100644 --- a/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj +++ b/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj @@ -14,6 +14,12 @@ + + + <_Parameter1>Umbraco.Tests.UnitTests + + + diff --git a/src/Umbraco.Web/WebApi/Filters/ValidationFilterAttribute.cs b/src/Umbraco.Web/WebApi/Filters/ValidationFilterAttribute.cs index f56d9c28c4..d4e6af93ac 100644 --- a/src/Umbraco.Web/WebApi/Filters/ValidationFilterAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/ValidationFilterAttribute.cs @@ -13,6 +13,7 @@ namespace Umbraco.Web.WebApi.Filters /// An action filter used to do basic validation against the model and return a result /// straight away if it fails. /// + /// Migrated to .NET core internal sealed class ValidationFilterAttribute : ActionFilterAttribute { public override void OnActionExecuting(HttpActionContext actionContext) From ed8f65b74e400ab506044ce28a8dbfbe41ccbff2 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Sun, 17 May 2020 08:52:28 +0200 Subject: [PATCH 4/5] Migrated AppendUserModifiedHeaderAttribute into netcore --- .../AppendUserModifiedHeaderAttributeTests.cs | 121 ++++++++++++++++++ .../OnlyLocalRequestsAttributeTests.cs | 4 +- .../AppendUserModifiedHeaderAttribute.cs | 80 ++++++++++++ .../AppendUserModifiedHeaderAttribute.cs | 1 + 4 files changed, 203 insertions(+), 3 deletions(-) create mode 100644 src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttributeTests.cs create mode 100644 src/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttribute.cs diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttributeTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttributeTests.cs new file mode 100644 index 0000000000..f9c7c72b59 --- /dev/null +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttributeTests.cs @@ -0,0 +1,121 @@ +using System; +using System.Collections.Generic; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Routing; +using Moq; +using NUnit.Framework; +using Umbraco.Core.Models.Membership; +using Umbraco.Web.BackOffice.Filters; +using Umbraco.Web.Security; + +namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Filters +{ + [TestFixture] + public class AppendUserModifiedHeaderAttributeTests + { + [Test] + public void Appends_Header_When_No_User_Parameter_Provider() + { + // Arrange + var context = CreateContext(); + var attribute = new AppendUserModifiedHeaderAttribute(); + + // Act + attribute.OnActionExecuting(context); + + // Assert + context.HttpContext.Response.Headers.TryGetValue("X-Umb-User-Modified", out var headerValue); + Assert.AreEqual("1", headerValue[0]); + } + + [Test] + public void Does_Not_Append_Header_If_Already_Exists() + { + // Arrange + var context = CreateContext(headerValue: "0"); + var attribute = new AppendUserModifiedHeaderAttribute(); + + // Act + attribute.OnActionExecuting(context); + + // Assert + context.HttpContext.Response.Headers.TryGetValue("X-Umb-User-Modified", out var headerValue); + Assert.AreEqual("0", headerValue[0]); + } + + [Test] + public void Does_Not_Append_Header_When_User_Id_Parameter_Provided_And_Does_Not_Match_Current_User() + { + // Arrange + var context = CreateContext(actionArgument: new KeyValuePair("UserId", 99)); + var userIdParameter = "UserId"; + var attribute = new AppendUserModifiedHeaderAttribute(userIdParameter); + + // Act + attribute.OnActionExecuting(context); + + // Assert + Assert.IsFalse(context.HttpContext.Response.Headers.ContainsKey("X-Umb-User-Modified")); + } + + [Test] + public void Appends_Header_When_User_Id_Parameter_Provided_And_Does_Not_Match_Current_User() + { + // Arrange + var context = CreateContext(actionArgument: new KeyValuePair("UserId", 100)); + var userIdParameter = "UserId"; + var attribute = new AppendUserModifiedHeaderAttribute(userIdParameter); + + // Act + attribute.OnActionExecuting(context); + + // Assert + context.HttpContext.Response.Headers.TryGetValue("X-Umb-User-Modified", out var headerValue); + Assert.AreEqual("1", headerValue[0]); + } + + private static ActionExecutingContext CreateContext(string headerValue = null, KeyValuePair actionArgument = default) + { + var httpContext = new DefaultHttpContext(); + if (!string.IsNullOrEmpty(headerValue)) + { + httpContext.Response.Headers.Add("X-Umb-User-Modified", headerValue); + } + + var currentUserMock = new Mock(); + currentUserMock + .SetupGet(x => x.Id) + .Returns(100); + + var webSecurityMock = new Mock(); + webSecurityMock + .SetupGet(x => x.CurrentUser) + .Returns(currentUserMock.Object); + + var serviceProviderMock = new Mock(); + serviceProviderMock + .Setup(x => x.GetService(typeof(IWebSecurity))) + .Returns(webSecurityMock.Object); + + httpContext.RequestServices = serviceProviderMock.Object; + + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + + var context = new ActionExecutingContext( + actionContext, + new List(), + new Dictionary(), + new Mock().Object); + + if (!EqualityComparer>.Default.Equals(actionArgument, default)) + { + context.ActionArguments.Add(actionArgument); + } + + return context; + } + } +} diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttributeTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttributeTests.cs index f00713f6a4..7d1fbdddf1 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttributeTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/OnlyLocalRequestsAttributeTests.cs @@ -1,11 +1,9 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Net; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; -using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Routing; using Moq; using NUnit.Framework; diff --git a/src/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttribute.cs new file mode 100644 index 0000000000..b967e51d52 --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttribute.cs @@ -0,0 +1,80 @@ +using System; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Core; +using Umbraco.Web.Security; + +namespace Umbraco.Web.BackOffice.Filters +{ + /// + /// Appends a custom response header to notify the UI that the current user data has been modified + /// + public sealed class AppendUserModifiedHeaderAttribute : ActionFilterAttribute + { + private readonly string _userIdParameter; + + /// + /// An empty constructor which will always set the header. + /// + public AppendUserModifiedHeaderAttribute() + { + } + + /// + /// A constructor specifying the action parameter name containing the user id to match against the + /// current user and if they match the header will be appended. + /// + /// + public AppendUserModifiedHeaderAttribute(string userIdParameter) + { + _userIdParameter = userIdParameter ?? throw new ArgumentNullException(nameof(userIdParameter)); + } + + public override void OnActionExecuting(ActionExecutingContext context) + { + if (_userIdParameter.IsNullOrWhiteSpace()) + { + AppendHeader(context); + } + else + { + if (!context.ActionArguments.ContainsKey(_userIdParameter)) + { + throw new InvalidOperationException($"No argument found for the current action with the name: {_userIdParameter}"); + } + + var webSecurityService = context.HttpContext.RequestServices.GetService(); + var user = webSecurityService.CurrentUser; + if (user == null) + { + return; + } + + var userId = GetUserIdFromParameter(context.ActionArguments[_userIdParameter]); + if (userId == user.Id) + { + AppendHeader(context); + } + } + } + + public static void AppendHeader(ActionExecutingContext context) + { + const string HeaderName = "X-Umb-User-Modified"; + if (context.HttpContext.Response.Headers.ContainsKey(HeaderName) == false) + { + context.HttpContext.Response.Headers.Add(HeaderName, "1"); + } + } + + private int GetUserIdFromParameter(object parameterValue) + { + if (parameterValue is int) + { + return (int)parameterValue; + } + + throw new InvalidOperationException($"The id type: {parameterValue.GetType()} is not a supported id."); + } + } +} diff --git a/src/Umbraco.Web/WebApi/Filters/AppendUserModifiedHeaderAttribute.cs b/src/Umbraco.Web/WebApi/Filters/AppendUserModifiedHeaderAttribute.cs index 5ac8d886f0..95e1a94787 100644 --- a/src/Umbraco.Web/WebApi/Filters/AppendUserModifiedHeaderAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/AppendUserModifiedHeaderAttribute.cs @@ -9,6 +9,7 @@ namespace Umbraco.Web.WebApi.Filters /// /// Appends a custom response header to notify the UI that the current user data has been modified /// + /// Migrated to NET core public sealed class AppendUserModifiedHeaderAttribute : ActionFilterAttribute { private readonly string _userIdParameter; From 0e4d8506fbbe1e8e424ae8b5706d88001f6666e0 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Mon, 18 May 2020 09:36:56 +0200 Subject: [PATCH 5/5] Updated following PR review. --- .../AppendUserModifiedHeaderAttributeTests.cs | 15 +++++++++++++-- .../Filters/AppendUserModifiedHeaderAttribute.cs | 5 ++--- .../Filters/ValidationFilterAttribute.cs | 2 +- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttributeTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttributeTests.cs index f9c7c72b59..c91660563e 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttributeTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttributeTests.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Routing; using Moq; using NUnit.Framework; using Umbraco.Core.Models.Membership; +using Umbraco.Web; using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.Security; @@ -95,10 +96,20 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Filters .SetupGet(x => x.CurrentUser) .Returns(currentUserMock.Object); + var umbracoContextMock = new Mock(); + umbracoContextMock + .SetupGet(x => x.Security) + .Returns(webSecurityMock.Object); + + var umbracoContextAccessorMock = new Mock(); + umbracoContextAccessorMock + .SetupGet(x => x.UmbracoContext) + .Returns(umbracoContextMock.Object); + var serviceProviderMock = new Mock(); serviceProviderMock - .Setup(x => x.GetService(typeof(IWebSecurity))) - .Returns(webSecurityMock.Object); + .Setup(x => x.GetService(typeof(IUmbracoContextAccessor))) + .Returns(umbracoContextAccessorMock.Object); httpContext.RequestServices = serviceProviderMock.Object; diff --git a/src/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttribute.cs index b967e51d52..50ef8cf906 100644 --- a/src/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/AppendUserModifiedHeaderAttribute.cs @@ -2,7 +2,6 @@ using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.Extensions.DependencyInjection; using Umbraco.Core; -using Umbraco.Web.Security; namespace Umbraco.Web.BackOffice.Filters { @@ -43,8 +42,8 @@ namespace Umbraco.Web.BackOffice.Filters throw new InvalidOperationException($"No argument found for the current action with the name: {_userIdParameter}"); } - var webSecurityService = context.HttpContext.RequestServices.GetService(); - var user = webSecurityService.CurrentUser; + var umbracoContextAccessor = context.HttpContext.RequestServices.GetService(); + var user = umbracoContextAccessor.UmbracoContext.Security.CurrentUser; if (user == null) { return; diff --git a/src/Umbraco.Web.BackOffice/Filters/ValidationFilterAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/ValidationFilterAttribute.cs index 360cc88e0a..77d44062d0 100644 --- a/src/Umbraco.Web.BackOffice/Filters/ValidationFilterAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/ValidationFilterAttribute.cs @@ -12,7 +12,7 @@ namespace Umbraco.Web.BackOffice.Filters public override void OnActionExecuting(ActionExecutingContext context) { var modelState = context.ModelState; - if (!context.ModelState.IsValid) + if (!modelState.IsValid) { context.Result = new BadRequestObjectResult(modelState); }