Commit graph

103 commits

Author SHA1 Message Date
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 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 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
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 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 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
Daimona Eaytoy 31729b044e Unbreak /examine for old log entries
For the moment, this is a simple workaround to get them back working.
Ideally we'd also need a maintenance script to update var dumps as I
wrote in the task, but it needs more thinking (see Phab).

Bug: T204236
Change-Id: Ia20a2eb495557f46f789467a96e654ec6cd3f355
2018-09-13 18:42:47 +02:00
Matěj Suchánek 6eb5d9766b Use correct variable in AbuseFilter::addLogEntries
The data was inserted to the foreign database, so the id needs
to be fetch from that one.

Change-Id: I8eef8d74fc924829447e31f4445154b01b92aa7a
2018-09-13 11:57:55 +02:00
jenkins-bot dee934cd5a Merge "Partly unbreak throttle action" 2018-09-09 12:03:40 +00:00
jenkins-bot fb864408e3 Merge "Replace wfDebug and wfDebugLog with logger" 2018-09-09 11:55:02 +00:00
Daimona Eaytoy 8885a5983e Partly unbreak throttle action
This action have ALWAYS had this problem: when creating a new filter,
the temporary ID is 'new', and the throttle ID is then 'new'.
This is used when creating/checking throttle keys to determine if the
user should be throttled. However, the 'new' key is not unique and
(although it's not the only part of the key), it may lead to
unpredictable behaviours. I'm not sure if this solves the task below,
but can probably help.
Also added a FIXME that we should handle shortly.

Bug: T195699
Change-Id: Id3b0ff524c52fb57fdd72f9608f758f8383e4320
2018-09-09 07:09:14 +00:00
jenkins-bot a9f9742b28 Merge "Remove the last PHPCS exclusion" 2018-09-05 17:07:51 +00:00
jenkins-bot 7cce6d1864 Merge "Remove _age variables from cache keys" 2018-09-03 12:08:29 +00:00
Daimona Eaytoy 63803568d6 Remove the last PHPCS exclusion
Bug: T178007
Change-Id: I5ddb811c2cb15040a859a63b64873f0fa53508ee
2018-09-03 10:42:30 +02:00
Daimona Eaytoy 2f0a0a0893 Replace wfDebug and wfDebugLog with logger
Per standard on
https://www.mediawiki.org/wiki/Manual:Structured_logging. The use inside
AbuseFilterParser is removed in
Ie55746bb7731062ae2d46d84857af2a05d78cf4c.

Change-Id: Ia62287c4ff5f904557cd6d43d47a9f4d9696b94b
2018-08-29 10:57:56 +02:00
Daimona Eaytoy 934399de45 Remove _age variables from cache keys
As we do for user_age, since these will always change. Also, rework the
method to avoid repetition of unset().

Change-Id: Ie5ceedd89cae3813bacf6680d588bc925362c2c2
2018-08-26 16:02:32 +02:00
Daimona Eaytoy 66318915db Use === operator with strpos
The condition always evaluated to true: for global filters strpos
returned 0, otherwise it returned false, which is == 0. Fortunately, in
the second case the function returned false as it should. Anyway, be
safe and use === operator as it should always be for strpos.

Change-Id: I7ffc990b2b8b9c47ebfb64d5234f561faaff5e88
2018-08-25 17:35:15 +02:00
jenkins-bot 81a4fdc964 Merge "Improve Ace syntax highlight" 2018-08-23 10:14:57 +00:00
Daimona Eaytoy e8a4517d6b Improve Ace syntax highlight
Several improvements, this is the list:
*Added highlighting for disabled and deprecated variables
*Simplified a bit Ace's keyword mapper
*Added highlighting for ternary operator
*Added logic to retrieve operators from AF tokenizer
*Removed $ symbol since it's not usable in declaring stuff
*Customized highlighting via CSS

Depends-On: I5c370b54e6516889624088e27928ad3a1f48a821
Change-Id: If95e34fc7260413c4fb39c18a1ef44f5a93e1a68
2018-08-22 15:23:35 +00:00
rarohde e1865fca74 Add page_age variable to AbuseFilter
Adds page_age variable that reports the number of seconds since the
first edit to the current article (or 0 for new articles).

Bug: T30844
Change-Id: I0993cecc322806382a1b567b60c0a4af69054841
2018-08-22 17:10:39 +02:00
Daimona Eaytoy 6bc630cfef Add aliases for "_text" and "article_" variables
Variables regarding title (full list in task description) are quite
deceiving, since they use "text" instead of "title". As proposed in the
task, this is the first patch to add aliases for those variables and
slightly deprecate the old ones. In the future we may be able to replace
every occurrence (either with a search function or directly on the
database), but even a coexistence would be enough to avoid
confusion. A wfDebug log is generated whenever a deprecated variable is
parsed. The "article_" prefix is also changed to "title_", in the same
way as above.
Also, added a hook which other extension may use to specify their
deprecated variables, which will be handled the same as core ones.

Bug: T173889
Change-Id: I5c370b54e6516889624088e27928ad3a1f48a821
2018-08-21 16:59:56 +02:00
Umherirrender c954b412c6 Include CheckUser in phan config
Depends-On: I51421184485c3117bbab9ce3dd42f2dbb6c6180c
Change-Id: Ida17580b301ff4a6b0d3d0020c48f65eb1e21026
2018-08-17 17:38:01 +02:00
Huji Lee df21fb2b20 Remove HitCounters from AbuseFilter and use hooks instead
Goes with Ief573fb412d332bd4ad6ad8de3052dd85d534b82

Bug: T159069
Change-Id: I38cd7cbf3e595890b53624a477010bd49c9b8552
2018-07-31 03:56:20 +00:00
jenkins-bot 55d825c325 Merge "Use empty arrays instead of empty strings for diffs" 2018-07-27 00:55:40 +00:00
jenkins-bot 0979e116b4 Merge "Show AF logs for a revdeled revision if the user can see the revision" 2018-07-18 02:04:16 +00:00
Daimona Eaytoy b825e396b5 Switch editing interface to OOUI & improve NoJS usability
This settles almost everything, leaving the tags part ready to be
further improved in the follow-ups.
Also, replaced some fields with totally different ones, improved the
warn preview area and improved a bit nojs experience by hiding unusable
buttons.

Bug: T132284
Bug: T154749
Change-Id: I7a5caa862a32f9792140c6a4d9708a2d20472672
2018-07-17 14:49:50 +02:00
jenkins-bot 3e28ac176e Merge "Use isset instead of empty+strlen when checking disallow parameters" 2018-07-17 02:20:32 +00:00
jenkins-bot fed9ca759d Merge "Remove and replace an argument in AbuseFilter::addLogEntries" 2018-07-16 22:40:00 +00:00