Skip to content

Commit

Permalink
Fetch configuration when validating SAML issuer and signature
Browse files Browse the repository at this point in the history 
This is already done in the JwtSecurityTokenHandler and the semantics should be similar.
This caused a regression in Microsoft.AspNetCore.Authentication.WsFederation.WsFederationHandler which assumes the token handler will fetch the current configuration as part of the token validation if the ConfigurationManager is a subclass of BaseConfigurationManager.
This change makes the SamlSecurityTokenHandler and Saml2SecurityTokenHandler equal to the JwtSecurityTokenHandler with regards to fetching the configuration before validation.
  • Loading branch information
hakon.lerring committed Nov 21, 2023
1 parent c633486 commit 41842f1
Show file tree
Hide file tree
Showing 4 changed files with 161 additions and 37 deletions.
73 changes: 56 additions & 17 deletions src/Microsoft.IdentityModel.Tokens.Saml/Saml/SamlSecurityTokenHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Linq;
using System.Security.Claims;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using System.Xml;
using Microsoft.IdentityModel.Abstractions;
Expand Down Expand Up @@ -70,12 +71,12 @@ public IEqualityComparer<SamlSubject> SamlSubjectEqualityComparer
/// <summary>
/// Gets or set the <see cref="SamlSerializer"/> that will be used to read and write a <see cref="SamlSecurityToken"/>.
/// </summary>
/// <exception cref="ArgumentNullException">'value' is null.</exception>
/// <exception cref="ArgumentNullException">'value' is null.</exception>
public SamlSerializer Serializer
{
get { return _serializer; }
set { _serializer = value ?? throw LogHelper.LogArgumentNullException(nameof(value)); }
}
}

