Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebAuthenticationBroker.AuthenticateAsync throws COMException #398

Closed
wbokkers opened this issue Nov 24, 2020 · 22 comments
Closed

WebAuthenticationBroker.AuthenticateAsync throws COMException #398

wbokkers opened this issue Nov 24, 2020 · 22 comments
Assignees
Labels
area-WebView bug Something isn't working

Comments

@wbokkers
Copy link

Describe the bug
On WinUI 3 preview 3 for Desktop, when calling WebAuthenticationBroker.AuthenticateAsync, I get the following exception:
System.Runtime.InteropServices.COMException (0x800706BD): There are no remote procedure calls active on this thread. (0x800706BD) at WinRT.ExceptionHelpers.ThrowExceptionForHR(Int32 hr)
at ABI.Windows.Security.Authentication.Web.IWebAuthenticationBrokerStatics.global::Windows.Security.Authentication.Web.IWebAuthenticationBrokerStatics.AuthenticateAsync(WebAuthenticationOptions options, Uri requestUri, Uri callbackUri)
at Windows.Security.Authentication.Web.WebAuthenticationBroker.AuthenticateAsync(WebAuthenticationOptions options, Uri requestUri, Uri callbackUri)

This code previously worked in a UWP application without problems.

Steps to reproduce the bug

  1. Call WebAuthenticationBroker.AuthenticateAsync

Example:
var result = await WebAuthenticationBroker.AuthenticateAsync(
WebAuthenticationOptions.None,
authorizeUri, redirectUri);

Expected behavior
I would expect the Microsoft authentication dialog to appear.

Version Info
NuGet package version:
[Microsoft.WinUI 3.0.0-preview3.201113.0]

Windows app type:

UWP Win32
No (=works fine) Yes (=exception)
Windows 10 version Saw the problem?
May 2020 Update (19041) Yes
Device form factor Saw the problem?
Desktop Yes
@stevenbrix
Copy link

stevenbrix commented Jan 26, 2021

Adding some folks who may know if this API works in Win32 or not - @MikeHillberg @jonwis @oldnewthing

@stevenbrix stevenbrix transferred this issue from microsoft/microsoft-ui-xaml Jan 26, 2021
@ghost ghost added the needs-triage label Jan 26, 2021
@BenJKuhn
Copy link
Member

BenJKuhn commented Feb 8, 2021

@wbokkers, thank you for bringing this to our attention. The best way you can help us is to file this in the feedback app. Once you've done that, please post the link here and I'll promote it to the right team for diagnosing & fixing. By filing in the feedback tool and collecting logs, you can help us better diagnose the issue.

Thanks,

Ben

@BenJKuhn BenJKuhn self-assigned this Feb 8, 2021
@riverar
Copy link
Contributor

riverar commented Feb 8, 2021

@BenJKuhn When you suggest folks file bugs in Feedback Hub, please provide the category and sub-category, as the likelihood we get it right (and the right on-demand diagnostics get executed) is very low.

@BenJKuhn
Copy link
Member

BenJKuhn commented Feb 8, 2021

@riverar, can you file that feedback in feedback hub (jk!).

In this case it's not going to matter much. I'm going to promote it in our database and directly assign it to the right team as soon as I get a link to the feedback. I understand that it's not always easy to map some features to feedback hub categories.

In this case, "Developer platform" -> "API feedback" would be appropriate.

Ben

@riverar
Copy link
Contributor

riverar commented Feb 8, 2021

@BenJKuhn Fair.

If I remember correctly, that API still requires a CoreWindow.

