Commit graph

5438 commits

Author SHA1 Message Date
Erik Bernhardson
5da2c4197d Push common search api parameters into SearchApi class
We have a number of parameters that are pretty much the same between
these different search api's. Lets make them actually the same by
sharing the definitions, and then letting individual classes tweak them
as needed by removing the offset, or adjusting the max limits as
necessary.

Change-Id: I6f987db8ecb63dc943b4d2518bfe3703c677448e
2016-07-26 08:56:00 -07:00
jenkins-bot
218e89d5b4 Merge "AuthManager: Ensure neededRequests have action and username set properly" 2016-06-01 21:26:13 +00:00
Ori Livneh
b19ff38846 Clean-up of MediaWikiTestCase::checkHasGzip()
* Move the method from MediaWikiTestCase to DumpTestCase. Only subclasses of
  DumpTestCase use it, and it is not sufficiently well-designed to be a part
  of MediaWikiTestCase. (I'd want something more generic, like
  "$this->markSkippedUnlessExecutable()", rather than a method dedicated solely
  to establishing the availability of the gzip binary.)
* Fix it so that the result of the check is actually cached. It wasn't,
  previously, because of how 'static' works in PHP.
* Be content with checking that gzip is in $PATH instead of actually executing
  it.

Change-Id: Iec687a6bfe75912e1875afc3abb4fb6197a0b3aa
2016-06-01 10:44:23 -07:00
Brad Jorsch
db521e5574 AuthManager: Ensure neededRequests have action and username set properly
They were coming out as null instead, which screws up when requests are
changing their fields based on the action.

Change-Id: Ic8caf57ebad35c3eb17d45f9d96c6de5b559a83a
2016-06-01 12:13:15 -04:00
jenkins-bot
9ec4072d83 Merge "Remove a test case from PHPSessionHandlerTest::testSessionHandling()" 2016-06-01 13:21:36 +00:00
jenkins-bot
5b45919b07 Merge "Add a parser test for $wgNoFollowDomainExceptions functionality" 2016-06-01 05:37:04 +00:00
Kunal Mehta
ee5078708a Add a parser test for $wgNoFollowDomainExceptions functionality
Verify that domains on the exception list don't get a "nofollow"
attribute.

Change-Id: I01a7fc0fd9ccd21069beb26dcf3f775c79e00202
2016-05-31 22:20:36 -07:00
Ori Livneh
8a4f10931d Remove a test case from PHPSessionHandlerTest::testSessionHandling()
The test sleeps for nine seconds (3 invocations, 3 seconds per
invocation) which is difficult to sit through.

The test code sets the value of two PHP parameters, session.gc_divisor
and session.gc_probability, to 1. This may be to ensure that PHP will
invoke the session handler's gc() method when the call is made to
session_start() below. But the call to PHPSessionHandler::gc() is
immaterial, for two reasons:

- PHPSessionHandler::gc() evicts items by calling the
  deleteObjectsExpiringBefore() on the BagOStuff instance it uses for
  storage. The only BagOStuff implementation that actually uses that
  method to evict items is SqlBagOStuff, which we're not using here,
  and which would be an odd choice of a storage backend for sessions.
- PHP calls SessionHandler::gc() _after_ opening the new (or resumed)
  session and loading its data, so even if deleteObjectsExpiringBefore()
  actually did anything, it would not influence the result of the test.

Bug: T135576
Change-Id: I6e153ec8bfa5972ed45a0e6b7720832692b952fb
2016-06-01 04:46:41 +00:00
Timo Tijhof
3bbccc8da6 User: Simplify process cache by using WANObjectCache::getWithSetCallback
Follows-up 7d67b4d919, 9c733318.

* Convert loadFromId() to use getWithSetCallback() and centralise
  cache access logic there instead of spread between loadFromCache()
  and saveToCache().

* Remove process cache from User class (added in 9c733318).
  Instead, tell WANObjectCache to process-cache the key for 30 seconds.

* No need to deal with process cache in purge() because load uses slaves by
  default and may be lagged. Reads that require READ_LATEST already bypass
  the cache.

* Remove saveToCache() and move logic to loadFromCache().
  It was technically a public method, but marked private and no longer used
  in any extensions.

* Remove redundant isAnon() check in loadFromCache().
  This is already done by loadFromId() and loadFromDatabase().

* Remove hasOrMadeRecentMasterChanges() check. It was used to add READ_LATEST
  to the flags. However, this check only occurred if either READ_LATEST was
  already set, or after consulting cache. Which means in general, it never
  does anything. If we want to keep this, we should probably move it higher up.

