Commit graph

125 commits

Author SHA1 Message Date
Kunal Mehta cb1458f91e Fix parameter order in doc comment
Spotted by phan 1.2.4.

Change-Id: I63aff03d48f1749e03d3398016ead01bc37fe73d
2019-02-23 21:29:39 -08:00
Thalia 540a557a59 Replace calls to deprecated Block::prevents
Where prevents is used as a setter, use the new setter methods;
where it is used to determine whether a block blocks the target
from editing their talk page, use appliesToUsertalk.

Block::prevents was deprecated and replaced by several other
methods in I0e131696419211.

Bug: T211578
Change-Id: I166cc6f64c0f895ff8c631d2655c1c3208131371
2019-02-22 19:29:02 +00:00
Daimona Eaytoy 85ba973747 Move the "global-" prefix to a const
And add an utility function to use it to build full names.

Change-Id: Ib5fdeb75c1324f672b4ded39681f006fde34b4d1
2019-02-06 14:42:05 +01:00
jenkins-bot 15a8340ee1 Merge "Reject empty warning and disallow messages when validating a filter" 2019-01-31 21:28:17 +00:00
Daimona Eaytoy ba1b27d7f6 Optionally pass the filter ID to checkConditions for error reporting
Now that Parser errors are on logstash, I noticed a huge spike of errors
on Wikimedia Commons, about 35000 per hour. They seem to be due to 2
broken filters, but id doesn't say which ones.

Change-Id: I8510319c075520f9a893cd7d56f2e30679e249ba
2019-01-24 10:03:52 +01:00
Daimona Eaytoy a207cf22f7 Unbreak tagging for createaccount actions
Tagging doesn't work for account creations, and probably never did. This
is because we used a wrong identifier for such actions. This patch fixes
the problem, although in the long term we should find a smarter way to
apply tags.
Also, clean AbuseFilter::$tagsToSet if the action will be prevented.

Depends-On: Ia8e38ba25d1989fe71714d2b76891c4587921466
Change-Id: I8edcca17ecdcf71397cc9b0d101e8b13ac112047
2019-01-23 21:25:47 +00:00
Daimona Eaytoy f3f8bd11b9 Re-execute checkAllFilters if the edit was stashed
This may solve several issues, see T176291#4105438 for further details.

Bug: T191430
Bug: T176291
Depends-On: Iebbdeac7898b35beea79aa3d0cdf9d0fb265d726
Change-Id: Ia8e38ba25d1989fe71714d2b76891c4587921466
2019-01-23 18:16:01 +00:00
Daimona Eaytoy bc875d8002 Fix SQL key
When updating the abuse_filter_history table, the sequence to use is the
one on afh_id... And we were using the af_id one since 2009.

Change-Id: I3e291c780119d74be5f47e745a8de13bda85486b
2019-01-23 16:24:02 +01:00
Daimona Eaytoy 0e6b783ed4 Reject empty warning and disallow messages when validating a filter
Right now, we allow empty messages, and when the "warn" action is
executed we use "abusefilter-warning" if no message is specified.
However, this also produces a PHP notice while editing a filter with
empty message (see Phab). With this patch, empty messages will be
rejected, and a follow-up will be discussed on Phab.

Update: added disallow message as follow-up of
Ic1de03a6944c43a346fa317ee0a217551f0d284a.

Bug: T203353
Depends-On: I8df247f61d9f3769e9580544f324dd174811e939
Change-Id: I71b1f81d10c02de4de141b1ab9b630d05cf4619c
2019-01-21 14:06:54 +01:00
jenkins-bot 41f6a85a42 Merge "Rewrite the method for getting a global emergency value" 2019-01-19 13:25:41 +00:00
jenkins-bot f8b5965ff9 Merge "Expand AbuseFilter::getFilter to select all fields and fix caching" 2019-01-19 13:17:16 +00:00
jenkins-bot a7955a5142 Merge "Move a method out of AbuseFilter.php" 2019-01-19 12:22:39 +00:00
jenkins-bot b44984c50a Merge "Remove unused stuff" 2019-01-19 12:18:22 +00:00
jenkins-bot 91e1833bc0 Merge "Fix topnav links" 2019-01-19 12:11:07 +00:00
jenkins-bot 575646393b Merge "Improve code readability" 2019-01-19 12:11:06 +00:00
jenkins-bot 7f62874658 Merge "Change method visibility for AbuseFilter class" 2019-01-19 12:02:51 +00:00
Daimona Eaytoy 6217ffb928 Remove unused stuff
Variables declared but never used, redundant code, and old leftovers.

Change-Id: Ic51044a45a1b49ad6c7af06c646b11893411a7cd
2019-01-18 17:04:19 +01:00
Daimona Eaytoy 34d3f9acb2 Fix topnav links
*Don't reuse a message (which is bad), instead add a note for
translators. We can also move it on translatewiki.
*Don't show the AbuseLog link if the user cannot see the AbuseLog.

