impress-org / givewp

GiveWP - The #1 Donation Plugin for WordPress. Easily accept donations and fundraise using your WordPress website.
https://givewp.com/
GNU General Public License v3.0
345 stars 191 forks source link

Give Session should only filter nonce_user_logged_out for GiveWP nonce actions #6586

Closed jeremyfelt closed 1 year ago

jeremyfelt commented 2 years ago

User Story

As a developer, I expect wp_create_nonce() and wp_verify_nonce() to provide and verify nonce data in a reliable way.

Details

Due to the order of operations in a current project using GiveWP, our call to wp_create_nonce() provides a nonce value that uses an empty string (WP default) as the $uid when a user is logged out. On form submission, our call to wp_verify_nonce() attempts to check against a nonce value that uses a string generated by GiveWP as the $uid.

https://github.com/impress-org/givewp/blob/19fb2ea87e9ad83d075cf7eef7a552dd88bc85f5/includes/class-give-session.php#L585-L587

There's a chance this is because when the nonce is first created, a session is not yet available, but I haven't dug too closely.

Expected Behavior

GiveWP should consider only filtering nonce_user_logged_out when a GiveWP nonce is being created or verified. The filter provides a second parameter for action that can be used to check for those prefixed with give- or against an allowed list of actions.

(e.g. WooCommerce's filtering of the same)

Thanks, y'all!

JoshuaHungDinh commented 2 years ago

Hey @jeremyfelt , thanks for taking the time to fill out a detailed issue and pointing us toward WooCommerce's filtering check. I'm sending this to the team to have it prioritized. I'd also like to invite you to submit a PR of any proposed solution you might have, if you feel it necessary. It may be the quickest way of completion.

Thanks again!

canny[bot] commented 2 years ago

This issue has been linked to a Canny post: Give Session should only filter nonce_user_logged_out for GiveWP nonce actions :tada:

JasonTheAdams commented 1 year ago

Hey @jeremyfelt! I wanted to circle back on this. We appreciate you reporting this. If this is urgent for you, I strongly recommend putting together a PR for this. Otherwise, it may be a bit before we have developer capacity to address this. Not usually what folks like to hear, I know, but I wanted to be candid on the likelihood of this getting done soon.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 14 additional days. Note, if this Issue is reporting a bug, please reach out to our support at https://givewp.com/support. If this is a feature request, please see our feedback board at feedback.givewp.com — that’s the best place to make feature requests, unless you’re providing a PR.

github-actions[bot] commented 1 year ago

This issue was closed because it has been stalled for an additional 14 days with no activity.