Commit graph

236 commits

Author SHA1 Message Date
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
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
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
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
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
Daimona Eaytoy caee78c24d Replace deprecated MWException
These are all unchecked.

Bug: T328220
Change-Id: I8d2f098a8b634d4a226b40ddaef31f0303a0789f
2023-06-07 17:41:20 +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
Jean-Luc Hassec 6c500f8ea9 Clean up unused DEMPTY data type
Bug: T334640
Change-Id: Ie20d760b6e31a9dc97083d3fe4008fb31c990076
2023-04-13 05:27:38 +00: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
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 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
dreamyjazz 627a73ec5e Ensure IP, XFF and UA are valid for abuse filter user actions in CheckUser
Change the IP to 127.0.0.1 (to indicate an internal IP), and blank
the XFF and UA when the performer of an action being logged by
CheckUser is the abuse filter user. Actions performed by the abuse
filter user can only be initated by the software, and as such should
not use the request's IP, XFF and UA. Also test the newly added
code.

Bug: T44345
Depends-On: I28acaaebd2d0067b700da0930e7b7ba924fa5c1c
Change-Id: Idd77545af94f9f9930d9ff38ab6423a72e680df9
2022-11-11 23:19:22 +00:00
Umherirrender 4fca77068c Clean up line indent with mixed tabs and whitespaces
Change-Id: Icc418130ad34e5f169bfc51bb13b58a7806bd636
2022-07-31 16:34:07 +02:00
Umherirrender da4bc8643a Use UserIdentity in VariableGenerator::addEditVars
Change-Id: If0a65d7a86de776e6499d43949bfb217f20d9b07
2022-07-29 12:55:52 +02:00
jenkins-bot c3c70f7fa0 Merge "FilterProfiler: use WRStats" 2022-07-06 00:05:15 +00:00
Tim Starling cdf2f474e8 FilterProfiler: use WRStats
A new core facility written for this use case.

Bug: T310662
Depends-On: I26b1cdba0a06ad16ad8bb71b455e1b6180924d17
Change-Id: I2b902d034a8c3308c0ba9878b69e873ca8fbda52
2022-07-06 09:35:08 +10:00
Matěj Suchánek 799e1db093 Convert remaining permissions checks to use Authority
Change-Id: I5e996cac37bc806db6c3d7ad5c666a606cd79236
2022-07-02 14:49:47 +02: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
jenkins-bot 8d4c5d4d33 Merge "Use LinkTarget in ConsequencesExecutor" 2022-06-29 08:52:37 +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
Umherirrender 20fd8f7b07 Use LinkTarget in ConsequencesExecutor
The Parameters class already only needs a LinkTarget

Change-Id: I4e8e1d7c92f41502a084be3359b97e0d434f08c0
2022-06-28 19:46:50 +02:00
Umherirrender 30fefb75bf Use UserIdentity in ConsequencesExecutor
Change-Id: I281a30610595ed3e984f43aa747eff37abe72939
2022-06-27 22:05:18 +02:00
Daimona Eaytoy f33bc5868c Set the 'timestamp' var in addGenericVars
This was most definitely my intention when I introduced the concept of
"generic vars", so it's a bit surprising to discover, 3.5 years later,
that the timestamp isn't computed there.

Also make the timestamp always be a string for consistency, since that's
the type documented on mw.org. I've manually checked all filters on
Wikimedia wikis using the timestamp variable, and added explicit int
casts where needed (although I think they'd still work due to implicit
casts).

Change-Id: Ib6e15225dd95c2eead7e48c200d203d6918e0c18
2022-06-26 14:49:40 +02:00
Umherirrender 3d3c45f348 tests: Mock WikiPage in unit test
Bug: T297688
Change-Id: Ic1655141564f02530b1ae6b625a1d3e261a00304
2022-06-24 22:22:24 +02:00
Matěj Suchánek 40564ca635 Remove $info argument from ReversibleConsequence::revert
It was a temporary catch-all variable, but we can replace it
(and probably won't need it).

Change-Id: Ie1a64455c47445050bd83c853b3cafd283d5d020
2022-06-08 11:59:18 +02:00
jenkins-bot 1a6985469b Merge "Inline/simplify smaller pieces of duplicate/complex PHP code" 2022-06-03 20:38:22 +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
jenkins-bot bb94c0914c Merge "Add support for regex string replacements." 2022-05-31 14:54:33 +00:00
Daimona Eaytoy a46db47bd5 Fix validation for ip_in_ranges
We want to make sure that all parameters are valid regardless of whether
there's a match.

Also make the minimum number of parameters = 2, so it's easier to switch
between this function and ip_in_range.

Change-Id: I141558a7ef4533485e315b3d93ea9b64f0959db7
2022-05-21 15:39:21 +02:00
fossifer b1739a588f Add ip_in_ranges function
Added support for ip_in_ranges which allow multiple ranges to be
checked at the same time. If the IP is in any of the ranges, the
function returns true.

Bug: T305017
Change-Id: Ic75c87ecd4cacf47ce2ff1b04173405230ff81d0
2022-05-11 12:27:16 +08:00
proc 1d1215bafb
Add support for regex string replacements.
Bug: T285468
Change-Id: I25f8ad1b58cc10f4c6f6ef5ebab99fe58ec71b1e
2022-04-20 18:38:24 +01:00
Matěj Suchánek 686d7ea88c Use RestrictionStore instead of deprecated method
Also restructure the unit test a bit.

Change-Id: If5ce26f1bc4efdb29653aed3fc47335dddc1e44c
2022-03-29 16:11:55 +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 2de5fce177 Refactor ConsequencesExecutor to process consequences in more steps
Introduce shorter methods, one for each steps, so that it's easier to
understand what the code is doing and figure out if the order makes
sense. The ConsequencesExecutor test is now a proper unit test. Also
simplify AbuseFilterConsequencesTest, removing old/wrong logic and
fixing two expected values that were actually wrong (but worked because
of the aforementioned wrong logic).

The only functional changes should be:
 - We pick the longest block *after* checking the ConsequenceDisabler
   consequences, so e.g. if a filter has a long block + warn and another
   filter has a shorter block, we still keep the second one if warn will
   disable the block.
 - Remove disallow in presence of dangerous actions after checking
   ConsequenceDisabler's and deduplicating blocks. Otherwise we may
   remove disallow for filters where block (etc.) doesn't end up being
   disabled. We may also want to consider not removing disallow at all,
   now that messages are customizable.

Bug: T303059
Change-Id: If00adbf2056758222eaaea70b16d3b4f89502c20
2022-03-19 15:49:36 +00:00
Alexander Vorwerk 4aedfe8d91 Use updated ObjectFactory namespace
Change-Id: I99c5e5664d2401c36a9890f148eba7c25e6e8324
2022-03-09 22:17:07 +00:00
jenkins-bot dad1fff238 Merge "Overhaul throttle identifiers" 2022-03-06 13:50:43 +00:00
Daimona Eaytoy a0fd0bae01 Overhaul throttle identifiers
- Use a /64 range for IPv6 instead of /16.
- Fix a curious and serious bug for IPv6, where grouping by range
  would only use the first (!) number of the IP address, due to the
  'v6-' prefix returned by IP::toHex.
- Fail hard if the identifier is unknown -- it's not something that's
  supposed to happen.
- Include the type name in each identifier, instead of prefixing all
  type names to all identifiers. This makes it easier to understand the
  parts of the key.
- Test the whole lot.

Bug: T211101
Change-Id: I54c4209f2f0d5a4c5e7b81bed240ca3e28a2ded7
2022-03-06 13:31:06 +00:00