* Let WANObjectCache handle cache version. That way, there is no longer separate
  logic for "populate cache" and "cache lookup failed". Instead, there is
  just "get data" that tries cache first.

  I've considered moving the version into the cache key (like we do elsewhere)
  but that would be problematic here since User cache must be purgeable
  cross-wiki and other wikis may run a different version (either in general,
  or even just during a deployment). As such, the key must remain unchanged when
  the version changes so that purges from newer wikis affect what older wikis see
  and vice versa.

Change-Id: Icfbc54dfd0ea594dd52fc1cfd403a7f66f1dd0b0
2016-05-31 21:13:08 -07:00
jenkins-bot
c54576b341 Merge "objectcache: Support key versioning in WANObjectCache" 2016-05-31 22:48:24 +00:00
Derk-Jan Hartman
844199e67c resourceloader: Strip leading BOM when concatenating files
We read files and concatenate their contents. Files may start with a BOM character.
BOM characters are only allowed at the beginning of a file, not half way.
Stripping it should be safe, since we already assume that everything is UTF-8.

Change-Id: I14ad698a684e78976e873e9ae2c367475550a063
2016-05-31 19:47:19 +01:00
jenkins-bot
d9fab38793 Merge "AuthManager fixups around the login→RESTART→create flow" 2016-05-31 16:41:53 +00:00
Brad Jorsch
9bb2875e2e AuthManager fixups around the login→RESTART→create flow
* ApiQueryAuthManagerInfo will differentiate between preserved linking
  data and a preserved createRequest.
* ApiQueryAuthManagerInfo will indicate the preserved username, if any,
  because the client will have to pass that back to action=createaccount.
* ApiClientLogin won't tell about the confusing
  CreateFromLoginAuthenticationRequest returned on RESTART responses.
* Explain how 'preservestate' works in ApiAMCreateAccount's auto-doc.
* ConfirmLinkSecondaryAuthenticationProvider will filter out requests
  that can no longer be used (i.e. if it was for linking the account
  that got used for creation).
* All the complicated code in AuthManager::beginAccountCreation() was
  trying to deal with allowing the client to pass only the
  CreateFromLoginAuthenticationRequest. That was dumb, removed it.
* Added methods to CreateFromLoginAuthenticationRequest to indicate its
  status with respect to different kinds of preserved state.
* Increase accuracy of the AuthenticationResponse::$createRequest doc.

Change-Id: I726d79de18e739d6e60c1eea51453433c21ba207
2016-05-31 11:44:02 -04:00
jenkins-bot
d6ac60cf08 Merge "Typo fix for AuthPluginPrimaryAuthenticationProvider::providerAllowsAuthenticationDataChange" 2016-05-30 12:31:51 +00:00
jenkins-bot
0acd748648 Merge "Fix required field calculation in AuthenticationRequest" 2016-05-30 12:22:30 +00:00
Gergő Tisza
d0e6051b5c Fix required field calculation in AuthenticationRequest
Instead of only flagging fields which are required by a request
needed by all primairy providers, it should be enough if all
requests needed by some primary provider require that field.

Also make CreationReasonAuthenticationRequest non-required so that
the list of required form fields is more in sync with that of
pre-AuthManager code.

Bug: T85853
Change-Id: I9d33bd22295758cc532a260b1848616b41d94f12
2016-05-30 11:57:58 +00:00
Gergő Tisza
32673970aa Typo fix for AuthPluginPrimaryAuthenticationProvider::providerAllowsAuthenticationDataChange
Change-Id: I7c05ea91009cdf765b06438e055de891e0edd1f4
2016-05-29 14:00:23 +00:00
Ori Livneh
acca48094c Make number of PBKDF2 iterations used for deriving session secret configurable
The intent is both to allow the number of iterations to be dialed up (either as
computational power increases, or on the basis of security needs) and dialed
down for the unit tests, where hash_pbkdf2() calls account for 15-40% of wall
time. The number of iterations is stored in the session, so changing the number
of iterations does not cause existing sessions to become invalid or corrupt.
Sessions that do not have wsSessionPbkdf2Iterations set (i.e., sessions which
precede this change) are transparently upgraded.

Change-Id: I084a97487ef4147eea0f0ce0cdf4b39ca569ef52
2016-05-28 07:06:30 -07:00
Kunal Mehta
b74c4b2f95 Improve @covers tags for LinkerTest
These hooks are now called from LinkRenderer, so make sure it is
covering the right code.

