Skip to content

Commit 0d0aa33

Browse files
authored
Remove MapIdentityApi's InfoResponse.Claims property (#51177)
## Description This is a public API change to remove the `InfoResponse.Claims` property which was first made public in RC2. The API was approved last week during API review ([Notes](#50037 (comment))). The API was already broken in common scenarios where a user has multiple roles causing multiple claims with the same type. This cause an `ArgumentException` in the `/manage/info` endpoint when it attempted to add duplicate Key in the now-deleted `public IDictionary<string, string> Claims` property. We had considered trying to fix the `Claims` property by making it an array or a dictionary of arrays, but @mconnew brought up the possibility that developers would not always expect clients to read their own claims. With cookies and JWE tokens claims are encrypted and potentially secret even to the authenticated user. When we consulted @blowdart, he agreed that we should avoid exposing claims from `MapIdentityApi<TUser>()` to be safe even though he hadn't personally seen an example of a claim that needed to be kept secret from an authenticated client.   Fixes #50037 ## Customer Impact Prior to this change, the `/manage/info` endpoint added by the new `MapIdentityApi<TUser>()` method introduced in .NET 8, would return an empty 500 response when the user making the request had a `ClaimsPrincipal` with multiple claims of the same type. This is very common when a user is in multiple roles and was [reported by a customer](#50037) shortly after the `/manage/info` endpoint was added in preview 7. This made the entire endpoint unusable for these users. ## Regression? - [ ] Yes - [x] No [If yes, specify the version the behavior has regressed from] ## Risk - [ ] High - [ ] Medium - [x] Low This is simply removing new API that was first added in .NET 8 preview 7 and made public in RC 2. ## Verification - [x] Manual (required) - [x] Automated ## Packaging changes reviewed? - [ ] Yes - [ ] No - [x] N/A
1 parent eb73b70 commit 0d0aa33

File tree

4 files changed

+42
-66
lines changed

4 files changed

+42
-66
lines changed

src/Identity/Core/src/Data/InfoResponse.cs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.Security.Claims;
5-
using Microsoft.AspNetCore.Http;
64
using Microsoft.AspNetCore.Routing;
75

86
namespace Microsoft.AspNetCore.Identity.Data;
@@ -21,9 +19,4 @@ public sealed class InfoResponse
2119
/// Indicates whether or not the <see cref="Email"/> has been confirmed yet.
2220
/// </summary>
2321
public required bool IsEmailConfirmed { get; init; }
24-
25-
/// <summary>
26-
/// The <see cref="ClaimsPrincipal.Claims"/> from the authenticated <see cref="HttpContext.User"/>.
27-
/// </summary>
28-
public required IDictionary<string, string> Claims { get; init; }
2922
}

src/Identity/Core/src/IdentityApiEndpointRouteBuilderExtensions.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ await signInManager.ValidateSecurityStampAsync(refreshTicket.Principal) is not T
340340
return TypedResults.NotFound();
341341
}
342342

343-
return TypedResults.Ok(await CreateInfoResponseAsync(user, claimsPrincipal, userManager));
343+
return TypedResults.Ok(await CreateInfoResponseAsync(user, userManager));
344344
});
345345

346346
accountGroup.MapPost("/info", async Task<Results<Ok<InfoResponse>, ValidationProblem, NotFound>>
@@ -382,7 +382,7 @@ await signInManager.ValidateSecurityStampAsync(refreshTicket.Principal) is not T
382382
}
383383
}
384384

385-
return TypedResults.Ok(await CreateInfoResponseAsync(user, claimsPrincipal, userManager));
385+
return TypedResults.Ok(await CreateInfoResponseAsync(user, userManager));
386386
});
387387

388388
async Task SendConfirmationEmailAsync(TUser user, UserManager<TUser> userManager, HttpContext context, string email, bool isChange = false)
@@ -452,14 +452,13 @@ private static ValidationProblem CreateValidationProblem(IdentityResult result)
452452
return TypedResults.ValidationProblem(errorDictionary);
453453
}
454454

