Fixes CookieTempDataProvider to set the secure attribute of a cookie …

…only if a request is secure
aspnet · Nov 8, 2016 · 07a2f1d · 07a2f1d
1 parent a3c06b0
commit 07a2f1d
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 5 deletions.
diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/CookieTempDataProvider.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/CookieTempDataProvider.cs
@@ -68,7 +68,7 @@ public void SaveTempData(HttpContext context, IDictionary<string, object> values
                 Path = string.IsNullOrEmpty(_options.Value.Path) ? GetPathBase(context) : _options.Value.Path,
                 Domain = string.IsNullOrEmpty(_options.Value.Domain) ? null : _options.Value.Domain,
                 HttpOnly = true,
-                Secure = true
+                Secure = context.Request.IsHttps,
             };
 
             var hasValues = (values != null && values.Count > 0);

diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataInCookiesTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/TempDataInCookiesTest.cs
@@ -56,7 +56,7 @@ public async Task RoundTripLargeData_WorksWithChunkingCookies(int size)
                 Assert.NotNull(cookieTempDataProviderCookie.Value);
                 Assert.Equal("/", cookieTempDataProviderCookie.Path);
                 Assert.Null(cookieTempDataProviderCookie.Domain);
-                Assert.True(cookieTempDataProviderCookie.Secure);
+                Assert.False(cookieTempDataProviderCookie.Secure);
             }
 
             // Act 2
@@ -107,7 +107,7 @@ public async Task Redirect_RetainsTempData_EvenIfAccessed_AndSetsAppropriateCook
             Assert.NotNull(setCookieHeader);
             Assert.Equal("/", setCookieHeader.Path);
             Assert.Null(setCookieHeader.Domain);
-            Assert.True(setCookieHeader.Secure);
+            Assert.False(setCookieHeader.Secure);
             Assert.Null(setCookieHeader.Expires);
 
             // Act 2
@@ -134,5 +134,35 @@ public async Task Redirect_RetainsTempData_EvenIfAccessed_AndSetsAppropriateCook
             Assert.NotNull(setCookieHeader.Expires);
             Assert.True(setCookieHeader.Expires < DateTimeOffset.Now); // expired cookie
         }
+
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public async Task CookieTempDataProviderCookie_SetsSecureAttributeOnCookie_OnlyIfRequestIsSecure(bool secureRequest)
+        {
+            // Arrange
+            var protocol = secureRequest ? "https" : "http";
+            var nameValueCollection = new List<KeyValuePair<string, string>>
+            {
+                new KeyValuePair<string, string>("value", "Foo"),
+            };
+            var content = new FormUrlEncodedContent(nameValueCollection);
+
+            // Act
+            var response = await Client.PostAsync($"{protocol}://localhost/TempData/SetTempData", content);
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+            IEnumerable<string> setCookieValues;
+            Assert.True(response.Headers.TryGetValues(HeaderNames.SetCookie, out setCookieValues));
+            var setCookieHeader = setCookieValues
+                .Select(setCookieValue => SetCookieHeaderValue.Parse(setCookieValue))
+                .FirstOrDefault(setCookieHeaderValue => setCookieHeaderValue.Name == CookieTempDataProvider.CookieName);
+            Assert.NotNull(setCookieHeader);
+            Assert.Equal("/", setCookieHeader.Path);
+            Assert.Null(setCookieHeader.Domain);
+            Assert.Equal(secureRequest, setCookieHeader.Secure);
+            Assert.Null(setCookieHeader.Expires);
+        }
     }
 }
diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewFeatures/CookieTempDataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewFeatures/CookieTempDataProviderTest.cs
@@ -90,6 +90,47 @@ public void SaveTempData_ProtectsAnd_Base64UrlEncodesDataAnd_SetsCookie()
             Assert.Equal(expectedDataToProtect, dataProtector.PlainTextToProtect);
         }
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void SaveTempData_SetsSecureAttributeOnCookie_OnlyIfRequestIsSecure(bool isSecure)
+        {
+            // Arrange
+            var values = new Dictionary<string, object>();
+            values.Add("int", 10);
+            var tempDataProviderStore = new TempDataSerializer();
+            var expectedDataToProtect = tempDataProviderStore.Serialize(values);
+            var expectedDataInCookie = Base64UrlTextEncoder.Encode(expectedDataToProtect);
+            var dataProtector = new PassThroughDataProtector();
+            var tempDataProvider = GetProvider(dataProtector);
+            var responseCookies = new MockResponseCookieCollection();
+            var httpContext = new Mock<HttpContext>();
+            httpContext
+                .SetupGet(hc => hc.Request.PathBase)
+                .Returns("/");
+            httpContext
+                .SetupGet(hc => hc.Request.IsHttps)
+                .Returns(isSecure);
+            httpContext
+                .Setup(hc => hc.Response.Cookies)
+                .Returns(responseCookies);
+
+            // Act
+            tempDataProvider.SaveTempData(httpContext.Object, values);
+
+            // Assert
+            Assert.Equal(1, responseCookies.Count);
+            var cookieInfo = responseCookies[CookieTempDataProvider.CookieName];
+            Assert.NotNull(cookieInfo);
+            Assert.Equal(expectedDataInCookie, cookieInfo.Value);
+            Assert.Equal(expectedDataToProtect, dataProtector.PlainTextToProtect);
+            Assert.Equal("/", cookieInfo.Options.Path);
+            Assert.Equal(isSecure, cookieInfo.Options.Secure);
+            Assert.True(cookieInfo.Options.HttpOnly);
+            Assert.Null(cookieInfo.Options.Expires);
+            Assert.Null(cookieInfo.Options.Domain);
+        }
+
         [Theory]
         [InlineData(null, "/")]
         [InlineData("", "/")]
@@ -112,6 +153,9 @@ public void SaveTempData_DefaultProviderOptions_SetsCookie_WithAppropriateCookie
             httpContext
                 .SetupGet(hc => hc.Request.PathBase)
                 .Returns(pathBase);
+            httpContext
+                .SetupGet(hc => hc.Request.IsHttps)
+                .Returns(false);
             httpContext
                 .Setup(hc => hc.Response.Cookies)
                 .Returns(responseCookies);
@@ -126,7 +170,7 @@ public void SaveTempData_DefaultProviderOptions_SetsCookie_WithAppropriateCookie
             Assert.Equal(expectedDataInCookie, cookieInfo.Value);
             Assert.Equal(expectedDataToProtect, dataProtector.PlainTextToProtect);
             Assert.Equal(expectedCookiePath, cookieInfo.Options.Path);
-            Assert.True(cookieInfo.Options.Secure);
+            Assert.False(cookieInfo.Options.Secure);
             Assert.True(cookieInfo.Options.HttpOnly);
             Assert.Null(cookieInfo.Options.Expires);
             Assert.Null(cookieInfo.Options.Domain);
@@ -154,6 +198,9 @@ public void SaveTempData_CustomProviderOptions_SetsCookie_WithAppropriateCookieO
                 new CookieTempDataProviderOptions() { Path = optionsPath, Domain = optionsDomain });
             var responseCookies = new MockResponseCookieCollection();
             var httpContext = new Mock<HttpContext>();
+            httpContext
+                .SetupGet(hc => hc.Request.IsHttps)
+                .Returns(false);
             httpContext
                 .SetupGet(hc => hc.Request.PathBase)
                 .Returns(requestPathBase);
@@ -172,7 +219,7 @@ public void SaveTempData_CustomProviderOptions_SetsCookie_WithAppropriateCookieO
             Assert.Equal(expectedDataToProtect, dataProtector.PlainTextToProtect);
             Assert.Equal(expectedCookiePath, cookieInfo.Options.Path);
             Assert.Equal(expectedDomain, cookieInfo.Options.Domain);
-            Assert.True(cookieInfo.Options.Secure);
+            Assert.False(cookieInfo.Options.Secure);
             Assert.True(cookieInfo.Options.HttpOnly);
             Assert.Null(cookieInfo.Options.Expires);
         }