Skip to content
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

Validate IssuerSigningKey: Refactor to use ValidationParameters over TVP #2764

Merged
merged 7 commits into from
Aug 6, 2024
Merged

Validate IssuerSigningKey: Refactor to use ValidationParameters over TVP #2764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e8e3905
Refactored ValidateIssuerSecurityKey to use ValidationParameters over…
iNinja Aug 6, 2024
7a8a1af
Renamed class, updated delegate in ValidationParameters. Updated tests
iNinja Aug 6, 2024
59baef7
Updated documentation reference
iNinja Aug 6, 2024
4049ff8
Update src/Microsoft.IdentityModel.Tokens/Validation/Validators.Issue…
iNinja Aug 6, 2024
a17b6c4
Removed extra whitespace
iNinja Aug 6, 2024
9fcf021
Mark CallContext as optional
iNinja Aug 6, 2024
eb07cce
Added configuration parameter to ValidateIssuerSigningKey
iNinja 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
7 changes: 6 additions & 1 deletion src/Microsoft.IdentityModel.Tokens/Validation/ValidationParameters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ internal class ValidationParameters
private LifetimeValidatorDelegate _lifetimeValidator = Validators.ValidateLifetime;
private TokenReplayValidatorDelegate _tokenReplayValidator = Validators.ValidateTokenReplay;
private TypeValidatorDelegate _typeValidator = Validators.ValidateTokenType;
private IssuerSigningKeyValidatorDelegate _issuerSigningKeyValidator = Validators.ValidateIssuerSigningKey;

