Opened 8 months ago
Closed 4 weeks ago
#63230 closed defect (bug) (fixed)
Correct expiration time documentation for `wp_set_auth_cookie()`
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 6.9 | Priority: | normal |
| Severity: | normal | Version: | |
| Component: | Login and Registration | Keywords: | needs-docs has-patch commit |
| Focuses: | docs | Cc: |
Description
The docs for wp_set_auth_cookie() indicate that without "Remember me" checked, the login cookie is set to expire after two days.
This is incorrect, logins that are set to be forgotten are set as session cookies to expire once the browser is closed. This is in line with industry standards.
Additionally the filter auth_cookie_expiration is fired for logins set to be forgotten but the value returned by the filter is ignored by WordPress. The second instance of the filter should be removed from the function (code ref).
Change History (19)
This ticket was mentioned in PR #8648 on WordPress/wordpress-develop by @abcd95.
8 months ago
#1
- Keywords has-patch added
@dilipbheda commented on PR #8648:
8 months ago
#2
#3
@
8 months ago
- Keywords needs-docs added
- Summary changed from Correct expriation time documentation for `wp_set_auth_cookie()` to Correct expiration time documentation for `wp_set_auth_cookie()`
As the one who posted his rant on Twitter/X which was the reason for this ticket I just want to say thanks for the ticket and the patch.
Additionally, I want to add, that this should be better documented. Maybe adding this lifetime information to this page:
https://developer.wordpress.org/advanced-administration/wordpress/cookies/
Many articles are still saying wrongly the cookie lifetime is 48 hours (or 2 days) and with "Remember me" modified to 14 days.
Especially this sentence should be changed:
The cookies lifetime can be adjusted with the auth_cookie_expiration hook. An example of this can be found at what’s the easiest way to stop wp from ever logging me out.
The filter does only affect the lifetime if "Remember me" was checked. Otherwise not. This should be added to the docs.
#4
in reply to:
↑ description
;
follow-up:
↓ 5
@
8 months ago
Replying to peterwilsoncc:
Additionally the filter
auth_cookie_expirationis fired for logins set to be forgotten but the value returned by the filter is ignored by WordPress.
Isn't the value still used on the server side, though? I believe the value is checked in wp_validate_auth_cookie() (regardless of whether "remember me" was used or not):
#5
in reply to:
↑ 4
;
follow-up:
↓ 6
@
8 months ago
Replying to siliconforks:
Isn't the value still used on the server side, though? I believe the value is checked in
wp_validate_auth_cookie()(regardless of whether "remember me" was used or not):
Looking at this code it reads the cookie via wp_parse_auth_cookie and then it casts it to integer:
$cookie_elements = wp_parse_auth_cookie( $cookie, $scheme ); // ... $expiration = $cookie_elements['expiration']; // ... $expired = (int) $expiration;
For a session cookie this means the value is 0.
What do you mean with server side?
#6
in reply to:
↑ 5
;
follow-up:
↓ 7
@
8 months ago
Replying to zodiac1978:
Looking at this code it reads the cookie via
wp_parse_auth_cookieand then it casts it to integer:
$cookie_elements = wp_parse_auth_cookie( $cookie, $scheme ); // ... $expiration = $cookie_elements['expiration']; // ... $expired = (int) $expiration;For a session cookie this means the value is 0.
What do you mean with server side?
I mean that the expiration time is used in a couple of different places:
- It is used as the expiration time for the browser cookie.
- It is also stored in the cookie itself (as a Unix timestamp - seconds since 1970) and validated by the server. That's what I mean by "server-side".
If "remember me" is not checked, then (1) above does not apply (because it will be a session cookie). However (2) still applies here.
The value of $expired in wp_parse_auth_cookie should never be 0 because it is using the value stored in the cookie itself rather than the cookie's expiration time.
#7
in reply to:
↑ 6
@
8 months ago
Replying to siliconforks:
The value of
$expiredinwp_parse_auth_cookieshould never be 0 because it is using the value stored in the cookie itself rather than the cookie's expiration time.
Correction: I meant wp_validate_auth_cookie here: $expired in wp_validate_auth_cookie should never be 0.
#8
follow-up:
↓ 10
@
8 months ago
There's two "expiration" values for this cookie. The $expiration variable is how long the value of the hash in the cookie remains valid (which is either 14 or 2 days depending on $remember). The $expire variable is how long the cookie remains valid in the browser, which is 14 days when $remember is set or 0 if not, meaning it's a session cookie in the latter case.
#9
@
8 months ago
I realised I was mistaken about don’t remember me after reviewing the code. The second instance is used, as John points out. The docs should document authentication time as:
Remember me: Remain logged in for 14 days, the cookie is stored for 14.5 days to avoid data loss in background Ajax requests immediately after the login expires and users are prompted to log back in.
Don’t remember me: remain logged in for the current browser session or up to two days (whichever is shorter) in browsers that remain open
The copy will need a little tidying up.
#10
in reply to:
↑ 8
@
8 months ago
- Keywords has-patch removed
- Type changed from defect (bug) to enhancement
Replying to johnbillion:
There's two "expiration" values for this cookie. The
$expirationvariable is how long the value of the hash in the cookie remains valid (which is either 14 or 2 days depending on$remember). The$expirevariable is how long the cookie remains valid in the browser, which is 14 days when$rememberis set or 0 if not, meaning it's a session cookie in the latter case.
Thanks for the explanation @johnbillion!
So, this is not wrong at all, but only badly documented, if I'm correct.
The grace period of 12 hours is added for the cookie, but the hash is invalid and therefore the login expires correctly after 14 days if "remember me" is checked. If it is not checked this is a session cookie, but the 48 hours still apply because the hash is invalidated. Correct? Even with a session restore from a browser this cookie will still be invalid after 48 hours, because the hash does expire. Correct?
My use case was to use auth_cookie_expiration to log out every subscriber on midnight. This would still apply even when the cookie is 12 hours longer available or a session cookie, because the hash in the DB invalidates. Correct?
This would mean we could ignore the patch and just add some more explanation to the docs. The doc page from auth_cookie_expiration and the mentioned Cookies page from the advanced administration handbook for example.
Agreed?
This ticket was mentioned in Slack in #core by wildworks. View the logs.
3 months ago
#12
@
3 months ago
This would mean we could ignore the patch and just add some more explanation to the docs. The doc page from auth_cookie_expiration and the mentioned Cookies page from the advanced administration handbook for example.
I actually agreed with this idea.
#13
@
3 months ago
- Type changed from enhancement to defect (bug)
This ticket was featured on today's Bugs Club. It seems that this ticket is a documentation issue or a bug, so I'll change its type.
This ticket was mentioned in PR #10139 on WordPress/wordpress-develop by @rollybueno.
2 months ago
#14
- Keywords has-patch added
Clarifies the docblock for wp_set_auth_cookie() to better explain how the
$remember parameter works.
- If $remember is true, WordPress sets a persistent cookie (14 days by default, filterable with auth_cookie_expiration).
- If $remember is false, WordPress sets a session cookie that ends when the browser closes. The auth_cookie_expiration filter still runs (2 days by default), but because the cookie’s expire value is 0, it won’t persist.
This update makes the function’s behavior easier for developers to understand.
Trac ticket: https://core.trac.wordpress.org/ticket/63230
#15
@
2 months ago
Hi all, since the recent discussion concludes that this only needs a better documentation, I've added new PR for clariying the remember me. Let me know if the new doc block is clear.
#16
@
4 weeks ago
@johnbillion @peterwilsoncc, do you have the bandwidth to review the latest pull request? If there's no activity, I plan to puntit to 7.0 before the RC1 release next week.
@rollybueno commented on PR #10139:
4 weeks ago
#17
There are errors on the test unit check for test_wp_insert_post_handle_malformed_post_date, which is unrelated to the patch(only inline docs are updated)
@himanshupathak95 We can remove the
elseblock and initialize the variable first. The$remembercondition will override it if true.`diffdiff --git a/src/wp-includes/pluggable.php b/src/wp-includes/pluggable.php
index e7ce2edb41..56b117636d 100644
--- a/src/wp-includes/pluggable.php
+++ b/src/wp-includes/pluggable.php
@@ -984,6 +984,9 @@ if ( ! function_exists( 'wp_set_auth_cookie' ) ) :
+ $expire = 0;
+ $expiration = time() + ( 2 * DAY_IN_SECONDS );
+
@@ -1001,10 +1004,6 @@ if ( ! function_exists( 'wp_set_auth_cookie' ) ) :