dotMorten / WinUIEx

WinUI Extensions
https://dotmorten.github.io/WinUIEx
MIT License
582 stars 37 forks source link

WebAuthenticator adds state parameter incorrectly #75

Closed Bondarenko1990 closed 1 year ago

Bondarenko1990 commented 1 year ago

I want to add in state parametr provider=1;

    private async void DoOAuth_Click(object sender, RoutedEventArgs e)
    {
        string clientId = "VxIw33TIRCi1Tbk6pjh2i";
        string clientSecret = "eEkUe5e9gUpO6KOYdL5pKTi683LADpi5_izZdHCI8Mndy32B";
        string state = DateTime.Now.Ticks.ToString();
        string callbackUri = "winuiex://";
        string authorizeUri = $"https://www.oauth.com/playground/auth-dialog.html?response_type=code&client_id={clientId}&redirect_uri={Uri.EscapeDataString(callbackUri)}&scope=photo+offline_access&state=provider=1";
        _ = base.ShowMessageDialogAsync("Login: nice-ferret@example.com\npassword: Black-Capybara-83", "Enter credentials in browser");
        var result = await WebAuthenticator.AuthenticateAsync(new Uri(authorizeUri), new Uri(callbackUri));
        Log("Logged in. Code returned: " + result.Properties["code"]);
    }

and in the method Authenticate, we get the old state value "provider=1", image and after that we get query with two state values -"response_type=code&client_id=VxIw33TIRCi1Tbk6pjh2i&redirect_uri=winuiex%3a%2f%2f&scope=photo+offline_access&state=appInstanceId%3d6e4168e9-a66a-4258-aae4-2921d57804ff%26signinId%3d9a25feea-f024-4430-8528-75cef7132cdd%26state%3dprovider%253d1" image I think its need to be remove "state=" and do so image

dotMorten commented 1 year ago

There's only one state parameter. The second one you see is encoded as part of the first one. This happens because the webauthenticator uses the state parameter to resume the correct app session, but if you provide a state parameter as well, it will be encoded into the new state parameter, then decodes it again after the response comes back.

Bondarenko1990 commented 1 year ago

state=appInstanceId%3d6e4168e9-a66a-4258-aae4-2921d57804ff%26signinId%3d9a25feea-f024-4430-8528-75cef7132cdd%26state%3dprovider%253d1

@dotMorten you probably misunderstood me. I understand that the second parameter is encoded in part of the first, but why with the name state if it already exists? state=appInstanceId%3d6e4168e9-a66a-4258-aae4-2921d57804ff%26signinId%3d9a25feea-f024-4430-8528-75cef7132cdd%26state%3dprovider%253d1
why the state parameter contains two state?

and try add state parametr provider=1 to authorizeUri string authorizeUri = $"https://www.oauth.com/playground/auth-dialog.html?response_type=code&client_id={clientId}&redirect_uri={Uri.EscapeDataString(callbackUri)}&scope=photo+offline_access&state=provider=1"; I have exception - System.ArgumentNullException, because key is null image maybe I didn't understand something. but with that authorizeUri the application does not work correctly.

dotMorten commented 1 year ago

why with the name state if it already exists?

Because it's part of the oauth standard to use state as a parameter for roundtripping back to the redirect. Most oauth services will strip all other parameters when getting to the redirect.

dotMorten commented 1 year ago

btw your string is encoded incorrectly: &state=provider=1. Shouldn't that second = be encoded?

Bondarenko1990 commented 1 year ago

why with the name state if it already exists?

Because it's part of the oauth standard to use state as a parameter for roundtripping back to the redirect. Most oauth services will strip all other parameters when getting to the redirect.

yes, but why we do we have to set 'state' twice? state=appInstanceId%3d6e4168e9-a66a-4258-aae4-2921d57804ff%26signinId%3d9a25feea-f024-4430-8528-75cef7132cdd%26state%3dprovider%253d1

Bondarenko1990 commented 1 year ago

btw your string is encoded incorrectly: &state=provider=1. Shouldn't that second = be encoded?

I tried image and got a bad result. application relaunched

Bondarenko1990 commented 1 year ago

btw your string is encoded incorrectly: &state=provider=1. Shouldn't that second = be encoded?

I tried image and got a bad result. application relaunched

@dotMorten Have you tested?

dotMorten commented 1 year ago

yes, but why we do we have to set 'state' twice? state=appInstanceId%3d6e4168e9-a66a-4258-aae4-2921d57804ff%26signinId%3d9a25feea-f024-4430-8528-75cef7132cdd%26state%3dprovider%253d1

Again the state isn't set twice. It is set once, and there happen be the string state%3d in there.

dotMorten commented 1 year ago

It does look like the oauth playground service has changed and is no longer correctly preserving encoding of the state parameter. This is a service issue, and not a WinUIEx issue though - I'll have to find a different service to use as an example, but if you use a proper oauth service this shouldn't be a problem for you

image