From 8211a1c31331be75d6486ca4db3619fa93e4d165 Mon Sep 17 00:00:00 2001 From: Chris Ross Date: Wed, 15 Jan 2020 11:07:15 -0800 Subject: [PATCH] =?UTF-8?q?[2.1]=20CookieChunkingManager=20needs=20to=20fl?= =?UTF-8?q?ow=20the=20Secure=20attribute=E2=80=A6=20(#17953)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- eng/PatchConfig.props | 6 +++ .../samples/OpenIdConnectSample/Startup.cs | 2 +- .../test/WsFederation/WsFederationTest.cs | 6 +-- .../CookiePolicy/test/CookieChunkingTests.cs | 41 +++++++++++++++---- .../ChunkingCookieManager.cs | 5 +++ 5 files changed, 47 insertions(+), 13 deletions(-) diff --git a/eng/PatchConfig.props b/eng/PatchConfig.props index b4ea70ab0a..a4aab7c0b5 100644 --- a/eng/PatchConfig.props +++ b/eng/PatchConfig.props @@ -50,4 +50,10 @@ Later on, this will be checked using this condition: Microsoft.AspNetCore.SignalR.Core; + + + Microsoft.AspNetCore.Authentication.Cookies; + Microsoft.AspNetCore.Mvc.Core; + + diff --git a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs index 638a6050d7..2ecfc882ec 100644 --- a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs +++ b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs @@ -45,7 +45,7 @@ namespace OpenIdConnectSample private void CheckSameSite(HttpContext httpContext, CookieOptions options) { - if (options.SameSite > (SameSiteMode)(-1)) + if (options.SameSite == SameSiteMode.None) { var userAgent = httpContext.Request.Headers["User-Agent"].ToString(); // TODO: Use your User Agent library of choice here. diff --git a/src/Security/Authentication/test/WsFederation/WsFederationTest.cs b/src/Security/Authentication/test/WsFederation/WsFederationTest.cs index bc1ef757f1..1c4fc9e94d 100644 --- a/src/Security/Authentication/test/WsFederation/WsFederationTest.cs +++ b/src/Security/Authentication/test/WsFederation/WsFederationTest.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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; @@ -190,7 +190,7 @@ namespace Microsoft.AspNetCore.Authentication.WsFederation response.EnsureSuccessStatusCode(); var cookie = response.Headers.GetValues(HeaderNames.SetCookie).Single(); - Assert.Equal(".AspNetCore.Cookies=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/; samesite=lax", cookie); + Assert.Equal(".AspNetCore.Cookies=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/; samesite=lax; httponly", cookie); Assert.Equal("OnRemoteSignOut", response.Headers.GetValues("EventHeader").Single()); Assert.Equal("", await response.Content.ReadAsStringAsync()); } @@ -440,4 +440,4 @@ namespace Microsoft.AspNetCore.Authentication.WsFederation } } } -} \ No newline at end of file +} diff --git a/src/Security/CookiePolicy/test/CookieChunkingTests.cs b/src/Security/CookiePolicy/test/CookieChunkingTests.cs index e645745b35..20e186783a 100644 --- a/src/Security/CookiePolicy/test/CookieChunkingTests.cs +++ b/src/Security/CookiePolicy/test/CookieChunkingTests.cs @@ -21,6 +21,29 @@ namespace Microsoft.AspNetCore.Internal Assert.Equal("TestCookie=" + testString + "; path=/; samesite=lax", values[0]); } + [Fact] + public void AppendLargeCookie_WithOptions_Appended() + { + HttpContext context = new DefaultHttpContext(); + var now = DateTimeOffset.UtcNow; + var options = new CookieOptions + { + Domain = "foo.com", + HttpOnly = true, + SameSite = SameSiteMode.Strict, + Path = "/bar", + Secure = true, + Expires = now.AddMinutes(5), + MaxAge = TimeSpan.FromMinutes(5) + }; + var testString = "abcdefghijklmnopqrstuvwxyz0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + new ChunkingCookieManager() { ChunkSize = null }.AppendResponseCookie(context, "TestCookie", testString, options); + + var values = context.Response.Headers["Set-Cookie"]; + Assert.Single(values); + Assert.Equal($"TestCookie={testString}; expires={now.AddMinutes(5).ToString("R")}; max-age=300; domain=foo.com; path=/bar; secure; samesite=strict; httponly", values[0]); + } + [Fact] public void AppendLargeCookieWithLimit_Chunked() { @@ -112,19 +135,19 @@ namespace Microsoft.AspNetCore.Internal HttpContext context = new DefaultHttpContext(); context.Request.Headers.Append("Cookie", "TestCookie=chunks-7"); - new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com" }); + new ChunkingCookieManager().DeleteCookie(context, "TestCookie", new CookieOptions() { Domain = "foo.com", Secure = true }); var cookies = context.Response.Headers["Set-Cookie"]; Assert.Equal(8, cookies.Count); Assert.Equal(new[] { - "TestCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax", - "TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax", - "TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax", - "TestCookieC3=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax", - "TestCookieC4=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax", - "TestCookieC5=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax", - "TestCookieC6=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax", - "TestCookieC7=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; samesite=lax", + "TestCookie=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure; samesite=lax", + "TestCookieC1=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure; samesite=lax", + "TestCookieC2=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure; samesite=lax", + "TestCookieC3=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure; samesite=lax", + "TestCookieC4=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure; samesite=lax", + "TestCookieC5=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure; samesite=lax", + "TestCookieC6=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure; samesite=lax", + "TestCookieC7=; expires=Thu, 01 Jan 1970 00:00:00 GMT; domain=foo.com; path=/; secure; samesite=lax", }, cookies); } } diff --git a/src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs b/src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs index 42cc4e2f0f..4f1700bb77 100644 --- a/src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs +++ b/src/Shared/ChunkingCookieManager/ChunkingCookieManager.cs @@ -169,6 +169,7 @@ namespace Microsoft.AspNetCore.Internal HttpOnly = options.HttpOnly, Path = options.Path, Secure = options.Secure, + MaxAge = options.MaxAge, }; var templateLength = template.ToString().Length; @@ -285,8 +286,10 @@ namespace Microsoft.AspNetCore.Internal Path = options.Path, Domain = options.Domain, SameSite = options.SameSite, + Secure = options.Secure, IsEssential = options.IsEssential, Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc), + HttpOnly = options.HttpOnly, }); for (int i = 1; i <= chunks; i++) @@ -300,8 +303,10 @@ namespace Microsoft.AspNetCore.Internal Path = options.Path, Domain = options.Domain, SameSite = options.SameSite, + Secure = options.Secure, IsEssential = options.IsEssential, Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc), + HttpOnly = options.HttpOnly, }); } }