Change-Id: I4ce73b2160275fdc4b0b7bec722471696d8c6a4d
2019-01-17 15:09:29 +01:00
Daimona Eaytoy 93e8cb5ac5 Tune logging channel
As follow-up of I10b1fd2d9bdfe518089c053d77fef568170ecb65, use
'AbuseFilter' instead of 'AbuseFilterDeprecatedVars' as channel name.
Raise level for null-title filtering. Since with a null title
several things are likely to break, a warning is more appropriate here.
Tweaked the message as well, to include the bug number and to avoid
pointlessly including the title (which is null).
Lower the level for stashedit hit/miss (as it's really spammy and not
that useful right now).
Use 'abusefilter' instead of 'AbuseFilter' for statsd so that everything
has the same prefix.
Also raise the level for parser exceptions and unrecognized
consequences.

Change-Id: I1f9988155e924232b201281795cd322636da8082
2019-01-16 08:56:22 +00:00
jenkins-bot e6ca0f288d Merge "Really disable the minor_edit variable" 2018-12-31 02:21:56 +00:00
jenkins-bot 2539f6883e Merge "Remove workaround to complete phase 1 of variables migration" 2018-12-30 23:19:20 +00:00
jenkins-bot 90796123a8 Merge "Add a new method and hook for static variables" 2018-12-30 22:50:35 +00:00
Daimona Eaytoy 217b4b57ff Remove workaround to complete phase 1 of variables migration
When all the other patches will be merged, this workaround won't be
necessary, and by removing it we're finishing the first phase of
variables migration. Which could also be the only one if we decide not
to go on and remove the old ones.

Bug: T173889
Depends-On: I5c370b54e6516889624088e27928ad3a1f48a821
Depends-On: I6576497feaf6d2c475ee33a91feb6a640e2c20fe
Depends-On: I87a48fdc8b392b25eb02807e8d0f712d0a399ece
Depends-On: Ib29eb15c1a51c037d036be8dc1541d96ea4b174b
Depends-On: I909a99e80a895a9b009c33ea72e8e0a4ea0a1375
Change-Id: If5f238cddb41ef92b141e36b4f2f15fd4cc86476
2018-12-30 22:43:14 +00:00
Daimona Eaytoy b0c5b97b28 Add a new method and hook for static variables
This is for adding variables which can be computed even without an
ongoing action. Currently, we don't have any, except for timestamp (but
that's a bit special). Other extensions could. For instance, we'll be
able to expose the content of the spam blacklist.

Bug: T211680
Change-Id: Iba59fe8d190dd338ecc8cfd682205bce33c9738b
2018-12-30 18:15:33 +01:00
Daimona Eaytoy 2fc56ce014 Use array_unique on the array of tags to add
Otherwise calling bufferTagsToSetByAction multiple times makes the list
grow, and IIRC the core doesn't call array_unique on the tags to apply.
Also clean the list after applying tags.

Change-Id: Iebbdeac7898b35beea79aa3d0cdf9d0fb265d726
2018-12-29 15:19:02 +01:00
Daimona Eaytoy 921db0397e Really disable the minor_edit variable
The variable was disabled with I7f13773766e12f3d4b86451fdf3ae23e067ac373
in 2016, but not in the same way as old_text and old_html were disabled
in 2009. This patch uses the methods introduced with
Ife168522e6b1d8eb94ebbb8a16ae8831ec1dc497 to disable minor_edit in a
standard way, so that it won't be showed in new AbuseLog entries, and
won't be usable when writing filter syntax.
A warning will be emitted if a pre-existing filter is using it, so that
we'll be able to completely disable it in the future.

Change-Id: I5ad5219ee19a5e6ba2bfdffb4e0aad63c8951491
2018-12-29 14:14:27 +01:00
Daimona Eaytoy 4950bf6664 Validate the abusefilter-blocker name
In T209565#4826952 I discovered that if the "abusefilter-blocker"
message is an invalid username, we silently end up without a system
user, thus risking to break something. Instead of silently failing, emit
a warning and use the default name. As I wrote in the code comment, we'd
better avoid throwing, because the message can be modified by anyone,
who could then break the site.

Change-Id: Ifa866bd9676945bf94e7e481adf6ad0d6cf4370c
2018-12-17 16:02:24 +01:00
Daimona Eaytoy 3fa6e2d31c Expand AbuseFilter::getFilter to select all fields and fix caching
This partly reverts If72b18bedac5e580487406e696aea1fd172ae45b. While
it's true that we don't need every filter, that method is public and
other code may need fields that we don't need. This way we can encourage the
use of this function (which caches the result) instead of direct DB
access.
Also, the method can currently accept global filters passed as
"global-<integer>", but saves them to cache with the same key as local
filters (i.e. local filter 15 and external global filter "global-15" are
both saved in AbuseFilter::$filterCache[15], which could lead to subtle
bug).

