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
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
Small performance benefit by just one db call instead of multiple
Most test cases only use one filter, but some 2 to 4
Change-Id: I498c447e3873d2138e21541467115c9a67bb909e
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
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
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
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
Why:
- These checks are no longer needed, now that Idc47bda has been merged
What:
- Remove the `method_exists()` checks
Change-Id: I6e428df6b6e036146ae4cc57374cde8810d3f5f7
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
Alternative approach to fixing the regression proposed by
Daimona in I78d3a2cd7bada962d7ef9b0f2c39d898bf8987ce.
Bug: T368203
Change-Id: I637367c3b3850f7988d890379fef7f4753159953
- 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
`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
This test globally set `setForceShowCaptcha` to true, which caused
problems for following tests.
Bug: T368705
Change-Id: I5077e4b874c1bf1c6b68895349af0c9ecd4094ed
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
Changes to the use statements done automatically via script
Addition of missing use statement done manually
Change-Id: I48fcc02c61d423c9c5111ae545634fdc5c5cc710
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
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
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
- 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
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
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
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
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
