microsoft-identity-web
microsoft-identity-web copied to clipboard
[Bug] 'ClaimsPrincipalFactory.FromTenantIdAndObjectId' populates incorrect ObjectIdentifier claim
Which version of Microsoft Identity Web are you using? Microsoft.Identity.Web v1.23.1
Where is the issue?
- Web app
- [ ] Sign-in users
- [ ] Sign-in users and call web APIs
- Web API
- [ ] Protected web APIs (validating tokens)
- [ ] Protected web APIs (validating scopes)
- [ ] Protected web APIs call downstream web APIs
- Token cache serialization
- [ ] In-memory caches
- [ ] Session caches
- [ ] Distributed caches
- Other (please describe)
- [x] Helper methods for instantiating
ClaimsPrincipal
objects
- [x] Helper methods for instantiating
Is this a new or an existing app? This is a new app or an experiment.
Repro
var principal = ClaimsPrincipalFactory.FromTenantIdAndObjectId(tenantId: "someTenant", objectId: "someObject");
var objectId = principal.GetObjectId();
Assert.Equal("someObject", objectId);
Expected behavior
The ClaimsPrincipalFactory.FromTenantIdAndObjectId
method should populate the correct ObjectIdentifier claim (oid
) when building the principal. It populates the uid
claim, however.
Actual behavior
ClaimsPrincipalFactory.FromTenantIdAndObjectId
populates the uid
claim for the object identifier, which is incorrect: it should be using the oid
claim instead.
Possible solution https://github.com/AzureAD/microsoft-identity-web/blob/bba91c4298411e780c9f082e8e3843eacd8e543c/src/Microsoft.Identity.Web/ClaimsPrincipalFactory.cs#L44
Use ClaimConstants.Oid
or ClaimConstants.ObjectId
instead.
Additional context / logs / screenshots / link to code
https://github.com/AzureAD/microsoft-identity-web/blob/bba91c4298411e780c9f082e8e3843eacd8e543c/src/Microsoft.Identity.Web/ClaimsPrincipalFactory.cs#L38-L46
@julealgon We need to give the home tenant and home object ID, especially important for guest accounts. The name of the method is misleading but it's more of an implementation detail with the claims.
The name of the method is misleading
In that case, I'd say both the method and argument name are misleading.
Wouldn't it make sense to rename these to avoid this confusion in the future?
@julealgon Can you give more details on your use case/scenario? What do you do with these claims? Is it related to using MSAL?
@julealgon Can you give more details on your use case/scenario? What do you do with these claims? Is it related to using MSAL?
I actually wanted to use that factory in my tests for tests that rely on resource-based authorization and thus push claims objects down into the implementation.
Released in 2.5.0