Skip to content

Update Wilson7 branch #49524

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.

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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 8 additions & 9 deletions src/Security/Authentication/JwtBearer/src/JwtBearerHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.IdentityModel.Protocols.OpenIdConnect;
using Microsoft.IdentityModel.Tokens;
using Microsoft.Net.Http.Headers;

Expand All @@ -20,8 +19,6 @@ namespace Microsoft.AspNetCore.Authentication.JwtBearer;
/// </summary>
public class JwtBearerHandler : AuthenticationHandler<JwtBearerOptions>
{
private OpenIdConnectConfiguration? _configuration;

/// <summary>
/// Initializes a new instance of <see cref="JwtBearerHandler"/>.
/// </summary>
Expand Down Expand Up @@ -101,7 +98,7 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
SecurityToken? validatedToken = null;
ClaimsPrincipal? principal = null;

if (Options.UseTokenHandlers)
if (!Options.UseSecurityTokenValidators)
{
foreach (var tokenHandler in Options.TokenHandlers)
{
Expand All @@ -123,12 +120,13 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
catch (Exception ex)
{
validationFailures ??= new List<Exception>(1);
RecordTokenValidationError(new SecurityTokenValidationException($"TokenHandler: '{tokenHandler}', threw an exception (see inner exception).", ex), validationFailures);
RecordTokenValidationError(ex, validationFailures);
}
}
}
else
{
#pragma warning disable CS0618 // Type or member is obsolete
foreach (var validator in Options.SecurityTokenValidators)
{
if (validator.CanReadToken(token))
Expand All @@ -145,6 +143,7 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
}
}
}
#pragma warning restore CS0618 // Type or member is obsolete
}

if (principal != null && validatedToken != null)
Expand Down Expand Up @@ -194,7 +193,7 @@ protected override async Task<AuthenticateResult> HandleAuthenticateAsync()
return AuthenticateResult.Fail(authenticationFailedContext.Exception);
}

