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.

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

[Bug] Clearing JwtSecurityTokenHandler.DefaultInboundClaimTypeMap no longer prevents sub from being mapped to http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier #2230

Closed
2 of 14 tasks
Cyberboss opened this issue Aug 14, 2023 · 3 comments

Comments

@Cyberboss
Copy link

Which version of Microsoft.IdentityModel are you using?
7.0.0-preview

Where is the issue?

  • M.IM.JsonWebTokens
  • M.IM.KeyVaultExtensions
  • M.IM.Logging
  • M.IM.ManagedKeyVaultSecurityKey
  • M.IM.Protocols
  • M.IM.Protocols.OpenIdConnect
  • M.IM.Protocols.SignedHttpRequest
  • M.IM.Protocols.WsFederation
  • M.IM.TestExtensions
  • M.IM.Tokens
  • M.IM.Tokens.Saml
  • M.IM.Validators
  • M.IM.Xml
  • S.IM.Tokens.Jwt
  • Other (please describe)

Is this a new or an existing app?
The app is in production and I have upgraded to a new version of Microsoft.IdentityModel.*

Repro

Requires sending a valid JWT with the sub field to an ASP.NET core authentication pipeline.

// configure bearer token validation
services
	.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
	.AddJwtBearer(jwtBearerOptions =>
	{
		// this line isn't actually run until the first request is made
		// at that point tokenFactory will be populated
		jwtBearerOptions.TokenValidationParameters = new TokenValidationParameters
		{
			ValidateIssuerSigningKey = true,
			IssuerSigningKey = new SymmetricSecurityKey(_256RandomBytes),

			ValidateIssuer = true,
			ValidIssuer = "IssuerName",

			ValidateLifetime = true,
			ValidateAudience = true,
			ValidAudience = "AudienceName",

			ClockSkew = TimeSpan.FromMinutes(1),

			RequireSignedTokens = true,

			RequireExpirationTime = true,
		};
		jwtBearerOptions.Events = new JwtBearerEvents
		{
			// Application is our composition root so this monstrosity of a line is okay
			// At least, that's what I tell myself to sleep at night
			OnTokenValidated = ctx =>
			{
				var userIdClaim = ctx.Principal.FindFirst(JwtRegisteredClaimNames.Sub);
				if (userIdClaim == default)
					throw new InvalidOperationException("Missing required claim!"); // This error triggers

				return Task.CompletedTask;
			},
		};
	});

	JwtSecurityTokenHandler.DefaultInboundClaimTypeMap.Clear();

Expected behavior
A "sub" claim is present in the TokenValidatedContext.Principal.

Actual behavior
The expected value is instead in the http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier claim.

Possible solution
Set JwtBearerOptions.MapInboundClaims to false and do not clear JwtSecurityTokenHandler.DefaultInboundClaimTypeMap.

Additional context / logs / screenshots / links to code
First encountered while upgrading Microsoft.AspNetCore.Authentication.JwtBearer from 8.0.0-preview.6.23329.11 to 8.0.0-preview.7.23375.9. Prior to this update, my project had a direct reference to System.IdentityModel.Tokens.Jwt version 6.32.1.

Parent commit broken, fixing commit tgstation/tgstation-server@651c810

Opened as a result of this discussion #2092 (reply in thread)

@eerhardt
Copy link
Contributor

Instead of JwtSecurityTokenHandler.DefaultInboundClaimTypeMap.Clear();, try calling JsonWebTokenHandler.DefaultInboundClaimTypeMap.Clear().

With 8.0.0-preview.7.23375.9 of ASP.NET Core, it now uses JsonWebTokenHandler by default and not JwtSecurityTokenHandler.

@Cyberboss
Copy link
Author

That works as well. I opted to set JwtBearerOptions.MapInboundClaims = false, which wasn't an option back when this was written with Microsoft.AspNetCore.Authentication.JwtBearer 2.1.0-preview2-final.

@brentschmaltz
Copy link
Member

Issued solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants