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

Validators call delegates before checking if validation should occur #1272

Merged
merged 1 commit into from
Jan 7, 2020
Merged

Validators call delegates before checking if validation should occur #1272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 29 additions & 13 deletions src/Microsoft.IdentityModel.Tokens/TokenValidationParameters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ public TokenValidationParameters()
/// Gets or sets a delegate that will be used to validate the audience.
/// </summary>
/// <remarks>
/// If set, this delegate will be called to validate the 'audience' instead of normal processing.
/// If <see cref="ValidateAudience"/> is false, this delegate will not be called.
/// If set, this delegate will be called to validate the 'audience', instead of normal processing. This means that no default 'audience' validation will occur.
/// Even if <see cref="ValidateAudience"/> is false, this delegate will still be called.
/// </remarks>
public AudienceValidator AudienceValidator { get; set; }

Expand Down Expand Up @@ -329,7 +329,8 @@ public virtual ClaimsIdentity CreateClaimsIdentity(SecurityToken securityToken,
/// Gets or sets a delegate for validating the <see cref="SecurityKey"/> that signed the token.
/// </summary>
/// <remarks>
/// If set, this delegate will be called to validate the <see cref="SecurityKey"/> that signed the token, instead of normal processing.
/// If set, this delegate will be called to validate the <see cref="SecurityKey"/> that signed the token, instead of normal processing. This means that no default <see cref="SecurityKey"/> validation will occur.
/// Even if <see cref="ValidateIssuerSigningKey"/> is false, this delegate will still be called.
/// </remarks>
public IssuerSigningKeyValidator IssuerSigningKeyValidator { get; set; }

Expand All @@ -355,17 +356,17 @@ public virtual ClaimsIdentity CreateClaimsIdentity(SecurityToken securityToken,
/// Gets or sets a delegate that will be used to validate the issuer of the token.
/// </summary>
/// <remarks>
/// If set, this delegate will be called to validate the 'issuer' of the token, instead of normal processing.
/// If <see cref="ValidateIssuer"/> is false, this delegate will not be called.
/// If set, this delegate will be called to validate the 'issuer' of the token, instead of normal processing. This means that no default 'issuer' validation will occur.
/// Even if <see cref="ValidateIssuer"/> is false, this delegate will still be called.
/// </remarks>
public IssuerValidator IssuerValidator { get; set; }

/// <summary>
/// Gets or sets a delegate that will be used to validate the lifetime of the token
/// </summary>
/// <remarks>
/// If set, this delegate will be called to validate the lifetime of the token, instead of normal processing.
/// If <see cref="ValidateLifetime"/> is false, this delegate will not be called.
/// If set, this delegate will be called to validate the lifetime of the token, instead of normal processing. This means that no default lifetime validation will occur.
/// Even if <see cref="ValidateLifetime"/> is false, this delegate will still be called.
/// </remarks>
public LifetimeValidator LifetimeValidator { get; set; }

Expand Down Expand Up @@ -500,8 +501,8 @@ public string RoleClaimType
/// Gets or sets a delegate that will be used to validate the token replay of the token
/// </summary>
/// <remarks>
/// If set, this delegate will be called to validate the token replay of the token, instead of normal processing.
/// If <see cref="ValidateTokenReplay"/> is false, this delegate will not be called.
/// If set, this delegate will be called to validate the token replay of the token, instead of normal processing. This means no default token replay validation will occur.
/// Even if <see cref="ValidateTokenReplay"/> is false, this delegate will still be called.
/// </remarks>
public TokenReplayValidator TokenReplayValidator { get; set; }

Expand All @@ -515,7 +516,9 @@ public string RoleClaimType
/// Gets or sets a boolean to control if the audience will be validated during token validation.
/// </summary>
/// <remarks>Validation of the audience, mitigates forwarding attacks. For example, a site that receives a token, could not replay it to another side.
/// A forwarded token would contain the audience of the original site.</remarks>
/// A forwarded token would contain the audience of the original site.
/// This boolean only applies to default audience validation. If <see cref="AudienceValidator"/> is set, it will be called regardless of whether this
/// property is true or false.</remarks>
[DefaultValue(true)]
public bool ValidateAudience { get; set; }

Expand All @@ -528,27 +531,40 @@ public string RoleClaimType
/// It is possible that a token issued for the same audience could be from a different tenant. For example an application could accept users from
/// contoso.onmicrosoft.com but not fabrikam.onmicrosoft.com, both valid tenants. A application that accepts tokens from fabrikam could forward them
/// to the application that accepts tokens for contoso.
/// This boolean only applies to default issuer validation. If <see cref= "IssuerValidator" /> is set, it will be called regardless of whether this
/// property is true or false.
/// </remarks>
[DefaultValue(true)]
public bool ValidateIssuer { get; set; }

/// <summary>
/// Gets or sets a boolean to control if the lifetime will be validated during token validation.
/// </summary>
/// </summary>
/// <remarks>
/// This boolean only applies to default lifetime validation. If <see cref= "LifetimeValidator" /> is set, it will be called regardless of whether this
/// property is true or false.
/// </remarks>
[DefaultValue(true)]
public bool ValidateLifetime { get; set; }

/// <summary>
/// Gets or sets a boolean that controls if validation of the <see cref="SecurityKey"/> that signed the securityToken is called.
/// </summary>
/// <remarks>It is possible for tokens to contain the public key needed to check the signature. For example, X509Data can be hydrated into an X509Certificate,
/// which can be used to validate the signature. In these cases it is important to validate the SigningKey that was used to validate the signature. </remarks>
/// which can be used to validate the signature. In these cases it is important to validate the SigningKey that was used to validate the signature.
/// This boolean only applies to default signing key validation. If <see cref= "IssuerSigningKeyValidator" /> is set, it will be called regardless of whether this
/// property is true or false.
/// </remarks>
[DefaultValue(false)]
public bool ValidateIssuerSigningKey { get; set; }

/// <summary>
/// Gets or sets a boolean to control if the token replay will be validated during token validation.
/// </summary>
/// </summary>
/// <remarks>
/// This boolean only applies to default token replay validation. If <see cref= "TokenReplayValidator" /> is set, it will be called regardless of whether this
/// property is true or false.
/// </remarks>
[DefaultValue(false)]
public bool ValidateTokenReplay { get; set; }

Expand Down
52 changes: 26 additions & 26 deletions src/Microsoft.IdentityModel.Tokens/Validators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,6 @@ public static void ValidateAudience(IEnumerable<string> audiences, SecurityToken
if (validationParameters == null)
throw LogHelper.LogArgumentNullException(nameof(validationParameters));

if (!validationParameters.ValidateAudience)
{
LogHelper.LogWarning(LogMessages.IDX10233);
return;
}

if (validationParameters.AudienceValidator != null)
{
if (!validationParameters.AudienceValidator(audiences, securityToken, validationParameters))
Expand All @@ -70,6 +64,12 @@ public static void ValidateAudience(IEnumerable<string> audiences, SecurityToken
return;
}

if (!validationParameters.ValidateAudience)
{
LogHelper.LogWarning(LogMessages.IDX10233);
return;
}

if (audiences == null)
throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidAudienceException(LogMessages.IDX10207) { InvalidAudience = null });

Expand Down Expand Up @@ -127,15 +127,15 @@ public static string ValidateIssuer(string issuer, SecurityToken securityToken,
if (validationParameters == null)
throw LogHelper.LogArgumentNullException(nameof(validationParameters));

if (validationParameters.IssuerValidator != null)
return validationParameters.IssuerValidator(issuer, securityToken, validationParameters);

if (!validationParameters.ValidateIssuer)
{
LogHelper.LogInformation(LogMessages.IDX10235);
return issuer;
}

if (validationParameters.IssuerValidator != null)
return validationParameters.IssuerValidator(issuer, securityToken, validationParameters);

if (string.IsNullOrWhiteSpace(issuer))
throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidIssuerException(LogMessages.IDX10211)
{ InvalidIssuer = issuer });
Expand Down Expand Up @@ -188,16 +188,16 @@ public static void ValidateIssuerSecurityKey(SecurityKey securityKey, SecurityTo
if (validationParameters == null)
throw LogHelper.LogArgumentNullException(nameof(validationParameters));

if (!validationParameters.ValidateIssuerSigningKey)
if (validationParameters.IssuerSigningKeyValidator != null)
{
LogHelper.LogInformation(LogMessages.IDX10237);
return;
if (!validationParameters.IssuerSigningKeyValidator(securityKey, securityToken, validationParameters))
throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidSigningKeyException(LogHelper.FormatInvariant(LogMessages.IDX10232, securityKey)) { SigningKey = securityKey });
}

if (validationParameters.IssuerSigningKeyValidator != null)
if (!validationParameters.ValidateIssuerSigningKey)
{
if (!validationParameters.IssuerSigningKeyValidator(securityKey, securityToken, validationParameters))
throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidSigningKeyException(LogHelper.FormatInvariant(LogMessages.IDX10232, securityKey)){ SigningKey = securityKey });
LogHelper.LogInformation(LogMessages.IDX10237);
return;
}

if (!validationParameters.RequireSignedTokens && securityKey == null)
Expand Down Expand Up @@ -250,12 +250,6 @@ public static void ValidateLifetime(DateTime? notBefore, DateTime? expires, Secu
if (validationParameters == null)
throw LogHelper.LogArgumentNullException(nameof(validationParameters));

if (!validationParameters.ValidateLifetime)
{
LogHelper.LogInformation(LogMessages.IDX10238);
return;
}

if (validationParameters.LifetimeValidator != null)
{
if (!validationParameters.LifetimeValidator(notBefore, expires, securityToken, validationParameters))
Expand All @@ -265,6 +259,12 @@ public static void ValidateLifetime(DateTime? notBefore, DateTime? expires, Secu
return;
}

if (!validationParameters.ValidateLifetime)
{
LogHelper.LogInformation(LogMessages.IDX10238);
return;
}

if (!expires.HasValue && validationParameters.RequireExpirationTime)
throw LogHelper.LogExceptionMessage(new SecurityTokenNoExpirationException(LogHelper.FormatInvariant(LogMessages.IDX10225, securityToken == null ? "null" : securityToken.GetType().ToString())));

Expand Down Expand Up @@ -304,16 +304,16 @@ public static void ValidateTokenReplay(DateTime? expirationTime, string security
if (validationParameters == null)
throw LogHelper.LogArgumentNullException(nameof(validationParameters));

if (!validationParameters.ValidateTokenReplay)
if (validationParameters.TokenReplayValidator != null)
{
LogHelper.LogInformation(LogMessages.IDX10246);
if (!validationParameters.TokenReplayValidator(expirationTime, securityToken, validationParameters))
throw LogHelper.LogExceptionMessage(new SecurityTokenReplayDetectedException(LogHelper.FormatInvariant(LogMessages.IDX10228, securityToken)));
return;
}

if (validationParameters.TokenReplayValidator != null)
if (!validationParameters.ValidateTokenReplay)
{
if (!validationParameters.TokenReplayValidator(expirationTime, securityToken, validationParameters))
throw LogHelper.LogExceptionMessage(new SecurityTokenReplayDetectedException(LogHelper.FormatInvariant(LogMessages.IDX10228, securityToken)));
LogHelper.LogInformation(LogMessages.IDX10246);
return;
}

Expand Down
6 changes: 4 additions & 2 deletions test/Microsoft.IdentityModel.TestUtils/ReferenceTheoryData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ public static TheoryData<TokenReplayTheoryData> TokenReplayValidationTheoryData
new TokenReplayTheoryData
{
TestId = $"ValidateTokenReplay: false, {nameof(ValidationDelegates.TokenReplayValidatorReturnsFalse)}",
TokenReplayValidator = ValidationDelegates.TokenReplayValidatorReturnsFalse
TokenReplayValidator = ValidationDelegates.TokenReplayValidatorReturnsFalse,
ExpectedException = ExpectedException.SecurityTokenReplayDetected("IDX10228:")
},
new TokenReplayTheoryData
{
TestId = $"ValidateTokenReplay: false, {nameof(ValidationDelegates.TokenReplayValidatorThrows)}",
TokenReplayValidator = ValidationDelegates.TokenReplayValidatorThrows
TokenReplayValidator = ValidationDelegates.TokenReplayValidatorThrows,
ExpectedException = ExpectedException.SecurityTokenReplayDetected("TokenReplayValidatorThrows")
},
new TokenReplayTheoryData
{
Expand Down
2 changes: 2 additions & 0 deletions test/Microsoft.IdentityModel.Tokens.Saml.Tests/Saml2SecurityTokenHandlerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,7 @@ public static TheoryData<Saml2TheoryData> WriteTokenTheoryData

theoryData.Add(new Saml2TheoryData
{
ExpectedException = new ExpectedException(typeof(SecurityTokenInvalidSigningKeyException)),
SecurityToken = tokenHandler.CreateToken(tokenDescriptor),
TestId = nameof(ValidationDelegates.IssuerSecurityKeyValidatorThrows) + "-false",
ValidationParameters = validationParameters
Expand Down Expand Up @@ -468,6 +469,7 @@ public static TheoryData<Saml2TheoryData> WriteTokenTheoryData

theoryData.Add(new Saml2TheoryData
{
ExpectedException = new ExpectedException(typeof(SecurityTokenInvalidAudienceException)),
SecurityToken = tokenHandler.CreateToken(tokenDescriptor),
TestId = nameof(ValidationDelegates.AudienceValidatorThrows) + "-false",
ValidationParameters = validationParameters
Expand Down
2 changes: 2 additions & 0 deletions test/Microsoft.IdentityModel.Tokens.Saml.Tests/SamlSecurityTokenHandlerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,7 @@ public static TheoryData<SamlTheoryData> WriteTokenTheoryData

theoryData.Add(new SamlTheoryData
{
ExpectedException = new ExpectedException(typeof(SecurityTokenInvalidSigningKeyException)),
SecurityToken = tokenHandler.CreateToken(tokenDescriptor),
TestId = nameof(ValidationDelegates.IssuerSecurityKeyValidatorThrows) + "-false",
ValidationParameters = validationParameters
Expand Down Expand Up @@ -910,6 +911,7 @@ public static TheoryData<SamlTheoryData> WriteTokenTheoryData

theoryData.Add(new SamlTheoryData
{
ExpectedException = new ExpectedException(typeof(SecurityTokenInvalidAudienceException)),
SecurityToken = tokenHandler.CreateToken(tokenDescriptor),
TestId = nameof(ValidationDelegates.AudienceValidatorThrows) + "-false",
ValidationParameters = validationParameters
Expand Down
1 change: 1 addition & 0 deletions test/Microsoft.IdentityModel.Tokens.Saml.Tests/ValidateTheoryData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public static void AddValidateAudienceTheoryData(List<TokenTheoryData> theoryDat
theoryData.Add(new TokenTheoryData
{
Audiences = new List<string> { "John" },
ExpectedException = ExpectedException.SecurityTokenInvalidAudienceException(),
TestId = "AudienceValidator throws, validateAudience false",
ValidationParameters = new TokenValidationParameters
{
Expand Down
Loading