[Bug] 'ClaimsPrincipalFactory.FromTenantIdAndObjectId' populates incorrect ObjectIdentifier claim

[julealgon]

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

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

[jennyf19]

@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.

[julealgon]

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?

[jennyf19]

@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]

@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.

[jennyf19]

Released in 2.5.0