Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support for nonce Attribute #532

Open
khelil opened this issue Apr 19, 2024 · 9 comments
Open

Add Support for nonce Attribute #532

khelil opened this issue Apr 19, 2024 · 9 comments
Assignees
Labels
enhancement Issues & PRs related to new features.

Comments

@khelil
Copy link

khelil commented Apr 19, 2024

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
Copy link
Collaborator

@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
Copy link
Author

khelil commented Apr 22, 2024

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
Copy link
Author

khelil commented Apr 23, 2024

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 khelil closed this as completed Apr 23, 2024
@timnolte
Copy link
Collaborator

@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
Copy link
Author

khelil commented Apr 24, 2024

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

@timnolte
Copy link
Collaborator

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

@timnolte
Copy link
Collaborator

@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
Copy link
Collaborator

timnolte commented Apr 24, 2024

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.

#389

@khelil
Copy link
Author

khelil commented Apr 24, 2024

@timnolte agree, more secure is alway better ;)

@timnolte timnolte reopened this Apr 24, 2024
@timnolte timnolte changed the title France Connect Missing nonce Add Support for nonce Attribute Apr 24, 2024
@timnolte timnolte self-assigned this Apr 24, 2024
@timnolte timnolte added the enhancement Issues & PRs related to new features. label Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues & PRs related to new features.
Projects
Development

No branches or pull requests

2 participants