Steps to reproduce

  1. Install Windows 10 19042
  2. Install Visual Studio 2019 Preview, with payloads indicated in the documentation (https://docs.microsoft.com/en-us/windows/apps/winui/winui3/#install-winui-3-preview-3)
  3. Install WinUI 3 Preview 3 templates (https://aka.ms/winui3/preview3-download)
  4. Launch Visual Studio 2019 Preview
  5. Create a new app, using the WinUI 3 Preview 3 Blank App, Packaged (WinUI in Desktop) template
  6. Change the inbox myButton_Click handler to be:
    private async void myButton_Click(object sender, RoutedEventArgs e)
    {
        await WebAuthenticationBroker.AuthenticateAsync(WebAuthenticationOptions.None, new Uri("https://example.org"), new Uri("https://example.org"));
    }
  7. Click Build > Build Solution
  8. Deploy and run the solution
  9. Observe reported error

Headscratcher corner

So taking a look at the "Developer platform" > "API Feedback" category/sub-category, I don't see any associated user-initiated feedback diagnostics. So that means there won't be any specific logs collected for this issue.

@riverar
Copy link
Contributor

riverar commented Feb 8, 2021

Filed feedback on behalf of @wbokkers to take advantage of @BenJKuhn's attention today
https://aka.ms/AAb2zbo

@jonwis
Copy link
Member

jonwis commented Feb 8, 2021

If I remember correctly, that API still requires a CoreWindow.

Chatting with the team that owns WebAuthenticationBroker that's likely the concern. The "Broker" was necessary in a UWP so the user could enter their credentials into a Window displayed by the platform, rather than into one under the app's control. The Window was inferred from the thread calling RequestAsync (recall that a CoreWindow lives on a specific thread that it owns) so the broker knew where to show itself on screen and other attribution information.

We're discussing options for how to support people who were building UWPs with CoreWindow but are now investigating Project Reunion WinUI3 apps on Desktop Bridge. This category of problem - APIs that implicitly or explicitly need a CoreWindow - is high on our list to address (hi @AdamBraden !).

Question for the thread - would you want a 1:1 replacement of WebAuthenticationBroker in Project Reunion that launched the browser for authentication then redirected back to the app? Or would you just want a Really Good Sample of how to register a protocol for your Win32 app and then use it from your authentication experience?

@BenJKuhn
Copy link
Member

BenJKuhn commented Feb 8, 2021

For reference by OS team, the Windows work item number is 31649739. I left a back-pointer in the workitem to here as well.

@riverar, for what it's worth, even the general diagnostics provide a number of ETW data providers that can be helpful in debugging API failures. It's not as nice as having a scenario-tailored log, but it's not an empty data set either.

@riverar
Copy link
Contributor

riverar commented Feb 8, 2021

Question for the thread - would you want a 1:1 replacement of WebAuthenticationBroker in Project Reunion that launched the browser for authentication then redirected back to the app? Or would you just want a Really Good Sample of how to register a protocol for your Win32 app and then use it from your authentication experience?

I would prefer for Microsoft to continue owning that experience. Writing all that OAuth2 goop correctly can be tough and presents quite a hurdle for developers wishing to just access some token-required services. Having a single implementation would also maintain the consistency WebAuthenticationBroker sought to provide.

Might be useful to bounce ideas off the active Microsoft Authentication Library folks as well. (psst @mahoekst)

@BenJKuhn
Copy link
Member

This is also a duplicate of issue #366. This is a known gap in platform support that is being looked at and tracked as an OS bug. A potential outcome of that might be that we consider addressing it as a new API in reunion. There are pro's and con's to both approaches. The specific crash in the specific API wouldn't be fixed as a Project Reunion bug though.

@riverar
Copy link
Contributor

riverar commented Feb 11, 2021

@BenJKuhn Given Reunion's goal of unifying access to existing Win32 and UWP APIs, it seems more appropriate to leave this (or the original) issue open, with a reference to the OS bug number, to track its progress.

Or perhaps this should be transferred to the Discussions area, as an Idea?

@BenJKuhn
Copy link
Member

We've been carrying a lot of issues over to the OS repo. The developer engagement on this repo is great, though too. The challenge is that we don't want this to become a general Windows OS developer forum. Let's try it this way. The existing issues both focus on the inbox API. I've opened a separate feature request, cross-linked to these, that covers creating a new API, and spells out that it's distinct from updating the inbox functionality. We can track the inbox OS and Reunion asks separately, though we'd likely do one or the other. It's a subtle distinction, but worth separating for clarity.

#441

@riverar
Copy link
Contributor

riverar commented Feb 11, 2021

@BenJKuhn Sounds fair, thanks! 👍

@yoshiask
Copy link

yoshiask commented Feb 25, 2022

Question for the thread - would you want a 1:1 replacement of WebAuthenticationBroker in Project Reunion that launched the browser for authentication then redirected back to the app? Or would you just want a Really Good Sample of how to register a protocol for your Win32 app and then use it from your authentication experience?

@jonwis Since it doesn't seem like WAB isn't getting fixed for non-UWP apps any time soon, is that Really Good Sample™️still on the table? I tried implementing this with protocol activation, but got stuck because it opened a new instance of my app instead of using the old one (yes, I tried activation redirection but there are barely any docs about it for UWP and none for WinAppSDK).

@wbokkers
Copy link
Author

@yoshiask To get single instancing to work with WinAppSDK, you can check out my solution here: https://gist.github.com/wbokkers/af326da5391bdb13b58529e720540178

@yoshiask
Copy link

@yoshiask To get single instancing to work with WinAppSDK, you can check out my solution here: https://gist.github.com/wbokkers/af326da5391bdb13b58529e720540178

Thanks! Unfortunately it doesn't seem to work, opening a second instance causes both instances and Visual Studio to hang until I kill the app from Task Manager.

@wbokkers
Copy link
Author

wbokkers commented Feb 28, 2022

@yoshiask, I will look into it. I have it working this way in our production code, but maybe I'm missing something.

@wbokkers
Copy link
Author

@yoshiask, I can't reproduce your issue, so I am of not much help.
I had some issues with instances not closing when converting from Windows App SDK 0.8 to version 1.0. That issue disappeared by using Process.GetCurrentProcess().Kill(); to close the app.
In version 0.8, I used Application.Current.Exit(); to close the app. Maybe you can try this.

@yoshiask
Copy link

@wbokkers I switched to Application.Current.Exit() very early on because it actually caused the second instance to crash with a C++ exception, the message being something like "unexpected process exit".

I did manage to get single-instance working, it turns out the biggest issue is that setting breakpoints at anything involving SingleInstanceApplication seems to break it.

@andreww-msft
Copy link
Contributor

The WindowsApp SDK single-instancing approach is spec'd here , and there are samples here.

@wbokkers
Copy link
Author

wbokkers commented Mar 1, 2022

@andreww-msft The spec is using C++ as an example with a main function. The samples are not about single-instancing. Am I correct? Can you point me to an example using WinUI 3 with C# to implement single instancing with redirection of all arguments to the single instance? I probably can make it work reading the spec, but I think it should be far less complicated to get a single instance app working in WinUI 3.

@andreww-msft
Copy link
Contributor

@andreww-msft The spec is using C++ as an example with a main function. The samples are not about single-instancing. Am I correct? Can you point me to an example using WinUI 3 with C# to implement single instancing with redirection of all arguments to the single instance? I probably can make it work reading the spec, but I think it should be far less complicated to get a single instance app working in WinUI 3.

@wbokkers The C# WinUI3 sample for instancing is here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-WebView bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants