Pass nonce action from "nonce_life" filter

[giuseppe.mazzapica]

At the moment, nonce_life ​https://developer.wordpress.org/reference/hooks/nonce_life/ filter pass to callbacks only the nonce lifespan to be filtered.

There are cases in which a shorter nonce lifespan might be useful (default lifespan is one day), and would be handy being able to recognize the context for the nonce creation.

It means that wp_nonce_tick() ​https://developer.wordpress.org/reference/functions/wp_nonce_tick/ should receive the action as argument.

Providing a default (probably -1 that is the default none action) this change will be 100% backward compatible.

Currently the only (hackish) way to filter the lifespan only for specific nonces is to add a filter before to call both wp_create_nonce and wp_verify_nonce and remove the filter right after that. Two filter additions and two filter removals that may be replaced with a single filter addition if context would be provided by the nonce_life hook.

[dwainm]

First attempt at giving more context to the 'nonce_life' filter

[dwainm]

Hi @giuseppe.mazzapica , would love your feedback on the patch uploaded :)

[giuseppe.mazzapica]

Hi @dwainm, thanks.

I think there are some issues in the patch.

In wp_verify_nonce default action is -1 and probably that should be used in wp_nonce_tick as well. (And doc bloc should say string|int).

Moreover, wp_nonce_tick is used in wp_create_nonce and not only in wp_verify_nonce.

Last very minor thing, there's an alignment issue in the doc bloc.

[dwainm]

Updated doc blocks and added argument to wp_create_nonce function call to wp_nonce_tick call

[dwainm]

Thank you for your feedback @giuseppe.mazzapica . Updated :)

[dwainm]

Hi @giuseppe.mazzapica @johnbillion

I would love to your feedback on the latest patch. Thank you.

[DrewAPicture]

Assigning to mark the good-first-bug as "claimed".

See 35188-2.patch​

[jorbin]

Thanks for the patch. When a filter or function's signature is updated, the inline documentation should be updated with a ​changelog entry. Additionally, the patch uses spaces and not tabs, so it needs to be updated for that well. The ​PHP style guide is in the handbook.

[dwainm]

35188-patch-3

[dwainm]

Adding changelog entries

[dwainm]

Thank you for your feedback @jorbin. I've updated the patch to include tabs not space and have updated the change log for both the function and the filter.

[audrasjb]

Removing good-first-bug and adding needs-refresh / needs-testing.

[audrasjb]

Self assigning for refresh/testing

[SergeyBiryukov]

The latest patch appears to miss the part where the $action parameter is actually passed to wp_nonce_tick(), as seen in earlier patches, e.g. 35188-2.patch​.

Trac ticket: https://core.trac.wordpress.org/ticket/35188

[costdev]

@audrasjb @SergeyBiryukov ​PR 3007 refreshes 35188-4.patch against trunk, updates the @since tags and adds the two missing changes from 35188-2.patch.

This still needs-testing. If someone can test the PR or add testing instructions for others to do so, that would be great!

[audrasjb]

So @costdev I went to some tests and it looks like it doesn't work as expected… or maybe it's just me… :D

First, create a simple plugin which will generate a link at the end of each singular post of your test site.
This link contains a nonce, and when you click on the link, you'll get the following message:

• if the nonce is valid (24 hours time limit by default), it will display "Nonce is valid".
• if the nonce is invalid, it will display "Nonce is invalid"

Then we'll add a small snippet to this plugin, to change the time limitation of our nonce only for the nonce-life-tester.

Here is the code of the plugin:

<?php
/*
Plugin Name: nonce-life-tester
Author: audrasjb
Version: 0.1
Author URI: https://profiles.wordpress.org/audrasjb
*/

function nonce_life_tester_display_link( $content ) {
        // Check if we're inside the main loop in a single Post.
        $nonce = $_GET['_wpnonce'];
        if ( isset( $nonce ) && ! empty( $nonce ) ) {
                // Check Nonce and display verification results.
                $verify = wp_verify_nonce( $nonce, 'nonce-life-tester' );
                switch ( $verify ) {
                        case 1:
                                $result = 'Nonce is valid (less than 12 hours old)';
                                break;
                        case 2:
                                $result = 'Nonce is valid (between 12 and 24 hours old)';
                                break;
                        default:
                                $result = 'Nonce is invalid';
                }
                $content .= '<p>Nonce verification: <code>' . $result . '</code></p>';
        } else {
                // Display a link with a Nonce.
                if ( is_singular() && in_the_loop() && is_main_query() ) {
                        $url = wp_nonce_url( get_permalink(), 'nonce-life-tester' );
                        $content .= '<p><a href="' . $url . '">Testing nonces</a></p>';
                }
        }
        return $content;
}
add_action( 'the_content', 'nonce_life_tester_display_link' );

function nonce_life_tester_reduce_time_limit( $lifespan, $action ) {
        // Modify the lifespan of our specific Nonce.
        if ( 'nonce-life-tester' === $action ) {
                return 10; // 10 Seconds.
        } else {
                return $lifespan;
        }
}
add_filter( 'nonce_life', 'nonce_life_tester_reduce_time_limit', 10, 2 );

Using the current PR, I always get Nonce is invalid message.

[antonvlasenko]

I've tried to reproduce the issue that @audrasjb outlined here, and I got the same result: Nonce is invalid.
But it might be enough to implement this simple fix to make it work:
​https://github.com/WordPress/wordpress-develop/pull/3007#pullrequestreview-1112720674

@costdev I think the $action parameter should also be added to this place where the wp_nonce_tick() function gets called:
​https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/pluggable.php#L2351
E.g.:

$i     = wp_nonce_tick( $action );

Nicely spotted @anton-vlasenko! That's the last time I refresh a PR without testing it properly! :joy:

[costdev]

@audrasjb I have updated the PR with a fix that @antonvlasenko pointed out. When you get time, can you take it for a test spin? 🙂

[audrasjb]

Ah thank for fixing this @costdev now it works fine with my little test plugin \o/

I think we're good to go!

[audrasjb]

In 54218:

General: Pass $action to nonce_life filter.

This changeset contextualizes the usage of nonce_life filter by passing the $action parameter. It allows to alterate the default lifespan of nonces on a case by case basis.

Props giuseppemazzapica, dwainm, DrewAPicture, jorbin, audrasjb, SergeyBiryukov, costdev, antonvlasenko.
Fixes #35188.

Committed in https://core.trac.wordpress.org/changeset/54218

Ticket: #58181

Description:
In [54218] / #35188 $action was accidentally passed to wp_get_session_token(). This function doesn't accept any parameters.
In this PR, I have removed it to avoid consequences.

Trac ticket: https://core.trac.wordpress.org/ticket/58181

Thanks for the PR! Merged in r55685.