455-
private static async Task<InfoResponse> CreateInfoResponseAsync<TUser>(TUser user, ClaimsPrincipal claimsPrincipal, UserManager<TUser> userManager)
455+
private static async Task<InfoResponse> CreateInfoResponseAsync<TUser>(TUser user, UserManager<TUser> userManager)
456456
where TUser : class
457457
{
458458
return new()
459459
{
460460
Email = await userManager.GetEmailAsync(user) ?? throw new NotSupportedException("Users must have an email."),
461461
IsEmailConfirmed = await userManager.IsEmailConfirmedAsync(user),
462-
Claims = claimsPrincipal.Claims.ToDictionary(c => c.Type, c => c.Value),
463462
};
464463
}
465464

src/Identity/Core/src/PublicAPI.Unshipped.txt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ Microsoft.AspNetCore.Identity.Data.InfoRequest.NewPassword.init -> void
1212
Microsoft.AspNetCore.Identity.Data.InfoRequest.OldPassword.get -> string?
1313
Microsoft.AspNetCore.Identity.Data.InfoRequest.OldPassword.init -> void
1414
Microsoft.AspNetCore.Identity.Data.InfoResponse
15-
Microsoft.AspNetCore.Identity.Data.InfoResponse.Claims.get -> System.Collections.Generic.IDictionary<string!, string!>!
16-
Microsoft.AspNetCore.Identity.Data.InfoResponse.Claims.init -> void
1715
Microsoft.AspNetCore.Identity.Data.InfoResponse.Email.get -> string!
1816
Microsoft.AspNetCore.Identity.Data.InfoResponse.Email.init -> void
1917
Microsoft.AspNetCore.Identity.Data.InfoResponse.InfoResponse() -> void

src/Identity/test/Identity.FunctionalTests/MapIdentityApiTests.cs

Lines changed: 39 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
using Identity.DefaultUI.WebSite;
1414
using Identity.DefaultUI.WebSite.Data;
1515
using Microsoft.AspNetCore.Builder;
16-
using Microsoft.AspNetCore.Hosting.Server;
1716
using Microsoft.AspNetCore.Http;
1817
using Microsoft.AspNetCore.Identity.EntityFrameworkCore;
1918
using Microsoft.AspNetCore.Identity.UI.Services;
@@ -862,7 +861,7 @@ public async Task CanResetRecoveryCodes()
862861
client.DefaultRequestHeaders.Authorization = new("Bearer", recoveryAccessToken);
863862

864863
var updated2faResponse = await client.PostAsJsonAsync("/identity/manage/2fa", new object());
865-
var updated2faContent = await updated2faResponse.Content.ReadFromJsonAsync<JsonElement>();;
864+
var updated2faContent = await updated2faResponse.Content.ReadFromJsonAsync<JsonElement>();
866865
Assert.Equal(8, updated2faContent.GetProperty("recoveryCodesLeft").GetInt32());
867866
Assert.Null(updated2faContent.GetProperty("recoveryCodes").GetString());
868867

@@ -1013,25 +1012,6 @@ public async Task CanResetPassword()
10131012
AssertOk(await client.PostAsJsonAsync("/identity/login", new { Email = confirmedEmail, Password = newPassword }));
10141013
}
10151014

1016-
[Fact]
1017-
public async Task CanGetClaims()
1018-
{
1019-
await using var app = await CreateAppAsync();
1020-
using var client = app.GetTestClient();
1021-
1022-
await RegisterAsync(client);
1023-
await LoginAsync(client);
1024-
1025-
var infoResponse = await client.GetFromJsonAsync<JsonElement>("/identity/manage/info");
1026-
Assert.Equal(Email, infoResponse.GetProperty("email").GetString());
1027-
1028-
var claims = infoResponse.GetProperty("claims");
1029-
Assert.Equal(Email, claims.GetProperty(ClaimTypes.Name).GetString());
1030-
Assert.Equal(Email, claims.GetProperty(ClaimTypes.Email).GetString());
1031-
Assert.Equal("pwd", claims.GetProperty("amr").GetString());
1032-
Assert.NotNull(claims.GetProperty(ClaimTypes.NameIdentifier).GetString());
1033-
}
1034-
10351015
[Theory]
10361016
[MemberData(nameof(AddIdentityModes))]
10371017
public async Task CanChangeEmail(string addIdentityModes)
@@ -1058,12 +1038,12 @@ public async Task CanChangeEmail(string addIdentityModes)
10581038
Assert.Equal(Email, infoResponse.GetProperty("email").GetString());
10591039
Assert.True(infoResponse.GetProperty("isEmailConfirmed").GetBoolean());
10601040

