Fix token replay detection

- Fixes #1107
- Fixes #711

Sustainsys.Saml2/Configuration/SPOptions.cs

@@ -1,5 +1,7 @@
-using Sustainsys.Saml2.Metadata;
+using Microsoft.IdentityModel.Tokens;
+using Sustainsys.Saml2.Metadata;
 using Sustainsys.Saml2.Saml2P;
+using Sustainsys.Saml2.Tokens;
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
@@ -22,7 +24,7 @@ public class SPOptions
         /// </summary>
         public SPOptions()
         {
-			MetadataCacheDuration = new XsdDuration(hours: 1);
+            MetadataCacheDuration = new XsdDuration(hours: 1);
             Compatibility = new Compatibility();
             OutboundSigningAlgorithm = XmlHelpers.GetDefaultSigningAlgorithmName();
             MinIncomingSigningAlgorithm = XmlHelpers.GetDefaultSigningAlgorithmName();
@@ -102,7 +104,7 @@ public Saml2PSecurityTokenHandler Saml2PSecurityTokenHandler
                 // Capture in a local variable to prevent race conditions. Reads and writes
                 // of references are atomic so there is no need for a lock.
                 var value = saml2PSecurityTokenHandler;
-                if(value == null)
+                if (value == null)
                 {
                     // Set the saved value, but don't trust it - still use a local var for the return.
                     saml2PSecurityTokenHandler = value = new Saml2PSecurityTokenHandler(this);
@@ -112,7 +114,7 @@ public Saml2PSecurityTokenHandler Saml2PSecurityTokenHandler
             }
             set
             {
-                saml2PSecurityTokenHandler = value; 
+                saml2PSecurityTokenHandler = value;
             }
         }
 
@@ -135,7 +137,7 @@ public EntityId EntityId
             }
             set
             {
-                if(saml2PSecurityTokenHandler != null)
+                if (saml2PSecurityTokenHandler != null)
                 {
                     throw new InvalidOperationException("Can't change entity id when a token handler has been instantiated.");
                 }
@@ -157,7 +159,7 @@ public string ModulePath
             }
             set
             {
-                if(value == null)
+                if (value == null)
                 {
                     throw new ArgumentNullException(nameof(value));
                 }
@@ -212,7 +214,7 @@ public ICollection<ContactPerson> Contacts
         }
 
         readonly ICollection<AttributeConsumingService> attributeConsumingServices
-			= new List<AttributeConsumingService>();
+            = new List<AttributeConsumingService>();
 
         /// <summary>
         /// Collection of attribute consuming services for the service provider.
@@ -292,7 +294,7 @@ public ReadOnlyCollection<ServiceCertificate> MetadataCertificates
                 var futureBothCertExists = metaDataCertificates
                     .Any(c => c.Status == CertificateStatus.Future && c.Use == CertificateUse.Both);
 
-                foreach(var cert in metaDataCertificates)
+                foreach (var cert in metaDataCertificates)
                 {
                     // Just like we stop publishing Encryption cert immediately when a Future one is added,
                     // in the case of a "Both" cert we should switch the current use to Signing so that Idp's stop sending
@@ -358,7 +360,7 @@ private IEnumerable<ServiceCertificate> PublishableServiceCertificates
         /// overriden for each <see cref="IdentityProvider"/>.
         /// </summary>
         public string OutboundSigningAlgorithm { get; set; }
-        
+
         /// <summary>
         /// Metadata flag that we want assertions to be signed.
         /// </summary>
@@ -379,7 +381,7 @@ private IEnumerable<ServiceCertificate> PublishableServiceCertificates
         public Compatibility Compatibility { get; set; }
 
         private string minIncomingSigningAlgorithm;
-        
+
         /// <summary>
         /// Minimum accepted signature algorithm for any incoming messages.
         /// </summary>
@@ -391,7 +393,7 @@ public string MinIncomingSigningAlgorithm
             }
             set
             {
-                if(!XmlHelpers.KnownSigningAlgorithms.Contains(value))
+                if (!XmlHelpers.KnownSigningAlgorithms.Contains(value))
                 {
                     throw new ArgumentException("The signing algorithm " + value +
                         " is unknown or not supported by the current .NET Framework.");
@@ -404,5 +406,22 @@ public string MinIncomingSigningAlgorithm
         /// Adapter to logging framework of hosting application.
         /// </summary>
         public ILoggerAdapter Logger { get; set; }
-    }
+
+        private ITokenReplayCache tokenReplayCache;
+        public ITokenReplayCache TokenReplayCache 
+        { 
+            get
+            {
+                if(tokenReplayCache == null)
+                {
+                    tokenReplayCache = new TokenReplayCache();
+                }
+                return tokenReplayCache;
+            }
+            set
+            {
+                tokenReplayCache = value;
+            }
+        }
+}
 }

Sustainsys.Saml2/SAML2P/Saml2PSecurityTokenHandler.cs

