OAuth2 API feedback

[dotMorten]

Started to try and use the 1.7exp1 OAuth APIs and got a little bit of feedback.

I must have missed this in the earlier design review, but now seeing this:

This took me a while to discover that I had to do this, and only because as the author of WinUIEx, I have the same requirement in my library, and it's not a fun requirement to have, as I know from experience it has tripped up a lot of users. My hope was that the Windows Apps SDK could solve this at a lower level and not have this same pitfall.

Secondary the example isn't great. It is better to do this in the static main before the application fully starts up, so you don't have to do the forced termination. Perhaps a simple solution could be to just have this be a standard part of the auto-generated main?

Lastly, my app hangs after calling CompleteAuthRequest and VS shows me this message shortly later:

Without the debugger attached the process just hangs for quite a while before shutting down.

[dotMorten]

One additional feedback item that a customer just reported in WinUIEx: Relying on the state parameter can have problems with some OAuth services. See dotMorten/WinUIEx#195 (comment)
I see that this API uses the same trick as WinUIEx to rely on roundtripping the state parameter to resume the correct process.

[akanpatel2206]

Started to try and use the 1.7exp1 OAuth APIs and got a little bit of feedback.

I must have missed this in the earlier design review, but now seeing this:

This took me a while to discover that I had to do this, and only because as the author of WinUIEx, I have the same requirement in my library, and it's not a fun requirement to have, as I know from experience it has tripped up a lot of users. My hope was that the Windows Apps SDK could solve this at a lower level and not have this same pitfall.

Secondary the example isn't great. It is better to do this in the static main before the application fully starts up, so you don't have to do the forced termination. Perhaps a simple solution could be to just have this be a standard part of the auto-generated main?

Lastly, my app hangs after calling CompleteAuthRequest and VS shows me this message shortly later: Without the debugger attached the process just hangs for quite a while before shutting down.

Thanks for trying out the feature @dotMorten
We have noted down the feedback and created some tasks for the same. Regarding the part where the app hangs for CompleteAuthRequest, would it be possible to share the code sample?
Asking this since I didn't face this on the sample I created, https://github.com/microsoft/WindowsAppSDK-Samples/tree/user/akanpatel2206/OAuth2_samples/Samples/OAuth.
So, your test sample can add to the learning and help debugging the issue.

[akanpatel2206]

One additional feedback item that a customer just reported in WinUIEx: Relying on the state parameter can have problems with some OAuth services. See dotMorten/WinUIEx#195 (comment) I see that this API uses the same trick as WinUIEx to rely on roundtripping the state parameter to resume the correct process.

Hi @dotMorten
Thanks for bringing attention to this.
But as per RFC 6749, https://www.rfc-editor.org/rfc/rfc6749#section-4.1.2, I see that the server should respond back with the same expected state value.

Let me know your thoughts.

[akanpatel2206]

Started to try and use the 1.7exp1 OAuth APIs and got a little bit of feedback.

I must have missed this in the earlier design review, but now seeing this:

This took me a while to discover that I had to do this, and only because as the author of WinUIEx, I have the same requirement in my library, and it's not a fun requirement to have, as I know from experience it has tripped up a lot of users. My hope was that the Windows Apps SDK could solve this at a lower level and not have this same pitfall.

Secondary the example isn't great. It is better to do this in the static main before the application fully starts up, so you don't have to do the forced termination. Perhaps a simple solution could be to just have this be a standard part of the auto-generated main?

Lastly, my app hangs after calling CompleteAuthRequest and VS shows me this message shortly later: Without the debugger attached the process just hangs for quite a while before shutting down.

For the app hangs part in CompleteAuthRequest, if you are using the same test app, can you check the configuration.

[akanpatel2206]

Started to try and use the 1.7exp1 OAuth APIs and got a little bit of feedback.

I must have missed this in the earlier design review, but now seeing this:

This took me a while to discover that I had to do this, and only because as the author of WinUIEx, I have the same requirement in my library, and it's not a fun requirement to have, as I know from experience it has tripped up a lot of users. My hope was that the Windows Apps SDK could solve this at a lower level and not have this same pitfall.

