Commit graph

1860 commits

Author SHA1 Message Date
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
Ed Sanders 48b5da806d Add missing typehints
Change-Id: I3003d40e641b1ebfff8fd986a58cbc2c4f8f18d6
2024-09-23 14:25:50 +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
Matěj Suchánek 3ec6902f41 Make PreferencesHandler implement the hook interface
Follow-up to Ic7024d9c5f369eb33c4198a59638de9a1d58b04b.

Change-Id: I143dc53806eda8424440803833ef2a25b2893d1d
2024-09-13 18:45:30 +02:00
jenkins-bot 2ef2257922 Merge "Log changes to protected variables access" 2024-09-13 12:49:19 +00:00
jenkins-bot 4f272aeb02 Merge "Add preference for viewing protected variables in AbuseFilter" 2024-09-13 12:27:36 +00:00
STran cbfaaa591d Log changes to protected variables access
Similar to how CheckUser logs access to IP information about temporary
accounts, AbuseFilter needs to log whenever protected variables are
accessed.

- Implement ProtectedVarsAccessLogger which handles access logging
- Log whenever a user changes their ability to access protected
  variables via Special:Preferences

Bug: T371798
Change-Id: Ic7024d9c5f369eb33c4198a59638de9a1d58b04b
2024-09-13 01:39:09 -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
Anne Haunime 69ea21dc99 Support named capturing groups in get_matches()
AF rules don't support associative arrays, so the named capturing groups are provided in the array only by their numeric keys.

Bug: T374294
Change-Id: I53b39917e6677f3a5b8f68bcf0faebf48668ea27
2024-09-07 11:25:48 +00:00
Erik Bernhardson 65c10f5fa0 Skip auth checks when autocreate is allowed by provider
Session providers can provide a `canAlwaysAutocreate` flag which
indicates account creation is exempt from autocreate permission
checks. This is used, for example, for providers that provide
users for supporting applications in a wiki farm.

Check the flag and exempt the auto creation from abuse filter
checks as well.

Bug: T373778
Change-Id: Id89358930b92cb8dd05c2b031e764412ee641269
2024-09-05 11:17:16 -07:00
jenkins-bot db67b31db2 Merge "Log entry IDs should not have thousands separators" 2024-09-05 08:00:34 +00:00
Daimona Eaytoy dcc271b636 Api: Avoid type error in AbuseLogPrivateDetails
Make the `reason` parameter default to the empty string, so that we
don't end up passing null to ManualLogEntry::setComment.

Bug: T373010
Change-Id: Ifca828401628368bdddae14df2bbeb7391b2c02d
2024-08-21 14:31:46 +02:00
Bartosz Dziewoński 237d54d545 Replace gettype() with get_debug_type() in exception messages etc.
get_debug_type() does the same thing but better (spelling type names
in the same way as in type declarations, and including names of
object classes and resource types). It was added in PHP 8, but the
symfony/polyfill-php80 package provides it while we still support 7.4.

Also remove uses of get_class() where the new method already provides
the same information.

For reference:
https://www.php.net/manual/en/function.get-debug-type.php
https://www.php.net/manual/en/function.gettype.php

Change-Id: I5e65a0759df7fa0c10bfa26ebc3cda436630f456
2024-08-12 23:05:16 +02:00
jenkins-bot 963a1fc114 Merge "Use ConnectionProvider instead of LoadBalancerFactory" 2024-08-11 16:25:06 +00:00
thiemowmde 0afb81f8da Use ConnectionProvider instead of LoadBalancerFactory
This requires 1.42. That works as this codebase already requires
1.43 via extension.json.

Change-Id: If1f194a3cea3b8d45d98183e294d65fe8568f7ab
2024-08-11 17:27:28 +02:00
thiemowmde 861b1bf05b Fix broken PHPDoc comment
Also bring @var comments in a canonical form.

Change-Id: I9916bde1d3aa5fb44753109112bb898811cbf0ac
2024-08-11 17:23:37 +02:00
Anne Haunime 335dbff81e Log entry IDs should not have thousands separators
Per discussion, as a compromise (that I’m fine with), I’m leaving these IDs localized (i.e. the digits may be other than arabic), and I’m only removing the thousands separators.

