From 25b1eaa9fd4c34d441624b92939a543e227af879 Mon Sep 17 00:00:00 2001 From: Magnus Nilsson Date: Tue, 19 Mar 2019 16:06:06 +0100 Subject: [PATCH 1/3] Tests for asp.net core context builder --- .appveyor.yml | 6 ++-- src/Castle.Sdk/Context.cs | 14 ++++++--- src/Tests/When_creating_request_context.cs | 36 ++++++++++++++++++++++ 3 files changed, 48 insertions(+), 8 deletions(-) create mode 100644 src/Tests/When_creating_request_context.cs diff --git a/.appveyor.yml b/.appveyor.yml index d86285f..2ee3b53 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -4,8 +4,8 @@ image: Visual Studio 2017 install: - ps: dotnet tool install coveralls.net --tool-path tools build_script: - - cmd: dotnet build src/Castle.Sdk -c Release -f netstandard2.0 - - cmd: dotnet build src/Castle.Sdk -c Release -f net461 + - cmd: dotnet build src/Castle.Sdk + - cmd: dotnet build src/Tests -c Debug test_script: - - ps: dotnet test /p:CollectCoverage=true /p:CoverletOutputFormat=opencover /p:Include="[Castle.Sdk*]*" src/Tests/Tests.csproj + - ps: dotnet test --no-build /p:CollectCoverage=true /p:CoverletOutputFormat=opencover /p:Include="[Castle.Sdk*]*" src/Tests/Tests.csproj - ps: .\tools\csmacnz.coveralls.exe --opencover -i src/Tests/coverage.opencover.xml --useRelativePaths --repoToken $env:COVERALLS_REPO_TOKEN --commitId $env:APPVEYOR_REPO_COMMIT --commitBranch $env:APPVEYOR_REPO_BRANCH --commitAuthor $env:APPVEYOR_REPO_COMMIT_AUTHOR --commitEmail $env:APPVEYOR_REPO_COMMIT_AUTHOR_EMAIL --commitMessage $env:APPVEYOR_REPO_COMMIT_MESSAGE --jobId $env:APPVEYOR_JOB_ID diff --git a/src/Castle.Sdk/Context.cs b/src/Castle.Sdk/Context.cs index 7824560..2ef4b45 100644 --- a/src/Castle.Sdk/Context.cs +++ b/src/Castle.Sdk/Context.cs @@ -32,18 +32,22 @@ public static RequestContext FromHttpRequest(System.Web.HttpRequestBase request) #if NETSTANDARD2_0 public static RequestContext FromHttpRequest(Microsoft.AspNetCore.Http.HttpRequest request) { - var clientId = request.Headers.TryGetValue("X-Castle-Client-ID", out var headerId) - ? headerId.First() - : request.Cookies["__cid"]; - return new RequestContext() { - ClientId = clientId, + ClientId = GetClientId(request.Headers, request.Cookies), Headers = request.Headers.ToDictionary(x => x.Key, y => y.Value.FirstOrDefault()), Ip = request.HttpContext.Connection.RemoteIpAddress.ToString(), }; } + internal static string GetClientId( + IDictionary headers, + Microsoft.AspNetCore.Http.IRequestCookieCollection cookies) + { + return headers.TryGetValue("X-Castle-Client-ID", out var headerId) + ? headerId.First() + : cookies["__cid"]; + } #endif } } diff --git a/src/Tests/When_creating_request_context.cs b/src/Tests/When_creating_request_context.cs new file mode 100644 index 0000000..0fd4cc4 --- /dev/null +++ b/src/Tests/When_creating_request_context.cs @@ -0,0 +1,36 @@ +using System.Collections.Generic; +using AutoFixture.Xunit2; +using Castle; +using FluentAssertions; +using Microsoft.AspNetCore.Http.Internal; +using Microsoft.Extensions.Primitives; +using Xunit; + +namespace Tests +{ + public class When_creating_request_context + { + [Theory, AutoData] + public void Should_get_client_id_from_castle_header_if_present( + string castleHeaderValue, + string otherHeader, + string otherHeaderValue, + string cookieValue) + { + var headers = new Dictionary() + { + ["X-Castle-Client-ID"] = castleHeaderValue, + [otherHeader] = otherHeaderValue + }; + + var cookies = new RequestCookieCollection(new Dictionary() + { + ["__cid"] = cookieValue + }); + + var result = Context.GetClientId(headers, cookies); + + result.Should().Be(castleHeaderValue); + } + } +} From e4d44915066eafa8b55ded04841fb72ace774680 Mon Sep 17 00:00:00 2001 From: Magnus Nilsson Date: Tue, 19 Mar 2019 17:21:50 +0100 Subject: [PATCH 2/3] Can get ip from http header when building context --- src/Castle.Sdk/Context.cs | 26 +++++-- src/Tests/When_creating_request_context.cs | 82 +++++++++++++++++++++- 2 files changed, 100 insertions(+), 8 deletions(-) diff --git a/src/Castle.Sdk/Context.cs b/src/Castle.Sdk/Context.cs index 2ef4b45..d4fae3d 100644 --- a/src/Castle.Sdk/Context.cs +++ b/src/Castle.Sdk/Context.cs @@ -8,7 +8,7 @@ namespace Castle public static class Context { #if NET461 - public static RequestContext FromHttpRequest(System.Web.HttpRequestBase request) + public static RequestContext FromHttpRequest(System.Web.HttpRequestBase request, string ipHeader = null) { var headers = new Dictionary(); foreach (string key in request.Headers.Keys) @@ -18,25 +18,29 @@ public static RequestContext FromHttpRequest(System.Web.HttpRequestBase request) var clientId = request.Headers.AllKeys.Contains("X-Castle-Client-ID", StringComparer.OrdinalIgnoreCase) ? request.Headers["X-Castle-Client-ID"] - : request.Cookies["__cid"]?.Value; + : request.Cookies["__cid"]?.Value ?? ""; + + var ip = ipHeader != null && request.Headers.AllKeys.Contains(ipHeader, StringComparer.OrdinalIgnoreCase) + ? request.Headers[ipHeader] + : request.UserHostAddress; return new RequestContext() { ClientId = clientId, Headers = headers, - Ip = request.UserHostAddress + Ip = ip }; } #endif #if NETSTANDARD2_0 - public static RequestContext FromHttpRequest(Microsoft.AspNetCore.Http.HttpRequest request) + public static RequestContext FromHttpRequest(Microsoft.AspNetCore.Http.HttpRequest request, string ipHeader = null) { return new RequestContext() { ClientId = GetClientId(request.Headers, request.Cookies), Headers = request.Headers.ToDictionary(x => x.Key, y => y.Value.FirstOrDefault()), - Ip = request.HttpContext.Connection.RemoteIpAddress.ToString(), + Ip = GetIp(() => request.HttpContext.Connection.RemoteIpAddress.ToString(), request.Headers, ipHeader) }; } @@ -46,7 +50,17 @@ public static RequestContext FromHttpRequest(Microsoft.AspNetCore.Http.HttpReque { return headers.TryGetValue("X-Castle-Client-ID", out var headerId) ? headerId.First() - : cookies["__cid"]; + : cookies["__cid"] ?? ""; + } + + internal static string GetIp( + Func getIpFromHttpContext, + IDictionary headers, + string ipHeader) + { + return ipHeader != null && headers.TryGetValue(ipHeader, out var headerValues) + ? headerValues.First() + : getIpFromHttpContext(); } #endif } diff --git a/src/Tests/When_creating_request_context.cs b/src/Tests/When_creating_request_context.cs index 0fd4cc4..25a714f 100644 --- a/src/Tests/When_creating_request_context.cs +++ b/src/Tests/When_creating_request_context.cs @@ -13,13 +13,31 @@ public class When_creating_request_context [Theory, AutoData] public void Should_get_client_id_from_castle_header_if_present( string castleHeaderValue, + string cookieValue) + { + var headers = new Dictionary() + { + ["X-Castle-Client-ID"] = castleHeaderValue, + }; + + var cookies = new RequestCookieCollection(new Dictionary() + { + ["__cid"] = cookieValue + }); + + var result = Context.GetClientId(headers, cookies); + + result.Should().Be(castleHeaderValue); + } + + [Theory, AutoData] + public void Should_get_client_id_from_cookie_if_castle_header_not_present( string otherHeader, string otherHeaderValue, string cookieValue) { var headers = new Dictionary() { - ["X-Castle-Client-ID"] = castleHeaderValue, [otherHeader] = otherHeaderValue }; @@ -30,7 +48,67 @@ public class When_creating_request_context var result = Context.GetClientId(headers, cookies); - result.Should().Be(castleHeaderValue); + result.Should().Be(cookieValue); + } + + [Theory, AutoData] + public void Should_use_empty_string_if_unable_to_get_client_id( + string otherHeader, + string otherHeaderValue, + string otherCookie, + string otherCookieValue) + { + var headers = new Dictionary() + { + [otherHeader] = otherHeaderValue + }; + + var cookies = new RequestCookieCollection(new Dictionary() + { + [otherCookie] = otherCookieValue + }); + + var result = Context.GetClientId(headers, cookies); + + result.Should().Be(""); + } + + [Theory, AutoData] + public void Should_get_ip_from_supplied_header( + string ipHeader, + string ip, + string otherHeader, + string otherHeaderValue, + string httpContextIp) + { + var headers = new Dictionary() + { + [ipHeader] = ip, + [otherHeader] = otherHeaderValue + }; + + var result = Context.GetIp(() => httpContextIp, headers, ipHeader); + + result.Should().Be(ip); + } + + [Theory, AutoData] + public void Should_get_ip_from_httpcontext_if_no_header_supplied( + string ipHeader, + string ip, + string otherHeader, + string otherHeaderValue, + string httpContextIp) + { + var headers = new Dictionary() + { + [ipHeader] = ip, + [otherHeader] = otherHeaderValue + }; + + var result = Context.GetIp(() => httpContextIp, headers, null); + + result.Should().Be(httpContextIp); } } } From 85c9bf1a1116780a7f870e899fba3eed46528437 Mon Sep 17 00:00:00 2001 From: Magnus Nilsson Date: Wed, 20 Mar 2019 10:42:10 +0100 Subject: [PATCH 3/3] Get context IP from header list --- src/Castle.Sdk/Context.cs | 53 +++++--- ...When_creating_request_context_for_Core.cs} | 39 ++++-- ..._creating_request_context_for_Framework.cs | 126 ++++++++++++++++++ 3 files changed, 193 insertions(+), 25 deletions(-) rename src/Tests/{When_creating_request_context.cs => Messages/When_creating_request_context_for_Core.cs} (67%) create mode 100644 src/Tests/Messages/When_creating_request_context_for_Framework.cs diff --git a/src/Castle.Sdk/Context.cs b/src/Castle.Sdk/Context.cs index d4fae3d..62bf888 100644 --- a/src/Castle.Sdk/Context.cs +++ b/src/Castle.Sdk/Context.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.Specialized; using System.Linq; using Castle.Messages.Requests; @@ -8,7 +9,7 @@ namespace Castle public static class Context { #if NET461 - public static RequestContext FromHttpRequest(System.Web.HttpRequestBase request, string ipHeader = null) + public static RequestContext FromHttpRequest(System.Web.HttpRequestBase request, string[] ipHeaders = null) { var headers = new Dictionary(); foreach (string key in request.Headers.Keys) @@ -16,13 +17,9 @@ public static RequestContext FromHttpRequest(System.Web.HttpRequestBase request, headers.Add(key, request.Headers[key]); } - var clientId = request.Headers.AllKeys.Contains("X-Castle-Client-ID", StringComparer.OrdinalIgnoreCase) - ? request.Headers["X-Castle-Client-ID"] - : request.Cookies["__cid"]?.Value ?? ""; + var clientId = GetClientIdForFramework(request.Headers, name => request.Cookies[name]?.Value); - var ip = ipHeader != null && request.Headers.AllKeys.Contains(ipHeader, StringComparer.OrdinalIgnoreCase) - ? request.Headers[ipHeader] - : request.UserHostAddress; + var ip = GetIpForFramework(request.Headers, ipHeaders, () => request.UserHostAddress); return new RequestContext() { @@ -33,18 +30,36 @@ public static RequestContext FromHttpRequest(System.Web.HttpRequestBase request, } #endif + internal static string GetClientIdForFramework(NameValueCollection headers, Func getCookieValue) + { + return headers.AllKeys.Contains("X-Castle-Client-ID", StringComparer.OrdinalIgnoreCase) + ? headers["X-Castle-Client-ID"] + : getCookieValue("__cid") ?? ""; + } + + internal static string GetIpForFramework(NameValueCollection headers, string[] ipHeaders, Func getIpFromHttpContext) + { + foreach (var header in ipHeaders ?? new string[] { }) + { + if (headers.AllKeys.Contains(header, StringComparer.OrdinalIgnoreCase)) + return headers[header]; + } + + return getIpFromHttpContext(); + } + #if NETSTANDARD2_0 - public static RequestContext FromHttpRequest(Microsoft.AspNetCore.Http.HttpRequest request, string ipHeader = null) + public static RequestContext FromHttpRequest(Microsoft.AspNetCore.Http.HttpRequest request, string[] ipHeaders = null) { return new RequestContext() { - ClientId = GetClientId(request.Headers, request.Cookies), + ClientId = GetClientIdForCore(request.Headers, request.Cookies), Headers = request.Headers.ToDictionary(x => x.Key, y => y.Value.FirstOrDefault()), - Ip = GetIp(() => request.HttpContext.Connection.RemoteIpAddress.ToString(), request.Headers, ipHeader) + Ip = GetIpForCore(request.Headers, ipHeaders, () => request.HttpContext.Connection.RemoteIpAddress.ToString()) }; } - internal static string GetClientId( + internal static string GetClientIdForCore( IDictionary headers, Microsoft.AspNetCore.Http.IRequestCookieCollection cookies) { @@ -53,14 +68,18 @@ public static RequestContext FromHttpRequest(Microsoft.AspNetCore.Http.HttpReque : cookies["__cid"] ?? ""; } - internal static string GetIp( - Func getIpFromHttpContext, + internal static string GetIpForCore( IDictionary headers, - string ipHeader) + string[] ipHeaders, + Func getIpFromHttpContext) { - return ipHeader != null && headers.TryGetValue(ipHeader, out var headerValues) - ? headerValues.First() - : getIpFromHttpContext(); + foreach (var header in ipHeaders ?? new string[] {}) + { + if (headers.TryGetValue(header, out var headerValues)) + return headerValues.First(); + } + + return getIpFromHttpContext(); } #endif } diff --git a/src/Tests/When_creating_request_context.cs b/src/Tests/Messages/When_creating_request_context_for_Core.cs similarity index 67% rename from src/Tests/When_creating_request_context.cs rename to src/Tests/Messages/When_creating_request_context_for_Core.cs index 25a714f..7285bb0 100644 --- a/src/Tests/When_creating_request_context.cs +++ b/src/Tests/Messages/When_creating_request_context_for_Core.cs @@ -6,9 +6,9 @@ using Microsoft.Extensions.Primitives; using Xunit; -namespace Tests +namespace Tests.Messages { - public class When_creating_request_context + public class When_creating_request_context_for_Core { [Theory, AutoData] public void Should_get_client_id_from_castle_header_if_present( @@ -25,7 +25,7 @@ public class When_creating_request_context ["__cid"] = cookieValue }); - var result = Context.GetClientId(headers, cookies); + var result = Context.GetClientIdForCore(headers, cookies); result.Should().Be(castleHeaderValue); } @@ -46,7 +46,7 @@ public class When_creating_request_context ["__cid"] = cookieValue }); - var result = Context.GetClientId(headers, cookies); + var result = Context.GetClientIdForCore(headers, cookies); result.Should().Be(cookieValue); } @@ -68,15 +68,17 @@ public class When_creating_request_context [otherCookie] = otherCookieValue }); - var result = Context.GetClientId(headers, cookies); + var result = Context.GetClientIdForCore(headers, cookies); result.Should().Be(""); } [Theory, AutoData] - public void Should_get_ip_from_supplied_header( + public void Should_get_ip_from_supplied_headers_in_order( string ipHeader, string ip, + string secondaryIpHeader, + string secondaryIp, string otherHeader, string otherHeaderValue, string httpContextIp) @@ -84,14 +86,35 @@ public class When_creating_request_context var headers = new Dictionary() { [ipHeader] = ip, + [secondaryIpHeader] = secondaryIp, [otherHeader] = otherHeaderValue }; - var result = Context.GetIp(() => httpContextIp, headers, ipHeader); + var result = Context.GetIpForCore(headers, new [] { ipHeader, secondaryIpHeader }, () => httpContextIp); result.Should().Be(ip); } + [Theory, AutoData] + public void Should_get_ip_from_second_header_if_first_is_not_found( + string ipHeader, + string secondaryIpHeader, + string secondaryIp, + string otherHeader, + string otherHeaderValue, + string httpContextIp) + { + var headers = new Dictionary() + { + [secondaryIpHeader] = secondaryIp, + [otherHeader] = otherHeaderValue + }; + + var result = Context.GetIpForCore(headers, new[] { ipHeader, secondaryIpHeader }, () => httpContextIp); + + result.Should().Be(secondaryIp); + } + [Theory, AutoData] public void Should_get_ip_from_httpcontext_if_no_header_supplied( string ipHeader, @@ -106,7 +129,7 @@ public class When_creating_request_context [otherHeader] = otherHeaderValue }; - var result = Context.GetIp(() => httpContextIp, headers, null); + var result = Context.GetIpForCore(headers, null, () => httpContextIp); result.Should().Be(httpContextIp); } diff --git a/src/Tests/Messages/When_creating_request_context_for_Framework.cs b/src/Tests/Messages/When_creating_request_context_for_Framework.cs new file mode 100644 index 0000000..9502a00 --- /dev/null +++ b/src/Tests/Messages/When_creating_request_context_for_Framework.cs @@ -0,0 +1,126 @@ +using System.Collections.Specialized; +using AutoFixture.Xunit2; +using Castle; +using FluentAssertions; +using Xunit; + +namespace Tests.Messages +{ + public class When_creating_request_context_for_Framework + { + [Theory, AutoData] + public void Should_get_client_id_from_castle_header_if_present( + string castleHeaderValue, + string cookieValue) + { + var headers = new NameValueCollection + { + ["X-Castle-Client-ID"] = castleHeaderValue + }; + + string GetCookie(string name) => name == "__cid" ? cookieValue : null; + + var result = Context.GetClientIdForFramework(headers, GetCookie); + + result.Should().Be(castleHeaderValue); + } + + [Theory, AutoData] + public void Should_get_client_id_from_cookie_if_castle_header_not_present( + string otherHeader, + string otherHeaderValue, + string cookieValue) + { + var headers = new NameValueCollection + { + [otherHeader] = otherHeaderValue + }; + + string GetCookie(string name) => name == "__cid" ? cookieValue : null; + + var result = Context.GetClientIdForFramework(headers, GetCookie); + + result.Should().Be(cookieValue); + } + + [Theory, AutoData] + public void Should_use_empty_string_if_unable_to_get_client_id( + string otherHeader, + string otherHeaderValue, + string otherCookie, + string otherCookieValue) + { + var headers = new NameValueCollection + { + [otherHeader] = otherHeaderValue + }; + + string GetCookie(string name) => name == otherCookie ? otherCookieValue : null; + + var result = Context.GetClientIdForFramework(headers, GetCookie); + + result.Should().Be(""); + } + + [Theory, AutoData] + public void Should_get_ip_from_supplied_headers_in_order( + string ipHeader, + string ip, + string secondaryIpHeader, + string secondaryIp, + string otherHeader, + string otherHeaderValue, + string httpContextIp) + { + var headers = new NameValueCollection + { + [ipHeader] = ip, + [secondaryIpHeader] = secondaryIp, + [otherHeader] = otherHeaderValue + }; + + var result = Context.GetIpForFramework(headers, new [] { ipHeader, secondaryIpHeader }, () => httpContextIp); + + result.Should().Be(ip); + } + + [Theory, AutoData] + public void Should_get_ip_from_second_header_if_first_is_not_found( + string ipHeader, + string secondaryIpHeader, + string secondaryIp, + string otherHeader, + string otherHeaderValue, + string httpContextIp) + { + var headers = new NameValueCollection + { + [secondaryIpHeader] = secondaryIp, + [otherHeader] = otherHeaderValue + }; + + var result = Context.GetIpForFramework(headers, new[] { ipHeader, secondaryIpHeader }, () => httpContextIp); + + result.Should().Be(secondaryIp); + } + + [Theory, AutoData] + public void Should_get_ip_from_httpcontext_if_no_header_supplied( + string ipHeader, + string ip, + string otherHeader, + string otherHeaderValue, + string httpContextIp) + { + var headers = new NameValueCollection + { + [ipHeader] = ip, + [otherHeader] = otherHeaderValue + }; + + var result = Context.GetIpForFramework(headers, null, () => httpContextIp); + + result.Should().Be(httpContextIp); + } + } +}