Skip to content

Fix ValidationParameters Inconsistencies #3173

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

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

Fix ValidationParameters Inconsistencies #3173

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aabf2e9
fix differences between ValidationParameters and TokenValidationParam…
omidmloo Mar 13, 2025
dab701f
fix: set TryAllIssuerSigningKeys in the empty constrructor
omidmloo Mar 15, 2025
582ad50
Merge branch 'dev' into fix-validation-parameters
JoshLozensky Mar 18, 2025
4faf727
fix: update the remarks to state that the default value is true
omidmloo Mar 22, 2025
701c043
Merge branch 'dev' into fix-validation-parameters
jennyf19 Apr 5, 2025
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
3 changes: 3 additions & 0 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 @@ -93,6 +93,7 @@ protected ValidationParameters(ValidationParameters other)
_signatureValidator = other.SignatureValidator;
TimeProvider = other.TimeProvider;
TryAllDecryptionKeys = other.TryAllDecryptionKeys;
TryAllIssuerSigningKeys = other.TryAllIssuerSigningKeys;
TokenDecryptionKeyResolver = other.TokenDecryptionKeyResolver;
_tokenDecryptionKeys = other.TokenDecryptionKeys;
TokenReplayCache = other.TokenReplayCache;
Expand Down Expand Up @@ -530,6 +531,7 @@ public TokenReplayValidationDelegate TokenReplayValidator
/// all available keys will be tried.
/// </summary>
/// <remarks>Default is false.</remarks>
Copy link
Contributor

@sruke sruke Mar 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the remarks to state that the default value is now true.

Copy link
Author

@omidmloo omidmloo Mar 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

[DefaultValue(true)]
Copy link
Contributor

@pmaytak pmaytak Mar 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs to be set in the empty constructor.

Copy link
Author

@omidmloo omidmloo Mar 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

@brentschmaltz brentschmaltz Jun 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be better to set the value on the property as if a new ctor is created it may be missed.
It would be a good idea to set the other properties: LogTokenId, TryAllDecryptionKeys
Add the attribute [DefaultValue(...)] as some of the bool properties as missing.

public bool TryAllIssuerSigningKeys { get; set; } = true;

public bool TryAllIssuerSigningKeys { get; set; }

/// <summary>
Expand Down Expand Up @@ -642,6 +644,7 @@ internal set
/// Gets or sets a boolean that controls if the actor claim should be validated.
/// </summary>
/// <remarks>Default value is false.</remarks>
[DefaultValue(false)]
public bool ValidateActor { get; set; }
}
}
Expand Down