Add CookieBuilder property to SessionOptions and obsolete duplicated properties

Nate McMaster · Nate McMaster · commit dece939aabc9 · 2017-06-30T10:02:53.000-07:00
diff --git a/Session.sln b/Session.sln
@@ -1,6 +1,6 @@
 Microsoft Visual Studio Solution File, Format Version 12.00
 # Visual Studio 15
-VisualStudioVersion = 15.0.26228.9
+VisualStudioVersion = 15.0.26621.2
 MinimumVisualStudioVersion = 10.0.40219.1
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{E9D63F97-6078-42AD-BFD3-F956BF921BB5}"
 EndProject
@@ -15,6 +15,16 @@ EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SessionSample", "samples\SessionSample\SessionSample.csproj", "{FE0B9969-3BDE-4A7D-BE1B-47EAE8DBF365}"
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "SolutionItems", "SolutionItems", "{3B45F658-5BF1-4E07-BE9C-6F5110AC2277}"
+	ProjectSection(SolutionItems) = preProject
+		.gitattributes = .gitattributes
+		.gitignore = .gitignore
+		.travis.yml = .travis.yml
+		appveyor.yml = appveyor.yml
+		NuGet.config = NuGet.config
+		NuGetPackageVerifier.json = NuGetPackageVerifier.json
+		README.md = README.md
+		version.props = version.props
+	EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "build", "build", "{4F21221F-2813-41B7-AAFC-E03FD52971CC}"
 	ProjectSection(SolutionItems) = preProject
@@ -50,4 +60,7 @@ Global
 		{FE0B9969-3BDE-4A7D-BE1B-47EAE8DBF365} = {94E80ED2-9F27-40AC-A9EF-C707BDFAA3BE}
 		{4F21221F-2813-41B7-AAFC-E03FD52971CC} = {3B45F658-5BF1-4E07-BE9C-6F5110AC2277}
 	EndGlobalSection
+	GlobalSection(ExtensibilityGlobals) = postSolution
+		SolutionGuid = {6AE224B9-B604-4E47-9617-9D114DAE9BE5}
+	EndGlobalSection
 EndGlobal
diff --git a/src/Microsoft.AspNetCore.Session/SessionMiddleware.cs b/src/Microsoft.AspNetCore.Session/SessionMiddleware.cs
@@ -83,7 +83,7 @@ public async Task Invoke(HttpContext context)
         {
             var isNewSessionKey = false;
             Func<bool> tryEstablishSession = ReturnTrue;
-            var cookieValue = context.Request.Cookies[_options.CookieName];
+            var cookieValue = context.Request.Cookies[_options.Cookie.Name];
             var sessionKey = CookieProtection.Unprotect(_dataProtector, cookieValue, _logger);
             if (string.IsNullOrWhiteSpace(sessionKey) || sessionKey.Length != SessionKeyLength)
             {
@@ -150,23 +150,9 @@ private static Task OnStartingCallback(object state)
 
             private void SetCookie()
             {
-                var cookieOptions = new CookieOptions
-                {
-                    Domain = _options.CookieDomain,
-                    SameSite = _options.SameSiteMode,
-                    HttpOnly = _options.CookieHttpOnly,
-                    Path = _options.CookiePath ?? SessionDefaults.CookiePath,
-                };
-                if (_options.CookieSecure == CookieSecurePolicy.SameAsRequest)
-                {
-                    cookieOptions.Secure = _context.Request.IsHttps;
-                }
-                else
-                {
-                    cookieOptions.Secure = _options.CookieSecure == CookieSecurePolicy.Always;
-                }
+                var cookieOptions = _options.Cookie.Build(_context);
 
-                _context.Response.Cookies.Append(_options.CookieName, _cookieValue, cookieOptions);
+                _context.Response.Cookies.Append(_options.Cookie.Name, _cookieValue, cookieOptions);
 
                 _context.Response.Headers["Cache-Control"] = "no-cache";
                 _context.Response.Headers["Pragma"] = "no-cache";
diff --git a/src/Microsoft.AspNetCore.Session/SessionOptions.cs b/src/Microsoft.AspNetCore.Session/SessionOptions.cs
@@ -12,45 +12,117 @@ namespace Microsoft.AspNetCore.Builder
     /// </summary>
     public class SessionOptions
     {
+        private CookieBuilder _cookieBuilder = new SessionCookieBuilder();
+
+        /// <summary>
+        /// Determines the settings used to create the cookie.
+        /// <para>
+        /// <see cref="CookieBuilder.Name"/> defaults to <see cref="SessionDefaults.CookieName"/>.
+        /// <see cref="CookieBuilder.Path"/> defaults to <see cref="SessionDefaults.CookiePath"/>.
+        /// <see cref="CookieBuilder.SameSite"/> defaults to <see cref="SameSiteMode.Lax"/>.
+        /// <see cref="CookieBuilder.HttpOnly"/> defaults to <c>true</c>
+        /// </para>
+        /// </summary>
+        public CookieBuilder Cookie
+        {
+            get => _cookieBuilder;
+            set => _cookieBuilder = value ?? throw new ArgumentNullException(nameof(value));
+        }
+
+        /// <summary>
+        /// The IdleTimeout indicates how long the session can be idle before its contents are abandoned. Each session access
+        /// resets the timeout. Note this only applies to the content of the session, not the cookie.
+        /// </summary>
+        public TimeSpan IdleTimeout { get; set; } = TimeSpan.FromMinutes(20);
+
+        #region Obsolete API
         /// <summary>
+        /// <para>
+        /// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.Name"/> on <see cref="Cookie"/>.
+        /// </para>
+        /// <para>
         /// Determines the cookie name used to persist the session ID.
-        /// Defaults to <see cref="SessionDefaults.CookieName"/>.
+        /// </para>
         /// </summary>
-        public string CookieName { get; set; } = SessionDefaults.CookieName;
+        [Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.Name) + ".")]
+        public string CookieName { get => Cookie.Name; set => Cookie.Name = value; }
 
         /// <summary>
+        /// <para>
+        /// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.Domain"/> on <see cref="Cookie"/>.
+        /// </para>
+        /// <para>
         /// Determines the domain used to create the cookie. Is not provided by default.
+        /// </para>
         /// </summary>
-        public string CookieDomain { get; set; }
+        [Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.Domain) + ".")]
+        public string CookieDomain { get => Cookie.Domain; set => Cookie.Domain = value; }
 
         /// <summary>
