Commit graph

638 commits

Author SHA1 Message Date
jenkins-bot bb026443dd Merge "Drop af_user(_text) and afh_user(_text) fields" 2024-06-17 12:25:54 +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
STran f5d7b68908 Force full evaluation of filter in FilterEvaluator->getUsedVars()
In some cases, evaluation short-circuits when getting a list of
used variables resulting in an incomplete array of variables. This
subsequently causes issues when using those arrays for validation
checks (eg. if protected variables are used).

- Force full evaluation by setting `mAllowShort` to false

Bug: T364485
Change-Id: Idf2112d9ebf63846cde3ce9b8a8ade0ed909505d
2024-06-11 02:43:47 -07:00
Matěj Suchánek 469d643530 Drop af_user(_text) and afh_user(_text) fields
They have been migrated to their respective _actor counterparts.

Bug: T188180
Change-Id: I76ab3a4eeaf93bf009ba3a5d4a0315443b6839ef
2024-06-10 18:48:21 +02:00
jenkins-bot a5afeff49c Merge "Drop $wgAbuseFilterActorTableSchemaMigrationStage" 2024-06-10 13:27:07 +00:00
Matěj Suchánek c2da0d4857 Drop $wgAbuseFilterActorTableSchemaMigrationStage
First patch in a series of dropping the old columns.
Wikis now need to run the maintenance script (e.g., via
update.php) prior to serving this commit.

Wikimedia wikis are already on SCHEMA_COMPAT_NEW stage.

Bug: T188180
Change-Id: I86ec2b816eed17b62bf02bfd085570f132011b3e
2024-06-10 12:08:30 +00: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
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
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
Kosta Harlan cd24c0ba2a
RCVariableGeneratorTest: Handle anonymous user test with temp accounts enabled
Why:

- Temp accounts will be enabled by default in CI, eventually. For tests
  that verify anonymous user editing behavior, we need to disable the
  temp user feature

What:

- Check if the user identity isn't registered, and if so, disable auto
  creating a temp user

Bug: T365645
Change-Id: I477fb6b44655e4190b5906c85390133e3e3a9feb
2024-05-23 09:41:05 +02:00
jenkins-bot ecf3268789 Merge "Provide integration with ConfirmEdit to show CAPTCHA" 2024-05-13 10:25:44 +00:00
Kosta Harlan f948c79066
Provide integration with ConfirmEdit to show CAPTCHA
Why:

- We want AbuseFilter to able to require a CAPTCHA if an action
  matches conditions in an AbuseFilter

What:

- Implement the ConfirmEditTriggersCaptcha hook, and check to see if
  the CaptchaConsequence set a global flag that indicates if we
  should show a CAPTCHA

Depends-On: Ie87e3d850541c7dc44aaeb6b30489a32a0c8cc60
Bug: T20110
Change-Id: I110a5f5321649dcf85993a0c209ab70b9886057c
2024-05-10 21:00:47 +02:00
xtex bc6240fbda
Replace some deprecated functions
Change-Id: I4070a3655f2fac1d7afe1c3a244a64cb55019b9a
2024-05-04 21:32:04 +08:00
jenkins-bot eb1db27c77 Merge "Clean up ActionVariablesIntegrationTest" 2024-05-03 07:16:12 +00:00
Umherirrender 06ccd0c0b8 tests: Use IDatabase for mocking instead of DBConnRef
DBConnRef is internal, use of IDatabase interface is more common

Change-Id: Ib14496dd4e5c02bb80a1e7f43e9489d5c22bda39
2024-05-02 22:32:05 +02:00
Matěj Suchánek 7f3ded3004 Clean up ActionVariablesIntegrationTest
Change-Id: Ia9ad89b699dac351e6b14a3d33dc0ceea7ed74b3
2024-05-01 16:22:15 +02: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
jenkins-bot 0038f18b9b Merge "Migrate to IDatabase::newUpdateQueryBuilder" 2024-04-20 20:40:53 +00:00
Umherirrender 6c870529e4 tests: Return FakeResultWrapper from mocked IReadableDatabase::select
To match the return type documentation of IReadableDatabase::select

Change-Id: I6a03c9468aa23f830f550e83eebf734ba0167c23
2024-04-19 21:20:07 +02:00
Timo Tijhof 4743f9d267 tests: Widen @covers tags in phpunit tests
Follows-up I5a5420df13893386.

> We lose useful coverage and waste valuable time on keeping tags
> accurate through refactors (or worse, forget to do so).
>
> Tracking tiny per-method details wastes time in realizing (and
> fixing) when people inevitably don't keep them in sync, and time
> lost in finding uncovered code to write tests to realize it was
> already covered but "not yet claimed".
>
> Given all used methods are de-facto and liberally claimed, and
> that we keep the coverage limited to the subject class, this
> maintains the spirit and intent. PHPUnit offers a more precise
> tool when you need it (i.e. when testing legacy monster classes),
> but for well-written code, the class-wide tag suffices.

