Better exception messages on SAML Response validation failures.

- Fixes Sustainsys#66.
- Saml2Respone.Validate is now a private method which is automatically called when needed by the Saml2Response class.
- Errors are reported through exceptions, with better exception messages.

Kentor.AuthServices.Tests/AllTestsOtherCultures.cs

@@ -36,7 +36,8 @@ where t.GetCustomAttribute<TestClassAttribute>() != null
                                    m => m.GetCustomAttribute<ClassCleanupAttribute>() != null).SingleOrDefault(),
                                    TestMethods = t.GetMethods().Where(
                                    m => m.GetCustomAttribute<TestMethodAttribute>() != null
-                                   && m.GetCustomAttribute<NotReRunnableAttribute>() == null).ToList()
+                                   && m.GetCustomAttribute<NotReRunnableAttribute>() == null
+                                   && m.GetCustomAttribute<IgnoreAttribute>() == null).ToList()
                                }).ToList();
 
             // These are the environments I have access to. Please feel free to add and checking whatever

Kentor.AuthServices.Tests/Internal/PendingAuthnRequestsTests.cs

@@ -51,5 +51,15 @@ public void PendingAuthnRequests_Remove_FalseOnRemovedTwice()
             PendingAuthnRequests.TryRemove(id, out responseData).Should().BeTrue();
             PendingAuthnRequests.TryRemove(id, out responseData).Should().BeFalse();
         }
+
+        [TestMethod]
+        public void PendingAuthnRequest_TryRemove_NullGivesNull()
+        {
+            Saml2Id id = null;
+            StoredRequestState state;
+
+            PendingAuthnRequests.TryRemove(id, out state).Should().BeFalse();
+            state.Should().BeNull();
+        }
     }
 }

Kentor.AuthServices.Tests/Kentor.AuthServices.Tests.csproj

@@ -113,6 +113,7 @@
     <Compile Include="Mvc\AuthServicesControllerTests.cs" />
     <Compile Include="ClaimsAuthenticationManagerStub.cs" />
     <Compile Include="ExceptionTestHelpers.cs" />
+    <Compile Include="Saml2ResponseFailedValidationExceptionTests.cs" />
     <Compile Include="WebSSO\MetadataCommandTests.cs" />
     <Compile Include="ExtendedMetadataSerializerTests.cs" />
     <Compile Include="Metadata\MetadataServer.cs" />

Kentor.AuthServices.Tests/Saml2ResponseFailedValidationExceptionTests.cs

@@ -0,0 +1,43 @@
+using System;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using FluentAssertions;
+
+namespace Kentor.AuthServices.Tests
+{
+    [TestClass]
+    public class Saml2ResponseFailedValidationExceptionTests
+    {
+        [TestMethod]
+        public void Saml2ResponseFailedValidationExecption_DefaultCtor()
+        {
+            ExceptionTestHelpers.TestDefaultCtor<Saml2ResponseFailedValidationException>();
+        }
+
+        [TestMethod]
+        public void Saml2ResponseFailedValidationExecption_SerializationCtor()
+        {
+            ExceptionTestHelpers.TestSerializationCtor<Saml2ResponseFailedValidationException>();
+        }
+
+        [TestMethod]
+        public void Saml2ResponseFailedValidationExecption_StringCtor()
+        {
+            var msg = "Message!";
+            var subject = new Saml2ResponseFailedValidationException(msg);
+
+            subject.Message.Should().Be(msg);
+        }
+
+        [TestMethod]
+        public void Saml2ResponseFailedValidationException_InnerExceptionCtor()
+        {
+            var msg = "Message!";
+            var innerException = new InvalidOperationException("InnerMessage!");
+
+            var subject = new Saml2ResponseFailedValidationException(msg, innerException);
+
+            subject.Message.Should().Be("Message!");
+            subject.InnerException.Message.Should().Be("InnerMessage!");
+        }
+    }
+}

