Commit graph

79 commits

Author SHA1 Message Date
Amir Sarabadani ead29fc0fe Re-enable skipped test
Depends-On: I04c1dcf5129df437589149f0f3e284974d7c98fa
Change-Id: Ib071cd62d21090d28088729d2b2850363b750dae
2023-08-25 17:06:15 +02:00
Amir Sarabadani 2745b58451 Mark FilteredActionsHandlerTest::getFilteredActionsHandler skipped
Temporarily as I rename Status class.

Change-Id: I5f89ecf27ce1471a74f31c6018806461781213c3
2023-08-25 14:59:08 +02:00
Derick Alangi 623b9dbea3
Migrate DeferredUpdatesManager to use DeferredUpdates directly
Reverts part of Id9056528a433faf0, to switch to DeferredUpdates in
CirrusSearch back from DeferredUpdatesManager.

Bug: T265749
Change-Id: I8126cc76440724753c356c48ba4e0fcc9be5b41a
2023-08-21 12:59:28 +01:00
Umherirrender cd7e9d31a7 Use namespaced Title
Bug: T321681
Change-Id: I66fd9b70a5de06ac3c81bdf6a2a5bca64ed094c2
2023-08-19 19:49:36 +02:00
Daimona Eaytoy a10765baec Use real user ID in EchoNotifierTest
Echo will at some point try to load the user with the given ID, and will
throw an exception if it doesn't exist. The test is currently passing
just because we're not properly cleaning DB tables, and the user with ID
1 happens to exist at that point, but it will fail with core change
Ie2f1809d.

Change-Id: Ie686f4d5c2842e45a6ed564b311bb5d9b0265091
2023-08-18 01:51:09 +02:00
Umherirrender 8e6d820154 Use HookHandlers for UserMerge hook
Bug: T254306
Depends-On: Ib78dae49854863af1a37a00636737c94694776ae
Change-Id: If2bc517fb612199f4d744efbfa261ee5c556b6a5
2023-08-16 09:44:30 +02:00
jenkins-bot 4f6ff34a26 Merge "tests: Avoid DB access in non-Database tests" 2023-08-07 13:13:27 +00:00
jenkins-bot e566cb1327 Merge "Avoid TestUser in non-database tests" 2023-08-07 13:13:25 +00:00
Daimona Eaytoy dcef8cebc6 tests: Avoid DB access in non-Database tests
These tests were accessing the Database, for mainly 3 reasons:
- User::newSystemUser
- Static methods in ChangeTags
- Echo's Event class

There isn't much we can do about them, so add tests to the Database
group where needed. In some cases, there are already comments that these
tests should be made unit tests once possible.

Bug: T155147
Change-Id: I8a0d52e0a4cae8a4059b62867853a73e60c878a1
2023-08-06 22:19:03 +00:00
Daimona Eaytoy 2a6b7edecb Avoid TestUser in non-database tests
TestUser requires a DB connection, so avoid using it in database-less
tests. Add to the Database group tests that are making DB writes (e.g.,
for log entries).

Change-Id: I211cb60296e5c2446128fcdf2caaadc728a8c272
2023-08-06 22:18:49 +00:00
Amir Sarabadani 049e602b07 BlockedDomains: Move filtering logic to a dedicated class
I'm planning to add support for bypass and regex-based blocking which
means it'll grow a bit. So let's give it a dedicated class.

Bug: T337431
Change-Id: I5a6fe2fd2f1efdebd8cada0ba6c481341f830e27
2023-08-06 16:27:23 +02:00
AnaïsGueyte 2efd6d9ac9 Replace userNameUtils with UserIdentityUtils
Where UserIdentity is available and where it's necessary to check ::isNamed

Bug: T342741
Change-Id: I5b52686f1c072282e76874f3863962345ca8097e
2023-07-27 10:29:31 -03:00
Daimona Eaytoy 2a89b3fb6d Skip CheckUserHandlerTest if CheckUser is not installed
The handler class uses hook interfaces from the CheckUser extension, so
it can't run if CheckUser is not installed.

