Commit graph

140 commits

Author SHA1 Message Date
jenkins-bot ad516227f4 Merge "Protected variable logs: fallback to accountname if user_name is not set" 2024-10-21 20:18:15 +00:00
Kosta Harlan 05da3118aa
Protected variable logs: fallback to accountname if user_name is not set
Why:

- For account creations and account autocreations, the user_name
  property is deliberately unset, to avoid displaying the IP address of
  an unregistered user. Instead, `accountname` is set with the newly
  created account name
- For logging that someone has seen a protected variable value, we need
  to record the username that was seen

What:

- Use `accountname` as a fallback in case `user_name` is not set, when
  logging protected variable access
- Update tests to cover this case.

Bug: T376885
Change-Id: I688a3529fac0ad8455977a0cfdb950f0105f550d
2024-10-21 21:15:51 +02:00
Umherirrender 57ecef75c5 Use namespaced classes
Changes to the use statements done automatically via script
Addition of missing use statement done manually

Change-Id: If80031678a474157e4cc78a3d3621dab53aded67
2024-10-19 21:55:40 +02:00
jenkins-bot 743bb64924 Merge "Log specific views of protected variables" 2024-10-03 14:37:48 +00:00
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
Dreamy Jazz 48b26792a9 SECURITY: abusefiltercheckmatch: Check if user can see log details
CVE-2024-PENDING

Why:
* The 'abusefiltercheckmatch' API allows callers to match
  arbitary filter conditions against existing AbuseFilter logs
* The API does not check if the performer has the ability to
  see the log details for the given filter, so can allow a user
  to bypass hidden and protected visibility settings.

What:
* Call AbuseFilterPermissionManager::canSeeLogDetailsForFilter
  before attempting to match a filter against a given AbuseFilter
  log.
* Add a test to verify that this security fix works.