1061-
var infoClaims = infoResponse.GetProperty("claims");
1062-
Assert.Equal("pwd", infoClaims.GetProperty("amr").GetString());
1063-
Assert.Equal(Email, infoClaims.GetProperty(ClaimTypes.Name).GetString());
1064-
Assert.Equal(Email, infoClaims.GetProperty(ClaimTypes.Email).GetString());
1041+
var infoClaims = await client.GetFromJsonAsync<JsonElement>("/auth/claims");
1042+
Assert.Equal("pwd", GetSingleClaim(infoClaims, "amr"));
1043+
Assert.Equal(Email, GetSingleClaim(infoClaims, ClaimTypes.Name));
1044+
Assert.Equal(Email, GetSingleClaim(infoClaims, ClaimTypes.Email));
10651045

1066-
var originalNameIdentifier = infoResponse.GetProperty("claims").GetProperty(ClaimTypes.NameIdentifier).GetString();
1046+
var originalNameIdentifier = GetSingleClaim(infoClaims, ClaimTypes.NameIdentifier);
10671047
var newEmail = $"New-{Email}";
10681048

10691049
// The email must pass DataAnnotations validation by EmailAddressAttribute.
@@ -1077,10 +1057,10 @@ public async Task CanChangeEmail(string addIdentityModes)
10771057
Assert.True(infoPostContent.GetProperty("isEmailConfirmed").GetBoolean());
10781058

10791059
// And none of the claims have yet been updated.
1080-
var infoPostClaims = infoPostContent.GetProperty("claims");
1081-
Assert.Equal(Email, infoPostClaims.GetProperty(ClaimTypes.Name).GetString());
1082-
Assert.Equal(Email, infoPostClaims.GetProperty(ClaimTypes.Email).GetString());
1083-
Assert.Equal(originalNameIdentifier, infoClaims.GetProperty(ClaimTypes.NameIdentifier).GetString());
1060+
var infoPostClaims = await client.GetFromJsonAsync<JsonElement>("/auth/claims");
1061+
Assert.Equal(Email, GetSingleClaim(infoPostClaims, ClaimTypes.Name));
1062+
Assert.Equal(Email, GetSingleClaim(infoPostClaims, ClaimTypes.Email));
1063+
Assert.Equal(originalNameIdentifier, GetSingleClaim(infoPostClaims, ClaimTypes.NameIdentifier));
10841064