Ref https://gerrit.wikimedia.org/r/q/owner:Krinkle+is:merged+message:Widen

Change-Id: If7304d8b5b43ab8a051fbcecced331a787bab960
2024-04-17 01:44:40 +01:00
Umherirrender 3691d773d3 Migrate to IDatabase::newUpdateQueryBuilder
Change-Id: I0b3fd864e5227068114ca7aa9e98361046f393c1
2024-04-15 23:07:44 +02:00
jenkins-bot f17f231a70 Merge "tests: Widen @covers tags in SpecsTest" 2024-04-15 16:56:57 +00:00
jenkins-bot 1807a07755 Merge "Remove small pieces of unused code from SpecsFormatterTest" 2024-04-15 16:02:31 +00:00
Timo Tijhof 56c0f6313e tests: Widen @covers tags in SpecsTest
> We lose useful coverage and waste valuable time on keeping tags
> accurate through refactors (or worse, forget to do so).
>
> Tracking tiny per-method details wastes time in realizing (and
> fixing) when people inevitably don't keep them in sync, and time
> lost in finding uncovered code to write tests to realize it was
> already covered but "not yet claimed".
>
> Given all used methods are de-facto and liberally claimed, and
> that we keep the coverage limited to the subject class, this
> maintains the spirit and intent. PHPUnit offers a more precise
> tool when you need it (i.e. when testing legacy monster classes),
> but for well-written code, the class-wide tag suffices.

Ref https://gerrit.wikimedia.org/r/q/owner:Krinkle+is:merged+message:Widen

Change-Id: I5a5420df1389338644a099ebfd072063653e6849
2024-04-15 16:36:36 +01:00
jenkins-bot be54bef5d8 Merge "Clean up injection of DatabaseBlockStore" 2024-04-15 04:01:23 +00:00
jenkins-bot 5c61521cbb Merge "Migrate to IDatabase::newInsertQueryBuilder/newDeleteQueryBuilder" 2024-04-14 18:28:13 +00:00
jenkins-bot bd2e33e3e5 Merge "Replace array_merge in tests with the [ ... ] syntax" 2024-04-12 09:32:57 +00:00
Matěj Suchánek ad2600b6c0 Clean up injection of DatabaseBlockStore
The static method has already been migrated.

Also rewrite the test cases to avoid non-static provider (T337144).

Change-Id: Ibf98539f442e1ba8a9e9eb510784d40778123f17
2024-04-12 11:01:00 +02:00
thiemowmde 07a04b74d7 Remove small pieces of unused code from SpecsFormatterTest
Change-Id: Ifbef4f3c73b5744dd1ead9df1743631753bd306a
2024-04-12 08:37:00 +00:00
thiemowmde 0b0fab9d1b Fix wrong getMockMessage() calls in tests
MessageLocalizer::msg( $key, ...$params ) accepts a variable number
of arguments, but getMockMessage does not. Passing only the first
argument as a string to getMockMessage is not supported.

Required for I86ff2d6.

Change-Id: I30ab799ceab36b80c330d1233f3009814d7f6c98
2024-04-11 15:32:17 +00:00
thiemowmde 7f214ec15c Replace array_merge in tests with the [ ... ] syntax
It does the same and is well supported in the PHP versions we
currently use, at least when the arrays contain integer keys.

Change-Id: Id038142621dab47bfc03db48ce676ed0b2cdc28b
2024-04-11 14:01:25 +02:00
Matěj Suchánek 68ff668543 Add new variable for last edit time
Bug: T269769
Change-Id: Ia41ecc2f8e6921ef3d5a16fec58202d584ad0727
2024-04-10 23:12:45 +00:00
Umherirrender 2df93d2b0f Migrate to IDatabase::newInsertQueryBuilder/newDeleteQueryBuilder
InsertQueryBuilder does not ignore insert of no rows,
adding some conditions to avoid calling the query builder

Change-Id: I1752b90cc3a7ec3a7f9ee32a1873bf8c82b6e02e
2024-04-02 21:15:40 +02:00
Timo Tijhof 473b4b1ec4 tests: Remove redundant wgMainCacheType=hash
Introduced in 2019 with 4c8dac4dc6. Redundant since 2020 with
commit c6c62e2c8f in MediaWiki core.

