Merge pull request #935 from Sustainsys/fix-916

- handle stored request state without returnurl during OWIN error response - Do not restore authproperties from relaystate for error response since they are not used in the OWIN calling code - Fixes #916
Sustainsys · Mar 29, 2018 · 5b52089 · 5b52089
2 parents 1308402 + 1473154
commit 5b52089
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 9 deletions.
diff --git a/Sustainsys.Saml2.Owin/Saml2AuthenticationHandler.cs b/Sustainsys.Saml2.Owin/Saml2AuthenticationHandler.cs
@@ -60,12 +60,10 @@ protected async override Task<AuthenticationTicket> 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 != null)
-            {
-                authProperties = new AuthenticationProperties(
-                    httpRequestData.StoredRequestState.RelayData);
+            var 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 +81,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
@@ -978,6 +978,78 @@ 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();
+
+            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 =
+                @"<saml2p:Response xmlns:saml2p=""urn:oasis:names:tc:SAML:2.0:protocol""
+                xmlns:saml2=""urn:oasis:names:tc:SAML:2.0:assertion""
+                ID = """ + MethodBase.GetCurrentMethod().Name + @""" Version=""2.0""
+                IssueInstant=""2013-01-01T00:00:00Z"">
+                <saml2:Issuer>
+                    https://idp.example.com
+                </saml2:Issuer>
+                <saml2p:Status>
+                    <saml2p:StatusCode Value=""urn:oasis:names:tc:SAML:2.0:status:Success"" />
+                </saml2p:Status>
+                <saml2:Assertion
+                Version=""2.0"" ID=""" + MethodBase.GetCurrentMethod().Name + @"_Assertion1""
+                IssueInstant=""2013-09-25T00:00:00Z"">
+                    <saml2:Issuer>https://idp.example.com</saml2:Issuer>
+                    <saml2:Subject>
+                        <saml2:NameID>SomeUser</saml2:NameID>
+                        <saml2:SubjectConfirmation Method=""urn:oasis:names:tc:SAML:2.0:cm:bearer"" />
+                    </saml2:Subject>
+                    <saml2:Conditions NotOnOrAfter=""2100-01-01T00:00:00Z"" />
+                </saml2:Assertion>
+            </saml2p:Response>";
+
+            // No signature, that's an error.
+            var bodyData = new KeyValuePair<string, string>[] {
+                new KeyValuePair<string, string>("SAMLResponse",
+                    Convert.ToBase64String(Encoding.UTF8.GetBytes(response))),
+                new KeyValuePair<string, string>("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_AcsWorks()
         {