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

WebAuthenticator does not extract parameters contained in a fragment (regression) #15661

Closed
rdavisau opened this issue Jun 15, 2023 · 1 comment · Fixed by #15662
Closed

WebAuthenticator does not extract parameters contained in a fragment (regression) #15661

rdavisau opened this issue Jun 15, 2023 · 1 comment · Fixed by #15662
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info fixed-in-8.0.0-preview.6.8686 Look for this fix in 8.0.0-preview.6.8686! i/regression This issue described a confirmed regression on a currently supported version platform/android 🤖 platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst platform/windows 🪟 t/bug Something isn't working

Comments

@rdavisau
Copy link
Contributor

Description

This test doesn't pass any more (case when parameters are contained in a fragment);

[InlineData("xamarinessentials://#access_token=blah&refresh_token=blah2&expires=1", "blah", "blah2", "1")]

Introduced by #15245, which changes parameter extraction to happen off of Uri.Query (seems fragment parameters are found in Uri.Fragment)
edeae34#diff-686959c852a62a0b284d6cf92e5b61a6871108fbfc177d8ef9be7ccc94fbc2dfR18

This prevents successful authentication when callback parameters are returned in a fragment.

Steps to Reproduce

Run the WebAuthenticator ParseQueryString tests

Link to public reproduction project repository

https://github.com/dotnet/maui/

Version with bug

8.0.0-preview.5.8529

Last version that worked well

8.0.0-preview.4.8333

Affected platforms

iOS, Android, Windows, macOS, Other (Tizen, Linux, etc. not supported by Microsoft directly)

Affected platform versions

all

Did you find any workaround?

User can extract any required parameters from WebAuthenticatorResult.CallbackUri manually

Relevant log output

No response

@rdavisau rdavisau added the t/bug Something isn't working label Jun 15, 2023
@Eilon Eilon added the area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info label Jun 15, 2023
@jfversluis jfversluis added the i/regression This issue described a confirmed regression on a currently supported version label Jun 26, 2023
@jstedfast
Copy link
Member

The URI in the test should probably be fixed:

[InlineData("xamarinessentials://?refresh_token=blah2&expires=1#access_token=blah", "blah", "blah2", "1")]

Or does our code generate these types of URIs where the fragment is assumed to be allowed to contain query parameters?

jstedfast added a commit that referenced this issue Jun 27, 2023
…eters (#15662)

### Description of Change

Makes `WebUtils.ParseQueryString` extract parameters from both
`Uri.Query` and `Uri.Fragment`, resolving a regression from net8p4 to
net8p5.

### Issues Fixed

Fixes #15661
@samhouts samhouts modified the milestones: .NET 8 Planning, .NET 8 Jul 10, 2023
@samhouts samhouts added the fixed-in-8.0.0-preview.6.8686 Look for this fix in 8.0.0-preview.6.8686! label Jul 11, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info fixed-in-8.0.0-preview.6.8686 Look for this fix in 8.0.0-preview.6.8686! i/regression This issue described a confirmed regression on a currently supported version platform/android 🤖 platform/iOS 🍎 platform/macOS 🍏 macOS / Mac Catalyst platform/windows 🪟 t/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants