From 2ffcc269a0a94c1ef0cf6999cc692f5caf83253e Mon Sep 17 00:00:00 2001 From: Franco Fung Date: Wed, 2 Oct 2024 10:08:31 -0700 Subject: [PATCH 01/10] Initial test to check for painpoints integrating Wilson validation model for SAML --- ...rityTokenHandler.ValidateToken.Internal.cs | 138 ++++++++++++++++++ .../Saml2/Saml2SecurityTokenHandler.cs | 95 +----------- ...rTests.ValidateTokenAsyncTests.Audience.cs | 126 ++++++++++++++++ ...lerTests.ValidateTokenAsyncTests.Common.cs | 115 +++++++++++++++ .../Saml2SecurityTokenHandlerTests.cs | 2 +- .../SkipValidationDelegates.cs | 99 +++++++++++++ 6 files changed, 480 insertions(+), 95 deletions(-) create mode 100644 src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs create mode 100644 test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs create mode 100644 test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Common.cs create mode 100644 test/Microsoft.IdentityModel.Tokens.Saml.Tests/SkipValidationDelegates.cs diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs new file mode 100644 index 0000000000..783bf10e0f --- /dev/null +++ b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs @@ -0,0 +1,138 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; + +#nullable enable +namespace Microsoft.IdentityModel.Tokens.Saml2 +{ + /// + /// A designed for creating and validating Saml2 Tokens. See: http://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf + /// + public partial class Saml2SecurityTokenHandler : SecurityTokenHandler + { + +#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously + internal async Task> ValidateTokenAsync( +#pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously + Saml2SecurityToken samlToken, + ValidationParameters validationParameters, + CallContext callContext, +#pragma warning disable CA1801 // Review unused parameters + CancellationToken cancellationToken) +#pragma warning restore CA1801 // Review unused parameters + { + var conditionsResult = ValidateConditions(samlToken, validationParameters, callContext); + + if (!conditionsResult.IsSuccess) + { + return conditionsResult.UnwrapError().AddStackFrame(new StackFrame(true)); + } + + //These TODO's follow the pattern of the current ValidateToken methods. They should be implemented in the future. + //TODO: ValidateSubject() - Skip for now + //TODO: ValidateIssuer() + //TODO: ValidateIssuerSecurityKey()...etc + + return new ValidatedToken(samlToken, this, validationParameters); + } + + + + // 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 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 but returned as ICollection + // no conversion occurs, ToList() is never called but we have to account for the possibility. + if (!(audienceRestriction.Audiences is List 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 diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.cs index da190bc158..9e2ad28749 100644 --- a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.cs +++ b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.cs @@ -22,7 +22,7 @@ namespace Microsoft.IdentityModel.Tokens.Saml2 /// /// A designed for creating and validating Saml2 Tokens. See: http://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf /// - public class Saml2SecurityTokenHandler : SecurityTokenHandler + public partial class Saml2SecurityTokenHandler : SecurityTokenHandler { private const string _actor = "Actor"; private const string _className = "Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler"; @@ -1048,99 +1048,6 @@ 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 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 but returned as ICollection - // no conversion occurs, ToList() is never called but we have to account for the possibility. - if (!(audienceRestriction.Audiences is List 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 - /// /// Validates the OneTimeUse condition. /// diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs new file mode 100644 index 0000000000..0fa69d4023 --- /dev/null +++ b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs @@ -0,0 +1,126 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.IdentityModel.TestUtils; +using Microsoft.IdentityModel.Tokens.Saml2; +using Xunit; + +namespace Microsoft.IdentityModel.Tokens.Saml.Tests +{ +#nullable enable + public partial class Saml2SecurityTokenHandlerTests + { + + [Theory, MemberData(nameof(ValidateTokenAsync_Audience_TestCases), DisableDiscoveryEnumeration = true)] + public async Task ValidateTokenAsync_Audience(ValidateTokenAsyncAudienceTheoryData theoryData) + { + var context = TestUtilities.WriteHeader($"{this}.ValidateTokenAsync_Audience", theoryData); + + Saml2SecurityTokenHandler saml2TokenHandler = new Saml2SecurityTokenHandler(); + + var saml2Token = CreateToken(theoryData.Audience!, theoryData.Saml2Condition!); + + var validationParameters = CreateTokenValidationParameters( + new List { theoryData.Audience! }, + saml2Token, + false); //TODO: continue looking into improving this approach + + await ValidateAndCompareResults(saml2Token, validationParameters, theoryData, context); + + TestUtilities.AssertFailIfErrors(context); + } + + public static TheoryData ValidateTokenAsync_Audience_TestCases + { + get + { + return new TheoryData + { + new ValidateTokenAsyncAudienceTheoryData("Valid_AudiencesMatch") + { + Audience = Default.Audience, + Saml2Condition = new Saml2Conditions + { + OneTimeUse = false, + NotOnOrAfter = DateTime.UtcNow.AddMinutes(5), + }, + ValidationParameters = CreateValidationParameters([Default.Audience]) + } + }; + + static ValidationParameters CreateValidationParameters( + List audiences, + bool ignoreTrailingSlashWhenValidatingAudience = false) + { + ValidationParameters validationParameters = new ValidationParameters(); + audiences.ForEach(audience => validationParameters.ValidAudiences.Add(audience)); + validationParameters.IgnoreTrailingSlashWhenValidatingAudience = ignoreTrailingSlashWhenValidatingAudience; + validationParameters.LifetimeValidator = SkipValidationDelegates.SkipLifetimeValidation; + validationParameters.TokenReplayValidator = SkipValidationDelegates.SkipTokenReplayValidation; + + return validationParameters; + } + } + } + + public class ValidateTokenAsyncAudienceTheoryData : ValidateTokenAsyncBaseTheoryData + { + public ValidateTokenAsyncAudienceTheoryData(string testId) : base(testId) { } + + public string? Audience { get; internal set; } = Default.Audience; + + public Saml2Conditions? Saml2Condition { get; internal set; } + + public Saml2SecurityToken? Saml2SecurityToken { get; internal set; } + + public bool ignoreTrailingSlashWhenValidatingAudience { get; internal set; } + } + + private static Saml2SecurityToken CreateToken(string audience, Saml2Conditions saml2Conditions) + { + Saml2SecurityTokenHandler saml2TokenHandler = new Saml2SecurityTokenHandler(); + + SecurityTokenDescriptor securityTokenDescriptor = new SecurityTokenDescriptor + { + Expires = DateTime.UtcNow + TimeSpan.FromDays(1), + Audience = audience, + SigningCredentials = Default.AsymmetricSigningCredentials, + Issuer = Default.Issuer, + Subject = Default.SamlClaimsIdentity + }; + + Saml2SecurityToken saml2Token = (Saml2SecurityToken)saml2TokenHandler.CreateToken(securityTokenDescriptor); + /* + if (saml2Conditions != null) + saml2Token.Assertion.Conditions = saml2Conditions;*/ //TODO: Figure out how to adapt thisto more complex scenarios + + return saml2Token; + } + + private static TokenValidationParameters CreateTokenValidationParameters( + List? audiences, + Saml2SecurityToken saml2SecurityToken, + bool ignoreTrailingSlashWhenValidatingAudience = false) + { + return new TokenValidationParameters + { + ValidateAudience = true, + ValidateIssuer = false, + ValidateLifetime = false, + ValidateTokenReplay = false, + ValidateIssuerSigningKey = false, + RequireSignedTokens = false, + ValidAudiences = audiences, + IgnoreTrailingSlashWhenValidatingAudience = ignoreTrailingSlashWhenValidatingAudience, + SignatureValidator = delegate (string token, TokenValidationParameters validationParameters) + { + return saml2SecurityToken; + } + }; + } + } +} +#nullable restore diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Common.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Common.cs new file mode 100644 index 0000000000..0f0bda551c --- /dev/null +++ b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Common.cs @@ -0,0 +1,115 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Threading; +using System.Threading.Tasks; +using Microsoft.IdentityModel.TestUtils; +using Microsoft.IdentityModel.Tokens.Saml2; + +namespace Microsoft.IdentityModel.Tokens.Saml.Tests +{ +#nullable enable + public partial class Saml2SecurityTokenHandlerTests + { + internal static async Task ValidateAndCompareResults( + Saml2SecurityToken saml2Token, + TokenValidationParameters tokenValidationParameters, + ValidateTokenAsyncBaseTheoryData theoryData, + CompareContext context) + { + Saml2SecurityTokenHandler saml2TokenHandler = new Saml2SecurityTokenHandler(); + + // Validate the token using TokenValidationParameters + TokenValidationResult legacyTokenValidationParametersResult = + await saml2TokenHandler.ValidateTokenAsync(saml2Token.Assertion.CanonicalString, tokenValidationParameters); + + // Validate the token using ValidationParameters + ValidationResult validationParametersResult = + await saml2TokenHandler.ValidateTokenAsync( + saml2Token, + theoryData.ValidationParameters!, + theoryData.CallContext, + CancellationToken.None); + + // Ensure the validity of the results match the expected result + if (legacyTokenValidationParametersResult.IsValid != theoryData.ExpectedIsValid) + context.AddDiff($"tokenValidationParametersResult.IsValid != theoryData.ExpectedIsValid"); + + if (validationParametersResult.IsSuccess != theoryData.ExpectedIsValid) + context.AddDiff($"validationParametersResult.IsSuccess != theoryData.ExpectedIsValid"); + + if (theoryData.ExpectedIsValid && + legacyTokenValidationParametersResult.IsValid && + validationParametersResult.IsSuccess) + { + // This should compare the ClaimsPrincipal and ClaimsIdentity from one result against the other but right now we have not defined how we will handle this + /*IdentityComparer.AreEqual( + legacyTokenValidationParametersResult.ClaimsIdentity, + validationParametersResult.UnwrapResult().ClaimsIdentity, + context); + IdentityComparer.AreEqual( + legacyTokenValidationParametersResult.Claims, + validationParametersResult.UnwrapResult().Claims, + context);*/ + } + else + { + // Verify the exception provided by the TokenValidationParameters path + theoryData.ExpectedException.ProcessException(legacyTokenValidationParametersResult.Exception, context); + + if (!validationParametersResult.IsSuccess) + { + // Verify the exception provided by the ValidationParameters path + if (theoryData.ExpectedExceptionValidationParameters is not null) + { + // If there is a special case for the ValidationParameters path, use that. + theoryData.ExpectedExceptionValidationParameters + .ProcessException(validationParametersResult.UnwrapError().GetException(), context); + } + else + { + theoryData.ExpectedException + .ProcessException(validationParametersResult.UnwrapError().GetException(), context); + + // If the expected exception is the same in both paths, verify the message matches + IdentityComparer.AreStringsEqual( + legacyTokenValidationParametersResult.Exception.Message, + validationParametersResult.UnwrapError().GetException().Message, + context); + } + } + + // Verify that the exceptions are of the same type. + IdentityComparer.AreEqual( + legacyTokenValidationParametersResult.Exception.GetType(), + validationParametersResult.UnwrapError().GetException().GetType(), + context); + + if (legacyTokenValidationParametersResult.Exception is SecurityTokenException) + { + // Verify that the custom properties are the same. + IdentityComparer.AreSecurityTokenExceptionsEqual( + legacyTokenValidationParametersResult.Exception, + validationParametersResult.UnwrapError().GetException(), + context); + } + } + } + } + + public class ValidateTokenAsyncBaseTheoryData : TheoryDataBase + { + public ValidateTokenAsyncBaseTheoryData(string testId) : base(testId) { } + + internal bool ExpectedIsValid { get; set; } = true; + + //internal TokenValidationParameters? TokenValidationParameters { get; set; } + + internal ValidationParameters? ValidationParameters { get; set; } + + // only set if we expect a different message on this path + internal ExpectedException? ExpectedExceptionValidationParameters { get; set; } = null; + } + +} +#nullable restore diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.cs index 6cb2ad5c94..94f5bfcb2a 100644 --- a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.cs +++ b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.cs @@ -14,7 +14,7 @@ namespace Microsoft.IdentityModel.Tokens.Saml2.Tests { - public class Saml2SecurityTokenHandlerTests + public partial class Saml2SecurityTokenHandlerTests { [Fact] public void Constructors() diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/SkipValidationDelegates.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/SkipValidationDelegates.cs new file mode 100644 index 0000000000..bb2c52a705 --- /dev/null +++ b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/SkipValidationDelegates.cs @@ -0,0 +1,99 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; + +//TODO: Remove this file and use the one in TestUtils once new validation model is public. +#nullable enable +namespace Microsoft.IdentityModel.Tokens.Saml.Tests +{ + public static class SkipValidationDelegates + { + internal static AlgorithmValidationDelegate SkipAlgorithmValidation = delegate ( + string algorithm, + SecurityKey securityKey, + SecurityToken securityToken, + ValidationParameters + validationParameters, + CallContext callContext) + { + return algorithm; + }; + + internal static AudienceValidationDelegate SkipAudienceValidation = delegate ( + IList audiences, + SecurityToken? securityToken, + ValidationParameters validationParameters, + CallContext callContext) + { + return "skipped"; // The audience that was validated. + }; + + internal static IssuerValidationDelegateAsync SkipIssuerValidation = delegate ( + string issuer, + SecurityToken securityToken, + ValidationParameters validationParameters, + CallContext callContext, + CancellationToken cancellationToken) + { + return Task.FromResult(new ValidationResult( + new ValidatedIssuer(issuer, IssuerValidationSource.NotValidated))); + }; + + internal static IssuerSigningKeyValidationDelegate SkipIssuerSigningKeyValidation = delegate ( + SecurityKey signingKey, + SecurityToken securityToken, + ValidationParameters validationParameters, + BaseConfiguration? configuration, + CallContext? callContext) + { + return new ValidatedSigningKeyLifetime( + null, // ValidFrom + null, // ValidTo + null);// ValidationTime + }; + + internal static LifetimeValidationDelegate SkipLifetimeValidation = delegate ( + DateTime? notBefore, + DateTime? expires, + SecurityToken? securityToken, + ValidationParameters validationParameters, + CallContext callContext) + { + return new ValidatedLifetime(notBefore, expires); + }; + + internal static SignatureValidationDelegate SkipSignatureValidation = delegate ( + SecurityToken securityToken, + ValidationParameters validationParameters, + BaseConfiguration? configuration, + CallContext? callContext) + { + // This key is not used during the validation process. It is only used to satisfy the delegate signature. + // Follow up PR will change this to remove the SecurityKey return value. + return new(result: new JsonWebKey()); + }; + + internal static TokenReplayValidationDelegate SkipTokenReplayValidation = delegate ( + DateTime? expirationTime, + string securityToken, + ValidationParameters validationParameters, + CallContext callContext) + { + return expirationTime; + }; + + internal static TokenTypeValidationDelegate SkipTokenTypeValidation = delegate ( + string? type, + SecurityToken? securityToken, + ValidationParameters validationParameters, + CallContext callContext) + { + return new ValidatedTokenType("skipped", 0); + }; + } +} +#nullable restore From f5f987b5c3861dfc05dddc23d38ebf81da446e0b Mon Sep 17 00:00:00 2001 From: Franco Fung Date: Thu, 3 Oct 2024 16:39:52 -0700 Subject: [PATCH 02/10] Added more test cases and removed duplicate skipDelegates.cs --- .../InternalsVisibleTo.cs | 1 + ...rTests.ValidateTokenAsyncTests.Audience.cs | 93 ++++++++++++++--- ...lerTests.ValidateTokenAsyncTests.Common.cs | 2 +- .../SkipValidationDelegates.cs | 99 ------------------- 4 files changed, 79 insertions(+), 116 deletions(-) delete mode 100644 test/Microsoft.IdentityModel.Tokens.Saml.Tests/SkipValidationDelegates.cs diff --git a/test/Microsoft.IdentityModel.TestUtils/InternalsVisibleTo.cs b/test/Microsoft.IdentityModel.TestUtils/InternalsVisibleTo.cs index 84597b7355..0cb96bb49b 100644 --- a/test/Microsoft.IdentityModel.TestUtils/InternalsVisibleTo.cs +++ b/test/Microsoft.IdentityModel.TestUtils/InternalsVisibleTo.cs @@ -3,3 +3,4 @@ [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.IdentityModel.JsonWebTokens.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")] [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.IdentityModel.Tokens.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")] +[assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.IdentityModel.Tokens.Saml.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")] diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs index 0fa69d4023..5da067ca33 100644 --- a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs +++ b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs @@ -21,12 +21,12 @@ public async Task ValidateTokenAsync_Audience(ValidateTokenAsyncAudienceTheoryDa Saml2SecurityTokenHandler saml2TokenHandler = new Saml2SecurityTokenHandler(); - var saml2Token = CreateToken(theoryData.Audience!, theoryData.Saml2Condition!); + var saml2Token = CreateToken(theoryData.TokenAudience!, theoryData.Saml2Condition!); var validationParameters = CreateTokenValidationParameters( - new List { theoryData.Audience! }, + theoryData.TVPAudiences, saml2Token, - false); //TODO: continue looking into improving this approach + theoryData.ignoreTrailingSlashWhenValidatingAudience); await ValidateAndCompareResults(saml2Token, validationParameters, theoryData, context); @@ -41,22 +41,82 @@ public static TheoryData ValidateTokenAsyn { new ValidateTokenAsyncAudienceTheoryData("Valid_AudiencesMatch") { - Audience = Default.Audience, - Saml2Condition = new Saml2Conditions - { - OneTimeUse = false, - NotOnOrAfter = DateTime.UtcNow.AddMinutes(5), - }, + TokenAudience = Default.Audience, + TVPAudiences = [Default.Audience], ValidationParameters = CreateValidationParameters([Default.Audience]) + }, + new ValidateTokenAsyncAudienceTheoryData("Invalid_AudiencesDoNotMatch") + { + // This scenario is the same if the token audience is an empty string or whitespace. + // As long as the token audience and the valid audience are not equal, the validation fails. + ValidationParameters = CreateValidationParameters([Default.Audience]), + TokenAudience = "InvalidAudience", + TVPAudiences = [Default.Audience], + ExpectedIsValid = false, + ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), + // ValidateTokenAsync with ValidationParameters returns a different error message to account for the + // removal of the ValidAudience property from the ValidationParameters class. + ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), + }, + new ValidateTokenAsyncAudienceTheoryData("Valid_AudienceWithinValidAudiences") + { + TokenAudience = Default.Audience, + TVPAudiences = ["ExtraAudience", Default.Audience, "AnotherAudience"], + ValidationParameters = CreateValidationParameters(["ExtraAudience", Default.Audience, "AnotherAudience"]), + }, + new ValidateTokenAsyncAudienceTheoryData("Valid_AudienceWithSlash_IgnoreTrailingSlashTrue") + { + // Audience has a trailing slash, but IgnoreTrailingSlashWhenValidatingAudience is true. + TokenAudience = Default.Audience + "/", + TVPAudiences = [Default.Audience], + ignoreTrailingSlashWhenValidatingAudience = true, + ValidationParameters = CreateValidationParameters([Default.Audience], true), + }, + new ValidateTokenAsyncAudienceTheoryData("Invalid_AudienceWithSlash_IgnoreTrailingSlashFalse") + { + // Audience has a trailing slash and IgnoreTrailingSlashWhenValidatingAudience is false. + TokenAudience = Default.Audience + "/", + TVPAudiences = [Default.Audience], + ValidationParameters = CreateValidationParameters([Default.Audience], false), + ExpectedIsValid = false, + ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), + ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), + }, + new ValidateTokenAsyncAudienceTheoryData("Valid_ValidAudiencesWithSlash_IgnoreTrailingSlashTrue") + { + // ValidAudiences has a trailing slash, but IgnoreTrailingSlashWhenValidatingAudience is true. + TokenAudience = Default.Audience, + ignoreTrailingSlashWhenValidatingAudience = true, + TVPAudiences = [Default.Audience + "/"], + ValidationParameters = CreateValidationParameters([Default.Audience + "/"], true), + }, + new ValidateTokenAsyncAudienceTheoryData("Invalid_ValidAudiencesWithSlash_IgnoreTrailingSlashFalse") + { + // ValidAudiences has a trailing slash and IgnoreTrailingSlashWhenValidatingAudience is false. + TokenAudience = Default.Audience, + TVPAudiences = [Default.Audience + "/"], + ValidationParameters = CreateValidationParameters([Default.Audience + "/"], false), + ExpectedIsValid = false, + ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), + ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), + }, + new ValidateTokenAsyncAudienceTheoryData("Invalid_AudienceNullIsTreatedAsEmptyList") + { + // JsonWebToken.Audiences defaults to an empty list if no audiences are provided. + TVPAudiences = [Default.Audience], + ValidationParameters = CreateValidationParameters([Default.Audience]), + TokenAudience = null, + ExpectedIsValid = false, + ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10206:"), } }; static ValidationParameters CreateValidationParameters( - List audiences, + List? audiences, bool ignoreTrailingSlashWhenValidatingAudience = false) { ValidationParameters validationParameters = new ValidationParameters(); - audiences.ForEach(audience => validationParameters.ValidAudiences.Add(audience)); + audiences?.ForEach(audience => validationParameters.ValidAudiences.Add(audience)); validationParameters.IgnoreTrailingSlashWhenValidatingAudience = ignoreTrailingSlashWhenValidatingAudience; validationParameters.LifetimeValidator = SkipValidationDelegates.SkipLifetimeValidation; validationParameters.TokenReplayValidator = SkipValidationDelegates.SkipTokenReplayValidation; @@ -70,13 +130,13 @@ public class ValidateTokenAsyncAudienceTheoryData : ValidateTokenAsyncBaseTheory { public ValidateTokenAsyncAudienceTheoryData(string testId) : base(testId) { } - public string? Audience { get; internal set; } = Default.Audience; + public string? TokenAudience { get; internal set; } = Default.Audience; - public Saml2Conditions? Saml2Condition { get; internal set; } + public List? TVPAudiences { get; internal set; } - public Saml2SecurityToken? Saml2SecurityToken { get; internal set; } + public Saml2Conditions? Saml2Condition { get; internal set; } - public bool ignoreTrailingSlashWhenValidatingAudience { get; internal set; } + public bool ignoreTrailingSlashWhenValidatingAudience { get; internal set; } = false; } private static Saml2SecurityToken CreateToken(string audience, Saml2Conditions saml2Conditions) @@ -93,9 +153,10 @@ private static Saml2SecurityToken CreateToken(string audience, Saml2Conditions s }; Saml2SecurityToken saml2Token = (Saml2SecurityToken)saml2TokenHandler.CreateToken(securityTokenDescriptor); + /* if (saml2Conditions != null) - saml2Token.Assertion.Conditions = saml2Conditions;*/ //TODO: Figure out how to adapt thisto more complex scenarios + saml2Token.Assertion.Conditions = saml2Conditions;*/ //TODO: Will adapt this to work with scenarios such as TokenReplay in Jwt or OneTimeUse in SAML. return saml2Token; } diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Common.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Common.cs index 0f0bda551c..42c6965c40 100644 --- a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Common.cs +++ b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Common.cs @@ -103,7 +103,7 @@ public ValidateTokenAsyncBaseTheoryData(string testId) : base(testId) { } internal bool ExpectedIsValid { get; set; } = true; - //internal TokenValidationParameters? TokenValidationParameters { get; set; } + internal TokenValidationParameters? TokenValidationParameters { get; set; } internal ValidationParameters? ValidationParameters { get; set; } diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/SkipValidationDelegates.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/SkipValidationDelegates.cs deleted file mode 100644 index bb2c52a705..0000000000 --- a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/SkipValidationDelegates.cs +++ /dev/null @@ -1,99 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -using System; -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; - -//TODO: Remove this file and use the one in TestUtils once new validation model is public. -#nullable enable -namespace Microsoft.IdentityModel.Tokens.Saml.Tests -{ - public static class SkipValidationDelegates - { - internal static AlgorithmValidationDelegate SkipAlgorithmValidation = delegate ( - string algorithm, - SecurityKey securityKey, - SecurityToken securityToken, - ValidationParameters - validationParameters, - CallContext callContext) - { - return algorithm; - }; - - internal static AudienceValidationDelegate SkipAudienceValidation = delegate ( - IList audiences, - SecurityToken? securityToken, - ValidationParameters validationParameters, - CallContext callContext) - { - return "skipped"; // The audience that was validated. - }; - - internal static IssuerValidationDelegateAsync SkipIssuerValidation = delegate ( - string issuer, - SecurityToken securityToken, - ValidationParameters validationParameters, - CallContext callContext, - CancellationToken cancellationToken) - { - return Task.FromResult(new ValidationResult( - new ValidatedIssuer(issuer, IssuerValidationSource.NotValidated))); - }; - - internal static IssuerSigningKeyValidationDelegate SkipIssuerSigningKeyValidation = delegate ( - SecurityKey signingKey, - SecurityToken securityToken, - ValidationParameters validationParameters, - BaseConfiguration? configuration, - CallContext? callContext) - { - return new ValidatedSigningKeyLifetime( - null, // ValidFrom - null, // ValidTo - null);// ValidationTime - }; - - internal static LifetimeValidationDelegate SkipLifetimeValidation = delegate ( - DateTime? notBefore, - DateTime? expires, - SecurityToken? securityToken, - ValidationParameters validationParameters, - CallContext callContext) - { - return new ValidatedLifetime(notBefore, expires); - }; - - internal static SignatureValidationDelegate SkipSignatureValidation = delegate ( - SecurityToken securityToken, - ValidationParameters validationParameters, - BaseConfiguration? configuration, - CallContext? callContext) - { - // This key is not used during the validation process. It is only used to satisfy the delegate signature. - // Follow up PR will change this to remove the SecurityKey return value. - return new(result: new JsonWebKey()); - }; - - internal static TokenReplayValidationDelegate SkipTokenReplayValidation = delegate ( - DateTime? expirationTime, - string securityToken, - ValidationParameters validationParameters, - CallContext callContext) - { - return expirationTime; - }; - - internal static TokenTypeValidationDelegate SkipTokenTypeValidation = delegate ( - string? type, - SecurityToken? securityToken, - ValidationParameters validationParameters, - CallContext callContext) - { - return new ValidatedTokenType("skipped", 0); - }; - } -} -#nullable restore From 89f7268dc63cd1a350fa7a790d13b1514cd7520d Mon Sep 17 00:00:00 2001 From: Franco Fung Date: Fri, 4 Oct 2024 10:11:18 -0700 Subject: [PATCH 03/10] removed failing non-applicable test --- ...TokenHandlerTests.ValidateTokenAsyncTests.Audience.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs index 5da067ca33..606beda444 100644 --- a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs +++ b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs @@ -99,15 +99,6 @@ public static TheoryData ValidateTokenAsyn ExpectedIsValid = false, ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), - }, - new ValidateTokenAsyncAudienceTheoryData("Invalid_AudienceNullIsTreatedAsEmptyList") - { - // JsonWebToken.Audiences defaults to an empty list if no audiences are provided. - TVPAudiences = [Default.Audience], - ValidationParameters = CreateValidationParameters([Default.Audience]), - TokenAudience = null, - ExpectedIsValid = false, - ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10206:"), } }; From ce4d68898503a3b1ea16149eed2a0a6b98b3d01d Mon Sep 17 00:00:00 2001 From: Franco Fung Date: Tue, 8 Oct 2024 15:40:26 -0700 Subject: [PATCH 04/10] Address PR feedback part 1 --- ...rityTokenHandler.ValidateToken.Internal.cs | 34 +++---------------- 1 file changed, 4 insertions(+), 30 deletions(-) diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs index 783bf10e0f..b8f89c62f3 100644 --- a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs +++ b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs @@ -15,7 +15,6 @@ namespace Microsoft.IdentityModel.Tokens.Saml2 /// public partial class Saml2SecurityTokenHandler : SecurityTokenHandler { - #pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously internal async Task> ValidateTokenAsync( #pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously @@ -41,38 +40,22 @@ internal async Task> ValidateTokenAsync( return new ValidatedToken(samlToken, this, validationParameters); } - - // 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 ValidateConditions(Saml2SecurityToken samlToken, ValidationParameters validationParameters, CallContext callContext) { if (samlToken == null) - return ValidationError.NullParameter(nameof(samlToken), new System.Diagnostics.StackFrame(true)); + return ValidationError.NullParameter(nameof(samlToken), new StackFrame(true)); if (validationParameters == null) - return ValidationError.NullParameter(nameof(validationParameters), new System.Diagnostics.StackFrame(true)); + return ValidationError.NullParameter(nameof(validationParameters), new 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; + return ValidationError.NullParameter(nameof(samlToken.Assertion), new StackFrame(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. - } + return ValidationError.NullParameter(nameof(samlToken.Assertion.Conditions), new StackFrame(true)); var lifetimeValidationResult = validationParameters.LifetimeValidator( samlToken.Assertion.Conditions.NotBefore, samlToken.Assertion.Conditions.NotOnOrAfter, samlToken, validationParameters, callContext); @@ -114,15 +97,6 @@ internal virtual ValidationResult ValidateConditions(Saml2S 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. } From 19f2bc3d665aee2db9d660f58968b4127de27499 Mon Sep 17 00:00:00 2001 From: Franco Fung Date: Tue, 8 Oct 2024 16:26:29 -0700 Subject: [PATCH 05/10] Added new ValidateTokeAsync method to InternalAPI.Unshipped.txt --- .../InternalAPI.Unshipped.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/InternalAPI.Unshipped.txt b/src/Microsoft.IdentityModel.Tokens.Saml/InternalAPI.Unshipped.txt index ff31950f14..e70a45343e 100644 --- a/src/Microsoft.IdentityModel.Tokens.Saml/InternalAPI.Unshipped.txt +++ b/src/Microsoft.IdentityModel.Tokens.Saml/InternalAPI.Unshipped.txt @@ -260,3 +260,4 @@ static readonly Microsoft.IdentityModel.Tokens.Saml2.Saml2AttributeKeyComparer.I static readonly Microsoft.IdentityModel.Tokens.Saml2.Saml2AuthorizationDecisionStatement.EmptyResource -> System.Uri virtual Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.ValidateConditions(Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityToken samlToken, Microsoft.IdentityModel.Tokens.ValidationParameters validationParameters, Microsoft.IdentityModel.Tokens.CallContext callContext) -> Microsoft.IdentityModel.Tokens.ValidationResult virtual Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.ValidateProxyRestriction(Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityToken samlToken, Microsoft.IdentityModel.Tokens.ValidationParameters validationParameters, Microsoft.IdentityModel.Tokens.CallContext callContext) -> Microsoft.IdentityModel.Tokens.ValidationError +Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.ValidateTokenAsync(Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityToken samlToken, Microsoft.IdentityModel.Tokens.ValidationParameters validationParameters, Microsoft.IdentityModel.Tokens.CallContext callContext, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task> From 67551dedd1dbb2c2caf9530a3c9a3da8cbc4d5b9 Mon Sep 17 00:00:00 2001 From: Franco Fung Date: Tue, 8 Oct 2024 16:49:05 -0700 Subject: [PATCH 06/10] Clean up PR feedback --- ...Saml2SecurityTokenHandler.ValidateToken.Internal.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs index b8f89c62f3..135f50cb8d 100644 --- a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs +++ b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Diagnostics; -using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -32,11 +31,6 @@ internal async Task> ValidateTokenAsync( return conditionsResult.UnwrapError().AddStackFrame(new StackFrame(true)); } - //These TODO's follow the pattern of the current ValidateToken methods. They should be implemented in the future. - //TODO: ValidateSubject() - Skip for now - //TODO: ValidateIssuer() - //TODO: ValidateIssuerSecurityKey()...etc - return new ValidatedToken(samlToken, this, validationParameters); } @@ -85,8 +79,8 @@ internal virtual ValidationResult ValidateConditions(Saml2S { // AudienceRestriction.Audiences is a List but returned as ICollection // no conversion occurs, ToList() is never called but we have to account for the possibility. - if (!(audienceRestriction.Audiences is List audiencesAsList)) - audiencesAsList = audienceRestriction.Audiences.ToList(); + if (audienceRestriction.Audiences is not List audiencesAsList) + audiencesAsList = [.. audienceRestriction.Audiences]; var audienceValidationResult = validationParameters.AudienceValidator( audiencesAsList, samlToken, validationParameters, callContext); From 632ba63aa63832c9ee795a827546465a385d4f73 Mon Sep 17 00:00:00 2001 From: Franco Fung Date: Tue, 8 Oct 2024 21:25:34 -0700 Subject: [PATCH 07/10] Simplify test into comparisson only tests. --- ...rTests.ValidateTokenAsyncTests.Audience.cs | 67 +++++++--- ...lerTests.ValidateTokenAsyncTests.Common.cs | 115 ------------------ 2 files changed, 51 insertions(+), 131 deletions(-) delete mode 100644 test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Common.cs diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs index 606beda444..9e8132d219 100644 --- a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs +++ b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Threading; using System.Threading.Tasks; using Microsoft.IdentityModel.TestUtils; using Microsoft.IdentityModel.Tokens.Saml2; @@ -15,22 +16,54 @@ public partial class Saml2SecurityTokenHandlerTests { [Theory, MemberData(nameof(ValidateTokenAsync_Audience_TestCases), DisableDiscoveryEnumeration = true)] - public async Task ValidateTokenAsync_Audience(ValidateTokenAsyncAudienceTheoryData theoryData) + public async Task ValidateTokenAsync_AudienceComparison(ValidateTokenAsyncAudienceTheoryData theoryData) { - var context = TestUtilities.WriteHeader($"{this}.ValidateTokenAsync_Audience", theoryData); + var context = TestUtilities.WriteHeader($"{this}.ValidateTokenAsync_AudienceComparison", theoryData); Saml2SecurityTokenHandler saml2TokenHandler = new Saml2SecurityTokenHandler(); var saml2Token = CreateToken(theoryData.TokenAudience!, theoryData.Saml2Condition!); - var validationParameters = CreateTokenValidationParameters( + var tokenValidationParameters = CreateTokenValidationParameters( theoryData.TVPAudiences, saml2Token, - theoryData.ignoreTrailingSlashWhenValidatingAudience); + theoryData.IgnoreTrailingSlashWhenValidatingAudience); - await ValidateAndCompareResults(saml2Token, validationParameters, theoryData, context); + // Validate the token using TokenValidationParameters + TokenValidationResult tokenValidationResult = + await saml2TokenHandler.ValidateTokenAsync(saml2Token.Assertion.CanonicalString, tokenValidationParameters); - TestUtilities.AssertFailIfErrors(context); + // Validate the token using ValidationParameters. + ValidationResult validationResult = + await saml2TokenHandler.ValidateTokenAsync( + saml2Token, theoryData.ValidationParameters!, theoryData.CallContext, CancellationToken.None); + + // Ensure the validity of the results match the expected result. + if (tokenValidationResult.IsValid != validationResult.IsSuccess) + { + context.AddDiff($"tokenValidationResult.IsValid != validationResult.IsSuccess"); + theoryData.ExpectedExceptionValidationParameters!.ProcessException(validationResult.UnwrapError().GetException(), context); + theoryData.ExpectedException.ProcessException(tokenValidationResult.Exception, context); + } + else + { + if (tokenValidationResult.IsValid) + { + //Verify the validated tokens by both paths match match. + ValidatedToken validatedToken = validationResult.UnwrapResult(); + IdentityComparer.AreEqual(validatedToken.SecurityToken, tokenValidationResult.SecurityToken, context); + } + else + { + // Verify the exception provided by both paths match. + var tokenValidationResultException = tokenValidationResult.Exception; + theoryData.ExpectedException.ProcessException(tokenValidationResult.Exception, context); + var validationResultException = validationResult.UnwrapError().GetException(); + theoryData.ExpectedExceptionValidationParameters!.ProcessException(validationResult.UnwrapError().GetException(), context); + } + + TestUtilities.AssertFailIfErrors(context); + } } public static TheoryData ValidateTokenAsync_Audience_TestCases @@ -69,7 +102,7 @@ public static TheoryData ValidateTokenAsyn // Audience has a trailing slash, but IgnoreTrailingSlashWhenValidatingAudience is true. TokenAudience = Default.Audience + "/", TVPAudiences = [Default.Audience], - ignoreTrailingSlashWhenValidatingAudience = true, + IgnoreTrailingSlashWhenValidatingAudience = true, ValidationParameters = CreateValidationParameters([Default.Audience], true), }, new ValidateTokenAsyncAudienceTheoryData("Invalid_AudienceWithSlash_IgnoreTrailingSlashFalse") @@ -86,7 +119,7 @@ public static TheoryData ValidateTokenAsyn { // ValidAudiences has a trailing slash, but IgnoreTrailingSlashWhenValidatingAudience is true. TokenAudience = Default.Audience, - ignoreTrailingSlashWhenValidatingAudience = true, + IgnoreTrailingSlashWhenValidatingAudience = true, TVPAudiences = [Default.Audience + "/"], ValidationParameters = CreateValidationParameters([Default.Audience + "/"], true), }, @@ -117,17 +150,23 @@ static ValidationParameters CreateValidationParameters( } } - public class ValidateTokenAsyncAudienceTheoryData : ValidateTokenAsyncBaseTheoryData + public class ValidateTokenAsyncAudienceTheoryData : TheoryDataBase { public ValidateTokenAsyncAudienceTheoryData(string testId) : base(testId) { } - public string? TokenAudience { get; internal set; } = Default.Audience; + internal ExpectedException? ExpectedExceptionValidationParameters { get; set; } = ExpectedException.NoExceptionExpected; - public List? TVPAudiences { get; internal set; } + internal bool ExpectedIsValid { get; set; } = true; + + public bool IgnoreTrailingSlashWhenValidatingAudience { get; internal set; } = false; + + internal ValidationParameters? ValidationParameters { get; set; } public Saml2Conditions? Saml2Condition { get; internal set; } - public bool ignoreTrailingSlashWhenValidatingAudience { get; internal set; } = false; + public string? TokenAudience { get; internal set; } = Default.Audience; + + public List? TVPAudiences { get; internal set; } } private static Saml2SecurityToken CreateToken(string audience, Saml2Conditions saml2Conditions) @@ -145,10 +184,6 @@ private static Saml2SecurityToken CreateToken(string audience, Saml2Conditions s Saml2SecurityToken saml2Token = (Saml2SecurityToken)saml2TokenHandler.CreateToken(securityTokenDescriptor); - /* - if (saml2Conditions != null) - saml2Token.Assertion.Conditions = saml2Conditions;*/ //TODO: Will adapt this to work with scenarios such as TokenReplay in Jwt or OneTimeUse in SAML. - return saml2Token; } diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Common.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Common.cs deleted file mode 100644 index 42c6965c40..0000000000 --- a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Common.cs +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -using System.Threading; -using System.Threading.Tasks; -using Microsoft.IdentityModel.TestUtils; -using Microsoft.IdentityModel.Tokens.Saml2; - -namespace Microsoft.IdentityModel.Tokens.Saml.Tests -{ -#nullable enable - public partial class Saml2SecurityTokenHandlerTests - { - internal static async Task ValidateAndCompareResults( - Saml2SecurityToken saml2Token, - TokenValidationParameters tokenValidationParameters, - ValidateTokenAsyncBaseTheoryData theoryData, - CompareContext context) - { - Saml2SecurityTokenHandler saml2TokenHandler = new Saml2SecurityTokenHandler(); - - // Validate the token using TokenValidationParameters - TokenValidationResult legacyTokenValidationParametersResult = - await saml2TokenHandler.ValidateTokenAsync(saml2Token.Assertion.CanonicalString, tokenValidationParameters); - - // Validate the token using ValidationParameters - ValidationResult validationParametersResult = - await saml2TokenHandler.ValidateTokenAsync( - saml2Token, - theoryData.ValidationParameters!, - theoryData.CallContext, - CancellationToken.None); - - // Ensure the validity of the results match the expected result - if (legacyTokenValidationParametersResult.IsValid != theoryData.ExpectedIsValid) - context.AddDiff($"tokenValidationParametersResult.IsValid != theoryData.ExpectedIsValid"); - - if (validationParametersResult.IsSuccess != theoryData.ExpectedIsValid) - context.AddDiff($"validationParametersResult.IsSuccess != theoryData.ExpectedIsValid"); - - if (theoryData.ExpectedIsValid && - legacyTokenValidationParametersResult.IsValid && - validationParametersResult.IsSuccess) - { - // This should compare the ClaimsPrincipal and ClaimsIdentity from one result against the other but right now we have not defined how we will handle this - /*IdentityComparer.AreEqual( - legacyTokenValidationParametersResult.ClaimsIdentity, - validationParametersResult.UnwrapResult().ClaimsIdentity, - context); - IdentityComparer.AreEqual( - legacyTokenValidationParametersResult.Claims, - validationParametersResult.UnwrapResult().Claims, - context);*/ - } - else - { - // Verify the exception provided by the TokenValidationParameters path - theoryData.ExpectedException.ProcessException(legacyTokenValidationParametersResult.Exception, context); - - if (!validationParametersResult.IsSuccess) - { - // Verify the exception provided by the ValidationParameters path - if (theoryData.ExpectedExceptionValidationParameters is not null) - { - // If there is a special case for the ValidationParameters path, use that. - theoryData.ExpectedExceptionValidationParameters - .ProcessException(validationParametersResult.UnwrapError().GetException(), context); - } - else - { - theoryData.ExpectedException - .ProcessException(validationParametersResult.UnwrapError().GetException(), context); - - // If the expected exception is the same in both paths, verify the message matches - IdentityComparer.AreStringsEqual( - legacyTokenValidationParametersResult.Exception.Message, - validationParametersResult.UnwrapError().GetException().Message, - context); - } - } - - // Verify that the exceptions are of the same type. - IdentityComparer.AreEqual( - legacyTokenValidationParametersResult.Exception.GetType(), - validationParametersResult.UnwrapError().GetException().GetType(), - context); - - if (legacyTokenValidationParametersResult.Exception is SecurityTokenException) - { - // Verify that the custom properties are the same. - IdentityComparer.AreSecurityTokenExceptionsEqual( - legacyTokenValidationParametersResult.Exception, - validationParametersResult.UnwrapError().GetException(), - context); - } - } - } - } - - public class ValidateTokenAsyncBaseTheoryData : TheoryDataBase - { - public ValidateTokenAsyncBaseTheoryData(string testId) : base(testId) { } - - internal bool ExpectedIsValid { get; set; } = true; - - internal TokenValidationParameters? TokenValidationParameters { get; set; } - - internal ValidationParameters? ValidationParameters { get; set; } - - // only set if we expect a different message on this path - internal ExpectedException? ExpectedExceptionValidationParameters { get; set; } = null; - } - -} -#nullable restore From e04c94c20446ca264e82bcdc4df7dcdcb3c69c54 Mon Sep 17 00:00:00 2001 From: Franco Fung <38921563+FuPingFranco@users.noreply.github.com> Date: Wed, 9 Oct 2024 14:22:35 -0700 Subject: [PATCH 08/10] Update test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs Co-authored-by: Westin Musser <127992899+westin-m@users.noreply.github.com> --- ...ecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs index 9e8132d219..aa51727c2f 100644 --- a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs +++ b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs @@ -49,7 +49,7 @@ await saml2TokenHandler.ValidateTokenAsync( { if (tokenValidationResult.IsValid) { - //Verify the validated tokens by both paths match match. + // Verify that the validated tokens from both paths match. ValidatedToken validatedToken = validationResult.UnwrapResult(); IdentityComparer.AreEqual(validatedToken.SecurityToken, tokenValidationResult.SecurityToken, context); } From 3dfc8ddc56bfd220f62a2b6a8dedd00d2cd3b4ad Mon Sep 17 00:00:00 2001 From: Franco Fung Date: Fri, 11 Oct 2024 12:57:21 -0700 Subject: [PATCH 09/10] Addressing PR feedback --- .../InternalAPI.Unshipped.txt | 8 ++ ...rityTokenHandler.ValidateToken.Internal.cs | 78 +++++++++++++++---- ...yTokenHandler.ValidateToken.StackFrames.cs | 27 +++++++ ...rTests.ValidateTokenAsyncTests.Audience.cs | 50 +++++++++--- 4 files changed, 135 insertions(+), 28 deletions(-) create mode 100644 src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.StackFrames.cs diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/InternalAPI.Unshipped.txt b/src/Microsoft.IdentityModel.Tokens.Saml/InternalAPI.Unshipped.txt index e70a45343e..247f47da39 100644 --- a/src/Microsoft.IdentityModel.Tokens.Saml/InternalAPI.Unshipped.txt +++ b/src/Microsoft.IdentityModel.Tokens.Saml/InternalAPI.Unshipped.txt @@ -216,6 +216,7 @@ Microsoft.IdentityModel.Tokens.Saml2.Saml2AttributeKeyComparer.AttributeKey.Orig Microsoft.IdentityModel.Tokens.Saml2.Saml2AttributeKeyComparer.AttributeKey.ValueType.get -> string Microsoft.IdentityModel.Tokens.Saml2.Saml2AttributeKeyComparer.Equals(Microsoft.IdentityModel.Tokens.Saml2.Saml2AttributeKeyComparer.AttributeKey x, Microsoft.IdentityModel.Tokens.Saml2.Saml2AttributeKeyComparer.AttributeKey y) -> bool Microsoft.IdentityModel.Tokens.Saml2.Saml2AttributeKeyComparer.GetHashCode(Microsoft.IdentityModel.Tokens.Saml2.Saml2AttributeKeyComparer.AttributeKey obj) -> int +Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.StackFrames Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.ValidatedConditions Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.ValidatedConditions.ValidatedAudience.get -> string Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.ValidatedConditions.ValidatedAudience.set -> void @@ -243,6 +244,13 @@ static Microsoft.IdentityModel.Tokens.Saml.SamlTokenUtilities.GetXsiTypeForValue static Microsoft.IdentityModel.Tokens.Saml.SamlTokenUtilities.PopulateValidationParametersWithCurrentConfigurationAsync(Microsoft.IdentityModel.Tokens.TokenValidationParameters validationParameters) -> System.Threading.Tasks.Task static Microsoft.IdentityModel.Tokens.Saml.SamlTokenUtilities.ResolveTokenSigningKey(Microsoft.IdentityModel.Xml.KeyInfo tokenKeyInfo, Microsoft.IdentityModel.Tokens.TokenValidationParameters validationParameters) -> Microsoft.IdentityModel.Tokens.SecurityKey static Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.IsSaml2Assertion(System.Xml.XmlReader reader) -> bool +static Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.StackFrames.AssertionConditionsNull -> System.Diagnostics.StackFrame +static Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.StackFrames.AssertionNull -> System.Diagnostics.StackFrame +static Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.StackFrames.AudienceValidationFailed -> System.Diagnostics.StackFrame +static Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.StackFrames.LifetimeValidationFailed -> System.Diagnostics.StackFrame +static Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.StackFrames.OneTimeUseValidationFailed -> System.Diagnostics.StackFrame +static Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.StackFrames.TokenNull -> System.Diagnostics.StackFrame +static Microsoft.IdentityModel.Tokens.Saml2.Saml2SecurityTokenHandler.StackFrames.TokenValidationParametersNull -> System.Diagnostics.StackFrame static Microsoft.IdentityModel.Tokens.Saml2.Saml2Serializer.CanCreateValidUri(string uriString, System.UriKind uriKind) -> bool static Microsoft.IdentityModel.Tokens.Saml2.Saml2Serializer.LogReadException(string format, params object[] args) -> System.Exception static Microsoft.IdentityModel.Tokens.Saml2.Saml2Serializer.LogReadException(string format, System.Exception inner, params object[] args) -> System.Exception diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs index 135f50cb8d..46aa9b26f6 100644 --- a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs +++ b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs @@ -24,6 +24,22 @@ internal async Task> ValidateTokenAsync( CancellationToken cancellationToken) #pragma warning restore CA1801 // Review unused parameters { + if (samlToken is null) + { + StackFrame stackFrame = StackFrames.TokenNull ??= new StackFrame(true); + return ValidationError.NullParameter( + nameof(samlToken), + new StackFrame(true)); + } + + if (validationParameters is null) + { + StackFrame stackFrame = StackFrames.TokenValidationParametersNull ??= new StackFrame(true); + return ValidationError.NullParameter( + nameof(validationParameters), + new StackFrame(true)); + } + var conditionsResult = ValidateConditions(samlToken, validationParameters, callContext); if (!conditionsResult.IsSuccess) @@ -39,39 +55,64 @@ internal record struct ValidatedConditions(string? ValidatedAudience, ValidatedL internal virtual ValidationResult ValidateConditions(Saml2SecurityToken samlToken, ValidationParameters validationParameters, CallContext callContext) { - if (samlToken == null) - return ValidationError.NullParameter(nameof(samlToken), new StackFrame(true)); - - if (validationParameters == null) - return ValidationError.NullParameter(nameof(validationParameters), new StackFrame(true)); - - if (samlToken.Assertion == null) - return ValidationError.NullParameter(nameof(samlToken.Assertion), new StackFrame(true)); + if (samlToken.Assertion is null) + { + StackFrame stackFrame = StackFrames.AssertionNull ??= new StackFrame(true); + return ValidationError.NullParameter( + nameof(samlToken.Assertion), + new StackFrame(true)); + } - if (samlToken.Assertion.Conditions == null) - return ValidationError.NullParameter(nameof(samlToken.Assertion.Conditions), new StackFrame(true)); + if (samlToken.Assertion.Conditions is null) + { + StackFrame stackFrame = StackFrames.AssertionConditionsNull ??= new StackFrame(true); + return ValidationError.NullParameter( + nameof(samlToken.Assertion.Conditions), + new StackFrame(true)); + } var lifetimeValidationResult = validationParameters.LifetimeValidator( - samlToken.Assertion.Conditions.NotBefore, samlToken.Assertion.Conditions.NotOnOrAfter, samlToken, validationParameters, callContext); + samlToken.Assertion.Conditions.NotBefore, + samlToken.Assertion.Conditions.NotOnOrAfter, + samlToken, + validationParameters, + callContext); + if (!lifetimeValidationResult.IsSuccess) - return lifetimeValidationResult.UnwrapError(); + { + StackFrame lifeTimeStackFrame = StackFrames.LifetimeValidationFailed ??= new StackFrame(true); + return lifetimeValidationResult.UnwrapError().AddStackFrame(lifeTimeStackFrame); + } 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); + samlToken.Assertion.Conditions.NotOnOrAfter, + samlToken.Assertion.CanonicalString, + validationParameters, + callContext); + if (!oneTimeUseValidationResult.IsSuccess) - return oneTimeUseValidationResult.UnwrapError(); + { + StackFrame tokenReplayStackFrame = StackFrames.OneTimeUseValidationFailed ??= new StackFrame(true); + return oneTimeUseValidationResult.UnwrapError().AddStackFrame(tokenReplayStackFrame); + } } if (samlToken.Assertion.Conditions.ProxyRestriction != null) { //throw LogExceptionMessage(new SecurityTokenValidationException(LogMessages.IDX13511)); - var proxyValidationError = ValidateProxyRestriction(samlToken, validationParameters, callContext); + var proxyValidationError = ValidateProxyRestriction( + samlToken, + validationParameters, + callContext); + if (proxyValidationError is not null) + { return proxyValidationError; + } } string? validatedAudience = null; @@ -83,7 +124,10 @@ internal virtual ValidationResult ValidateConditions(Saml2S audiencesAsList = [.. audienceRestriction.Audiences]; var audienceValidationResult = validationParameters.AudienceValidator( - audiencesAsList, samlToken, validationParameters, callContext); + audiencesAsList, + samlToken, + validationParameters, + callContext); if (!audienceValidationResult.IsSuccess) return audienceValidationResult.UnwrapError(); @@ -91,7 +135,7 @@ internal virtual ValidationResult ValidateConditions(Saml2S validatedAudience = audienceValidationResult.UnwrapResult(); } - return new ValidatedConditions(validatedAudience, lifetimeValidationResult.UnwrapResult()); // no error occurred. There is nothing else to return. + return new ValidatedConditions(validatedAudience, lifetimeValidationResult.UnwrapResult()); } #pragma warning disable CA1801 // Review unused parameters diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.StackFrames.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.StackFrames.cs new file mode 100644 index 0000000000..61466f8406 --- /dev/null +++ b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.StackFrames.cs @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.Diagnostics; + +#nullable enable +namespace Microsoft.IdentityModel.Tokens.Saml2 +{ + public partial class Saml2SecurityTokenHandler : SecurityTokenHandler + { + // Cached stack frames to build exceptions from validation errors + internal static class StackFrames + { + // Stack frames from ValidateTokenAsync using SecurityToken + internal static StackFrame? TokenNull; + internal static StackFrame? TokenValidationParametersNull; + + // Stack frames from ValidateConditions + internal static StackFrame? AudienceValidationFailed; + internal static StackFrame? AssertionNull; + internal static StackFrame? AssertionConditionsNull; + internal static StackFrame? LifetimeValidationFailed; + internal static StackFrame? OneTimeUseValidationFailed; + } + } +} +#nullable restore diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs index aa51727c2f..4f7df18af3 100644 --- a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs +++ b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs @@ -27,6 +27,7 @@ public async Task ValidateTokenAsync_AudienceComparison(ValidateTokenAsyncAudien var tokenValidationParameters = CreateTokenValidationParameters( theoryData.TVPAudiences, saml2Token, + theoryData.NullTokenValidationParameters, theoryData.IgnoreTrailingSlashWhenValidatingAudience); // Validate the token using TokenValidationParameters @@ -36,7 +37,10 @@ public async Task ValidateTokenAsync_AudienceComparison(ValidateTokenAsyncAudien // Validate the token using ValidationParameters. ValidationResult validationResult = await saml2TokenHandler.ValidateTokenAsync( - saml2Token, theoryData.ValidationParameters!, theoryData.CallContext, CancellationToken.None); + saml2Token, + theoryData.ValidationParameters!, + theoryData.CallContext, + CancellationToken.None); // Ensure the validity of the results match the expected result. if (tokenValidationResult.IsValid != validationResult.IsSuccess) @@ -72,23 +76,28 @@ public static TheoryData ValidateTokenAsyn { return new TheoryData { - new ValidateTokenAsyncAudienceTheoryData("Valid_AudiencesMatch") + new ValidateTokenAsyncAudienceTheoryData("Valid_AudiencesMatch") { TokenAudience = Default.Audience, TVPAudiences = [Default.Audience], ValidationParameters = CreateValidationParameters([Default.Audience]) }, - new ValidateTokenAsyncAudienceTheoryData("Invalid_AudiencesDoNotMatch") + new ValidateTokenAsyncAudienceTheoryData("Invalid_AudiencesDoNotMatch") { - // This scenario is the same if the token audience is an empty string or whitespace. - // As long as the token audience and the valid audience are not equal, the validation fails. ValidationParameters = CreateValidationParameters([Default.Audience]), TokenAudience = "InvalidAudience", TVPAudiences = [Default.Audience], ExpectedIsValid = false, ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), - // ValidateTokenAsync with ValidationParameters returns a different error message to account for the - // removal of the ValidAudience property from the ValidationParameters class. + ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), + }, + new ValidateTokenAsyncAudienceTheoryData("Invalid_TokenAudienceIsWhiteSpace") + { + ValidationParameters = CreateValidationParameters([Default.Audience]), + TokenAudience = " ", + TVPAudiences = [Default.Audience], + ExpectedIsValid = false, + ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), }, new ValidateTokenAsyncAudienceTheoryData("Valid_AudienceWithinValidAudiences") @@ -132,7 +141,17 @@ public static TheoryData ValidateTokenAsyn ExpectedIsValid = false, ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), - } + }, + new ValidateTokenAsyncAudienceTheoryData("Invalid_TokenValidationParametersAndValidationParametersAreNull") + { + ExpectedException = ExpectedException.ArgumentNullException("IDX10000:"), + ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenArgumentNullException("IDX10000:"), + ExpectedIsValid = false, + TokenAudience = Default.Audience, + TVPAudiences = [Default.Audience], + ValidationParameters = null, + NullTokenValidationParameters = true + }, }; static ValidationParameters CreateValidationParameters( @@ -160,11 +179,13 @@ public ValidateTokenAsyncAudienceTheoryData(string testId) : base(testId) { } public bool IgnoreTrailingSlashWhenValidatingAudience { get; internal set; } = false; + public bool NullTokenValidationParameters { get; internal set; } = false; + internal ValidationParameters? ValidationParameters { get; set; } public Saml2Conditions? Saml2Condition { get; internal set; } - public string? TokenAudience { get; internal set; } = Default.Audience; + public string? TokenAudience { get; internal set; } public List? TVPAudiences { get; internal set; } } @@ -187,11 +208,17 @@ private static Saml2SecurityToken CreateToken(string audience, Saml2Conditions s return saml2Token; } - private static TokenValidationParameters CreateTokenValidationParameters( + private static TokenValidationParameters? CreateTokenValidationParameters( List? audiences, Saml2SecurityToken saml2SecurityToken, + bool nullTokenValidationParameters, bool ignoreTrailingSlashWhenValidatingAudience = false) { + if (nullTokenValidationParameters) + { + return null; + } + return new TokenValidationParameters { ValidateAudience = true, @@ -205,7 +232,8 @@ private static TokenValidationParameters CreateTokenValidationParameters( SignatureValidator = delegate (string token, TokenValidationParameters validationParameters) { return saml2SecurityToken; - } + }, + RequireAudience = true }; } } From 73497d171708d814cd807d1c7fe8fef2db40b891 Mon Sep 17 00:00:00 2001 From: Franco Fung Date: Mon, 14 Oct 2024 10:11:49 -0700 Subject: [PATCH 10/10] Addressing PR feedback --- ...rityTokenHandler.ValidateToken.Internal.cs | 24 +-- ...rTests.ValidateTokenAsyncTests.Audience.cs | 166 ++++++++++-------- 2 files changed, 100 insertions(+), 90 deletions(-) diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs index 46aa9b26f6..7918058462 100644 --- a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs +++ b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateToken.Internal.cs @@ -26,18 +26,18 @@ internal async Task> ValidateTokenAsync( { if (samlToken is null) { - StackFrame stackFrame = StackFrames.TokenNull ??= new StackFrame(true); + StackFrames.TokenNull ??= new StackFrame(true); return ValidationError.NullParameter( nameof(samlToken), - new StackFrame(true)); + StackFrames.TokenNull); } if (validationParameters is null) { - StackFrame stackFrame = StackFrames.TokenValidationParametersNull ??= new StackFrame(true); + StackFrames.TokenValidationParametersNull ??= new StackFrame(true); return ValidationError.NullParameter( nameof(validationParameters), - new StackFrame(true)); + StackFrames.TokenValidationParametersNull); } var conditionsResult = ValidateConditions(samlToken, validationParameters, callContext); @@ -57,18 +57,18 @@ internal virtual ValidationResult ValidateConditions(Saml2S { if (samlToken.Assertion is null) { - StackFrame stackFrame = StackFrames.AssertionNull ??= new StackFrame(true); + StackFrames.AssertionNull ??= new StackFrame(true); return ValidationError.NullParameter( nameof(samlToken.Assertion), - new StackFrame(true)); + StackFrames.AssertionNull); } if (samlToken.Assertion.Conditions is null) { - StackFrame stackFrame = StackFrames.AssertionConditionsNull ??= new StackFrame(true); + StackFrames.AssertionConditionsNull ??= new StackFrame(true); return ValidationError.NullParameter( nameof(samlToken.Assertion.Conditions), - new StackFrame(true)); + StackFrames.AssertionConditionsNull); } var lifetimeValidationResult = validationParameters.LifetimeValidator( @@ -80,8 +80,8 @@ internal virtual ValidationResult ValidateConditions(Saml2S if (!lifetimeValidationResult.IsSuccess) { - StackFrame lifeTimeStackFrame = StackFrames.LifetimeValidationFailed ??= new StackFrame(true); - return lifetimeValidationResult.UnwrapError().AddStackFrame(lifeTimeStackFrame); + StackFrames.LifetimeValidationFailed ??= new StackFrame(true); + return lifetimeValidationResult.UnwrapError().AddStackFrame(StackFrames.LifetimeValidationFailed); } if (samlToken.Assertion.Conditions.OneTimeUse) @@ -96,8 +96,8 @@ internal virtual ValidationResult ValidateConditions(Saml2S if (!oneTimeUseValidationResult.IsSuccess) { - StackFrame tokenReplayStackFrame = StackFrames.OneTimeUseValidationFailed ??= new StackFrame(true); - return oneTimeUseValidationResult.UnwrapError().AddStackFrame(tokenReplayStackFrame); + StackFrames.OneTimeUseValidationFailed ??= new StackFrame(true); + return oneTimeUseValidationResult.UnwrapError().AddStackFrame(StackFrames.OneTimeUseValidationFailed); } } diff --git a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs index 4f7df18af3..a6c0905388 100644 --- a/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs +++ b/test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.ValidateTokenAsyncTests.Audience.cs @@ -74,85 +74,95 @@ public static TheoryData ValidateTokenAsyn { get { - return new TheoryData + + var theoryData = new TheoryData(); + + theoryData.Add(new ValidateTokenAsyncAudienceTheoryData("Valid_AudiencesMatch") + { + TokenAudience = Default.Audience, + TVPAudiences = [Default.Audience], + ValidationParameters = CreateValidationParameters([Default.Audience]) + }); + + theoryData.Add(new ValidateTokenAsyncAudienceTheoryData("Valid_AudienceWithinValidAudiences") + { + TokenAudience = Default.Audience, + TVPAudiences = ["ExtraAudience", Default.Audience, "AnotherAudience"], + ValidationParameters = CreateValidationParameters(["ExtraAudience", Default.Audience, "AnotherAudience"]), + }); + + theoryData.Add(new ValidateTokenAsyncAudienceTheoryData("Valid_AudienceWithSlash_IgnoreTrailingSlashTrue") + { + // Audience has a trailing slash, but IgnoreTrailingSlashWhenValidatingAudience is true. + TokenAudience = Default.Audience + "/", + TVPAudiences = [Default.Audience], + IgnoreTrailingSlashWhenValidatingAudience = true, + ValidationParameters = CreateValidationParameters([Default.Audience], true), + }); + + theoryData.Add(new ValidateTokenAsyncAudienceTheoryData("Valid_ValidAudiencesWithSlash_IgnoreTrailingSlashTrue") + { + // ValidAudiences has a trailing slash, but IgnoreTrailingSlashWhenValidatingAudience is true. + TokenAudience = Default.Audience, + IgnoreTrailingSlashWhenValidatingAudience = true, + TVPAudiences = [Default.Audience + "/"], + ValidationParameters = CreateValidationParameters([Default.Audience + "/"], true), + }); + + theoryData.Add(new ValidateTokenAsyncAudienceTheoryData("Invalid_AudiencesDoNotMatch") + { + ValidationParameters = CreateValidationParameters([Default.Audience]), + TokenAudience = "InvalidAudience", + TVPAudiences = [Default.Audience], + ExpectedIsValid = false, + ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), + ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), + }); + + theoryData.Add(new ValidateTokenAsyncAudienceTheoryData("Invalid_TokenAudienceIsWhiteSpace") + { + ValidationParameters = CreateValidationParameters([Default.Audience]), + TokenAudience = " ", + TVPAudiences = [Default.Audience], + ExpectedIsValid = false, + ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), + ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), + }); + + theoryData.Add(new ValidateTokenAsyncAudienceTheoryData("Invalid_AudienceWithSlash_IgnoreTrailingSlashFalse") + { + // Audience has a trailing slash and IgnoreTrailingSlashWhenValidatingAudience is false. + TokenAudience = Default.Audience + "/", + TVPAudiences = [Default.Audience], + ValidationParameters = CreateValidationParameters([Default.Audience], false), + ExpectedIsValid = false, + ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), + ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), + }); + + theoryData.Add(new ValidateTokenAsyncAudienceTheoryData("Invalid_ValidAudiencesWithSlash_IgnoreTrailingSlashFalse") + { + // ValidAudiences has a trailing slash and IgnoreTrailingSlashWhenValidatingAudience is false. + TokenAudience = Default.Audience, + TVPAudiences = [Default.Audience + "/"], + ValidationParameters = CreateValidationParameters([Default.Audience + "/"], false), + ExpectedIsValid = false, + ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), + ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), + }); + + theoryData.Add(new ValidateTokenAsyncAudienceTheoryData("Invalid_TokenValidationParametersAndValidationParametersAreNull") { - new ValidateTokenAsyncAudienceTheoryData("Valid_AudiencesMatch") - { - TokenAudience = Default.Audience, - TVPAudiences = [Default.Audience], - ValidationParameters = CreateValidationParameters([Default.Audience]) - }, - new ValidateTokenAsyncAudienceTheoryData("Invalid_AudiencesDoNotMatch") - { - ValidationParameters = CreateValidationParameters([Default.Audience]), - TokenAudience = "InvalidAudience", - TVPAudiences = [Default.Audience], - ExpectedIsValid = false, - ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), - ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), - }, - new ValidateTokenAsyncAudienceTheoryData("Invalid_TokenAudienceIsWhiteSpace") - { - ValidationParameters = CreateValidationParameters([Default.Audience]), - TokenAudience = " ", - TVPAudiences = [Default.Audience], - ExpectedIsValid = false, - ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), - ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), - }, - new ValidateTokenAsyncAudienceTheoryData("Valid_AudienceWithinValidAudiences") - { - TokenAudience = Default.Audience, - TVPAudiences = ["ExtraAudience", Default.Audience, "AnotherAudience"], - ValidationParameters = CreateValidationParameters(["ExtraAudience", Default.Audience, "AnotherAudience"]), - }, - new ValidateTokenAsyncAudienceTheoryData("Valid_AudienceWithSlash_IgnoreTrailingSlashTrue") - { - // Audience has a trailing slash, but IgnoreTrailingSlashWhenValidatingAudience is true. - TokenAudience = Default.Audience + "/", - TVPAudiences = [Default.Audience], - IgnoreTrailingSlashWhenValidatingAudience = true, - ValidationParameters = CreateValidationParameters([Default.Audience], true), - }, - new ValidateTokenAsyncAudienceTheoryData("Invalid_AudienceWithSlash_IgnoreTrailingSlashFalse") - { - // Audience has a trailing slash and IgnoreTrailingSlashWhenValidatingAudience is false. - TokenAudience = Default.Audience + "/", - TVPAudiences = [Default.Audience], - ValidationParameters = CreateValidationParameters([Default.Audience], false), - ExpectedIsValid = false, - ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), - ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), - }, - new ValidateTokenAsyncAudienceTheoryData("Valid_ValidAudiencesWithSlash_IgnoreTrailingSlashTrue") - { - // ValidAudiences has a trailing slash, but IgnoreTrailingSlashWhenValidatingAudience is true. - TokenAudience = Default.Audience, - IgnoreTrailingSlashWhenValidatingAudience = true, - TVPAudiences = [Default.Audience + "/"], - ValidationParameters = CreateValidationParameters([Default.Audience + "/"], true), - }, - new ValidateTokenAsyncAudienceTheoryData("Invalid_ValidAudiencesWithSlash_IgnoreTrailingSlashFalse") - { - // ValidAudiences has a trailing slash and IgnoreTrailingSlashWhenValidatingAudience is false. - TokenAudience = Default.Audience, - TVPAudiences = [Default.Audience + "/"], - ValidationParameters = CreateValidationParameters([Default.Audience + "/"], false), - ExpectedIsValid = false, - ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException("IDX10214:"), - ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenInvalidAudienceException("IDX10215:"), - }, - new ValidateTokenAsyncAudienceTheoryData("Invalid_TokenValidationParametersAndValidationParametersAreNull") - { - ExpectedException = ExpectedException.ArgumentNullException("IDX10000:"), - ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenArgumentNullException("IDX10000:"), - ExpectedIsValid = false, - TokenAudience = Default.Audience, - TVPAudiences = [Default.Audience], - ValidationParameters = null, - NullTokenValidationParameters = true - }, - }; + ExpectedException = ExpectedException.ArgumentNullException("IDX10000:"), + ExpectedExceptionValidationParameters = ExpectedException.SecurityTokenArgumentNullException("IDX10000:"), + ExpectedIsValid = false, + TokenAudience = Default.Audience, + TVPAudiences = [Default.Audience], + ValidationParameters = null, + NullTokenValidationParameters = true + }); + + return theoryData; static ValidationParameters CreateValidationParameters( List? audiences,