Commit graph

5712 commits

Author SHA1 Message Date
Daimona Eaytoy 9bc885b6b3 Add a ChangeTagger class
The logic about action IDs and the persistent buffer is now encapsulated
inside a single service, which is a step towards getting rid of global
state in the AbuseFilter class, and reducing the responsibilities of the
Runner.

An important change made here is that we now require a LinkTarget rather
than a Title. This removes a dependency on the Title class (a monster
object), makes tests simpler, and denies the need to inject a
TitleFactory. This means living without some bits of context (e.g. we're
no longer using makeTitleSafe to ensure a valid title, and we have to
build a "prefixedtext" manually), but this shouldn't be a problem, given
that the titles are only used to create a cache key: invalid titles are
not a problem, and concatenating namespace + title should always be
sufficient.

Bug: T265370
Change-Id: Iff59cd3d889454a482a89c16691bfefcc5ec0a12
2020-10-21 13:19:30 +02:00
Translation updater bot f53a7d2275 Localisation updates from https://translatewiki.net.
Change-Id: I5edcd144146b0ab26014062888f20a9040c94061
2020-10-21 08:25:57 +02:00
Daimona Eaytoy 215f16a177 Prevent uncaught warnings/exception on Special:AbuseFilter
This patch addresses two issues observed in WMF production:
 - Specifying a search mode without a search pattern would result in a
   call to mb_stripos (in AbuseFilterPager) with an empty delimiter,
   which triggers a PHP warning. Avoid this by checking that the search
   pattern is not the empty string, and unset the search mode if that's
   the case.
 - Trying to use an invalid search mode would result in an unhandled
   LogicException. We have some code in place to check the validity of
   the URL parameter, but the relevant code didn't reset the search mode
   to null, hence AbuseFilterPager would throw before we can show a
   pretty error to the user.

Bug: T265994
Change-Id: Ib19d36d6265981097bbb551783fdac8bdaa98854
2020-10-20 13:59:45 +02:00
Translation updater bot 5f0345477e Localisation updates from https://translatewiki.net.
Change-Id: I6d43e208612685bcc11ea0c79660a54225ec29b0
2020-10-20 08:28:55 +02:00
jenkins-bot 3b59156b4c Merge "Minor updates related to var dumps" 2020-10-19 08:27:05 +00:00
jenkins-bot e002cbb4fa Merge "Exclude implicit groups when degrouping the user" 2020-10-19 07:56:41 +00:00
jenkins-bot 21ccbae8d4 Merge "Avoid direct coupling between SpecialAbuseFilter and AbuseFilterView" 2020-10-19 07:55:57 +00:00
Translation updater bot 62be32fe07 Localisation updates from https://translatewiki.net.
Change-Id: Ib3c077654f1f34477ecda240926eea25f42eba83
2020-10-19 08:33:57 +02:00
Daimona Eaytoy a330d0c454 Exclude implicit groups when degrouping the user
It doesn't make much sense to try to remove implicit groups like 'user'
and '*'. As a matter of fact, these groups are also excluded in
AbuseFilterViewRevert when undoing degroups.

Change-Id: I292499611ccfbd12df28b713d4244530db15c26d
2020-10-18 15:34:04 +02:00
Daimona Eaytoy f589629b12 Avoid direct coupling between SpecialAbuseFilter and AbuseFilterView
While this might seem a small change, it removes the last remaining
coupling between SpecialAbuseFilter and the *View classes, that were
forming a huge tangle.

Change-Id: I5a9d6516e3fa2d3efc4bb2e19b05379dc33cd84d
2020-10-17 00:37:11 +02:00
jenkins-bot 94af753348 Merge "Use new services in AbuseFilterRunner" 2020-10-16 13:20:08 +00:00
jenkins-bot c094da9cec Merge "Simplify code for tagging the action on cache hit if the cond limit was hit" 2020-10-16 11:49:20 +00:00
Translation updater bot b355892f63 Localisation updates from https://translatewiki.net.
Change-Id: I834f8082ade6bca86619e359c6d9af1efe5a987c
2020-10-16 08:28:36 +02:00
jenkins-bot bad6a351c9 Merge "Improve display of log entries when global filters are not enabled" 2020-10-15 14:11:37 +00:00
Matěj Suchánek adbe9bcbce Improve display of log entries when global filters are not enabled
Don't create <a> tags without a href. Show a placeholder
message instead of nothing (alternatively, we could create
a new message for each existing one).

