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

Update nonces page to explain using nonce_user_logged_out #876

Closed
htdat opened this issue Jun 19, 2023 · 11 comments
Closed

Update nonces page to explain using nonce_user_logged_out #876

htdat opened this issue Jun 19, 2023 · 11 comments
Assignees
Labels
APIs Issues for Common APIs Handbook developer documentation (DevHub) Improvements or additions to developer documentation [Status] Done Issue is completed

Comments

@htdat
Copy link

htdat commented Jun 19, 2023

Issue Description

Explaining that if just using WP core, nonces for all guest users are the same. That is, using nonces for guests by default will not yield any benefit at all in terms of security.

If plugins/themes want to protect their functionality from CSRF attacks for guests (non logged-in users), they would add their own filters for this nonce_user_logged_out hook (added in 2012). A prominent example is in the WooCommerce plugin with its own filter.

URL of the Page with the Issue

https://developer.wordpress.org/apis/security/nonces/

Section of Page with the issue

https://developer.wordpress.org/apis/security/nonces/#why-use-a-nonce
https://developer.wordpress.org/apis/security/nonces/#creating-a-nonce

Why is this a problem?

See my issue description.

Suggested Fix

See my issue description.

@htdat htdat added the tracking issue Use to track a series of related issues. label Jun 19, 2023
@htdat
Copy link
Author

htdat commented Jun 19, 2023

👋 I am happy to get assigned and work on this issue. But I am not sure how I can send a diff or draft for review? Any pointer is hugely appreciated.

@stevenlinx stevenlinx added developer documentation (DevHub) Improvements or additions to developer documentation APIs Issues for Common APIs Handbook and removed tracking issue Use to track a series of related issues. labels Jun 20, 2023
@github-actions
Copy link

Heads up @zzap - the "apis" label was applied to this issue.

@stevenlinx
Copy link
Member

@htdat
I've assigned the ticket to you.

All you need to do is to propose your changes as a comment such as the following:

section of the page:
[section]

original text:
[original text]

revised text:
[proposed text]

@htdat
Copy link
Author

htdat commented Jun 20, 2023

@stevenlinx - Thanks a lot. Will do.

@htdat
Copy link
Author

htdat commented Jul 14, 2023

@stevenlinx - thank you for your patience. Here is what I'd like to propose:

new section of the page:

text for this new section

WordPress core, by default, generates the same nonce for guests as they have the same user ID (value 0). That is, it does not prevent guests from CSRF attacks. To enhance this security aspect for critical actions, you can develop a session mechanism for your guests, and hook to the nonce_user_logged_out filter for replacing the user ID value 0 with a random ID from the session mechanism. An example can be found on plugin WooCommerce.

text for this new section (revised after feedback #876 (comment))

WordPress core, by default, generates the same nonce for guests as they have the same user ID (value 0). That is, it does not prevent guests from CSRF attacks. To enhance this security aspect for critical actions, you can develop a session mechanism for your guests, and hook to the nonce_user_logged_out filter for replacing the user ID value 0 with another random ID from the session mechanism.

@stevenlinx
Copy link
Member

Thank you for the reply.

An example can be found on plugin WooCommerce.

The above part violates External Linking Policy:
https://make.wordpress.org/docs/handbook/documentation-team-handbook/external-linking-policy/
"No plugins/themes/services/hosting etc. will be promoted or recommended. In HelpHub/DevHub, we document only what’s in Core or will be in Core."

I wonder if you can provide an example code that's more vanilla and can be applied more universally.

Thanks.

@htdat
Copy link
Author

htdat commented Jul 24, 2023

@stevenlinx - thanks for the feedback.

I wonder if you can provide an example code that's more vanilla and can be applied more universally.

I am afraid that it's not possible as building a session mechanism is a non-trivial task. Taking into account your feedback, I've removed the mention of WooCommerce and its link, and updated a bit the formating. You can see it in the new (last) heading in my previous comment #876 (comment).

@stevenlinx
Copy link
Member

I've made the revision.

@htdat
Copy link
Author

htdat commented Jul 25, 2023

Thank you. LGTM! What should I do next?

@stevenlinx
Copy link
Member

1.)

What should I do next?

Nothing.

2.)
I'll close the ticket.

@github-actions github-actions bot added the [Status] Done Issue is completed label Jul 26, 2023
@haszari
Copy link

haszari commented Oct 12, 2023

Related, the nonce_user_logged_out page has no explanation / info on it – just raw signature. Would be great to clarify things like:

  • What the return value is. "Filters whether" is not clear to me, spelling this out in plain language would likely help many folks understand.
  • How it's typically used. I suspect there's a best practice code pattern combining generating a nonce and then later checking it that uses nonce_user_logged_out. Can we put that pattern on the page, so unfamiliar developers don't have to rediscover this info independently (or rely on thousands of highly SEOd third party WordPress howto sites).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APIs Issues for Common APIs Handbook developer documentation (DevHub) Improvements or additions to developer documentation [Status] Done Issue is completed
Projects
None yet
Development

No branches or pull requests

3 participants