Commit graph

495 commits

Author SHA1 Message Date
Daimona Eaytoy 196a3ba7d5 Ensure the specified filter is valid in ViewHistory
We do not validate the param, and instead only check if it was
specified. In the specific case of ViewHistory, specifying as "filter"
something invalid for a title (e.g. with a + inside) will throw an
exception, seen in production.

Change-Id: I636b4e56f39282593c737ace1d6ff2d90900d997
2019-01-29 19:58:59 +01:00
Daimona Eaytoy 8f9b27d856 Fix MWException from AbuseFilterView
The case default was recently added, but didn't take into account that
"false" is valid too. Noticed by chance just before the train rolled
out.

Change-Id: I67ca475fa16ea449820f8c735531c2cc1b0ec975
2019-01-24 21:48:50 +01: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
jenkins-bot b9c697ef7c Merge "Don't send long patterns with GET" 2019-01-20 14:20:31 +00:00
jenkins-bot 1ed8fd0a5a Merge "Simplify filter editor generation and restore ltr attribute" 2019-01-19 13:28:50 +00:00
jenkins-bot 41f6a85a42 Merge "Rewrite the method for getting a global emergency value" 2019-01-19 13:25:41 +00:00
jenkins-bot 196272fbc1 Merge "Move changed field styles to TD for history pager" 2019-01-19 13:18:32 +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 b35ba5af45 Merge "Warn the user if they try to leave the page with unsaved changes" 2019-01-19 12:57:50 +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 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 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 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
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
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
Daimona Eaytoy 38749b46bb Warn the user if they try to leave the page with unsaved changes
While editing filters, sometimes it happen that you make some change,
forget about it and then reload/close the page, and no warnings will be
issued. This patch makes use of the core module used for normal page
editing to display a warning if trying to leave a filter editing page
with any unsaved change (both to the filter pattern or other form
elements).

Change-Id: I78d79215565d5c82028b1a2a4276497ccbffdea2
2018-12-04 13:06:46 +01: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