Change-Id: I5f40366f27cc885e95e1bb93ec421b09c7caa9a6
2023-07-15 22:04:42 +02:00
Amir Sarabadani 85639c857a Re-enable skipped tests with fixes
Depends-On: Ia55cb6cbdb28484e820f9cf3d6aacac00a86ffab
Bug: T341828
Change-Id: Id0aad8aeb7b5380f4d934d9133abf0e12dee29fe
2023-07-13 23:01:22 +02:00
Amir Sarabadani 4eab548a02 Temporarily skip tests being broken in READ NEW of externallinks
Bug: T341828
Change-Id: I2d4ef8fc3bffa43bc611af3eab0494f4900df557
2023-07-13 22:42:47 +02:00
thiemowmde 24888bea15 Mark protected stuff in classes with no subclasses as private
Protected effectively means "public to subclasses" and should be
avoided for the same reasons as marking everything as public should
be avoided.

Change-Id: Iba674b486ce53fd1f94f70163d47824e969abb77
2023-06-23 12:28:06 +02:00
Timo Tijhof 203d54be11 BlockedExternalDomains: Optimize host extraction by using parse_url
Unlike what the 20-year old source comments in UrlUtils.php would
have you believe, parse_url() works fine nowadays, including for
protocol-relative URLs and indeed lots of prod code uses it directly.

The class still has some convenience value for case where you need to
expand or manipulate URLs, but for the common case of extracting a part
of it, you really don't need it.

Test plan:
$ php phpunit.php ../../extensions/AbuseFilter/tests/phpunit/integration/FilteredActionsHandlerTest.php

Bug: T337431
Change-Id: I1e76d2f5aef65365743214530faba656325b965a
2023-06-19 13:36:27 +00:00
Amir Sarabadani 8b67de5bc1 blocked domains: Make sure users can't bypass the list by using uppercase
Added tests too

Bug: T337431
Change-Id: Ie3406d0b3c7d82ba44c11865e493375453555664
2023-06-16 01:22:48 +02:00
jenkins-bot 596a36866b Merge "Add missing AbuseFilterServices::getHookRunner()" 2023-06-15 18:06:28 +00:00
thiemowmde 7e6132d4d7 Remove bits of unused code across the codebase
Mostly found with the code inspection tools in PHPStorm.

Change-Id: I7f59dddca0aaab0ddd1093d52c07ec12efd20d6d
2023-06-14 19:41:00 +00:00
Lucas Werkmeister 9bb4b1e5db Add missing AbuseFilterServices::getHookRunner()
And register AbuseFilterRunnerFactory as a service name that’s allowed
to not have a getRunnerFactory() method without the test complaining
(the service was renamed, getFilterRunnerFactory() exists).

Change-Id: Idedb87e64a6df02b0edae8d9e7dbf441752dc480
Needed-By: If5af88e7f70b83d53f66b9617a5ef37daf81830f
2023-06-14 17:35:43 +02:00
Matěj Suchánek 8fb53edfbb Retrieve external links from PreparedUpdate
When forFilter is true and PreparedUpdate is available
(most save operations), retrieve all_links from
PreparedUpdate::getParserOutputForMetaData. Otherwise
do what was done before.

Note that this change probably leaves some dead code. It will be dealt
with later.

NOTE: this changes code potentially executed on every save operation.

Bug: T65632
Bug: T264104
Change-Id: I3628a56e5277846c1b90444fb55983870eb54c1e
2023-06-13 14:30:06 +02:00
Amir Sarabadani 60cbc3b464 BlockedDomains: Use cleaner array building and add tests
Regarding array building: Instead of adding to array with
$array[] = 'foo' and then doing array_flip(), simply do
$array['foo'] = true;

Regarding tests: I originally wanted to create a unit test but I ended
up mocking so many things that it wasn't worth it and the config variable
is globaly which first we need to clean up after deployment is done.

Bug: T337431
Change-Id: Iac8dca7078668ee3441d19b6aafe499c1aa0d732
2023-06-12 17:46:55 +00:00
Amir Sarabadani 0acfe05251 Add abusefilter-bypass-blocked-external-domains right
This is similar to sboverride right in SpamBlacklist. Defaults are also
the same

Bug: T337431
Change-Id: Iaff91c1f9f7aece0787348dd071701ef99e0291d
2023-06-08 22:06:19 +02:00
Amir Sarabadani 53eb27f086 Introduce Special:BlockedExternalDomains
It is behind a feature flag. Improvements on it can happen in follow
ups. The patch is already quite massive.

Bug: T337431
Bug: T279275
Change-Id: I3df949c4d41ce65bb4afa013da9c691ac05fc760
2023-05-30 20:48:42 +02:00
Umherirrender faaa5126eb tests: Make some PHPUnit data providers static
Initally used a new sniff with autofix (T333745)

