user icon indicating copy to clipboard operation
user copied to clipboard

CurrentUser::$absoluteAuthTimeout logic is not compatible with auto-login cookie

Open jettero777 opened this issue 7 months ago • 11 comments

As I see from the code the CurrentUser::$absoluteAuthTimeout is stored as __auth_absolute_expire in PHP session only but not in cookie, PHP session expires by default in 24 minutes, so usually auto-login cookie is used to keep sessions alive, but __auth_absolute_expire logic is not working as intended with the cookies:

if __auth_absolute_expire expires it ends current user session stored in PHP session but new user session will be created right away on the next request by CookieLoginMiddleware using the auto-login cookie,

or if a user takes a break for 24 minute the PHP session with __auth_absolute_expire will expire itself, and will be created fresh session with new __auth_absolute_expire on new request

so I think __auth_absolute_expire should be stored in cookie too to fix it

UPD and same for __auth_expire - it is not working as intended if auto-login cookie is used, but I'm not sure if it does make sense to add __auth_expire to cookie as there is similar expire mechanism for cookie

jettero777 avatar May 30 '25 19:05 jettero777

How would you fix that?

samdark avatar May 31 '25 06:05 samdark

@samdark I think __auth_absolute_expire should be stored in session and in auth cookie too, now cookie has array with identity id, auth key and expire, and absolute expire can be added there. So if session expired __auth_absolute_expire should be set in new session from cookie, and the cookie also should respect __auth_absolute_expire and should expire on it.

And regarding __auth_expire - there is same issue here as with __auth_absolute_expire - it doesn't work as expected with cookies, but storing it in cookie would be misleading, because there is yet another expire stored in the cookie and having 2 expires in cookie may cause confusion. So better if single expire will be used for both cases - for session and for cookie.

I think auth session expire (as timeout based on inactivity) and absolute expire (as max time from first login) should be configured once in one place and it should be used as expire and absolute expire for all use-cases:

  1. session alone
  2. session + cookie
  3. cookie alone

jettero777 avatar May 31 '25 12:05 jettero777

Currently, the entire CurrentUser logic is hardcoded to use sessions. This represents short-term logic (current user session).

CookieLoginMiddleware, on the other hand, handles long-term logic.

I believe these are different concerns and should not overlap. We can add cookies support to CurrentUser, but these should be other cookies, not from CookieLoginMiddleware.

vjik avatar Jun 02 '25 09:06 vjik

Currently, the entire CurrentUser logic is hardcoded to use sessions. This represents short-term logic (current user session). CookieLoginMiddleware, on the other hand, handles long-term logic.

Well, as I understand primarily use-case for absolute expire is for long sessions, like it is done in wordpress where auth session max time is 14 days (if 'remember me' option used) and then user forcibly logged out.

If it is hardcoded to be used with PHP sessions only, which by default expire in 24 minutes, then absolute expire is not that useful. User will be logged out anyway on first inactivity over 24 minutes.

jettero777 avatar Jun 02 '25 17:06 jettero777

In other words, I think primarily use for absolute expire feature is to limit 'remember me' cookie longevity. And if long session are handled by midleware then this feature should be moved there.

You really didn't intend to forcibly log out user in the middle of short session, right? :) It is not good for UX.

jettero777 avatar Jun 02 '25 18:06 jettero777

It is not good for UX.

But is required in some cases for projects with increased security measures.

samdark avatar Jun 03 '25 17:06 samdark

@samdark I see, good point. What about to duplicate this feature for long sessions in CookieLoginMiddleware then?

I would like to limit number of days when user can use auto-login cookie without full logging in. It is also needed for security reasons and is quite usual feature on the websites.

jettero777 avatar Jun 03 '25 18:06 jettero777

I think it is a good idea. Do you want to implement it and send a pull request?

samdark avatar Jun 04 '25 03:06 samdark

In other words, I think primarily use for absolute expire feature is to limit 'remember me' cookie longevity.

No. "Remember me" is other thing, it implements in CookieLoginMiddleware.

vjik avatar Jun 04 '25 07:06 vjik

@samdark Well, I started to do it, but on the second look seems I was wrong.

I had thought that the auto-login cookie is being set every request, as it was in Yii2, where the cookie is being prolonged every request until user inactivity is longer than was cookie duration and it expires then.

But seems here it is set only on first login and that's it, and cookie will expire at first duration time regardless if user visits in the meantime or not. So effectively it is already is absolute timeout.

Is it intended behavior? I think usually it is expected to be prolonged (by default, if you don't need enhanced security). And in CurrentUser with PHP session it is also being prolonged upon visits.

jettero777 avatar Jun 04 '25 08:06 jettero777

Both could be expected depending on the policy one has to implement.

samdark avatar Jun 09 '25 12:06 samdark