Azure-Samples / active-directory-b2c-xamarin-native

This is a simple Xamarin Forms app showcasing how to use MSAL to authenticate users via Azure Active Directory B2C, and access a Web API with the resulting tokens.
http://aka.ms/aadb2c
MIT License
111 stars 65 forks source link

Assumption that policy name does not contain '.' #152

Closed endintiers closed 3 years ago

endintiers commented 3 years ago

In: https://github.com/Azure-Samples/active-directory-b2c-xamarin-native/blob/master/UserDetailsClient/UserDetailsClient.Core/Features/LogOn/B2CAuthenticationService.cs At line 129: string userIdentifier = account.HomeAccountId.ObjectId.Split('.')[0];

This assumes that the ObjectId, which is {policyName}.{tenantId} does not contain the character '.' except between the policy name and the tenant id.

If a policy/flow name contains '.', say "B2C_xyzdev.signupsignin" this code will split in 3 parts, not 2 and the userIdentifier would contain "B2C_xyzdev" rather than B2C_xyzdev.signupsignin, so that the valid account would never be found.

After fixing this the returned account still isn't valid, so I wonder if a similar assumption is made in the actual Client library? I'll check that out and report there if I find anything.

jmprieur commented 3 years ago

@jennyf19 FYI

endintiers commented 3 years ago

I have been reading the Microsoft.Identity.Client code. All through there, when dealing with Accounts, there are things like: string[] elements = str.Split('.'); And then the code assumes that there will be 2 and only 2 elements (or sometimes 1 for ADFS).

So probably the easy solution is to document: "Don't use '.' in your B2C flow names"

Maybe the sample could check for "too many '.'s" in the ObjectId as a backup?

endintiers commented 3 years ago

I mentioned it here as well: https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/2444

jmprieur commented 3 years ago

Thanks @endintiers

jennyf19 commented 3 years ago

@endintiers this has been fixed a few releases ago 4.32. should be working now. please reopen if you still have issues.