Bug: T348717
Change-Id: I77b484fec2071267c53a139104c23755a13f0129
2024-08-09 05:58:23 +00:00
jenkins-bot de16ec7509 Merge "ConsequencesExecutor: Use Message objects in the Status" 2024-07-30 00:21:19 +00:00
jenkins-bot 0c51fbd3e6 Merge "Use namespaced MessageSpecifier" 2024-07-28 21:30:01 +00:00
Bartosz Dziewoński 80f56e599b ConsequencesExecutor: Use Message objects in the Status
In my recent change c458651370, which used Status::getMessages()
in FilteredActionsHandler, I overlooked the fact that it returns
MessageSpecifier objects instead of Message objects, and the return
value of MessageSpecifier::getParams() is not exactly specified
(the docs only promise that it's an array).

Now I'm working on a MediaWiki core change (I625a48a6ec) that
causes a different MessageSpecifier to be used, which stores
parameters in a different format, and would break that code.

To avoid problems, ConsequencesExecutor now stores Message objects
in the Status, which guarantees that FilteredActionsHandler will
get the same objects back.

Change-Id: I2c1bc8dde9a078d03badecf6d89443b65eeb92c5
2024-07-28 20:08:56 +00:00
Bartosz Dziewoński 517beb3c0d Use namespaced MessageSpecifier
Depends-On: I9ff4ff7beb098b60c92f564591937c7d789c6684
Change-Id: I7097b4d80df790ef14a5bc053306dc2f1fd195da
2024-07-28 21:59:35 +02:00
Umherirrender e88494212e Use expression builder to avoid IDatabase::makeList
Bug: T350968
Change-Id: Iacb407a9aef293f401e0dbf754bb1f51f6b390c5
2024-07-22 21:42:28 +00:00
Umherirrender 91b369b7af Use expression builder instead of raw sql
Bug: T350968
Change-Id: Ibad11ea11e7955172d35d4499372d0fcd726bf74
2024-07-21 22:07:58 +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
jenkins-bot 98eab47d9b Merge "Simplify FilterEvaluator::getUsedVars using ::checkSyntax" 2024-07-08 12:42:18 +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
jenkins-bot 69508bf153 Merge "Add missing permission check to canSeeLogDetailsForFilter" 2024-07-05 10:09:47 +00:00
Matěj Suchánek bf180e0490 Simplify FilterEvaluator::getUsedVars using ::checkSyntax
Alternative approach to fixing the regression proposed by
Daimona in I78d3a2cd7bada962d7ef9b0f2c39d898bf8987ce.

Bug: T368203
Change-Id: I637367c3b3850f7988d890379fef7f4753159953
2024-07-05 11:32:09 +02: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
Daimona Eaytoy 6ac574dada Add missing permission check to canSeeLogDetailsForFilter
`canSeeLogDetails` should also be checked when a filter is protected, as
it is the base right for being able to see abuselog entries. With this
in mind, check that immediately at the beginning of the method, instead
of repeating calls. Also merge the conditionals, and return early when a
permission check fails.

Move a test up so that it comes immediately after its data provider, and
add test cases for a few combinations of rights.

Change-Id: Ic3cf58f43803bef8bf2d65566434baff145b3fd5
2024-07-02 22:43:09 +02:00
jenkins-bot bc0ab54e2d Merge "Support more log actions in testing interface" 2024-06-29 21:43:59 +00:00
jenkins-bot 7cca085e24 Merge "Remove modification of wgCheckUserLogAdditionalRights" 2024-06-28 17:05:35 +00:00
Jakob Warkotsch 0bd53dfeb1 Remove unused phan suppressions
Change-Id: Ib42f9b00948506ae2c053f51cb7dfddb0f7eb480
2024-06-28 12:15:49 +02:00
Dreamy Jazz 7254e65665 Remove modification of wgCheckUserLogAdditionalRights
Why:
* The wgCheckUserLogAdditionalRights global is shortly being
  removed as no-longer necessary after T324907 removed the need to
  generate the action text on insert time. As such, the action
  text can be generated on view and therefore respect the rights
  that the viewing user has.
* This means that the config can be removed, as users with the
  'abusefilter-view' right will be able to see the action text
  associated with the entry because the log formatter will check
  for the right on view.

What:
* Remove the modification of wgCheckUserLogAdditionalRights in
  AbuseLogger::insertLocalLogEntries.

Bug: T346022
Change-Id: I7504e4729fde5e51f6a795fc3e60d735152b2eea
2024-06-27 16:43:25 +00: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 788571fb3a Merge "LoadExtensionSchemaUpdates: Remove unused path from 'runMaintenance' action" 2024-06-20 16:29:16 +00:00
jenkins-bot 8dd15bac76 Merge "Fix variable descriptions showing raw "($1)"" 2024-06-20 15:46:12 +00:00
jenkins-bot 72ab79dcd1 Merge "Remove AbuseFilterActorMigration" 2024-06-20 15:44:49 +00:00
Bartosz Dziewoński 29e8b6e13b LoadExtensionSchemaUpdates: Remove unused path from 'runMaintenance' action
MediaWiki has never used this path for running the maintenance
scripts, only the class name provided in the other parameter.
Providing the parameter is no longer needed in MediaWiki 1.43.

Bug: T367918
Change-Id: Ie098fa124039cc1122135cae72c74579d43dc04f
2024-06-19 19:58:17 +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 bb026443dd Merge "Drop af_user(_text) and afh_user(_text) fields" 2024-06-17 12:25:54 +00:00
Matěj Suchánek 1373bf8d11 Fix variable descriptions showing raw "($1)"
In 1904cf8, "($1)" were appended to the messages, but
the argument was not substituted in the var dump table,
showing literal ($1). Substitute <code>var_name</code>
to keep previous experience.

However, many translations have not been updated yet.
If the variable name was indicated by the message
argument, it would often be missing. Therefore, make
sure the placeholder is always present.

Bug: T360909
Change-Id: I1e4a97210c891c375b0f14c0891c2d25a0a389d1
2024-06-16 22:11:08 +02: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
jenkins-bot 92e1335cc8 Merge "Use ObjectCacheFactory" 2024-06-13 13:45:45 +00:00