Bug: T174000
Change-Id: Id55b90881aacc620ff3c519ad6eedf212f36c4ed
2020-10-15 15:05:16 +02:00
Translation updater bot 433944e3cc Localisation updates from https://translatewiki.net.
Change-Id: I5d16d4a84067d49fde8e36f5a64d62308b617ea9
2020-10-15 08:37:15 +02:00
Daimona Eaytoy 1efc324d97 Use new services in AbuseFilterRunner
The first one is UserGroupManager, used for the 'degroup' action. This
is a simple one-line replacement (repeated twice), and the current code
was already using this service under the hood.

The second one is BlockUser, which is not a one-line change (but still
quite simple). In particular, this allows us to avoid duplication with
core logic when constructing the log entry (this is now done by
BlockUser).

Bug: T248743
Change-Id: Ib7c1dc107a169b575f7021e64b6a8fee09529548
2020-10-14 23:08:32 +00:00
DannyS712 2b5f80ec01 Update reference to core hooks documentation
now at Hooks.md

Change-Id: I9dc876b2e31a3a85186f2fe9735881de0a8663d8
2020-10-14 22:09:15 +00:00
jenkins-bot 170bd831e7 Merge "ViewEdit: avoid linebreaks in form labels" 2020-10-14 15:29:02 +00:00
jenkins-bot 70febfe8ee Merge "Clean up view classes" 2020-10-14 09:43:55 +00:00
Translation updater bot ac3aecbdd1 Localisation updates from https://translatewiki.net.
Change-Id: Ibf3a5062bb79b06bdd6994acf9cfc346f9460b28
2020-10-14 08:27:03 +02:00
Daimona Eaytoy a7182acafd Simplify code for tagging the action on cache hit if the cond limit was hit
This code was simply caching the AbuseFilter::$tagsToSet property, but
this is not necessary. The only tag that can be buffered during edit
stashing is the conds limit tag. So we just save whether the conds limit
was hit, and apply the tag from a single point afterwards.

Also avoid checking whether 'tag' is enabled as an action, since this tag
should always be added when applicable.

Next step is creating some sort of Watcher service that will do
everything on its own: check whether the limit was hit, save this
information, and tag the action later.

Bug: T265370
Change-Id: I90319a658736fad7d564cb51152061709c230411
2020-10-13 16:05:18 +00:00
Daimona Eaytoy 45d80bc7e5 Clean up view classes
- Depend on a generic IContextSource rather than SpecialAbuseFilter
  (lower coupling);
- Inject a LinkRenderer (IContextSource doesn't have a ::getLinkRenderer
  method)
- Add a helper method in SpecialAbuseFilter to get the page title, that
  can also be used elsewhere (and the name constant can be made private
  now)
- Pull down the mFilter property (and rename it to just 'filter') to
  classes that actually need it. Some classes didn't need this at all
  and the types were different among subclasses

Now the only cause of coupling between the View classes and
SpecialAbuseFilter is the static call in getTitle.

