Commit graph

36 commits

Author SHA1 Message Date
Brad Jorsch
6a43d7749e Unpersist the session on session load failure
There's no point in keeping broken cookies around, it just means all
future requests will continue to flood the logs.

Change-Id: Ib10c9ed9049b71ed434950fc731ea77960ceca0c
2016-02-29 22:29:23 +00:00
Brad Jorsch
6d4436c915 Unpersist the session on logout
Clearing the cookies in this case is probably a good idea.

This also clears cookies when a non-persisted session's metadata is
dirty, for parallelism with what happens to persisted sessions.

Bug: T127436
Change-Id: I76897eaac063e5e3c3563398d0f4cb36cf93783b
2016-02-29 15:39:25 -05:00
Brad Jorsch
e75f2ab0f2 Improve SessionManager unit test coverage, and fix two namespacing bugs
Change-Id: Ie0bcba77625e04ca3e89eb400626f63024c6e1a1
2016-02-26 20:14:27 +00:00
Ori Livneh
2c1e550b6f Revert "Log multiple IPs using the same session or the same user account"
This reverts commit f22549a605.
Per T125455#2054194.

Bug: T125455
Change-Id: Ic2049381e98586e91974fc5b47d9e857a73414a4
2016-02-23 20:13:48 +00:00
Kunal Mehta
6e9b4f0e9c Convert all array() syntax to []
Per wikitech-l consensus:
 https://lists.wikimedia.org/pipermail/wikitech-l/2016-February/084821.html

Notes:
* Disabled CallTimePassByReference due to false positives (T127163)

Change-Id: I2c8ce713ce6600a0bb7bf67537c87044c7a45c4b
2016-02-17 01:33:00 -08:00
Brad Jorsch
1aedd2df73 Session: Implement ArrayAccess
Now that we dropped support for PHP 5.3.3, we can do this.

The behavior of $session['foo'] when that key doesn't already exist is a
little unexpected (it implicitly assigns null), but it's the best we can
do.

Change-Id: Ibef878867d46591a8bf542139a1719dfec3b83ab
2016-02-16 17:33:33 +00:00
jenkins-bot
197ee74f2b Merge "Log multiple IPs using the same session or the same user account" 2016-02-12 00:35:12 +00:00
Bryan Davis
9750adacdd SessionProvider::mergeMetadata: Log additional data
Add the data values and types to the exception raised when mismatched
session data is processed. This is done by passing the old and new
values on via a new MetadataMergeException class. The attached data is
added to the debug logging context info when caught.

Change-Id: If8a7174399289bc284ca1b36052ba515c8857c50
2016-02-11 14:21:14 -07:00
Gergő Tisza
f22549a605 Log multiple IPs using the same session or the same user account
As an attempt to detect SessionManager errors that log people into
the wrong account, log multiple IPs using the same session, or the same
user account.

Bug: T125455
Change-Id: I27468a3f6d582d9b46984227b9307dc71190fd6a
2016-02-11 08:00:28 -08:00
jenkins-bot
bc86ab6eb2 Merge "Remove unused imports from Session classes" 2016-02-10 19:21:27 +00:00
Brad Jorsch
cf9fc81e96 Remove "$that" from SessionManager
Also "function () use ( &$ref )" that was being done for similar
reasons.

Change-Id: If4ec263a9a9c02c1c6a414b26a0e77ba144437f1
2016-02-10 12:10:38 -05:00
Thiemo Mättig
e5159debbb Remove unused imports from Session classes
Change-Id: Ia3cdd923728870f5eff0bd90fd2f6605047291c0
2016-02-10 18:09:02 +01:00
jenkins-bot
79e51b55e5 Merge "Update session log messages" 2016-02-08 16:02:45 +00:00
jenkins-bot
8ea3e9135d Merge "Introduce User::INVALID_TOKEN" 2016-02-08 00:55:04 +00:00
Reedy
0f19a8e771 Remove duplicate array keys from tests
Change-Id: I437b87151be6589a8d5c984b90cd249c2d0ecd3c
2016-02-07 21:22:53 +00:00
Bryan Davis
f60fd42b33 Update session log messages
* Use PSR-3 templates and context where applicable
* Add log coverage for exceptional events

