This repository has been archived by the owner. It is now read-only.

UseJwtBearerAuthentication and UserManager disagree one where the user ID claim is stored #1043

Closed
mjrousos opened this Issue Nov 18, 2016 · 8 comments

Comments

Projects
None yet
6 participants
@mjrousos
Copy link
Member

mjrousos commented Nov 18, 2016

I'm trying to use JwtBearerMiddleware (from Microsoft.AspNetCore.Authentication.JwtBearer) to authenticate with a bearer token. I then use UserManager.GetUserAsync(HttpContext.User) in one of my controller actions to map the user from the token back into an ASP.NET Core Identity user.

I'm running into issues, though, because those two components (JwtBearerMiddleware and UserManager) don't agree on where the subject/ID claim from a token should be stored.

UserManager.GetUserAsync calls GetUserId to extract the user ID from claims so that it can lookup the user in the user store. That method (GetUserId) expects the ID to be present as a sub claim type, by default.

This all makes sense since my JWT does include ID as a sub claim (my JWT is pasted below, for reference).

The problem is that when creating a ClaimsPrincipal from my JWT, JwtBearerMiddleware is storing the sub claim as a NameIdentifier claim. Because of this, the UserManager is unable to find the user's ID and GetUserAsync fails.

It would help if JwtBearerMiddleware had a way to indicate what claim type the id/sub should have in the produced ClaimsPrincipal, but I don't see any such option. When registering ASP.NET Core Identity services via services.AddIdentity there is an option to specify options.ClaimsIdentity.UserIdClaimType but that option isn't flowing to my UserManager; I haven't dug into why yet.

In the end, I worked around the issue by adding custom middleware that sets a ClaimPrincipal's sub claim to be equal to its NameIdentifier claim if a NameIdentifier is present but a sub isn't. This shouldn't be necessary, though.

Here's how I'm registering Identity:

            services.AddIdentity<ApplicationUser, IdentityRole>(options => {
                    // This is ignored by UserManagers created via dependency injection
                    options.ClaimsIdentity.UserIdClaimType = ClaimTypes.NameIdentifier;
                })
                .AddEntityFrameworkStores<ApplicationDbContext>()
                .AddDefaultTokenProviders();

Here is how I add the JwtBearerMiddleware to the HTTP processing pipeline:

            app.UseIdentity();

            var authority = "http://localhost:5000";
            app.UseJwtBearerAuthentication(new JwtBearerOptions
            {
                Authority = authority,
                Audience = $"{authority}/resources",
                RequireHttpsMetadata = false,
            });

Here is the UserManager usage in my controller:

        [Authorize]
        [HttpGet]
        public async Task<IEnumerable<DeviceDTO>> Index()
        {
            var user = await GetCurrentUserAsync();
            if (!string.IsNullOrEmpty(user?.Id))
            {
                var devices = DBContext.Devices.Where(d => d.Owner.Id == user.Id).ToArray();
                return devices.Select(d => new DeviceDTO(d));
            }
            return Enumerable.Empty<DeviceDTO>();
        }

And here is my JWT token:

eyJhbGciOiJSUzI1NiIsImtpZCI6IjY3MUE0N0NFNjVFMTBBOThCQjg2RURDRDVGOTY4NEU5RDA0OEZBRTkiLCJ0eXAiOiJKV1QiLCJ4NXQiOiJaeHBIem1YaENwaTdodTNOWDVhRTZkQkktdWsifQ.eyJuYmYiOjE0Nzk1MDA5NTQsImV4cCI6MTQ4MDEwNTc1NCwiaXNzIjoiaHR0cDovL2xvY2FsaG9zdDo1MDAwIiwiYXVkIjoiaHR0cDovL2xvY2FsaG9zdDo1MDAwL3Jlc291cmNlcyIsImNsaWVudF9pZCI6ImpzIiwic3ViIjoiY2YxMTIzOTQtMzlhZi00ZjU1LWE3ZTktZmZjMGE5NWFmZWE0IiwiYXV0aF90aW1lIjoxNDc5NTAwOTUzLCJpZHAiOiJsb2NhbCIsIm5hbWUiOiJNaWtlIiwicm9sZSI6IkFkbWluaXN0cmF0b3IiLCJzY29wZSI6WyJkZXZpY2VPd25lciIsInByb2ZpbGUiLCJyb2xlcyJdLCJhbXIiOlsicHdkIl19.qWMA2GjyP3UYSuNkXkFIQIkqvhkFSNsKesbKs4EjfATxg4_61Tb87wqEw7Kzow5UKrQXcRrK6IwSexmgdYVinC5uy_9XybfvV9YVl4spuH6S001-mGSelexqZEMyVQ-EYxVun9gTn3aihbH8hjUo8kI227QKtW1vMHnvC7w1Ivrt5dEQ05xKgXu_arVLbKFY2FCfVqLXJue0ad5k-vfM_mFiP09EgVBKqQqcBKlN3GDIQI3k7dWVgUbJvn8hDDJqwapmKtS4nAknpKCfbuPp7vFribgurc4mRIyV8dQWtdnaHSuqSA2QRU3-KVCH17Ud24dym9vKkvXDFq2xWMXKfA

