Commit graph

168 commits

Author SHA1 Message Date
STran b66daede0a Log specific views of protected variables
Like CheckUser, AbuseFilter should also log when specific protected
logs are viewed.

- Add support for debouncing logs to reduce log spam
- Log when AbuseFilterViewExamine with protected variables available
  is accessed
- Log when SpecialAbuseLog with protected variables available is
  accessed
- Log when QueryAbuseLog with protected variables available is accessed

Bug: T365743
Change-Id: If31a71ea5c7e2dd7c5d26ad37dc474787a7d5b1a
2024-10-02 00:53:34 -07:00
STran bd819b98a2 Add preference for viewing protected variables in AbuseFilter
Users need to enable a preference before gaining access to the IPs
from `user_unnamed_ip`, a protected variable.

- Add a preference that the user can check to toggle their access
- Check for the preference and the view right for logs that reveal
  protected variables on:
  + AbuseFilterViewExamine
  + SpecialAbuseLog
  + QueryAbuseLog

Bug: T371798
Change-Id: I5363380d999118982b216585ea73ee4274a6eac1
2024-09-12 07:59:24 -07:00
Bartosz Dziewoński 517beb3c0d Use namespaced MessageSpecifier
Depends-On: I9ff4ff7beb098b60c92f564591937c7d789c6684
Change-Id: I7097b4d80df790ef14a5bc053306dc2f1fd195da
2024-07-28 21:59:35 +02:00
Bartosz Dziewoński 3df92fcbe4 Use stable andExpr() / orExpr() methods
Change-Id: I0010a7c9d273e63acbed78190f0c23283a192ef2
2024-07-11 18:36:04 +02:00
STran 30227231f6 Disallow protected variable access on AbuseFilterViewTestBatch
A filter using a protected variable can be loaded via filter id
using testing tools even though the user might not have the right
to view protected variables. This can potentially leak PII and as
such, testing tools should check for the right before allowing
protected filters to be seen.

- Unload a filter asap if it uses protected variables and the
  requestor doesn't have viewing rights. This:
    + disallows loading of existing protected filters on page load
    + disallows testing against rules that use protected variables
    + disallows subsequent requests for protected filters (via API)

There is a known bug (see T369620) where no user feedback is
provided if an API request for a filter returns no result (typically
when no filter matches the requested id). This commit adds another
pathway to that bug (the filter exists but is protected and not
returned by the API) but does not update this UI/UX.

Bug: T364834
Change-Id: I6a572790edd743596d70c9c4a2ee52b4561e25f3
2024-07-10 05:31:03 -07:00
jenkins-bot 218627233b Merge "Only return filters visible to user in search" 2024-07-09 09:45:55 +00:00
STran ceaedb8b95 Only return filters visible to user in search
Search is restricted to users with the right to view private variables
but not necessarily the right to view protected variables. Users who
don't have the right to view protected variables shouldn't be able to
search against protected variables, as this might leak the PII.

- Filter out filters using protected variables in search results
  if the user doesn't have the right to view protected variables

Bug: T367390
Change-Id: I7412112c9cc676f29d706b116b779bc17183a952
2024-07-08 02:47:57 -07:00
Daimona Eaytoy 99bb44beb4 Miscellaneous minor fixes
- Rename `$hidden` to `$privacyLevel` in Flags::__construct for
  consistency with other places.
- Rename `shouldProtectFilter` and simplify its return value to always
  be an array, since that's how it's currently used. Rename a variable
  that is assigned the return value of this method.
- Add a missing message key to a list of dynamic message keys.
- Rename a property from 'hidden' to 'privacy' in FilterStoreTest for
  consistency. Add a test for removing the protected flag.
- Update old comment referencing `filterHidden`; the method was removed
  in I40b8c8452d9df.
- Use ISQLPlatform::bitAnd() instead of manual SQL in
  AbuseFilterHistoryPager.
- Update mysterious reference to "formatRow" in SpecialAbuseLog.
- Update other references to the very same method in two other places,
  this time credited as "SpecialAbuseLog".
- Add type hints to a few methods; this not only helps with type safety,
  but it also allows PHPUnit to automatically use the proper type in
  mocks.

Change-Id: Ib0167d993b761271c1e5311808435a616b6576fe
2024-07-03 02:31:38 +02:00
Matěj Suchánek 92334b698b Support more log actions in testing interface
- Allow testing "move_redir" page moves.
- Allow testing "create2" and "byemail" account creations.
- Add remarks in the code that "autocreate" account creations
  cannot be tested since they are not in the recent changes.

Change-Id: Idd38327df1477e1cba4396003a6c0f23cb75d276
2024-06-19 17:35:43 +02:00
jenkins-bot c1961b9d99 Merge "Use expression builder in AbuseFilterView::buildTestConditions" 2024-06-17 19:55:35 +00:00
jenkins-bot 4e919e4338 Merge "Add protected variable view permission checks" 2024-06-13 13:18:14 +00:00
STran abe6f1f4ee Add protected variable view permission checks
Some features restrict access when filters are private. These features
should treat protected filters similarly.

If the user doesn't have view rights for protected filters:
  - Disallow viewing of logs generated by protected filters
  - Disallow querying of matches against protected filters

Bug: T363906
Change-Id: Id84bd4ca7c8e0419fccc3ad83afff35067c9bf70
2024-06-13 03:15:04 -07:00
Umherirrender bd52ef6263 Use expression builder in AbuseFilterView::buildTestConditions
Bug: T350968
Change-Id: I2b91a7e5b18ac9bb4f79018014ed60e2f0830487
2024-06-12 20:43:46 +00:00
Umherirrender c3af3157b4 Use namespaced classes
Changes to the use statements done automatically via script
Addition of missing use statement done manually

Change-Id: I48fcc02c61d423c9c5111ae545634fdc5c5cc710
2024-06-12 20:01:35 +02:00
jenkins-bot 056bcafe08 Merge "Use expression builder to build where conditions" 2024-06-09 03:45:36 +00:00
jenkins-bot 7bbf55935c Merge "Hide the checkbox for protecting a filter from users without the right" 2024-06-07 10:39:43 +00:00
jenkins-bot 4b9c2e612c Merge "Clarify protected status in filter checkboxes" 2024-06-06 18:00:27 +00:00
Thalia 6428ff9232 Hide the checkbox for protecting a filter from users without the right
Any filter using protected variables must be marked as protected via
the a checkbox on the filter edit form. This checkbox should not be
visible to users without the right to use protected variables.

Bug: T364485
Change-Id: If2c4b8f50f447e951d820798f181839d10501aa3
2024-06-06 17:49:46 +01:00
STran 1c96981117 Clarify protected status in filter checkboxes
The UI/UX for acknowledging a filter will be protected/is protected
could be clearer. The checkbox implemented currently doesn't make
it clear that the acknowledgement is mandatory and filters that are
already protected allow for the checkbox to be unchecked even though
that doesn't reflect that the filter cannot be unprotected.

- Update copy for the protected filter acknowledgement to make it clear
  that it's a mandatory acknowledgement, not an optional one
- Update copy for the error that shows when a filter that should be
  protected doesn't have the acknowledgement checked
- When a filter is already protected, disable the acknowledgement
  checkbox to indicate this is not mutable

Bug: T364485
Change-Id: I667fcca4511dff1ac3ca69930c5b5e5eb5001787
2024-06-06 00:23:39 -07:00
Thalia 8f39ef3b8a Fix permission error shown on history page for protected filter
When a user without the right to see protected filters visits
Special:AbuseFilter/history/<ID>, show the permission error
message for protected filters.

Before this commit, the error message for hidden filters is
used instead, even if the filter is not hidden.

