[wp-trac] [WordPress Trac] #46595: Allow more than one valid recovery mode link

WordPress Trac noreply at wordpress.org
Fri Apr 5 13:27:51 UTC 2019

#46595: Allow more than one valid recovery mode link
 Reporter:  flixos90                             |       Owner:
                                                 |  timothyblynjacobs
     Type:  defect (bug)                         |      Status:  assigned
 Priority:  normal                               |   Milestone:  5.2
Component:  Bootstrap/Load                       |     Version:  trunk
 Severity:  normal                               |  Resolution:
 Keywords:  servehappy has-patch has-unit-tests  |     Focuses:
  dev-feedback                                   |

Comment (by flixos90):

 Thanks @pbearne for tackling this, great work! Only some observations
 regarding [attachment:"46595.7.patch"] to improve the code:

 * Let's rename the option to `recovery_keys` as it's multiple now.
 * In `WP_Recovery_Mode_Key_Service`, the
 `generate_and_store_recovery_mode_key()` is too complex and odd with the
 two return values. Let's instead add a new
 `generate_recovery_mode_token()` method that just does `return
 wp_generate_password( 22, false )` and introduce a parameter to the former
 method, so that it is `generate_and_store_recovery_mode_key( $token )` -
 it should then continue to only return the key. Code using these parts
 should then call `generate_recovery_mode_token()` first and then pass that
 * The `clean_expired_keys( $ttl, $token = '' )` method does too much as
 well. Let's split it into `clean_key( $token )` and `clean_expired_keys(
 $ttl )` so that both methods have distinct functionality.
 * `validate_and_consume_recovery_mode_key()` is too long of a name and too
 detailed. Let's stick with the previous name
 `validate_recovery_mode_key()` and clarify the one-time usage of a key in
 the doc block. That method should then call `clean_key( $token )`, as it
 should only need to worry about the currently checked key and token.
 * `clean_expired_keys( $ttl )` should be handled separately and be hooked
 into WP Cron so that it is executed regularly.
 * I'd prefer if we didn't add the `generate_recovery_mode_key` action.
 That's something we should do if it proves to be needed.

Ticket URL: <https://core.trac.wordpress.org/ticket/46595#comment:13>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform

More information about the wp-trac mailing list