From 5870fce03593c1c216983668359d040fb9cc02e7 Mon Sep 17 00:00:00 2001 From: John Luo Date: Mon, 22 May 2017 01:19:34 -0700 Subject: [PATCH] Add configure delegate for CookieOptions - allows configuration of CookieOptions such as SameSite without explicit duplication of the option on AntiforgeryOptions --- .../AntiforgeryOptions.cs | 16 ++++++++-- .../Internal/DefaultAntiforgeryTokenStore.cs | 32 +++++++++---------- .../DefaultAntiforgeryTokenStoreTest.cs | 8 ++--- 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.AspNetCore.Antiforgery/AntiforgeryOptions.cs b/src/Microsoft.AspNetCore.Antiforgery/AntiforgeryOptions.cs index b8c6e4b816..241edb814e 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/AntiforgeryOptions.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/AntiforgeryOptions.cs @@ -47,18 +47,30 @@ namespace Microsoft.AspNetCore.Antiforgery } /// + /// This is obsolete and will be removed in a future version. + /// The recommended alternative is to use ConfigureCookieOptions. /// The path set on the cookie. If set to null, the "path" attribute on the cookie is set to the current /// request's value. If the value of is /// null or empty, then the "path" attribute is set to the value of . /// + [Obsolete("This is obsolete and will be removed in a future version. The recommended alternative is to use ConfigureCookieOptions.")] public PathString? CookiePath { get; set; } /// - /// The domain set on the cookie. By default its null which results in the "domain" attribute not being - /// set. + /// This is obsolete and will be removed in a future version. + /// The recommended alternative is to use ConfigureCookieOptions. + /// The domain set on the cookie. By default its null which results in the "domain" attribute not being set. /// + [Obsolete("This is obsolete and will be removed in a future version. The recommended alternative is to use ConfigureCookieOptions.")] public string CookieDomain { get; set; } + /// + /// Configures the of the antiforgery cookies. Without additional configuration, the + /// default values antiforgery cookie options are true for , null for + /// and for . + /// + public Action ConfigureCookieOptions { get; set; } + /// /// Specifies the name of the antiforgery token field that is used by the antiforgery system. /// diff --git a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs index 0a78eddb41..2fd0d9507e 100644 --- a/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs +++ b/src/Microsoft.AspNetCore.Antiforgery/Internal/DefaultAntiforgeryTokenStore.cs @@ -69,34 +69,34 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal Debug.Assert(httpContext != null); Debug.Assert(token != null); - var options = new CookieOptions(); - options.HttpOnly = true; - options.Domain = _options.CookieDomain; - // Note: don't use "newCookie.Secure = _options.RequireSSL;" since the default - // value of newCookie.Secure is populated out of band. - if (_options.RequireSsl) + var options = new CookieOptions { - options.Secure = true; - } - SetCookiePath(httpContext, options); + HttpOnly = true, +#pragma warning disable 618 + Domain = _options.CookieDomain, +#pragma warning restore 618 + SameSite = SameSiteMode.Strict, + Secure = _options.RequireSsl + }; - httpContext.Response.Cookies.Append(_options.CookieName, token, options); - } - - private void SetCookiePath(HttpContext httpContext, CookieOptions cookieOptions) - { +#pragma warning disable 618 if (_options.CookiePath != null) { - cookieOptions.Path = _options.CookiePath.ToString(); + options.Path = _options.CookiePath.ToString(); } +#pragma warning restore 618 else { var pathBase = httpContext.Request.PathBase.ToString(); if (!string.IsNullOrEmpty(pathBase)) { - cookieOptions.Path = pathBase; + options.Path = pathBase; } } + + _options.ConfigureCookieOptions?.Invoke(httpContext, options); + + httpContext.Response.Cookies.Append(_options.CookieName, token, options); } } } diff --git a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs index f328ff908b..96fd2703fd 100644 --- a/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs +++ b/test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs @@ -311,7 +311,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal } [Fact] - public void SaveCookieToken_NonNullAntiforgeryOptionsCookiePath_UsesOptionsCookiePath() + public void SaveCookieToken_NonNullAntiforgeryOptionsConfigureCookieOptionsPath_UsesCookieOptionsPath() { // Arrange var expectedCookiePath = "/"; @@ -330,7 +330,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal .Returns("/index.html"); var options = new AntiforgeryOptions(); options.CookieName = _cookieName; - options.CookiePath = expectedCookiePath; + options.ConfigureCookieOptions = (context, cookieOptions) => cookieOptions.Path = expectedCookiePath; var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act @@ -346,7 +346,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal } [Fact] - public void SaveCookieToken_NonNullAntiforgeryOptionsCookieDomain_UsesOptionsCookieDomain() + public void SaveCookieToken_NonNullAntiforgeryOptionsConfigureCookieOptionsDomain_UsesCookieOptionsDomain() { // Arrange var expectedCookieDomain = "microsoft.com"; @@ -364,7 +364,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal .Returns("/index.html"); var options = new AntiforgeryOptions(); options.CookieName = _cookieName; - options.CookieDomain = expectedCookieDomain; + options.ConfigureCookieOptions = (context, cookieOptions) => cookieOptions.Domain = expectedCookieDomain; var tokenStore = new DefaultAntiforgeryTokenStore(new TestOptionsManager(options)); // Act