Change-Id: Ieb04f019453033c275e211cfc9fd68d5d7c392ef
2018-12-16 14:23:45 +01:00
Daimona Eaytoy aa280998c0 Fix big problems with normalizeThrottleParameters
My final testing unveiled 4 problems, see T209565#4780868. Testing again
after this patch yields the expected outcome.

Update: A fifth problem is that we cannot disable throttling if throttle
groups are empty or fully invalid: that case is similar to the one with
invalid rate, the throttle limit is never reached and thus throttle just
doesn't work. Instead, ask users to fix it by hand.

Bug: T203336
Bug: T209565
Change-Id: Id03c9880f60764efc596ac40b8662087fdb30550
2018-12-15 18:36:16 +01:00
Daimona Eaytoy db31c6dfea Rewrite the method for getting a global emergency value
Currently it barely has any reason to exist, as it's a single-line
method. This patch moves there the global state, and also changes the
signature to allow shorter calls.

Change-Id: I7851fa41cbd96912b3859319ba97a501b1cbaa57
2018-12-10 18:28:32 +01:00
Daimona Eaytoy 1dcf3fc98c Move a method out of AbuseFilter.php
AbuseFilter::buildFilterLoader is only used in ViewExamine and
ViewTestBatch, so this patch moves it to AbuseFilterView and makes it
non-static.

Change-Id: I7f11cfd7ac81e536492eb59c40da7c14771cee2b
2018-12-09 14:33:30 +01:00
daniel 688eccea47 Expose text from all slots to AbuseFilter
This is a first step towards MCR support in AbuseFilter. The textual
representation of all slots is concatenated. Since AbuseFilter uses
getTextForSearchIndex to determine the textual representation of
content, blind concatenation should not break any assumptions
made by AbsueFilter rules: this naive approach is no worse than
AbuseFilters handling of non-textual content in general, and should
work fine for textual content.

Bug: T209291
Change-Id: Ic141085cad2e11bfe106fe83dafcb35ac31206ba
2018-12-05 09:24:08 -08:00
Daimona Eaytoy 6aff37fb52 Further clarify docs for emergency disable
This is a follow-up to Ic3bc6e36506973b19a9b1bcecbc1a5080faed2ec. I
believe it's important to specify how many recent actions we're looking
at, and I also think it's not nice to rely on a variable amount of
actions to determine whether a filter should be throttled. Also, require
a $group parameter in filterUsedKey (we always pass one, and there's no
reason not to).

Change-Id: I0384d3f1913ead593f605248950606c81c8f8542
2018-11-28 19:29:15 +01:00
Daimona Eaytoy 235162e302 Change method visibility for AbuseFilter class
Some public/protected methods are actually meant to be private.
This patch is only a first step: other methods need to be made
protected/private.

Change-Id: I432c65d333b4dc497532679750f44b2c7e078bf0
2018-11-26 17:35:08 +01:00
Daimona Eaytoy 1f2b7474ed Clarify code and docs for automatic throttling
For the docs part: make it clear how things work there. For the code
part, these are mostly style changes: shorter variable names, no
unnecessary parameters, make the method private, use clearer variable
names.

Change-Id: Ic3bc6e36506973b19a9b1bcecbc1a5080faed2ec
2018-11-26 16:51:10 +01:00
Daimona Eaytoy 7427333ed5 Improve code readability
Simplify some logic constructs, reduce the amount of return statements
inside methods, explicitly declare variables before using them, reduce
code duplication, add names to JS anonymous function to produce clearer
stack traces.

Change-Id: Ife4546a91c30d4c519d09a712ba56a2f33abe579
2018-11-19 16:01:37 +01:00
Daimona Eaytoy 4480c9493a Remove wgParser and wgRequest
As part of the deprecation process of non-config globals.

Change-Id: Ia84ddc20adbfda72347cf256601050b055b87ecf
2018-11-19 13:40:58 +01:00
Daimona Eaytoy badde6ba75 Revert "Revert "Add typehinting for every object-only parameter""
This reverts commit 1ed75b4ae0.
Fixed the one which caused errors, by making articleFromTitle
only use WikiPage, instead of silently mixing WikiPage and Article.

Note for reviewers: this patch is identical to the one which was
previously +2ed, which was mostly correct. To see the actual change,
diff AFComputedVariable with 1..current.

