Commit graph

475 commits

Author SHA1 Message Date
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 a2bee3bcf3 Merge "Simplify parser methods" 2019-01-19 12:11:04 +00:00
jenkins-bot 7f62874658 Merge "Change method visibility for AbuseFilter class" 2019-01-19 12:02:51 +00:00
jenkins-bot 0d4e982069 Merge "Reduce code duplication" 2019-01-19 12:00:47 +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 b1e8f38c64 Merge "Replace RecentChange::$mAttribs with getter functions" 2019-01-11 20:01:12 +00:00
addshore a6a93be530 Pass MCR AF text into newVariableHolderForEdit
Follow up to Idbb3a70d08a195dfa21422e07f593d1eeba4521d

This also fixes the fetching of text for the stash edit code path
which was missed by the previous patch.

This now also uses the full old text in the variable holder.

Bug: T213453
Change-Id: Ib80bc6385ebb5dd82bb1a384dd0e162608bfcbfa
2019-01-10 23:42:58 +00:00
addshore 3e93c06223 Use slot in onEditFilterMergedContent
Related to If3c4592eb6dade6960463abfda017af35d04f563
in Wikibase, needed for SDoC.

Bug: T213453
Change-Id: Idbb3a70d08a195dfa21422e07f593d1eeba4521d
2019-01-10 20:57:30 +00:00
Daimona Eaytoy fda8f01431 Replace RecentChange::$mAttribs with getter functions
The RecentChange class has several getters and setters for the $mAttribs
property. Although the property is public, it's saner to use such
methods.

Change-Id: Ie8e37e80fdcf2917ee0e87b2a409f0afb91a4f92
2019-01-02 11:36:57 +01: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
jenkins-bot 102f6f7497 Merge "Fix big problems with normalizeThrottleParameters" 2018-12-17 03:34:34 +00: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 f49d4e5caa Emit debug logs when filtering without title
We have two situations where we try to execute filters without a title.
However, the code doesn't handle it correctly: some points expect $title
to actually be a Title object, and we also pass it around using a hook
which explicitly says it always pass a Title. This patch adds two debug
points to help understand why we end up with null titles, so that we can
fix it upstream.

Bug: T144265
Change-Id: I35bfc483a0c69a5cbd38eae8ba299189955fa1ec
2018-12-13 20:34:21 +00: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
jenkins-bot 1dd8f41d0d Merge "Use the updated TitleMove hook to filter move actions" 2018-12-04 19:32:04 +00:00
Daimona Eaytoy 206bdc1f6a Use the updated TitleMove hook to filter move actions
For several reasons:
*We're not really checking permissions (and the hook previously used is
meant to be used in such case)
*We'll show a cleaner error message (i.e. without the "You do not have
permission..." part)
*Filtering will happen closer to the actual move

Bug: T208907
Depends-On: I4733724075b7514e9db59e7be772d9409aa9da87
Change-Id: If88f736a446247f8b4b13c055c641d56f544d1ea
2018-12-04 18:58:04 +01:00
jenkins-bot 23a7aa69a5 Merge "Fix regex group counting for get_matches" 2018-12-04 13:58:06 +00:00
jenkins-bot bb289862ff Merge "Remove code for old global variables" 2018-12-04 06:27:32 +00:00
Huji Lee b523194032 SECURITY: Remove private information from the API results
Later, we will add a new POST request which will allow retrieving
the private details; it will have a mandatory "reason" parameter,
and will result in a log entry in the private details access log,
just like the web interface.

Bug: T210329
Change-Id: Iaca492371f48fecf543268c179a651841ed12c3f
Signed-off-by: sbassett <sbassett@wikimedia.org>
2018-12-03 23:11:32 +00:00
Daimona Eaytoy 7ca0941d1f Remove code for old global variables
Those two global config variables were removed more than 2 years ago, in
I790d39c2849922d7daf7479f298cd90cf30af129. Nothing else in the code
references them, so we can just remove the warning.

Change-Id: I427d06a80131447ea64064f45e84349f93e72cca
2018-12-02 16:24:09 +01: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 e055ecc7c6 Reduce code duplication
Change-Id: I03bd56e4bf455865b27338ac39b3dcef20a88447
2018-11-19 15:50:36 +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
jenkins-bot 0d58f78030 Merge "Revert "Revert "Add typehinting for every object-only parameter""" 2018-11-18 16:27:27 +00:00
jenkins-bot 6541d7c5cc Merge "Check that the user block is sitewide when determining permissions" 2018-11-15 17:26:21 +00:00
Daimona Eaytoy 346063eec0 Check that the user block is sitewide when determining permissions
And bump MediaWiki version.

Bug: T208621
Change-Id: Icfcf09c5d7c7498711cb000c3bb16480270efb9c
2018-11-15 17:59:22 +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
Brad Jorsch f6349e7a32 Update tests that fail with comment/actor migration
* AbuseFilterConsequencesTest is somehow leaving blocks behind. Mark
  ipblocks as being used to avoid that.
* AFComputedVariable::getLastPageAuthors() uses indeterminate order for
  multiple revisions with the same timestamp. Fall back to rev_id
  ordering like MySQL accidentally did before.
* AbuseFilterTest tries to create revisions attributed to users that
  don't exist. Switch to interwiki usernames.

Change-Id: I30f7cdcc3875f3f7af116c1e41e88f62ab9e91d0
2018-11-09 17:03:36 -05: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
Daimona Eaytoy 16475c0266 Fix regex group counting for get_matches
Adding the * as character to match after parentheses, since it may be
used with backtrack verbs (e.g. (*FAIL), (*SKIP)). I guess this is a
very, very rare use case, but since the fix is easy, let's include it.
Also, added a ToDo since we should probably find a better way to count
capturing groups, although I cannot figure out any.

Change-Id: Idcb303b4740530af9d3f009414d35d68f59effd0
2018-11-01 11:52:33 +01:00
C. Scott Ananian b73786df5c Replace deprecated OutputPage::parse/parseInline()
The OutputPage::parse/parseInline() methods emit untidy output and
are often used with the wrong user interface/content language
selection.  Replace with new methods added in 1.33 which are
tidy and consistent.

Bug: T198214
Depends-On: Ica79c2acbc542ef37f971c0be2582ae771a23bd0
Change-Id: Iec8071f4e50f169356e4f68ccb746c55f1606ea6
2018-10-26 13:33:20 -04:00
jenkins-bot c8d85e27b8 Merge "Use proper cache key construction for throttle, rules, and autoblock keys" 2018-10-24 10:10:51 +00:00