-
Notifications
You must be signed in to change notification settings - Fork 587
Add new CookiePolicy middleware #452
Changes from 6 commits
c51207e
8d7a28f
e92c41c
9464816
b97bed9
5219539
d65aa60
507f349
41b4039
aa9a68b
5b6774c
bf4fe44
a7f0ba9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
// 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 | ||
{ | ||
/// <summary> | ||
/// Extension methods provided by the cookie policy middleware | ||
/// </summary> | ||
public static class CookiePolicyAppBuilderExtensions | ||
{ | ||
/// <summary> | ||
/// Adds a cookie policy middleware to your web application pipeline. | ||
/// </summary> | ||
/// <param name="app">The IApplicationBuilder passed to your configuration method</param> | ||
/// <param name="options">The options for the middleware</param> | ||
/// <returns>The original app parameter</returns> | ||
public static IApplicationBuilder UseCookiePolicy(this IApplicationBuilder app, CookiePolicyOptions options) | ||
{ | ||
return app.UseMiddleware<CookiePolicyMiddleware>(options); | ||
} | ||
|
||
/// <summary> | ||
/// Adds a cookie policy middleware to your web application pipeline. | ||
/// </summary> | ||
/// <param name="app">The IApplicationBuilder passed to your configuration method</param> | ||
/// <param name="configureOptions">Used to configure the options for the middleware</param> | ||
/// <returns>The original app parameter</returns> | ||
public static IApplicationBuilder UseCookiePolicy(this IApplicationBuilder app, Action<CookiePolicyOptions> configureOptions) | ||
{ | ||
var options = new CookiePolicyOptions(); | ||
if (configureOptions != null) | ||
{ | ||
configureOptions(options); | ||
} | ||
return app.UseCookiePolicy(options); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,157 @@ | ||
// 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.Http.Features; | ||
using Microsoft.AspNet.Http.Features.Internal; | ||
|
||
namespace Microsoft.AspNet.CookiePolicy | ||
{ | ||
public class CookiePolicyMiddleware | ||
{ | ||
private readonly RequestDelegate _next; | ||
|
||
public CookiePolicyMiddleware( | ||
RequestDelegate next, | ||
CookiePolicyOptions options) | ||
{ | ||
Options = options; | ||
_next = next; | ||
} | ||
|
||
public CookiePolicyOptions Options { get; set; } | ||
|
||
public Task Invoke(HttpContext context) | ||
{ | ||
// REVIEW: Do we need to check if there is a Cookie feature already present like SendFile?? | ||
context.Features.Set<IResponseCookiesFeature>(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; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: empty line. |
||
if (Policy.OnAppendCookie != null) | ||
{ | ||
var context = new AppendCookieContext(Context, options: null, name: key, value: value); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if they want to add options in the callback? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you'd always call the other overload if the callback is set. |
||
Policy.OnAppendCookie(context); | ||
key = context.CookieName; | ||
value = context.CookieValue; | ||
} | ||
Cookies.Append(key, value); | ||
} | ||
|
||
public void Append(string key, string value, CookieOptions options) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. argument validation? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There didn't appear to be any argument validation in the base HttpAbstractions ResponseCookies so I just mimiced that behavior since we are just wrapping no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's at least a NotNull check on 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; | ||
} | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: empty line |
||
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); | ||
} | ||
|
||
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; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. default throw? |
||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<AppendCookieContext> OnAppendCookie { get; set; } | ||
public Action<DeleteCookieContext> OnDeleteCookie { get; set; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
<PropertyGroup> | ||
<VisualStudioVersion Condition="'$(VisualStudioVersion)' == ''">14.0</VisualStudioVersion> | ||
<VSToolsPath Condition="'$(VSToolsPath)' == ''">$(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion)</VSToolsPath> | ||
</PropertyGroup> | ||
<Import Project="$(VSToolsPath)\DNX\Microsoft.DNX.Props" Condition="'$(VSToolsPath)' != ''" /> | ||
<PropertyGroup Label="Globals"> | ||
<ProjectGuid>86183dc3-02a8-4a68-8b60-71ecec066e79</ProjectGuid> | ||
<RootNamespace>Microsoft.AspNet.CookiePolicy</RootNamespace> | ||
<BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)'=='' ">..\..\artifacts\obj\$(MSBuildProjectName)</BaseIntermediateOutputPath> | ||
<OutputPath Condition="'$(OutputPath)'=='' ">..\..\artifacts\bin\$(MSBuildProjectName)\</OutputPath> | ||
</PropertyGroup> | ||
|
||
<PropertyGroup> | ||
<SchemaVersion>2.0</SchemaVersion> | ||
</PropertyGroup> | ||
<Import Project="$(VSToolsPath)\DNX\Microsoft.DNX.targets" Condition="'$(VSToolsPath)' != ''" /> | ||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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")] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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-*", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove transitive dependencies on Abstractions, Features. |
||
"Microsoft.AspNet.Http": "1.0.0-*", | ||
"Microsoft.Framework.OptionsModel": "1.0.0-*" | ||
}, | ||
"frameworks": { | ||
"dnx451": { }, | ||
"dnxcore50": { } | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This piece needs review, is it safe to blindly set the feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider wrapping the existing feature (if any) rather than just wrapping context.Response.Cookies. If none, default to a new ResponseCookiesFeature.