Kentor.AuthServices.VSPremium.Tests/Saml2ResponseTests.cs

@@ -17,7 +17,7 @@ namespace Kentor.AuthServices.VSPremium.Tests
     public class Saml2ResponseTests
     {
         [TestMethod]
-        public void Saml2Response_Validate_FalseOnMissingReferenceInSignature()
+        public void Saml2Response_Validate_ThrowsOnMissingReferenceInSignature()
         {
             var response =
             @"<saml2p:Response xmlns:saml2p=""urn:oasis:names:tc:SAML:2.0:protocol""
@@ -155,7 +155,10 @@ public void Saml2Response_Validate_FalseOnMissingReferenceInSignature()
 
                 var samlResponse = Saml2Response.Read(xmlDoc.OuterXml);
 
-                samlResponse.Validate(Options.FromConfiguration).Should().BeFalse();
+                Action a = () => samlResponse.GetClaims(Options.FromConfiguration);
+
+                a.ShouldThrow<Saml2ResponseFailedValidationException>()
+                    .WithMessage("No reference found in Xml signature, it doesn't validate the Xml data.");
             }
         }
     }

Kentor.AuthServices/BadFormatSamlResponseException.cs

@@ -8,7 +8,7 @@
 namespace Kentor.AuthServices
 {
     /// <summary>
-    /// A samle response was found, but could not be parsed due to formatting issues.
+    /// A SAML response was found, but could not be parsed due to formatting issues.
     /// </summary>
     [Serializable]
     public class BadFormatSamlResponseException: AuthServicesException
@@ -42,7 +42,6 @@ public BadFormatSamlResponseException(string message, Exception innerException)
         /// <param name="context">Serialization context</param>
         protected BadFormatSamlResponseException(SerializationInfo info, StreamingContext context)
             : base(info, context)
-        {
-        }
+        { }
     }
 }

Kentor.AuthServices/Kentor.AuthServices.csproj

@@ -54,6 +54,7 @@
       <Link>VersionInfo.cs</Link>
     </Compile>
     <Compile Include="Metadata\KeyInfoSerializer.cs" />
+    <Compile Include="Saml2ResponseFailedValidationException.cs" />
     <Compile Include="WebSso\AcsCommand.cs" />
     <Compile Include="Metadata\AttributeConsumingService.cs" />
     <Compile Include="Configuration\RequestedAttributeElement.cs" />

Kentor.AuthServices/SAML2P/Saml2Response.cs

@@ -244,12 +244,16 @@ public Uri DestinationUrl
             }
         }
 
+        StoredRequestState requestState;
+
         /// <summary>
         /// State stored by a corresponding request
         /// </summary>
-        public StoredRequestState RequestState { get; private set; }
-
-        bool valid = false, validated = false;
+        public StoredRequestState GetRequestState(IOptions options)
+        {
+            Validate(options);
+            return requestState;
+        }
 
         /// <summary>Gets all assertion element nodes from this response message.</summary>
         /// <value>All assertion element nodes.</value>
@@ -269,67 +273,89 @@ private IEnumerable<XmlElement> AllAssertionElementNodes
             }
         }
 
-        /// <summary>
-        /// Validates InResponseTo and the signature of the response. Note that the status code of the
-        /// message can still be an error code, although the message itself is valid.
-        /// </summary>
-        /// <param name="options">Options with info about trusted Idps.</param>
-        /// <returns>Is the response signed by the Idp and fulfills other formal requirements?</returns>
-        public bool Validate(IOptions options)
+        bool validated = false;
+        Saml2ResponseFailedValidationException validationException;
+
+        private void Validate(IOptions options)
         {
             if (!validated)
             {
-                valid = ValidateInResponseTo(options) && ValidateSignature(options);
-
-                validated = true;
+                try
+                {
+                    ValidateInResponseTo(options);
+                    ValidateSignature(options);
+                }
+                catch (Saml2ResponseFailedValidationException ex)
+                {
+                    validationException = ex;
+                    throw;
+                }
+                finally
+                {
+                    validated = true;
+                }
+            }
+            else
+            {
+                if (validationException != null)
+                {
+                    throw validationException;
+                }
             }
-            return valid;
         }
 
