From c51207e2654cf026aa698f5b21933ea2a79b6fde Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 15 Sep 2015 17:49:39 -0700 Subject: [PATCH 01/13] Add new CookiePolicy middleware --- Security.sln | 30 +++ .../AppendCookieContext.cs | 23 ++ .../CookiePolicyAppBuilderExtensions.cs | 35 +++ .../CookiePolicyMiddleware.cs | 168 +++++++++++++ .../CookiePolicyOptions.cs | 16 ++ .../DeleteCookieContext.cs | 21 ++ .../HttpOnlyPolicy.cs | 11 + .../Microsoft.AspNet.CookiePolicy.xproj | 19 ++ .../Properties/AssemblyInfo.cs | 8 + .../SecurePolicy.cs | 12 + .../project.json | 18 ++ .../CookiePolicyTests.cs | 231 ++++++++++++++++++ .../Microsoft.AspNet.CookiePolicy.Test.xproj | 21 ++ .../TestExtensions.cs | 74 ++++++ .../Transaction.cs | 51 ++++ .../project.json | 21 ++ 16 files changed, 759 insertions(+) create mode 100644 src/Microsoft.AspNet.CookiePolicy/AppendCookieContext.cs create mode 100644 src/Microsoft.AspNet.CookiePolicy/CookiePolicyAppBuilderExtensions.cs create mode 100644 src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs create mode 100644 src/Microsoft.AspNet.CookiePolicy/CookiePolicyOptions.cs create mode 100644 src/Microsoft.AspNet.CookiePolicy/DeleteCookieContext.cs create mode 100644 src/Microsoft.AspNet.CookiePolicy/HttpOnlyPolicy.cs create mode 100644 src/Microsoft.AspNet.CookiePolicy/Microsoft.AspNet.CookiePolicy.xproj create mode 100644 src/Microsoft.AspNet.CookiePolicy/Properties/AssemblyInfo.cs create mode 100644 src/Microsoft.AspNet.CookiePolicy/SecurePolicy.cs create mode 100644 src/Microsoft.AspNet.CookiePolicy/project.json create mode 100644 test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs create mode 100644 test/Microsoft.AspNet.CookiePolicy.Test/Microsoft.AspNet.CookiePolicy.Test.xproj create mode 100644 test/Microsoft.AspNet.CookiePolicy.Test/TestExtensions.cs create mode 100644 test/Microsoft.AspNet.CookiePolicy.Test/Transaction.cs create mode 100644 test/Microsoft.AspNet.CookiePolicy.Test/project.json diff --git a/Security.sln b/Security.sln index 7fd810f8b..ee8180ede 100644 --- a/Security.sln +++ b/Security.sln @@ -46,6 +46,10 @@ Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNet.Authorizat EndProject Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNet.Authorization", "src\Microsoft.AspNet.Authorization\Microsoft.AspNet.Authorization.xproj", "{6AB3E514-5894-4131-9399-DC7D5284ADDB}" EndProject +Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNet.CookiePolicy", "src\Microsoft.AspNet.CookiePolicy\Microsoft.AspNet.CookiePolicy.xproj", "{86183DC3-02A8-4A68-8B60-71ECEC066E79}" +EndProject +Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNet.CookiePolicy.Test", "test\Microsoft.AspNet.CookiePolicy.Test\Microsoft.AspNet.CookiePolicy.Test.xproj", "{1790E052-646F-4529-B90E-6FEA95520D69}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -242,6 +246,30 @@ Global {6AB3E514-5894-4131-9399-DC7D5284ADDB}.Release|Mixed Platforms.Build.0 = Release|Any CPU {6AB3E514-5894-4131-9399-DC7D5284ADDB}.Release|x86.ActiveCfg = Release|Any CPU {6AB3E514-5894-4131-9399-DC7D5284ADDB}.Release|x86.Build.0 = Release|Any CPU + {86183DC3-02A8-4A68-8B60-71ECEC066E79}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {86183DC3-02A8-4A68-8B60-71ECEC066E79}.Debug|Any CPU.Build.0 = Debug|Any CPU + {86183DC3-02A8-4A68-8B60-71ECEC066E79}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {86183DC3-02A8-4A68-8B60-71ECEC066E79}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {86183DC3-02A8-4A68-8B60-71ECEC066E79}.Debug|x86.ActiveCfg = Debug|Any CPU + {86183DC3-02A8-4A68-8B60-71ECEC066E79}.Debug|x86.Build.0 = Debug|Any CPU + {86183DC3-02A8-4A68-8B60-71ECEC066E79}.Release|Any CPU.ActiveCfg = Release|Any CPU + {86183DC3-02A8-4A68-8B60-71ECEC066E79}.Release|Any CPU.Build.0 = Release|Any CPU + {86183DC3-02A8-4A68-8B60-71ECEC066E79}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {86183DC3-02A8-4A68-8B60-71ECEC066E79}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {86183DC3-02A8-4A68-8B60-71ECEC066E79}.Release|x86.ActiveCfg = Release|Any CPU + {86183DC3-02A8-4A68-8B60-71ECEC066E79}.Release|x86.Build.0 = Release|Any CPU + {1790E052-646F-4529-B90E-6FEA95520D69}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {1790E052-646F-4529-B90E-6FEA95520D69}.Debug|Any CPU.Build.0 = Debug|Any CPU + {1790E052-646F-4529-B90E-6FEA95520D69}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {1790E052-646F-4529-B90E-6FEA95520D69}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {1790E052-646F-4529-B90E-6FEA95520D69}.Debug|x86.ActiveCfg = Debug|Any CPU + {1790E052-646F-4529-B90E-6FEA95520D69}.Debug|x86.Build.0 = Debug|Any CPU + {1790E052-646F-4529-B90E-6FEA95520D69}.Release|Any CPU.ActiveCfg = Release|Any CPU + {1790E052-646F-4529-B90E-6FEA95520D69}.Release|Any CPU.Build.0 = Release|Any CPU + {1790E052-646F-4529-B90E-6FEA95520D69}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {1790E052-646F-4529-B90E-6FEA95520D69}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {1790E052-646F-4529-B90E-6FEA95520D69}.Release|x86.ActiveCfg = Release|Any CPU + {1790E052-646F-4529-B90E-6FEA95520D69}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -263,5 +291,7 @@ Global {2755BFE5-7421-4A31-A644-F817DF5CAA98} = {4D2B6A51-2F9F-44F5-8131-EA5CAC053652} {7AF5AD96-EB6E-4D0E-8ABE-C0B543C0F4C2} = {7BF11F3A-60B6-4796-B504-579C67FFBA34} {6AB3E514-5894-4131-9399-DC7D5284ADDB} = {4D2B6A51-2F9F-44F5-8131-EA5CAC053652} + {86183DC3-02A8-4A68-8B60-71ECEC066E79} = {4D2B6A51-2F9F-44F5-8131-EA5CAC053652} + {1790E052-646F-4529-B90E-6FEA95520D69} = {7BF11F3A-60B6-4796-B504-579C67FFBA34} EndGlobalSection EndGlobal diff --git a/src/Microsoft.AspNet.CookiePolicy/AppendCookieContext.cs b/src/Microsoft.AspNet.CookiePolicy/AppendCookieContext.cs new file mode 100644 index 000000000..f9d816661 --- /dev/null +++ b/src/Microsoft.AspNet.CookiePolicy/AppendCookieContext.cs @@ -0,0 +1,23 @@ +// 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 Microsoft.AspNet.Http; + +namespace Microsoft.AspNet.CookiePolicy +{ + public class AppendCookieContext + { + public AppendCookieContext(HttpContext context, CookieOptions options, string name, string value) + { + Context = context; + CookieOptions = options; + CookieName = name; + CookieValue = value; + } + + public HttpContext Context { get; } + public CookieOptions CookieOptions { get; } + public string CookieName { get; set; } + public string CookieValue { get; set; } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyAppBuilderExtensions.cs b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyAppBuilderExtensions.cs new file mode 100644 index 000000000..4d03d7615 --- /dev/null +++ b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyAppBuilderExtensions.cs @@ -0,0 +1,35 @@ +// 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 Microsoft.AspNet.CookiePolicy; + +namespace Microsoft.AspNet.Builder +{ + /// + /// Extension methods provided by the cookie policy middleware + /// + public static class CookiePolicyAppBuilderExtensions + { + /// + /// Adds a claims transformation middleware to your web application pipeline. + /// + /// The IApplicationBuilder passed to your configuration method + /// The original app parameter + public static IApplicationBuilder UseCookiePolicy(this IApplicationBuilder app) + { + return app.UseCookiePolicy(configureOptions: o => { }); + } + + /// + /// Adds a claims transformation middleware to your web application pipeline. + /// + /// The IApplicationBuilder passed to your configuration method + /// Used to configure the options for the middleware + /// The original app parameter + public static IApplicationBuilder UseCookiePolicy(this IApplicationBuilder app, Action configureOptions) + { + return app.UseMiddleware(configureOptions); + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs new file mode 100644 index 000000000..98ee01baf --- /dev/null +++ b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs @@ -0,0 +1,168 @@ +// 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.Threading.Tasks; +using Microsoft.AspNet.Builder; +using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Features; +using Microsoft.Framework.OptionsModel; +using Microsoft.AspNet.Http.Features.Internal; + +namespace Microsoft.AspNet.CookiePolicy +{ + public class CookiePolicyMiddleware + { + private readonly RequestDelegate _next; + + public CookiePolicyMiddleware( + RequestDelegate next, + IOptions options, + Action configureOptions) + { + Options = options.Value ?? new CookiePolicyOptions(); // REVIEW: or should we throw ArgumentNull exception? + if (configureOptions != null) + { + configureOptions(Options); + } + _next = next; + } + + public CookiePolicyOptions Options { get; set; } + + public Task Invoke(HttpContext context) + { + // Check if there is a SendFile feature already present + if (context.Features.Get() == null) + { + context.Features.Set(new CookiesWrapperFeature(context, Options)); + } + + return _next(context); + } + + private class CookiesWrapperFeature : IResponseCookiesFeature + { + public CookiesWrapperFeature(HttpContext context, CookiePolicyOptions options) + { + Wrapper = new CookiesWrapper(context, options, context.Response.Cookies); + } + + public IResponseCookies Wrapper { get; } + + public IResponseCookies Cookies + { + get + { + return Wrapper; + } + } + } + + private class CookiesWrapper : IResponseCookies + { + public CookiesWrapper(HttpContext context, CookiePolicyOptions options, IResponseCookies cookies) + { + Context = context; + Cookies = cookies; + Policy = options; + } + + public HttpContext Context { get; } + + public IResponseCookies Cookies { get; } + + public CookiePolicyOptions Policy { get; } + + private bool PolicyRequiresCookieOptions() + { + return Policy.HttpOnly != HttpOnlyPolicy.None || Policy.Secure != SecurePolicy.None; + } + + public void Append(string key, string value) + { + if (PolicyRequiresCookieOptions()) + { + Append(key, value, new CookieOptions()); + return; + } + + if (Policy.OnAppendCookie != null) + { + var context = new AppendCookieContext(Context, options: null, name: key, value: value); + Policy.OnAppendCookie(context); + key = context.CookieName; + value = context.CookieValue; + } + Cookies.Append(key, value); + } + + public void Append(string key, string value, CookieOptions options) + { + ApplyPolicy(options); + if (Policy.OnAppendCookie != null) + { + var context = new AppendCookieContext(Context, options, key, value); + Policy.OnAppendCookie(context); + key = context.CookieName; + value = context.CookieValue; + } + Cookies.Append(key, value, options); + } + + public void Delete(string key) + { + if (PolicyRequiresCookieOptions()) + { + Delete(key, new CookieOptions()); + return; + } + + + if (Policy.OnDeleteCookie != null) + { + var context = new DeleteCookieContext(Context, options: null, name: key); + Policy.OnDeleteCookie(context); + key = context.CookieName; + } + Cookies.Delete(key); + } + + public void Delete(string key, CookieOptions options) + { + ApplyPolicy(options); + if (Policy.OnDeleteCookie != null) + { + var context = new DeleteCookieContext(Context, options, key); + Policy.OnDeleteCookie(context); + key = context.CookieName; + } + Cookies.Delete(key, options); + } + + // Returns true if cookie options need to be applied + private void ApplyPolicy(CookieOptions options) + { + switch (Policy.Secure) + { + case SecurePolicy.Always: + options.Secure = true; + break; + case SecurePolicy.SameAsRequest: + options.Secure = Context.Request.IsHttps; + break; + case SecurePolicy.None: + break; + } + switch (Policy.HttpOnly) + { + case HttpOnlyPolicy.Always: + options.HttpOnly = true; + break; + case HttpOnlyPolicy.None: + break; + } + } + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyOptions.cs b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyOptions.cs new file mode 100644 index 000000000..ce5a86698 --- /dev/null +++ b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyOptions.cs @@ -0,0 +1,16 @@ +// 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; + +namespace Microsoft.AspNet.CookiePolicy +{ + public class CookiePolicyOptions + { + public HttpOnlyPolicy HttpOnly { get; set; } = HttpOnlyPolicy.None; + public SecurePolicy Secure { get; set; } = SecurePolicy.None; + + public Action OnAppendCookie { get; set; } + public Action OnDeleteCookie { get; set; } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.CookiePolicy/DeleteCookieContext.cs b/src/Microsoft.AspNet.CookiePolicy/DeleteCookieContext.cs new file mode 100644 index 000000000..c8cd208fb --- /dev/null +++ b/src/Microsoft.AspNet.CookiePolicy/DeleteCookieContext.cs @@ -0,0 +1,21 @@ +// 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 Microsoft.AspNet.Http; + +namespace Microsoft.AspNet.CookiePolicy +{ + public class DeleteCookieContext + { + public DeleteCookieContext(HttpContext context, CookieOptions options, string name) + { + Context = context; + CookieOptions = options; + CookieName = name; + } + + public HttpContext Context { get; } + public CookieOptions CookieOptions { get; } + public string CookieName { get; set; } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.CookiePolicy/HttpOnlyPolicy.cs b/src/Microsoft.AspNet.CookiePolicy/HttpOnlyPolicy.cs new file mode 100644 index 000000000..276e3ed3e --- /dev/null +++ b/src/Microsoft.AspNet.CookiePolicy/HttpOnlyPolicy.cs @@ -0,0 +1,11 @@ +// 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.AspNet.CookiePolicy +{ + public enum HttpOnlyPolicy + { + None, + Always + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.CookiePolicy/Microsoft.AspNet.CookiePolicy.xproj b/src/Microsoft.AspNet.CookiePolicy/Microsoft.AspNet.CookiePolicy.xproj new file mode 100644 index 000000000..7790eac27 --- /dev/null +++ b/src/Microsoft.AspNet.CookiePolicy/Microsoft.AspNet.CookiePolicy.xproj @@ -0,0 +1,19 @@ + + + + 14.0 + $(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion) + + + + 86183dc3-02a8-4a68-8b60-71ecec066e79 + Microsoft.AspNet.CookiePolicy + ..\..\artifacts\obj\$(MSBuildProjectName) + ..\..\artifacts\bin\$(MSBuildProjectName)\ + + + + 2.0 + + + \ No newline at end of file diff --git a/src/Microsoft.AspNet.CookiePolicy/Properties/AssemblyInfo.cs b/src/Microsoft.AspNet.CookiePolicy/Properties/AssemblyInfo.cs new file mode 100644 index 000000000..b2437d9ad --- /dev/null +++ b/src/Microsoft.AspNet.CookiePolicy/Properties/AssemblyInfo.cs @@ -0,0 +1,8 @@ +// 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.Reflection; +using System.Resources; + +[assembly: AssemblyMetadata("Serviceable", "True")] +[assembly: NeutralResourcesLanguage("en-us")] \ No newline at end of file diff --git a/src/Microsoft.AspNet.CookiePolicy/SecurePolicy.cs b/src/Microsoft.AspNet.CookiePolicy/SecurePolicy.cs new file mode 100644 index 000000000..962ecddff --- /dev/null +++ b/src/Microsoft.AspNet.CookiePolicy/SecurePolicy.cs @@ -0,0 +1,12 @@ +// 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.AspNet.CookiePolicy +{ + public enum SecurePolicy + { + None, + Always, + SameAsRequest + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.CookiePolicy/project.json b/src/Microsoft.AspNet.CookiePolicy/project.json new file mode 100644 index 000000000..609eecd89 --- /dev/null +++ b/src/Microsoft.AspNet.CookiePolicy/project.json @@ -0,0 +1,18 @@ +{ + "version": "1.0.0-*", + "description": "ASP.NET 5 cookie policy classes.", + "repository": { + "type": "git", + "url": "git://github.com/aspnet/security" + }, + "dependencies": { + "Microsoft.AspNet.Http.Abstractions": "1.0.0-*", + "Microsoft.AspNet.Http.Features": "1.0.0-*", + "Microsoft.AspNet.Http": "1.0.0-*", + "Microsoft.Framework.OptionsModel": "1.0.0-*" + }, + "frameworks": { + "dnx451": { }, + "dnxcore50": { } + } +} diff --git a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs new file mode 100644 index 000000000..4ce8beea8 --- /dev/null +++ b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs @@ -0,0 +1,231 @@ +// 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.Threading.Tasks; +using Microsoft.AspNet.Builder; +using Microsoft.AspNet.Http; +using Microsoft.AspNet.TestHost; +using Microsoft.Framework.DependencyInjection; +using Xunit; + +namespace Microsoft.AspNet.CookiePolicy.Test +{ + public class CookiePolicyTests + { + [Fact] + public async Task CookiePolicySecureAlwaysSetsSecure() + { + var server = TestServer.Create(app => + { + app.UseCookiePolicy(options => options.Secure = SecurePolicy.Always); + app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); + app.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }); + }, + services => services.AddAuthentication()); + + var transaction = await server.SendAsync("http://example.com/login"); + + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + } + + [Fact] + public async Task CookiePolicySecureNoneLeavesSecureAlone() + { + var server = TestServer.Create(app => + { + app.UseCookiePolicy(options => options.Secure = SecurePolicy.None); + app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); + app.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }); + }, + services => services.AddAuthentication()); + + var transaction = await server.SendAsync("http://example.com/login"); + + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + } + + [Fact] + public async Task CookiePolicySecureSameAsRequestIsFalseOnHttp() + { + var server = TestServer.Create(app => + { + app.UseCookiePolicy(options => options.Secure = SecurePolicy.SameAsRequest); + app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); + app.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }); + }, + services => services.AddAuthentication()); + + var transaction = await server.SendAsync("http://example.com/login"); + + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/", transaction.SetCookie[3]); + } + + [Fact] + public async Task CookiePolicySecureSameAsRequestIsTrueOnHttps() + { + var server = TestServer.Create(app => + { + app.UseCookiePolicy(options => options.Secure = SecurePolicy.SameAsRequest); + app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); + app.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }); + }, + services => services.AddAuthentication()); + + var transaction = await server.SendAsync("https://example.com/login"); + + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + } + + [Fact] + public async Task CookiePolicyHttpOnlyAlwaysSetsHttpOnly() + { + var server = TestServer.Create(app => + { + app.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.Always); + app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); + app.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); + return Task.FromResult(0); + }); + }, + services => services.AddAuthentication()); + + var transaction = await server.SendAsync("http://example.com/login"); + + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/; httponly", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/; httponly", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/; httponly", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]); + } + + [Fact] + public async Task CookiePolicyHttpOnlyNoneLeavesItAlone() + { + var server = TestServer.Create(app => + { + app.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.None); + app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); + app.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); + return Task.FromResult(0); + }); + }, + services => services.AddAuthentication()); + + var transaction = await server.SendAsync("http://example.com/login"); + + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]); + } + + [Fact] + public async Task CookiePolicyCanHijackAppend() + { + var server = TestServer.Create(app => + { + app.UseCookiePolicy(options => options.OnAppendCookie = ctx => ctx.CookieName = ctx.CookieValue = "Hao"); + app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); + app.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }); + }, + services => services.AddAuthentication()); + + var transaction = await server.SendAsync("http://example.com/login"); + + Assert.NotNull(transaction.SetCookie); + Assert.Equal("Hao=Hao; path=/", transaction.SetCookie[0]); + Assert.Equal("Hao=Hao; path=/", transaction.SetCookie[1]); + Assert.Equal("Hao=Hao; path=/", transaction.SetCookie[2]); + Assert.Equal("Hao=Hao; path=/; secure", transaction.SetCookie[3]); + } + + [Fact] + public async Task CookiePolicyCanHijackDelete() + { + var server = TestServer.Create(app => + { + app.UseCookiePolicy(options => options.OnDeleteCookie = ctx => ctx.CookieName = "A"); + app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); + app.Run(context => + { + context.Response.Cookies.Delete("A"); + context.Response.Cookies.Delete("B", new CookieOptions { Secure = false }); + context.Response.Cookies.Delete("C", new CookieOptions()); + context.Response.Cookies.Delete("D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }); + }, + services => services.AddAuthentication()); + + var transaction = await server.SendAsync("http://example.com/login"); + + Assert.NotNull(transaction.SetCookie); + Assert.Equal(2, transaction.SetCookie.Count); + Assert.Equal("A=; expires=Thu, 01-Jan-1970 00:00:00 GMT", transaction.SetCookie[0]); + Assert.Equal("A=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", transaction.SetCookie[1]); + } + + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.CookiePolicy.Test/Microsoft.AspNet.CookiePolicy.Test.xproj b/test/Microsoft.AspNet.CookiePolicy.Test/Microsoft.AspNet.CookiePolicy.Test.xproj new file mode 100644 index 000000000..b0a49fddd --- /dev/null +++ b/test/Microsoft.AspNet.CookiePolicy.Test/Microsoft.AspNet.CookiePolicy.Test.xproj @@ -0,0 +1,21 @@ + + + + 14.0 + $(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion) + + + + 1790e052-646f-4529-b90e-6fea95520d69 + Microsoft.AspNet.CookiePolicy.Test + ..\..\artifacts\obj\$(MSBuildProjectName) + ..\..\artifacts\bin\$(MSBuildProjectName)\ + + + 2.0 + + + + + + \ No newline at end of file diff --git a/test/Microsoft.AspNet.CookiePolicy.Test/TestExtensions.cs b/test/Microsoft.AspNet.CookiePolicy.Test/TestExtensions.cs new file mode 100644 index 000000000..90b01af4b --- /dev/null +++ b/test/Microsoft.AspNet.CookiePolicy.Test/TestExtensions.cs @@ -0,0 +1,74 @@ +// 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.IO; +using System.Linq; +using System.Net.Http; +using System.Security.Claims; +using System.Text; +using System.Threading.Tasks; +using System.Xml; +using System.Xml.Linq; +using Microsoft.AspNet.Http; +using Microsoft.AspNet.TestHost; + +namespace Microsoft.AspNet.CookiePolicy +{ + // REVIEW: Should find a shared home for these potentially (Copied from Auth tests) + public static class TestExtensions + { + public const string CookieAuthenticationScheme = "External"; + + public static async Task SendAsync(this TestServer server, string uri, string cookieHeader = null) + { + var request = new HttpRequestMessage(HttpMethod.Get, uri); + if (!string.IsNullOrEmpty(cookieHeader)) + { + request.Headers.Add("Cookie", cookieHeader); + } + var transaction = new Transaction + { + Request = request, + Response = await server.CreateClient().SendAsync(request), + }; + if (transaction.Response.Headers.Contains("Set-Cookie")) + { + transaction.SetCookie = transaction.Response.Headers.GetValues("Set-Cookie").ToList(); + } + transaction.ResponseText = await transaction.Response.Content.ReadAsStringAsync(); + + if (transaction.Response.Content != null && + transaction.Response.Content.Headers.ContentType != null && + transaction.Response.Content.Headers.ContentType.MediaType == "text/xml") + { + transaction.ResponseElement = XElement.Parse(transaction.ResponseText); + } + return transaction; + } + + public static void Describe(this HttpResponse res, ClaimsPrincipal principal) + { + res.StatusCode = 200; + res.ContentType = "text/xml"; + var xml = new XElement("xml"); + if (principal != null) + { + foreach (var identity in principal.Identities) + { + xml.Add(identity.Claims.Select(claim => + new XElement("claim", new XAttribute("type", claim.Type), + new XAttribute("value", claim.Value), + new XAttribute("issuer", claim.Issuer)))); + } + } + using (var memory = new MemoryStream()) + { + using (var writer = new XmlTextWriter(memory, Encoding.UTF8)) + { + xml.WriteTo(writer); + } + res.Body.Write(memory.ToArray(), 0, memory.ToArray().Length); + } + } + } +} diff --git a/test/Microsoft.AspNet.CookiePolicy.Test/Transaction.cs b/test/Microsoft.AspNet.CookiePolicy.Test/Transaction.cs new file mode 100644 index 000000000..afa9c0c99 --- /dev/null +++ b/test/Microsoft.AspNet.CookiePolicy.Test/Transaction.cs @@ -0,0 +1,51 @@ +// 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.Linq; +using System.Net.Http; +using System.Xml.Linq; + +namespace Microsoft.AspNet.CookiePolicy +{ + // REVIEW: Should find a shared home for these potentially (Copied from Auth tests) + public class Transaction + { + public HttpRequestMessage Request { get; set; } + public HttpResponseMessage Response { get; set; } + + public IList SetCookie { get; set; } + + public string ResponseText { get; set; } + public XElement ResponseElement { get; set; } + + public string AuthenticationCookieValue + { + get + { + if (SetCookie != null && SetCookie.Count > 0) + { + var authCookie = SetCookie.SingleOrDefault(c => c.Contains(".AspNet." + TestExtensions.CookieAuthenticationScheme + "=")); + if (authCookie != null) + { + return authCookie.Substring(0, authCookie.IndexOf(';')); + } + } + + return null; + } + } + + public string FindClaimValue(string claimType, string issuer = null) + { + var claim = ResponseElement.Elements("claim") + .SingleOrDefault(elt => elt.Attribute("type").Value == claimType && + (issuer == null || elt.Attribute("issuer").Value == issuer)); + if (claim == null) + { + return null; + } + return claim.Attribute("value").Value; + } + } +} diff --git a/test/Microsoft.AspNet.CookiePolicy.Test/project.json b/test/Microsoft.AspNet.CookiePolicy.Test/project.json new file mode 100644 index 000000000..672774509 --- /dev/null +++ b/test/Microsoft.AspNet.CookiePolicy.Test/project.json @@ -0,0 +1,21 @@ +{ + "compilationOptions": { + "warningsAsErrors": true + }, + "dependencies": { + "Microsoft.AspNet.CookiePolicy": "1.0.0-*", + "Microsoft.AspNet.TestHost": "1.0.0-*", + "Microsoft.Framework.DependencyInjection": "1.0.0-*", + "Moq": "4.2.1312.1622", + "xunit.runner.aspnet": "2.0.0-aspnet-*" + }, + "commands": { + "test": "xunit.runner.aspnet" + }, + "frameworks": { + "dnx451": { + "dependencies": { + } + } + } +} From 8d7a28fc0fa27b7625ad020933dadfa33a86eeb1 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 15 Sep 2015 17:51:18 -0700 Subject: [PATCH 02/13] Fix doc comments --- .../CookiePolicyAppBuilderExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyAppBuilderExtensions.cs b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyAppBuilderExtensions.cs index 4d03d7615..1b1fbbe49 100644 --- a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyAppBuilderExtensions.cs +++ b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyAppBuilderExtensions.cs @@ -12,7 +12,7 @@ namespace Microsoft.AspNet.Builder public static class CookiePolicyAppBuilderExtensions { /// - /// Adds a claims transformation middleware to your web application pipeline. + /// Adds a cookie policy middleware to your web application pipeline. /// /// The IApplicationBuilder passed to your configuration method /// The original app parameter @@ -22,7 +22,7 @@ public static IApplicationBuilder UseCookiePolicy(this IApplicationBuilder app) } /// - /// Adds a claims transformation middleware to your web application pipeline. + /// Adds a cookie policy middleware to your web application pipeline. /// /// The IApplicationBuilder passed to your configuration method /// Used to configure the options for the middleware From e92c41c1d9b325752d026750f6a7413e3761c86c Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 15 Sep 2015 18:10:13 -0700 Subject: [PATCH 03/13] Add a map test --- .../CookiePolicyAppBuilderExtensions.cs | 12 ++- .../CookiePolicyMiddleware.cs | 11 +-- .../CookiePolicyTests.cs | 84 +++++++++++++------ 3 files changed, 70 insertions(+), 37 deletions(-) diff --git a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyAppBuilderExtensions.cs b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyAppBuilderExtensions.cs index 1b1fbbe49..aa8c9af01 100644 --- a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyAppBuilderExtensions.cs +++ b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyAppBuilderExtensions.cs @@ -15,10 +15,11 @@ public static class CookiePolicyAppBuilderExtensions /// Adds a cookie policy middleware to your web application pipeline. /// /// The IApplicationBuilder passed to your configuration method + /// The options for the middleware /// The original app parameter - public static IApplicationBuilder UseCookiePolicy(this IApplicationBuilder app) + public static IApplicationBuilder UseCookiePolicy(this IApplicationBuilder app, CookiePolicyOptions options) { - return app.UseCookiePolicy(configureOptions: o => { }); + return app.UseMiddleware(options); } /// @@ -29,7 +30,12 @@ public static IApplicationBuilder UseCookiePolicy(this IApplicationBuilder app) /// The original app parameter public static IApplicationBuilder UseCookiePolicy(this IApplicationBuilder app, Action configureOptions) { - return app.UseMiddleware(configureOptions); + var options = new CookiePolicyOptions(); + if (configureOptions != null) + { + configureOptions(options); + } + return app.UseCookiePolicy(options); } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs index 98ee01baf..36c706e85 100644 --- a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs +++ b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs @@ -1,12 +1,10 @@ // 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.Threading.Tasks; using Microsoft.AspNet.Builder; using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Features; -using Microsoft.Framework.OptionsModel; using Microsoft.AspNet.Http.Features.Internal; namespace Microsoft.AspNet.CookiePolicy @@ -17,14 +15,9 @@ public class CookiePolicyMiddleware public CookiePolicyMiddleware( RequestDelegate next, - IOptions options, - Action configureOptions) + CookiePolicyOptions options) { - Options = options.Value ?? new CookiePolicyOptions(); // REVIEW: or should we throw ArgumentNull exception? - if (configureOptions != null) - { - configureOptions(Options); - } + Options = options; _next = next; } diff --git a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs index 4ce8beea8..e48142c62 100644 --- a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs +++ b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs @@ -5,7 +5,6 @@ using Microsoft.AspNet.Builder; using Microsoft.AspNet.Http; using Microsoft.AspNet.TestHost; -using Microsoft.Framework.DependencyInjection; using Xunit; namespace Microsoft.AspNet.CookiePolicy.Test @@ -18,7 +17,6 @@ public async Task CookiePolicySecureAlwaysSetsSecure() var server = TestServer.Create(app => { app.UseCookiePolicy(options => options.Secure = SecurePolicy.Always); - app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); app.Run(context => { context.Response.Cookies.Append("A", "A"); @@ -27,8 +25,7 @@ public async Task CookiePolicySecureAlwaysSetsSecure() context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); return Task.FromResult(0); }); - }, - services => services.AddAuthentication()); + }); var transaction = await server.SendAsync("http://example.com/login"); @@ -39,13 +36,63 @@ public async Task CookiePolicySecureAlwaysSetsSecure() Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); } + [Fact] + public async Task CanMapTwoCookiePolicies() + { + var server = TestServer.Create(app => + { + app.Map("/always", map => + { + map.UseCookiePolicy(options => options.Secure = SecurePolicy.Always); + map.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }); + }); + app.Map("/none", map => + { + map.UseCookiePolicy(options => options.Secure = SecurePolicy.None); + map.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }); + }); + + }); + + var transaction = await server.SendAsync("http://example.com/Always"); + + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + + transaction = await server.SendAsync("http://example.com/None"); + + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + + } + + [Fact] public async Task CookiePolicySecureNoneLeavesSecureAlone() { var server = TestServer.Create(app => { app.UseCookiePolicy(options => options.Secure = SecurePolicy.None); - app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); app.Run(context => { context.Response.Cookies.Append("A", "A"); @@ -54,8 +101,7 @@ public async Task CookiePolicySecureNoneLeavesSecureAlone() context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); return Task.FromResult(0); }); - }, - services => services.AddAuthentication()); + }); var transaction = await server.SendAsync("http://example.com/login"); @@ -72,7 +118,6 @@ public async Task CookiePolicySecureSameAsRequestIsFalseOnHttp() var server = TestServer.Create(app => { app.UseCookiePolicy(options => options.Secure = SecurePolicy.SameAsRequest); - app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); app.Run(context => { context.Response.Cookies.Append("A", "A"); @@ -81,8 +126,7 @@ public async Task CookiePolicySecureSameAsRequestIsFalseOnHttp() context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); return Task.FromResult(0); }); - }, - services => services.AddAuthentication()); + }); var transaction = await server.SendAsync("http://example.com/login"); @@ -99,7 +143,6 @@ public async Task CookiePolicySecureSameAsRequestIsTrueOnHttps() var server = TestServer.Create(app => { app.UseCookiePolicy(options => options.Secure = SecurePolicy.SameAsRequest); - app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); app.Run(context => { context.Response.Cookies.Append("A", "A"); @@ -108,8 +151,7 @@ public async Task CookiePolicySecureSameAsRequestIsTrueOnHttps() context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); return Task.FromResult(0); }); - }, - services => services.AddAuthentication()); + }); var transaction = await server.SendAsync("https://example.com/login"); @@ -126,7 +168,6 @@ public async Task CookiePolicyHttpOnlyAlwaysSetsHttpOnly() var server = TestServer.Create(app => { app.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.Always); - app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); app.Run(context => { context.Response.Cookies.Append("A", "A"); @@ -135,8 +176,7 @@ public async Task CookiePolicyHttpOnlyAlwaysSetsHttpOnly() context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); return Task.FromResult(0); }); - }, - services => services.AddAuthentication()); + }); var transaction = await server.SendAsync("http://example.com/login"); @@ -153,7 +193,6 @@ public async Task CookiePolicyHttpOnlyNoneLeavesItAlone() var server = TestServer.Create(app => { app.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.None); - app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); app.Run(context => { context.Response.Cookies.Append("A", "A"); @@ -162,8 +201,7 @@ public async Task CookiePolicyHttpOnlyNoneLeavesItAlone() context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); return Task.FromResult(0); }); - }, - services => services.AddAuthentication()); + }); var transaction = await server.SendAsync("http://example.com/login"); @@ -180,7 +218,6 @@ public async Task CookiePolicyCanHijackAppend() var server = TestServer.Create(app => { app.UseCookiePolicy(options => options.OnAppendCookie = ctx => ctx.CookieName = ctx.CookieValue = "Hao"); - app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); app.Run(context => { context.Response.Cookies.Append("A", "A"); @@ -189,8 +226,7 @@ public async Task CookiePolicyCanHijackAppend() context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); return Task.FromResult(0); }); - }, - services => services.AddAuthentication()); + }); var transaction = await server.SendAsync("http://example.com/login"); @@ -207,7 +243,6 @@ public async Task CookiePolicyCanHijackDelete() var server = TestServer.Create(app => { app.UseCookiePolicy(options => options.OnDeleteCookie = ctx => ctx.CookieName = "A"); - app.UseCookieAuthentication(options => options.LoginPath = new PathString("/page")); app.Run(context => { context.Response.Cookies.Delete("A"); @@ -216,8 +251,7 @@ public async Task CookiePolicyCanHijackDelete() context.Response.Cookies.Delete("D", new CookieOptions { Secure = true }); return Task.FromResult(0); }); - }, - services => services.AddAuthentication()); + }); var transaction = await server.SendAsync("http://example.com/login"); From 94648167a1eec320cb5ab4f97b1b63033b17ac7c Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 15 Sep 2015 18:19:31 -0700 Subject: [PATCH 04/13] Use map test for everything --- .../CookiePolicyTests.cs | 175 +++++------------- 1 file changed, 46 insertions(+), 129 deletions(-) diff --git a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs index e48142c62..62e9b2993 100644 --- a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs +++ b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs @@ -12,36 +12,11 @@ namespace Microsoft.AspNet.CookiePolicy.Test public class CookiePolicyTests { [Fact] - public async Task CookiePolicySecureAlwaysSetsSecure() + public async Task UseMapToTestSecurePolicyAndHttpOnlyModes() { var server = TestServer.Create(app => { - app.UseCookiePolicy(options => options.Secure = SecurePolicy.Always); - app.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); - return Task.FromResult(0); - }); - }); - - var transaction = await server.SendAsync("http://example.com/login"); - - Assert.NotNull(transaction.SetCookie); - Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]); - Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); - Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); - Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); - } - - [Fact] - public async Task CanMapTwoCookiePolicies() - { - var server = TestServer.Create(app => - { - app.Map("/always", map => + app.Map("/secureAlways", map => { map.UseCookiePolicy(options => options.Secure = SecurePolicy.Always); map.Run(context => @@ -53,7 +28,7 @@ public async Task CanMapTwoCookiePolicies() return Task.FromResult(0); }); }); - app.Map("/none", map => + app.Map("/secureNone", map => { map.UseCookiePolicy(options => options.Secure = SecurePolicy.None); map.Run(context => @@ -65,10 +40,46 @@ public async Task CanMapTwoCookiePolicies() return Task.FromResult(0); }); }); + app.Map("/secureSame", map => + { + map.UseCookiePolicy(options => options.Secure = SecurePolicy.SameAsRequest); + map.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }); + }); + app.Map("/httpOnlyNone", map => + { + map.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.None); + map.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); + return Task.FromResult(0); + }); + }); + app.Map("/httpOnlyAlways", map => + { + map.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.Always); + map.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); + return Task.FromResult(0); + }); + }); }); - var transaction = await server.SendAsync("http://example.com/Always"); + var transaction = await server.SendAsync("http://example.com/secureAlways"); Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]); @@ -76,140 +87,46 @@ public async Task CanMapTwoCookiePolicies() Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); - transaction = await server.SendAsync("http://example.com/None"); - - Assert.NotNull(transaction.SetCookie); - Assert.Equal("A=A; path=/", transaction.SetCookie[0]); - Assert.Equal("B=B; path=/", transaction.SetCookie[1]); - Assert.Equal("C=C; path=/", transaction.SetCookie[2]); - Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); - - } - - - [Fact] - public async Task CookiePolicySecureNoneLeavesSecureAlone() - { - var server = TestServer.Create(app => - { - app.UseCookiePolicy(options => options.Secure = SecurePolicy.None); - app.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); - return Task.FromResult(0); - }); - }); - - var transaction = await server.SendAsync("http://example.com/login"); + transaction = await server.SendAsync("http://example.com/secureNone"); Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/", transaction.SetCookie[0]); Assert.Equal("B=B; path=/", transaction.SetCookie[1]); Assert.Equal("C=C; path=/", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); - } - - [Fact] - public async Task CookiePolicySecureSameAsRequestIsFalseOnHttp() - { - var server = TestServer.Create(app => - { - app.UseCookiePolicy(options => options.Secure = SecurePolicy.SameAsRequest); - app.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); - return Task.FromResult(0); - }); - }); - var transaction = await server.SendAsync("http://example.com/login"); + transaction = await server.SendAsync("http://example.com/secureSame"); Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/", transaction.SetCookie[0]); Assert.Equal("B=B; path=/", transaction.SetCookie[1]); Assert.Equal("C=C; path=/", transaction.SetCookie[2]); Assert.Equal("D=D; path=/", transaction.SetCookie[3]); - } - [Fact] - public async Task CookiePolicySecureSameAsRequestIsTrueOnHttps() - { - var server = TestServer.Create(app => - { - app.UseCookiePolicy(options => options.Secure = SecurePolicy.SameAsRequest); - app.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); - return Task.FromResult(0); - }); - }); - - var transaction = await server.SendAsync("https://example.com/login"); + transaction = await server.SendAsync("https://example.com/secureSame"); Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]); Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); - } - [Fact] - public async Task CookiePolicyHttpOnlyAlwaysSetsHttpOnly() - { - var server = TestServer.Create(app => - { - app.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.Always); - app.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); - return Task.FromResult(0); - }); - }); - - var transaction = await server.SendAsync("http://example.com/login"); + transaction = await server.SendAsync("http://example.com/httpOnlyAlways"); Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/; httponly", transaction.SetCookie[0]); Assert.Equal("B=B; path=/; httponly", transaction.SetCookie[1]); Assert.Equal("C=C; path=/; httponly", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]); - } - - [Fact] - public async Task CookiePolicyHttpOnlyNoneLeavesItAlone() - { - var server = TestServer.Create(app => - { - app.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.None); - app.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); - return Task.FromResult(0); - }); - }); - var transaction = await server.SendAsync("http://example.com/login"); + transaction = await server.SendAsync("http://example.com/httpOnlyNone"); Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/", transaction.SetCookie[0]); Assert.Equal("B=B; path=/", transaction.SetCookie[1]); Assert.Equal("C=C; path=/", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]); + } [Fact] From b97bed99148a4fd1871abd4fb128383fcb6009b7 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 15 Sep 2015 18:22:43 -0700 Subject: [PATCH 05/13] Nuke comment --- src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs index 36c706e85..11429ed3e 100644 --- a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs +++ b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs @@ -133,7 +133,6 @@ public void Delete(string key, CookieOptions options) Cookies.Delete(key, options); } - // Returns true if cookie options need to be applied private void ApplyPolicy(CookieOptions options) { switch (Policy.Secure) From 5219539c47e492b410f9522edc5b35c7bdc724a7 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 15 Sep 2015 18:24:21 -0700 Subject: [PATCH 06/13] Update comment --- .../CookiePolicyMiddleware.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs index 11429ed3e..e1daa55e1 100644 --- a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs +++ b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs @@ -25,11 +25,8 @@ public CookiePolicyMiddleware( public Task Invoke(HttpContext context) { - // Check if there is a SendFile feature already present - if (context.Features.Get() == null) - { - context.Features.Set(new CookiesWrapperFeature(context, Options)); - } + // REVIEW: Do we need to check if there is a Cookie feature already present like SendFile?? + context.Features.Set(new CookiesWrapperFeature(context, Options)); return _next(context); } From d65aa607ffb7b1f415212b00665780de29561692 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Wed, 16 Sep 2015 13:26:52 -0700 Subject: [PATCH 07/13] CR feedback --- .../CookiePolicyMiddleware.cs | 13 +- .../project.json | 2 - .../Cookies/CookieMiddlewareTests.cs | 4 +- .../CookiePolicyTests.cs | 171 ++++++++++-------- .../project.json | 7 +- 5 files changed, 103 insertions(+), 94 deletions(-) diff --git a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs index e1daa55e1..9b0866a94 100644 --- a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs +++ b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs @@ -71,19 +71,11 @@ private bool PolicyRequiresCookieOptions() public void Append(string key, string value) { - if (PolicyRequiresCookieOptions()) + if (PolicyRequiresCookieOptions() || Policy.OnAppendCookie != null) { Append(key, value, new CookieOptions()); return; } - - if (Policy.OnAppendCookie != null) - { - var context = new AppendCookieContext(Context, options: null, name: key, value: value); - Policy.OnAppendCookie(context); - key = context.CookieName; - value = context.CookieValue; - } Cookies.Append(key, value); } @@ -102,13 +94,12 @@ public void Append(string key, string value, CookieOptions options) public void Delete(string key) { - if (PolicyRequiresCookieOptions()) + if (PolicyRequiresCookieOptions() || Policy.OnDeleteCookie != null) { Delete(key, new CookieOptions()); return; } - if (Policy.OnDeleteCookie != null) { var context = new DeleteCookieContext(Context, options: null, name: key); diff --git a/src/Microsoft.AspNet.CookiePolicy/project.json b/src/Microsoft.AspNet.CookiePolicy/project.json index 609eecd89..a4eecd438 100644 --- a/src/Microsoft.AspNet.CookiePolicy/project.json +++ b/src/Microsoft.AspNet.CookiePolicy/project.json @@ -6,8 +6,6 @@ "url": "git://github.com/aspnet/security" }, "dependencies": { - "Microsoft.AspNet.Http.Abstractions": "1.0.0-*", - "Microsoft.AspNet.Http.Features": "1.0.0-*", "Microsoft.AspNet.Http": "1.0.0-*", "Microsoft.Framework.OptionsModel": "1.0.0-*" }, diff --git a/test/Microsoft.AspNet.Authentication.Test/Cookies/CookieMiddlewareTests.cs b/test/Microsoft.AspNet.Authentication.Test/Cookies/CookieMiddlewareTests.cs index 4af812193..3fb293a59 100644 --- a/test/Microsoft.AspNet.Authentication.Test/Cookies/CookieMiddlewareTests.cs +++ b/test/Microsoft.AspNet.Authentication.Test/Cookies/CookieMiddlewareTests.cs @@ -731,7 +731,7 @@ public async Task ChallengeDoesNotSet401OnUnauthorized() transaction.Response.StatusCode.ShouldBe(HttpStatusCode.OK); } -/* [Fact] + [Fact] public async Task UseCookieWithInstanceDoesntUseSharedOptions() { var server = TestServer.Create(app => @@ -761,7 +761,7 @@ public async Task UseCookieWithOutInstanceDoesUseSharedOptions() transaction.Response.StatusCode.ShouldBe(HttpStatusCode.OK); Assert.True(transaction.SetCookie[0].StartsWith("One=")); - }*/ + } [Fact] public async Task MapWithSignInOnlyRedirectToReturnUrlOnLoginPath() diff --git a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs index 62e9b2993..6219fa6a1 100644 --- a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs +++ b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs @@ -12,73 +12,9 @@ namespace Microsoft.AspNet.CookiePolicy.Test public class CookiePolicyTests { [Fact] - public async Task UseMapToTestSecurePolicyAndHttpOnlyModes() + public async Task SecureAlwaysSetsSecure() { - var server = TestServer.Create(app => - { - app.Map("/secureAlways", map => - { - map.UseCookiePolicy(options => options.Secure = SecurePolicy.Always); - map.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); - return Task.FromResult(0); - }); - }); - app.Map("/secureNone", map => - { - map.UseCookiePolicy(options => options.Secure = SecurePolicy.None); - map.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); - return Task.FromResult(0); - }); - }); - app.Map("/secureSame", map => - { - map.UseCookiePolicy(options => options.Secure = SecurePolicy.SameAsRequest); - map.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); - return Task.FromResult(0); - }); - }); - app.Map("/httpOnlyNone", map => - { - map.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.None); - map.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); - return Task.FromResult(0); - }); - }); - app.Map("/httpOnlyAlways", map => - { - map.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.Always); - map.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); - return Task.FromResult(0); - }); - }); - - }); - + var server = CreateTestServer(); var transaction = await server.SendAsync("http://example.com/secureAlways"); Assert.NotNull(transaction.SetCookie); @@ -86,16 +22,26 @@ public async Task UseMapToTestSecurePolicyAndHttpOnlyModes() Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + } - transaction = await server.SendAsync("http://example.com/secureNone"); + [Fact] + public async Task SecureNoneLeavesSecureUnchanged() + { + var server = CreateTestServer(); + var transaction = await server.SendAsync("http://example.com/secureNone"); Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/", transaction.SetCookie[0]); Assert.Equal("B=B; path=/", transaction.SetCookie[1]); Assert.Equal("C=C; path=/", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + } - transaction = await server.SendAsync("http://example.com/secureSame"); + [Fact] + public async Task SecureSameUsesRequest() + { + var server = CreateTestServer(); + var transaction = await server.SendAsync("http://example.com/secureSame"); Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/", transaction.SetCookie[0]); @@ -110,23 +56,32 @@ public async Task UseMapToTestSecurePolicyAndHttpOnlyModes() Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + } - transaction = await server.SendAsync("http://example.com/httpOnlyAlways"); + [Fact] + public async Task HttpOnlyAlwaysSetsItAlways() + { + var server = CreateTestServer(); + var transaction = await server.SendAsync("http://example.com/httpOnlyAlways"); Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/; httponly", transaction.SetCookie[0]); Assert.Equal("B=B; path=/; httponly", transaction.SetCookie[1]); Assert.Equal("C=C; path=/; httponly", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]); + } - transaction = await server.SendAsync("http://example.com/httpOnlyNone"); + [Fact] + public async Task HttpOnlyNoneLeavesItAlone() + { + var server = CreateTestServer(); + var transaction = await server.SendAsync("http://example.com/httpOnlyNone"); Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/", transaction.SetCookie[0]); Assert.Equal("B=B; path=/", transaction.SetCookie[1]); Assert.Equal("C=C; path=/", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]); - } [Fact] @@ -173,9 +128,77 @@ public async Task CookiePolicyCanHijackDelete() var transaction = await server.SendAsync("http://example.com/login"); Assert.NotNull(transaction.SetCookie); - Assert.Equal(2, transaction.SetCookie.Count); - Assert.Equal("A=; expires=Thu, 01-Jan-1970 00:00:00 GMT", transaction.SetCookie[0]); - Assert.Equal("A=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", transaction.SetCookie[1]); + Assert.Equal(1, transaction.SetCookie.Count); + Assert.Equal("A=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", transaction.SetCookie[0]); + } + + private TestServer CreateTestServer() + { + return TestServer.Create(app => + { + app.Map("/secureAlways", map => + { + map.UseCookiePolicy(options => options.Secure = SecurePolicy.Always); + map.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }); + }); + app.Map("/secureNone", map => + { + map.UseCookiePolicy(options => options.Secure = SecurePolicy.None); + map.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }); + }); + app.Map("/secureSame", map => + { + map.UseCookiePolicy(options => options.Secure = SecurePolicy.SameAsRequest); + map.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }); + }); + app.Map("/httpOnlyNone", map => + { + map.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.None); + map.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); + return Task.FromResult(0); + }); + }); + app.Map("/httpOnlyAlways", map => + { + map.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.Always); + map.Run(context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); + return Task.FromResult(0); + }); + }); + + }); + } } diff --git a/test/Microsoft.AspNet.CookiePolicy.Test/project.json b/test/Microsoft.AspNet.CookiePolicy.Test/project.json index 672774509..336c06a37 100644 --- a/test/Microsoft.AspNet.CookiePolicy.Test/project.json +++ b/test/Microsoft.AspNet.CookiePolicy.Test/project.json @@ -6,16 +6,13 @@ "Microsoft.AspNet.CookiePolicy": "1.0.0-*", "Microsoft.AspNet.TestHost": "1.0.0-*", "Microsoft.Framework.DependencyInjection": "1.0.0-*", - "Moq": "4.2.1312.1622", "xunit.runner.aspnet": "2.0.0-aspnet-*" }, "commands": { "test": "xunit.runner.aspnet" }, "frameworks": { - "dnx451": { - "dependencies": { - } - } + "dnx451": { }, + "dnxcore50": { } } } From 507f349fca36c12382cbd36e05a1f96541ea6c0e Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 17 Sep 2015 12:28:05 -0700 Subject: [PATCH 08/13] Wrap feature --- .../CookiePolicyMiddleware.cs | 46 +++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs index 9b0866a94..fe0fcb494 100644 --- a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs +++ b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.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; using System.Threading.Tasks; using Microsoft.AspNet.Builder; using Microsoft.AspNet.Http; @@ -35,7 +36,7 @@ private class CookiesWrapperFeature : IResponseCookiesFeature { public CookiesWrapperFeature(HttpContext context, CookiePolicyOptions options) { - Wrapper = new CookiesWrapper(context, options, context.Response.Cookies); + Wrapper = new CookiesWrapper(context, options); } public IResponseCookies Wrapper { get; } @@ -51,16 +52,23 @@ public IResponseCookies Cookies private class CookiesWrapper : IResponseCookies { - public CookiesWrapper(HttpContext context, CookiePolicyOptions options, IResponseCookies cookies) + public CookiesWrapper(HttpContext context, CookiePolicyOptions options) { Context = context; - Cookies = cookies; + Feature = context.Features.Get() ?? new ResponseCookiesFeature(context.Features); Policy = options; } public HttpContext Context { get; } - public IResponseCookies Cookies { get; } + public IResponseCookiesFeature Feature { get; } + + public IResponseCookies Cookies { + get + { + return Feature.Cookies; + } + } public CookiePolicyOptions Policy { get; } @@ -74,13 +82,21 @@ public void Append(string key, string value) if (PolicyRequiresCookieOptions() || Policy.OnAppendCookie != null) { Append(key, value, new CookieOptions()); - return; } - Cookies.Append(key, value); + else + { + Cookies.Append(key, value); + + } } public void Append(string key, string value, CookieOptions options) { + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + ApplyPolicy(options); if (Policy.OnAppendCookie != null) { @@ -97,20 +113,20 @@ public void Delete(string key) if (PolicyRequiresCookieOptions() || Policy.OnDeleteCookie != null) { Delete(key, new CookieOptions()); - return; } - - if (Policy.OnDeleteCookie != null) + else { - var context = new DeleteCookieContext(Context, options: null, name: key); - Policy.OnDeleteCookie(context); - key = context.CookieName; + Cookies.Delete(key); } - Cookies.Delete(key); } public void Delete(string key, CookieOptions options) { + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + ApplyPolicy(options); if (Policy.OnDeleteCookie != null) { @@ -133,6 +149,8 @@ private void ApplyPolicy(CookieOptions options) break; case SecurePolicy.None: break; + default: + throw new InvalidOperationException(); } switch (Policy.HttpOnly) { @@ -141,6 +159,8 @@ private void ApplyPolicy(CookieOptions options) break; case HttpOnlyPolicy.None: break; + default: + throw new InvalidOperationException(); } } } From 41b4039005876bbb78a2dcb853b5b34b82338b30 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 17 Sep 2015 12:39:31 -0700 Subject: [PATCH 09/13] Add test for cookie feature wrapping --- .../CookiePolicyTests.cs | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs index 6219fa6a1..14f441388 100644 --- a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs +++ b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs @@ -1,9 +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. +using System; using System.Threading.Tasks; using Microsoft.AspNet.Builder; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Features; +using Microsoft.AspNet.Http.Features.Internal; +using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.TestHost; using Xunit; @@ -132,6 +136,59 @@ public async Task CookiePolicyCanHijackDelete() Assert.Equal("A=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/", transaction.SetCookie[0]); } + [Fact] + public async Task CookiePolicyCallsCookieFeature() + { + var server = TestServer.Create(app => + { + app.Use(next => context => + { + context.Features.Set(new TestCookieFeature()); + return next(context); + }); + app.UseCookiePolicy(options => options.OnDeleteCookie = ctx => ctx.CookieName = "A"); + app.Run(context => + { + Assert.Throws(() => context.Response.Cookies.Delete("A")); + Assert.Throws(() => context.Response.Cookies.Delete("A", new CookieOptions())); + Assert.Throws(() => context.Response.Cookies.Append("A", "A")); + Assert.Throws(() => context.Response.Cookies.Append("A", "A", new CookieOptions())); + return context.Response.WriteAsync("Done"); + }); + }); + + var transaction = await server.SendAsync("http://example.com/login"); + Assert.Equal("Done", transaction.ResponseText); + } + + private class TestCookieFeature : IResponseCookiesFeature + { + public IResponseCookies Cookies { get; } = new BadCookies(); + + private class BadCookies : IResponseCookies + { + public void Append(string key, string value) + { + throw new NotImplementedException(); + } + + public void Append(string key, string value, CookieOptions options) + { + throw new NotImplementedException(); + } + + public void Delete(string key) + { + throw new NotImplementedException(); + } + + public void Delete(string key, CookieOptions options) + { + throw new NotImplementedException(); + } + } + } + private TestServer CreateTestServer() { return TestServer.Create(app => From aa9a68bafa163ce23853960a6d7612b8aff0bfd4 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 17 Sep 2015 12:40:42 -0700 Subject: [PATCH 10/13] Remove review comment --- src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs index fe0fcb494..4004e6520 100644 --- a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs +++ b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs @@ -26,9 +26,7 @@ public CookiePolicyMiddleware( public Task Invoke(HttpContext context) { - // REVIEW: Do we need to check if there is a Cookie feature already present like SendFile?? context.Features.Set(new CookiesWrapperFeature(context, Options)); - return _next(context); } From 5b6774c5c880a82d3b1ed95153adaf7764a39b58 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 17 Sep 2015 12:42:46 -0700 Subject: [PATCH 11/13] Remove options dependency --- src/Microsoft.AspNet.CookiePolicy/project.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Microsoft.AspNet.CookiePolicy/project.json b/src/Microsoft.AspNet.CookiePolicy/project.json index a4eecd438..6d78e5eb8 100644 --- a/src/Microsoft.AspNet.CookiePolicy/project.json +++ b/src/Microsoft.AspNet.CookiePolicy/project.json @@ -6,8 +6,7 @@ "url": "git://github.com/aspnet/security" }, "dependencies": { - "Microsoft.AspNet.Http": "1.0.0-*", - "Microsoft.Framework.OptionsModel": "1.0.0-*" + "Microsoft.AspNet.Http": "1.0.0-*" }, "frameworks": { "dnx451": { }, From bf4fe44dd943d42a7230d979776aa9cd79e347eb Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 17 Sep 2015 14:44:37 -0700 Subject: [PATCH 12/13] Refactor tests --- .../CookiePolicyMiddleware.cs | 15 +- .../CookiePolicyTests.cs | 202 ++++++++---------- 2 files changed, 103 insertions(+), 114 deletions(-) diff --git a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs index 4004e6520..d726d80cd 100644 --- a/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs +++ b/src/Microsoft.AspNet.CookiePolicy/CookiePolicyMiddleware.cs @@ -26,15 +26,16 @@ public CookiePolicyMiddleware( public Task Invoke(HttpContext context) { - context.Features.Set(new CookiesWrapperFeature(context, Options)); + var feature = context.Features.Get() ?? new ResponseCookiesFeature(context.Features); + context.Features.Set(new CookiesWrapperFeature(context, Options, feature)); return _next(context); } private class CookiesWrapperFeature : IResponseCookiesFeature { - public CookiesWrapperFeature(HttpContext context, CookiePolicyOptions options) + public CookiesWrapperFeature(HttpContext context, CookiePolicyOptions options, IResponseCookiesFeature feature) { - Wrapper = new CookiesWrapper(context, options); + Wrapper = new CookiesWrapper(context, options, feature); } public IResponseCookies Wrapper { get; } @@ -50,10 +51,10 @@ public IResponseCookies Cookies private class CookiesWrapper : IResponseCookies { - public CookiesWrapper(HttpContext context, CookiePolicyOptions options) + public CookiesWrapper(HttpContext context, CookiePolicyOptions options, IResponseCookiesFeature feature) { Context = context; - Feature = context.Features.Get() ?? new ResponseCookiesFeature(context.Features); + Feature = feature; Policy = options; } @@ -61,7 +62,8 @@ public CookiesWrapper(HttpContext context, CookiePolicyOptions options) public IResponseCookiesFeature Feature { get; } - public IResponseCookies Cookies { + public IResponseCookies Cookies + { get { return Feature.Cookies; @@ -84,7 +86,6 @@ public void Append(string key, string value) else { Cookies.Append(key, value); - } } diff --git a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs index 14f441388..12a9a503c 100644 --- a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs +++ b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs @@ -7,7 +7,6 @@ using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Features; using Microsoft.AspNet.Http.Features.Internal; -using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.TestHost; using Xunit; @@ -15,77 +14,119 @@ namespace Microsoft.AspNet.CookiePolicy.Test { public class CookiePolicyTests { + private RequestDelegate SecureCookieAppends = context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); + return Task.FromResult(0); + }; + private RequestDelegate HttpCookieAppends = context => + { + context.Response.Cookies.Append("A", "A"); + context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); + context.Response.Cookies.Append("C", "C", new CookieOptions()); + context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); + return Task.FromResult(0); + }; + + [Fact] public async Task SecureAlwaysSetsSecure() { - var server = CreateTestServer(); - var transaction = await server.SendAsync("http://example.com/secureAlways"); - - Assert.NotNull(transaction.SetCookie); - Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]); - Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); - Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); - Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + await RunTest("/secureAlways", + options => options.Secure = SecurePolicy.Always, + SecureCookieAppends, + async server => + { + var transaction = await server.SendAsync("http://example.com/secureAlways"); + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + }); } [Fact] public async Task SecureNoneLeavesSecureUnchanged() { - var server = CreateTestServer(); - var transaction = await server.SendAsync("http://example.com/secureNone"); - - Assert.NotNull(transaction.SetCookie); - Assert.Equal("A=A; path=/", transaction.SetCookie[0]); - Assert.Equal("B=B; path=/", transaction.SetCookie[1]); - Assert.Equal("C=C; path=/", transaction.SetCookie[2]); - Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + await RunTest("/secureNone", + options => options.Secure = SecurePolicy.None, + SecureCookieAppends, + async server => + { + var transaction = await server.SendAsync("http://example.com/secureNone"); + Assert.NotNull(transaction.SetCookie); + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + }); } [Fact] public async Task SecureSameUsesRequest() { - var server = CreateTestServer(); - var transaction = await server.SendAsync("http://example.com/secureSame"); + await RunTest("/secureSame", + options => options.Secure = SecurePolicy.SameAsRequest, + SecureCookieAppends, + async server => + { + var transaction = await server.SendAsync("http://example.com/secureSame"); - Assert.NotNull(transaction.SetCookie); - Assert.Equal("A=A; path=/", transaction.SetCookie[0]); - Assert.Equal("B=B; path=/", transaction.SetCookie[1]); - Assert.Equal("C=C; path=/", transaction.SetCookie[2]); - Assert.Equal("D=D; path=/", transaction.SetCookie[3]); + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/", transaction.SetCookie[3]); - transaction = await server.SendAsync("https://example.com/secureSame"); + transaction = await server.SendAsync("https://example.com/secureSame"); - Assert.NotNull(transaction.SetCookie); - Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]); - Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); - Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); - Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); + }); } [Fact] public async Task HttpOnlyAlwaysSetsItAlways() { - var server = CreateTestServer(); - var transaction = await server.SendAsync("http://example.com/httpOnlyAlways"); + await RunTest("/httpOnlyAlways", + options => options.HttpOnly = HttpOnlyPolicy.Always, + HttpCookieAppends, + async server => + { + var transaction = await server.SendAsync("http://example.com/httpOnlyAlways"); - Assert.NotNull(transaction.SetCookie); - Assert.Equal("A=A; path=/; httponly", transaction.SetCookie[0]); - Assert.Equal("B=B; path=/; httponly", transaction.SetCookie[1]); - Assert.Equal("C=C; path=/; httponly", transaction.SetCookie[2]); - Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]); + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/; httponly", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/; httponly", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/; httponly", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]); + }); } [Fact] public async Task HttpOnlyNoneLeavesItAlone() { - var server = CreateTestServer(); - var transaction = await server.SendAsync("http://example.com/httpOnlyNone"); + await RunTest("/httpOnlyNone", + options => options.HttpOnly = HttpOnlyPolicy.None, + HttpCookieAppends, + async server => + { + var transaction = await server.SendAsync("http://example.com/httpOnlyNone"); - Assert.NotNull(transaction.SetCookie); - Assert.Equal("A=A; path=/", transaction.SetCookie[0]); - Assert.Equal("B=B; path=/", transaction.SetCookie[1]); - Assert.Equal("C=C; path=/", transaction.SetCookie[2]); - Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]); + Assert.NotNull(transaction.SetCookie); + Assert.Equal("A=A; path=/", transaction.SetCookie[0]); + Assert.Equal("B=B; path=/", transaction.SetCookie[1]); + Assert.Equal("C=C; path=/", transaction.SetCookie[2]); + Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]); + }); } [Fact] @@ -189,74 +230,21 @@ public void Delete(string key, CookieOptions options) } } - private TestServer CreateTestServer() + private Task RunTest( + string path, + Action configureCookiePolicy, + RequestDelegate configureSetup, + Func configureTest) { - return TestServer.Create(app => + var server = TestServer.Create(app => { - app.Map("/secureAlways", map => - { - map.UseCookiePolicy(options => options.Secure = SecurePolicy.Always); - map.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); - return Task.FromResult(0); - }); - }); - app.Map("/secureNone", map => + app.Map(path, map => { - map.UseCookiePolicy(options => options.Secure = SecurePolicy.None); - map.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); - return Task.FromResult(0); - }); + map.UseCookiePolicy(configureCookiePolicy); + map.Run(configureSetup); }); - app.Map("/secureSame", map => - { - map.UseCookiePolicy(options => options.Secure = SecurePolicy.SameAsRequest); - map.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { Secure = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { Secure = true }); - return Task.FromResult(0); - }); - }); - app.Map("/httpOnlyNone", map => - { - map.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.None); - map.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); - return Task.FromResult(0); - }); - }); - app.Map("/httpOnlyAlways", map => - { - map.UseCookiePolicy(options => options.HttpOnly = HttpOnlyPolicy.Always); - map.Run(context => - { - context.Response.Cookies.Append("A", "A"); - context.Response.Cookies.Append("B", "B", new CookieOptions { HttpOnly = false }); - context.Response.Cookies.Append("C", "C", new CookieOptions()); - context.Response.Cookies.Append("D", "D", new CookieOptions { HttpOnly = true }); - return Task.FromResult(0); - }); - }); - }); - + return configureTest(server); } - } } \ No newline at end of file From a7f0ba96dc01a2e9b35d85cd51fa2c2da3d8db48 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 17 Sep 2015 14:58:18 -0700 Subject: [PATCH 13/13] Clean up tests --- .../CookiePolicyTests.cs | 68 ++++++++++++------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs index 12a9a503c..78f20c9cf 100644 --- a/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs +++ b/test/Microsoft.AspNet.CookiePolicy.Test/CookiePolicyTests.cs @@ -31,22 +31,21 @@ public class CookiePolicyTests return Task.FromResult(0); }; - [Fact] public async Task SecureAlwaysSetsSecure() { await RunTest("/secureAlways", options => options.Secure = SecurePolicy.Always, SecureCookieAppends, - async server => + new RequestTest("http://example.com/secureAlways", + transaction => { - var transaction = await server.SendAsync("http://example.com/secureAlways"); Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]); Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); - }); + })); } [Fact] @@ -55,16 +54,16 @@ public async Task SecureNoneLeavesSecureUnchanged() await RunTest("/secureNone", options => options.Secure = SecurePolicy.None, SecureCookieAppends, - async server => + new RequestTest("http://example.com/secureNone", + transaction => { - var transaction = await server.SendAsync("http://example.com/secureNone"); Assert.NotNull(transaction.SetCookie); Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/", transaction.SetCookie[0]); Assert.Equal("B=B; path=/", transaction.SetCookie[1]); Assert.Equal("C=C; path=/", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); - }); + })); } [Fact] @@ -73,24 +72,24 @@ public async Task SecureSameUsesRequest() await RunTest("/secureSame", options => options.Secure = SecurePolicy.SameAsRequest, SecureCookieAppends, - async server => + new RequestTest("http://example.com/secureSame", + transaction => { - var transaction = await server.SendAsync("http://example.com/secureSame"); - Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/", transaction.SetCookie[0]); Assert.Equal("B=B; path=/", transaction.SetCookie[1]); Assert.Equal("C=C; path=/", transaction.SetCookie[2]); Assert.Equal("D=D; path=/", transaction.SetCookie[3]); - - transaction = await server.SendAsync("https://example.com/secureSame"); - + }), + new RequestTest("https://example.com/secureSame", + transaction => + { Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/; secure", transaction.SetCookie[0]); Assert.Equal("B=B; path=/; secure", transaction.SetCookie[1]); Assert.Equal("C=C; path=/; secure", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; secure", transaction.SetCookie[3]); - }); + })); } [Fact] @@ -99,16 +98,15 @@ public async Task HttpOnlyAlwaysSetsItAlways() await RunTest("/httpOnlyAlways", options => options.HttpOnly = HttpOnlyPolicy.Always, HttpCookieAppends, - async server => + new RequestTest("http://example.com/httpOnlyAlways", + transaction => { - var transaction = await server.SendAsync("http://example.com/httpOnlyAlways"); - Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/; httponly", transaction.SetCookie[0]); Assert.Equal("B=B; path=/; httponly", transaction.SetCookie[1]); Assert.Equal("C=C; path=/; httponly", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]); - }); + })); } [Fact] @@ -117,16 +115,15 @@ public async Task HttpOnlyNoneLeavesItAlone() await RunTest("/httpOnlyNone", options => options.HttpOnly = HttpOnlyPolicy.None, HttpCookieAppends, - async server => + new RequestTest("http://example.com/httpOnlyNone", + transaction => { - var transaction = await server.SendAsync("http://example.com/httpOnlyNone"); - Assert.NotNull(transaction.SetCookie); Assert.Equal("A=A; path=/", transaction.SetCookie[0]); Assert.Equal("B=B; path=/", transaction.SetCookie[1]); Assert.Equal("C=C; path=/", transaction.SetCookie[2]); Assert.Equal("D=D; path=/; httponly", transaction.SetCookie[3]); - }); + })); } [Fact] @@ -230,11 +227,29 @@ public void Delete(string key, CookieOptions options) } } - private Task RunTest( + private class RequestTest + { + public RequestTest(string testUri, Action verify) + { + TestUri = testUri; + Verification = verify; + } + + public async Task Execute(TestServer server) + { + var transaction = await server.SendAsync(TestUri); + Verification(transaction); + } + + public string TestUri { get; set; } + public Action Verification { get; set; } + } + + private async Task RunTest( string path, Action configureCookiePolicy, RequestDelegate configureSetup, - Func configureTest) + params RequestTest[] tests) { var server = TestServer.Create(app => { @@ -244,7 +259,10 @@ private Task RunTest( map.Run(configureSetup); }); }); - return configureTest(server); + foreach (var test in tests) + { + await test.Execute(server); + } } } } \ No newline at end of file