openid-connect-generic icon indicating copy to clipboard operation
openid-connect-generic copied to clipboard

Add Support for `nonce` Attribute

Open khelil opened this issue 1 year ago • 9 comments

Hi 🖖

I'm trying to configure the plugin with France Connect, the french government SSO.

After configuration and connection try, i've got this error : {"status":"fail","message":"The following fields are missing or empty : nonce"}

I've looked for previous issues and looks like the nonce param is not set in the plugin as it is optional for an OpenId flow. The problem is that nonce param is requested from France Connect.

Is it planned to add this to the plugin ?

Thx

khelil avatar Apr 19 '24 12:04 khelil

@khelil hmm, I'll have to do some digging into this. I have not found an IDP at this point that has required that.

timnolte avatar Apr 21 '24 01:04 timnolte

thanks for you answer @timnolte

France Connect is the french gov IDP. It's used to access sensitive and personal datas so i suppose that why they're requesting the nonce param.

If it's not planned from your side, i will try to implement it and will push a PR if you're interested...

khelil avatar Apr 22 '24 12:04 khelil

Ok got this working, won't push a PR as France Connect is too specific and i had to tweaks some functions to make it work.

If someone need, the WordPress France Connect plugin is available here : https://github.com/Partikuls/france-connect-wordpress

khelil avatar Apr 23 '24 16:04 khelil

@khelil hmm, I'm curious what you all had to change as the plugin should work for any OpenID Connect compliant IDP. Was there more than just the nonce?

timnolte avatar Apr 23 '24 22:04 timnolte

Yes @timnolte, here are the changes:

  • addition of nonce, same logic as state param for creation
  • nonce is stored in state_values array in same transient
  • for verification France Connect IDP sends nonce in response of request to endpoint /token
  • if nonce or state have been modified, user needs to be disconnected from the IDP, I therefore had to modify the authentication_request_callback() function and state validation logic to add redirection to IDP in both case
  • lastly in request_authentication_token() function, had to make a GET call instead of POST

khelil avatar Apr 24 '24 00:04 khelil

@khelil hmm, that last point of having to change to a GET call seems wrong. 🤔

timnolte avatar Apr 24 '24 00:04 timnolte

@khelil the OpenId Connect specs clearly state that token requests must be sent via POST.

https://openid.net/specs/openid-connect-core-1_0.html#TokenRequest

timnolte avatar Apr 24 '24 00:04 timnolte

To a degree I believe that the nonce support should be added in the same way that acr support was added. The exception being that this should be a boolean plugin setting.

https://github.com/oidc-wp/openid-connect-generic/pull/389

timnolte avatar Apr 24 '24 00:04 timnolte

@timnolte agree, more secure is alway better ;)

khelil avatar Apr 24 '24 16:04 khelil