Skip to content

Commit

Permalink
Use Cookie Manager in Asp.Net Core
Browse files Browse the repository at this point in the history
- Fixes #1150
  • Loading branch information
AndersAbel committed Mar 24, 2020
2 parents 2ad5e26 + 3b25431 commit 3d56bdb
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 33 deletions.
8 changes: 6 additions & 2 deletions Sustainsys.Saml2.AspNetCore2/CommandResultExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Sustainsys.Saml2.WebSso;
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authentication.Cookies;
using Microsoft.AspNetCore.DataProtection;
using Microsoft.AspNetCore.Http;
using System;
Expand All @@ -14,6 +15,7 @@ static class CommandResultExtensions
this CommandResult commandResult,
HttpContext httpContext,
IDataProtector dataProtector,
ICookieManager cookieManager,
string signInScheme,
string signOutScheme,
bool emitSameSiteNone)
Expand All @@ -30,7 +32,8 @@ static class CommandResultExtensions
var cookieData = HttpRequestData.ConvertBinaryData(
dataProtector.Protect(commandResult.GetSerializedRequestState()));

httpContext.Response.Cookies.Append(
cookieManager.AppendResponseCookie(
httpContext,
commandResult.SetCookieName,
cookieData,
new CookieOptions()
Expand All @@ -51,7 +54,8 @@ static class CommandResultExtensions

if(!string.IsNullOrEmpty(commandResult.ClearCookieName))
{
httpContext.Response.Cookies.Delete(
cookieManager.DeleteCookie(
httpContext,
commandResult.ClearCookieName,
new CookieOptions
{
Expand Down
4 changes: 3 additions & 1 deletion Sustainsys.Saml2.AspNetCore2/HttpRequestExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Authentication.Cookies;

namespace Sustainsys.Saml2.AspNetCore2
{
static class HttpRequestExtensions
{
public static HttpRequestData ToHttpRequestData(
this HttpContext httpContext,
ICookieManager cookieManager,
Func<byte[], byte[]> cookieDecryptor)
{
var request = httpContext.Request;
Expand All @@ -36,7 +38,7 @@ static class HttpRequestExtensions
uri,
pathBase,
formData,
request.Cookies,
cookieName => cookieManager.GetRequestCookie(httpContext, cookieName),
cookieDecryptor,
httpContext.User);
}
Expand Down
4 changes: 4 additions & 0 deletions Sustainsys.Saml2.AspNetCore2/PostConfigureSaml2Options.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authentication.Cookies;

namespace Sustainsys.Saml2.AspNetCore2
{
Expand Down Expand Up @@ -53,6 +54,9 @@ public void PostConfigure(string name, Saml2Options options)
options.SignOutScheme = options.SignOutScheme
?? authOptions.Value.DefaultSignOutScheme
?? authOptions.Value.DefaultAuthenticateScheme;

// ChunkingCookieManager uses the headers directly when no chunk size is set
options.CookieManager = options.CookieManager ?? new ChunkingCookieManager();
}
}
}
12 changes: 6 additions & 6 deletions Sustainsys.Saml2.AspNetCore2/Saml2Handler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public async Task ChallengeAsync(AuthenticationProperties properties)
var redirectUri = properties.RedirectUri ?? CurrentUri;
properties.RedirectUri = null;

var requestData = context.ToHttpRequestData(null);
var requestData = context.ToHttpRequestData(options.CookieManager, null);

EntityId entityId = null;

Expand All @@ -104,7 +104,7 @@ public async Task ChallengeAsync(AuthenticationProperties properties)
options,
properties.Items);

await result.Apply(context, dataProtector, null, null, emitSameSiteNone);
await result.Apply(context, dataProtector, options.CookieManager, null, null, emitSameSiteNone);
}

/// <InheritDoc />
Expand All @@ -123,10 +123,10 @@ public async Task<bool> HandleRequestAsync()
options.SPOptions.ModulePath.Length).TrimStart('/');

var commandResult = CommandFactory.GetCommand(commandName).Run(
context.ToHttpRequestData(dataProtector.Unprotect), options);
context.ToHttpRequestData(options.CookieManager, dataProtector.Unprotect), options);

await commandResult.Apply(
context, dataProtector, options.SignInScheme, options.SignOutScheme, emitSameSiteNone);
context, dataProtector, options.CookieManager, options.SignInScheme, options.SignOutScheme, emitSameSiteNone);

