Commit graph

559 commits

Author SHA1 Message Date
jenkins-bot 78c7334d6a Merge "Split VariableGenerator::addEditVars" 2023-07-17 23:26:53 +00: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
Matěj Suchánek 49edc86a78 Split VariableGenerator::addEditVars
This method actually consists of two: add derived vars, and initialize
content vars. The former part depends on no parameters of this method.
On the other hand, the latter part combines multiple implementations
for some of the content variables using branching.

The branching is a dirty workaround and inferior to the GRASP principle:
"When related alternatives or behaviors vary by type, assign
responsibility for the behavior to the types for which the behavior
varies."
In other words, the callers (extensions) should be responsible for
choosing the initialization strategy themselves, instead of letting
VariableGenerator figure it out.

As the first step, split the former part to a separate method.
For now, it will be implicitly called by ::addEditVars.

Change-Id: I5ff00dbdbf29ec54eabfd95c44a4fd7f713969f5
2023-07-05 14:58:32 +00:00
Abijeet b1e404fc79 ConsequencesFactory: Avoid creating Session object during service wiring
Service wiring should only depend on config, not on request state.

Creating a session object during service wiring causes issues with entry
points such as opensearch_desc.php that disable the session.

Bug: T340113
Change-Id: I2450b0b6821ff0b097e283ff660a0b8aeea9590a
2023-06-27 20:11:38 +05:30
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
jenkins-bot a2524dd966 Merge "Make some non-static providers static" 2023-06-16 19:57:17 +00:00
Matěj Suchánek 1bcc28891f Make some non-static providers static
Or at least one step forward to better isolation.
Some providers were replaced with individual test cases.

Bug: T337144
Change-Id: I5cd8c5e79993260f18c3a17c40b8501a4da3c17f
2023-06-16 20:54:33 +02: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
Matěj Suchánek d82a716ad0 Make old_links retrieval cleaner
The method for old_links retrieval depends on the "forFilter"
value, which we know in advance. If it's true, old_links should
be retrieved from the database. Make a case in the switch
that does nothing but retrieves links from the database,
and direct the evaluation to it.

This change was split from I3628a56e5 to make its review easier.

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

Change-Id: I33b688f6be3c58beec403f7bf26407a42e7c18ab
2023-06-13 14:03:21 +02:00
jenkins-bot 54b9cbd6da Merge "BlockedDomains: Use cleaner array building and add tests" 2023-06-12 18:06:38 +00: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
thiemowmde 84058c3d96 Make use of the ??= operator and such where it makes sense
We can avoid a bit of code duplication and move code closer together
when it belongs together.

Change-Id: Iffca7e4abfbf03d4663ee909220057bcbd54da75
2023-06-12 10:27:03 +02: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
jenkins-bot d6d8608161 Merge "Replace deprecated MWException" 2023-06-07 23:25:54 +00:00
Daimona Eaytoy caee78c24d Replace deprecated MWException
These are all unchecked.

Bug: T328220
Change-Id: I8d2f098a8b634d4a226b40ddaef31f0303a0789f
2023-06-07 17:41:20 +02:00
Thalia 573838efc5 Degroup: Return early if user is a temporary user
Treat temporary users the same as IP users. Neither has user groups,
so return early for both.

Bug: T335062
Change-Id: I20b48608cf6ba5f8e8e36a378d66c603d84b032f
2023-06-06 14:10:21 +01:00
jenkins-bot 64ed21cff7 Merge "Use new DeferredUpdatesManager service" 2023-06-01 19:00:42 +00: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
Daimona Eaytoy 1c0e558c78 Use new DeferredUpdatesManager service
And remove some hacks for unit tests.

Change-Id: I4e9932a003ac7420f307f01b8d12062fd05a3bb8
2023-05-30 12:50:08 +00:00
jenkins-bot 7f4af85ce0 Merge "Add tests for temporary user in CheckUserHandlerTest" 2023-05-27 16:31:04 +00:00
jenkins-bot 17cb8ac514 Merge "Update user type checks to handle temporary users" 2023-05-26 11:56:35 +00:00
Thalia 56c86a761f Add tests for temporary user in CheckUserHandlerTest
Follow-up to d42b7335d5

Change-Id: Ia70377bf03a48e006174d6f658cf11142f1dd624
2023-05-26 14:34:04 +03:00
AnaïsGueyte d42b7335d5 Update user type checks to handle temporary users
* Set the same block expiry for temp and anon users
* Don't block autopromote for temp users; they can't be autopromoted
* Bail early from CheckUserHandler if the user is temporary

