Improved error on missing ReturnUrl

- Better exception that describes what to do, instead of getting a NullReferenceException further downstream. - Made returnUrl in the configuration non-required. If using only SP-initiated login, it is not used. - Inspired by discussion in #319.
Sustainsys · Oct 29, 2015 · ef5c5a6 · ef5c5a6
1 parent e14a26b
commit ef5c5a6
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 6 deletions.
diff --git a/Kentor.AuthServices.Tests/WebSSO/AcsCommandTests.cs b/Kentor.AuthServices.Tests/WebSSO/AcsCommandTests.cs
@@ -14,6 +14,7 @@
 using Kentor.AuthServices.Configuration;
 using Kentor.AuthServices.WebSso;
 using System.Reflection;
+using System.Configuration;
 
 namespace Kentor.AuthServices.Tests.WebSso
 {
@@ -226,5 +227,49 @@ public void AcsCommand_Run_WithReturnUrl_SuccessfulResult()
             new AcsCommand().Run(r, StubFactory.CreateOptions())
                 .ShouldBeEquivalentTo(expected, opt => opt.IgnoringCyclicReferences());
         }
+
+        [TestMethod]
+        public void AcsCommand_Run_UnsolicitedResponse_ThrowsOnNoConfiguredReturnUrl()
+        {
+           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 + @"_Assertion""
+                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>";
+
+            var formValue = Convert.ToBase64String(Encoding.UTF8.GetBytes(
+                SignedXmlHelper.SignXml(response)));
+
+            var r = new HttpRequestData(
+                "POST",
+                new Uri("http://localhost"),
+                "/ModulePath",
+                new KeyValuePair<string, string[]>[]
+                {
+                    new KeyValuePair<string, string[]>("SAMLResponse", new string[] { formValue })
+                });
+
+            var options = StubFactory.CreateOptions();
+            ((SPOptions)options.SPOptions).ReturnUrl = null;
+
+            new AcsCommand().Invoking(a => a.Run(r, options))
+                .ShouldThrow<ConfigurationErrorsException>().WithMessage(AcsCommand.MissingReturnUrlMessage);
+        }
     }
-}
+}
diff --git a/Kentor.AuthServices/Configuration/KentorAuthServicesSection.cs b/Kentor.AuthServices/Configuration/KentorAuthServicesSection.cs
@@ -81,7 +81,7 @@ public EntityId EntityId
         /// <summary>
         /// The Url to redirect back to after successfull authentication.
         /// </summary>
-        [ConfigurationProperty("returnUrl", IsRequired = true)]
+        [ConfigurationProperty("returnUrl")]
         public Uri ReturnUrl
         {
             get

diff --git a/Kentor.AuthServices/WebSSO/AcsCommand.cs b/Kentor.AuthServices/WebSSO/AcsCommand.cs
@@ -1,6 +1,7 @@
 using Kentor.AuthServices.Configuration;
 using Kentor.AuthServices.Saml2P;
 using System;
+using System.Configuration;
 using System.IdentityModel.Metadata;
 using System.IdentityModel.Services;
 using System.Net;
@@ -63,6 +64,9 @@ public CommandResult Run(HttpRequestData request, IOptions options)
             throw new NoSamlResponseFoundException();
         }
 
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA2204:Literals should be spelled correctly", MessageId = "returnUrl")]
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA2204:Literals should be spelled correctly", MessageId = "SpOptions")]
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA2204:Literals should be spelled correctly", MessageId = "ReturnUrl")]
         private static CommandResult ProcessResponse(IOptions options, Saml2Response samlResponse)
         {
             var principal = new ClaimsPrincipal(samlResponse.GetClaims(options));
@@ -72,19 +76,32 @@ private static CommandResult ProcessResponse(IOptions options, Saml2Response sam
 
             var requestState = samlResponse.GetRequestState(options);
 
+            if(requestState == null && options.SPOptions.ReturnUrl == null)
+            {
+                throw new ConfigurationErrorsException(MissingReturnUrlMessage);
+            }
+
             return new CommandResult()
             {
                 HttpStatusCode = HttpStatusCode.SeeOther,
-                Location =
-                    requestState != null && requestState.ReturnUrl != null
-                    ? requestState.ReturnUrl
-                    : options.SPOptions.ReturnUrl,
+                Location = requestState?.ReturnUrl ?? options.SPOptions.ReturnUrl,
                 Principal = principal,
                 RelayData =
                     requestState == null
                     ? null
                     : requestState.RelayData
             };
         }
+
+        internal const string MissingReturnUrlMessage =
+@"Unsolicited SAML response received, but no ReturnUrl is configured.
+
+When receiving unsolicited SAML responses (i.e. IDP initiated login),
+AuthServices will redirect the client to the configured ReturnUrl after
+successful authentication, but it is not configured.
+
+In code-based config, add a ReturnUrl by setting the
+options.SpOptions.ReturnUrl property. In the config file, set the returnUrl
+attribute of the <kentor.authServices> element.";
     }
 }