Closed pingzing closed 7 years ago
Hi @pingzing, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.
TTYL, MSBOT;
Hi, I am closing and re-opening this PR to bump the CLA bot. Sorry for the inconvenience!
Hi @pingzing, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! You've already signed the contribution license agreement. Thanks! We will now validate the agreement and then real humans will evaluate your PR.
TTYL, MSBOT;
Thanks Neil! We'll take a look. I'm assigning @daboxu who owns our .NET SDKs.
This is an unfortunately-large commit to fix a small issue. Additionally, it's not ready for merge (I'll explain why below, just wanted to issue that warning up front).
The WebAuthenticationBroker.AuthenticateAsync() methods aren't implemented on Windows 10 IoT Core. This library relies on WebAuthenticatorBroker to display authentication UI, and so attempting to use it on IoT Core causes it to fail far beyond any consumer's ability to handle.
The fix was rather simple--check to see if we're running on Windows IoT core, and if so, provide our own UI (as the library does on Desktop) instead of relying on WebAuthenticatorBroker.
The basic summary of the changes is as follows:
AnalyticsInfo.VersionInfo.DeviceFamily
yields "Windows.Universal" to see if we're running on IoT Core (If there's a more robust way, I'd love to know). If we are, it runs down the new code path using our custom ContentDialog. If not, it uses WebAuthenticationBroker as usual.As mentioned above, this isn't quite ready for merging yet--I was uncertain how to obtain the assembly public keys for use in the various
[InternalsVisibleTo]
attributes necessary in the Release version of the assemblies. If someone can advise on how to obtain those, I'll happily include them, and then this should be merge-able.