Commit graph

86 commits

Author SHA1 Message Date
Daimona Eaytoy e7926114ff SECURITY: Avoid info leak in SpecialAbuseLog
Deleted/suppressed usernames and summaries leak through AbuseLog.
Temporarily hide all non-public revision from AbuseLog, until we can
properly fix the issue.

Bug: T224203
Change-Id: If3d3256404d0f3dbde171831937d1a816b3e2734
2019-09-19 17:46:12 +02:00
Daimona Eaytoy 7917354716 Remove redundant logic from special pages
SpecialPage::setHeaders already handles page title, robot policy and
articleRelated. Moreover, avoid having different messages for the H1
title on the special page and the description shown elsewhere, just like
the base SpecialPage class suggests doing. The deleted messages have
been moved to the default message used by SpecialPage::getDescription.

Change-Id: Iab6beaf64b142e30469afd798c569ef40182153e
2019-09-08 12:30:01 +02:00
Daimona Eaytoy 489da0d229 Add a 'strict' option to VariableHolder::getVar
This will help mitigating problems like T230256 by enforcing that the
requested variables must exist. For now, it will only log bad usages,
thus providing a way to identify affected filters and fix them.

Bug: T230256
Change-Id: I7a61916576e444a56f0e07da7b6e5033346226bd
2019-09-04 18:19:23 +00:00
jenkins-bot 3d319edba9 Merge "Upgrade phan-config to 0.7.1" 2019-09-02 12:46:29 +00:00
jenkins-bot 2ed5be29e1 Merge "Use permissions accessors" 2019-09-02 10:55:46 +00:00
Daimona Eaytoy 393e47c5a7 Upgrade phan-config to 0.7.1
Change-Id: I859d81eda8601da91602b27a223b6d6d59ecf563
2019-09-01 09:42:26 +00:00
jenkins-bot 134c75dd5e Merge "Remove AbuseFilter::saveFilter dependency on AbuseFilterViewEdit" 2019-08-31 17:06:00 +00:00
Daimona Eaytoy eb91595d8a Use row->afl_action instead of $vars
There's not need to use the variableholder in this hacky (albeit common)
whay, since the row already holds the action.
Note, this doesn't guarantee that the next two lines won't fail - I'd
need to see the actual var dump (T231542#5450720) to determine exactly
why this is failing.

Bug: T231542
Change-Id: I2112b046d00e06b575d15ab3d7da57484fd9cbbd
2019-08-29 13:48:31 +02:00
Daimona Eaytoy 540edc5174 Remove AbuseFilter::saveFilter dependency on AbuseFilterViewEdit
This dependency is wrong, and removing it will also allow creating an
edit API.

Bug: T213037
Depends-On: Id8412e2b8a4e873fd4821ecc1a3c95710be9a870
Change-Id: If8e745a3227cea5093ea3fd8f5b201adedaba3ae
2019-08-27 16:26:18 +00:00
Daimona Eaytoy 87713008d5 Use permissions accessors
There are lots of calls to $user->isAllowed which could be simplified
using available accessors like canEdit(). So simplify those calls and
avoid duplication.

Note that using canEdit also fixes a bug which affected blocked users:
we used to show e.g. the import link, and not to display as disabled
several text fields, while blocked users cannot actually edit filters.

Depends-On: I22743557e162fd23b3b4e52951a649d8c21109c8
Change-Id: I62779e940949ef49018a9c6d901bb6e10aa81da8
2019-08-27 13:21:55 +02:00
Daimona Eaytoy c469fb4b76 Mostly remove $wgUser
There are lots of cases where we can inject a User object without
additional efforts. Now $wgUser is only used inside AFComputedVariable,
which is a little bit harder to handle because some instances of that
class are serialized in the DB, and thus we cannot easily change the
constructor until T213006 is resolved.

This partly copies what Ia474f02dfeee8c7d067ee7e555c08cbfef08f6a6 tried
to do, but adopting a different approach for various can*() methods:
they're now static methods in the AbuseFilter class, so future callers
don't need to instantiate an AbuseFilterView class. This also allows to
re-use those methods in an API module for editing filters (T213037).

Bug: T213037
Bug: T159299
Change-Id: I22743557e162fd23b3b4e52951a649d8c21109c8
2019-08-27 13:20:37 +02:00
Daimona Eaytoy c34181e548 Add a new API module to retrieve private details from AbuseLog
Follow-up of Iaca492371f48fecf543268c179a651841ed12c3f. This patch adds
the new module, plus some technical changes to private details-related
methods and globals.

Bug: T210329
Depends-On: I613dbadb8f75c8c4116a362607563a436a73d321
Change-Id: I3c45b74c36c191083df184ed57416067a75f6591
2019-08-09 21:10:28 +00:00
Daimona Eaytoy c7ccb68058 Use "privatedetails" instead of "private" where needed
To keep a clear distinction between "private details" (i.e. user's ip)
and "private filters" (i.e. not publicly viewable). This patch renames
rights, i18n keys and methods names.
The patch for renaming globals and rights in WMF config is
I7e6b3d4453403edb6aa602587374b4ff5b6d625f.

Bug: T211004
Change-Id: I613dbadb8f75c8c4116a362607563a436a73d321
2019-08-09 21:10:22 +00:00
Daimona Eaytoy 483dab1732 Add a parent class for special pages
This allows us to extract yet another static method from the AbuseFilter
class. This class should be expanded in the future, and an example use
case could be Ia5fd4f0b35fcabf045a7b49fa40fa85b72c92544.

Depends-On: I7c0170167b508132cd16e566c654a6c98dd683e9
Change-Id: I1bb45e47c3b42c01388b99778ce833e4e44419e1
2019-08-06 14:17:38 +00:00
Daimona Eaytoy 71e3719e12 Clarify "filter" field in SpecialAbuseLog and ApiQueryAbuseLog
The "filter" fields can also accept a list of filters, and also global filters, so make it clear in the UI and in messages.

Change-Id: Ib258716d8e6792fd496938ebb4e8a2565d6370b7
2019-08-04 16:55:05 +00:00
DannyS712 eb278479d5 Add help links to special pages
Bug: T226938
Change-Id: I50a76733b3b8d4ee72ccc6816b58a67a66b2f603
2019-07-03 16:06:16 +00:00
Daimona Eaytoy 6ea767f171 Tweak methods related to global filters
To make the switch to afl_filter_id and afl_global easier.

Bug: T227095
Depends-On: Ie550889495232b534c0f9aec31039cf21b2135b1
Change-Id: If557bad8f5c1a6d15e3556e4bfbd0330d7d49c59
2019-07-02 17:02:50 +02:00
Daimona Eaytoy 0b925da36e Drop afl_log_id
Field unused since its introduction.

Bug: T214592
Change-Id: I1f4f775e9678de5184672251631a490e4eb81764
2019-06-28 17:55:55 +00:00
Daimona Eaytoy 6100955242 Use more verbose names for filter IDs
Follow-up of Ie550889495232b534c0f9aec31039cf21b2135b1, suggested by
Krinkle.

Change-Id: Ia8f40644c7f4a6ed53186a5edab5df1bd313166a
2019-06-25 18:20:32 +00:00
Daimona Eaytoy e7cd4b2a98 Rewrite AbuseFilter::decodeGlobalName
Now it returns an array with a bit more info, and has a different name
to reflect the fact that its input is now split in two parts. Plus, make
it throw whenever it gets an unexpected input, and add a bunch of test
cases for it.

Depends-On: Ib5fdeb75c1324f672b4ded39681f006fde34b4d1
Change-Id: Ie550889495232b534c0f9aec31039cf21b2135b1
2019-06-12 23:56:25 +00:00
Thiemo Kreuz aba489a1f4 Add missing type hint to SpecialPage::execute()
[Also make use of the list() feature in one case I forgot before in
If2b6c95.]
 -> Changed to use direct array access by Daimona per inline comment.

Change-Id: I708dff30b6e00ccab3257b2e6fa5995eb9e30e0f
2019-05-16 14:31:54 +00:00
jenkins-bot cdad0e1a14 Merge "Revert "Use string cast for Postgres compatibility"" 2019-04-18 15:31:51 +00:00
Daimona Eaytoy 9a315f2a6e Revert "Use string cast for Postgres compatibility"
This reverts commit 4ab12305f1.

Bug: T221357
Change-Id: Id0f26f48ad9904e73a8b65d76586957c2be93e82
2019-04-18 11:51:16 +00:00
jenkins-bot ca6ef32a69 Merge "Use string cast for Postgres compatibility" 2019-04-11 21:50:06 +00:00
jenkins-bot efe32b7c93 Merge "Add doc for every class member" 2019-04-06 14:37:19 +00:00
jenkins-bot cf5df265b0 Merge "Allow filtering AbuseLog for filter group" 2019-04-06 12:24:10 +00:00
Daimona Eaytoy 7fb3ea9002 Reduce the amount of returns
Having a single return statement inside a function isn't always the
best, but having 5 is probably worse. This patch changes three long
if-return/if-return/... to a single if/elseif + return.

Change-Id: I5f4603627c61cf1b93859fe6bcd952eac8e82359
2019-03-30 09:52:56 +00:00
Daimona Eaytoy 4ab12305f1 Use string cast for Postgres compatibility
We JOIN integer and text, so Postgres would always fail on these. As
mentioned in the task description, this is only a temporary solution
(although a clean and durable one), while the long-term one is
I7460a2d63f60c2933b36f8383a8abdbba8649e12.

Bug: T42757
Change-Id: Ifddd0bca1e8eaa7c70511fb0d0588457b4fd0669
2019-03-23 12:44:02 +01:00
Daimona Eaytoy 01f699ff07 Remove useless SpecialAbuseLog::getNotDeletedCond
The method is used to make afl_deleted = null treated as afl_deleted =
0. Digging into code history, I found that it's in place because:

*In rEABF14b850f891de27ea09a1439e3835f66c49ad773f the afl_deleted field
was introduced as NULL, and wasn't used.
*In rEABFfe39e38282fc4c7903eb3f8080dbf0bab0f697f4 it was ALTERed to be
"NOT NULL DEFAULT 0"
*And in rEABFa2ead8bfb5166e0b354f3bb3e09f39795cb5b1c0 this function was
introduced to "negate the need for a schema change".

However, when ALTERing afl_deleted to be NOT NULL DEFAULT 0, all NULL
values have been automatically converted to 0 thanks to the DEFAULT
clause, and being the column NOT NULL, of course no NULL are still
there... The ALTER was applied to all wikis (in 2010), so afl_deleted is
NOT NULL everywhere and we can safely treat it as such.

Change-Id: Iebd843629d26e392d2e24efc2795c767e854897a
2019-03-23 11:49:30 +01:00
Daimona Eaytoy f2c1beec44 Replace double-equals with triple-equals
Since double-equals are evil. I left some of them in place where I
wasn't sure, but I may be changed some which were intended to be
doubles. It could be a good idea to delay merging this patch until we'll
have more code coverage.

Change-Id: I1721a3ba532d481e3ecf35f51099c1438b6b73b2
2019-03-22 16:12:13 +01:00
jenkins-bot ef8c8e6006 Merge "Make the filter field on ViewHistory of "int" type" 2019-03-17 12:00:47 +00:00
Daimona Eaytoy bedbe36744 Add doc for every class member
Adding PHPdocs to every class members, in every file. This patch only
touches comments, and moved properties on their own lines. Note that
some of these properties would need to be moved, somehow changed, or
just removed (either because they're old, unused leftovers, or just
because we can move them to local scope), but I wanted to keep this
patch doc-only.

Change-Id: I9fe701445bea8f09d82783789ff1ec537ac6704b
2019-03-17 11:40:24 +01:00
Daimona Eaytoy 6158d96c41 Don't escape externalLink text
As escaping is handled by makeExternalLink itself. This currently makes
seccheck fail for any patch and is a merge blocker.

Change-Id: I2d21632bbc59abd4ea48aebdb6572d53f8fc89cd
2019-03-11 13:23:49 +01:00
Daimona Eaytoy 27f8b9ab34 Make the filter field on ViewHistory of "int" type
Following up I636b4e56f39282593c737ace1d6ff2d90900d997, enforce a basic
clientside validation and don't fill the field with the URL parameter if
it's not valid.

Change-Id: If4fd015dff64237375a0c4d3b9fbcefbd54dba3e
2019-02-10 12:11:52 +01:00
Daimona Eaytoy e0b2b9ffd5 Allow filtering AbuseLog for filter group
This adds the capability to filter AbuseLog using filter groups, if
there's at list an extra group (like flow). Since abuse_filter_log
doesn't store info about filter groups, this needs query on
abuse_filter, and its result must then be intersected with explicitly
searched filters, if any.
The way I wrote it takes several lines and IFs, but is meant to be less
subject to regression in case something gets moved.

Change-Id: I747ba491d2b390562ce5f71396eed095116d8eaf
2019-02-08 10:51:11 +01:00
jenkins-bot 981a447aaf Merge "Remove ancient permission checks" 2019-02-07 21:13:16 +00:00
Daimona Eaytoy 2a0246ddb5 Remove ancient permission checks
In both SpecialAbuseLog and ApiQueryAbuseLog, we use
Title::getUserPermissionsErrors to check if the user is allowed to
perform 'abusefilter-log' on the API page... However, this is a
completely redundant check (which is also pretty expensive and queries
the master): for the SpecialPage, we can specify the required right in
the constructor and use checkPermissions, and for the API we can simply use checkUserRightsAny.
If I'm not mistaken, there's no benefit in using
getUserPermissionsErrors.

Change-Id: I4c4dbace67b24cc1f45e50ab1c0d251522935513
2019-01-31 21:16:18 +00:00
Daimona Eaytoy 4b33b2b5a7 Strike suppressed AbuseLog entries
Instead of adding a message, do like core does by striking and greying
out the row. Plus, don't show the AbuseLog page description when hiding
entries, as it doesn't fit.

Change-Id: I645a89dd8df79d45ca440e0ba62adcdee921b8e9
2019-01-23 11:34:43 +01: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 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
Daimona Eaytoy 103dfa3b66 Remove info leak
Oversighted/deleted edits and log actions were entirely accessible to
non-oversighters via AbuseFilter/examine for RC, and via AbuseFilter/test.
Now, we take into account the revision/log visibility and user permissions to
determine what to show.
Other changes in this patch:
*Show the examine link if and only if the user can examine the given row
*If a revision is hidden but the user can see it, don't hide its elements in
 ChangesList (only leave them striked/greyed)
*Make APIs better understand revision visibility.
*Make a clear distinction between deleted and suppressed edits/log
entries.

Co-authored with rxy <git@rxy.jp>

Bug: T207085
Change-Id: Icfa48e366a7e5e3abd5d2155ecfddfc09b378088
2018-10-23 10:53:39 +00: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
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
Matěj Suchánek 853936316f Filter AbuseLog by triggering action
For now, there is an "Other" field which will show all but hard-coded actions.

Bug: T187971
Change-Id: If564aced2e9cd933d8cfcf7cb96166aa279f2823
2018-08-23 11:40:15 +02:00
Daimona Eaytoy 66774b8d7a Show an extract of suppression log for Special:AbuseLog
Like we do in core for similar special pages. This is really helpful
when (un)hiding an entry.

Bug: T200645
Change-Id: I16450a2573e8987e31a83ec34f3dbb16fac94f81
2018-08-21 19:19:10 +02:00
Matěj Suchánek 10ad58a6f3 Migrate AbuseFilter suppress log
Also make entries in Special:Log/suppress filterable.

Change-Id: Ic23e724997e4748c8d0da8138aa73d31b17b7064
2018-08-21 16:05:54 +00:00
Umherirrender 2b615cfa29 Avoid variable reuse to pass taint-check
Also set param-taint for value of WebRequest::getText

Bug: T197002
Change-Id: I9e52d24f88789c99c726e32df20840707d1b47ae
2018-08-20 19:54:20 +02:00
Daimona Eaytoy 9d21c7d03d Change priority order for messages in hidden abuselog entries
Check if the entry is deleted first, since it's the strongest deletion
here (oversight level). Bonus: don't use implicit conversion when
checking the return value of SpecialAbuseLog::isHidden.

Bug: T200644
Change-Id: Ie5c4575ad29fe3dcb85a26cc74f1c59207df2852
2018-08-19 18:22:07 +02:00
Daimona Eaytoy 0e87c44c74 Show AF logs for a revdeled revision if the user can see the revision
The function used to determine if a row is hidden has three possible
return values: true, false and "implicit". While the first and the
second one refer to AF own suppressing system, 'implicit' means that the
revision associated with the log entry is deleted. However, we checked
for such return value with a boolean cast, which caused true and
'implicit' to be equally treated, thus hiding revdel'ed revisions to
sysops. Bonus: fixed a comment typo.

Bug: T191699
Change-Id: I87d3a6437bb966198175e4bfd063e30ed79c345f
2018-07-14 00:46:19 +02:00