Secondary the example isn't great. It is better to do this in the static main before the application fully starts up, so you don't have to do the forced termination. Perhaps a simple solution could be to just have this be a standard part of the auto-generated main?

Lastly, my app hangs after calling CompleteAuthRequest and VS shows me this message shortly later: Without the debugger attached the process just hangs for quite a while before shutting down.

For the app hangs part in CompleteAuthRequest, if you are using the same test app, can you check the configuration? While downloading this test app to a new device, we faced an issue saying class_not_registered, and it was fixed using this.

[dotMorten]

Sorry will try and get you a repro tomorrow. With respect to configuration settings I’m using C# and those settings doesn’t really apply.

[akanpatel]

Started to try and use the 1.7exp1 OAuth APIs and got a little bit of feedback.

I must have missed this in the earlier design review, but now seeing this:

This took me a while to discover that I had to do this, and only because as the author of WinUIEx, I have the same requirement in my library, and it's not a fun requirement to have, as I know from experience it has tripped up a lot of users. My hope was that the Windows Apps SDK could solve this at a lower level and not have this same pitfall.

Secondary the example isn't great. It is better to do this in the static main before the application fully starts up, so you don't have to do the forced termination. Perhaps a simple solution could be to just have this be a standard part of the auto-generated main?

Lastly, my app hangs after calling CompleteAuthRequest and VS shows me this message shortly later: Without the debugger attached the process just hangs for quite a while before shutting down.

In the sample provided under
https://github.com/microsoft/WindowsAppSDK-Samples/blob/user/akanpatel2206/OAuth2_samples/Samples/OAuth/cs-winui-packaged/TestOAuthCSharp/StartProgram.cs

I'm handling the instance redirection in main. If the redirect is true, we don't create a new instance of the app.

static void Main(string[] args)
{
WinRT.ComWrappersSupport.InitializeComWrappers();
AppInstance.GetCurrent().Activated += OnActivated;

        bool isRedirect = DecideRedirection().GetAwaiter().GetResult();

        if (!isRedirect)
        {
            Microsoft.UI.Xaml.Application.Start((p) =>
            {
                dispatcherQueue = DispatcherQueue.GetForCurrentThread();
                var context = new DispatcherQueueSynchronizationContext(dispatcherQueue);
                SynchronizationContext.SetSynchronizationContext(context);
                new App();
            });
        }
    }

I will merge the samples to release/experimental branch for the same and update the document to modify the example. Let me know if this example looks fine to you.

[akanpatel2206]

hi @dotMorten Gentle reminder for the update on the sample you have created? Also, if you can share the code for which the app hangs, it would be helpful in debugging the issue.

[dotMorten]

@akanpatel2206 Sorry! Was a busy couple of weeks, and I got away from here.

Just run this sample: OAuthRepro.zip
Click the sign in button, and sign in in the browser (using a mocked oauth server - no password required). When it tries to resume the app after signing in, you'll see the crash.

[akanpatel2206]

Hi @dotMorten
Thanks for the repo.
Using the below RequestAuthWithParamsAsync API fixed the issue in the app.

There is a bug in RequestAuthAsync API. When the CompleteAuthRequest is handled in a separated process or not redirected to the original app instance, it fails to complete the async operation in callback.

Another update,
Because of security concerns for implicit grant type, we cannot provide RequestAuthAsync API by default, as it supports implicit grant type.
In 1.7 Experimental 2, we are going to remove this. Please verify with the RequestAuthWithParamsAsync for authorization code grant type and provide your valuable feedback. We understand it involves some extra parameters, but the feedback on working of API would be very valuable.

[dotMorten]

@akanpatel Thanks that helped.
A few things I'm now seeing:

• Upon reactivation, the window isn't being activated and bringing it back to the top after returning from the browser. Sure I could add the code for that myself, but everyone will need this so should be automated.
• result.State parameter has a weird value, even though I never set it up front (if I do set it, it comes back with the correct value).
• result.AdditionalParams has one key in it. The key is an empty string and a null value (this happens if I request auth code. if I request implicit token, I still get an empty key plus a few extra values for the expiration of the token)