From 609d2b843b02451ca4acb326be617143bb7679aa Mon Sep 17 00:00:00 2001 From: Steve Patches Date: Mon, 26 Mar 2018 17:12:36 -0400 Subject: [PATCH 1/2] handle stored request state without returnurl during OWIN error response --- .../Saml2AuthenticationHandler.cs | 14 +- .../Saml2AuthenticationMiddlewareTests.cs | 145 ++++++++++++++++++ 2 files changed, 154 insertions(+), 5 deletions(-) diff --git a/Sustainsys.Saml2.Owin/Saml2AuthenticationHandler.cs b/Sustainsys.Saml2.Owin/Saml2AuthenticationHandler.cs index e68879b79..7e604325c 100644 --- a/Sustainsys.Saml2.Owin/Saml2AuthenticationHandler.cs +++ b/Sustainsys.Saml2.Owin/Saml2AuthenticationHandler.cs @@ -61,11 +61,18 @@ protected async override Task AuthenticateCoreAsync() private AuthenticationTicket CreateErrorAuthenticationTicket(HttpRequestData httpRequestData, Exception ex) { AuthenticationProperties authProperties = null; - if (httpRequestData.StoredRequestState != null) + if (httpRequestData.StoredRequestState?.RelayData != null) { authProperties = new AuthenticationProperties( httpRequestData.StoredRequestState.RelayData); + } + else + { + authProperties = new AuthenticationProperties(); + } + if (httpRequestData.StoredRequestState?.ReturnUrl != null) + { // ReturnUrl is removed from AuthProps dictionary to save space, need to put it back. authProperties.RedirectUri = httpRequestData.StoredRequestState.ReturnUrl.OriginalString; } @@ -83,10 +90,7 @@ private AuthenticationTicket CreateErrorAuthenticationTicket(HttpRequestData htt redirectUrl = httpRequestData.ApplicationUrl; } - authProperties = new AuthenticationProperties - { - RedirectUri = redirectUrl.OriginalString - }; + authProperties.RedirectUri = redirectUrl.OriginalString; } // The Google middleware adds this, so let's follow that example. diff --git a/Tests/Owin.Tests/Saml2AuthenticationMiddlewareTests.cs b/Tests/Owin.Tests/Saml2AuthenticationMiddlewareTests.cs index 9c8952cce..158e34fd7 100644 --- a/Tests/Owin.Tests/Saml2AuthenticationMiddlewareTests.cs +++ b/Tests/Owin.Tests/Saml2AuthenticationMiddlewareTests.cs @@ -978,6 +978,151 @@ public async Task Saml2AuthenticationMiddleware_AcsRedirectsToAuthPropsReturnUri context.Authentication.AuthenticationResponseGrant.Should().BeNull(); } + [TestMethod] + public async Task Saml2AuthenticationMiddleware_AcsRedirectsToAuthProps_StoredRequestStateWithNoReturnUrl() + { + var context = OwinTestHelpers.CreateOwinContext(); + context.Request.Method = "POST"; + + var authProps = new AuthenticationProperties(); + authProps.Dictionary.Add("key1", "value1"); + + var state = new StoredRequestState(new EntityId("https://idp.example.com"), + null, + new Saml2Id("InResponseToId"), + authProps.Dictionary); + + var relayState = SecureKeyGenerator.CreateRelayState(); + + var cookieData = HttpRequestData.ConvertBinaryData( + CreateAppBuilder().CreateDataProtector( + typeof(Saml2AuthenticationMiddleware).FullName) + .Protect(state.Serialize())); + + context.Request.Headers["Cookie"] = $"{StoredRequestState.CookieNameBase}{relayState}={cookieData}"; + + var response = + @" + + https://idp.example.com + + + + + + https://idp.example.com + + SomeUser + + + + + "; + + // No signature, that's an error. + var bodyData = new KeyValuePair[] { + new KeyValuePair("SAMLResponse", + Convert.ToBase64String(Encoding.UTF8.GetBytes(response))), + new KeyValuePair("RelayState",relayState) + }; + + var encodedBodyData = new FormUrlEncodedContent(bodyData); + + context.Request.Body = encodedBodyData.ReadAsStreamAsync().Result; + context.Request.ContentType = encodedBodyData.Headers.ContentType.ToString(); + context.Request.Host = new HostString("localhost"); + context.Request.Path = new PathString("/Saml2/Acs"); + + var middleware = new Saml2AuthenticationMiddleware(null, CreateAppBuilder(), + new Saml2AuthenticationOptions(true) + { + SignInAsAuthenticationType = "AuthType" + }); + + await middleware.Invoke(context); + + context.Response.StatusCode.Should().Be(302); + context.Response.Headers["Location"].Should().Be("http://localhost/LoggedIn?error=access_denied"); + context.Authentication.AuthenticationResponseGrant.Should().BeNull(); + } + + [TestMethod] + public async Task Saml2AuthenticationMiddleware_AcsRedirectsToAuthProps_StoredRequestStateWithNoRelayData() + { + var context = OwinTestHelpers.CreateOwinContext(); + context.Request.Method = "POST"; + + var authProps = new AuthenticationProperties(); + + var state = new StoredRequestState(new EntityId("https://idp.example.com"), + new Uri("http://localhost/PathInRequestState?value=42"), + new Saml2Id("InResponseToId"), + null); + + var relayState = SecureKeyGenerator.CreateRelayState(); + + var cookieData = HttpRequestData.ConvertBinaryData( + CreateAppBuilder().CreateDataProtector( + typeof(Saml2AuthenticationMiddleware).FullName) + .Protect(state.Serialize())); + + context.Request.Headers["Cookie"] = $"{StoredRequestState.CookieNameBase}{relayState}={cookieData}"; + + var response = + @" + + https://idp.example.com + + + + + + https://idp.example.com + + SomeUser + + + + + "; + + // No signature, that's an error. + var bodyData = new KeyValuePair[] { + new KeyValuePair("SAMLResponse", + Convert.ToBase64String(Encoding.UTF8.GetBytes(response))), + new KeyValuePair("RelayState",relayState) + }; + + var encodedBodyData = new FormUrlEncodedContent(bodyData); + + context.Request.Body = encodedBodyData.ReadAsStreamAsync().Result; + context.Request.ContentType = encodedBodyData.Headers.ContentType.ToString(); + context.Request.Host = new HostString("localhost"); + context.Request.Path = new PathString("/Saml2/Acs"); + + var middleware = new Saml2AuthenticationMiddleware(null, CreateAppBuilder(), + new Saml2AuthenticationOptions(true) + { + SignInAsAuthenticationType = "AuthType" + }); + + await middleware.Invoke(context); + + context.Response.StatusCode.Should().Be(302); + context.Response.Headers["Location"].Should().Be("http://localhost/PathInRequestState?value=42&error=access_denied"); + context.Authentication.AuthenticationResponseGrant.Should().BeNull(); + } + [TestMethod] public async Task Saml2AuthenticationMiddleware_AcsWorks() { From 147315487ef110763a738fc34f9814e19f675688 Mon Sep 17 00:00:00 2001 From: Steve Patches Date: Thu, 29 Mar 2018 10:52:08 -0400 Subject: [PATCH 2/2] Do not restore authproperties from relaystate for error response since they are not used in the OWIN calling code --- .../Saml2AuthenticationHandler.cs | 11 +-- .../Saml2AuthenticationMiddlewareTests.cs | 73 ------------------- 2 files changed, 1 insertion(+), 83 deletions(-) diff --git a/Sustainsys.Saml2.Owin/Saml2AuthenticationHandler.cs b/Sustainsys.Saml2.Owin/Saml2AuthenticationHandler.cs index 7e604325c..a65a61282 100644 --- a/Sustainsys.Saml2.Owin/Saml2AuthenticationHandler.cs +++ b/Sustainsys.Saml2.Owin/Saml2AuthenticationHandler.cs @@ -60,16 +60,7 @@ protected async override Task AuthenticateCoreAsync() [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA2204:Literals should be spelled correctly", MessageId = "ReturnUrl")] private AuthenticationTicket CreateErrorAuthenticationTicket(HttpRequestData httpRequestData, Exception ex) { - AuthenticationProperties authProperties = null; - if (httpRequestData.StoredRequestState?.RelayData != null) - { - authProperties = new AuthenticationProperties( - httpRequestData.StoredRequestState.RelayData); - } - else - { - authProperties = new AuthenticationProperties(); - } + var authProperties = new AuthenticationProperties(); if (httpRequestData.StoredRequestState?.ReturnUrl != null) { diff --git a/Tests/Owin.Tests/Saml2AuthenticationMiddlewareTests.cs b/Tests/Owin.Tests/Saml2AuthenticationMiddlewareTests.cs index 158e34fd7..82ac9a6e7 100644 --- a/Tests/Owin.Tests/Saml2AuthenticationMiddlewareTests.cs +++ b/Tests/Owin.Tests/Saml2AuthenticationMiddlewareTests.cs @@ -985,7 +985,6 @@ public async Task Saml2AuthenticationMiddleware_AcsRedirectsToAuthProps_StoredRe context.Request.Method = "POST"; var authProps = new AuthenticationProperties(); - authProps.Dictionary.Add("key1", "value1"); var state = new StoredRequestState(new EntityId("https://idp.example.com"), null, @@ -1051,78 +1050,6 @@ public async Task Saml2AuthenticationMiddleware_AcsRedirectsToAuthProps_StoredRe context.Authentication.AuthenticationResponseGrant.Should().BeNull(); } - [TestMethod] - public async Task Saml2AuthenticationMiddleware_AcsRedirectsToAuthProps_StoredRequestStateWithNoRelayData() - { - var context = OwinTestHelpers.CreateOwinContext(); - context.Request.Method = "POST"; - - var authProps = new AuthenticationProperties(); - - var state = new StoredRequestState(new EntityId("https://idp.example.com"), - new Uri("http://localhost/PathInRequestState?value=42"), - new Saml2Id("InResponseToId"), - null); - - var relayState = SecureKeyGenerator.CreateRelayState(); - - var cookieData = HttpRequestData.ConvertBinaryData( - CreateAppBuilder().CreateDataProtector( - typeof(Saml2AuthenticationMiddleware).FullName) - .Protect(state.Serialize())); - - context.Request.Headers["Cookie"] = $"{StoredRequestState.CookieNameBase}{relayState}={cookieData}"; - - var response = - @" - - https://idp.example.com - - - - - - https://idp.example.com - - SomeUser - - - - - "; - - // No signature, that's an error. - var bodyData = new KeyValuePair[] { - new KeyValuePair("SAMLResponse", - Convert.ToBase64String(Encoding.UTF8.GetBytes(response))), - new KeyValuePair("RelayState",relayState) - }; - - var encodedBodyData = new FormUrlEncodedContent(bodyData); - - context.Request.Body = encodedBodyData.ReadAsStreamAsync().Result; - context.Request.ContentType = encodedBodyData.Headers.ContentType.ToString(); - context.Request.Host = new HostString("localhost"); - context.Request.Path = new PathString("/Saml2/Acs"); - - var middleware = new Saml2AuthenticationMiddleware(null, CreateAppBuilder(), - new Saml2AuthenticationOptions(true) - { - SignInAsAuthenticationType = "AuthType" - }); - - await middleware.Invoke(context); - - context.Response.StatusCode.Should().Be(302); - context.Response.Headers["Location"].Should().Be("http://localhost/PathInRequestState?value=42&error=access_denied"); - context.Authentication.AuthenticationResponseGrant.Should().BeNull(); - } - [TestMethod] public async Task Saml2AuthenticationMiddleware_AcsWorks() {