r77555 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r77554‎ | r77555 | r77556 >
Date:04:17, 2 December 2010
Author:werdna
Status:reverted (Comments)
Tags:
Comment:
Merge DisableAccount extension into core, disabled by default because the rights are unassigned
Modified paths:
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/SpecialPage.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialDisableAccount.php (added) (history)
  • /trunk/phase3/languages/messages/MessagesEn.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/AutoLoader.php
@@ -625,6 +625,7 @@
626626 'SpecialBookSources' => 'includes/specials/SpecialBooksources.php',
627627 'SpecialCategories' => 'includes/specials/SpecialCategories.php',
628628 'SpecialComparePages' => 'includes/specials/SpecialComparePages.php',
 629+ 'SpecialDisableAccount' => 'includes/specials/SpecialDisableAccount.php',
629630 'SpecialExport' => 'includes/specials/SpecialExport.php',
630631 'SpecialFilepath' => 'includes/specials/SpecialFilepath.php',
631632 'SpecialImport' => 'includes/specials/SpecialImport.php',
Index: trunk/phase3/includes/specials/SpecialDisableAccount.php
@@ -0,0 +1,65 @@
 2+<?php
 3+
 4+class SpecialDisableAccount extends SpecialPage {
 5+ function __construct() {
 6+ parent::__construct( 'DisableAccount', 'disableaccount',
 7+ true, array( $this, 'show' ) );
 8+ }
 9+
 10+ public function show( $par ) {
 11+ $formFields = array(
 12+ 'account' => array(
 13+ 'type' => 'text',
 14+ 'validation-callback' => array( __CLASS__, 'validateUser' ),
 15+ 'label-message' => 'disableaccount-user',
 16+ ),
 17+ 'confirm' => array(
 18+ 'type' => 'toggle',
 19+ 'validation-callback' => array( __CLASS__, 'checkConfirmation' ),
 20+ 'label-message' => 'disableaccount-confirm',
 21+ ),
 22+ );
 23+
 24+ $htmlForm = new HTMLForm( $formFields, 'disableaccount' );
 25+
 26+ $htmlForm->setSubmitCallback( array( __CLASS__, 'submit' ) );
 27+ $htmlForm->setTitle( $this->getTitle() );
 28+
 29+ $htmlForm->show();
 30+ }
 31+
 32+ static function validateUser( $field, $allFields ) {
 33+ $u = User::newFromName( $field );
 34+
 35+ if ( $u && $u->getID() != 0 ) {
 36+ return true;
 37+ } else {
 38+ return wfMsgExt( 'disableaccount-nosuchuser', 'parseinline', array( $field ) );
 39+ }
 40+ }
 41+
 42+ static function checkConfirmation( $field, $allFields ) {
 43+ if ( $field ) {
 44+ return true;
 45+ } else {
 46+ return wfMsgExt( 'disableaccount-mustconfirm', 'parseinline' );
 47+ }
 48+ }
 49+
 50+ static function submit( $fields ) {
 51+ $user = User::newFromName( $fields['account'] );
 52+
 53+ $user->setPassword( null );
 54+ $user->setEmail( null );
 55+ $user->setToken();
 56+ $user->addGroup( 'inactive' );
 57+
 58+ $user->saveSettings();
 59+ $user->invalidateCache();
 60+
 61+ global $wgOut;
 62+ $wgOut->addWikiMsg( 'disableaccount-success', $user->getName() );
 63+
 64+ return true;
 65+ }
 66+}
\ No newline at end of file
Index: trunk/phase3/includes/SpecialPage.php
@@ -133,6 +133,7 @@
134134 'Listbots' => array( 'SpecialRedirectToSpecial', 'Listbots', 'Listusers', 'bot' ),
135135 'Activeusers' => 'SpecialActiveUsers',
136136 'Userrights' => 'UserrightsPage',
 137+ 'DisableAccount' => 'SpecialDisableAccount',
137138
138139 # Recent changes and logs
139140 'Newimages' => array( 'IncludableSpecialPage', 'Newimages' ),
Index: trunk/phase3/languages/messages/MessagesEn.php
@@ -4359,4 +4359,16 @@
43604360 'sqlite-has-fts' => '$1 with full-text search support',
43614361 'sqlite-no-fts' => '$1 without full-text search support',
43624362
 4363+## Special:DisableAccount
 4364+'disableaccount-desc' => 'Allows administrators to disable individual accounts.',
 4365+'right-disableaccount' => 'Disable accounts',
 4366+'disableaccount' => 'Disable a user account',
 4367+'disableaccount-user' => 'User name:',
 4368+'disableaccount-confirm' => "Disable this user account.
 4369+The user will not be able to log in, reset their password, or receive email notifications.
 4370+If the user is currently logged in anywhere, they will be immediately logged out.
 4371+''Note that disabling an account is not reversible without system administrator intervention.''",
 4372+'disableaccount-mustconfirm' => 'You must confirm that you wish to disable this account.',
 4373+'disableaccount-nosuchuser' => 'The user account "$1" does not exist.',
 4374+'disableaccount-success' => 'The user account "$1" has been permanently disabled.',