Bug: T364465
Change-Id: If2573fe256a7e29e8184feaf2f0622659706fd56
2024-06-05 15:53:04 +01:00
jenkins-bot 3897096fd7 Merge "Implement 'protected' filter acknowledgement checkbox" 2024-06-05 13:42:33 +00:00
STran 69a28f7f03 Implement 'protected' filter acknowledgement checkbox
- Add a basic checkbox on the filter edit page that must be checked if a
  filter uses a protected variable to ensure that the user is aware that
  their filter will also become protected

Bug: T364485
Change-Id: I7c7652f7d1a81223229b839ff7eee5da4af74c8a
2024-06-05 05:43:25 -07:00
jenkins-bot 4e14afa6fb Merge "Allow variables to be restricted by user right" 2024-06-04 17:20:17 +00:00
STran bf28dbce0e Allow variables to be restricted by user right
Some exposed variables (eg. `user_ip`) used in filters are sensitive
and need to only be available to restricted groups of users.

Back-end changes:
- Add `AbuseFilterProtectedVariables` which defines what variables are
  protected by the new right `abusefilter-access-protected-vars`
- Add the concept of a `protected` variable, the use of which will
  denote the entire filter as protected via a flag on `af_hidden`

New UX features:
- Display changes to the protected status of filters on history and diff
  pages
- Check for protected variables and the right to see them in filter
  validation and don't allow a filter to be saved if it uses a variable
  that the user doesn't have access to
- Check for the right to view protected variables before allowing access
  and edits to existing filters that use them

Bug: T364465
Bug: T363906
Change-Id: I828bbb4015e87040f69a8e10c7888273c4f24dd3
2024-06-04 06:54:53 -07:00
jenkins-bot 10663ed4fd Merge "Convert af_hidden into a bitmask" 2024-05-30 18:11:24 +00:00
Umherirrender 69debb0a22 Use expression builder to build where conditions
Bug: T350968
Change-Id: I8a0fdf868efd403e02a68c5293a4e603d16657e6
2024-05-30 11:35:57 +02:00
Bartosz Dziewoński 94251ca97e Use StatusValue::getMessages() instead of deprecated methods
Added in MediaWiki in Ibc4ce11594cf36ce7b2495d2636ee080d3443b04.

Change-Id: I0b51f1210b9501961586fa25bf1f49bc68bab3d1
2024-05-28 21:04:59 +00:00
STran ca23e9f06b Convert af_hidden into a bitmask
Protected variables will cause the filter using them to become
protected as well. `af_hidden` can be used to track this flag,
as it is a TINYINT and can be converted into a bitmask with no
schema changes.

This is not a backwards-compatible change, as now all checks must
check the `hidden` flag specifically or otherwise will be cast to
true if any flag is set.

To support this change:
- "hidden" is considered a flag set in the `af_hidden`. This is a
  change in concept with no need for updates to the column values,
  as there is currently only one flag in the bitmask.
- `Flag`s store the bitmask as well as the state of single flags
  and can return either.
- Any checks against the `af_hidden` value no longer check a
  boolean value and instead now check the `hidden` flag value.

Bug: T363906
Change-Id: I358205cb1119cf1e4004892c37e36e0c0a864f37
2024-05-28 00:59:08 -07:00
jenkins-bot 58c5edce98 Merge "Add user_unnamed_ip variable" 2024-05-23 18:10:52 +00:00
STran fe0b1cb9e9 Add user_unnamed_ip variable
After temporary accounts are enabled, filters that rely on an ip
in the `user_name` will fail (eg. `ip_in_range` and `ip_in_ranges`).
To keep these filters working:

- Expose the IP through another variable, `user_unnamed_ip`, that can be
  used instead of `user_name`.
- The variable is scoped to only reveal the IPs of temporary accounts
  and un-logged in users.
- Wikis that don't have temporary accounts enabled will be able to see
  this variable but it won't provide information that `user_name`
  wasn't already providing
- Introduce the concept of transforming variable values before writing
  to the blob store and after retrieval, as IPs need to be deleted from
  the logs eventually and can't be stored as-is in the amend-only blob
  store

Bug: T357772
Change-Id: I8c11e06ccb9e78b9a991e033fe43f5dded8f7bb2
2024-05-23 07:19:48 -07:00
thiemowmde 32bee4950f Remove bogus non-breaking spaces
This issue exists ever since this code was added in 2009. Note how
this element is invisible anyway. The non-breaking space is never
seen. The purpose of this element is to act as a container for a
debug result that will be put into this container via JavaScript.
I confirmed this still works fine without the placeholder character
being there.

The problem here is that this HTML entity is double escaped because
of the element() function. That would need to be a rawElement() call
or we can just remove it.

Change-Id: Id560f392be4cc2106a7ac224309c8b605bec3f6c
2024-05-13 13:15:51 +02:00
Matěj Suchánek f9dcf46d70 Replace most Xml methods with Html
Xml::buildForm and Xml::fieldset are left.

Change-Id: Iff88869fd002165ec9ee80897d4deb585005b9d1
2024-05-08 13:08:52 +02:00
xtex bc6240fbda
Replace some deprecated functions
Change-Id: I4070a3655f2fac1d7afe1c3a244a64cb55019b9a
2024-05-04 21:32:04 +08:00
Umherirrender 6dccb17255 Migrate to IReadableDatabase::newSelectQueryBuilder
Also use expression builder to avoid raw sql

Bug: T312420
Change-Id: I83eb39f1c65a698108ae5bb72f633afda37a9f23
2024-04-30 20:45:51 +02:00
Umherirrender 3691d773d3 Migrate to IDatabase::newUpdateQueryBuilder
Change-Id: I0b3fd864e5227068114ca7aa9e98361046f393c1
2024-04-15 23:07:44 +02:00
Bartosz Dziewoński ac777ee88a Fix new Phan errors
MediaWiki core change Icb8822def9ce56f42ff52a8e469bb08d61d576c6
improved the type hints for OutputPage::addWikiMsg(), resulting in
two new errors:

* AbuseFilterViewEdit.php: False positive, update suppression
  to include new error code.

* SpecialAbuseLog.php: Genuine bug, the return value of
  Status::getErrors() can't be used directly as a message key.
  I have another change pending that introduces a nicer way
  to do this: Ibc4ce11594cf36ce7b2495d2636ee080d3443b04,
  but in the meantime, make do with the available getters.

Change-Id: Iee0e87496e27a5261adccb977361b3ccf4c9ee2c
2024-04-10 23:12:28 +00:00
libraryupgrader a8c9fab2cc build: Updating mediawiki/mediawiki-codesniffer to 43.0.0
The following sniffs are failing and were disabled:
* MediaWiki.Commenting.FunctionComment.MissingDocumentationPublic

Change-Id: I6075c76d53a899aac56af027f9a956a6b9e6a667
2024-03-16 18:53:05 +00:00
Umherirrender 5ab7282b4a Fix casing of dropdown-related methods
Methods gets renamed to lowercase variant in core (f1d7e68c)

Follow-Up: Ifda13ba9dee316709c424636ec3b285de8d0e9b1
Change-Id: I0ee5602536033268ff49aadf8d14320f8e5d03d2
2024-03-09 15:44:59 +01:00
James D. Forrester 1cfd2c8d3b build: Upgrade mediawiki/mediawiki-phan-config from 0.13.0 to 0.14.0 manually
The new version fixes a false-positive, success.

Change-Id: I69400879b4e79695be787b911fb3bd0ff923cf83
2024-02-08 18:05:12 -05:00
gerritbot 71c181219a Remove indirect calls to IDBAccessObject::READ_* constants
We are getting rid of the schema of implementing this interface and
calling self::READ_* constants, it's confusing, inconsistent, prone to
clashes and isn't really useful for non-ORM systems (which we are not)

Bug: T354194
Change-Id: I5d7a2c91a49311a6bdf6e56053c08610d4d6d110
2024-01-26 09:25:35 -05:00
Dreamy Jazz 1c0a5b9c6f Use ::getLocalUrl instead of ::getFullUrl for HideAbuseLog form
Why:
* The AbuseFilter Special:AbuseLog/hide page has a form that allows
  those with sufficent rights to hide abuse filter log entries.
* This form defines a custom action, which uses a URL including the
  wgServer by calling ::getFullUrl.
* When on WMF wikis and using mobile view, the domain name includes
  'm' and as such the wgServer is not the correct URL for the form
  action in this case.
* HTMLForm by default uses ::getLocalUrl for the action and as such
  Special:AbuseLog/hide should also use ::getLocalUrl to prevent
  these issues.

What:
* Change the call to ::getFullUrl in HideAbuseLog::show for the
  action text for the HTMLForm instance to instead be a call to
  ::getLocalUrl.

Bug: T355012
Change-Id: I6c909d5e6724dd620cf656c9a55439ed5d5c2fb4
2024-01-20 11:42:16 +00:00
Novem Linguae 88e9d8d0b6 Special:AbuseFilter page title should mention filter name
- Mentions filter number and name in the title
- Distinguishes between viewing and editing

Bug: T353106
Change-Id: Idda9854a78937033b168603810154b48288c3f4c
2023-12-22 04:55:37 -08:00
Umherirrender bd84a6514c Use namespaced classes
This requires 1.42 for some new names

Changes to the use statements done automatically via script
Addition of missing use statements and changes to docs done manually

Change-Id: Ic1e2c9a0c891382744e4792bba1effece48e53f3
2023-12-10 23:03:12 +01:00
thiemowmde 2def63118e Replace BadMethodCallException with LogicException
The BadMethodCallException is documented as "thrown if a callback
refers to an undefined method or if some arguments are missing".
This is not what happens in these places.

Change-Id: Ic95b67acc2e17eea1dd0fa1d72f9ac94a86bcf17
2023-11-16 19:10:50 +00:00
Bartosz Dziewoński 0f63d6011c Parse wikitext in page titles with filter number as parameter
Follow-up to 0ff282dcc3.

Bug: T343994
Change-Id: I14cc0cebcf10fb552475ef6ffbab9dcfa55221cd
2023-10-05 20:54:58 +00:00
Bartosz Dziewoński 1e54192b1b Fix non-escaped messages used in page subtitles
Reported by Phan, and also discoverable with ?uselang=x-xss.

Change-Id: Ieb41d11acf543784f1cfbca5ea2272ac0bffc9a9
2023-10-05 22:51:21 +02:00
jenkins-bot df88b1b4b7 Merge "Revert "Remove unused SecurityCheck-ReDoS suppression"" 2023-09-25 20:19:36 +00:00
Daimona Eaytoy 7896e8f2c4 Revert "Remove unused SecurityCheck-ReDoS suppression"
This reverts commit 2107ee4f38.

Reason for revert: T347220

Change-Id: If76c4d0d7564d48bf231173d1d8f5177127fc5fa
2023-09-25 16:07:21 +00:00
C. Scott Ananian 0ff282dcc3 Use OutputPage::setPageTitleMsg() instead of ::setPageTitle()
The new method formats the message with Message::escaped() which
better protects from bad HTML in the message.

The ::setPageTitleMsg() method was added in 1.41 and this extension
already requires MW >= 1.41.

Bug: T343994
Change-Id: Ic07cde3bafeaa0325024fe89b4948680d04c4820
2023-09-22 21:18:03 +00:00
C. Scott Ananian 2107ee4f38 Remove unused SecurityCheck-ReDoS suppression
Change-Id: I7809c29e1150ea6770f3aa70a3fb790f3e4ce32f
2023-09-22 21:17:31 +00:00