Reuse existing session token when refreshing sessions#948
Reuse existing session token when refreshing sessions#948goldenapples wants to merge 2 commits intoauth0:5.xfrom
Conversation
Fixes a bug when the rolling sessions functionality would update a user's session token on every pageview, invalidating any nonce checks before they're run. This change preserves the existing session token by passing it in to wp_set_auth_cookie(). In addition, the lifetime of the token itself needs to be extended so that it doesn't expire before the logged-in cookies.
|
|
||
| if ('' !== $token) { | ||
| /** This filter is documented in wp-includes/pluggable.php */ | ||
| $expiration = apply_filters('auth_cookie_expiration', 14 * DAY_IN_SECONDS, $userId, true); |
There was a problem hiding this comment.
The 14 * DAY_IN_SECONDS default mirrors what WordPress core uses inside wp_set_auth_cookie(), but it's an implicit coupling. If core ever changes that default, these two would diverge.
There was a problem hiding this comment.
True. I'm not sure what the best default value to use here is anyway, so I just went with the logic from core. Maybe the default should match the Auth0 session expiration lifetime, since we have access to that?
Regardless, sites can modify this using the core "auth_cookie_expiration" filter.
src/Actions/Authentication.php
Outdated
|
|
||
| // Update the server-side session record (wp_set_auth_cookie does not do this | ||
| // when a token is supplied). | ||
| \WP_Session_Tokens::get_instance($userId)->update($token, [ |
There was a problem hiding this comment.
WP_Session_Tokens::update() merges into existing session data. Just confirming , is there any session metadata beyond expiration that should also be refreshed for rolling sessions?
There was a problem hiding this comment.
Oh, apparently I misunderstood how the token manager handles updates. Actually, from what I see, WP_Session_Tokens::update() overwrites all the data attached to the session, which would wipe out useful audit data like IP address, user agent, and original login date; as well as any custom data attached with the attach_session_information filter.
I think it's better here to read the existing token and only update the "expiration" field on it. Updated in dfbde56.
Because WP_Session_Tokens::update() overwrites the entire session object in its datastore, the previous code was wiping out fields like IP address and original login date as well as any custom data attached to the token. This reads the original token data array so that only the exp field is updated.
2 participants
Fixes a bug when the rolling sessions functionality would update a user's session token on every pageview, invalidating any nonce checks before they're run. See #934 for examples of the issues this caused end users.
The root cause of the issue is:
wp_set_auth_cookie()on a shutdown hook of every pageview, which causes a new session token to be issued for the userThis change preserves the existing session token by passing it in to wp_set_auth_cookie(). In addition, the lifetime of the token itself needs to be extended so that it doesn't expire before the logged-in cookies.
Checklist