Change-Id: I3df0c3a7621f0cc9a64a16b0a402a15aae2d5d73
2020-10-13 10:38:43 +02:00
jenkins-bot 95766762c4 Merge "Migrate a few hook handlers to DI" 2020-10-13 08:36:27 +00:00
jenkins-bot 3e61e886ba Merge "Add an AbuseFilterPermissionManager service" 2020-10-13 08:36:25 +00:00
Translation updater bot 026a369401 Localisation updates from https://translatewiki.net.
Change-Id: Ie8c02de97d619b407d1b92b3d238fe0899e732a2
2020-10-13 08:40:40 +02:00
jenkins-bot 51ce0bacf6 Merge "Delegate some switch cases to the parent in GlobalAFPager" 2020-10-12 10:13:25 +00:00
Translation updater bot 297a1bf180 Localisation updates from https://translatewiki.net.
Change-Id: I6ff4a99ada61d51d34c8342172da1899660fcc7a
2020-10-12 08:33:29 +02:00
Matěj Suchánek 7ef2259228 Migrate a few hook handlers to DI
Bug: T261067
Change-Id: If699917c3d2e9e22525c7d0495554e25f6b45125
2020-10-10 17:23:04 +00:00
jenkins-bot 42525e4d5a Merge "Cleanup filter id handling on Special:AbuseFilter/history" 2020-10-10 12:28:05 +00:00
Daimona Eaytoy 2026e3ac3a Add an AbuseFilterPermissionManager service
This service should act as a mediator between the AF code and the
permission manager, and it should know what are the permissions required
by each action.

Change-Id: Ieb177d9992147b11fa7b8f05929da6c182cc2286
2020-10-10 14:03:29 +02:00
jenkins-bot f1de9145f5 Merge "Remove sorting by user from Special:AbuseFilter/history" 2020-10-10 11:58:54 +00:00
Matěj Suchánek d91ddd2169 Cleanup filter id handling on Special:AbuseFilter/history
In particular, the interface shouldn't generate links to
"Special:AbuseFilter/history/0" (AbuseFilterHistoryPager::getTitle,
can be seen when visiting "Special:AbuseFilter/history").

