WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 2 months ago

Last modified 2 months ago

#47480 closed enhancement (fixed)

Set expiration of the recovery mode cookie

Reported by: david.binda Owned by: sergey
Milestone: 5.3 Priority: normal
Severity: normal Version: 5.2
Component: Site Health Keywords: servehappy commit has-patch
Focuses: Cc:

Description

The recovery mode cookie is set with no expiration, but any request containing a recovery mode cookie is handled by WordPress as a request which is attempting to enter the recovery mode and the validity of the cookie is being checked during the request processing, which includes a expiration of the cookie (by default set to a week).

It means that whenever a recovery mode is entered and not properly existed via a button in wp-admin, the recovery cookie stays in the browser and WordPress would eventually presents a wp_die error page to a user who did not exit the recovery mode by expected path. The UI is quite rough, as it requires the user to manually reload the page in order to access their WordPress site again.

It feels like such an edge-case could be mitigated by setting the cookie's expiration to the same amount of time for which the token in it is valid - eg.: to a week by default.

Attachments (2)

47480.diff (1.5 KB) - added by david.binda 3 months ago.
47480-2.diff (1.5 KB) - added by david.binda 3 months ago.

Download all attachments as: .zip

Change History (14)

@david.binda
3 months ago

#1 @spacedmonkey
3 months ago

  • Keywords needs-patch servehappy added

#2 @TimothyBlynJacobs
3 months ago

This gets a +1 from me. But I think the apply_filters call and the time() + expression should probably be separated into two different statements.

$length = apply_filters();
$expire = time() + $length;

@david.binda
3 months ago

#3 @david.binda
3 months ago

Thanks for the feedback @TimothyBlynJacobs ! I've just uploaded an enhanced diff.

Just to explain the original notation - I've been following an example in wp_set_auth_cookie, which does the same stuff. But I'm fine with either :)

Please, let me know if it works for you, thanks!

This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.


3 months ago

#5 @spacedmonkey
3 months ago

  • Keywords needs-refresh added
  • Milestone changed from Awaiting Review to 5.3

Marking at 5.3 for merge. @davidbinda can you update the docs and I will get this merged.

#6 @david.binda
3 months ago

@spacedmonkey , sure, just, I'm not really sure how the docs should be updated. Do you think the since doc? The filter actually exists since 5.2. Or should the docs for the filter mention that the length is actually not used only for validity, but also for the cookie expiration? Thanks for clarifying this!

This ticket was mentioned in Slack in #core-php by spacedmonkey. View the logs.


2 months ago

#8 @afragen
2 months ago

  • Keywords has-patch added; needs-patch removed

#9 @spacedmonkey
2 months ago

  • Keywords needs-patch commit added; needs-refresh has-patch removed
  • Owner set to sergey
  • Status changed from new to assigned

@david.binda You are right. Forget me. I will mark this for commit.

#10 @SergeyBiryukov
2 months ago

  • Component changed from General to Bootstrap/Load
  • Keywords has-patch added; needs-patch removed

#11 @SergeyBiryukov
2 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 45545:

Bootstrap/Load: Set expiration of the recovery mode cookie to the same amount of time for which the token in it is valid: a week by default.

Props david.binda, TimothyBlynJacobs.
Fixes #47480.

#12 @spacedmonkey
2 months ago

  • Component changed from Bootstrap/Load to Site Health
Note: See TracTickets for help on using tickets.