/// <summary>
/// Gets the securityToken type supported by this handler.
Expand Down Expand Up @@ -131,7 +132,7 @@ public override bool CanReadToken(string securityToken)
using (var sr = new StringReader(securityToken))
{
var settings = new XmlReaderSettings { DtdProcessing = DtdProcessing.Prohibit };
using (var reader = XmlDictionaryReader.CreateDictionaryReader(XmlReader.Create(sr, settings)))
using (var reader = XmlDictionaryReader.CreateDictionaryReader(XmlReader.Create(sr, settings)))
{
return CanReadToken(reader);
}
Expand Down Expand Up @@ -186,7 +187,7 @@ protected virtual ICollection<SamlAttribute> ConsolidateAttributes(ICollection<S
}

/// <summary>
/// Override this method to provide a SamlAdvice to place in the Samltoken.
/// Override this method to provide a SamlAdvice to place in the Samltoken.
/// </summary>
/// <param name="tokenDescriptor">Contains information about the token.</param>
/// <returns>SamlAdvice, default is null.</returns>
Expand Down Expand Up @@ -286,7 +287,7 @@ protected virtual SamlAttributeStatement CreateAttributeStatement(SamlSubject su
}

/// <summary>
/// Creates a SamlAuthenticationStatement for each AuthenticationInformation found in AuthenticationInformation.
/// Creates a SamlAuthenticationStatement for each AuthenticationInformation found in AuthenticationInformation.
/// Override this method to provide a custom implementation.
/// </summary>
/// <param name="subject">The SamlSubject of the Statement.</param>
Expand Down Expand Up @@ -496,11 +497,11 @@ public virtual SecurityToken CreateToken(SecurityTokenDescriptor tokenDescriptor
}

/// <summary>
/// Builds an XML formated string from a collection of saml attributes that represent an Actor.
/// Builds an XML formated string from a collection of saml attributes that represent an Actor.
/// </summary>
/// <param name="attributes"><see cref="ICollection{SamlAttribute}"/>.</param>
/// <returns>A well formed XML string.</returns>
/// <remarks>The string is of the form "&lt;Actor&gt;&lt;SamlAttribute name, ns&gt;&lt;SamlAttributeValue&gt;...&lt;/SamlAttributeValue&gt;, ...&lt;/SamlAttribute&gt;...&lt;/Actor&gt;"</remarks>
/// <remarks>The string is of the form "&lt;Actor&gt;&lt;SamlAttribute name, ns&gt;&lt;SamlAttributeValue&gt;...&lt;/SamlAttributeValue&gt;, ...&lt;/SamlAttribute&gt;...&lt;/Actor&gt;"</remarks>
protected virtual string CreateXmlStringFromAttributes(ICollection<SamlAttribute> attributes)
{
if (attributes == null)
Expand Down Expand Up @@ -766,7 +767,7 @@ public virtual SamlSecurityToken ReadSamlToken(XmlReader reader)
/// <summary>
/// Deserializes from XML a token of the type handled by this instance.
/// </summary>
/// <param name="reader">An XML reader positioned at the token's start
/// <param name="reader">An XML reader positioned at the token's start
/// element.</param>
/// <param name="validationParameters"> validation parameters for the <see cref="SamlSecurityToken"/>.</param>
/// <returns>An instance of <see cref="SamlSecurityToken"/>.</returns>
Expand Down Expand Up @@ -801,7 +802,7 @@ protected virtual SecurityKey ResolveIssuerSigningKey(string token, SamlSecurity
}

/// <summary>
/// This method gets called when a special type of SamlAttribute is detected. The SamlAttribute passed in wraps a SamlAttribute
/// This method gets called when a special type of SamlAttribute is detected. The SamlAttribute passed in wraps a SamlAttribute
/// that contains a collection of AttributeValues, each of which are mapped to a claim. All of the claims will be returned
/// in an ClaimsIdentity with the specified issuer.
/// </summary>
Expand Down Expand Up @@ -910,7 +911,7 @@ protected virtual void ValidateConditions(SamlSecurityToken securityToken, Token
{
if (validationParameters.RequireAudience)
throw LogExceptionMessage(new SamlSecurityTokenException(LogMessages.IDX11401));

return;
}

Expand Down Expand Up @@ -979,7 +980,7 @@ protected virtual void ValidateLifetime(DateTime? notBefore, DateTime? expires,
/// <exception cref="SecurityTokenValidationException">If <see cref="ReadSamlToken(string)"/> returns null"/>.</exception>
/// <exception cref="SecurityTokenValidationException">If <see cref="TokenValidationParameters.SignatureValidator"/> returns null OR an object other than a <see cref="SamlSecurityToken"/>.</exception>
/// <exception cref="SecurityTokenValidationException">If a signature is not found and <see cref="TokenValidationParameters.RequireSignedTokens"/> is true.</exception>
/// <exception cref="SecurityTokenSignatureKeyNotFoundException">If the 'token' has a key identifier and none of the <see cref="SecurityKey"/>(s) provided result in a validated signature.
/// <exception cref="SecurityTokenSignatureKeyNotFoundException">If the 'token' has a key identifier and none of the <see cref="SecurityKey"/>(s) provided result in a validated signature.
/// This can indicate that a key refresh is required.</exception>
/// <exception cref="SecurityTokenInvalidSignatureException">If after trying all the <see cref="SecurityKey"/>(s), none result in a validated signture AND the 'token' does not have a key identifier.</exception>
/// <returns>A <see cref="SamlSecurityToken"/> that has had the signature validated if token was signed.</returns>
Expand Down Expand Up @@ -1139,6 +1140,9 @@ public override ClaimsPrincipal ValidateToken(XmlReader reader, TokenValidationP
if (validationParameters == null)
throw LogArgumentNullException(nameof(validationParameters));

validationParameters = PopulateValidationParametersWithCurrentConfigurationAsync(validationParameters).ConfigureAwait(false).GetAwaiter()
.GetResult();

var samlToken = ReadSamlToken(reader);
if (samlToken == null)
throw LogExceptionMessage(
Expand All @@ -1150,25 +1154,41 @@ public override ClaimsPrincipal ValidateToken(XmlReader reader, TokenValidationP
}

/// <inheritdoc/>
public override Task<TokenValidationResult> ValidateTokenAsync(string token, TokenValidationParameters validationParameters)
public override async Task<TokenValidationResult> ValidateTokenAsync(string token, TokenValidationParameters validationParameters)
{
try
{
var claimsPrincipal = ValidateToken(token, validationParameters, out var validatedToken);
return Task.FromResult(new TokenValidationResult
if (string.IsNullOrWhiteSpace(token))
throw LogArgumentNullException(nameof(token));

if (validationParameters == null)
throw LogArgumentNullException(nameof(validationParameters));

if (token.Length > MaximumTokenSizeInBytes)
throw LogExceptionMessage(new ArgumentException(FormatInvariant(TokenLogMessages.IDX10209, LogHelper.MarkAsNonPII(token.Length), LogHelper.MarkAsNonPII(MaximumTokenSizeInBytes))));

validationParameters = await PopulateValidationParametersWithCurrentConfigurationAsync(validationParameters).ConfigureAwait(false);

var samlToken = ValidateSignature(token, validationParameters);
if (samlToken == null)
throw LogExceptionMessage(new SecurityTokenValidationException(
FormatInvariant(TokenLogMessages.IDX10254, LogHelper.MarkAsNonPII(_className), LogHelper.MarkAsNonPII("ValidateToken"), LogHelper.MarkAsNonPII(GetType()), LogHelper.MarkAsNonPII("ValidateSignature"), LogHelper.MarkAsNonPII(typeof(SamlSecurityToken)))));

var claimsPrincipal = ValidateToken(samlToken, token, validationParameters, out var validatedToken);
return new TokenValidationResult
{
SecurityToken = validatedToken,
ClaimsIdentity = claimsPrincipal?.Identities.First(),
IsValid = true,
});
};
}
catch (Exception ex)
{
return Task.FromResult(new TokenValidationResult
return new TokenValidationResult
{
IsValid = false,
Exception = ex
});
};
}
}

Expand All @@ -1193,6 +1213,9 @@ public override ClaimsPrincipal ValidateToken(string token, TokenValidationParam
if (token.Length > MaximumTokenSizeInBytes)
throw LogExceptionMessage(new ArgumentException(FormatInvariant(TokenLogMessages.IDX10209, LogHelper.MarkAsNonPII(token.Length), LogHelper.MarkAsNonPII(MaximumTokenSizeInBytes))));

validationParameters = PopulateValidationParametersWithCurrentConfigurationAsync(validationParameters).ConfigureAwait(false).GetAwaiter()
.GetResult();

var samlToken = ValidateSignature(token, validationParameters);
if (samlToken == null)
throw LogExceptionMessage(new SecurityTokenValidationException(
Expand Down Expand Up @@ -1225,6 +1248,22 @@ private ClaimsPrincipal ValidateToken(SamlSecurityToken samlToken, string token,
return new ClaimsPrincipal(identities);
}

private static async Task<TokenValidationParameters> PopulateValidationParametersWithCurrentConfigurationAsync(
TokenValidationParameters validationParameters)
{
if (validationParameters.ConfigurationManager != null)
{
var currentConfiguration = await validationParameters.ConfigurationManager.GetBaseConfigurationAsync(CancellationToken.None).ConfigureAwait(false);
validationParameters = validationParameters.Clone();
var issuers = new[] { currentConfiguration.Issuer };

validationParameters.ValidIssuers = (validationParameters.ValidIssuers == null ? issuers : validationParameters.ValidIssuers.Concat(issuers));
validationParameters.IssuerSigningKeys = (validationParameters.IssuerSigningKeys == null ? currentConfiguration.SigningKeys : validationParameters.IssuerSigningKeys.Concat(currentConfiguration.SigningKeys));
}

return validationParameters;
}

/// <summary>
/// Serializes a <see cref="SamlSecurityToken"/> to a string.
/// </summary>
Expand Down

0 comments on commit 41842f1

Please sign in to comment.