Commit graph

63 commits

Author SHA1 Message Date
Brad Jorsch
1ecfef7a3d Log a backtrace with "sessions are supposed to be disabled" message
Bug: T127233
Change-Id: I27ca561ab5967cde270f2b665b51da687f0c081a
2016-03-09 18:07:41 +00:00
Siebrand Mazeland
5b119a0e44 Replace uses of join() by implode()
All of core uses implode() consistently now.

Change-Id: Iba50898c64c43f356d1caf8869f484e90d9ff651
2016-03-08 18:24:16 +00:00
addshore
fee0afdc8a Move WatchedItem logic to WatchedItemStore
This also removes assumptions that when a page
in one Namespace should be watched / removed
that the page in the talk / subject ns for the
page should have the same action applied

This should maintain all backward compatability
for the WatchedItem class

This also includes tests written by:
 - WMDE-leszek
 - Addshore

Bug: T127956
Change-Id: Iad9abafe4417bb479151a3bfbee6e1c78a3afe3c
2016-03-08 15:41:22 +00:00
Gergő Tisza
664ba620e9 Prevent "Failed to create..." warnings when session loading fails
If there is an existing session for a given ID but loading it fails,
there is no point in trying to create a new empty session with that
ID. Just fail silently (the reason for not loading the session
should be logged elsewhere), don't spam the logs and don't slow
down execution by throwing and catching an exception.

Change-Id: I8299872cc29c32cb245686ed0bca6b9a5902cdc1
2016-03-02 15:36:02 -08:00
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
Gergő Tisza
20a247e40e Do not vary on session cookies when the session is disabled
MW_NO_SESSION is supposed to be used in a deterministic way
(either always or never for a given endpoint) so there is
no point to split the cache on session cookies when it is
used (except when it's set to 'warn' in which case it is
not really used).

Bug: T127233
Change-Id: If44e0ec2167359e081e1b4cd4d21e62bd357d23e
2016-02-25 12:51:32 -08:00
Ori Livneh
43420a0506 SessionBackend: skip isUserSessionPrevented check for anons
For anon requests, the call to SessionManager::isUserSessionPrevented(
$this->user->getName() ) is both expensive (because of the need to
sanitize the IP) and pointless, because the session-prevention feature
is intended for named accounts. So short-circuit the check if the user is not
logged in.

Change-Id: I17386b97e229b492723b46db1e1ae16fd4b0fc5a
2016-02-24 15:36:10 -08: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
jenkins-bot
1dd2acf48c Merge "Enforce MW_NO_SESSION, add MW_NO_SESSION_HANDLER" 2016-02-22 23:49:12 +00:00
Brad Jorsch
f61cb18b71 Enforce MW_NO_SESSION, add MW_NO_SESSION_HANDLER
When an entry point specifies MW_NO_SESSION, actually enforce that by
having both SessionManager and PHP's session handling (session_start()
and friends) throw exceptions.

If an entry point needs the old behavior of using PHP's default session
handling (as defined in php.ini), it should define
MW_NO_SESSION_HANDLER instead of or in addition to MW_NO_SESSION.

This also makes PHPSessionHandler be installed in CLI mode, where it
wasn't installed before.

Bug: T127233
Change-Id: I2a3db06ee8e44a044096c57a819b5fd5e51c5c5c
2016-02-22 12:17:31 -05:00
Brad Jorsch
405c2c8b6b Fix session store logging
Two things were wrong here:
1. SessionManager shouldn't override the configured logger of the
   underlying store.
2. CachedBagOStuff shouldn't override the configured logger of the
   backend store on construction.

Change-Id: I24ed5a81d5b238a42934a7890dd8d0a9f9bb3b6f
2016-02-22 17:13:03 +00:00
jenkins-bot
5518125474 Merge "SessionManager: Autocreate should use READ_LATEST when necessary" 2016-02-18 19:23:28 +00:00
Brad Jorsch
a30ca127b7 SessionManager: Autocreate should use READ_LATEST when necessary
If we failed to fetch the user ID using the default flags but succeeded
with READ_LATEST, we should also be passing READ_LATEST to
$user->loadFromId().