Bug: T139216
Change-Id: I51e9fc3899cf5505917d7899a395350dd86f5c0b
2024-03-29 15:37:58 -07:00
Matěj Suchánek 4c2b09fa4e Replace deprecated ChangeTags method calls with ChangeTagStore
Bug: T360664
Change-Id: Icd7c38be9a8b54c29da7d33dfa9691e1c94feeb1
2024-03-29 11:08:41 +01:00
jenkins-bot 430b7f81ad Merge "FilterLookup: Stop using DBAccessObjectUtils::getDBOptions()" 2024-03-25 18:20:52 +00:00
Amir Sarabadani fe0fed1d8f FilterLookup: Stop using DBAccessObjectUtils::getDBOptions()
And more db clean ups:
 - Use QueryBuilders
 - Stop relying on actor migration to simplify query building
 - Using expression builder in one case.
 - Change the default actor migration stage to read new and write both.

Bug: T354194
Depends-On: I7c116cab0c748707d9a9fd17feeffe26e7d188ec
Depends-On: I74002911749335f4323a03fb430d02f936771b7e
Change-Id: Id84d1db7a2991f3cccc2f4f1502ba77643ddef24
2024-03-21 20:22:02 +01: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
Dreamy Jazz 85022190e5 Add user_type variable
Why:
* An AbuseFilter variable is needed that allows filters to determine
  what type the current user is. That is, whether the user is an
  IP address, temporary account, named user or external user.
* Currently filters implement this by inspecting the value in
  the 'user_name' variable, but this is likely to break when
  temporary accounts are enabled as IPs would be hidden.
* Giving a dedicated variable that indicates the type of the user
  allows filters to work out this information without having to
  know the specific username of the user before performing the
  check.

What:
* Add the 'user_type' variable which is lazily computed. It can have
  the value 'named', 'temp', 'ip' or 'external' depending on the
  type of the user. If the user does not match any of these, then
  the value is 'unknown'.
* Replace call to deprecated User::newFromIdentity with a use of the
  UserFactory service that is dependency injected.
* Add and update tests to ensure consistent test coverage.

Bug: T357615
Change-Id: Ifffa891879e7e49d2430a0330116b34c5a03049d
2024-03-07 10:38:25 +00:00
Derick Alangi 82d876ada8 Handlers: Drop AutoPromoteGroupsHandler::factory()
This patch solves a pending TODO which is to remove the ::factory()
method from the AutoPromoteGroupsHandler class. If the cache instance
is injected, we'll use it otherwise we'll default to a HashBagOStuff.

Bug: T358346
Change-Id: I2bc414da8733431d1d11025e954282fc7c73aa80
2024-03-06 21:38:12 +00:00
Dreamy Jazz f4e3f78b7e Add integration test to verify logs are saved in CheckUser
Why:
* In f3c87749b8, the sending of logs
  to CheckUser when using a temporary account was fixed. As part
  of that change, it was suggested to add a test to verify that
  the call to `Hooks::updateCheckUserData` actually causes an
  insert into the relevant CheckUser result table.
* This change adds this test as a follow-up.

What:
* Create a test for the AbuseLogger that verifies that an event is
  sent to CheckUser and is saved into the DB for an abuse filter
  hit. This test is only run if the CheckUser extension is installed.

Bug: T358632
Change-Id: I33ed0810db13e38eacf4e682eb54d4ffcd583084
2024-03-01 16:11:33 +00:00
Derick Alangi e1f78da351
Fix wrong namespacing of integration test using \Unit\
Change-Id: Idcafe1dd97951db41b78c2a1c54d8519ae8802b7
2024-02-23 15:55:47 +03:00
Dreamy Jazz a5b68cf46d Don't attempt to steal or create the FilterUser in CheckUserHandler
Why:
* When CheckUser asks the AbuseFilter extension for modifications
  to rows inserted into the CheckUser tables, the AbuseFilter
  extension attempts to get the Filter user via User::newSystemUser
* User::newSystemUser can deadlock if multiple requests to create
  the system user are being made at once.
* The CheckUserHander does not need to create the abuse filter system
  and instead only needs to know if a given $user is the equal to
  the FilterUser.
* As such the FilterUser service needs to provide a way to check if
  a given $user is equal without creating the FilterUser.

What:
* Add FilterUser::isUserSameAs which returns a boolean value
  indicating whether the Abuse Filter system user is the equal
  to a given UserIdentity in the same way that UserIdentity::equals
  is implemented.
* Refactor ::getUser to get the username for the filter user in
  a separate method, so that the ::isUserSameAs method can also
  use this method. Name this new method ::getFilterUserName.
* Add a test for the FilterUser service to ensure consistent test
  coverage
* Convert the @covers and @coversDefaultClass annotations to be
  a @covers for the class. This is because PHPUnit recommends this in
  https://docs.phpunit.de/en/9.6/annotations.html#appendixes-annotations-covers-tables-annotations

Bug: T356275
Bug: T346967
Change-Id: I8a101781bb47612deabb0f2a06a398ac13e860e6
2024-01-31 19:32:52 +00:00