Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#47480 closed enhancement (fixed)

Set expiration of the recovery mode cookie

Reported by: davidbinda's profile david.binda Owned by: sergey's profile 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 5 years ago.
47480-2.diff (1.5 KB) - added by david.binda 5 years ago.

Download all attachments as: .zip

Change History (14)

@david.binda
5 years ago

#1 @spacedmonkey
5 years ago

  • Keywords needs-patch servehappy added

#2 @TimothyBlynJacobs
5 years 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
5 years ago

#3 @david.binda
5 years 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.


5 years ago

#5 @spacedmonkey
5 years 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
5 years 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.


5 years ago

#8 @afragen
5 years ago

  • Keywords has-patch added; needs-patch removed

#9 @spacedmonkey
5 years 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
5 years ago

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

#11 @SergeyBiryukov
5 years 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
5 years ago

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