Page MenuHomePhabricator

Logged in status incorrectly displayed after a PHP session timeout
Open, MediumPublic

Description

Author: dnessett

Description:
This bug was discovered while working on bug 32122
(https://bugzilla.wikimedia.org/show_bug.cgi?id=32122). In order to reproduce
it reliably, a developer must make the following changes to php.ini (this
should *not* be done on a production machine, since the settings force the PHP
garbage collector to run on every page access).

session.gc_probability = 100
session.gc_divisor = 100
session.gc_maxlifetime = 60
session.save_path = <some directory writable by httpd>

login (DO NOT CHECK THE "REMEMBER ME" BOX).

Print the contents of the session file (this is most easily accomplished by deleting all session files before login, which will mean only one session file exists after login). The session contents will look something like:

wsUserID|i:1;wsToken|s:32:"895091d5eb444a89d6e29b679b4ec8ac";wsUserName|s:9:"WikiSysop";wsLoginToken|N;

+ Wait 60 seconds or more.

Refresh the page.

The login status line will show something like:

<username> My talk My preferences My watchlist My contributions Log out

where <username> indicates the name of the user who logged in. However, the session file will contain something like:

wsUserID|i:1;wsUserName|s:9:"WikiSysop";

This means the user is actually logged out. So the login status line is incorrectly displayed. If you refresh the page once again, the login status line correctly indicates the user is logged out.

In a post to Mediawiki-l (http://www.mail-archive.com/mediawiki-l@lists.wikimedia.org/msg08967.html), Brion Vibber indicates this problem would be fixed if the enhancement request in https://bugzilla.wikimedia.org/show_bug.cgi?id=31639 were implemented.


Version: 1.16.x
Severity: normal

Details

Reference
bz32364

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:01 AM
bzimport set Reference to bz32364.
bzimport added a subscriber: Unknown Object (MLST).
Krinkle subscribed.

Let's re-test and verify. I suspect this may still be broken, despite the three scenarios in T33639 being presumed fixed.

I suggest these two scenarios at least:

  • Page A is edited at t=1.
    • Log in. User X is touched at t=2.
    • View A. Note "Last-Modifed" in response.
    • Clear session store rows.
    • Hard refresh A. Note any "If-Modified-Since" in the request, HTTP response code, and "Last-Modified in response. Expected: 200 OK.

This might send IMS, and might receive HTTP 304 (showing logged-in, because browser has a newer one and so is new enough) which would be bad. It will probably also receive replacement cookies (new anon session?), and a Last-Modified that goes back in time (no user-specific bump applied). Which means if we tested it with multiple pages, then subsequent pages will be fine because they won't match due to having a new/different session cookie.

  • Log in. User X is touched at t=2.
    • Edit page A at t=3.
    • View A. Note "Last-Modifed" in response.
    • Clear session store rows.
    • Hard refresh A. Note "If-Modified-Since" in the request, HTTP response code, and "Last-Modified in response. Expected: 200 OK.

It will probably send IMS, and might receive HTTP 304 (showing logged-in, because timestamp is the same, since rev timestamp was higher than user touched), same as before.

If these do work as expected, it is presuambly because the browser is taking the new cookies into account immediately and using it to discard the same response. That would be nice. It would also require MW to actually send replacement cookies in this scenario, instead of recycling the same session ID or something like that. Or we might get lucky with a secondary cookie like UserID/UserName being removed/replaced whilst recycling the session ID, that might suffice.