#51951 closed feature request (invalid)
nonce_user_logged_out filter not consistent with $user->exists()
Reported by: | chamois_blanc | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | minor | Version: | 5.6 |
Component: | Security | Keywords: | 2nd-opinion |
Focuses: | Cc: |
Description
The nonce_user_logged_out name seems to suggest that it takes effect when a user is logged out. But in the code, it only takes effect when the user ID is zero, which is not quite the same thing:
$user->ID != 0 is not equivalent to $user->exists()
Some plugins assign a user ID to distinguish non-logged in users. WooCommerce, for instance, assigns a user ID when an item is added to the cart.
I'm not sure how important the inconsistency is but I thought it was worth mentioning. On the flip side, I can see how calling $user->exists() may be slower and therefore undesirable.
It would be useful to add another filter on $uid in both wp_create_nonce and wp_verify_nonce, so that it can be changed if $user->exists() returns false (i.e. the user is not logged in). The code would look something like this:
$user = wp_get_current_user(); $uid = (int) $user->ID; if ( ! $uid ) { /** * Filters whether the user who generated the nonce is logged out. * * @since 3.5.0 * * @param int $uid ID of the nonce-owning user. * @param string $action The nonce action. */ $uid = apply_filters( 'nonce_user_logged_out', $uid, $action ); } /** * Filters the user ID. [ADDED] * * @since ??? * * @param int $uid ID of the nonce-owning user. * @param string $action The nonce action. */ $uid = apply_filters( 'nonce_user_id', $uid, $action );
This would allow to set $uid to zero (or some other value) for non-logged in users and avoid issues with nonce in such cases. Another solution is obviously not to check nonce for non-logged in users in each ajax action function, but that is probably less safe.
Thoughts?
After some more research, I found out WooCommerce is using the nonce_user_logged_out filter to change the user ID. I think I can put my own filter on top to override it for my custom actions.
I think there is some truth to my observation, but probably not quite enough to make it worth adding a new filter. I will close this feature request as 'invalid'.