10851065
// We cannot log in with the new email until we confirm the email change.
10861066
await AssertProblemAsync(await client.PostAsJsonAsync("/identity/login", new { Email = newEmail, Password }),
@@ -1103,10 +1083,10 @@ public async Task CanChangeEmail(string addIdentityModes)
11031083
Assert.Equal(newEmail, infoAfterEmailChange.GetProperty("email").GetString());
11041084

11051085
// The email still won't be available as a claim until we get a new token.
1106-
var claimsAfterEmailChange = infoAfterEmailChange.GetProperty("claims");
1107-
Assert.Equal(Email, claimsAfterEmailChange.GetProperty(ClaimTypes.Name).GetString());
1108-
Assert.Equal(Email, claimsAfterEmailChange.GetProperty(ClaimTypes.Email).GetString());
1109-
Assert.Equal(originalNameIdentifier, infoClaims.GetProperty(ClaimTypes.NameIdentifier).GetString());
1086+
var claimsAfterEmailChange = await client.GetFromJsonAsync<JsonElement>("/auth/claims");
1087+
Assert.Equal(Email, GetSingleClaim(claimsAfterEmailChange, ClaimTypes.Name));
1088+
Assert.Equal(Email, GetSingleClaim(claimsAfterEmailChange, ClaimTypes.Email));
1089+
Assert.Equal(originalNameIdentifier, GetSingleClaim(claimsAfterEmailChange, ClaimTypes.NameIdentifier));
11101090

11111091
// And now the email has changed, the refresh token is invalidated by the security stamp.
11121092
AssertUnauthorizedAndEmpty(await client.PostAsJsonAsync("/identity/refresh", new { RefreshToken = originalRefreshToken }));
@@ -1118,10 +1098,10 @@ public async Task CanChangeEmail(string addIdentityModes)
11181098
Assert.Equal(newEmail, infoAfterFinalLogin.GetProperty("email").GetString());
11191099
Assert.True(infoAfterFinalLogin.GetProperty("isEmailConfirmed").GetBoolean());
11201100

1121-
var claimsAfterFinalLogin = infoAfterFinalLogin.GetProperty("claims");
1122-
Assert.Equal(newEmail, claimsAfterFinalLogin.GetProperty(ClaimTypes.Name).GetString());
1123-
Assert.Equal(newEmail, claimsAfterFinalLogin.GetProperty(ClaimTypes.Email).GetString());
1124-
Assert.Equal(originalNameIdentifier, infoClaims.GetProperty(ClaimTypes.NameIdentifier).GetString());
1101+
var claimsAfterFinalLogin = await client.GetFromJsonAsync<JsonElement>("/auth/claims");
1102+
Assert.Equal(newEmail, GetSingleClaim(claimsAfterFinalLogin, ClaimTypes.Name));
1103+
Assert.Equal(newEmail, GetSingleClaim(claimsAfterFinalLogin, ClaimTypes.Email));
1104+
Assert.Equal(originalNameIdentifier, GetSingleClaim(claimsAfterFinalLogin, ClaimTypes.NameIdentifier));
11251105
}
11261106

11271107
[Fact]
@@ -1152,12 +1132,13 @@ public async Task CannotUpdateClaimsDuringInfoPostWithCookies()
11521132

11531133
var infoResponse = await client.GetFromJsonAsync<JsonElement>("/identity/manage/info");
11541134
Assert.Equal(Email, infoResponse.GetProperty("email").GetString());
1155-
var infoClaims = infoResponse.GetProperty("claims");
1156-
Assert.Equal("pwd", infoClaims.GetProperty("amr").GetString());
1157-
Assert.Equal(Email, infoClaims.GetProperty(ClaimTypes.Name).GetString());
1158-
Assert.Equal(Email, infoClaims.GetProperty(ClaimTypes.Email).GetString());
11591135

1160-
var originalNameIdentifier = infoResponse.GetProperty("claims").GetProperty(ClaimTypes.NameIdentifier).GetString();
1136+
var infoClaims = await client.GetFromJsonAsync<JsonElement>("/auth/claims");
1137+
Assert.Equal("pwd", GetSingleClaim(infoClaims, "amr"));
1138+
Assert.Equal(Email, GetSingleClaim(infoClaims, ClaimTypes.Name));
1139+
Assert.Equal(Email, GetSingleClaim(infoClaims, ClaimTypes.Email));
1140+
1141+
var originalNameIdentifier = GetSingleClaim(infoClaims, ClaimTypes.NameIdentifier);
11611142
var newEmail = $"NewEmailPrefix-{Email}";
11621143

11631144
var infoPostResponse = await client.PostAsJsonAsync("/identity/manage/info", new { newEmail });
@@ -1169,9 +1150,9 @@ public async Task CannotUpdateClaimsDuringInfoPostWithCookies()
11691150
Assert.Equal(Email, infoPostContent.GetProperty("email").GetString());
11701151

11711152
// The claims have not been updated to match.
1172-
var infoPostClaims = infoPostContent.GetProperty("claims");
1173-
Assert.Equal(Email, infoPostClaims.GetProperty(ClaimTypes.Email).GetString());
1174-
Assert.Equal(originalNameIdentifier, infoClaims.GetProperty(ClaimTypes.NameIdentifier).GetString());
1153+
var infoPostClaims = await client.GetFromJsonAsync<JsonElement>("/auth/claims");
1154+
Assert.Equal(Email, GetSingleClaim(infoPostClaims, ClaimTypes.Email));
1155+
Assert.Equal(originalNameIdentifier, GetSingleClaim(infoPostClaims, ClaimTypes.NameIdentifier));
11751156

