The expiry time for a login is determined when a user first logs in, to be 30 days (by default) from that first login. This does not change even if the user returns to the site within those 30 days. I'd like it if the expiry time could be reset to a further 30 days every time the user returns to the site.
I've managed to implement this by changing a single line of code in _persistent_login_create_cookie(). This was line 417 in the 6.x-1.4 release. Originally, it read:
$expires = (isset($edit['pl_expires']) ? $edit['pl_expires'] : (($days > 0) ? time() + $days * 86400 : 0));
I changed it to:
$expires = (($days > 0) ? time() + $days * 86400 : 0);
Is there any unforseen security risk that I've introduced by doing this? Would it be possible to add this change as an optional feature?
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | persistent_login-7.x-735004-8.patch | 2.85 KB | hargobind |
| #2 | persistent_login-735004-2.patch | 3.06 KB | gapple |
Comments
Comment #1
gappleThis could potentially increase the time during which an attacker could continue to access another user's account.
If the session is only valid for a fixed time, the attacker will only have that amount of time where they can continue to access the user's account. Once that period is up, their session key will no longer be valid.
If the session expiry is continually moved forward, the attacker could continue to access the other user's account indefinitely as long as they continue to re-visit the site before the new session period expires.
I think having this behaviour configurable is suitable feature request, as long as the consequences are well explained. I would recommend that if the persistent login's validity is extended upon use, that the period be shortened.
Comment #2
gappleI've attached a patch that should make this work (also available from my github repository https://github.com/gapple/persistent_login/tree/issue/735004-extend-expiry).
I'd very much appreciate a review on the logic to determine the expiry period to make sure I'm not missing something.
Comment #3
gappleBumping version; needs 7.x-1.x patch
Comment #4
gappleBumping to 8.x.
Not sure if this will get backported to 7.x, probably won't be added to 6.x since it's reaching the end of its support life soon.
Comment #5
hargobindHere's a forwardport of the patch in #2 for 7.x. I know that it's standard practice to create a patch for the latest core version first, but my immediate need was for 7.x.
Comment #7
hargobindIgnore the testbot as there doesn't seem to be a way to test with 7.x.
Comment #8
hargobindResubmitting a patch for the latest 7.x-1.x branch.
Comment #9
gappleComment #11
gappleComment #12
hargobindAny chance you could commit the 7.x patch from #8?
Comment #13
gappleClosing out 7.x issues