Bug: T125452
Change-Id: I8f96fa1c5766c739a21219abcae2dbb76de53e2a
2016-02-06 16:18:01 -07:00
Brad Jorsch
30a9eae821 Introduce User::INVALID_TOKEN
To avoid having to have SessionManager try to reset sessions on every
request, we set the user_token to a special value. When that value is
present, User::getToken() returns a different value every time (so
existing checks will fail) and User::setToken() refuses to alter it.

Bug: T124414
Change-Id: Ie4c84ce993e40a081288cf5a543f8ba99f98806a
2016-02-03 21:45:45 +00:00
Brad Jorsch
25dbd91513 Clean up after Ie161e0f
Ie161e0f was done in a hurry, and so didn't do things in the best ways.
This introduces a new "CachedBagOStuff" that transparently handles all
the logic that had been copy-pasted all over in Ie161e0f.

The differences between CachedBagOStuff and MultiWriteBagOStuff are:
* CachedBagOStuff supports only one "backend".
* There's a flag for writes to only go to the in-memory cache.
* The in-memory cache is always updated.
* Locks go to the backend cache (with MultiWriteBagOStuff, it would wind
  up going to the HashBagOStuff used for the in-memory cache).

Change-Id: Iea494729bd2e8c6c5ab8facf4c241232e31e8215
2016-02-03 21:45:18 +00:00
Brad Jorsch
2257fe4228 Revert "Remove SessionManager, temporarily"
This reverts commit 823db5d63dd5200d04c63da50ba6bf16f928e70b.

Change-Id: Ibb3e023e4eb6715295586dea87d0725c344a8271
2016-02-03 21:44:59 +00:00
Brad Jorsch
5083e810eb Remove SessionManager, temporarily
The plan here is to take it out of 1.27.0-wmf.12 and put it back in
1.27.0-wmf.13.

Since BotPasswords depends on SessionManager, that's getting temporarily
removed too.

This reverts the following commits:
* 6acd424e0d SessionManager: Notify AuthPlugin before calling hooks
* 4d1ad32d8a Close a loophole in CookieSessionProvider
* fcdd643a46 SessionManager: Don't save non-persisted sessions to backend storage
* 058aec4c76 MessageCache: Don't get a ParserOptions for $wgUser before the end of Setup.php
* b5c0c03bb7 SessionManager: Save user name to metadata even if the user doesn't exist locally
* 13f2f09a19 SECURITY: Fix User::setToken() call on User::newSystemUser
* 305bc75b27 SessionManager: Don't generate user tokens when checking the tokens
* 7c4bd85d21 RequestContext::exportSession() should only export persisted session IDs
* 296ccfd4a9 SessionManager: Save 'persisted' flag in session metadata
* 94ba53f677 Move CSRF token handling into MediaWiki\Session\Session
* 46a565d6b0 Avoid false "added in both Session and $_SESSION" when value is null
* c00d0b5d94 Log backtrace for "User::loadFromSession called before the end of Setup.php"
* 4eeff5b559 Use $wgSecureCookie to decide whether to actually mark secure cookies as 'secure'
* 7491b52f70 Call session_cache_limiter() before starting a session
* 2c34aeea72 SessionManager: Abstract forceHTTPS cookie setting
* 9aa53627a5 Ignore auth cookies with value 'deleted'
* 43f904b51a SessionManager: Kill getPersistedSessionId()
* 50c5256352 SessionManager: Add SessionBackend::setProviderMetadata()
* f640d40315 SessionManager: Notify AuthPlugin when auto-creating accounts
* 70b05d1ac1 Add checks of $wgEnableBotPasswords in more places
* bfed32eb78 Do not raise a PHP warning when session write fails
* 722a7331ad Only check LoggedOut timestamp on the user loaded from session
* 4f5057b84b SessionManager: Change behavior of getSessionById()
* 66e82e614e Fix typo in [[MediaWiki:Botpasswords-editexisting/en]]
* f9fd9516d9 Add "bot passwords"
* d7716f1df0 Add missing argument for wfDebugLog
* a73c5b7395 Add SessionManager