11761157
// Two emails have now been sent. The first was sent during registration. And the second for the email change.
11771158
Assert.Equal(2, emailSender.Emails.Count);
@@ -1191,9 +1172,9 @@ public async Task CannotUpdateClaimsDuringInfoPostWithCookies()
11911172
Assert.Equal(newEmail, infoAfterEmailChange.GetProperty("email").GetString());
11921173

11931174
// The email still won't be available as a claim until we get a new cookie.
1194-
var claimsAfterEmailChange = infoAfterEmailChange.GetProperty("claims");
1195-
Assert.Equal(Email, claimsAfterEmailChange.GetProperty(ClaimTypes.Email).GetString());
1196-
Assert.Equal(originalNameIdentifier, infoClaims.GetProperty(ClaimTypes.NameIdentifier).GetString());
1175+
var claimsAfterEmailChange = await client.GetFromJsonAsync<JsonElement>("/auth/claims");
1176+
Assert.Equal(Email, GetSingleClaim(claimsAfterEmailChange, ClaimTypes.Email));
1177+
Assert.Equal(originalNameIdentifier, GetSingleClaim(claimsAfterEmailChange, ClaimTypes.NameIdentifier));
11971178

11981179
// We will finally see all the claims updated after logging in again.
11991180
var secondLoginResponse = await client.PostAsJsonAsync("/identity/login?useCookies=true", new { Email = newEmail, Password });
@@ -1202,10 +1183,10 @@ public async Task CannotUpdateClaimsDuringInfoPostWithCookies()
12021183
var infoAfterFinalLogin = await client.GetFromJsonAsync<JsonElement>("/identity/manage/info");
12031184
Assert.Equal(newEmail, infoAfterFinalLogin.GetProperty("email").GetString());
12041185

1205-
var claimsAfterFinalLogin = infoAfterFinalLogin.GetProperty("claims");
1206-
Assert.Equal(newEmail, claimsAfterFinalLogin.GetProperty(ClaimTypes.Name).GetString());
1207-
Assert.Equal(newEmail, claimsAfterFinalLogin.GetProperty(ClaimTypes.Email).GetString());
1208-
Assert.Equal(originalNameIdentifier, infoClaims.GetProperty(ClaimTypes.NameIdentifier).GetString());
1186+
var claimsAfterFinalLogin = await client.GetFromJsonAsync<JsonElement>("/auth/claims");
1187+
Assert.Equal(newEmail, GetSingleClaim(claimsAfterFinalLogin, ClaimTypes.Name));
1188+
Assert.Equal(newEmail, GetSingleClaim(claimsAfterFinalLogin, ClaimTypes.Email));
1189+
Assert.Equal(originalNameIdentifier, GetSingleClaim(claimsAfterFinalLogin, ClaimTypes.NameIdentifier));
12091190
}
12101191

12111192
[Fact]
@@ -1321,6 +1302,8 @@ private async Task<WebApplication> CreateAppAsync<TUser, TContext>(Action<IServi
13211302
authGroup.MapGet("/hello",
13221303
(ClaimsPrincipal user) => $"Hello, {user.Identity?.Name}!");
13231304

1305+
authGroup.MapGet("/claims", (ClaimsPrincipal user) => user.Claims.Select(c => new { c.Type, c.Value }));
1306+
13241307
await dbConnection.OpenAsync();
13251308
await app.Services.GetRequiredService<TContext>().Database.EnsureCreatedAsync();
13261309

@@ -1367,6 +1350,9 @@ private Task<WebApplication> CreateAppAsync(Action<IServiceCollection>? configur
13671350

13681351
public static object[][] AddIdentityModes => AddIdentityActions.Keys.Select(key => new object[] { key }).ToArray();
13691352

1353+
private static string? GetSingleClaim(JsonElement claims, string name)
1354+
=> claims.EnumerateArray().Single(e => e.GetProperty("type").GetString() == name).GetProperty("value").GetString();
1355+
13701356
private static string GetEmailConfirmationLink(TestEmail email)
13711357
{
13721358
// Update if we add more links to the email.

0 commit comments

Comments
 (0)