Problem
Following T292842: Log when the user's access level changes, logs are sometimes generated when a user's access level hasn't changed.
Steps to reproduce:
- Log in as a user who has rights to view IPInfo data and IPInfo logs, and who has enabled IPInfo, including accepting the user agreement
- Open Special:Log/ipinfo
- Open Special:Contributions/<some ip> and view the IP information (expand the infobox if necessary)
- Refresh Special:Log
Expected: No new log lines saying that the user enabled/disabled access (though maybe 1 log line saying that they viewed the data)
Actual: Several new log lines saying something like: Admin disabled their own access to IPInfo
This is only one example of how to reproduce; there may be others.
Cause
In PreferencesHandler::onSaveUserOptions we check whether to log:
$isEnabled = isset( $originalOptions[ 'ipinfo-enable' ] ) && isset( $originalOptions[ 'ipinfo-use-agreement' ] ) && $originalOptions[ 'ipinfo-enable' ] && $originalOptions[ 'ipinfo-use-agreement' ]; $willEnable = isset( $modifiedOptions[ 'ipinfo-enable' ] ) && isset( $modifiedOptions[ 'ipinfo-use-agreement' ] ) && $modifiedOptions[ 'ipinfo-enable' ] && $modifiedOptions[ 'ipinfo-use-agreement' ]; if ( $isEnabled !== $willEnable ) { $logger = $this->loggerFactory->getLogger(); if ( $willEnable ) { $logger->logAccessEnabled( $user ); } else { $logger->logAccessDisabled( $user ); } }
If either of $modifiedOptions[ 'ipinfo-enable' ] or $modifiedOptions[ 'ipinfo-use-agreement' ] are not set, the user is not disabling the feature, but $willEnable will be false, so $isEnabled !== $willEnable will pass and a log will be made.
Solution
We need to check explicitly for the user disabling IPInfo, rather than assuming that not enabling implies disabling. We already do something like this further up the function: https://gerrit.wikimedia.org/g/mediawiki/extensions/IPInfo/+/d13f6e60beaca0656e522a5a73a69143d954ba47/src/HookHandler/PreferencesHandler.php#125