-        private bool ValidateInResponseTo(IOptions options)
+        [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA2204:Literals should be spelled correctly", MessageId = "InResponseTo")]
+        private void ValidateInResponseTo(IOptions options)
         {
-            if (InResponseTo == null && options.IdentityProviders[Issuer].AllowUnsolicitedAuthnResponse)
+            if (InResponseTo == null)
             {
-                return true;
+                if (options.IdentityProviders[Issuer].AllowUnsolicitedAuthnResponse)
+                {
+                    return;
+                }
+                string msg = string.Format(CultureInfo.InvariantCulture,
+                    "Unsolicited responses are not allowed for idp \"{0}\".", Issuer.Id);
+                throw new Saml2ResponseFailedValidationException(msg);
             }
             else
             {
                 StoredRequestState storedRequestState;
                 bool knownInResponseToId = PendingAuthnRequests.TryRemove(InResponseTo, out storedRequestState);
                 if (!knownInResponseToId)
                 {
-                    return false;
+                    string msg = string.Format(CultureInfo.InvariantCulture,
+                        "Replayed or unknown InResponseTo \"{0}\".", InResponseTo);
+
+                    throw new Saml2ResponseFailedValidationException(msg);
                 }
-                RequestState = storedRequestState;
-                if (RequestState.Idp.Id != Issuer.Id)
+                requestState = storedRequestState;
+                if (requestState.Idp.Id != Issuer.Id)
                 {
-                    return false;
+                    var msg = string.Format(CultureInfo.InvariantCulture,
+                        "Expected response from idp \"{0}\" but received response from idp \"{1}\".",
+                        requestState.Idp.Id, issuer.Id);
+                    throw new Saml2ResponseFailedValidationException(msg);
                 }
-                return true;
             }
         }
 
-        private bool ValidateSignature(IOptions options)
+        private void ValidateSignature(IOptions options)
         {
             var idpKey = options.IdentityProviders[Issuer].SigningKey;
 
             // If the response message is signed, we check just this signature because the whole content has to be correct then
             var responseSignature = xmlDocument.DocumentElement["Signature", SignedXml.XmlDsigNamespaceUrl];
             if (responseSignature != null)
             {
-                return CheckSignature(XmlDocument.DocumentElement, idpKey);
+                CheckSignature(XmlDocument.DocumentElement, idpKey);
             }
             else
             {
                 // If the response message is not signed, all assersions have to be signed correctly
                 foreach (var assertionNode in AllAssertionElementNodes)
                 {
-                    if (!CheckSignature(assertionNode, idpKey))
-                    {
-                        return false;
-                    }
+                    CheckSignature(assertionNode, idpKey);
                 }
-                return true;
             }
         }
 
@@ -343,54 +369,51 @@ private bool ValidateSignature(IOptions options)
         /// <summary>Checks the signature.</summary>
         /// <param name="signedRootElement">The signed root element.</param>
         /// <param name="idpKey">The assymetric key of the algorithm.</param>
-        /// <returns><c>true</c> if the whole signature was successful; otherwise <c>false</c></returns>
-        private static bool CheckSignature(XmlElement signedRootElement, AsymmetricAlgorithm idpKey)
+        private static void CheckSignature(XmlElement signedRootElement, AsymmetricAlgorithm idpKey)
         {
             var xmlDocument = new XmlDocument { PreserveWhitespace = true };
             xmlDocument.LoadXml(signedRootElement.OuterXml);
 
             var signature = xmlDocument.DocumentElement["Signature", SignedXml.XmlDsigNamespaceUrl];
             if (signature == null)
             {
-                return false;
+                throw new Saml2ResponseFailedValidationException("The SAML Response is not signed and contains unsigned Assertions. Response cannot be trusted.");
             }
 
             var signedXml = new SignedXml(xmlDocument);
             signedXml.LoadXml(signature);
 
             var signedRootElementId = "#" + signedRootElement.GetAttribute("ID");
 
-            var reference = signedXml.SignedInfo.References.Cast<Reference>().FirstOrDefault();
-
-            if (signedXml.SignedInfo.References.Count != 1 || reference.Uri != signedRootElementId)
+            if (signedXml.SignedInfo.References.Count == 0)
             {
-                return false;
+                throw new Saml2ResponseFailedValidationException("No reference found in Xml signature, it doesn't validate the Xml data.");
             }
 
-            foreach (Transform transform in reference.TransformChain)
+            if (signedXml.SignedInfo.References.Count != 1)
             {
-                if (!allowedTransforms.Contains(transform.Algorithm))
-                {
-                    return false;
-                }
+                throw new Saml2ResponseFailedValidationException("Multiple references for Xml signatures are not allowed.");
             }
 
-            return signedXml.CheckSignature(idpKey);
-        }
+            var reference = signedXml.SignedInfo.References.Cast<Reference>().Single();
 
-        private void ThrowOnNotValid()
-        {
-            if (!validated)
+            if (reference.Uri != signedRootElementId)
             {
-                throw new InvalidOperationException("The Saml2Response must be validated first.");
+                throw new Saml2ResponseFailedValidationException("Incorrect reference on Xml signature. The reference must be to the root element of the element containing the signature.");
             }
-            if (!valid)
+
+            foreach (Transform transform in reference.TransformChain)
             {
-                throw new InvalidOperationException("The Saml2Response didn't pass validation");
+                if (!allowedTransforms.Contains(transform.Algorithm))
+                {
+                    throw new Saml2ResponseFailedValidationException(
+                        "Transform \"" + transform.Algorithm + "\" found in Xml signature is not allowed in SAML.");
+                }
             }
-            if (status != Saml2StatusCode.Success)
+
+            if (!signedXml.CheckSignature(idpKey))
             {
-                throw new InvalidOperationException("The Saml2Response must have status success to extract claims.");
+                throw new Saml2ResponseFailedValidationException("Signature validation failed on SAML response or contained assertion.");
             }
         }
 
@@ -404,7 +427,7 @@ private void ThrowOnNotValid()
         /// <returns>ClaimsIdentities</returns>
         // Method might throw expections so make it a method and not a property.
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1024:UsePropertiesWhereAppropriate")]
-        public IEnumerable<ClaimsIdentity> GetClaims(ISPOptions options)
+        public IEnumerable<ClaimsIdentity> GetClaims(IOptions options)
         {
             if (createClaimsException != null)
             {
@@ -427,15 +450,20 @@ public IEnumerable<ClaimsIdentity> GetClaims(ISPOptions options)
             return claimsIdentities;
         }
 
-        private IEnumerable<ClaimsIdentity> CreateClaims(ISPOptions options)
+        private IEnumerable<ClaimsIdentity> CreateClaims(IOptions options)
         {
-            ThrowOnNotValid();
+            Validate(options);
+
+            if (status != Saml2StatusCode.Success)
+            {
+                throw new InvalidOperationException("The Saml2Response must have status success to extract claims.");
+            }
 
             foreach (XmlElement assertionNode in AllAssertionElementNodes)
             {
                 using (var reader = new XmlNodeReader(assertionNode))
                 {
-                    var handler = options.Saml2PSecurityTokenHandler;
+                    var handler = options.SPOptions.Saml2PSecurityTokenHandler;
 
                     var token = (Saml2SecurityToken)handler.ReadToken(reader);
                     handler.DetectReplayedToken(token);