Skip to content

Commit

Permalink
Use Owin Cookie Manager
Browse files Browse the repository at this point in the history
  • Loading branch information
AndersAbel committed Mar 22, 2020
2 parents d7154f4 + 7ba9f85 commit 810c721
Show file tree
Hide file tree
Showing 10 changed files with 157 additions and 45 deletions.
13 changes: 8 additions & 5 deletions Sustainsys.Saml2.Owin/CommandResultExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Sustainsys.Saml2.WebSso;
using Microsoft.Owin;
using Microsoft.Owin.Infrastructure;
using Microsoft.Owin.Security.DataProtection;
using System;
using System.Collections.Generic;
Expand All @@ -14,6 +15,7 @@ static class CommandResultExtensions
public static void Apply(this CommandResult commandResult,
IOwinContext context,
IDataProtector dataProtector,
ICookieManager cookieManager,
bool emitSameSiteNone)
{
if (commandResult == null)
Expand All @@ -39,7 +41,7 @@ static class CommandResultExtensions
context.Authentication.SignOut();
}

ApplyCookies(commandResult, context, dataProtector, emitSameSiteNone);
ApplyCookies(commandResult, context, dataProtector, cookieManager, emitSameSiteNone);

foreach(var h in commandResult.Headers)
{
Expand All @@ -62,6 +64,7 @@ static class CommandResultExtensions
CommandResult commandResult,
IOwinContext context,
IDataProtector dataProtector,
ICookieManager cookieManager,
bool emitSameSiteNone)
{
var serializedCookieData = commandResult.GetSerializedRequestState();
Expand All @@ -71,7 +74,7 @@ static class CommandResultExtensions
var protectedData = HttpRequestData.ConvertBinaryData(
dataProtector.Protect(serializedCookieData));

context.Response.Cookies.Append(
cookieManager.AppendResponseCookie(context,
commandResult.SetCookieName,
protectedData,
new CookieOptions()
Expand All @@ -82,14 +85,14 @@ static class CommandResultExtensions
});
}

commandResult.ApplyClearCookie(context);
commandResult.ApplyClearCookie(context, cookieManager);
}

