CommunityToolkit / Graph-Controls

Set of Helpers and Controls for Windows development using the Microsoft Graph.
https://docs.microsoft.com/en-us/windows/communitytoolkit/graph/overview
Other
154 stars 39 forks source link

MsalProvider.cs ctors convert scope strings to lowercase but should not do that! #185

Open tnsturm opened 2 years ago

tnsturm commented 2 years ago

Toolkit Version 7.1.1

MsalProvider.cs Contructors convert the scope arrays to lowercase:

Scopes = scopes.Select(s => s.ToLower()).ToArray() ?? new string[] { string.Empty };

It should not do this because OpenID specs define scopes as CASE SENSITIVE. https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims "Multiple scope values MAY be used by creating a space delimited, case sensitive list of ASCII scope values."

https://tools.ietf.org/html/rfc6749#section-3.3 "The value of the scope parameter is expressed as a list of space-delimited, case-sensitive strings. The strings are defined by the authorization server. If the value contains multiple space-delimited strings, their order does not matter, and each string adds an additional access range to the requested scope."

I'm running an .NET Core 6 Azure Web App Api with an case sensitive scope in the AAD app registration. Using standard MSAL PublicClientApplication runs fine with that. Using the MsalProvider wrapper of the community toolkit breaks this, because the "lowercased" scope is not accepted by Microsoft.IdentityModel.Tokens.SecurityToken:

IDX10214: Audience validation failed. Audiences: 'https://abce.de/webapp-abcd.azurewebsites.net'. Did not match: validationParameters.ValidAudience: 'https://abce.de/WebApp-abce.azurewebsites.net'

Please not the uppercase 'W' in the scope.

Earlier versions of MSAL also had that bug and fixed it: https://github.com/AzureAD/microsoft-authentication-library-for-objc/issues/395 https://github.com/AzureAD/microsoft-authentication-library-for-js/issues/1922

Best Regards, Torsten

ghost commented 2 years ago

Hello tnsturm, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