/// <summary>
/// This is the default value of <see cref="ClaimsIdentity.AuthenticationType"/> when creating a <see cref="ClaimsIdentity"/>.
Expand Down Expand Up @@ -269,7 +270,11 @@ public virtual ClaimsIdentity CreateClaimsIdentity(SecurityToken securityToken,
/// If both <see cref="IssuerSigningKeyValidatorUsingConfiguration"/> and <see cref="IssuerSigningKeyValidator"/> are set, IssuerSigningKeyResolverUsingConfiguration takes
/// priority.
/// </remarks>
public IssuerSigningKeyValidator IssuerSigningKeyValidator { get; set; }
public IssuerSigningKeyValidatorDelegate IssuerSigningKeyValidator
{
get => _issuerSigningKeyValidator;
set => _issuerSigningKeyValidator = value ?? throw new ArgumentNullException(nameof(value), "IssuerSigningKeyValidator cannot be set as null.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider creating a log message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have a number of similar scenarios. I will definitely look into generalising them and reusing a log message or creating a new one as part of this stream.

}

/// <summary>
/// Gets a <see cref="IDictionary{String, Object}"/> that is unique to this instance.
Expand Down
114 changes: 24 additions & 90 deletions ...alidation/Validators.IssuerSecurityKey.cs → ...Validation/Validators.IssuerSigningKey.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ namespace Microsoft.IdentityModel.Tokens
/// </summary>
/// <param name="signingKey">The security key to validate.</param>
/// <param name="securityToken">The <see cref="SecurityToken"/> that is being validated.</param>
/// <param name="validationParameters">The <see cref="TokenValidationParameters"/> to be used for validating the token.</param>
/// <param name="callContext"></param>
/// <param name="cancellationToken"></param>
iNinja marked this conversation as resolved.
Show resolved Hide resolved
/// <param name="validationParameters">The <see cref="ValidationParameters"/> to be used for validating the token.</param>
/// <param name="configuration">The <see cref="BaseConfiguration"/> to be used for validation.</param>
/// <param name="callContext">The <see cref="CallContext"/> to be used for logging.</param>
/// <returns>A <see cref="SigningKeyValidationResult"/>that contains the results of validating the issuer.</returns>
/// <remarks>This delegate is not expected to throw.</remarks>
internal delegate Task<SigningKeyValidationResult> IssuerSecurityKeyValidationDelegate(
internal delegate SigningKeyValidationResult IssuerSigningKeyValidatorDelegate(
SecurityKey signingKey,
SecurityToken securityToken,
TokenValidationParameters validationParameters,
CallContext callContext,
CancellationToken cancellationToken);
ValidationParameters validationParameters,
BaseConfiguration? configuration,
CallContext? callContext);

/// <summary>
/// SigningKeyValidation
Expand All @@ -40,28 +40,20 @@ public static partial class Validators
/// </summary>
/// <param name="securityKey">The <see cref="SecurityKey"/> that signed 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>
/// <param name="validationParameters">The <see cref="ValidationParameters"/> to be used for validating the token.</param>
/// <param name="configuration">The <see cref="BaseConfiguration"/> to be used for validation.</param>
/// <param name="callContext">The <see cref="CallContext"/> to be used for logging.</param>
/// <exception cref="ArgumentNullException"> if 'securityKey' is null and ValidateIssuerSigningKey is true.</exception>
/// <exception cref="ArgumentNullException"> if 'securityToken' is null and ValidateIssuerSigningKey is true.</exception>
/// <exception cref="ArgumentNullException"> if 'validationParameters' is null.</exception>
internal static SigningKeyValidationResult ValidateIssuerSecurityKey(SecurityKey securityKey, SecurityToken securityToken, TokenValidationParameters validationParameters, CallContext callContext)
{
return ValidateIssuerSecurityKey(securityKey, securityToken, validationParameters, null, callContext);
}

/// <summary>
/// Validates the <see cref="SecurityKey"/> that signed a <see cref="SecurityToken"/>.
/// </summary>
/// <param name="securityKey">The <see cref="SecurityKey"/> that signed 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="configuration">The <see cref="BaseConfiguration"/> required for issuer and signing key validation.</param>
/// <param name="callContext"></param>
/// <exception cref="ArgumentNullException"> if 'securityKey' is null and ValidateIssuerSigningKey is true.</exception>
/// <exception cref="ArgumentNullException"> if 'securityToken' is null and ValidateIssuerSigningKey is true.</exception>
/// <exception cref="ArgumentNullException"> if 'validationParameters' is null.</exception>
internal static SigningKeyValidationResult ValidateIssuerSecurityKey(SecurityKey securityKey, SecurityToken securityToken, TokenValidationParameters validationParameters, BaseConfiguration? configuration, CallContext callContext)
internal static SigningKeyValidationResult ValidateIssuerSigningKey(
SecurityKey securityKey,
SecurityToken securityToken,
ValidationParameters validationParameters,
#pragma warning disable CA1801 // Review unused parameters
BaseConfiguration? configuration,
#pragma warning restore CA1801 // Review unused parameters
CallContext? callContext)
{
if (validationParameters == null)
return new SigningKeyValidationResult(
Expand All @@ -74,28 +66,7 @@ internal static SigningKeyValidationResult ValidateIssuerSecurityKey(SecurityKey
typeof(ArgumentNullException),
new StackFrame(true)));

if (validationParameters.IssuerSigningKeyValidatorUsingConfiguration != null)
{
return ValidateSigningKeyUsingDelegateAndConfiguration(securityKey, securityToken, validationParameters, configuration);
}

if (validationParameters.IssuerSigningKeyValidator != null)
{
return ValidateSigningKeyUsingDelegateAndConfiguration(securityKey, securityToken, validationParameters, null);
}

if (!validationParameters.ValidateIssuerSigningKey)
{
LogHelper.LogVerbose(LogMessages.IDX10237);
return new SigningKeyValidationResult(securityKey);
}

if (!validationParameters.RequireSignedTokens && securityKey == null)
{
LogHelper.LogInformation(LogMessages.IDX10252);
return new SigningKeyValidationResult(securityKey);
}
else if (securityKey == null)
if (securityKey == null)
{
return new SigningKeyValidationResult(
securityKey,
Expand Down Expand Up @@ -126,10 +97,13 @@ internal static SigningKeyValidationResult ValidateIssuerSecurityKey(SecurityKey
/// Given a signing key, when it's derived from a certificate, validates that the certificate is already active and non-expired
/// </summary>
/// <param name="securityKey">The <see cref="SecurityKey"/> that signed the <see cref="SecurityToken"/>.</param>
/// <param name="validationParameters">The <see cref="TokenValidationParameters"/> to be used for validating the token.</param>
/// <param name="validationParameters">The <see cref="ValidationParameters"/> to be used for validating the token.</param>
/// <param name="callContext"></param>
#pragma warning disable CA1801 // Review unused parameters
internal static SigningKeyValidationResult ValidateIssuerSigningKeyLifeTime(SecurityKey securityKey, TokenValidationParameters validationParameters, CallContext callContext)
internal static SigningKeyValidationResult ValidateIssuerSigningKeyLifeTime(
SecurityKey securityKey,
ValidationParameters validationParameters,
CallContext? callContext)
#pragma warning restore CA1801 // Review unused parameters
{
X509SecurityKey? x509SecurityKey = securityKey as X509SecurityKey;
Expand Down Expand Up @@ -174,46 +148,6 @@ internal static SigningKeyValidationResult ValidateIssuerSigningKeyLifeTime(Secu

return new SigningKeyValidationResult(securityKey);
}

private static SigningKeyValidationResult ValidateSigningKeyUsingDelegateAndConfiguration(SecurityKey securityKey, SecurityToken securityToken, TokenValidationParameters validationParameters, BaseConfiguration? configuration)
{
try
{
bool success;
if (configuration != null)
success = validationParameters.IssuerSigningKeyValidatorUsingConfiguration(securityKey, securityToken, validationParameters, configuration);
else
success = validationParameters.IssuerSigningKeyValidator(securityKey, securityToken, validationParameters);

if (!success)
return new SigningKeyValidationResult(
securityKey,
ValidationFailureType.SigningKeyValidationFailed,
new ExceptionDetail(
new MessageDetail(
LogMessages.IDX10232,
securityKey),
typeof(SecurityTokenInvalidSigningKeyException),
new StackFrame(true)));

return new SigningKeyValidationResult(securityKey);
}
#pragma warning disable CA1031 // Do not catch general exception types
catch (Exception exception)
#pragma warning restore CA1031 // Do not catch general exception types
{
return new SigningKeyValidationResult(
securityKey,
ValidationFailureType.SigningKeyValidationFailed,
new ExceptionDetail(
new MessageDetail(
LogMessages.IDX10232,
securityKey),
exception.GetType(),
new StackFrame(true),
exception));
}
}
}
}
#nullable restore
Loading
Loading