Wrap the WebAuthenticator state components together using UrlEncode

[hansmbakker]

When using WebAuthenticator with the Philips Hue Remote OAuth2 API, the state parameter fell apart - its components became separate parameters in the url because the & (at least after the redirect) was not urlencoded.

This caused only appInstanceId to be read as part of the state by a new instance of my app, but signinId was lost - resulting in the new instance to live on instead of being redirected to the original instance of the app.

Example (values have been invalidated by different letters and digits):

https://account.meethue.com/get-token/?client_id=G5Q7aNyjbZwlBpf3xH5eH7jiVtCMP6dN&response_type=code&state=appInstanceId=1063c55b-2419-4e76-8b7c-76fee974e288&signinId=0c54a562-47f9-4638-b206-86f73b7a15c3&devicename=&appid=my_app&deviceid=&redirect_url_base=myfrontend%3A%2F%2Fhuecallback&app_name=my%20app&code_challenge=4QZ2r6ur3ClvuI7HKUbzynpoVwvZXRt3Suv_hNb7OFE&code_challenge_method=S256&revision=2

This patch wraps appInstanceId, signinId and the optional custom state values together by calling UrlEncode on it. Later in GetState the components are unpacked again.

[hansmbakker]

@dotMorten would you have time to look at this?

[dotMorten]

@hansmbakker sure. Once vacation is over. This is a free spare time project and I work on it when I have the time to do it. It’s open source so feel free to use your own build with the fix until then. I appreciate the PR and will get to it.

[hansmbakker]

Completely understand, and thank you very much! Enjoy the holidays!

[dotMorten]

@hansmbakker I'm stepping through the code here, and without your changes, I still see the state parameter being correctly encoded. The query.ToString() call correctly encodes it: ?response_type=code&client_id=VxIw33TIRCi1Tbk6pjh2i&redirect_uri=winuiex%3a%2f%2f&scope=photo+offline_access&state=appInstanceId%3d7921aae2-3500-488c-9fe8-ae54483064f5%26signinId%3dbb904819-87fc-4082-966b-71447f43c56c

Your change just seem to double-encode it. Is that really necessary? Could it be the oauth service you're using that incorrectly decodes it? I'd be worried some services decodes it and others don't, so with other services, you'd somehow have to know you have to double-decode it too.

[dotMorten]

After digging in deeper, the issue here is many services doesn't like url encoded state parameters and will decode them, while others would just leave them alone. Because of this inconsistent behavior, I'm not too comfortable with this fix, but it did lead me to a better solution so thanks for that. I've pushed a fix that instead uses JSON serialization of the state parameter to avoid the problem all-together.

[hansmbakker]

@dotMorten thank you for looking into this!

I'm seeing a bit of an odd behaviour: After updating to version 2.1 of WinUIEx (that is - using the NuGet package), a second app instance is still being opened instead of the redirect going to my initial instance. I am seeing that the state is encoded as JSON, which verifies that I'm using the correct version.

When not using the NuGet package, but instead putting these files in my project:

• WebAuthenticator.cs
• WebAuthenticatorResult.cs
• Helpers.cs the redirection works as expected.

This is probably a mistake on my side, but do you have any hints how I can best debug what is happening inside the NuGet package? It is not possible to put a breakpoint in decompiled code or in unloaded modules (and when the NuGet dll is loaded, the NuGet code is run immediately because of the [System.Runtime.CompilerServices.ModuleInitializer] attribute)

[dotMorten]

Thanks for following up. Any chance you could log a new issue with a small repro in it? Thanks

[hansmbakker]

@dotMorten done 👍