+        /// <para>
+        /// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.Path"/> on <see cref="Cookie"/>.
+        /// </para>
+        /// <para>
         /// Determines the path used to create the cookie.
         /// Defaults to <see cref="SessionDefaults.CookiePath"/>.
+        /// </para>
         /// </summary>
-        public string CookiePath { get; set; } = SessionDefaults.CookiePath;
+        [Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.Path) + ".")]
+        public string CookiePath { get => Cookie.Path; set => Cookie.Path = value; }
 
         /// <summary>
+        /// <para>
+        /// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.HttpOnly"/> on <see cref="Cookie"/>.
+        /// </para>
+        /// <para>
         /// Determines if the browser should allow the cookie to be accessed by client-side JavaScript. The
         /// default is true, which means the cookie will only be passed to HTTP requests and is not made available
         /// to script on the page.
+        /// </para>
         /// </summary>
-        public bool CookieHttpOnly { get; set; } = true;
+        [Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.HttpOnly) + ".")]
+        public bool CookieHttpOnly { get => Cookie.HttpOnly; set => Cookie.HttpOnly = value; }
 
         /// <summary>
+        /// <para>
+        /// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.SameSite"/> on <see cref="Cookie"/>.
+        /// </para>
+        /// <para>
         /// Determines if the browser should allow the cookie to be attached to same-site or cross-site requests. The
         /// default is Lax, which means the cookie is allowed to be attached to same-site and safe cross-site requests.
+        /// </para>
         /// </summary>
-        public SameSiteMode SameSiteMode { get; set; } = SameSiteMode.Lax;
+        [Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.SameSite) + ".")]
+        public SameSiteMode SameSiteMode { get => Cookie.SameSite; set => Cookie.SameSite = value; }
 
         /// <summary>
-        /// Determines if the cookie should only be transmitted on HTTPS requests. 
+        /// <para>
+        /// This property is obsolete and will be removed in a future version. The recommended alternative is <seealso cref="CookieBuilder.SecurePolicy"/> on <see cref="Cookie"/>.
+        /// </para>
+        /// <para>
+        /// Determines if the cookie should only be transmitted on HTTPS requests.
+        /// </para>
         /// </summary>
-        public CookieSecurePolicy CookieSecure { get; set; } = CookieSecurePolicy.None;
+        [Obsolete("This property is obsolete and will be removed in a future version. The recommended alternative is " + nameof(Cookie) + "." + nameof(CookieBuilder.SecurePolicy) + ".")]
+        public CookieSecurePolicy CookieSecure { get => Cookie.SecurePolicy; set => Cookie.SecurePolicy = value; }
+        #endregion
 
-        /// <summary>
-        /// The IdleTimeout indicates how long the session can be idle before its contents are abandoned. Each session access
-        /// resets the timeout. Note this only applies to the content of the session, not the cookie.
-        /// </summary>
-        public TimeSpan IdleTimeout { get; set; } = TimeSpan.FromMinutes(20);
+        private class SessionCookieBuilder : CookieBuilder
+        {
+            public SessionCookieBuilder()
+            {
+                Name = SessionDefaults.CookieName;
+                Path = SessionDefaults.CookiePath;
+                SecurePolicy = CookieSecurePolicy.None;
+                SameSite = SameSiteMode.Lax;
+                HttpOnly = true;
+            }
+
+            public override TimeSpan? Expiration
+            {
+                get => null;
+                set => throw new InvalidOperationException(nameof(Expiration) + " cannot be set for the cookie defined by " + nameof(SessionOptions));
+            }
+        }
     }
-}
+}
diff --git a/test/Microsoft.AspNetCore.Session.Tests/SessionTests.cs b/test/Microsoft.AspNetCore.Session.Tests/SessionTests.cs
@@ -105,8 +105,11 @@ public async Task SecureSessionBasedOnHttpsAndSecurePolicy(
                {
                    app.UseSession(new SessionOptions
                    {
-                       CookieName = "TestCookie",
-                       CookieSecure = cookieSecurePolicy
+                       Cookie =
+                       {
+                           Name = "TestCookie",
+                           SecurePolicy = cookieSecurePolicy
+                       }
                    });
                    app.Run(context =>
                    {

Original file line number	Diff line number	Diff line change
`@@ -105,8 +105,11 @@ public async Task SecureSessionBasedOnHttpsAndSecurePolicy(`
`105`	`105`	`{`
`106`	`106`	`app.UseSession(new SessionOptions`
`107`	`107`	`{`
`108`		`- CookieName = "TestCookie",`
`109`		`- CookieSecure = cookieSecurePolicy`
	`108`	`+ Cookie =`
	`109`	`+ {`
	`110`	`+ Name = "TestCookie",`
	`111`	`+ SecurePolicy = cookieSecurePolicy`
	`112`	`+ }`
`110`	`113`	`});`
`111`	`114`	`app.Run(context =>`
`112`	`115`	`{`