oidc-wp / openid-connect-generic

WordPress plugin to provide an OpenID Connect Generic client
https://wordpress.org/plugins/daggerhart-openid-connect-generic/
261 stars 155 forks source link

Add Support for `nonce` Attribute #532

Open khelil opened 7 months ago

khelil commented 7 months ago

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

timnolte commented 7 months ago

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

khelil commented 7 months ago

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 commented 7 months ago

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

timnolte commented 7 months ago

@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?

khelil commented 7 months ago

Yes @timnolte, here are the changes:

timnolte commented 7 months ago

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

timnolte commented 7 months ago

@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 commented 7 months ago

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

khelil commented 7 months ago

@timnolte agree, more secure is alway better ;)