search

jumbojett / OpenID-Connect-PHP

Minimalist OpenID Connect client
https://github.com/jumbojett/OpenID-Connect-PHP
Apache License 2.0
606 stars 363 forks source link

Allow payload in state parameter #171

Open floriankick opened 5 years ago

floriankick commented 5 years ago

The state parameter exists to allow the client application to keep some state information throughout the asynchronous process.

So for some applications it is neccessary to allow custom payload in the state parameter. Currently the Client uses the state parameter for the XSRF security check. I suggest to do the security check by signing the state payload using openssl and a private key and checking the signature when the state is returned.

Notice:

  1. Additionally it would be possible to not only sign but also encrypt the payload.
  2. Not all Systems support PHP Sessions and by checking the Signature we don't need the Secret in the Session.
  3. If the User does not set any payload, we have to fill it with some random data, otherwise the signature won't work properly
JuliusPC commented 3 years ago

I would implement it by allowing to append a custom string to the already randomly generated state and a way retrieving the value afterwards (possible, since we know the length of the generated random string). This is simple and requires no key management.

@jumbojett Would you merge a PR implementing this?

abulhol commented 3 years ago

I also have a use case that requires appending a string to the state. I want to allow authorization through various endpoints, like here: https://stackoverflow.com/questions/54759982/same-redirect-uri-for-multiple-providers-in-oidc So I would like to add the name of the provider to the state, in order to then match the request in the callback.

Currently, all the functions related to this are protected, so it is not possible to modify the state value.

abulhol commented 3 years ago

On the other hand, it seems like it is better to use a different solution to solve my use case. Anyway, as this issue is quite old, you should maybe close it? @jumbojett

JuliusPC commented 3 years ago

On the other hand, it seems like it is better to use a different solution to solve my use case.

I agree. Using the state for this may make your application susceptible to Mix-Up-Attacks: https://danielfett.de/2020/05/04/mix-up-revisited/

As the linked article suggests, you should use different (per OpenID Provider) specific redirect_uris.