Skip to content

Remove Delegate Checks Audience Validator and Prevents Null Setting of Delegate #2758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
FuPingFranco merged 3 commits into from
Aug 6, 2024
Merged

Remove Delegate Checks Audience Validator and Prevents Null Setting of Delegate #2758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
08365d5
Re-factor audience validator to use ValidatationParameters.
Jul 31, 2024
d4e57b1
Addressing feedback
Jul 31, 2024
0cc32fc
Address PR feedback. (#2765)
FuPingFranco Aug 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Microsoft.IdentityModel.Tokens/LogMessages.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ internal static class LogMessages
public const string IDX10209 = "IDX10209: Token has length: '{0}' which is larger than the MaximumTokenSizeInBytes: '{1}'.";
public const string IDX10211 = "IDX10211: Unable to validate issuer. The 'issuer' parameter is null or whitespace.";
public const string IDX10214 = "IDX10214: Audience validation failed. Audiences: '{0}'. Did not match: validationParameters.ValidAudience: '{1}' or validationParameters.ValidAudiences: '{2}'.";
public const string IDX10215 = "IDX10215: Audience validation failed. Audiences: '{0}'. Did not match: validationParameters.ValidAudiences: '{1}'.";
public const string IDX10222 = "IDX10222: Lifetime validation failed. The token is not yet valid. ValidFrom (UTC): '{0}', Current time (UTC): '{1}'.";
public const string IDX10223 = "IDX10223: Lifetime validation failed. The token is expired. ValidTo (UTC): '{0}', Current time (UTC): '{1}'.";
public const string IDX10224 = "IDX10224: Lifetime validation failed. The NotBefore (UTC): '{0}' is after Expires (UTC): '{1}'.";
Expand Down
30 changes: 13 additions & 17 deletions src/Microsoft.IdentityModel.Tokens/Validation/ValidationParameters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ internal class ValidationParameters
private string _nameClaimType = ClaimsIdentity.DefaultNameClaimType;
private string _roleClaimType = ClaimsIdentity.DefaultRoleClaimType;
private Dictionary<string, object> _instancePropertyBag;
private IList<string> _validTokenTypes = [];
private IList<string> _validTokenTypes;
private IList<string> _validAudiences;

private AlgorithmValidatorDelegate _algorithmValidator = Validators.ValidateAlgorithm;
private AudienceValidatorDelegate _audienceValidator = Validators.ValidateAudience;
Expand Down Expand Up @@ -89,9 +90,9 @@ protected ValidationParameters(ValidationParameters other)
ValidateSignatureLast = other.ValidateSignatureLast;
ValidateWithLKG = other.ValidateWithLKG;
ValidAlgorithms = other.ValidAlgorithms;
ValidAudiences = other.ValidAudiences;
_validAudiences = other.ValidAudiences;
ValidIssuers = other.ValidIssuers;
ValidTypes = other.ValidTypes;
_validTokenTypes = other.ValidTypes;
}

/// <summary>
Expand Down Expand Up @@ -513,9 +514,12 @@ public TypeValidatorDelegate TypeValidator

/// <summary>
/// Gets the <see cref="IList{String}"/> that contains valid audiences that will be used to check against the token's audience.
/// The default is <c>null</c>.
/// The default is an empty collection.
/// </summary>
public IList<string> ValidAudiences { get; }
public IList<string> ValidAudiences =>
_validAudiences ??
Interlocked.CompareExchange(ref _validAudiences, [], null) ??
_validAudiences;

/// <summary>
/// Gets the <see cref="IList{String}"/> that contains valid issuers that will be used to check against the token's issuer.
Expand All @@ -529,19 +533,11 @@ public TypeValidatorDelegate TypeValidator
/// In the case of a JWE, this property will ONLY apply to the inner token header.
/// The default is an empty collection.
/// </summary>
/// <exception cref="ArgumentNullException">Thrown when the value is set as null.</exception>
/// <returns>The <see cref="IList{String}"/> that contains valid token types that will be used to check against the token's 'typ' claim.</returns>
public IList<string> ValidTypes
{
get
{
return _validTokenTypes;
}
set
{
_validTokenTypes = value ?? throw new ArgumentNullException(nameof(value));
}
}
public IList<string> ValidTypes =>
_validTokenTypes ??
Interlocked.CompareExchange(ref _validTokenTypes, [], null) ??
_validTokenTypes;

public bool ValidateActor { get; set; }
}
Expand Down
112 changes: 24 additions & 88 deletions src/Microsoft.IdentityModel.Tokens/Validation/Validators.Audience.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ namespace Microsoft.IdentityModel.Tokens
/// <returns>A <see cref="IssuerValidationResult"/>that contains the results of validating the issuer.</returns>
/// <remarks>This delegate is not expected to throw.</remarks>
internal delegate AudienceValidationResult AudienceValidatorDelegate(
IEnumerable<string> audiences,
IList<string> audiences,
SecurityToken? securityToken,
TokenValidationParameters validationParameters,
ValidationParameters validationParameters,
CallContext callContext);