Change-Id: I2389a8133e25ab929e9f27f41fa9a05df8147a50
2016-02-01 22:06:49 +00:00
Brad Jorsch
4d1ad32d8a Close a loophole in CookieSessionProvider
There's a crazy-small chance that someone could have a logged-out
session (e.g. by logging out or visiting a page that creates a session
despite being logged out), then the session expires, then someone else
logs in and gets the same session ID (which is about a 1 in a
quindecillion chance), then the first person comes in and picks up the
second person's session.

To avoid that, if there's no UserID cookie set (or the cookie value is
0) then indicate that the SessionInfo is for a logged-out user.

No idea if this is actually what happened in T125283, but it's worth
fixing anyway.

Bug: T125283
Change-Id: I44096c69aa7bd285e4e2472959e8d892200c5f2c
2016-01-31 00:46:45 +00:00
Brad Jorsch
fcdd643a46 SessionManager: Don't save non-persisted sessions to backend storage
This introduces an in-process cache (using a HashBagOStuff) for session
data, and only saves to the external cache when the session is
persisted.

Bug: T125267
Change-Id: Ie161e0f7522cd68515b060ad8cf8c151b7198b0b
2016-01-30 00:26:03 +00:00
Brad Jorsch
305bc75b27 SessionManager: Don't generate user tokens when checking the tokens
Looking at the pre-SessionManager token checking, it's apparently valid
to log in despite user_token being empty. The stored token just gets
compared against the empty string that got returned previously.

This also cleans up some checks that assumed $user->getToken() didn't
automatically create the token if one wasn't already set.

Bug: T125114
Change-Id: Ia3d2382e96e2a0146f33fb7193a2e00ea72e51a0
2016-01-28 13:36:43 -05:00
jenkins-bot
3767f6c58c Merge "Move CSRF token handling into MediaWiki\Session\Session" 2016-01-28 07:11:27 +00:00
Brad Jorsch
296ccfd4a9 SessionManager: Save 'persisted' flag in session metadata
This allows SessionManager::getSessionById()->isPersisted() to be
reliably set. Otherwise it depends on whether the SessionBackend is
still loaded or not.

Change-Id: I17733559ac5d8fff13881664333f61d36f610b6d
2016-01-27 17:13:35 -05:00
Brad Jorsch
94ba53f677 Move CSRF token handling into MediaWiki\Session\Session
User keeps most of its token-related methods because anon edit tokens
are special. Login and createaccount tokens are completely moved.

Change-Id: I524218fab7e2d78fd24482ad364428e98dc48bdf
2016-01-27 15:27:20 -05:00
Brad Jorsch
46a565d6b0 Avoid false "added in both Session and $_SESSION" when value is null
Needs to use array_key_exists(), not isset().

Bug: T124371
Change-Id: I794f0ec793fc91ec68393443f839cfc8a154613e
2016-01-26 13:21:57 -05:00
Brad Jorsch
4eeff5b559 Use $wgSecureCookie to decide whether to actually mark secure cookies as 'secure'
The pre-SessionManager code did this, and the change in combination with
the API not honoring forceHTTPS led to T124252.

Bug: T124252
Change-Id: Ic6a79fbb30491040facd7c200b1f47d6b99ce637
2016-01-25 14:25:09 -05:00
Brad Jorsch
2c34aeea72 SessionManager: Abstract forceHTTPS cookie setting
This allows CentralAuthSessionProvider to avoid doing craziness like
this all the time:

  Set-Cookie: forceHTTPS=true; path=/; httponly
  Set-Cookie: forceHTTPS=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0; path=/; httponly
  Set-Cookie: forceHTTPS=true; path=/; domain=.wikipedia.org; httponly
  Set-Cookie: forceHTTPS=true; path=/; httponly
  Set-Cookie: forceHTTPS=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0; path=/; httponly
  Set-Cookie: forceHTTPS=true; path=/; domain=.wikipedia.org; httponly