Bug: T332865
Change-Id: I892127a7cf794c52b1106d0239d273476a6113c3
2023-05-20 21:44:55 +02:00
Bartosz Dziewoński 0364194d72 API tests: Assert error codes, not error messages
Depends-On: I752f82f29bf5f9405ea117ebf9e5cf70335464ad
Needed-By: Ie17987991d1e9a0d77da97e3a81fe0a21c6d7866
Change-Id: I06c89534be605557ee9b0d90d2748f806fa2ae9e
2023-04-26 13:21:53 +02:00
Matěj Suchánek 0628dbdab6 Add tests for extension.json and services
Change-Id: Ie83e4a85a408e1ba1d2cc827c4bf353bdd5500df
2023-03-28 09:35:02 +02:00
Matěj Suchánek bb78cb0a56 Use actor table in AbuseFilter
This patch migrates abuse_filter and abuse_filter_history tables
to new actor schema.

MigrateActorsAF was copy-pasted from core's
maintenance/includes/MigrateActors.php before removal (ba3155214).

Bug: T188180
Change-Id: Ic755526d5f989c4a66b1d37527cda235f61cb437
2023-03-22 14:01:29 +01:00
Matěj Suchánek 86ac5bdb40 Clean up database access in non-deployed code
Change-Id: Ibcc41c2dd7f60a806199eaa2c47628a28dadd143
2023-03-03 18:55:08 +01:00
Matěj Suchánek 702d77e3ce Create real integration test for variables
For fixing bugs like T65632, T105325, or T264104, we will need
to update code in more than one place at once. To prevent
regressions, create an integration test which tests the whole
pipeline, from the request submission to variable evaluation.
Edits are simulated using action=edit API call because the hook
AbuseFilter uses is run from EditPage.

To increase confidence in test coverage, remove some annotations
from AbuseFilterConsequencesTest or make them less greedy.
Ideally, it would only test consequences.

This patch includes refactoring of AbuseFilterCreateAccountTestTrait
which now only inserts the user into the database if it really
should be created.
It also restores test coverage of some other classes.

Change-Id: I661f4e0e2bcac4770e499708fca4e4e153f31fed
2022-11-26 18:51:38 +01:00
Reedy 4f4f01f96d EchoNotifierTest: Use namespaced Event class
Re-enables test

Depends-On: Ib57ea2db947285946f31fa9912b37181044df9d3
Change-Id: I082868f4759a5da14235803ebd8a80e794cfe41c
2022-11-12 06:28:33 +00:00
Reedy 97e0f30155 EchoNotifierTest: Temporarily skip testNotifyForFilter
Depends-On: Iddb4a5d4057f9c6ed00f754d2e3cd79cd873f212
Change-Id: Id28792658de950b99a8786f881563476def59eba
2022-11-03 00:28:15 +00:00
Umherirrender da7683bcbc tests: Improve tests for postgres
Change-Id: I9720b6c7d096ae8415c00eb0ac1ddc461ea0a8dc
2022-07-09 21:40:27 +00:00
Matěj Suchánek e7492a230f Replace unnecessary use of User
In action=abusefilterunblockautopromote, leave UserIdentity
instantiation to the parent. Note that this changes the "code"
in the response from "baduser_user" to "baduser".

Change-Id: I97d2bf3fa3c5486e461823f840cad2763e1bcfea
2022-07-02 23:58:08 +00:00
DannyS712 139ca18efe Migrate AbuseFilterPermissionManager to authority
Almost all callers already provide an Authority in the form
of a User object, so mostly just need to change the typehints

Depends-On: I58661943c7e1acb6ff09798ee1a30be0fde3f459
Change-Id: I2ad86859c8194c14d7331f58db62b7cff4698085
2022-07-01 06:58:17 +00:00
Umherirrender 32a97e8d15 tests: MWTimestamp::setFakeTime is reset by core
It is in MediaWikiTestCaseTrait since 438b392

Change-Id: Ib89406fdbad0c9fecada50c8f1ee45e27d17c522
2022-06-28 20:48:31 +00:00
Matěj Suchánek 7ae2060b27 Avoid array to object cast in filterToDatabaseRow
Both callers immediately call get_object_vars
to cast it back to array. Avoid this roundtrip.

Change-Id: I6525d76f8a03a4d28c2b50b580c539affe98064f
2022-06-28 18:46:28 +00:00
Thiemo Kreuz bbded6231c Inline/simplify smaller pieces of duplicate/complex PHP code
Change-Id: I59d0f17b77c8c3d47bc532bdefd9d8c0883f180b
2022-06-03 21:04:38 +02:00
Daimona Eaytoy 8ee9a21750 Clean up test files
Convert a few integration tests to unit tests now that it's possible,
split the AbuseFilterSaveTest file into three different classes.