Bug: T372998
Change-Id: I4a2467dc4e0d1f8401d5428a89c7f6d6ebcdfa70
2024-10-01 00:18:55 +01:00
STran 51381f0067 Bugfix: Fix minor issues with protected vars logging
- Fix an issue where if a user didn't have view permissions they could
  get the preference check error (a preference they wouldn't have) on
  SpecialAbuseLog
- Fix an issue where the `change-access` hadn't been updated to the used
  disabled/enabled log types
- Fix an issue where a ProtectedVarsAccessLoggerTest test wasn't
  correctly using the data provider data
- Improve naming since ProtectedVarsAccessLogger exists in its own test
  file instead of being a subset of tests on AbuseLoggerTest

Bug: T371798
Change-Id: I53f22855e63d9e1339361a5c9ee7886e0f74714a
2024-09-23 03:42:41 -07:00
STran 0b3d0b3b6d Write protected variables access logs to CheckUser if installed
Write logs related to temporary accounts to CheckUser if the extension
is available so that logs are topically centralized.

Bug: T373525
Depends-On: I35d50df7cd6754e29d964cc716fb3c42406272df
Change-Id: Ic95f211f4db7ce6dc2d769d2f3af206f4a3935e4
2024-09-18 07:59:05 -07:00
jenkins-bot 4f272aeb02 Merge "Add preference for viewing protected variables in AbuseFilter" 2024-09-13 12:27:36 +00: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
Dreamy Jazz 7ecc204050 Expand SearchFilters.php to search by consequence
Why:
* SearchFilters.php allows the caller to search by a regex that
  is applied to the pattern.
* This script can be expanded to allow callers to specify what
  consequence should be associated with the filters that are
  outputted.

What:
* Add a 'consequence' option to the SearchFilters.php maintenance
  script, which is applied through a LIKE query on the
  af_actions column.
** This can be specified with or without the pattern option.
** Instead of making pattern required, the script now requires
   that one of consequence or pattern is provided.
* Expand the tests for the script for this new code, along with
  using the new ::expectCallToFatalError method to be able to
  test previously untestable code.

Bug: T373148
Change-Id: I1b507d8f9dc1f4cf91ee4f83ccde745eb6d46d6d
2024-08-22 22:07:37 +01:00
jenkins-bot cafb0c49e9 Merge "Remove usage of writeapi userright" 2024-08-01 21:06:48 +00:00
Fomafix 805a1e8248 Use overrideConfigValues/overrideConfigValue instead of setMwGlobals
Also use MainConfigNames.

Change-Id: I35c7099a63f8665ddf433116732a93ede8711e35
2024-07-30 15:04:40 +00:00
Bartosz Dziewoński 1c0ab3010a AbuseFilterExtensionJsonTest: Allow skipping other extension hooks
Change-Id: I1146cec2b27c964f5ed07e7da76fc7b9ec4a09c5
2024-07-28 20:08:30 +00:00
Umherirrender 6db3b3287f tests: Use LanguageFactory to create en language
Bug: T343771
Change-Id: Id2423c87c17a2f357d5e1cfeef3aeb83b6ad9a0d
2024-07-20 21:41:52 +02:00
jenkins-bot 5a18e60b76 Merge "Disallow protected variable access on AbuseFilterViewTestBatch" 2024-07-10 18:48:07 +00: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
Kosta Harlan b58d91bcac ConfirmEditHandlerTest: Loosen message check test
Why:

- Ie13181b78b8e2903c6cc0f0f778689bcc8b8ce2e modifies the status message
  returned

What:

- Loosen the check for the status error message such that 'captcha-edit'
  and 'captcha-edit-fail' are both valid; we can revert this after
  Ie13181b78b8e2903c6cc0f0f778689bcc8b8ce2e is merged

Change-Id: I5a0698d84932a474800a68dba9b76b3433b19290
2024-07-10 08:20:18 +00:00
jenkins-bot c316be857b Merge "ConfirmEditHandlerTest: Remove method_exists checks" 2024-07-09 11:32:25 +00:00
jenkins-bot 218627233b Merge "Only return filters visible to user in search" 2024-07-09 09:45:55 +00:00
Kosta Harlan 62629ec3e9
ConfirmEditHandlerTest: Remove method_exists checks
Why:

- These checks are no longer needed, now that Idc47bda has been merged

What:

- Remove the `method_exists()` checks

Change-Id: I6e428df6b6e036146ae4cc57374cde8810d3f5f7
2024-07-09 10:27:03 +02: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
Wandji69 6a091dcb39 Tests: Repalce "db" with getDb() method
Bug: T316841
Change-Id: I40eb000f008e51aba581ee8e33a8421ff111fbf1
2024-06-28 16:32:16 +00:00
Jakob Warkotsch 9fc29beb09 Reset setForceShowCaptcha to false after test
This test globally set `setForceShowCaptcha` to true, which caused
problems for following tests.

Bug: T368705
Change-Id: I5077e4b874c1bf1c6b68895349af0c9ecd4094ed
2024-06-28 12:15:49 +02:00
Kosta Harlan b93543ef00 ConfirmEditHandler: Use SimpleCaptcha API to invoke CAPTCHA display
Why:

- The previous attempt to integrate AbuseFilter with ConfirmEdit set
  a flag on the request object
  (I110a5f5321649dcf85993a0c209ab70b9886057c) didn't work in WMF
  production because in WMF, we load ConfirmEdit first, followed by
  AbuseFilter. Therefore any flag set in an AbuseFilter hook is ignored
  by ConfirmEdit

What:

- Remove implementation of ConfirmEditTriggersCaptchaHook, as this does
  not work when AbuseFilter is loaded after ConfirmEdit.
- Repurpose onConfirmEditTriggersCaptcha to handle non-edit actions only
- Implement the EditFilterMergedContent hook and call SimpleCaptcha's
  public confirmEditMerged method if CaptchaConsequence has specified
  that a CAPTCHA should be displayed, and if the CAPTCHA has not already
  been solved

Soft-Depends-On: Idc47bdae8007da938f31e1c0f33e9be4813f41d7
Bug: T20110
Change-Id: I7dd3a7c41606dcf5123518c2d3d0f4355f5edfd3
2024-06-26 16:07:40 +00:00
jenkins-bot 72ab79dcd1 Merge "Remove AbuseFilterActorMigration" 2024-06-20 15:44:49 +00:00
jenkins-bot bb026443dd Merge "Drop af_user(_text) and afh_user(_text) fields" 2024-06-17 12:25:54 +00:00
Matěj Suchánek cb08d684d5 Remove AbuseFilterActorMigration
Bug: T188180
Change-Id: Idcacc9f63075b621bbc858a461dc6fb7ab7a9a39
Depends-On: I7dd5fc0f9d80636b0cdf3d995fe22c1f43a5b68d
Depends-On: Ibdb2b4096f26fc6752456a05f8d70a9a6d9609ad
2024-06-15 09:42:27 +02: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
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
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
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
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
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
jenkins-bot 0038f18b9b Merge "Migrate to IDatabase::newUpdateQueryBuilder" 2024-04-20 20:40:53 +00: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 5c61521cbb Merge "Migrate to IDatabase::newInsertQueryBuilder/newDeleteQueryBuilder" 2024-04-14 18:28:13 +00: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
jenkins-bot 430b7f81ad Merge "FilterLookup: Stop using DBAccessObjectUtils::getDBOptions()" 2024-03-25 18:20:52 +00:00