Remove support for per-group preference defaults

Not used and introduces serious compexity, likely causing
the bug with users receiving notifications they've opted out of.

Bug: T174220
Change-Id: I888c6009fffad17121765678387022ed7d454cb0
This commit is contained in:
Max Semenik 2017-09-22 13:08:39 -07:00
parent 4f467b283a
commit e13be59e42
3 changed files with 130 additions and 120 deletions

View file

@ -46,12 +46,6 @@
"AddNewAccount": [
"LoginNotify\\Hooks::onAddNewAccount"
],
"UserLoadOptions": [
"LoginNotify\\Hooks::onUserLoadOptions"
],
"UserSaveOptions": [
"LoginNotify\\Hooks::onUserSaveOptions"
],
"LocalUserCreated": [
"LoginNotify\\Hooks::onLocalUserCreated"
]
@ -72,8 +66,6 @@
"LoginNotifyCheckKnownIPs": true,
"@docLoginNotifyEnableOnSuccess": "Whether to trigger a notification after successful logins from unknown IPs.",
"LoginNotifyEnableOnSuccess": true,
"@docLoginNotifyEnableForPriv": "Set different default notification preferences for different user groups. For user groups that have any of the user rights listed in this array, the preferences specified in Hooks:getOverriddenOptions() are on by default.",
"LoginNotifyEnableForPriv": [ "editinterface", "userrights" ],
"@docLoginNotifySecretKey": "Override this to use a different secret than $wgSecretKey",
"LoginNotifySecretKey": null,
"@docLoginNotifyCookieExpire": "Expiry in seconds. Default is 180 days",

View file

@ -15,10 +15,6 @@ use MediaWiki\Auth\AuthenticationResponse;
use User;
class Hooks {
const OPTIONS_FAKE_TRUTH = 2;
const OPTIONS_FAKE_FALSE = 'fake-false';
/**
* Add LoginNotify events to Echo
*
@ -199,112 +195,4 @@ class Hooks {
$loginNotify->setCurrentAddressAsKnown( $user );
}
}
/**
* Hook for loading options.
*
* This is a bit hacky. Used to be able to set a different
* default for admins than other users
*
* @param User $user The user in question.
* @param mixed[] &$options The options.
* @return bool
*/
public static function onUserLoadOptions( User $user, array &$options ) {
global $wgLoginNotifyEnableForPriv;
if ( !is_array( $wgLoginNotifyEnableForPriv ) ) {
return true;
}
if ( !self::isUserOptionOverridden( $user ) ) {
return true;
}
$defaultOpts = User::getDefaultOptions();
$optionsToCheck = self::getOverriddenOptions();
foreach ( $optionsToCheck as $opt ) {
if ( $options[$opt] === self::OPTIONS_FAKE_FALSE ) {
$options[$opt] = '0';
}
if ( $defaultOpts[$opt] !== false ) {
continue;
}
if ( $options[$opt] === false ) {
$options[$opt] = self::OPTIONS_FAKE_TRUTH;
}
}
return true;
}
/**
* Hook for saving options.
*
* This is a bit hacky. Used to be able to set a different
* default for admins than other users. Since admins are higher value
* targets, it may make sense to have notices enabled by default for
* them, but disabled for normal users.
*
* @todo This is a bit icky. Need to decide if we really want to do this.
* @todo If someone explicitly enables, gets admin rights, gets de-admined,
* this will then disable the preference, which is definitely non-ideal.
* @param User $user The user that is being saved.
* @param mixed[] &$options The options.
* @return bool
*/
public static function onUserSaveOptions( User $user, array &$options ) {
$optionsToCheck = self::getOverriddenOptions();
$defaultOpts = User::getDefaultOptions();
if ( !self::isUserOptionOverridden( $user ) ) {
return true;
}
foreach ( $optionsToCheck as $opt ) {
if ( $defaultOpts[$opt] !== false ) {
continue;
}
if ( $options[$opt] === self::OPTIONS_FAKE_TRUTH ) {
$options[$opt] = false;
}
if ( $options[$opt] !== self::OPTIONS_FAKE_TRUTH
&& $options[$opt]
) {
// Its checked on the form. Keep at default
}
if ( !$options[$opt] ) {
// Somehow this means it got unchecked on form
$options[$opt] = self::OPTIONS_FAKE_FALSE;
}
}
return true;
}
/**
* Helper for onUser(Load|Save)Options
*
* @return array Which option keys to check
*/
private static function getOverriddenOptions() {
// For login-success, it makes most sense to email
// people about it, but auto-subscribing people to email
// is a bit icky as nobody likes to be spammed.
return [
'echo-subscriptions-web-login-fail',
'echo-subscriptions-web-login-success'
];
}
private static function isUserOptionOverridden( User $user ) {
global $wgLoginNotifyEnableForPriv;
// Note: isAllowedAny calls into session for per-session restrictions,
// which we do not want to take into account, and more importantly
// causes an infinite loop.
$rights = User::getGroupPermissions( $user->getEffectiveGroups() );
if ( !array_intersect( $rights, $wgLoginNotifyEnableForPriv ) ) {
// Not a user we care about.
return false;
}
return true;
}
}