And here it is translated:

{
 alg: "RS256",
 kid: "671A47CE65E10A98BB86EDCD5F9684E9D048FAE9",
 typ: "JWT",
 x5t: "ZxpHzmXhCpi7hu3NX5aE6dBI-uk"
},
{
 nbf: 1479500954,
 exp: 1480105754,
 iss: "http://localhost:5000",
 aud: "http://localhost:5000/resources",
 client_id: "js",
 sub: "cf112394-39af-4f55-a7e9-ffc0a95afea4",
 auth_time: 1479500953,
 idp: "local",
 name: "Mike",
 role: "Administrator",
 scope: [
  "deviceOwner",
  "profile",
  "roles"
 ],
 amr: [
  "pwd"
 ]
}

Here is the workaround I used to unblock myself which I think shouldn't be necessary:

app.Use(async (context, next) =>
{
    if (context.User.HasClaim(c => c.Type == ClaimTypes.NameIdentifier) && !context.User.HasClaim(c => c.Type == "sub"))
    {
        var newIdentity = ((ClaimsIdentity)context.User.Identity);
        newIdentity.AddClaim(new Claim("sub", context.User.FindFirst(ClaimTypes.NameIdentifier).Value));
        context.User = new ClaimsPrincipal(newIdentity);
    }
    await next();
});
@brockallen

This comment has been minimized.

Copy link

brockallen commented Nov 21, 2016

The JWT bearer middleware is converting the JWT claim types into the antiquated SOAP claim types (you can thank the AAD team for that one). You can disable this by adding this one line of code to your startup code (somewhere):

JwtSecurityTokenHandler.DefaultInboundClaimTypeMap.Clear();
@mjrousos

This comment has been minimized.

Copy link
Member Author

mjrousos commented Nov 22, 2016

Thanks, @brockallen, very helpful! It looks like DefaultInboundClaimTypeMap changes need to be made before UseIdentity() is called. After doing that, my app works as expected.

The claim type mapping of JwtSecurityTokenHandler is not very well documented or discoverable. This seems like something that should be included in ASP.NET Core security/identity docs.

@brockallen

This comment has been minimized.

Copy link

brockallen commented Nov 22, 2016

The claim type mapping of JwtSecurityTokenHandler is not very well documented or discoverable. This seems like something that should be included in ASP.NET Core security/identity docs.

+1, or they should just not do the mapping by default, but that's an uphill battle

@Eilon

This comment has been minimized.

Copy link
Member

Eilon commented Nov 22, 2016

@brentschmaltz can you open a new bug in the AAD repo and we'll close this?

@Tratcher

This comment has been minimized.

Copy link
Member

Tratcher commented Dec 8, 2016

@brentschmaltz

This comment has been minimized.

Copy link
Contributor

brentschmaltz commented Dec 15, 2016

HelloKitty added a commit to HaloLive/HaloLive.Library that referenced this issue Jun 25, 2017

Add JwtSecurityTokenHandler claim map hack
There is an issue where JWT was mapping sub to nameidentifier. This
caused issues with Identity. See:
aspnet/Security#1043
@HelloKitty

This comment has been minimized.

Copy link

HelloKitty commented May 9, 2018

It has been a long time and many changes from 1.x to 2.0. Is it still needed to clear the JwtSecurityTokenHandler in 2.0?

edit: They may or may not be. Something is wrong, I'm probably doing something wrong, but using [Authorize(AuthenticationSchemes = JwtBearerDefaults.AuthenticationScheme)] seems to remedy whatever issue I was now encountering. Though I'm unsure is this is related to the original issue.

@mjrousos

This comment has been minimized.

Copy link
Member Author

mjrousos commented May 10, 2018

Regarding the original issue, JwtBearer authentication (from Microsoft.AspNetCore.Authentication.JwtBearer) still maps the sub claim to http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier in 2.1 and @brockallen's work around (JwtSecurityTokenHandler.DefaultInboundClaimTypeMap.Clear()) still appears to work if you need a sub claim instead of a SOAP nameidentifier one.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.