Change-Id: Ifaa28d471f585dce9d968cc1173c7fdceb408239
2016-05-27 19:30:42 -07:00
Kunal Mehta
f1d3f704df Fix @covers tags for LinkRenderer tests
phpunit's coverage report needs class names that include the namespace,
otherwise it'll just fail.

Bug: T136420
Change-Id: Ie748237176ea1363b35d73084e63e6fafe808286
2016-05-27 11:53:04 -07:00
Arlo Breault
20bdcc7620 Sync up with Parsoid parserTests.
This now aligns with Parsoid commit b0d48342e0c540c17b2c073833d4a7ab147e2852

Change-Id: I9eb00fdc854eac7ddfc37520bd6b3cf8913db523
2016-05-26 19:48:17 -07:00
Kunal Mehta
5f8a771f7e Add tests for Linker::getLinkColour()
Change-Id: Ic1553e21def47f5c4923ba747146b36b0b3ffdfc
2016-05-26 14:47:45 -07:00
Kunal Mehta
53ccc289fa ChangesList: Use LinkRenderer instead of Linker::link()
Change-Id: Iae32a9e365aad268d2671df6a0b916e4d9c0a801
2016-05-26 14:47:45 -07:00
jenkins-bot
3b197fd9e4 Merge "LinkRenderer: Re-implement noclasses as makePreloadedLink function" 2016-05-26 21:22:00 +00:00
jenkins-bot
00aa8293b5 Merge "TitleParser: In formatTitle(), don't throw exceptions on bad namespaces" 2016-05-26 21:01:39 +00:00
Ori Livneh
e638075936 Whenever possible, reuse User objects in unit tests
The unit tests spend nearly half of their run time resetting the user table for
each test. But the majority of tests do not depend on the user table having the
exact value that the setup code resets it to, and do not need to modify the
user objects they require to run.

Fix that by providing an API for tests to get User objects, and to indicate
whether the User object will be subject to destructive modification or not.
This allows User objects to be reused across multiple unit tests.

Change-Id: I17ef1f519759c5e7796c259282afe730ef722e96
2016-05-26 20:42:31 +00:00
Kunal Mehta
a22ab0eab0 TitleParser: In formatTitle(), don't throw exceptions on bad namespaces
This ocassionally happens for whatever reason, but it doesn't really
make sense to throw an exception when creating a broken-looking link
would also work. We already do this for TitleParser::getPrefixedDBkey(),
and this also matches the behavior of Title::getNsText().

Bug: T136352
Bug: T136356
Change-Id: Ic7eb17f8917f7fbb28b11d94b742dac1fe5582a1
2016-05-26 13:34:18 -07:00
Timo Tijhof
0233903960 mediawiki.special.recentchanges: Use module.exports instead of mw.special
This isn't generally used as a public method. The only reason it's exposed is
for an integration test to access it.

This commit and others prepare for removal of the 'mediawiki.special.js' file.
This script does nothing other than create "mw.special = {};". While that init
pattern is common in extensions and not wrong, 'mediawiki.special' violates
T92459 due to having styles. Given there's only script uses of it, it's easiest
to remove it in favour of the new module export pattern.

Change-Id: I2e78828828601e1160550efe02c07172ac32e985
2016-05-26 17:45:39 +00:00
Kunal Mehta
a95e8e7262 LinkRenderer: Re-implement noclasses as makePreloadedLink function
'noclasses' makes more sense as a per-link option rather than an
instance member of the LinkRenderer instance, since it depends entirely
on whether the calling code has preloaded the link classes.

Introduce LinkRenderer::makePreloadedLink() which makes this clear and
requires passing in the classes as a separate parameter. As a side-
effect, due to the way LinkRenderer::mergeAttribs() is implemented, the
'class' attribute will always appear before the 'title' attribute in the
final output.

Change-Id: I0545aa9d7139794bc22f9d3d6d6eccde003b2982
2016-05-26 00:24:06 -07:00
Aaron Schulz
0a38dbc809 objectcache: Support key versioning in WANObjectCache
* getWithSetCallback() takes a 'version' parameter.
* If the value at a key has a different version, then
  getWithSetCallback() will automatically use a separate
  key. Which value "wins" the main key does not matter.
* Purges are handled by using the main key as a sort of
  check key (with no hold-off). Note that this key is always
  purged on delete().
* Changed stash keys to track the same info as other keys
  both for consistency and because this change needs the
  generation timestamp. Renamed the stash prefix to avoid
  corrupt results with Het Deploy.
* This is useful for things like the User class that use
  versioning and have cross-wiki key access and purges.
  Currently, bumps to version must be deployed to all wikis
  at once, which this aims to avoid.

Change-Id: I26ae62f116e32b48bcf06bc13f8b9e79ae976745
2016-05-25 18:03:21 +00:00
jenkins-bot
b7a3feff1b Merge "Remove explicit tabindex from diff links" 2016-05-25 13:13:33 +00:00
Derk-Jan Hartman
17221b41cb Remove explicit tabindex from diff links
These were added to Special:RecentChanges in 2004, but it doesn't
match what we do in any of the other lists. For accessibility
purposes, in flow indexing is preferred these days, or alternatively
a JS controlled roving tabindex, but this was neither.

Bug: T116127
Change-Id: Id455fafe4bdea40fb5988bdec14eed672844c8e3
2016-05-25 00:46:54 +02:00
jenkins-bot
0df66aa5eb Merge "Disable CAS check when saving TestUser data." 2016-05-24 11:13:44 +00:00
daniel
8f4587b343 Disable CAS check when saving TestUser data.
During testing, we are not worried about data loss, so we can safely
bypass the CAS check when setting up a test fixture.

This change was added to address sporadic test failures like the following:

18:03:38 1) ApiEchoMarkReadTest::testMarkReadWithList
18:03:38 MWException: CAS update failed on user_touched for user ID '2' (read from slave); the version of the user to be saved is older than the current version.
18:03:38
18:03:38 /mnt/jenkins-workspace/workspace/mediawiki-extensions-hhvm/src/includes/user/User.php:3931
18:03:38 /mnt/jenkins-workspace/workspace/mediawiki-extensions-hhvm/src/tests/phpunit/includes/TestUser.php:83
18:03:38 /mnt/jenkins-workspace/workspace/mediawiki-extensions-hhvm/src/tests/phpunit/includes/api/ApiTestCase.php:30
18:03:38 /mnt/jenkins-workspace/workspace/mediawiki-extensions-hhvm/src/extensions/Echo/tests/phpunit/api/ApiEchoMarkReadTest.php:11
18:03:38 /mnt/jenkins-workspace/workspace/mediawiki-extensions-hhvm/src/tests/phpunit/MediaWikiTestCase.php:370

Bug: T131178
Change-Id: I99b43e0db85bc2c1cd335c82971df4e95520d34b
2016-05-24 12:58:22 +02:00
jenkins-bot
8b9584646b Merge "Add LinkRenderer (rewrite of Linker::link())" 2016-05-24 03:29:32 +00:00
jenkins-bot
f8cc99ea98 Merge "Avoid invidual LinkCache lookups in Linker::makeBrokenImageLinkObj()" 2016-05-24 03:29:28 +00:00
jenkins-bot
9a309c92e6 Merge "Remove 'noclasses' from Linker::linkKnown() defaults" 2016-05-24 03:07:01 +00:00
Kunal Mehta
67e62c0b25 Add LinkRenderer (rewrite of Linker::link())
This is a rewrite of Linker::link() to a non-static, LinkTarget-based
interface. Users of plain Linker::link() with no options can use the
LinkRenderer instance provided by MediaWikiServices. Others that
have specific options should create and configure their own instance,
which can be used to create as many links as necessary.

The main entrypoints for making links are:
* ->makeLink( $target, $text, $attribs, $query );
* ->makeKnownLink( $target, $text, $attribs, $query );
* ->makeBrokenLink( $target, $text, $attribs, $query );

The order of the parameters are the same as Linker::link(), except
$options are now part of the LinkRenderer instance, and
known/broken status requires calling the function explicitly.
Additionally, instead of passing in raw $html for the link text, the
$text parameter will automatically be escaped unless it is specially
marked as safe HTML using the MediaWiki\Linker\HtmlArmor class.

The LinkBegin and LinkEnd hooks are now deprecated, but still function
for backwards-compatability. Clients should migrate to the nearly-
equivalent LinkRendererBegin and LinkRendererEnd hooks.
The main differences between the hooks are:
* Passing HtmlPageLinkRenderer object instead of deprecated DummyLinker
* Using LinkTarget instead of Title
* Begin hook can no longer change known/broken status of link. Use the
TitleIsAlwaysKnown hook for that.
* $options are no longer passed, they can be read (but shouldn't be
modified!) from the LinkRenderer object.