public static void ApplyClearCookie(this CommandResult commandResult, IOwinContext context)
public static void ApplyClearCookie(this CommandResult commandResult, IOwinContext context, ICookieManager cookieManager)
{
if (!string.IsNullOrEmpty(commandResult.ClearCookieName))
{
context.Response.Cookies.Delete(
cookieManager.DeleteCookie(context,
commandResult.ClearCookieName,
new CookieOptions
{
Expand Down
4 changes: 3 additions & 1 deletion Sustainsys.Saml2.Owin/OwinContextExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Sustainsys.Saml2.WebSso;
using Microsoft.Owin;
using Microsoft.Owin.Infrastructure;
using Microsoft.Owin.Security.DataProtection;
using System;
using System.Collections.Generic;
Expand All @@ -16,6 +17,7 @@ static class OwinContextExtensions
{
public async static Task<HttpRequestData> ToHttpRequestData(
this IOwinContext context,
ICookieManager cookieManager,
Func<byte[], byte[]> cookieDecryptor)
{
if(context == null)
Expand All @@ -40,7 +42,7 @@ static class OwinContextExtensions
context.Request.Uri,
applicationRootPath,
formData?.Select(f => new KeyValuePair<string, IEnumerable<string>>(f.Key, f.Value)),
context.Request.Cookies,
cookieName => cookieManager.GetRequestCookie(context, cookieName),
cookieDecryptor,
context.Request.User as ClaimsPrincipal);
}
Expand Down
12 changes: 8 additions & 4 deletions Sustainsys.Saml2.Owin/Saml2AuthenticationHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ protected async override Task<AuthenticationTicket> AuthenticateCoreAsync()
return null;
}

var httpRequestData = await Context.ToHttpRequestData(Options.DataProtector.Unprotect);
var httpRequestData = await Context.ToHttpRequestData(Options.CookieManager, Options.DataProtector.Unprotect);
try
{
var result = CommandFactory.GetCommand(CommandFactory.AcsCommandName)
Expand All @@ -37,6 +37,7 @@ protected async override Task<AuthenticationTicket> AuthenticateCoreAsync()
result.Apply(
Context,
Options.DataProtector,
Options.CookieManager,
Options.Notifications.EmitSameSiteNone(Request.GetUserAgent()));
}

Expand Down Expand Up @@ -133,7 +134,7 @@ protected override async Task ApplyResponseChallengeAsync()
var result = SignInCommand.Run(
idp,
redirectUri,
await Context.ToHttpRequestData(Options.DataProtector.Unprotect),
await Context.ToHttpRequestData(Options.CookieManager, Options.DataProtector.Unprotect),
Options,
challenge.Properties.Dictionary);

Expand All @@ -142,6 +143,7 @@ protected override async Task ApplyResponseChallengeAsync()
result.Apply(
Context,
Options.DataProtector,
Options.CookieManager,
Options.Notifications.EmitSameSiteNone(Request.GetUserAgent()));
}
}
Expand All @@ -159,7 +161,7 @@ protected async override Task ApplyResponseGrantAsync()

if (revoke != null)
{
var request = await Context.ToHttpRequestData(Options.DataProtector.Unprotect);
var request = await Context.ToHttpRequestData(Options.CookieManager, Options.DataProtector.Unprotect);
var urls = new Saml2Urls(request, Options);

string redirectUrl = revoke.Properties.RedirectUri;
Expand All @@ -183,6 +185,7 @@ protected async override Task ApplyResponseGrantAsync()
result.Apply(
Context,
Options.DataProtector,
Options.CookieManager,
Options.Notifications.EmitSameSiteNone(Request.GetUserAgent()));
}
}
Expand Down Expand Up @@ -215,13 +218,14 @@ public override async Task<bool> InvokeAsync()
try
{
var result = CommandFactory.GetCommand(remainingPath.Value)
.Run(await Context.ToHttpRequestData(Options.DataProtector.Unprotect), Options);
.Run(await Context.ToHttpRequestData(Options.CookieManager, Options.DataProtector.Unprotect), Options);

if (!result.HandledResult)
{
result.Apply(
Context,
Options.DataProtector,
Options.CookieManager,
Options.Notifications.EmitSameSiteNone(Request.GetUserAgent()));
}

Expand Down
9 changes: 8 additions & 1 deletion Sustainsys.Saml2.Owin/Saml2AuthenticationOptions.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
using Sustainsys.Saml2.Configuration;
using Microsoft.Owin.Security;
using Microsoft.Owin.Security.DataProtection;
using Microsoft.Owin.Infrastructure;

namespace Sustainsys.Saml2.Owin
{
/// <summary>
/// Options for Sustainsys Saml2 Saml2 Authentication.
/// Options for Sustainsys Saml2 OWIN Authentication.
/// </summary>
public class Saml2AuthenticationOptions : AuthenticationOptions, IOptions
{
Expand All @@ -27,6 +28,7 @@ public Saml2AuthenticationOptions(bool loadConfiguration)
AuthenticationMode = AuthenticationMode.Passive;
Description.Caption = Constants.DefaultCaption;
Notifications = new Saml2Notifications();
CookieManager = new CookieManager();

if (loadConfiguration)
{
Expand Down Expand Up @@ -76,6 +78,11 @@ public string Caption
}
}

/// <summary>
/// Gets or sets the cookie manager used for reading and writing cookies
/// </summary>
public ICookieManager CookieManager { get; set; }

/// <summary>
/// Gets or sets the type used to secure data handled by the middleware.
/// </summary>
Expand Down
43 changes: 38 additions & 5 deletions Sustainsys.Saml2/WebSSO/HttpRequestData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,46 @@ public class HttpRequestData
Func<byte[], byte[]> cookieDecryptor,
ClaimsPrincipal user)
{
Init(httpMethod, url, applicationPath, formData, cookies, cookieDecryptor, user);
Init(
httpMethod,
url,
applicationPath,
formData,
cookieName => cookies.Any( c => c.Key == cookieName ) ? cookies.SingleOrDefault( c => c.Key == cookieName ).Value : null,
cookieDecryptor,
user);
}

/// <summary>
/// Ctor
/// </summary>
/// <param name="httpMethod">Http method of the request</param>
/// <param name="url">Full url requested</param>
/// <param name="formData">Form data, if present (only for POST requests)</param>
/// <param name="applicationPath">Path to the application root</param>
/// <param name="cookieReader">Function that reads cookie if it exists</param>
/// <param name="cookieDecryptor">Function that decrypts cookie
/// contents to clear text.</param>
/// <param name="user">Claims Principal associated with the request</param>
[System.Diagnostics.CodeAnalysis.SuppressMessage( "Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "Decryptor" )]
[System.Diagnostics.CodeAnalysis.SuppressMessage( "Microsoft.Design", "CA1006:DoNotNestGenericTypesInMemberSignatures" )]
public HttpRequestData(
string httpMethod,
Uri url,
string applicationPath,
IEnumerable<KeyValuePair<string, IEnumerable<string>>> formData,
Func<string, string> cookieReader,
Func<byte[], byte[]> cookieDecryptor,
ClaimsPrincipal user )
{
if (cookieReader == null) throw new ArgumentNullException(nameof(cookieReader));
Init(httpMethod, url, applicationPath, formData, cookieReader, cookieDecryptor, user);
}

// Used by tests.
internal HttpRequestData(string httpMethod, Uri url)
{
Init(httpMethod, url, "/", null, Enumerable.Empty<KeyValuePair<string, string>>(), null, null);
Init(httpMethod, url, "/", null, cookieName => null, null, null);
}

// Used by tests.
Expand All @@ -89,7 +122,7 @@ internal HttpRequestData(string httpMethod, Uri url)
Uri url,
string applicationPath,
IEnumerable<KeyValuePair<string, IEnumerable<string>>> formData,
IEnumerable<KeyValuePair<string, string>> cookies,
Func<string, string> cookieReader,
Func<byte[], byte[]> cookieDecryptor,
ClaimsPrincipal user)
{
Expand All @@ -106,9 +139,9 @@ internal HttpRequestData(string httpMethod, Uri url)
if (relayState != null)
{
var cookieName = StoredRequestState.CookieNameBase + relayState;
if (cookies.Any(c => c.Key == cookieName))
var cookieData = cookieReader(cookieName);
if (!string.IsNullOrEmpty(cookieData))
{
var cookieData = cookies.SingleOrDefault(c => c.Key == cookieName).Value;
byte[] encryptedData = GetBinaryData(cookieData);

var decryptedData = cookieDecryptor(encryptedData);
Expand Down
48 changes: 27 additions & 21 deletions Tests/Owin.Tests/CommandResultExtensionsTests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
using FluentAssertions;
using Microsoft.IdentityModel.Tokens.Saml2;
using Microsoft.Owin;
using Microsoft.Owin.Infrastructure;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using NSubstitute;
using Sustainsys.Saml2.Metadata;
using Sustainsys.Saml2.WebSso;
using System;
Expand All @@ -15,15 +18,15 @@ public class CommandResultExtensionsTests
[TestMethod]
public void CommandResultExtensions_Apply_NullCheck_CommandResult()
{
Action a = () => ((CommandResult)null).Apply(OwinTestHelpers.CreateOwinContext(), null, true);
Action a = () => ((CommandResult)null).Apply(OwinTestHelpers.CreateOwinContext(), null, new CookieManager(), true);

a.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be("commandResult");
}

[TestMethod]
public void CommandResultExtensions_Apply_NullCheck_OwinContext()
{
Action a = () => new CommandResult().Apply(context:null, dataProtector:null, emitSameSiteNone:true);
Action a = () => new CommandResult().Apply(context:null, dataProtector:null, cookieManager: new CookieManager(), emitSameSiteNone:true);

a.Should().Throw<ArgumentNullException>().And.ParamName.Should().Be("context");
}
Expand All @@ -39,7 +42,7 @@ public void CommandResultExtensions_Apply_Content()

var context = OwinTestHelpers.CreateOwinContext();

cr.Apply(context, null, true);
cr.Apply(context, null, new CookieManager(), true);

context.Response.StatusCode.Should().Be(200);
context.Response.ContentType.Should().Be("application/whatever+text");
Expand Down Expand Up @@ -68,17 +71,19 @@ public void CommandResultExtensions_Apply_Cookie()

var context = OwinTestHelpers.CreateOwinContext();

var mockCookieManager = Substitute.For<ICookieManager>();
var dataProtector = new StubDataProtector();
cr.Apply(context, dataProtector, false);

var setCookieHeader = context.Response.Headers["Set-Cookie"];
cr.Apply(context, dataProtector, mockCookieManager, false);

var protectedData = HttpRequestData.ConvertBinaryData(
StubDataProtector.Protect(cr.GetSerializedRequestState()));

var expected = $"CookieName={protectedData}; path=/; HttpOnly";

setCookieHeader.Should().Be(expected);
mockCookieManager.Received().AppendResponseCookie(
context,
"CookieName",
protectedData,
Arg.Is<CookieOptions>( c => c.HttpOnly && c.Path == "/" )
);
}

[TestMethod]
Expand All @@ -98,7 +103,7 @@ public void CommandREsultExtensions_Apply_Cookie_EmitSameSiteNone_AndSecure()
var context = OwinTestHelpers.CreateOwinContext();

var dataProtector = new StubDataProtector();
cr.Apply(context, dataProtector, true);
cr.Apply(context, dataProtector, new CookieManager(), true);

var setCookieHeader = context.Response.Headers["Set-Cookie"];

Expand Down Expand Up @@ -126,7 +131,7 @@ public void CommandResultExtensions_DoesNotApplyCookieWhenNoNameSet()
var context = OwinTestHelpers.CreateOwinContext();

var dataProtector = new StubDataProtector();
cr.Apply(context, dataProtector, true);
cr.Apply(context, dataProtector, new CookieManager(), true);

var setCookieHeader = context.Response.Headers["Set-Cookie"];

Expand All @@ -146,13 +151,14 @@ public void CommandResultExtensions_Apply_ClearCookie()

var context = OwinTestHelpers.CreateOwinContext();
var dataProtector = new StubDataProtector();
cr.Apply(context, dataProtector, true);

var setCookieHeader = context.Response.Headers["Set-Cookie"];

var expected = "CookieName=; path=/; expires=Thu, 01-Jan-1970 00:00:00 GMT";

setCookieHeader.Should().Be(expected);
var mockCookieManager = Substitute.For<ICookieManager>();
cr.Apply(context, dataProtector, mockCookieManager, true);

mockCookieManager.Received().DeleteCookie(
context,
"CookieName",
Arg.Is<CookieOptions>(c => c.Path == "/" && !c.Expires.HasValue)
);
}

[TestMethod]
Expand All @@ -166,7 +172,7 @@ public void CommandResultExtensions_Apply_ClearCookie_Secure()

var context = OwinTestHelpers.CreateOwinContext();
var dataProtector = new StubDataProtector();
cr.Apply(context, dataProtector, true);
cr.Apply(context, dataProtector, new CookieManager(), true);

var setCookieHeader = context.Response.Headers["Set-Cookie"];

Expand All @@ -187,7 +193,7 @@ public void CommandResultExtensions_Apply_Redirect()

var context = OwinTestHelpers.CreateOwinContext();

cr.Apply(context, null, true);
cr.Apply(context, null, new CookieManager(), true);

context.Response.StatusCode.Should().Be(303);
context.Response.Headers["Location"].Should().Be(redirectUrl);
Expand All @@ -201,7 +207,7 @@ public void CommandResult_Apply_Headers()

var context = OwinTestHelpers.CreateOwinContext();

cr.Apply(context, null, true);
cr.Apply(context, null, new CookieManager(), true);

context.Response.Headers["header"].Should().Be("value");
}
Expand Down

0 comments on commit 810c721

Please sign in to comment.