return true;
}
Expand All @@ -147,13 +147,13 @@ public async Task SignOutAsync(AuthenticationProperties properties)
}

await LogoutCommand.InitiateLogout(
context.ToHttpRequestData(dataProtector.Unprotect),
context.ToHttpRequestData(options.CookieManager, dataProtector.Unprotect),
new Uri(properties.RedirectUri, UriKind.RelativeOrAbsolute),
options,
// In the Asp.Net Core2 model, it's the caller's responsibility to terminate the
// local session on an SP-initiated logout.
terminateLocalSession: false)
.Apply(context, dataProtector, null, null, emitSameSiteNone);
.Apply(context, dataProtector, options.CookieManager, null, null, emitSameSiteNone);
}
}
}
6 changes: 6 additions & 0 deletions Sustainsys.Saml2.AspNetCore2/Saml2Options.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Http;
using System;
using Microsoft.AspNetCore.Authentication.Cookies;

namespace Sustainsys.Saml2.AspNetCore2
{
Expand Down Expand Up @@ -41,6 +42,11 @@ public Saml2Options()
/// </summary>
public SPOptions SPOptions { get; set; }

/// <summary>
/// Cookie manager for reading/writing cookies
/// </summary>
public ICookieManager CookieManager { get; set; }

/// <summary>
/// Information about known identity providers.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

<ItemGroup>
<PackageReference Include="Microsoft.AspNetCore.Authentication" Version="2.1.2" />
<PackageReference Include="Microsoft.AspNetCore.Authentication.Cookies" Version="2.1.2" />
<PackageReference Include="Microsoft.AspNetCore.Http" Version="2.1.1" />
</ItemGroup>

Expand Down
45 changes: 31 additions & 14 deletions Tests/AspNetCore2.Tests/CommandResultExtensionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Sustainsys.Saml2.Metadata;
using Sustainsys.Saml2.WebSso;
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authentication.Cookies;
using Microsoft.AspNetCore.Http;
using Microsoft.IdentityModel.Tokens.Saml2;
using Microsoft.VisualStudio.TestTools.UnitTesting;
Expand Down Expand Up @@ -63,6 +64,7 @@ public async Task CommandResultExtensions_Apply()

ClaimsPrincipal principal = null;
AuthenticationProperties authProps = null;
var cookieManager = Substitute.For<ICookieManager>();
var authService = Substitute.For<IAuthenticationService>();
context.RequestServices.GetService(typeof(IAuthenticationService))
.Returns(authService);
Expand All @@ -72,21 +74,27 @@ public async Task CommandResultExtensions_Apply()
Arg.Do<ClaimsPrincipal>(p => principal = p),
Arg.Do<AuthenticationProperties>(ap => authProps = ap));

await commandResult.Apply(context, new StubDataProtector(), "TestSignInScheme", null, true);
await commandResult.Apply(context, new StubDataProtector(), cookieManager, "TestSignInScheme", null, true);

var expectedCookieData = HttpRequestData.ConvertBinaryData(
StubDataProtector.Protect(commandResult.GetSerializedRequestState()));

context.Response.StatusCode.Should().Be(302);
context.Response.Headers["Location"].SingleOrDefault()
.Should().Be(redirectLocation, "location header should be set");
context.Response.Cookies.Received().Append(
"Saml2.123", expectedCookieData, Arg.Is<CookieOptions>(
co => co.HttpOnly && co.Secure && co.SameSite == SameSiteMode.None));

