From 2634fe318d8b81a3fc6e7e67a2dca980fb9bcc69 Mon Sep 17 00:00:00 2001 From: Chris R Date: Tue, 24 May 2016 13:53:57 -0700 Subject: [PATCH] #814 Rework CookieAuth for compat with CookiePolicy. --- .../ChunkingCookieManager.cs | 114 ++----- .../CookieAuthenticationMiddleware.cs | 2 +- .../ChunkingCookieManager.cs | 281 ++++++++++++++++++ .../Constants.cs | 13 + .../Infrastructure/CookieChunkingTests.cs | 68 +---- .../CookiePolicyTests.cs | 99 ++++++ .../project.json | 1 + .../CookieInteropTests.cs | 147 ++++++++- 8 files changed, 569 insertions(+), 156 deletions(-) create mode 100644 src/Microsoft.Owin.Security.Interop/ChunkingCookieManager.cs create mode 100644 src/Microsoft.Owin.Security.Interop/Constants.cs diff --git a/src/Microsoft.AspNetCore.Authentication.Cookies/ChunkingCookieManager.cs b/src/Microsoft.AspNetCore.Authentication.Cookies/ChunkingCookieManager.cs index 380e6f9374..c7bff38d1f 100644 --- a/src/Microsoft.AspNetCore.Authentication.Cookies/ChunkingCookieManager.cs +++ b/src/Microsoft.AspNetCore.Authentication.Cookies/ChunkingCookieManager.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Globalization; using System.Linq; -using System.Text.Encodings.Web; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; @@ -18,13 +17,16 @@ namespace Microsoft.AspNetCore.Authentication.Cookies /// public class ChunkingCookieManager : ICookieManager { - public ChunkingCookieManager(UrlEncoder urlEncoder) + private const string ChunkKeySuffix = "C"; + private const string ChunkCountPrefix = "chunks-"; + + public ChunkingCookieManager() { // Lowest common denominator. Safari has the lowest known limit (4093), and we leave little extra just in case. // See http://browsercookielimits.x64.me/. - ChunkSize = 4090; + // Leave at least 20 in case CookiePolicy tries to add 'secure' and/or 'httponly'. + ChunkSize = 4070; ThrowForPartialCookies = true; - Encoder = urlEncoder ?? UrlEncoder.Default; } /// @@ -41,14 +43,12 @@ namespace Microsoft.AspNetCore.Authentication.Cookies /// public bool ThrowForPartialCookies { get; set; } - private UrlEncoder Encoder { get; set; } - - // Parse the "chunks:XX" to determine how many chunks there should be. + // Parse the "chunks-XX" to determine how many chunks there should be. private static int ParseChunksCount(string value) { - if (value != null && value.StartsWith("chunks:", StringComparison.Ordinal)) + if (value != null && value.StartsWith(ChunkCountPrefix, StringComparison.Ordinal)) { - var chunksCountString = value.Substring("chunks:".Length); + var chunksCountString = value.Substring(ChunkCountPrefix.Length); int chunksCount; if (int.TryParse(chunksCountString, NumberStyles.None, CultureInfo.InvariantCulture, out chunksCount)) { @@ -60,7 +60,7 @@ namespace Microsoft.AspNetCore.Authentication.Cookies /// /// Get the reassembled cookie. Non chunked cookies are returned normally. - /// Cookies with missing chunks just have their "chunks:XX" header returned. + /// Cookies with missing chunks just have their "chunks-XX" header returned. /// /// /// @@ -82,11 +82,10 @@ namespace Microsoft.AspNetCore.Authentication.Cookies var chunksCount = ParseChunksCount(value); if (chunksCount > 0) { - var quoted = false; var chunks = new string[chunksCount]; for (var chunkId = 1; chunkId <= chunksCount; chunkId++) { - var chunk = requestCookies[key + "C" + chunkId.ToString(CultureInfo.InvariantCulture)]; + var chunk = requestCookies[key + ChunkKeySuffix + chunkId.ToString(CultureInfo.InvariantCulture)]; if (string.IsNullOrEmpty(chunk)) { if (ThrowForPartialCookies) @@ -102,20 +101,11 @@ namespace Microsoft.AspNetCore.Authentication.Cookies // Missing chunk, abort by returning the original cookie value. It may have been a false positive? return value; } - if (IsQuoted(chunk)) - { - // Note: Since we assume these cookies were generated by our code, then we can assume that if one cookie has quotes then they all do. - quoted = true; - chunk = RemoveQuotes(chunk); - } + chunks[chunkId - 1] = chunk; } - var merged = string.Join(string.Empty, chunks); - if (quoted) - { - merged = Quote(merged); - } - return merged; + + return string.Join(string.Empty, chunks); } return value; } @@ -123,7 +113,7 @@ namespace Microsoft.AspNetCore.Authentication.Cookies /// /// Appends a new response cookie to the Set-Cookie header. If the cookie is larger than the given size limit /// then it will be broken down into multiple cookies as follows: - /// Set-Cookie: CookieName=chunks:3; path=/ + /// Set-Cookie: CookieName=chunks-3; path=/ /// Set-Cookie: CookieNameC1=Segment1; path=/ /// Set-Cookie: CookieNameC2=Segment2; path=/ /// Set-Cookie: CookieNameC3=Segment3; path=/ @@ -149,9 +139,7 @@ namespace Microsoft.AspNetCore.Authentication.Cookies throw new ArgumentNullException(nameof(options)); } - var escapedKey = Encoder.Encode(key); - - var template = new SetCookieHeaderValue(escapedKey) + var template = new SetCookieHeaderValue(key) { Domain = options.Domain, Expires = options.Expires, @@ -163,22 +151,14 @@ namespace Microsoft.AspNetCore.Authentication.Cookies var templateLength = template.ToString().Length; value = value ?? string.Empty; - var quoted = false; - if (IsQuoted(value)) - { - quoted = true; - value = RemoveQuotes(value); - } - var escapedValue = Encoder.Encode(value); // Normal cookie - var responseHeaders = context.Response.Headers; - if (!ChunkSize.HasValue || ChunkSize.Value > templateLength + escapedValue.Length + (quoted ? 2 : 0)) + var responseCookies = context.Response.Cookies; + if (!ChunkSize.HasValue || ChunkSize.Value > templateLength + value.Length) { - template.Value = quoted ? Quote(escapedValue) : escapedValue; - responseHeaders.Append(Constants.Headers.SetCookie, template.ToString()); + responseCookies.Append(key, value, options); } - else if (ChunkSize.Value < templateLength + (quoted ? 2 : 0) + 10) + else if (ChunkSize.Value < templateLength + 10) { // 10 is the minimum data we want to put in an individual cookie, including the cookie chunk identifier "CXX". // No room for data, we can't chunk the options and name @@ -188,30 +168,25 @@ namespace Microsoft.AspNetCore.Authentication.Cookies { // Break the cookie down into multiple cookies. // Key = CookieName, value = "Segment1Segment2Segment2" - // Set-Cookie: CookieName=chunks:3; path=/ + // Set-Cookie: CookieName=chunks-3; path=/ // Set-Cookie: CookieNameC1="Segment1"; path=/ // Set-Cookie: CookieNameC2="Segment2"; path=/ // Set-Cookie: CookieNameC3="Segment3"; path=/ - var dataSizePerCookie = ChunkSize.Value - templateLength - (quoted ? 2 : 0) - 3; // Budget 3 chars for the chunkid. - var cookieChunkCount = (int)Math.Ceiling(escapedValue.Length * 1.0 / dataSizePerCookie); + var dataSizePerCookie = ChunkSize.Value - templateLength - 3; // Budget 3 chars for the chunkid. + var cookieChunkCount = (int)Math.Ceiling(value.Length * 1.0 / dataSizePerCookie); - template.Value = "chunks:" + cookieChunkCount.ToString(CultureInfo.InvariantCulture); - responseHeaders.Append(Constants.Headers.SetCookie, template.ToString()); + responseCookies.Append(key, ChunkCountPrefix + cookieChunkCount.ToString(CultureInfo.InvariantCulture), options); - var chunks = new string[cookieChunkCount]; var offset = 0; for (var chunkId = 1; chunkId <= cookieChunkCount; chunkId++) { - var remainingLength = escapedValue.Length - offset; + var remainingLength = value.Length - offset; var length = Math.Min(dataSizePerCookie, remainingLength); - var segment = escapedValue.Substring(offset, length); + var segment = value.Substring(offset, length); offset += length; - template.Name = escapedKey + "C" + chunkId.ToString(CultureInfo.InvariantCulture); - template.Value = quoted ? Quote(segment) : segment; - chunks[chunkId - 1] = template.ToString(); + responseCookies.Append(key + ChunkKeySuffix + chunkId.ToString(CultureInfo.InvariantCulture), segment, options); } - responseHeaders.Append(Constants.Headers.SetCookie, chunks); } } @@ -239,9 +214,8 @@ namespace Microsoft.AspNetCore.Authentication.Cookies throw new ArgumentNullException(nameof(options)); } - var escapedKey = Encoder.Encode(key); var keys = new List(); - keys.Add(escapedKey + "="); + keys.Add(key + "="); var requestCookie = context.Request.Cookies[key]; var chunks = ParseChunksCount(requestCookie); @@ -249,7 +223,7 @@ namespace Microsoft.AspNetCore.Authentication.Cookies { for (int i = 1; i <= chunks + 1; i++) { - var subkey = escapedKey + "C" + i.ToString(CultureInfo.InvariantCulture); + var subkey = key + ChunkKeySuffix + i.ToString(CultureInfo.InvariantCulture); keys.Add(subkey + "="); } } @@ -304,35 +278,5 @@ namespace Microsoft.AspNetCore.Authentication.Cookies }); } } - - private static bool IsQuoted(string value) - { - if (value == null) - { - throw new ArgumentNullException(nameof(value)); - } - - return value.Length >= 2 && value[0] == '"' && value[value.Length - 1] == '"'; - } - - private static string RemoveQuotes(string value) - { - if (value == null) - { - throw new ArgumentNullException(nameof(value)); - } - - return value.Substring(1, value.Length - 2); - } - - private static string Quote(string value) - { - if (value == null) - { - throw new ArgumentNullException(nameof(value)); - } - - return '"' + value + '"'; - } } } diff --git a/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationMiddleware.cs b/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationMiddleware.cs index ff54957cfe..14d152a818 100644 --- a/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationMiddleware.cs +++ b/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationMiddleware.cs @@ -42,7 +42,7 @@ namespace Microsoft.AspNetCore.Authentication.Cookies } if (Options.CookieManager == null) { - Options.CookieManager = new ChunkingCookieManager(urlEncoder); + Options.CookieManager = new ChunkingCookieManager(); } if (!Options.LoginPath.HasValue) { diff --git a/src/Microsoft.Owin.Security.Interop/ChunkingCookieManager.cs b/src/Microsoft.Owin.Security.Interop/ChunkingCookieManager.cs new file mode 100644 index 0000000000..b323258d9b --- /dev/null +++ b/src/Microsoft.Owin.Security.Interop/ChunkingCookieManager.cs @@ -0,0 +1,281 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using Microsoft.Owin.Infrastructure; + +namespace Microsoft.Owin.Security.Interop +{ + // This MUST be kept in sync with Microsoft.AspNetCore.Authentication.Cookies.ChunkingCookieManager + /// + /// This handles cookies that are limited by per cookie length. It breaks down long cookies for responses, and reassembles them + /// from requests. + /// + public class ChunkingCookieManager : ICookieManager + { + private const string ChunkKeySuffix = "C"; + private const string ChunkCountPrefix = "chunks-"; + + public ChunkingCookieManager() + { + // Lowest common denominator. Safari has the lowest known limit (4093), and we leave little extra just in case. + // See http://browsercookielimits.x64.me/. + // Leave at least 20 in case CookiePolicy tries to add 'secure' and/or 'httponly'. + ChunkSize = 4070; + ThrowForPartialCookies = true; + } + + /// + /// The maximum size of cookie to send back to the client. If a cookie exceeds this size it will be broken down into multiple + /// cookies. Set this value to null to disable this behavior. The default is 4090 characters, which is supported by all + /// common browsers. + /// + /// Note that browsers may also have limits on the total size of all cookies per domain, and on the number of cookies per domain. + /// + public int? ChunkSize { get; set; } + + /// + /// Throw if not all chunks of a cookie are available on a request for re-assembly. + /// + public bool ThrowForPartialCookies { get; set; } + + // Parse the "chunks-XX" to determine how many chunks there should be. + private static int ParseChunksCount(string value) + { + if (value != null && value.StartsWith(ChunkCountPrefix, StringComparison.Ordinal)) + { + var chunksCountString = value.Substring(ChunkCountPrefix.Length); + int chunksCount; + if (int.TryParse(chunksCountString, NumberStyles.None, CultureInfo.InvariantCulture, out chunksCount)) + { + return chunksCount; + } + } + return 0; + } + + /// + /// Get the reassembled cookie. Non chunked cookies are returned normally. + /// Cookies with missing chunks just have their "chunks-XX" header returned. + /// + /// + /// + /// The reassembled cookie, if any, or null. + public string GetRequestCookie(IOwinContext context, string key) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (key == null) + { + throw new ArgumentNullException(nameof(key)); + } + + var requestCookies = context.Request.Cookies; + var value = requestCookies[key]; + var chunksCount = ParseChunksCount(value); + if (chunksCount > 0) + { + var chunks = new string[chunksCount]; + for (var chunkId = 1; chunkId <= chunksCount; chunkId++) + { + var chunk = requestCookies[key + ChunkKeySuffix + chunkId.ToString(CultureInfo.InvariantCulture)]; + if (string.IsNullOrEmpty(chunk)) + { + if (ThrowForPartialCookies) + { + var totalSize = 0; + for (int i = 0; i < chunkId - 1; i++) + { + totalSize += chunks[i].Length; + } + throw new FormatException( + string.Format(CultureInfo.CurrentCulture, + "The chunked cookie is incomplete. Only {0} of the expected {1} chunks were found, totaling {2} characters. A client size limit may have been exceeded.", + chunkId - 1, chunksCount, totalSize)); + } + // Missing chunk, abort by returning the original cookie value. It may have been a false positive? + return value; + } + + chunks[chunkId - 1] = chunk; + } + + return string.Join(string.Empty, chunks); + } + return value; + } + + /// + /// Appends a new response cookie to the Set-Cookie header. If the cookie is larger than the given size limit + /// then it will be broken down into multiple cookies as follows: + /// Set-Cookie: CookieName=chunks-3; path=/ + /// Set-Cookie: CookieNameC1=Segment1; path=/ + /// Set-Cookie: CookieNameC2=Segment2; path=/ + /// Set-Cookie: CookieNameC3=Segment3; path=/ + /// + /// + /// + /// + /// + public void AppendResponseCookie(IOwinContext context, string key, string value, CookieOptions options) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (key == null) + { + throw new ArgumentNullException(nameof(key)); + } + + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + + var domainHasValue = !string.IsNullOrEmpty(options.Domain); + var pathHasValue = !string.IsNullOrEmpty(options.Path); + var expiresHasValue = options.Expires.HasValue; + + var templateLength = key.Length + "=".Length + + (domainHasValue ? "; domain=".Length + options.Domain.Length : 0) + + (pathHasValue ? "; path=".Length + options.Path.Length : 0) + + (expiresHasValue ? "; expires=ddd, dd-MMM-yyyy HH:mm:ss GMT".Length : 0) + + (options.Secure ? "; secure".Length : 0) + + (options.HttpOnly ? "; HttpOnly".Length : 0); + + // Normal cookie + var responseCookies = context.Response.Cookies; + if (!ChunkSize.HasValue || ChunkSize.Value > templateLength + value.Length) + { + responseCookies.Append(key, value, options); + } + else if (ChunkSize.Value < templateLength + 10) + { + // 10 is the minimum data we want to put in an individual cookie, including the cookie chunk identifier "CXX". + // No room for data, we can't chunk the options and name + throw new InvalidOperationException("The cookie key and options are larger than ChunksSize, leaving no room for data."); + } + else + { + // Break the cookie down into multiple cookies. + // Key = CookieName, value = "Segment1Segment2Segment2" + // Set-Cookie: CookieName=chunks-3; path=/ + // Set-Cookie: CookieNameC1="Segment1"; path=/ + // Set-Cookie: CookieNameC2="Segment2"; path=/ + // Set-Cookie: CookieNameC3="Segment3"; path=/ + var dataSizePerCookie = ChunkSize.Value - templateLength - 3; // Budget 3 chars for the chunkid. + var cookieChunkCount = (int)Math.Ceiling(value.Length * 1.0 / dataSizePerCookie); + + responseCookies.Append(key, ChunkCountPrefix + cookieChunkCount.ToString(CultureInfo.InvariantCulture), options); + + var offset = 0; + for (var chunkId = 1; chunkId <= cookieChunkCount; chunkId++) + { + var remainingLength = value.Length - offset; + var length = Math.Min(dataSizePerCookie, remainingLength); + var segment = value.Substring(offset, length); + offset += length; + + responseCookies.Append(key + ChunkKeySuffix + chunkId.ToString(CultureInfo.InvariantCulture), segment, options); + } + } + } + + /// + /// Deletes the cookie with the given key by setting an expired state. If a matching chunked cookie exists on + /// the request, delete each chunk. + /// + /// + /// + /// + public void DeleteCookie(IOwinContext context, string key, CookieOptions options) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (key == null) + { + throw new ArgumentNullException(nameof(key)); + } + + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + + var keys = new List(); + keys.Add(key + "="); + + var requestCookie = context.Request.Cookies[key]; + var chunks = ParseChunksCount(requestCookie); + if (chunks > 0) + { + for (int i = 1; i <= chunks + 1; i++) + { + var subkey = key + ChunkKeySuffix + i.ToString(CultureInfo.InvariantCulture); + keys.Add(subkey + "="); + } + } + + var domainHasValue = !string.IsNullOrEmpty(options.Domain); + var pathHasValue = !string.IsNullOrEmpty(options.Path); + + Func rejectPredicate; + Func predicate = value => keys.Any(k => value.StartsWith(k, StringComparison.OrdinalIgnoreCase)); + if (domainHasValue) + { + rejectPredicate = value => predicate(value) && value.IndexOf("domain=" + options.Domain, StringComparison.OrdinalIgnoreCase) != -1; + } + else if (pathHasValue) + { + rejectPredicate = value => predicate(value) && value.IndexOf("path=" + options.Path, StringComparison.OrdinalIgnoreCase) != -1; + } + else + { + rejectPredicate = value => predicate(value); + } + + var responseHeaders = context.Response.Headers; + string[] existingValues; + if (responseHeaders.TryGetValue(Constants.Headers.SetCookie, out existingValues) && existingValues != null) + { + responseHeaders.SetValues(Constants.Headers.SetCookie, existingValues.Where(value => !rejectPredicate(value)).ToArray()); + } + + AppendResponseCookie( + context, + key, + string.Empty, + new CookieOptions() + { + Path = options.Path, + Domain = options.Domain, + Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc), + }); + + for (int i = 1; i <= chunks; i++) + { + AppendResponseCookie( + context, + key + "C" + i.ToString(CultureInfo.InvariantCulture), + string.Empty, + new CookieOptions() + { + Path = options.Path, + Domain = options.Domain, + Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc), + }); + } + } + } +} diff --git a/src/Microsoft.Owin.Security.Interop/Constants.cs b/src/Microsoft.Owin.Security.Interop/Constants.cs new file mode 100644 index 0000000000..1e75761b70 --- /dev/null +++ b/src/Microsoft.Owin.Security.Interop/Constants.cs @@ -0,0 +1,13 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.Owin.Security.Interop +{ + internal static class Constants + { + internal static class Headers + { + internal const string SetCookie = "Set-Cookie"; + } + } +} diff --git a/test/Microsoft.AspNetCore.Authentication.Test/Cookies/Infrastructure/CookieChunkingTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/Cookies/Infrastructure/CookieChunkingTests.cs index 4b92f08c8d..670baf5db4 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/Cookies/Infrastructure/CookieChunkingTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/Cookies/Infrastructure/CookieChunkingTests.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Authentication.Cookies.Infrastructure HttpContext context = new DefaultHttpContext(); string testString = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; - new ChunkingCookieManager(null) { ChunkSize = null }.AppendResponseCookie(context, "TestCookie", testString, new CookieOptions()); + new ChunkingCookieManager() { ChunkSize = null }.AppendResponseCookie(context, "TestCookie", testString, new CookieOptions()); var values = context.Response.Headers["Set-Cookie"]; Assert.Equal(1, values.Count); Assert.Equal("TestCookie=" + testString + "; path=/", values[0]); @@ -27,12 +27,12 @@ namespace Microsoft.AspNetCore.Authentication.Cookies.Infrastructure HttpContext context = new DefaultHttpContext(); string testString = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; - new ChunkingCookieManager(null) { ChunkSize = 30 }.AppendResponseCookie(context, "TestCookie", testString, new CookieOptions()); + new ChunkingCookieManager() { ChunkSize = 30 }.AppendResponseCookie(context, "TestCookie", testString, new CookieOptions()); var values = context.Response.Headers["Set-Cookie"]; Assert.Equal(9, values.Count); Assert.Equal(new[] { - "TestCookie=chunks:8; path=/", + "TestCookie=chunks-8; path=/", "TestCookieC1=abcdefgh; path=/", "TestCookieC2=ijklmnop; path=/", "TestCookieC3=qrstuvwx; path=/", @@ -44,36 +44,13 @@ namespace Microsoft.AspNetCore.Authentication.Cookies.Infrastructure }, values); } - [Fact] - public void AppendLargeQuotedCookieWithLimit_QuotedChunked() - { - HttpContext context = new DefaultHttpContext(); - - string testString = "\"abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ\""; - new ChunkingCookieManager(null) { ChunkSize = 32 }.AppendResponseCookie(context, "TestCookie", testString, new CookieOptions()); - var values = context.Response.Headers["Set-Cookie"]; - Assert.Equal(9, values.Count); - Assert.Equal(new[] - { - "TestCookie=chunks:8; path=/", - "TestCookieC1=\"abcdefgh\"; path=/", - "TestCookieC2=\"ijklmnop\"; path=/", - "TestCookieC3=\"qrstuvwx\"; path=/", - "TestCookieC4=\"yz012345\"; path=/", - "TestCookieC5=\"6789ABCD\"; path=/", - "TestCookieC6=\"EFGHIJKL\"; path=/", - "TestCookieC7=\"MNOPQRST\"; path=/", - "TestCookieC8=\"UVWXYZ\"; path=/", - }, values); - } - [Fact] public void GetLargeChunkedCookie_Reassembled() { HttpContext context = new DefaultHttpContext(); context.Request.Headers["Cookie"] = new[] { - "TestCookie=chunks:7", + "TestCookie=chunks-7", "TestCookieC1=abcdefghi", "TestCookieC2=jklmnopqr", "TestCookieC3=stuvwxyz0", @@ -83,39 +60,18 @@ namespace Microsoft.AspNetCore.Authentication.Cookies.Infrastructure "TestCookieC7=STUVWXYZ" }; - string result = new ChunkingCookieManager(null).GetRequestCookie(context, "TestCookie"); + string result = new ChunkingCookieManager().GetRequestCookie(context, "TestCookie"); string testString = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; Assert.Equal(testString, result); } - [Fact] - public void GetLargeChunkedCookieWithQuotes_Reassembled() - { - HttpContext context = new DefaultHttpContext(); - context.Request.Headers["Cookie"] = new[] - { - "TestCookie=chunks:7", - "TestCookieC1=\"abcdefghi\"", - "TestCookieC2=\"jklmnopqr\"", - "TestCookieC3=\"stuvwxyz0\"", - "TestCookieC4=\"123456789\"", - "TestCookieC5=\"ABCDEFGHI\"", - "TestCookieC6=\"JKLMNOPQR\"", - "TestCookieC7=\"STUVWXYZ\"" - }; - - string result = new ChunkingCookieManager(null).GetRequestCookie(context, "TestCookie"); - string testString = "\"abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ\""; - Assert.Equal(testString, result); - } - [Fact] public void GetLargeChunkedCookieWithMissingChunk_ThrowingEnabled_Throws() { HttpContext context = new DefaultHttpContext(); context.Request.Headers["Cookie"] = new[] { - "TestCookie=chunks:7", + "TestCookie=chunks-7", "TestCookieC1=abcdefghi", // Missing chunk "TestCookieC2=jklmnopqr", "TestCookieC3=stuvwxyz0", @@ -125,7 +81,7 @@ namespace Microsoft.AspNetCore.Authentication.Cookies.Infrastructure "TestCookieC7=STUVWXYZ" }; - Assert.Throws(() => new ChunkingCookieManager(null).GetRequestCookie(context, "TestCookie")); + Assert.Throws(() => new ChunkingCookieManager().GetRequestCookie(context, "TestCookie")); } [Fact] @@ -134,7 +90,7 @@ namespace Microsoft.AspNetCore.Authentication.Cookies.Infrastructure HttpContext context = new DefaultHttpContext(); context.Request.Headers["Cookie"] = new[] { - "TestCookie=chunks:7", + "TestCookie=chunks-7", "TestCookieC1=abcdefghi", // Missing chunk "TestCookieC2=jklmnopqr", "TestCookieC3=stuvwxyz0", @@ -144,8 +100,8 @@ namespace Microsoft.AspNetCore.Authentication.Cookies.Infrastructure "TestCookieC7=STUVWXYZ" }; - string result = new ChunkingCookieManager(null) { ThrowForPartialCookies = false }.GetRequestCookie(context, "TestCookie"); - string testString = "chunks:7"; + string result = new ChunkingCookieManager() { ThrowForPartialCookies = false }.GetRequestCookie(context, "TestCookie"); + string testString = "chunks-7"; Assert.Equal(testString, result); } @@ -153,9 +109,9 @@ namespace Microsoft.AspNetCore.Authentication.Cookies.Infrastructure public void DeleteChunkedCookieWithOptions_AllDeleted() { HttpContext context = new DefaultHttpContext(); - context.Request.Headers.Append("Cookie", "TestCookie=chunks:7"); + context.Request.Headers.Append("Cookie", "TestCookie=chunks-7"); - new ChunkingCookieManager(null).DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com" }); + new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com" }); var cookies = context.Response.Headers["Set-Cookie"]; Assert.Equal(8, cookies.Count); Assert.Equal(new[] diff --git a/test/Microsoft.AspNetCore.CookiePolicy.Test/CookiePolicyTests.cs b/test/Microsoft.AspNetCore.CookiePolicy.Test/CookiePolicyTests.cs index f08d7fef8e..34d967617b 100644 --- a/test/Microsoft.AspNetCore.CookiePolicy.Test/CookiePolicyTests.cs +++ b/test/Microsoft.AspNetCore.CookiePolicy.Test/CookiePolicyTests.cs @@ -2,12 +2,17 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Security.Claims; +using System.Security.Principal; using System.Threading.Tasks; +using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Net.Http.Headers; using Xunit; namespace Microsoft.AspNetCore.CookiePolicy.Test @@ -228,6 +233,100 @@ namespace Microsoft.AspNetCore.CookiePolicy.Test Assert.Equal("Done", transaction.ResponseText); } + [Fact] + public async Task CookiePolicyAppliesToCookieAuth() + { + var builder = new WebHostBuilder() + .ConfigureServices(services => + { + services.AddAuthentication(); + }) + .Configure(app => + { + app.UseCookiePolicy(new CookiePolicyOptions + { + HttpOnly = HttpOnlyPolicy.Always, + Secure = CookieSecurePolicy.Always, + }); + app.UseCookieAuthentication(new CookieAuthenticationOptions() + { + CookieName = "TestCookie", + CookieHttpOnly = false, + CookieSecure = CookieSecurePolicy.None, + }); + app.Run(context => + { + return context.Authentication.SignInAsync(CookieAuthenticationDefaults.AuthenticationScheme, + new ClaimsPrincipal(new ClaimsIdentity(new GenericIdentity("TestUser", "Cookies")))); + }); + }); + var server = new TestServer(builder); + + var transaction = await server.SendAsync("http://example.com/login"); + + Assert.NotNull(transaction.SetCookie); + Assert.Equal(1, transaction.SetCookie.Count); + var cookie = SetCookieHeaderValue.Parse(transaction.SetCookie[0]); + Assert.Equal("TestCookie", cookie.Name); + Assert.True(cookie.HttpOnly); + Assert.True(cookie.Secure); + Assert.Equal("/", cookie.Path); + } + + [Fact] + public async Task CookiePolicyAppliesToCookieAuthChunks() + { + var builder = new WebHostBuilder() + .ConfigureServices(services => + { + services.AddAuthentication(); + }) + .Configure(app => + { + app.UseCookiePolicy(new CookiePolicyOptions + { + HttpOnly = HttpOnlyPolicy.Always, + Secure = CookieSecurePolicy.Always, + }); + app.UseCookieAuthentication(new CookieAuthenticationOptions() + { + CookieName = "TestCookie", + CookieHttpOnly = false, + CookieSecure = CookieSecurePolicy.None, + }); + app.Run(context => + { + return context.Authentication.SignInAsync(CookieAuthenticationDefaults.AuthenticationScheme, + new ClaimsPrincipal(new ClaimsIdentity(new GenericIdentity(new string('c', 1024 * 5), "Cookies")))); + }); + }); + var server = new TestServer(builder); + + var transaction = await server.SendAsync("http://example.com/login"); + + Assert.NotNull(transaction.SetCookie); + Assert.Equal(3, transaction.SetCookie.Count); + + var cookie = SetCookieHeaderValue.Parse(transaction.SetCookie[0]); + Assert.Equal("TestCookie", cookie.Name); + Assert.Equal("chunks-2", cookie.Value); + Assert.True(cookie.HttpOnly); + Assert.True(cookie.Secure); + Assert.Equal("/", cookie.Path); + + cookie = SetCookieHeaderValue.Parse(transaction.SetCookie[1]); + Assert.Equal("TestCookieC1", cookie.Name); + Assert.True(cookie.HttpOnly); + Assert.True(cookie.Secure); + Assert.Equal("/", cookie.Path); + + cookie = SetCookieHeaderValue.Parse(transaction.SetCookie[2]); + Assert.Equal("TestCookieC2", cookie.Name); + Assert.True(cookie.HttpOnly); + Assert.True(cookie.Secure); + Assert.Equal("/", cookie.Path); + } + private class TestCookieFeature : IResponseCookiesFeature { public IResponseCookies Cookies { get; } = new BadCookies(); diff --git a/test/Microsoft.AspNetCore.CookiePolicy.Test/project.json b/test/Microsoft.AspNetCore.CookiePolicy.Test/project.json index 6e64aba57d..26cc946be1 100644 --- a/test/Microsoft.AspNetCore.CookiePolicy.Test/project.json +++ b/test/Microsoft.AspNetCore.CookiePolicy.Test/project.json @@ -5,6 +5,7 @@ "dependencies": { "dotnet-test-xunit": "1.0.0-*", "Microsoft.NETCore.Platforms": "1.0.1-*", + "Microsoft.AspNetCore.Authentication.Cookies": "1.0.0-*", "Microsoft.AspNetCore.CookiePolicy": "1.0.0-*", "Microsoft.AspNetCore.TestHost": "1.0.0-*", "Microsoft.Extensions.DependencyInjection": "1.0.0-*", diff --git a/test/Microsoft.Owin.Security.Interop.Test/CookieInteropTests.cs b/test/Microsoft.Owin.Security.Interop.Test/CookieInteropTests.cs index 2abb115e47..79288b026d 100644 --- a/test/Microsoft.Owin.Security.Interop.Test/CookieInteropTests.cs +++ b/test/Microsoft.Owin.Security.Interop.Test/CookieInteropTests.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; using System.IO; using System.Linq; using System.Net.Http; @@ -14,6 +15,7 @@ using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Net.Http.Headers; using Microsoft.Owin.Security.Cookies; using Microsoft.Owin.Testing; using Owin; @@ -71,12 +73,73 @@ namespace Microsoft.Owin.Security.Interop var newServer = new AspNetCore.TestHost.TestServer(builder); var request = new HttpRequestMessage(HttpMethod.Get, "http://example.com/login"); - request.Headers.Add("Cookie", transaction.SetCookie.Split(new[] { ';' }, 2).First()); + foreach (var cookie in SetCookieHeaderValue.ParseList(transaction.SetCookie)) + { + request.Headers.Add("Cookie", cookie.Name + "=" + cookie.Value); + } var response = await newServer.CreateClient().SendAsync(request); Assert.Equal("Alice", await response.Content.ReadAsStringAsync()); } + [Fact] + public async Task AspNetCoreWithLargeInteropCookieContainsIdentity() + { + var identity = new ClaimsIdentity("Cookies"); + identity.AddClaim(new Claim(ClaimTypes.Name, new string('a', 1024 * 5))); + + var dataProtection = DataProtectionProvider.Create(new DirectoryInfo("..\\..\\artifacts")); + var dataProtector = dataProtection.CreateProtector( + "Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationMiddleware", // full name of the ASP.NET Core type + CookieAuthenticationDefaults.AuthenticationType, "v2"); + + var interopServer = TestServer.Create(app => + { + app.Properties["host.AppName"] = "Microsoft.Owin.Security.Tests"; + + app.UseCookieAuthentication(new Cookies.CookieAuthenticationOptions + { + TicketDataFormat = new AspNetTicketDataFormat(new DataProtectorShim(dataProtector)), + CookieName = AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.CookiePrefix + + AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme, + CookieManager = new ChunkingCookieManager(), + }); + + app.Run(context => + { + context.Authentication.SignIn(identity); + return Task.FromResult(0); + }); + }); + + var transaction = await SendAsync(interopServer, "http://example.com"); + + var builder = new WebHostBuilder() + .Configure(app => + { + app.UseCookieAuthentication(new AspNetCore.Builder.CookieAuthenticationOptions + { + DataProtectionProvider = dataProtection + }); + app.Run(async context => + { + var result = await context.Authentication.AuthenticateAsync("Cookies"); + await context.Response.WriteAsync(result.Identity.Name); + }); + }) + .ConfigureServices(services => services.AddAuthentication()); + var newServer = new AspNetCore.TestHost.TestServer(builder); + + var request = new HttpRequestMessage(HttpMethod.Get, "http://example.com/login"); + foreach (var cookie in SetCookieHeaderValue.ParseList(transaction.SetCookie)) + { + request.Headers.Add("Cookie", cookie.Name + "=" + cookie.Value); + } + var response = await newServer.CreateClient().SendAsync(request); + + Assert.Equal(1024 * 5, (await response.Content.ReadAsStringAsync()).Length); + } + [Fact] public async Task InteropWithNewCookieContainsIdentity() { @@ -102,13 +165,13 @@ namespace Microsoft.Owin.Security.Interop .ConfigureServices(services => services.AddAuthentication()); var newServer = new AspNetCore.TestHost.TestServer(builder); - var cookie = await SendAndGetCookie(newServer, "http://example.com/login"); + var cookies = await SendAndGetCookies(newServer, "http://example.com/login"); var server = TestServer.Create(app => { app.Properties["host.AppName"] = "Microsoft.Owin.Security.Tests"; - app.UseCookieAuthentication(new Owin.Security.Cookies.CookieAuthenticationOptions + app.UseCookieAuthentication(new Cookies.CookieAuthenticationOptions { TicketDataFormat = new AspNetTicketDataFormat(new DataProtectorShim(dataProtector)), CookieName = AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.CookiePrefix @@ -122,18 +185,74 @@ namespace Microsoft.Owin.Security.Interop }); }); - var transaction2 = await SendAsync(server, "http://example.com/me/Cookies", cookie); + var transaction2 = await SendAsync(server, "http://example.com/me/Cookies", cookies); Assert.Equal("Alice", FindClaimValue(transaction2, ClaimTypes.Name)); } - private static async Task SendAndGetCookie(AspNetCore.TestHost.TestServer server, string uri) + [Fact] + public async Task InteropWithLargeNewCookieContainsIdentity() + { + var user = new ClaimsPrincipal(); + var identity = new ClaimsIdentity("scheme"); + identity.AddClaim(new Claim(ClaimTypes.Name, new string('a', 1024 * 5))); + user.AddIdentity(identity); + + var dataProtection = DataProtectionProvider.Create(new DirectoryInfo("..\\..\\artifacts")); + var dataProtector = dataProtection.CreateProtector( + "Microsoft.AspNetCore.Authentication.Cookies.CookieAuthenticationMiddleware", // full name of the ASP.NET Core type + CookieAuthenticationDefaults.AuthenticationType, "v2"); + + var builder = new WebHostBuilder() + .Configure(app => + { + app.UseCookieAuthentication(new AspNetCore.Builder.CookieAuthenticationOptions + { + DataProtectionProvider = dataProtection + }); + app.Run(context => context.Authentication.SignInAsync("Cookies", user)); + }) + .ConfigureServices(services => services.AddAuthentication()); + var newServer = new AspNetCore.TestHost.TestServer(builder); + + var cookies = await SendAndGetCookies(newServer, "http://example.com/login"); + + var server = TestServer.Create(app => + { + app.Properties["host.AppName"] = "Microsoft.Owin.Security.Tests"; + + app.UseCookieAuthentication(new Cookies.CookieAuthenticationOptions + { + TicketDataFormat = new AspNetTicketDataFormat(new DataProtectorShim(dataProtector)), + CookieName = AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.CookiePrefix + + AspNetCore.Authentication.Cookies.CookieAuthenticationDefaults.AuthenticationScheme, + CookieManager = new ChunkingCookieManager(), + }); + + app.Run(async context => + { + var result = await context.Authentication.AuthenticateAsync("Cookies"); + Describe(context.Response, result); + }); + }); + + var transaction2 = await SendAsync(server, "http://example.com/me/Cookies", cookies); + + Assert.Equal(1024 * 5, FindClaimValue(transaction2, ClaimTypes.Name).Length); + } + + private static async Task> SendAndGetCookies(AspNetCore.TestHost.TestServer server, string uri) { var request = new HttpRequestMessage(HttpMethod.Get, uri); var response = await server.CreateClient().SendAsync(request); if (response.Headers.Contains("Set-Cookie")) { - return response.Headers.GetValues("Set-Cookie").ToList().First(); + IList cookieHeaders = new List(); + foreach (var cookie in SetCookieHeaderValue.ParseList(response.Headers.GetValues("Set-Cookie").ToList())) + { + cookieHeaders.Add(cookie.Name + "=" + cookie.Value); + } + return cookieHeaders; } return null; } @@ -148,7 +267,7 @@ namespace Microsoft.Owin.Security.Interop return claim.Attribute("value").Value; } - private static void Describe(IOwinResponse res, Owin.Security.AuthenticateResult result) + private static void Describe(IOwinResponse res, AuthenticateResult result) { res.StatusCode = 200; res.ContentType = "text/xml"; @@ -171,12 +290,12 @@ namespace Microsoft.Owin.Security.Interop } } - private static async Task SendAsync(TestServer server, string uri, string cookieHeader = null, bool ajaxRequest = false) + private static async Task SendAsync(TestServer server, string uri, IList cookieHeaders = null, bool ajaxRequest = false) { var request = new HttpRequestMessage(HttpMethod.Get, uri); - if (!string.IsNullOrEmpty(cookieHeader)) + if (cookieHeaders != null) { - request.Headers.Add("Cookie", cookieHeader); + request.Headers.Add("Cookie", cookieHeaders); } if (ajaxRequest) { @@ -189,11 +308,11 @@ namespace Microsoft.Owin.Security.Interop }; if (transaction.Response.Headers.Contains("Set-Cookie")) { - transaction.SetCookie = transaction.Response.Headers.GetValues("Set-Cookie").SingleOrDefault(); + transaction.SetCookie = transaction.Response.Headers.GetValues("Set-Cookie").ToList(); } - if (!string.IsNullOrEmpty(transaction.SetCookie)) + if (transaction.SetCookie != null && transaction.SetCookie.Any()) { - transaction.CookieNameValue = transaction.SetCookie.Split(new[] { ';' }, 2).First(); + transaction.CookieNameValue = transaction.SetCookie.First().Split(new[] { ';' }, 2).First(); } transaction.ResponseText = await transaction.Response.Content.ReadAsStringAsync(); @@ -211,7 +330,7 @@ namespace Microsoft.Owin.Security.Interop public HttpRequestMessage Request { get; set; } public HttpResponseMessage Response { get; set; } - public string SetCookie { get; set; } + public IList SetCookie { get; set; } public string CookieNameValue { get; set; } public string ResponseText { get; set; }