Change-Id: If8fc332810c2703773a153760ad076c398a84e4e
2016-02-18 11:13:33 -05:00
Gergő Tisza
c4c501d9bb Do not unauthenticate if autocreation fails due to a race
Bug: T70012
Change-Id: I523ee94744ac943ede78af59ab381b65ae26e672
2016-02-18 03:13:36 +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
jenkins-bot
ebdd92fcd1 Merge "CookieSessionProvider: It's persisted if we have a 'Token' cookie" 2016-02-16 23:55:44 +00: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
Brad Jorsch
220ec673f5 CookieSessionProvider: It's persisted if we have a 'Token' cookie
This aspect of the pre-SessionManager Setup.php code had been
overlooked.

This also updates an inaccurate doc comment that I happened to notice.

Bug: T125313
Change-Id: Id49836086be837012b02885ce5384ae6a4ebc105
2016-02-16 12:13:32 -05: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
jenkins-bot
03dbbef54f Merge "SessionProvider::mergeMetadata: Log additional data" 2016-02-11 21:30:55 +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
0b54186340 Fix invalid key warning in CookieSessionProvider error handling code
Change-Id: I68172ff5fcd6acaab7ecf298ed4a47f23cb8c9f7
2016-02-11 10:34:56 -08: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
e089a5c023 Merge "PHPSessionHandler: Implement SessionHandlerInterface" 2016-02-10 20:37:27 +00: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
Brad Jorsch
0f6b181c73 PHPSessionHandler: Implement SessionHandlerInterface
Now that support for PHP 5.3 has been dropped, we can do this.

Change-Id: If7fa9801194683eea6764e5748157d8a66a616df
2016-02-10 11:49:19 -05: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
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
jenkins-bot
a55b821f3a Merge "Disable automatic cache headers associated with starting a session" 2016-02-04 16:29:20 +00: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
Bryan Davis
514725e7fe Disable automatic cache headers associated with starting a session
Follow up to 7491b52. The 'private, must-revalidate' argument to
session_cache_limiter() does not match any expected values for the
function. This results in the PHP runtime treating it like the
documented empty string argument which completely disables the automatic
addition of cache related headers. Change the implementation to use the
empty string argument explicitly rather than continuing to rely on
the undocumented and potentially confusing existing behavior.

session_cache_limiter( '' ) is called unconditionally in
MediaWiki\Session\PHPSessionHandler::install(). This is safe now that it
is understood that we are disabling the setting of the automatic
headers.

Bug: T124510
Change-Id: I63164f8b7a408e370ff01dead42be27a0135dd35
2016-02-03 21:45:28 +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
6acd424e0d SessionManager: Notify AuthPlugin before calling hooks
This avoids a race in CentralAuth:
* The user doesn't exist locally, so CA wants to create it
* Auto-creation adds the user to the database
* A hook function tries to access the session; now the user does exist
  locally but isn't yet attached, so CA rejects the session.

Bug: T125283
Change-Id: I6024885e3cf9c85c527fc160577f66ff97451c98
2016-02-01 06:43:29 +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
Gergő Tisza
13f2f09a19 SECURITY: Fix User::setToken() call on User::newSystemUser
This was supposed to reset the user token but did set it to '1'
because User::setToken accepts bool/string but only treats true
as bool.

Bug: T125161
Change-Id: Ia4196eba92cd4d170a3023db0f540a2972ffad4f
2016-01-29 19:40:21 -06: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
b5c0c03bb7 SessionManager: Save user name to metadata even if the user doesn't exist locally
Bug: T125133
Change-Id: I12ff84db614f0d5839457aa92be58c62b3a4e8fd
2016-01-28 16:22:02 -05: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