context.Response.Cookies.Received().Delete(
cookieManager.Received().AppendResponseCookie(
context,
"Saml2.123",
expectedCookieData,
Arg.Is<CookieOptions>(co => co.HttpOnly && co.Secure && co.SameSite == SameSiteMode.None)
);

cookieManager.Received().DeleteCookie(
context,
"Clear-Cookie",
Arg.Is<CookieOptions>(co => co.Secure));
Arg.Is<CookieOptions>(co => co.Secure)
);

context.Response.Headers.Received().Add("header", "value");

Expand Down Expand Up @@ -114,13 +122,21 @@ public async Task CommandResultExtensions_Apply_NonSecureCookie_NoSameSite()
ClearCookieName = "DeleteName"
};

await commandResult.Apply(context, new StubDataProtector(), null, null, false);
var cookieManager = Substitute.For<ICookieManager>();
await commandResult.Apply(context, new StubDataProtector(), cookieManager, null, null, false);

context.Response.Cookies.Received().Delete(
"DeleteName", Arg.Is<CookieOptions>(co => !co.Secure));
cookieManager.Received().DeleteCookie(
context,
"DeleteName",
Arg.Is<CookieOptions>( co => !co.Secure )
);

context.Response.Cookies.Received().Append(
"CookieName", Arg.Any<string>(), Arg.Is<CookieOptions>(co => !co.Secure && co.SameSite == (SameSiteMode)(-1)));
cookieManager.Received().AppendResponseCookie(
context,
"CookieName",
Arg.Any<string>(),
Arg.Is<CookieOptions>( co => !co.Secure && co.SameSite == (SameSiteMode)( -1 ) )
);
}

[TestMethod]
Expand All @@ -134,12 +150,13 @@ public async Task CommandResultExtensions_Apply_Minimal()
context.RequestServices.GetService(typeof(IAuthenticationService))
.Returns(authService);

await commandResult.Apply(context, new StubDataProtector(), "TestSignInScheme", null, false);
var cookieManager = Substitute.For<ICookieManager>();
await commandResult.Apply(context, new StubDataProtector(), cookieManager, "TestSignInScheme", null, false);

context.Response.StatusCode.Should().Be(200);
context.Response.Headers.Keys.Should().NotContain("Location");
context.Response.Cookies.DidNotReceive().Append(
Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CookieOptions>());
cookieManager.DidNotReceive().AppendResponseCookie(
context, Arg.Any<string>(), Arg.Any<string>(), Arg.Any<CookieOptions>());

await authService.DidNotReceive().SignInAsync(
Arg.Any<HttpContext>(), Arg.Any<string>(), Arg.Any<ClaimsPrincipal>(), Arg.Any<AuthenticationProperties>());
Expand Down
21 changes: 11 additions & 10 deletions Tests/AspNetCore2.Tests/HttpContextExtensionsTests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using FluentAssertions;
using Sustainsys.Saml2.WebSso;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Authentication.Cookies;
using Microsoft.Extensions.Primitives;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using NSubstitute;
Expand All @@ -24,7 +25,7 @@ public void HttpContextExtensions_ToHttpRequestData()
{
var context = TestHelpers.CreateHttpContext();

var actual = context.ToHttpRequestData(StubDataProtector.Unprotect);
var actual = context.ToHttpRequestData(Substitute.For<ICookieManager>(), StubDataProtector.Unprotect);

actual.Url.Should().Be(new Uri("https://sp.example.com/somePath?param=value"));
actual.Form.Count.Should().Be(2);
Expand All @@ -39,7 +40,7 @@ public void HttpContextExtensions_ToHttpRequestData_Get()
{
var context = TestHelpers.CreateHttpGet();

var actual = context.ToHttpRequestData(StubDataProtector.Unprotect);
var actual = context.ToHttpRequestData(Substitute.For<ICookieManager>(), StubDataProtector.Unprotect);

actual.Url.Should().Be(new Uri("https://sp.example.com/somePath?param=value"));
actual.Form.Count.Should().Be(0);
Expand All @@ -55,7 +56,7 @@ public void HttpContextExtensions_ToHttpRequestData_InvalidPost()
context.Request.HasFormContentType.Returns(false);
context.Request.Form.Returns(i => { throw new InvalidOperationException(); });

context.ToHttpRequestData(StubDataProtector.Unprotect);
context.ToHttpRequestData(Substitute.For<ICookieManager>(), StubDataProtector.Unprotect);

// No exception = pass
}
Expand All @@ -67,7 +68,7 @@ public void HttpContextExtensions_ToHttpRequestData_ApplicationNotInRoot()

context.Request.PathBase = "/ApplicationPath";

var actual = context.ToHttpRequestData(null);
var actual = context.ToHttpRequestData(Substitute.For<ICookieManager>(), null);

actual.ApplicationUrl.Should().Be(new Uri("https://sp.example.com/ApplicationPath"));
}
Expand All @@ -83,12 +84,12 @@ public void HttpContextExtensions_ToHttpRequestData_ReadsRelayStateCookie()