@@ -28,19 +28,6 @@ public Saml2PSecurityTokenHandler(SPOptions spOptions)
 			Serializer = new Saml2PSerializer(spOptions);
 		}
 
-		// Overridden to fix the fact that the base class version uses NotBefore as the token replay expiry time
-		// Due to the fact that we can't override the ValidateToken function (it's overridden in the base class!)
-		// we have to parse the token again.
-		// This can be removed when:
-		// https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/898
-		// is fixed.
-		protected override void ValidateTokenReplay(DateTime? expirationTime, string securityToken, TokenValidationParameters validationParameters)
-		{
-			var saml2Token = ReadSaml2Token(securityToken);
-			base.ValidateTokenReplay(saml2Token.Assertion.Conditions.NotOnOrAfter,
-				securityToken, validationParameters);
-		}
-
 		// TODO: needed with Microsoft.identitymodel?
 		/// <summary>
 		/// Process authentication statement from SAML assertion. WIF chokes if the authentication statement 
@@ -84,10 +71,27 @@ protected override void ProcessAuthenticationStatement(Saml2AuthenticationStatem
             }
         }
 
-		protected override Saml2SecurityToken ValidateSignature(string token, TokenValidationParameters validationParameters)
+		// Override and build our own logic. The problem is ValidateTokenReplay that serializes the token back. And that
+		// breaks because it expects some optional values to be present.
+		public override ClaimsPrincipal ValidateToken(string token, TokenValidationParameters validationParameters, out Microsoft.IdentityModel.Tokens.SecurityToken validatedToken)
 		{
-			// Just skip signature validation -- we do this elsewhere
-			return ReadSaml2Token(token);
+			var samlToken = ReadSaml2Token(token);
+
+			ValidateConditions(samlToken, validationParameters);
+			ValidateSubject(samlToken, validationParameters);
+
+			var issuer = ValidateIssuer(samlToken.Issuer, samlToken, validationParameters);
+
+			// Just using the assertion id for token replay. As that is part of the signed value it cannot
+			// be altered by someone replaying the token.
+			ValidateTokenReplay(samlToken.Assertion.Conditions.NotOnOrAfter, samlToken.Assertion.Id.Value, validationParameters);
+
+			// ValidateIssuerSecurityKey not called - we have our own signature validation.
+
+			validatedToken = samlToken;
+			var identity = CreateClaimsIdentity(samlToken, issuer, validationParameters);
+
+			return new ClaimsPrincipal(identity);
 		}
-    }
+	}
 }

Sustainsys.Saml2/SAML2P/Saml2Response.cs

@@ -597,6 +597,9 @@ private IEnumerable<ClaimsIdentity> CreateClaims(IOptions options, IdentityProvi
 			validationParameters.RequireSignedTokens = false;
 			validationParameters.ValidateIssuer = false;
             validationParameters.ValidAudience = options.SPOptions.EntityId.Id;
+            validationParameters.RequireAudience = false; // Audience restriction optional in SAML2 spec.
+            validationParameters.TokenReplayCache = options.SPOptions.TokenReplayCache;
+            validationParameters.ValidateTokenReplay = true;
 
             options.Notifications.Unsafe.TokenValidationParametersCreated(validationParameters, idp, XmlElement);
 

Tests/Tests.Shared/Saml2P/Saml2ResponseTests.cs

@@ -1918,8 +1918,6 @@ public void Saml2Response_GetClaims_ThrowsOnWeakSigningAlgoritm()
         [TestMethod]
         public void Saml2Response_GetClaims_ThrowsOnReplayAssertionId()
         {
-            Assert.Inconclusive("Deliberately ignored test for now");
-
             var response =
             @"<?xml version=""1.0"" encoding=""UTF-8""?>
             <saml2p:Response xmlns:saml2p=""urn:oasis:names:tc:SAML:2.0:protocol""
@@ -1955,8 +1953,6 @@ public void Saml2Response_GetClaims_ThrowsOnReplayAssertionId()
         [TestMethod]
         public void Saml2Response_GetClaims_ThrowsOnReplayAssertionIdSameConfig()
         {
-            Assert.Inconclusive("Ingored for now");
-
             var response =
             @"<?xml version=""1.0"" encoding=""UTF-8""?>
             <saml2p:Response xmlns:saml2p=""urn:oasis:names:tc:SAML:2.0:protocol""

Tests/Tests.Shared/WebSSO/Saml2ArtifactBindingTests.cs

@@ -139,8 +139,6 @@ public void Saml2ArtifactBinding_Unbind_FromGet_ArtifactIsntHashOfEntityId()
                 null,
                 new StoredRequestState(issuer, null, null, null));
 
-            StubServer.LastArtifactResolutionSoapActionHeader = null;
-
             var result = Saml2Binding.Get(Saml2BindingType.Artifact).Unbind(r, StubFactory.CreateOptions());
 
             var xmlDocument = XmlHelpers.XmlDocumentFromString(