Bug: T124421
Change-Id: I7e02afd032a246df6850208c26d3447798bc0fc2
2016-01-25 03:53:06 +00:00
Brad Jorsch
4d6d06253b Move avoidance of setting deleted cookies into WebResponse
There's no reason this should be only in CookieSessionProvider when
we're already handling deduplication in WebResponse.

Further, this fixes the bug in the existing CookieSessionProvider
implementation that a setCookie() followed by a clearCookie() wouldn't
actually clear the cookie.

This reverts commit 1ce684fcef.

Bug: T124252
Change-Id: I1098d054facacd59f03ebed7c747ec9ff6bf66e7
Depends-On: I61d14bf80fa7c857dec9cffb366dc3f84dbb4faf
2016-01-25 03:34:11 +00:00
Gergő Tisza
1ce684fcef Only delete cookies which are actually set
Some API clients seem to be confused by cookie deletion.
Prevent cookie deletion on the first leg of the API login sequence
(for a client with an empty cookie jar) by only emitting deletion
headers for cookies which are set in the current request.

Bug: T124252
Change-Id: I180e094ea32f951e22adab2ec87d16e5de7cef97
2016-01-22 16:27:32 -07:00
Brad Jorsch
43f904b51a SessionManager: Kill getPersistedSessionId()
It's not guaranteed that loadSessionFromStore() will succeed after
whatever alterations the SessionProvider might have made later in the
request.

So instead, let's make a new global object that stores the SessionId
of the persistent session that was loaded during Setup.php, if any. Then
we can check that when we need to know whether the session was
persisted.

Bug: T124468
Change-Id: I1e8e616c83b16aadd86b0a0a40826d40f6e8abe4
2016-01-22 14:47:33 -05:00
Brad Jorsch
4f5057b84b SessionManager: Change behavior of getSessionById()
It's easily possible for SessionManager::getSessionById() to not be
able to load the specified session and to not be able to create an empty
one by that ID, for example if the user's token changed. So change this
from an exceptional condition to an expected one, and adjust callers to
deal with it appropriately.

Let's also make the checks for invalid data structure when loading the
session from the store delete the bogus data entirely.

At the same time, let's change the silly "$noEmpty" parameter to
"$create" and make the default behavior be not to create an empty
session.

Bug: T124126
Change-Id: I085d2026d1b366b1af9fd0e8ca3d815fd8288030
2016-01-20 12:45:26 -05:00
Brad Jorsch
f9fd9516d9 Add "bot passwords"
Bot passwords are something like OAuth-lite, or Google's application
passwords: the bot can use API action=login to log in with the special
username and password, and will then be allowed to use the API with a
restricted set of rights.

This is intended to provide an easy migration path for legacy bots and
for bots on wikis without OAuth, since AuthManager is going to greatly
complicate non-interactive authentication. If OAuth is available, an
owner-only consumer would be a better choice.

Bug: T121113
Change-Id: Iaa4015e00edbfbfaedcc8b2d27a2d3fd25009159
Depends-On: I7e15331efb162275c4116bcae61f19d6b884cbe3
2016-01-12 22:37:44 +00:00
Brad Jorsch
ef5bd7347b Move grant and IP restriction logic from OAuth to core
This also adds code to User to allow SessionProviders to apply the grant
restrictions without needing to hook UserGetRights.

Change-Id: Ida2b686157aab7c8240d6a7a5a5046374ef86d52
2016-01-12 22:37:33 +00:00
Brad Jorsch
a73c5b7395 Add SessionManager
SessionManager is a general-purpose session management framework, rather
than the cookie-based sessions that PHP wants to provide us.

While fallback is provided for using $_SESSION and other PHP session
management functions, they should be avoided in favor of using
SessionManager directly.

For proof-of-concept extensions, see OAuth change Ib40b221 and
CentralAuth change I27ccabdb.

Bug: T111296
Change-Id: Ic1ffea74f3ccc8f93c8a23b795ecab6f06abca72
2016-01-12 21:57:01 +00:00