var cookieData = HttpRequestData.ConvertBinaryData(
StubDataProtector.Protect(storedRequestState.Serialize()));
var cookieName = $"{StoredRequestState.CookieNameBase}SomeState";

context.Request.Cookies = new StubCookieCollection(
Enumerable.Repeat(new KeyValuePair<string, string>(
StoredRequestState.CookieNameBase + "SomeState", cookieData), 1));
var cookieManager = Substitute.For<ICookieManager>();
cookieManager.GetRequestCookie(context, cookieName).Returns(cookieData);

var actual = context.ToHttpRequestData(StubDataProtector.Unprotect);
var actual = context.ToHttpRequestData(cookieManager, StubDataProtector.Unprotect);

actual.StoredRequestState.Should().BeEquivalentTo(storedRequestState);
}
Expand All @@ -99,7 +100,7 @@ public void HttpContextExtensions_ToHttpRequestData_HandlesRelayStateWithoutCook
var context = TestHelpers.CreateHttpContext();
context.Request.QueryString = new QueryString("?RelayState=SomeState");

context.Invoking(c => c.ToHttpRequestData(null))
context.Invoking(c => c.ToHttpRequestData(Substitute.For<ICookieManager>(), null ))
.Should().NotThrow();
}

Expand All @@ -112,7 +113,7 @@ public void HttpContextExtensions_ToHttpRequestData_ExtractsUser()
new Claim(ClaimTypes.NameIdentifier, "NameID")
}));

var actual = context.ToHttpRequestData(null);
var actual = context.ToHttpRequestData(Substitute.For<ICookieManager>(), null);

actual.User.Should().BeSameAs(context.User);
}
Expand Down
29 changes: 29 additions & 0 deletions Tests/AspNetCore2.Tests/PostConfigureSaml2OptionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Sustainsys.Saml2;
using Microsoft.AspNetCore.Authentication;
using Microsoft.Extensions.Options;
using Microsoft.AspNetCore.Authentication.Cookies;

namespace Sustainsys.Saml2.AspNetCore2.Tests
{
Expand Down Expand Up @@ -151,5 +152,33 @@ public void PostConfigureSaml2Options_PostConfigure_EnsureSignOutSchemePreserves

options.SignOutScheme.Should().Be("specificSignOutScheme");
}

[TestMethod]
public void PostConfigureSaml2Options_PostConfigure_CustomCookieManagerIsUsed()
{
var options = new Saml2Options();
var cookieManager = Substitute.For<ICookieManager>();
options.CookieManager = cookieManager;

var subject = new PostConfigureSaml2Options( null,
TestHelpers.GetAuthenticationOptions() );

subject.PostConfigure( null, options );

options.CookieManager.Should().BeSameAs( cookieManager );
}

[TestMethod]
public void PostConfigureSaml2Options_PostConfigure_ProvidesDefaultCookieManager()
{
var options = new Saml2Options();

var subject = new PostConfigureSaml2Options( null,
TestHelpers.GetAuthenticationOptions() );

subject.PostConfigure( null, options );

options.CookieManager.Should().NotBeNull();
}
}
}

0 comments on commit 3d56bdb

Please sign in to comment.