From 9a0376d3c5fac6be1a1ae24d7ed68dded91501c9 Mon Sep 17 00:00:00 2001
From: Ignacio Inglese <iinglese@microsoft.com>
Date: Mon, 30 Sep 2024 18:26:59 +0100
Subject: [PATCH] Refactor ValidateConditions in Saml2SecurityTokenHandler
 (#2855)

* Sample refactor of ValidateConditions in Saml2SecurityTokenHandler (validate lifetime, audience, token replay, and potentially proxy)

* Made ValidateConditions virtual
---
 .../Saml2/Saml2SecurityTokenHandler.cs        | 93 +++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.cs
index 90184df46f..da190bc158 100644
--- a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.cs
+++ b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.cs
@@ -1048,6 +1048,99 @@ protected virtual void ValidateConditions(Saml2SecurityToken samlToken, TokenVal
                 throw LogExceptionMessage(new Saml2SecurityTokenException(LogMessages.IDX13002));
         }
 
+#nullable enable
+        // ValidatedConditions is basically a named tuple but using a record struct better expresses the intent.
+        internal record struct ValidatedConditions(string? ValidatedAudience, ValidatedLifetime? ValidatedLifetime);
+
+        internal virtual ValidationResult<ValidatedConditions> ValidateConditions(Saml2SecurityToken samlToken, ValidationParameters validationParameters, CallContext callContext)
+        {
+            if (samlToken == null)
+                return ValidationError.NullParameter(nameof(samlToken), new System.Diagnostics.StackFrame(true));
+
+            if (validationParameters == null)
+                return ValidationError.NullParameter(nameof(validationParameters), new System.Diagnostics.StackFrame(true));
+
+            if (samlToken.Assertion == null)
+                return ValidationError.NullParameter(nameof(samlToken.Assertion), new System.Diagnostics.StackFrame(true));
+
+            // TokenValidationParameters.RequireAudience is only used for SAML.
+            // Should we add this to ValidationParameters? 
+            // Should it be just a field in Saml2SecurityTokenHandler?
+            bool requireAudience = true;
+
+            if (samlToken.Assertion.Conditions == null)
+            {
+                if (requireAudience)
+                    return new ValidationError(
+                        new MessageDetail(LogMessages.IDX13002),
+                        ValidationFailureType.AudienceValidationFailed,
+                        typeof(Saml2SecurityTokenException),
+                        new System.Diagnostics.StackFrame(true));
+
+                return new ValidatedConditions(null, null); // no error occurred. There is no validated audience or lifetime.
+            }
+
+            var lifetimeValidationResult = validationParameters.LifetimeValidator(
+                samlToken.Assertion.Conditions.NotBefore, samlToken.Assertion.Conditions.NotOnOrAfter, samlToken, validationParameters, callContext);
+            if (!lifetimeValidationResult.IsSuccess)
+                return lifetimeValidationResult.UnwrapError();
+
+            if (samlToken.Assertion.Conditions.OneTimeUse)
+            {
+                //ValidateOneTimeUseCondition(samlToken, validationParameters);
+                // We can keep an overridable method for this, or rely on the TokenReplayValidator delegate.
+                var oneTimeUseValidationResult = validationParameters.TokenReplayValidator(
+                    samlToken.Assertion.Conditions.NotOnOrAfter, samlToken.Assertion.CanonicalString, validationParameters, callContext);
+                if (!oneTimeUseValidationResult.IsSuccess)
+                    return oneTimeUseValidationResult.UnwrapError();
+            }
+
+            if (samlToken.Assertion.Conditions.ProxyRestriction != null)
+            {
+                //throw LogExceptionMessage(new SecurityTokenValidationException(LogMessages.IDX13511));
+                var proxyValidationError = ValidateProxyRestriction(samlToken, validationParameters, callContext);
+                if (proxyValidationError is not null)
+                    return proxyValidationError;
+            }
+
+            string? validatedAudience = null;
+            foreach (var audienceRestriction in samlToken.Assertion.Conditions.AudienceRestrictions)
+            {
+                // AudienceRestriction.Audiences is a List<string> but returned as ICollection<string>
+                // no conversion occurs, ToList() is never called but we have to account for the possibility.
+                if (!(audienceRestriction.Audiences is List<string> audiencesAsList))
+                    audiencesAsList = audienceRestriction.Audiences.ToList();
+
+                var audienceValidationResult = validationParameters.AudienceValidator(
+                    audiencesAsList, samlToken, validationParameters, callContext);
+                if (!audienceValidationResult.IsSuccess)
+                    return audienceValidationResult.UnwrapError();
+
+                // Audience is valid, save it for later.
+                validatedAudience = audienceValidationResult.UnwrapResult();
+            }
+
+            if (requireAudience && validatedAudience is null)
+            {
+                return new ValidationError(
+                    new MessageDetail(LogMessages.IDX13002),
+                    ValidationFailureType.AudienceValidationFailed,
+                    typeof(Saml2SecurityTokenException),
+                    new System.Diagnostics.StackFrame(true));
+            }
+
+            return new ValidatedConditions(validatedAudience, lifetimeValidationResult.UnwrapResult()); // no error occurred. There is nothing else to return.
+        }
+
+#pragma warning disable CA1801 // Review unused parameters
+        internal virtual ValidationError? ValidateProxyRestriction(Saml2SecurityToken samlToken, ValidationParameters validationParameters, CallContext callContext)
+#pragma warning restore CA1801 // Review unused parameters
+        {
+            // return an error, or ignore and allow overriding?
+            return null;
+        }
+#nullable restore
+
         /// <summary>
         /// Validates the OneTimeUse condition.
         /// </summary>