Bug: T469
Change-Id: I057cc86ae6404a080aa3c8e0e956ecbb10a897d5
2016-05-23 12:00:09 -07:00
jenkins-bot
f826f2f5f6 Merge "add LanguageTest::testEquals for Id7ed6a21c" 2016-05-23 16:07:06 +00:00
daniel
bbd518baff add LanguageTest::testEquals for Id7ed6a21c
Change-Id: I99ea4c51bfc5245eab0bcca73870c56a6fab2c43
2016-05-23 16:45:06 +02:00
umherirrender
72632115d6 Fix various phpcs error from last security patches
Found by tests:
https://integration.wikimedia.org/ci/job/mediawiki-core-phpcs-trusty/1069/console

Breaking merges

Change-Id: If01b94705cd7b939ac380053730b1b602c838a8e
2016-05-20 20:20:36 +02:00
Brian Wolff
13ece3550e Add rel="noreferrer noopener" when target attribute would open window
noreferrer is used as support for noopener is very limited.
This is to prevent the attack detailed at
https://mathiasbynens.github.io/rel-noopener/ where you can
navigate the parent window, even if the new window is a cross-origin.

Bug: T133507
Change-Id: I6e4ab938861e246ff44048077b94847e303f1859

Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
2016-05-20 09:49:41 -07:00
Brad Jorsch
f459c1aeca SECURITY: Improve cross-domain-policy mangling
Take into account that the tag might have parameters.

Bug: T123653
Change-Id: Ie9799f5ea45badfb4e7b4be7e7fbc1c35cc86f26

Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
2016-05-20 09:48:11 -07:00
csteipp
fdc70074bb SECURITY: Don't use m modifier when checking link prefix
SVG filter incorrectly used the m modifier when checking if an href
attribute started with 'https?://', incorrectly matching attributes
such as, "javascript:alert('&#10;http://foo')".

Bug: T122653
Change-Id: I41291fff344241cad3171f3e8050de99b62a2296

Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
2016-05-20 09:47:45 -07:00
Brian Wolff
7e4a134f49 SECURITY: Include quote characters in strip markers so esc in attr
Strip markers get substituted for general html, which means the
substitution text general does not escape quote characters. If
someone can convince MW to put a strip marker in an attribute,
you can get around escaping requirements that way. This patch
adds the characters `"' to the strip marker text. At least one
of these characters should be escaped inside attributes (regardless
of what quote character you use for attributes), thus normal html
escaping will deactivate the strip markers, preventing the
vulnrability.

This will break any extension that escapes input with htmlspecialchars,
to add to html/half parsed html output, but assumes that strip markers
are unmangled. I don't think its very common to do this. The primary
example I found was some core usages of Xml::escapeTagsOnly(). (And
even in that case, it only affected the corner case of being called
via {{#tag:..}})

Based on MatmaRex's suggestion.

Change-Id: If887065e12026530f36e5f35dd7ab0831d313561

Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
2016-05-20 09:25:49 -07:00
Brad Jorsch
9ec1ef7308 API: Add "standard" header and hook for lacksSameOriginSecurity()
The header is intended for use with XMLHttpRequest when the request
might be part of an XSS. The hook is for extensions that might need to
add additional checks of some sort.

Bug: T98313
Change-Id: I0e5f2d3b29a79a12461dc33c90c812a56810f536

Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
2016-05-20 09:25:14 -07:00
Kunal Mehta
1f7d032f83 Avoid invidual LinkCache lookups in Linker::makeBrokenImageLinkObj()
Change-Id: I29ab072519937b770e75a40382d2f77cbabe098b
2016-05-19 19:09:05 -07:00
Kunal Mehta
c66b6a1e43 Remove 'noclasses' from Linker::linkKnown() defaults
The intention for Linker::linkKnown() was to be used when the caller had
already preloaded the target's existence ('known') and called
Linker::getLinkColour() directly ('noclasses'). However, nearly all
usage of linkKnown() only did the first part, and not the latter.

So do what people actually ended up using the function for, and remove
'noclasses' from the default parameters. As long as the target the link
is being created for is already in LinkCache, this shouldn't cause any
extra database queries.

Change-Id: Ia5a4c2f18ec780627146617a1498bd04fcfbb3ee
2016-05-19 19:07:58 -07:00
jenkins-bot
0433d5feb7 Merge "PHPUnit: turn off verbose option" 2016-05-19 20:45:25 +00:00
jenkins-bot
5fd224c00b Merge "Allow resources to be salvaged across service resets." 2016-05-19 12:45:16 +00:00