AzureAD / microsoft-authentication-library-for-dotnet

Microsoft Authentication Library (MSAL) for .NET
https://aka.ms/msal-net
MIT License
1.36k stars 337 forks source link

AcquireTokenForManagedIdentity should throw if a user-assigned managed identity clientID or resourceID is supplied when not supported #4837

Closed christothes closed 3 weeks ago

christothes commented 3 weeks ago

The following ManagedIdentitySources do not currently support user-assigned managed identities:

AzureArc, CloudShell, ServiceFabric

If a user-assigned clientId or resourceId is specified for these sources, we should throw with a message similar to:

https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/5c7c527b173b5f2a719926fb89ccd68bc55d1b3e/src/client/Microsoft.Identity.Client/MsalErrorMessage.cs#L424

This behavior should be consistent cross-language.

bgavrilMS commented 3 weeks ago

Thanks @christothes , I opened separate bugs for the 3 other MSALs.

neha-bhargava commented 3 weeks ago

@christothes In MSAL .Net we throw the exception for Cloud Shell and Azure Arc. But the service fabric does support user assigned managed identity as documented in the public docs. See:

rayluo commented 3 weeks ago

Documenting the cross-team discussion outcome here.

We acknowledged that (1) Azure Identity's design interprets the absence of ClientId/ResourceId as either system-assigned managed identity OR default (which could be system-assigned or user-assigned). (2) MSAL's design asks the app developer to explicitly declare upfront that whether they intend to use a SAMI or a UAMI, even though in some cases (Service Fabric) we don't actually have a way to validate their input.

We reached a reasonable compromise that MSAL and Azure Identity both keep their existing behaviors, and Azure Identity can implement this logic, "if the input contains some sort of ID and MSAL's GetManagedIdentitySource() returns ServiceFabric, then throw an exception".

bgavrilMS commented 3 weeks ago

@rayluo - can we close the Java, Py and Node JS associated issues too? See links to them at the start of this thread.

christothes commented 3 weeks ago

@rayluo What about the other sources besides SF that do not support user-assigned identities?

rayluo commented 2 weeks ago

@rayluo What about the other sources besides SF that do not support user-assigned identities?

Providing a full explanation here. Feel free to reopen the respective issue(s) if a discrepancy would be observed.