Change-Id: I6747eaed861af6c40a3b1610aebcc1174296e9ed
2018-11-15 10:09:16 +01:00
jenkins-bot 213c2aa011 Merge "Change throttle selector to restore old functionality, overall improvement" 2018-11-15 00:58:11 +00:00
Daimona Eaytoy d3a8491c3f Change throttle selector to restore old functionality, overall improvement
Long (sigh) explanation in T203587#4569698. Also, simplified the way
TagMultiselect are generated, this one and the one for change tags.
This new selector is back-compat both with the old textarea and the OOUI
checkboxMultiselect; actually, this one is //fully// compatible with the
old textarea.
Add validation for throttle parameters and unit tests for validation
(split from I976c95658cddb2585910b6f8a5f047aadc4e4d47).
Added a trim when retrieving throttle identifier to allow syntax like
'ip, user'.
Improved the message shown on history.
Re-added the maintenance script to clean DB.

As I wrote in the task, a review by two other people would be great, at
least for the maintenance script (it could potentially break the DB).

Bug: T203587
Bug: T203336
Bug: T203584
Bug: T203585
Depends-On: I3b2e763bd8835207dc5df1db43d3e1881e6961c3
Change-Id: I7831dbb0bab55807392ac1f7915d6cb0cb713593
2018-11-14 12:51:36 +01:00
jenkins-bot 58018ac7cc Merge "Use log channel 'AbuseFilter' instead of 'AbuseFilter<Suffix>'" 2018-11-08 14:32:58 +00:00
Timo Tijhof e7c0d5f238 Use log channel 'AbuseFilter' instead of 'AbuseFilter<Suffix>'
The channels are a fairly low-level primitive. Having multiple
in production for the same extension I think makes the logs
difficult to navigate and easy to miss things.

For the purpose of grouping, we have normalized_message instead,
which works by using the Monolog template string capabilities,
this is enabled in WMF Logstash (and in Beta).

Change-Id: I10b1fd2d9bdfe518089c053d77fef568170ecb65
2018-11-07 20:21:10 +00:00
Daimona Eaytoy 6658a24554 Remove typehint to avoid fatal error
Temporarily remove the typehint, as it causes some fatals. This doesn't
solve the underlying problem, for which we should first investigate with
I35bfc483a0c69a5cbd38eae8ba299189955fa1ec.

Bug: T208144
Change-Id: I0fdda51010243690ff3806c16d4e203c9ccd8e0a
2018-11-07 11:23:50 +01:00
Aaron Schulz 5071c6574a Use proper cache key construction for throttle, rules, and autoblock keys
Change-Id: I72ab39048f955d4262fae81141cf97243e5cd184
2018-10-21 00:42:08 -07:00
Jforrester 1ed75b4ae0 Revert "Add typehinting for every object-only parameter"
This reverts commit 69d7669069.

Reason for revert: Causing UBN train blocker

Bug: T207220
Change-Id: I3445d9b3065149e2beb149e10fbbf5502b480f57
2018-10-17 01:22:23 +00:00
Daimona Eaytoy 69d7669069 Add typehinting for every object-only parameter
This patch covers every object-only parameter, adding a typehint for it
to avoid errors.

Change-Id: Iebf700621b9dbff78c3bd8f3c136ed15ef4b8d4b
2018-10-15 09:56:09 +02:00
Matěj Suchánek a3cc3dff75 Remove some $wgUser usage
Bug: T159299
Change-Id: I1613e2bb0c551cbadc0c57351fc40bd9e21abf52
Depends-On: I35adef06dfc799cddeddfa6c5eed53b8b1bb7282
Depends-On: Id19a6d883ac6e0cc9c26c923486bca0e414ecaa7
2018-10-14 11:24:52 +02:00
se4598 9d12e1b353 Allow selecting custom disallow message
You can now select a custom message to be displayed for disallowing a edit
the same way as for warn mode. This can be the same or a totally different
message.

This also solves the usecase, when a edit filter is set to warn AND disallow,
to be able to show the user a custom message, but the generic is shown
on the second try (disallow). Now it can be only set to disallow.

Bug: T27086
Change-Id: Ic1de03a6944c43a346fa317ee0a217551f0d284a
2018-10-11 10:35:01 +02:00
Daimona Eaytoy e60dacbbea Fix code comments
Fixed some comments adding explanations, fixing syntax, and parameter types
for docblocks. Also fixed some whitespace mess, and added a missing use
statement.

Change-Id: I3547c90bdaa2cab5443e8bf0c63b217fe6ba663f
2018-10-03 16:45:03 +02:00
Daimona Eaytoy 9144dbf4a1 Remove unused parameter
Nothing uses it, plus it wouldn't work anyway: AbuseFilterParser
constructor only uses $vars if it's instanceof
AbuseFilterVariableHolder.

Change-Id: Idbf53f6058148e9f0e73beb949e1c028a81663ce
2018-09-19 19:58:30 +02:00