Change-Id: Id3dc1bb4fc3c5e853603bf0ec04a6b1751f7d862
2020-10-10 11:40:46 +00:00
Daimona Eaytoy f0539e0c1e Represent new filters with null instead of 'new'
PHP is not strongly typed, so it's not a good idea to use scalars of
different types (here it's an integer vs the string 'new') to represent
different possibilities. This can have bad effects when type juggling
occurs, and it's also harder to figure out what the type of the
parameter can be (because a numeric ID might have been passed as a
string). Using integer vs null avoids all of this, and also allows us to
use nullable typehints.

These changes were partly copied from
If981cb35bf19a8469aa6c43c907e107cf8c65bc2 and should help with the
migration to the Filter value objects.

Change-Id: I8837d46c3c33761fea53f67b530b721dc7bd49b0
2020-10-10 12:23:50 +02:00
jenkins-bot c0defc1055 Merge "Add a new FilterProfiler service" 2020-10-10 10:08:58 +00:00
jenkins-bot f4570e232d Merge "Hide filter group selector when filter selector is hidden" 2020-10-09 23:02:04 +00:00
jenkins-bot da3bfa3314 Merge "Reduce dependencies of AbuseFilter::saveFilter" 2020-10-09 14:58:30 +00:00
Matěj Suchánek 826f03d928 Remove sorting by user from Special:AbuseFilter/history
This feature didn't work and even if we fixed it as suggested
in the task, it would still be bogus. For deterministic paging,
the afh_user_text field should be in an index together with
another field(s). But currently it's indexed alone.

By the way, the indexes on abuse_filter_history should be fixed
anyway. Special:AbuseFilter/history also allows filtering by
filter/user which require index on the fields. They are present
but are not composite, so either the sorting is done
inefficiently without an index or there is a fullscan.

Also remove the getIndexField override. TablePager knows best
what value can be used there, we don't really have to override
it.

Bug: T204210
Change-Id: I7335f82c917a1d219fd7f0999da5b62433f14bd8
2020-10-09 15:41:19 +02:00
Matěj Suchánek 2c650f7710 Hide filter group selector when filter selector is hidden
This was a means to bypass the limitation to filter by
triggered filter (for example, when a group contains
a single filter).

Change-Id: Icd7b0b64ff16b4ce26f4d52ad9d9abce62972e60
2020-10-09 13:33:13 +00:00
Daimona Eaytoy 9f2906e34b Reduce dependencies of AbuseFilter::saveFilter
This patch removes the dependency of saveFilter on the ContextSource
kitchen sink. It also removes some unneded dependency, and adds
$originalRow/$originalActions as parameter, rather than hacky properties
in $newRow that are easy to forget. The related test can also be greatly
simplified.

This also introduces a behaviour change: checking $newRow instead of the Request allows us
to account for values normalization done in
AbuseFilterViewEdit::loadRequest, and to also work correctly for imports
(and generally speaking, it makes the method suitable for an
AbuseFilterEdit API module, too).

Next step is moving this method to a service. Some signatures,
indenting, name choices etc. are subpar, but this is just because these
methods are temporary anyway.

Bug: T213037
Change-Id: I235b928d7b9c2ef1c46ea0bf3e3ed212500b4161
2020-10-09 11:52:02 +00:00
jenkins-bot 73d6544bc6 Merge "Avoid array_filter on explode()" 2020-10-09 07:49:24 +00:00
Translation updater bot 42dcd268c3 Localisation updates from https://translatewiki.net.
Change-Id: I7d576c9f503d83808c5b6971f3a31cf3296b63ac
2020-10-09 08:24:34 +02:00
jenkins-bot f2648afb15 Merge "Prevent cache pollution in fetchAllTags and clean up" 2020-10-08 18:02:21 +00:00
Daimona Eaytoy 7a1d6dbdbb Avoid array_filter on explode()
The array_filter is likely meant to empty the array if the empty string
was exploded ( `explode( "\n", '' ) === [ '' ]` ). However, it can also
remove other stuff, e.g. the string '0'. An explicit comparison is
easier to read & interpret, marginally faster, and avoids rare but not
impossible edge cases.

Change-Id: Ie77d65b56319664a2ac370f32341dc72b619a635
2020-10-08 18:56:27 +02:00
Matěj Suchánek 9e6bc2f4ee Move log formatters to a separate directory and namespace
This will clean up the includes/ directory a little.

Change-Id: I61adacf32257bb2402a272b60b52b69505d981c5
2020-10-07 16:25:38 +02:00
Matěj Suchánek b7cda4de4c Prevent cache pollution in fetchAllTags and clean up
Previously, the cached value would depend on the tags
parameter to be updated. The provided value may be
different for each call, so callers may receive
unexpected values.

For example, while core usually calls this with core-defined
hooks, our method AbuseFilter::isAllowedTag calls this
providing an empty array. If core's call happened shortly
after ours and hit cache, its array would be overwritten
with only AbuseFilter's tags, the rest would be lost.

Also do some clean up:

- only call array_filter on explode'd array
- call array_unique on the value, since it's usual that
  multiple filters share the same tag

Noticed when thinking about moving this to a service.

Change-Id: I4f4322e80ec89e48458a3bf46a1146863bec8237
2020-10-07 15:20:41 +02:00
Daimona Eaytoy 58538103c9 Delegate some switch cases to the parent in GlobalAFPager
af_actions and af_hidden are treated in the same way, so avoid
duplicating that code. Some of the remaining cases are also quite
similar (although not identical), so we might want to merge them in the
future.

Change-Id: I1b48502e077e58eb9ff459326bba18bb1d127242
2020-10-07 12:46:52 +02:00
Translation updater bot c3fd58c7e8 Localisation updates from https://translatewiki.net.
Change-Id: I862e64df2d7e0eb2c8836540debdcd032153b40d
2020-10-07 08:34:11 +02:00
jenkins-bot c6ab7b8e1f Merge "Use triple equals in abuse filter parser tests" 2020-10-06 17:42:29 +00:00
Huji Lee d07717dd0d Use triple equals in abuse filter parser tests
The only exception is mwexamples-comparisons.t which intentionally
includes examples with = and == to test the "weak" version of the
comparison operator.

Bug: T262063
Change-Id: I6f92aadc69489da481a606bfda89617b8efbb261
2020-10-06 12:29:47 -04:00