43634375 );

Follow-up revisions

RevisionCommit summaryAuthorDate
r77557Merge r77555 into core. As a bonus, fix the PHP notices that HTMLForm has bee...werdna04:42, 2 December 2010
r77563Follow-up r77555: Remove unused message (used for extensions only)...raymond08:03, 2 December 2010
r77572Follow-up r77555: Add a commented out line with the new disableaccount rightraymond10:47, 2 December 2010
r78116RELEASE-NOTES for r77555werdna04:08, 9 December 2010
r86146Revert r77555 and followups r77563, r77572, r78116 (merge DisableAccount to c...demon22:49, 15 April 2011

Comments

#Comment by Raymond (talk | contribs)   06:55, 2 December 2010

Isn't this very similar to the setting of $wgBlockDisablesLogin?

/**

* If true, blocked users will not be allowed to login. When using this with
* a public wiki, the effect of logging out blocked users may actually be
* avers: unless the user's address is also blocked (e.g. auto-block),
* logging the user out will again allow reading and editing, just as for
* anonymous visitors.
*/

$wgBlockDisablesLogin = false;

This is already set to true for:

'wgBlockDisablesLogin' => array(

   'arbcom_dewiki' => 'true', // # 23357
   'arbcom_nlwiki' => 'true', // # 22630
   'auditcomwiki' => 'true', // #23231
   'boardwiki' => 'true', // #23231
   'chairwiki' => 'true', // #23231
   'collabwiki' => 'true', // #23231
   'chapcomwiki' => 'true', // # 22319
   'execwiki' => 'true', // # 22319
   'internalwiki' => 'true', // #23231
   'noboard_chapterswikimedia' => true,
   'officewiki' => 'true', // #23231
   'otrs_wikiwiki' => 'true', // # 22319
   'wikimaniateamwiki' => 'true', // # 22319

),

#Comment by Werdna (talk | contribs)   06:57, 2 December 2010

Doesn't block email notification. Also, blocking is a terrible way to disable an account, for non-technical reasons.

#Comment by Raymond (talk | contribs)   11:13, 2 December 2010

$user->addGroup( 'inactive' );

I am a bit unhappy with this usergroup name. Wouldn't be "disabled" a more descriptive usergroup name, would it?

If changed, we have to create theses messages:

'group-disabled' => 'Disabled users', 'group-disabled-member' => 'Disabled user', 'grouppage-disabled' => 'Project:Disabled users',

otherwise to move theses messages from WikimediaMessages.i18n.php to MessagesEn.php:

'group-inactive' => 'Inactive users', 'group-inactive-member' => 'inactive user', 'grouppage-inactive' => 'Project:Inactive users',

#Comment by Catrope (talk | contribs)   23:03, 3 December 2010
+		$user->setPassword( null );
+		$user->setEmail( null );
+		$user->setToken();
+		$user->addGroup( 'inactive' );
+
+		$user->saveSettings();
+		$user->invalidateCache();

This should all be used to $user->disable() IMO.

#Comment by VasilievVV (talk | contribs)   13:52, 4 December 2010

Please, do not add "reversible by sysadmins only" features. That's a bad idea because a user cannot reverse the action he has done.

May you use "inactive" (I agree with Raymond about its title, by the way) flag for disabling and enabling accounts instead?

#Comment by Happy-melon (talk | contribs)   00:55, 15 December 2010

This isn't even properly "reversible" with sysadmin intervention: you still need to get a new email/password combo from the user, without the old data to authenticate with: the old data is still irretrievably lost.

Why would rejigging $wgBlockDisablesLogin as $wgBlockDisablesAccount, and adding a check into UserMailer, not be a cleaner and (more importantly) fully-reversible way of doing this?

#Comment by 😂 (talk | contribs)   15:28, 27 March 2011

This should be reverted for these reasons.

#Comment by Nikerabbit (talk | contribs)   23:11, 10 December 2010

Could you document "disableaccount" message. Is it a button, header or what.

#Comment by Werdna (talk | contribs)   00:10, 19 January 2011

Name of the special page.

Status & tagging log