DuendeSoftware / Support

Support for Duende Software products
21 stars 0 forks source link

`state` disappeared from URI's after version upgrade #1395

Closed rick-micpoint closed 1 month ago

rick-micpoint commented 2 months ago

Which version of Duende IdentityServer are you using? We switched from 6.0.0 to 7.0.6

Which version of .NET are you using? .NET 8.0

Describe the bug

We use OidcClient together with WinUIEx.WebAuthenticator to authenticate on Windows from our .NET MAUI app. This worked fine, until we upgraded IdentityServer from 6.0.0 to 7.0.6. The callback URL no longer seemed to work properly, and every login attempt caused a new instance of the application to open.

Diving deeper, it looks like WinUIEx.WebAuthenticator uses the state query parameters to identify if the callback is meant for a specific instance of the app. The query parameters seem to have changed however from 6.0.0 to 7.0.6. In 6.0.0 state is in the authorize URL, and has a JSONobject as its value in the callback URL. In 7.0.6, the state query parameter is no longer in the authorize URL, and while it exists in the callback URL, the data it contains is no longer JSON.

WinUIEx.WebAuthenticator expect JSON, but it's no longer there, so it can no longer validate if the callback is meant for this app instance.

To Reproduce

Expected behavior

Everything expects the state parameter to contain JSON. If I need to set something explicitly that is fine, but we need to have the same behaviour back.

Log output/exception with stacktrace

6.0.0 authorize URL

https://example.com/connect/authorize?response_type=code&state=x3gg-aNe_TJsl38w4UFnmg&code_challenge=vrKXVqY_jTbcEC1wdLt1GSihC2GUfhGBRXpeiyYdt1Y&code_challenge_method=S256&client_id=Example.App&scope=openid%20profile%etc&redirect_uri=example%3A%2F%2Fcallback

6.0.0 callback URL state

state=%7B%22appInstanceId%22%3A%22%22,%22signinId%22%3A%22cb3dafc4-62ba-494c-9a9f-b9457649a431%22,%22state%22%3A%22xXx_uvNsMHK5Al0uo6-4VA%22%7D

As JSON

{"appInstanceId":"","signinId":"cb3dafc4-62ba-494c-9a9f-b9457649a431","state":"xXx_uvNsMHK5Al0uo6-4VA"}

7.0.6 authorize URL

https://example.com/connect/authorize?client_id=Example.App&request_uri=urn%3Aietf%3Aparams%3Aoauth%3Arequest_uri%3A7EB5E96DAB6EE43C3A8FCFF495DF60ED6D39D095591419959EDE0CFE8594372B

7.0.6 callback URL state

state=TJ9sFlyShzo61hPMSZ6tOA
josephdecock commented 1 month ago

The changes to the authorize URL are happening because IdentityServer supports pushed authorization. The idea is that instead of putting parameters into the front channel, they are moved to a backchannel (direct server to server) call, and then in the front channel you just refer to those pushed parameters by identifier (the request uri). In IdentityServer 7, we added support for PAR, and OidcClient will see that in the discovery doc and by default try to use it.

You might be able to work around this by disabling PAR in the OidcClient options, but it would be better for us to figure out why pushed state is getting mucked up - both because if there is a bug I would like to fix it and because it would allow you to use a good security feature. Ultimately this could be a serialization issue in any of:

So, 2 questions:

  1. Does disabling PAR in the client make the error go away?
  2. Can you enable PAR and then look at the IdentityServer logs? You should be able to see both the PAR request and the subsequent authorize call, and hopefully be able to see what the state value is throughout the process. Is the value that's coming in during PAR the expected JSON?
rick-micpoint commented 1 month ago

Disabling PAR indeed "fixes" the problem. This is done by setting DisablePushedAuthorization to true in the OidcClientOptions. We sadly do not have the capacity right now to dive deeper into the issue and find the actual culprit.

RolandGuijt commented 1 month ago

Ok. I'll close the issue for now. Should you have anything to add in the future please reopen.