search

microsoft / WindowsAppSDK

The Windows App SDK empowers all Windows desktop apps with modern Windows UI, APIs, and platform features, including back-compat support, shipped via NuGet.
https://docs.microsoft.com/windows/apps/windows-app-sdk/
MIT License
3.85k stars 326 forks source link

OAuth2 API feedback #4904

Open dotMorten opened 4 days ago

dotMorten commented 4 days ago

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: Image

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: Image Without the debugger attached the process just hangs for quite a while before shutting down.

dotMorten commented 4 days ago

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

akanpatel2206 commented 3 days ago

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: Image

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: Image 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 commented 1 day ago

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. Image

Let me know your thoughts.