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
Changes to the use statements done automatically via script
Addition of missing use statement done manually
Change-Id: If80031678a474157e4cc78a3d3621dab53aded67
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
- 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
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
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
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
- 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
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
Changes to the use statements done automatically via script
Addition of missing use statement done manually
Change-Id: I48fcc02c61d423c9c5111ae545634fdc5c5cc710
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
This removes the last usages of the problematic open/closeElement
from this codebase.
One actual issue gets fixed: Some of the <th> floated around without
a <tr>. That's technically invalid. Luckily the browsers are flexible
and show it correctly. Visually nothing changes.
Similarly <th> should be wrapped in a <thead>. This wasn't done
before.
Change-Id: Ia45096670888173e49f9c25e72f429f0961b75ae
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
The following sniffs are failing and were disabled:
* MediaWiki.Commenting.FunctionComment.MissingDocumentationPublic
Change-Id: I6075c76d53a899aac56af027f9a956a6b9e6a667
This field gets added automatically when using the special page form but
is only shown to admins and other people who have access. It's not private
information (users can find it in history) but this is to avoid making
these admins an easy target for harassment (Talking to PM of moderation
team he agreed this is a good compromise).
Bug: T341626
Change-Id: I8410f39db54b96981b05de8e064fed65df30ef2f
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
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
For example:
* Use the more meaningful str_contains().
* Add missing type hints.
* Make use of early returns/guard clauses.
Change-Id: Id150d1b17a80ea637a0639a8f2fd7fd017ad23b1
Protected effectively means "public to subclasses" and should be
avoided for the same reasons as marking everything as public should
be avoided.
Change-Id: Iba674b486ce53fd1f94f70163d47824e969abb77
* Remove stray `@ingroup` from file blocks, move to class block.
* Fix mention of "WAN" cache where actually APCU is used.
* Document that the storage class takes a local-server cache.
This is an important requirement since the class has no
coordination for purging or other invalidation. It expects
an uncoordinated cache.
* Rename "load" to "loadConfig" as it's ambigious what it means among
the half dozen other "load*" methods in this class. Also inline
loadFromConfig and loadComputedUncached while at it to further
reduce this.
* Rename "loadConfigContent" to "fetchLatestConfig" to match
the existing fetchConfig, which does the same thing except it queries
the primary db using READ_LATEST.
* Use Html.php when building HTML, instead of legacy Xml.php.
While at it, also switch a few to Html::element instead of
Html::rawElement (aka Xml::tags) by using Message->text() for
messages that are not expected to contain rich wikitext.
Change-Id: Ic74d1597aa9201b371894e7a4bf9361752d9db21
This is a direct follow up for I6373fa6 where we apparently fixed
half of the cases while breaking the other half. There was actualy
a code path that can return null, and anther one that can return a
status object.
Since there is never anything done with the status object we can as
well get rid of it and always return null in case of an error.
Bug: T337431
Bug: T279275
Change-Id: I2ccb58756182897bcd6649c9f589e2f7a0321b20