View file

@ -0,0 +1,130 @@
<?php
namespace LoginNotify\Maintenance;
use BatchRowIterator;
use LoggedUpdateMaintenance;
use MediaWiki\MediaWikiServices;
use RecursiveIteratorIterator;
use User;
$IP = getenv( 'MW_INSTALL_PATH' );
if ( $IP === false ) {
$IP = __DIR__ . '/../../..';
}
require_once "$IP/maintenance/Maintenance.php";
/**
* Cleans up old preference values
* @codingStandardsIgnoreStart
*/
class MigratePreferences extends LoggedUpdateMaintenance {
// @codingStandardsIgnoreEnd
// Previously, these constants were used by Hooks to force different per-user defaults
const OPTIONS_FAKE_TRUTH = 2;
const OPTIONS_FAKE_FALSE = 'fake-false';
private static $mapping = [
self::OPTIONS_FAKE_FALSE => false,
self::OPTIONS_FAKE_TRUTH => true,
];
public function __construct() {
parent::__construct();
$this->addDescription( 'Cleans up old-style preferences used by LoginNotify' );
$this->setBatchSize( 500 );
}
/**
* Do the actual work. All child classes will need to implement this.
* Return true to log the update as done or false (usually on failure).
* @return bool
*/
protected function doDBUpdates() {
$dbr = $this->getDB( DB_REPLICA, 'vslow' );
$lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
$iterator = new BatchRowIterator( $dbr,
[ 'user_properties', 'user' ],
[ 'up_user', 'up_property' ],
$this->mBatchSize
);
$iterator->addConditions( [
'user_id=up_user',
'up_property' => [
'echo-subscriptions-web-login-fail',
'echo-subscriptions-web-login-success',
'echo-subscriptions-email-login-fail',
'echo-subscriptions-email-login-success',
],
'up_value' => [
self::OPTIONS_FAKE_TRUTH,
self::OPTIONS_FAKE_FALSE,
],
] );
$iterator->setFetchColumns( [ '*' ] );
$lastRow = (object)[ 'user_id' => 0 ];
$optionsToUpdate = [];
$rows = 0;
$total = 0;
$iterator = new RecursiveIteratorIterator( $iterator );
foreach ( $iterator as $row ) {
$userId = $row->user_id;
$option = $row->up_property;
$value = $row->up_value;
if ( $userId != $lastRow->user_id ) {
$rows += $this->updateUser( $lastRow, $optionsToUpdate );
if ( $rows >= $this->mBatchSize ) {
$this->output( " Updated {$rows} rows up to user ID {$lastRow->user_id}\n" );
$lbFactory->waitForReplication( [ 'wiki' => wfWikiID() ] );
$total += $rows;
$rows = 0;
}
}
if ( isset( self::$mapping[ $value ] ) ) {
$optionsToUpdate[$option] = self::$mapping[ $value ];
}
$lastRow = $row;
}
$total += $this->updateUser( $lastRow, $optionsToUpdate );
$this->output( "{$total} rows updated.\n" );
return true;
}
/**
* Update one user's preferences
*
* @param object $userRow Row from the user table
* @param array $options Associative array of preference => value
* @return int Number of options updated
*/
private function updateUser( $userRow, array &$options ) {
if ( $userRow->user_id && $options ) {
$user = User::newFromRow( $userRow );
foreach ( $options as $option => $value ) {
$user->setOption( $option, $value );
}
$user->saveSettings();
}
$count = count( $options );
$options = [];
return $count;
}
/**
* Get the update key name to go in the update log table
* @return string
*/
protected function getUpdateKey() {
return 'LoginNotify::migratePreferences';
}
}
$maintClass = MigratePreferences::class;
require_once RUN_MAINTENANCE_IF_MAIN;