diff --git a/AMWD.Common.AspNetCore/Attributes/IPWhitelistAttribute.cs b/AMWD.Common.AspNetCore/Attributes/IPAllowListAttribute.cs similarity index 94% rename from AMWD.Common.AspNetCore/Attributes/IPWhitelistAttribute.cs rename to AMWD.Common.AspNetCore/Attributes/IPAllowListAttribute.cs index ef2f9bb..4441633 100644 --- a/AMWD.Common.AspNetCore/Attributes/IPWhitelistAttribute.cs +++ b/AMWD.Common.AspNetCore/Attributes/IPAllowListAttribute.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Mvc.Filters /// /// Implements an IP filter. Only defined addresses are allowed to access. /// - public class IPWhitelistAttribute : ActionFilterAttribute + public class IPAllowListAttribute : ActionFilterAttribute { /// /// Gets or sets a value indicating whether local (localhost) access is granted (Default: true). diff --git a/AMWD.Common.AspNetCore/Attributes/IPBlacklistAttribute.cs b/AMWD.Common.AspNetCore/Attributes/IPBlockListAttribute.cs similarity index 85% rename from AMWD.Common.AspNetCore/Attributes/IPBlacklistAttribute.cs rename to AMWD.Common.AspNetCore/Attributes/IPBlockListAttribute.cs index 9ca9c88..36d3e33 100644 --- a/AMWD.Common.AspNetCore/Attributes/IPBlacklistAttribute.cs +++ b/AMWD.Common.AspNetCore/Attributes/IPBlockListAttribute.cs @@ -10,12 +10,12 @@ namespace Microsoft.AspNetCore.Mvc.Filters /// /// Implements an IP filter. The defined addresses are blocked. /// - public class IPBlacklistAttribute : ActionFilterAttribute + public class IPBlockListAttribute : ActionFilterAttribute { /// /// Gets or sets a value indicating whether local (localhost) access is blocked (Default: false). /// - public bool RestrictLocalAccess { get; set; } + public bool BlockLocalAccess { get; set; } /// /// Gets or sets a configuration key where the blocked IP addresses are defined. @@ -35,7 +35,7 @@ namespace Microsoft.AspNetCore.Mvc.Filters /// /// Gets or sets a comma separated list of blocked IP addresses. /// - public string RestrictedIpAddresses { get; set; } + public string BlockedIpAddresses { get; set; } /// public override void OnActionExecuting(ActionExecutingContext context) @@ -43,13 +43,13 @@ namespace Microsoft.AspNetCore.Mvc.Filters base.OnActionExecuting(context); context.HttpContext.Items["RemoteAddress"] = context.HttpContext.GetRemoteIpAddress(); - if (!RestrictLocalAccess && context.HttpContext.IsLocalRequest()) + if (!BlockLocalAccess && context.HttpContext.IsLocalRequest()) return; var remoteIpAddress = context.HttpContext.GetRemoteIpAddress(); - if (!string.IsNullOrWhiteSpace(RestrictedIpAddresses)) + if (!string.IsNullOrWhiteSpace(BlockedIpAddresses)) { - string[] ipAddresses = RestrictedIpAddresses.Split(','); + string[] ipAddresses = BlockedIpAddresses.Split(','); foreach (string ipAddress in ipAddresses) { if (string.IsNullOrWhiteSpace(ipAddress)) diff --git a/AMWD.Common.AspNetCore/Extensions/HttpContextExtensions.cs b/AMWD.Common.AspNetCore/Extensions/HttpContextExtensions.cs index d630d61..13f49c3 100644 --- a/AMWD.Common.AspNetCore/Extensions/HttpContextExtensions.cs +++ b/AMWD.Common.AspNetCore/Extensions/HttpContextExtensions.cs @@ -1,4 +1,5 @@ -using System.Net; +using System.Linq; +using System.Net; using Microsoft.AspNetCore.Antiforgery; using Microsoft.Extensions.DependencyInjection; @@ -9,30 +10,57 @@ namespace Microsoft.AspNetCore.Http /// public static class HttpContextExtensions { + // Search these additional headers for a remote client ip address. + private static readonly string[] defaultIpHeaderNames = new[] + { + "X-Forwarded-For", // commonly used on all known proxies + "X-Real-IP", // wide-spread alternative to X-Forwarded-For + "CF-Connecting-IP" // set by Cloudflare + }; + /// /// Retrieves the antiforgery token. /// /// The current . - /// Name and value of the token. - public static (string Name, string Value) GetAntiforgeryToken(this HttpContext httpContext) + /// FormName, HeaderName and Value of the antiforgery token. + public static (string FormName, string HeaderName, string Value) GetAntiforgeryToken(this HttpContext httpContext) { - var af = httpContext.RequestServices.GetService(); - var set = af?.GetAndStoreTokens(httpContext); + var antiforgery = httpContext.RequestServices.GetService(); + var tokenSet = antiforgery?.GetAndStoreTokens(httpContext); - return (Name: set?.FormFieldName, Value: set?.RequestToken); + return (tokenSet?.FormFieldName, tokenSet?.HeaderName, tokenSet?.RequestToken); } /// /// Returns the remote ip address. /// + /// + /// Searches for additional headers in the following order: + /// + /// X-Forwarded-For + /// X-Real-IP + /// CF-Connecting-IP + /// + /// /// The current . - /// The name of the header to resolve the when behind a proxy (Default: X-Forwarded-For). + /// The name of the header to resolve the when behind a proxy. /// The ip address of the client. - public static IPAddress GetRemoteIpAddress(this HttpContext httpContext, string headerName = "X-Forwarded-For") + public static IPAddress GetRemoteIpAddress(this HttpContext httpContext, string ipHeaderName = null) { - string forwardedHeader = httpContext.Request.Headers[headerName].ToString(); - if (!string.IsNullOrWhiteSpace(forwardedHeader) && IPAddress.TryParse(forwardedHeader, out var forwarded)) - return forwarded; + string forwardedForAddress = null; + + var headerNames = string.IsNullOrWhiteSpace(ipHeaderName) ? defaultIpHeaderNames : new[] { ipHeaderName }.Concat(defaultIpHeaderNames); + foreach (string headerName in headerNames) + { + if (!httpContext.Request.Headers.ContainsKey(headerName)) + continue; + + forwardedForAddress = httpContext.Request.Headers[headerName].ToString(); + break; + } + + if (!string.IsNullOrWhiteSpace(forwardedForAddress) && IPAddress.TryParse(forwardedForAddress, out var remoteAddress)) + return remoteAddress; return httpContext.Connection.RemoteIpAddress; } @@ -40,12 +68,20 @@ namespace Microsoft.AspNetCore.Http /// /// Returns whether the request was made locally. /// + /// + /// Searches for additional headers in the following order: + /// + /// X-Forwarded-For + /// X-Real-IP + /// CF-Connecting-IP + /// + /// /// The current . - /// The name of the header to resolve the when behind a proxy (Default: X-Forwarded-For). + /// The name of the header to resolve the when behind a proxy. /// - public static bool IsLocalRequest(this HttpContext httpContext, string headerName = "X-Forwarded-For") + public static bool IsLocalRequest(this HttpContext httpContext, string ipHeaderName = null) { - var remoteIpAddress = httpContext.GetRemoteIpAddress(headerName); + var remoteIpAddress = httpContext.GetRemoteIpAddress(ipHeaderName); return httpContext.Connection.LocalIpAddress.Equals(remoteIpAddress); } diff --git a/CHANGELOG.md b/CHANGELOG.md index 1bae954..4ccaa58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,11 +5,25 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [Upcoming](https://git.am-wd.de/AM.WD/common/compare/v1.11.1...main) - 0000-00-00 +## [Upcoming](https://git.am-wd.de/AM.WD/common/compare/v1.12.0...main) - 0000-00-00 _no changes yet_ +## [v1.12.0](https://git.am-wd.de/AM.WD/common/compare/v1.11.1...v1.12.0) - 2023-06-01 + +### Changed + +- Renamed `IPBlacklistAttribute` to `IPBlockListAttribute` +- Renamed `IPWhitelistAttribute` to `IPAllowListAttribute` +- `HttpContextExtensions` + - `GetAntiforgeryToken()` now returns the header name also + - `GetRemoteIpAddress()` checks following additional headers by default: + - `X-Forwarded-For` + - `X-Real-IP` + - `CF-Connecting-IP` + + ## [v1.11.1](https://git.am-wd.de/AM.WD/common/compare/v1.11.0...v1.11.1) - 2023-05-11 ### Fixed diff --git a/Directory.Build.targets b/Directory.Build.targets index 84ed1ef..39a9eb1 100644 --- a/Directory.Build.targets +++ b/Directory.Build.targets @@ -6,8 +6,4 @@ - - - - diff --git a/UnitTests/AspNetCore/Attributes/IPWhitelistAttributeTests.cs b/UnitTests/AspNetCore/Attributes/IPAllowListAttributeTests.cs similarity index 91% rename from UnitTests/AspNetCore/Attributes/IPWhitelistAttributeTests.cs rename to UnitTests/AspNetCore/Attributes/IPAllowListAttributeTests.cs index 0545aac..12b2bc3 100644 --- a/UnitTests/AspNetCore/Attributes/IPWhitelistAttributeTests.cs +++ b/UnitTests/AspNetCore/Attributes/IPAllowListAttributeTests.cs @@ -14,7 +14,7 @@ namespace UnitTests.AspNetCore.Attributes { [TestClass] [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] - public class IPWhitelistAttributeTests + public class IPAllowListAttributeTests { private Dictionary requestHeaders; private Dictionary itemsCallback; @@ -37,7 +37,7 @@ namespace UnitTests.AspNetCore.Attributes { // arrange var remote = IPAddress.Parse("192.168.178.1"); - var attribute = new IPWhitelistAttribute(); + var attribute = new IPAllowListAttribute(); var context = GetContext(remote); // act @@ -57,7 +57,7 @@ namespace UnitTests.AspNetCore.Attributes { // arrange var remote = IPAddress.Parse("192.168.178.1"); - var attribute = new IPWhitelistAttribute + var attribute = new IPAllowListAttribute { AllowedIpAddresses = "192.168.178:1" }; @@ -79,7 +79,7 @@ namespace UnitTests.AspNetCore.Attributes public void ShouldAllowLocalAccess() { // arrange - var attribute = new IPWhitelistAttribute(); + var attribute = new IPAllowListAttribute(); var context = GetContext(); // act @@ -95,7 +95,7 @@ namespace UnitTests.AspNetCore.Attributes public void ShouldDenyLocalAccess() { // arrange - var attribute = new IPWhitelistAttribute + var attribute = new IPAllowListAttribute { AllowLocalAccess = false }; @@ -120,7 +120,7 @@ namespace UnitTests.AspNetCore.Attributes { // arrange var remote = IPAddress.Parse(address); - var attribute = new IPWhitelistAttribute + var attribute = new IPAllowListAttribute { AllowLocalAccess = false, AllowedIpAddresses = ",127.0.0.0/8,192.168.178.10" @@ -154,7 +154,7 @@ namespace UnitTests.AspNetCore.Attributes configExists = true; allowedIpsConfig.Add("127.0.0.0/8"); allowedIpsConfig.Add("192.168.178.10"); - var attribute = new IPWhitelistAttribute + var attribute = new IPAllowListAttribute { AllowLocalAccess = true, ConfigurationKey = configKey @@ -178,7 +178,7 @@ namespace UnitTests.AspNetCore.Attributes configExists = true; allowedIpsConfig.Add(""); allowedIpsConfig.Add("192.168.178.10"); - var attribute = new IPWhitelistAttribute + var attribute = new IPAllowListAttribute { AllowLocalAccess = false, ConfigurationKey = configKey @@ -206,7 +206,7 @@ namespace UnitTests.AspNetCore.Attributes configKey = "White:List"; configExists = true; allowedIpsConfig.Add("192.168.178.10"); - var attribute = new IPWhitelistAttribute + var attribute = new IPAllowListAttribute { AllowLocalAccess = false, ConfigurationKey = configKey @@ -239,7 +239,7 @@ namespace UnitTests.AspNetCore.Attributes // arrange configKey = "White:List"; configExists = false; - var attribute = new IPWhitelistAttribute + var attribute = new IPAllowListAttribute { AllowLocalAccess = false, ConfigurationKey = configKey diff --git a/UnitTests/AspNetCore/Attributes/IPBlacklistAttributeTests.cs b/UnitTests/AspNetCore/Attributes/IPBlockListAttributeTests.cs similarity index 87% rename from UnitTests/AspNetCore/Attributes/IPBlacklistAttributeTests.cs rename to UnitTests/AspNetCore/Attributes/IPBlockListAttributeTests.cs index 5ca53c5..0062362 100644 --- a/UnitTests/AspNetCore/Attributes/IPBlacklistAttributeTests.cs +++ b/UnitTests/AspNetCore/Attributes/IPBlockListAttributeTests.cs @@ -14,7 +14,7 @@ namespace UnitTests.AspNetCore.Attributes { [TestClass] [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage] - public class IPBlacklistAttributeTests + public class IPBlockListAttributeTests { private Dictionary requestHeaders; private Dictionary itemsCallback; @@ -37,7 +37,7 @@ namespace UnitTests.AspNetCore.Attributes { // arrange var remote = IPAddress.Parse("192.168.178.1"); - var attribute = new IPBlacklistAttribute(); + var attribute = new IPBlockListAttribute(); var context = GetContext(remote); // act @@ -54,9 +54,9 @@ namespace UnitTests.AspNetCore.Attributes { // arrange var remote = IPAddress.Parse("192.168.178.1"); - var attribute = new IPBlacklistAttribute + var attribute = new IPBlockListAttribute { - RestrictedIpAddresses = "192.168.178:1" + BlockedIpAddresses = "192.168.178:1" }; var context = GetContext(remote); @@ -73,10 +73,10 @@ namespace UnitTests.AspNetCore.Attributes public void ShouldAllowLocalAccess() { // arrange - var attribute = new IPBlacklistAttribute + var attribute = new IPBlockListAttribute { - RestrictLocalAccess = false, - RestrictedIpAddresses = "127.0.0.0/8" + BlockLocalAccess = false, + BlockedIpAddresses = "127.0.0.0/8" }; var context = GetContext(); @@ -93,10 +93,10 @@ namespace UnitTests.AspNetCore.Attributes public void ShouldBlockLocalAccess() { // arrange - var attribute = new IPBlacklistAttribute + var attribute = new IPBlockListAttribute { - RestrictLocalAccess = true, - RestrictedIpAddresses = ",127.0.0.0/8" + BlockLocalAccess = true, + BlockedIpAddresses = ",127.0.0.0/8" }; var context = GetContext(); @@ -119,10 +119,10 @@ namespace UnitTests.AspNetCore.Attributes { // arrange var remote = IPAddress.Parse(address); - var attribute = new IPBlacklistAttribute + var attribute = new IPBlockListAttribute { - RestrictLocalAccess = true, - RestrictedIpAddresses = "127.0.0.0/8,192.168.178.10" + BlockLocalAccess = true, + BlockedIpAddresses = "127.0.0.0/8,192.168.178.10" }; var context = GetContext(remote); @@ -153,9 +153,9 @@ namespace UnitTests.AspNetCore.Attributes configExists = true; restrictedIpsConfig.Add("127.0.0.0/8"); restrictedIpsConfig.Add("192.168.178.10"); - var attribute = new IPBlacklistAttribute + var attribute = new IPBlockListAttribute { - RestrictLocalAccess = false, + BlockLocalAccess = false, ConfigurationKey = configKey }; var context = GetContext(); @@ -178,9 +178,9 @@ namespace UnitTests.AspNetCore.Attributes restrictedIpsConfig.Add(""); restrictedIpsConfig.Add("127.0.0.0/8"); restrictedIpsConfig.Add("192.168.178.10"); - var attribute = new IPBlacklistAttribute + var attribute = new IPBlockListAttribute { - RestrictLocalAccess = true, + BlockLocalAccess = true, ConfigurationKey = configKey }; var context = GetContext(); @@ -207,9 +207,9 @@ namespace UnitTests.AspNetCore.Attributes configExists = true; restrictedIpsConfig.Add("127.0.0.0/8"); restrictedIpsConfig.Add("192.168.178.10"); - var attribute = new IPBlacklistAttribute + var attribute = new IPBlockListAttribute { - RestrictLocalAccess = true, + BlockLocalAccess = true, ConfigurationKey = configKey }; var remote = IPAddress.Parse(address); @@ -240,9 +240,9 @@ namespace UnitTests.AspNetCore.Attributes // arrange configKey = "Black:List"; configExists = false; - var attribute = new IPBlacklistAttribute + var attribute = new IPBlockListAttribute { - RestrictLocalAccess = true, + BlockLocalAccess = true, ConfigurationKey = configKey }; var context = GetContext(); diff --git a/UnitTests/AspNetCore/Extensions/HttpContextExtensionsTests.cs b/UnitTests/AspNetCore/Extensions/HttpContextExtensionsTests.cs index 247459b..9a6dc52 100644 --- a/UnitTests/AspNetCore/Extensions/HttpContextExtensionsTests.cs +++ b/UnitTests/AspNetCore/Extensions/HttpContextExtensionsTests.cs @@ -14,7 +14,8 @@ namespace UnitTests.AspNetCore.Extensions { private Mock sessionMock; - private string tokenName; + private string tokenFormName; + private string tokenHeaderName; private string tokenValue; private Dictionary requestHeaders; @@ -26,7 +27,8 @@ namespace UnitTests.AspNetCore.Extensions [TestInitialize] public void InitializeTests() { - tokenName = null; + tokenFormName = null; + tokenHeaderName = null; tokenValue = null; requestHeaders = new Dictionary(); @@ -42,34 +44,38 @@ namespace UnitTests.AspNetCore.Extensions public void ShouldReturnAntiforgery() { // arrange - tokenName = "af-token"; + tokenFormName = "af-token"; + tokenHeaderName = "af-header"; tokenValue = "security_first"; var context = GetContext(); // act - var result = context.GetAntiforgeryToken(); + var (formName, headerName, value) = context.GetAntiforgeryToken(); // assert - Assert.AreEqual(tokenName, result.Name); - Assert.AreEqual(tokenValue, result.Value); + Assert.AreEqual(tokenFormName, formName); + Assert.AreEqual(tokenHeaderName, headerName); + Assert.AreEqual(tokenValue, value); } [TestMethod] public void ShouldReturnAntiforgeryNullService() { // arrange - tokenName = "af-token"; + tokenFormName = "af-token"; + tokenHeaderName = "af-header"; tokenValue = "security_first"; var context = GetContext(hasAntiforgery: false); // act - var result = context.GetAntiforgeryToken(); + var (formName, headerName, value) = context.GetAntiforgeryToken(); // assert - Assert.AreEqual(null, result.Name); - Assert.AreEqual(null, result.Value); + Assert.IsNull(formName); + Assert.IsNull(headerName); + Assert.IsNull(value); } [TestMethod] @@ -79,11 +85,12 @@ namespace UnitTests.AspNetCore.Extensions var context = GetContext(); // act - var result = context.GetAntiforgeryToken(); + var (formName, headerName, value) = context.GetAntiforgeryToken(); // assert - Assert.AreEqual(null, result.Name); - Assert.AreEqual(null, result.Value); + Assert.IsNull(formName); + Assert.IsNull(headerName); + Assert.IsNull(value); } #endregion Antiforgery @@ -105,13 +112,16 @@ namespace UnitTests.AspNetCore.Extensions Assert.AreEqual(remote, result); } - [TestMethod] - public void ShouldReturnDefaultHeader() + [DataTestMethod] + [DataRow("X-Forwarded-For")] + [DataRow("X-Real-IP")] + [DataRow("CF-Connecting-IP")] + public void ShouldReturnDefaultHeader(string headerName) { // arrange remote = IPAddress.Parse("1.2.3.4"); var header = IPAddress.Parse("5.6.7.8"); - requestHeaders.Add("X-Forwarded-For", header.ToString()); + requestHeaders.Add(headerName, header.ToString()); var context = GetContext(); @@ -130,12 +140,14 @@ namespace UnitTests.AspNetCore.Extensions remote = IPAddress.Parse("1.2.3.4"); string headerName = "FooBar"; var headerIp = IPAddress.Parse("5.6.7.8"); + requestHeaders.Add(headerName, headerIp.ToString()); + requestHeaders.Add("X-Forwarded-For", remote.ToString()); var context = GetContext(); // act - var result = context.GetRemoteIpAddress(headerName: headerName); + var result = context.GetRemoteIpAddress(ipHeaderName: headerName); // assert Assert.AreNotEqual(remote, result); @@ -221,7 +233,7 @@ namespace UnitTests.AspNetCore.Extensions var context = GetContext(); // act - bool result = context.IsLocalRequest(headerName: headerName); + bool result = context.IsLocalRequest(ipHeaderName: headerName); // assert Assert.IsTrue(result); @@ -254,7 +266,7 @@ namespace UnitTests.AspNetCore.Extensions var context = GetContext(); // act - bool result = context.IsLocalRequest(headerName: headerName); + bool result = context.IsLocalRequest(ipHeaderName: headerName); // assert Assert.IsFalse(result); @@ -385,7 +397,7 @@ namespace UnitTests.AspNetCore.Extensions var antiforgeryMock = new Mock(); antiforgeryMock .Setup(af => af.GetAndStoreTokens(It.IsAny())) - .Returns(string.IsNullOrWhiteSpace(tokenName) ? null : new AntiforgeryTokenSet(tokenValue, tokenValue, tokenName, tokenName)); + .Returns(() => string.IsNullOrWhiteSpace(tokenValue) ? null : new AntiforgeryTokenSet(tokenValue, tokenValue, tokenFormName, tokenHeaderName)); requestServicesMock .Setup(rs => rs.GetService(typeof(IAntiforgery)))