if (Options.UseTokenHandlers)
if (!Options.UseSecurityTokenValidators)
{
return AuthenticateResults.TokenHandlerUnableToValidate;
}
Expand Down Expand Up @@ -251,10 +250,10 @@ private async Task<TokenValidationParameters> SetupTokenValidationParametersAsyn
if (Options.ConfigurationManager != null)
{
// GetConfigurationAsync has a time interval that must pass before new http request will be issued.
_configuration = await Options.ConfigurationManager.GetConfigurationAsync(Context.RequestAborted);
var issuers = new[] { _configuration.Issuer };
var configuration = await Options.ConfigurationManager.GetConfigurationAsync(Context.RequestAborted);
var issuers = new[] { configuration.Issuer };
tokenValidationParameters.ValidIssuers = (tokenValidationParameters.ValidIssuers == null ? issuers : tokenValidationParameters.ValidIssuers.Concat(issuers));
tokenValidationParameters.IssuerSigningKeys = (tokenValidationParameters.IssuerSigningKeys == null ? _configuration.SigningKeys : tokenValidationParameters.IssuerSigningKeys.Concat(_configuration.SigningKeys));
tokenValidationParameters.IssuerSigningKeys = (tokenValidationParameters.IssuerSigningKeys == null ? configuration.SigningKeys : tokenValidationParameters.IssuerSigningKeys.Concat(configuration.SigningKeys));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ public class JwtBearerOptions : AuthenticationSchemeOptions
/// </summary>
public JwtBearerOptions()
{
#pragma warning disable CS0618 // Type or member is obsolete
SecurityTokenValidators = new List<ISecurityTokenValidator> { _defaultHandler };
#pragma warning restore CS0618 // Type or member is obsolete
TokenHandlers = new List<TokenHandler> { _defaultTokenHandler };
}

Expand Down Expand Up @@ -111,6 +113,7 @@ public JwtBearerOptions()
/// <summary>
/// Gets the ordered list of <see cref="ISecurityTokenValidator"/> used to validate access tokens.
/// </summary>
[Obsolete("SecurityTokenValidators is no longer used by default. Use TokenHandlers instead. To continue using SecurityTokenValidators, set UseSecurityTokenValidators to true. See https://aka.ms/aspnetcore8/security-token-changes")]
public IList<ISecurityTokenValidator> SecurityTokenValidators { get; private set; }

/// <summary>
Expand Down Expand Up @@ -180,5 +183,5 @@ public bool MapInboundClaims
/// <para>The default token handler is a <see cref="JsonWebTokenHandler"/> which is faster than a <see cref="JwtSecurityTokenHandler"/>.</para>
/// <para>There is an ability to make use of a Last-Known-Good model for metadata that protects applications when metadata is published with errors.</para>
/// </remarks>
public bool UseTokenHandlers { get; set; }
public bool UseSecurityTokenValidators { get; set; }
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#nullable enable
Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerHandler.JwtBearerHandler(Microsoft.Extensions.Options.IOptionsMonitor<Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions!>! options, Microsoft.Extensions.Logging.ILoggerFactory! logger, System.Text.Encodings.Web.UrlEncoder! encoder) -> void
Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions.TokenHandlers.get -> System.Collections.Generic.IList<Microsoft.IdentityModel.Tokens.TokenHandler!>!
Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions.UseTokenHandlers.get -> bool
Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions.UseTokenHandlers.set -> void
Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions.UseSecurityTokenValidators.get -> bool
Microsoft.AspNetCore.Authentication.JwtBearer.JwtBearerOptions.UseSecurityTokenValidators.set -> void
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#nullable enable
Microsoft.AspNetCore.Authentication.WsFederation.WsFederationHandler.WsFederationHandler(Microsoft.Extensions.Options.IOptionsMonitor<Microsoft.AspNetCore.Authentication.WsFederation.WsFederationOptions!>! options, Microsoft.Extensions.Logging.ILoggerFactory! logger, System.Text.Encodings.Web.UrlEncoder! encoder) -> void
Microsoft.AspNetCore.Authentication.WsFederation.WsFederationOptions.TokenHandlers.get -> System.Collections.Generic.ICollection<Microsoft.IdentityModel.Tokens.TokenHandler!>!
Microsoft.AspNetCore.Authentication.WsFederation.WsFederationOptions.UseTokenHandlers.get -> bool
Microsoft.AspNetCore.Authentication.WsFederation.WsFederationOptions.UseTokenHandlers.set -> void
Microsoft.AspNetCore.Authentication.WsFederation.WsFederationOptions.UseSecurityTokenHandlers.get -> bool
Microsoft.AspNetCore.Authentication.WsFederation.WsFederationOptions.UseSecurityTokenHandlers.set -> void
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
<value>The service descriptor is missing.</value>
</data>
<data name="Exception_NoTokenValidatorFound" xml:space="preserve">
<value>No token validator was found for the given token.</value>
<value>No token validator or token handler was found for the given token.</value>
</data>
<data name="Exception_OptionMustBeProvided" xml:space="preserve">
<value>The '{0}' option must be provided.</value>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
var tvp = await SetupTokenValidationParametersAsync();
ClaimsPrincipal? principal = null;
SecurityToken? validatedToken = null;
if (Options.UseTokenHandlers)
if (!Options.UseSecurityTokenHandlers)
{
foreach (var tokenHandler in Options.TokenHandlers)
{
Expand Down Expand Up @@ -277,6 +277,7 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
else
{

#pragma warning disable CS0618 // Type or member is obsolete
foreach (var validator in Options.SecurityTokenHandlers)
{
if (validator.CanReadToken(token))
Expand All @@ -294,11 +295,11 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
break;
}
}
#pragma warning restore CS0618 // Type or member is obsolete
}

if (principal == null)
{
// TODO - need new string for TokenHandler
if (validationFailures == null || validationFailures.Count == 0)
{
throw new SecurityTokenException(Resources.Exception_NoTokenValidatorFound);
Expand Down Expand Up @@ -375,7 +376,6 @@ private async Task<TokenValidationParameters> SetupTokenValidationParametersAsyn

if (Options.ConfigurationManager is BaseConfigurationManager baseConfigurationManager)
{
// TODO - we need to add a parameter to TokenValidationParameters for the CancellationToken.
tokenValidationParameters.ConfigurationManager = baseConfigurationManager;
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ public override void Validate()
/// <summary>
/// Gets or sets the collection of <see cref="ISecurityTokenValidator"/> used to read and validate the <see cref="SecurityToken"/>s.
/// </summary>
[Obsolete("SecurityTokenHandlers is no longer used by default. Use TokenHandlers instead. To continue using SecurityTokenHandlers, set UseSecurityTokenHandlers to true. See https://aka.ms/aspnetcore8/security-token-changes")]
public ICollection<ISecurityTokenValidator> SecurityTokenHandlers
{
get
Expand All @@ -118,7 +119,7 @@ public ICollection<ISecurityTokenValidator> SecurityTokenHandlers
}

/// <summary>
/// Gets or sets the collection of <see cref="ISecurityTokenValidator"/> used to read and validate the <see cref="SecurityToken"/>s.
/// Gets the collection of <see cref="ISecurityTokenValidator"/> used to read and validate the <see cref="SecurityToken"/>s.
/// </summary>
public ICollection<TokenHandler> TokenHandlers
{
Expand Down Expand Up @@ -208,5 +209,5 @@ public TokenValidationParameters TokenValidationParameters
/// <para>The default token handler for JsonWebTokens is a <see cref="JsonWebTokenHandler"/> which is faster than a <see cref="JwtSecurityTokenHandler"/>.</para>
/// <para>There is an ability to make use of a Last-Known-Good model for metadata that protects applications when metadata is published with errors.</para>
/// </remarks>
public bool UseTokenHandlers { get; set; }
public bool UseSecurityTokenHandlers { get; set; }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: naming is slightly different than what was called out in API review because API review mistakenly used the name 'SecurityTokenValidators ' where for wsfed it is called 'SecurityTokenHandlers'

#49469

Copy link
Contributor

Choose a reason for hiding this comment

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

which are not the TokenHandler unfortunately.
I wonder if, for the sake of simplicity and uniformity, we shouldn't keep UseSecurityTokenValidators even for WsFed ... just a suggestion.
Although looking at the code .. probably not.

}
Loading