/// <summary>
Expand All @@ -34,7 +34,7 @@ public static partial class Validators
/// <summary>
/// Determines if the audiences found in a <see cref="SecurityToken"/> are valid.
/// </summary>
/// <param name="audiences">The audiences found in the <see cref="SecurityToken"/>.</param>
/// <param name="tokenAudiences">The audiences found in the <see cref="SecurityToken"/>.</param>
/// <param name="securityToken">The <see cref="SecurityToken"/> being validated.</param>
/// <param name="validationParameters">The <see cref="TokenValidationParameters"/> to be used for validating the token.</param>
/// <param name="callContext"></param>
Expand All @@ -44,12 +44,12 @@ public static partial class Validators
/// <exception cref="SecurityTokenInvalidAudienceException">If none of the 'audiences' matched either <see cref="TokenValidationParameters.ValidAudience"/> or one of <see cref="TokenValidationParameters.ValidAudiences"/>.</exception>
/// <remarks>An EXACT match is required.</remarks>
#pragma warning disable CA1801 // TODO: remove pragma disable once callContext is used for logging
internal static AudienceValidationResult ValidateAudience(IEnumerable<string> audiences, SecurityToken? securityToken, TokenValidationParameters validationParameters, CallContext callContext)
internal static AudienceValidationResult ValidateAudience(IList<string> tokenAudiences, SecurityToken? securityToken, ValidationParameters validationParameters, CallContext callContext)
#pragma warning restore CA1801
{
if (validationParameters == null)
return new AudienceValidationResult(
Utility.SerializeAsSingleCommaDelimitedString(audiences),
Utility.SerializeAsSingleCommaDelimitedString(tokenAudiences),
ValidationFailureType.NullArgument,
new ExceptionDetail(
new MessageDetail(
Expand All @@ -58,15 +58,9 @@ internal static AudienceValidationResult ValidateAudience(IEnumerable<string> au
typeof(ArgumentNullException),
new StackFrame(true)));

if (!validationParameters.ValidateAudience)
{
LogHelper.LogWarning(LogMessages.IDX10233);
return new AudienceValidationResult(Utility.SerializeAsSingleCommaDelimitedString(audiences));
}

if (audiences == null)
if (tokenAudiences == null)
return new AudienceValidationResult(
Utility.SerializeAsSingleCommaDelimitedString(audiences),
Utility.SerializeAsSingleCommaDelimitedString(tokenAudiences),
ValidationFailureType.NullArgument,
new ExceptionDetail(
new MessageDetail(
Expand All @@ -75,23 +69,9 @@ internal static AudienceValidationResult ValidateAudience(IEnumerable<string> au
typeof(SecurityTokenInvalidAudienceException),
new StackFrame(true)));

if (string.IsNullOrWhiteSpace(validationParameters.ValidAudience) && (validationParameters.ValidAudiences == null))
if (tokenAudiences.Count == 0)
return new AudienceValidationResult(
Utility.SerializeAsSingleCommaDelimitedString(audiences),
ValidationFailureType.NullArgument,
new ExceptionDetail(
new MessageDetail(
LogMessages.IDX10208,
null),
typeof(SecurityTokenInvalidAudienceException),
new StackFrame(true)));

if (audiences is not List<string> audiencesAsList)
audiencesAsList = audiences.ToList();

if (audiencesAsList.Count == 0)
return new AudienceValidationResult(
Utility.SerializeAsSingleCommaDelimitedString(audiencesAsList),
Utility.SerializeAsSingleCommaDelimitedString(tokenAudiences),
ValidationFailureType.NullArgument,
new ExceptionDetail(
new MessageDetail(
Expand All @@ -100,81 +80,37 @@ internal static AudienceValidationResult ValidateAudience(IEnumerable<string> au
typeof(SecurityTokenInvalidAudienceException),
new StackFrame(true)));

string? validAudience = AudienceIsValidReturning(audiencesAsList, validationParameters);
string? validAudience = ValidTokenAudience(tokenAudiences, validationParameters.ValidAudiences, validationParameters.IgnoreTrailingSlashWhenValidatingAudience);
if (validAudience != null)
{
return new AudienceValidationResult(validAudience);
}

return new AudienceValidationResult(
Utility.SerializeAsSingleCommaDelimitedString(audiencesAsList),
Utility.SerializeAsSingleCommaDelimitedString(tokenAudiences),
ValidationFailureType.AudienceValidationFailed,
new ExceptionDetail(
new MessageDetail(
LogMessages.IDX10214,
LogHelper.MarkAsNonPII(Utility.SerializeAsSingleCommaDelimitedString(audiencesAsList)),
LogHelper.MarkAsNonPII(validationParameters.ValidAudience ?? "null"),
LogMessages.IDX10215,
LogHelper.MarkAsNonPII(Utility.SerializeAsSingleCommaDelimitedString(tokenAudiences)),
LogHelper.MarkAsNonPII(Utility.SerializeAsSingleCommaDelimitedString(validationParameters.ValidAudiences))),
typeof(SecurityTokenInvalidAudienceException),
new StackFrame(true)));
}

private static bool AudienceIsValid(List<string> audiences, TokenValidationParameters validationParameters)
{
return AudienceIsValidReturning(audiences, validationParameters) != null;
}

private static string? AudienceIsValidReturning(List<string> audiences, TokenValidationParameters validationParameters)
{
string? validAudience = null;
if (!string.IsNullOrWhiteSpace(validationParameters.ValidAudience))
validAudience = AudiencesMatchSingle(audiences, validationParameters.ValidAudience, validationParameters.IgnoreTrailingSlashWhenValidatingAudience);

if (validAudience == null && validationParameters.ValidAudiences != null)
{
if (validationParameters.ValidAudiences is not List<string> validAudiences)
validAudiences = validationParameters.ValidAudiences.ToList();

validAudience = AudiencesMatchList(audiences, validAudiences, validationParameters.IgnoreTrailingSlashWhenValidatingAudience);
}

return validAudience;
}

private static string? AudiencesMatchSingle(List<string> audiences, string validAudience, bool ignoreTrailingSlashWhenValidatingAudience)
private static string? ValidTokenAudience(IList<string> tokenAudiences, IList<string> validAudiences, bool ignoreTrailingSlashWhenValidatingAudience)
{
for (int i = 0; i < audiences.Count; i++)
for (int i = 0; i < tokenAudiences.Count; i++)
{
string tokenAudience = audiences[i];
if (string.IsNullOrWhiteSpace(tokenAudience))
string tokenAudience = tokenAudiences[i];
if (string.IsNullOrEmpty(tokenAudience))
continue;

if (AudiencesMatch(ignoreTrailingSlashWhenValidatingAudience, tokenAudience, validAudience))
for (int j = 0; j < validAudiences.Count; j++)
{
if (LogHelper.IsEnabled(EventLogLevel.Informational))
LogHelper.LogInformation(LogMessages.IDX10234, LogHelper.MarkAsNonPII(tokenAudience));

return tokenAudience;
}
}

return null;
}

private static string? AudiencesMatchList(IList<string> audiences, List<string> validAudiences, bool ignoreTrailingSlashWhenValidatingAudience)
{
for (int i = 0; i < audiences.Count; i++)
{
string tokenAudience = audiences[i];
if (string.IsNullOrWhiteSpace(tokenAudience))
continue;

foreach (string validAudience in validAudiences)
{
if (string.IsNullOrEmpty(validAudience))
if (string.IsNullOrEmpty(validAudiences[j]))
continue;

if (AudiencesMatch(ignoreTrailingSlashWhenValidatingAudience, tokenAudience, validAudience))

if (AudienceMatches(ignoreTrailingSlashWhenValidatingAudience, tokenAudience, validAudiences[j]))
{
if (LogHelper.IsEnabled(EventLogLevel.Informational))
LogHelper.LogInformation(LogMessages.IDX10234, LogHelper.MarkAsNonPII(tokenAudience));
Expand All @@ -187,17 +123,17 @@ private static bool AudienceIsValid(List<string> audiences, TokenValidationParam
return null;
}

private static bool AudiencesMatch(bool ignoreTrailingSlashWhenValidatingAudience, string tokenAudience, string validAudience)
private static bool AudienceMatches(bool ignoreTrailingSlashWhenValidatingAudience, string tokenAudience, string validAudience)
{
if (validAudience.Length == tokenAudience.Length)
return string.Equals(validAudience, tokenAudience);
else if (ignoreTrailingSlashWhenValidatingAudience && NewAudiencesMatchIgnoringTrailingSlash(tokenAudience, validAudience))
else if (ignoreTrailingSlashWhenValidatingAudience && AudienceMatchesIgnoringTrailingSlash(tokenAudience, validAudience))
return true;

return false;
}

private static bool NewAudiencesMatchIgnoringTrailingSlash(string tokenAudience, string validAudience)
private static bool AudienceMatchesIgnoringTrailingSlash(string tokenAudience, string validAudience)
{
int length = -1;

Expand Down
Loading