Change-Id: Ia2c0d7ab878b20a89324336a532abdc44f1e6b74
2022-03-20 17:40:49 +00:00
Daimona Eaytoy b5c22f2b77 Improve wording for throttled filter warnings
List which actions were disabled, or explicitly say that no actions were
disabled if that's the case. Also avoid the word "throttle" in messages
as it may be hard to translate. Also don't suggest optimizations to the
filter conditions -- unoptimized rules have nothing to do with a filter
being throttled.

Bug: T200036
Change-Id: Id989fb185453d068b7685241ee49189a2df67b5f
2022-02-22 11:10:19 +00:00
jenkins-bot a332b3ff0f Merge "Remove afl_filter entirely" 2021-09-25 01:39:08 +00:00
Daimona Eaytoy e8471a717c Add method to properly check visibility of AbuseLog entries
This replaces the previous pattern of callers having to use
RevisionLookup if the result was 'implicit'. Also, in some cases where
we were just hiding things if the visibility was !== true, properly
handle the implicit case by using the new method. Make the new method
return string constants rather than bool|string.

The new method also fixes some potential info leaks which happened when
the row was hidden, the user could view suppressed AbuseLog entries, but
the associated revision was also deleted and the user couldn't see it
(this shouldn't be relevant for WMF wikis since AF deletion is
oversight-level).

Also add a bunch of tests for the various cases to ensure we don't
regress again.

Bug: T261532
Change-Id: I929f865acf5d207b739cb3af043f70cb59243ee0
2021-09-25 00:08:33 +00:00
Daimona Eaytoy dae374aec2 Remove afl_filter entirely
As per T220791, the old schema and the flag can be removed in 1.38.

Bug: T220791
Change-Id: Ic6b1c8a22d17a301faf32d2e23778d90c41c39de
2021-09-18 11:06:10 +00:00
Daimona Eaytoy b2dc2c4dd8 Refactor ParserStatus
ParserStatus is now more lightweight, and doesn't know about "result"
and "from cache". Instead, it has an isValid() method which is merely a
shorthand for checking whether getException() is null.

Introduce a child class, RuleCheckerStatus, which knows about result and
cache and can be (un)serialized.

This removes the ambiguity of the $result field, and helps the
transition to a new RuleChecker class.

Change-Id: I0dac7ab4febbfdabe72596631db630411d967ab5
2021-09-17 11:25:54 +00:00
Daimona Eaytoy 7c26c4b8d5 More cleanup for parser-related classes
Change-Id: I6a2bbf519e1d5c6fe2778f69624bd80b9ea1ef86
2021-09-10 12:50:20 +00:00
Daimona Eaytoy a722dfe1a4 Rename ParserFactory -> RuleCheckerFactory
The old parser now has the correct name "Evaluator", so the
ParserFactory name was outdated. Additionally, the plan is to create a
new RuleChecker class, acting as a facade for the different
parsing-related stages (lexer, parser, evaluator, etc.), which is what
most if not all callers should use. The RuleCheckerFactory still returns
a FilterEvaluator for now.
Also, "Parser" is a specific term defining *how* things happen
internally, whereas "RuleChecker" describes *what* callers should expect
from the new class.

Change-Id: I25b47a162d933c1e385175aae715ca38872b1442
2021-09-08 21:59:34 +02:00
Daimona Eaytoy 357ddd498c Clean up / simplify parser-related classes
Remove unnecessary setters, injecting everything in the constructor.
These were leftovers from before the introduction of ParserFactory.
Remove public access to the conds used, include the information inside
the returned ParserStatus instead, and consequently simplify callers.

Change-Id: I0a30e044877c6c858af3ff73f819d5ec7c4cc769
2021-09-08 13:41:52 +02:00
Daimona Eaytoy f8e9ac7e2a Rename AbuseFilterCachingParser -> FilterEvaluator
It's an evaluator, not a parser.

Change-Id: Ib6d33e8423ea72709cf5a33f4397ba33e352ea80
2021-09-08 13:40:47 +02:00
Daimona Eaytoy 6684ea6450 Remove AFPTransitionBase
Also cleanup the mPos hack in the CachingParser.

Change-Id: Ib5693802a3ceb80cb736880ed65e27340abef689
2021-09-06 19:33:48 +00:00