Bug: T335062
Change-Id: I6b72537f568c4c70a0b86f1825ea30b767f5634a
2023-05-25 17:26:58 -02:30
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
Umherirrender 0c513ba63e tests: Use static provider in AbuseFilterHookRunnerTest
Shows up a deprecation message

Follow-Up: I5ff35ad0e894f0a27beae00257dc1fc599ad518d
Change-Id: I2e74ca48374f1ab5901ae3adf891ea29cd322251
2023-05-19 22:19:48 +02:00
gerritbot 142e4be136 Update moved class FauxRequest
See T321882. Moved in I832b133aaf61ee

Bug: T321681
Change-Id: I0ab062426628bb33d2bbfc686e4befc66ee43f38
2023-05-19 10:23:42 +00: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
jenkins-bot d3b5dbb092 Merge "Add tests for extension.json and services" 2023-04-20 00:48:30 +00:00
Jean-Luc Hassec 6c500f8ea9 Clean up unused DEMPTY data type
Bug: T334640
Change-Id: Ie20d760b6e31a9dc97083d3fe4008fb31c990076
2023-04-13 05:27:38 +00:00
Matěj Suchánek 0628dbdab6 Add tests for extension.json and services
Change-Id: Ie83e4a85a408e1ba1d2cc827c4bf353bdd5500df
2023-03-28 09:35:02 +02:00
jenkins-bot 290dd70bb2 Merge "Replace deprecated database object access methods" 2023-03-27 09:11:46 +00: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 8f6a428f02 Replace deprecated database object access methods
Use the very new getPrimaryDatabase and getReplicaDatabase.
We skip FilterLookup and CentralDBManager in this patch.

Change-Id: I22c6f8fa60be90599ee177a4ac4a97e1547f79be
2023-03-08 16:50:56 +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
James D. Forrester ebc2e653b0 build: Upgrade mediawiki/mediawiki-codesniffer from 40.0.1 to 41.0.0
Change-Id: I4489dd3935e28a8cdab0d3612ff869854e995c8b
2023-02-26 00:32:51 +00:00
Dreamy Jazz 8e4a1237f1 Hook on privateEvent and logEvent insert hooks like CuChangesInsert
Hook on to CheckUserInsertPrivateEventRow and CheckUserInsertLogEventRow
to override the IP, XFF and User-Agent string when the user is the
abuse filter user for log events.

These two hooks are being added as log entries are being removed from
cu_changes and added into two new tables. Because the columns and their
names are different for these tables, reusing the same hook won't work
for callers that rely on setting values for a specific column name.

Edits and log entries performed by the abuse filter user need to be
marked as being by the software (and not using the IP, XFF and
User-Agent provided in the main request).

These hooks will not be run until the appropriate config is set to
write to the two new tables. Until that point using the one currently
defined hook will work for all actions.

Bug: T324907
Bug: T44345
Depends-On: I7c7754323ade9a8d96273c1742f30b1b5fbe5828
Follow-Up: Idd77545af94f9f9930d9ff38ab6423a72e680df9
Change-Id: Id78417e9d95220946f110afbe1430df5b3bb4f4f
2023-01-08 13:09:52 +00:00
jenkins-bot 595b0a9969 Merge "Ensure IP, XFF and UA are valid for abuse filter user actions in CheckUser" 2023-01-06 20:51:17 +00:00
jenkins-bot 24d797e6cc Merge "Create real integration test for variables" 2022-12-22 02:07:02 +00:00
Matěj Suchánek 3e0d1b0d38 Set old_content_model & new_content_model for past changes
We might consider adding an in-process cache because there
will be a duplicate database lookup for content model and
wikitext of the same revision.

Bug: T230295
Change-Id: I9723f21069e03a49fa7131bd8f79c6e7e442104b
2022-12-18 16:01:45 +00:00
Matěj Suchánek 396d892c60 Use ActionSpecifier to load the IP address
To avoid access to the global request context.

Change-Id: I4d97dbe8b693f1fcd5a4e84f2376752d8e954c18
2022-12-17 22:52:24 +01:00
Matěj Suchánek 52dcd4624f Use ActionSpecifier throughout the code
The motivation is to have a single immutable object providing
information about the action. It can represent the current
action being filtered, but also a past action stored in the
abuse log. It will hopefully help us get rid of passing
User(Identity) and Title/LinkTarget objects around together.

Change-Id: I52fa3a7ea14c98d33607d4260acfed3d3ba60f65
2022-12-16 22:52:03 +00: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