From: Aryeh Gregor <simetrical@users.mediawiki.org>
Date: Thu, 3 Jan 2008 23:43:24 +0000 (+0000)
Subject: Various Userrights-related fixes:
X-Git-Tag: 1.31.0-rc.0~50146
X-Git-Url: http://git.cyclocoop.org/%24action?a=commitdiff_plain;h=c7c46174844f697800a9d4f2dfb543b73b14f3b6;p=lhc%2Fweb%2Fwiklou.git

Various Userrights-related fixes:
* Adjust UserrightsForm so that it inherits from SpecialPage; nuke HTMLForm.  Since this breaks backward compatibility, renamed to UserrightsPage.
* Created SpecialPage::isRestricted() and enforced use of SpecialPage::userCanExecute() instead of hardcoded checks.  These can now be overridden so that more complicated restriction systems work sanely.  Used them for UserrightsPage (fixes bug 12489).
* A few random comment/documentation tweaks.
Also, update Special:Version date.
---

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 312ea4f16e..0d1c751429 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
@@ -93,7 +93,6 @@ function __autoload($className) {
 		'FileStore' => 'includes/FileStore.php',
 		'FSException' => 'includes/FileStore.php',
 		'FSTransaction' => 'includes/FileStore.php',
-		'HTMLForm' => 'includes/HTMLForm.php',
 		'HistoryBlob' => 'includes/HistoryBlob.php',
 		'ConcatenatedGzipHistoryBlob' => 'includes/HistoryBlob.php',
 		'HistoryBlobStub' => 'includes/HistoryBlob.php',
@@ -232,7 +231,7 @@ function __autoload($className) {
 		'UploadForm' => 'includes/SpecialUpload.php',
 		'UploadFormMogile' => 'includes/SpecialUploadMogile.php',
 		'LoginForm' => 'includes/SpecialUserlogin.php',
-		'UserrightsForm' => 'includes/SpecialUserrights.php',
+		'UserrightsPage' => 'includes/SpecialUserrights.php',
 		'SpecialVersion' => 'includes/SpecialVersion.php',
 		'WantedCategoriesPage' => 'includes/SpecialWantedcategories.php',
 		'WantedPagesPage' => 'includes/SpecialWantedpages.php',
diff --git a/includes/HTMLForm.php b/includes/HTMLForm.php
deleted file mode 100644
index 69ec1007c2..0000000000
--- a/includes/HTMLForm.php
+++ /dev/null
@@ -1,107 +0,0 @@
-<?php
-/**
- * This file contain a class to easily build HTML forms
- */
-
-/**
- * Class to build various forms
- *
- * @author jeluf, hashar
- */
-class HTMLForm {
-	/** name of our form. Used as prefix for labels */
-	var $mName, $mRequest;
-
-	function HTMLForm( &$request ) {
-		$this->mRequest = $request;
-	}
-
-	/**
-	 * @private
-	 * @param $name String: name of the fieldset.
-	 * @param $content String: HTML content to put in.
-	 * @return string HTML fieldset
-	 */
-	function fieldset( $name, $content ) {
-		return "<fieldset><legend>".wfMsg($this->mName.'-'.$name)."</legend>\n" .
-			$content . "\n</fieldset>\n";
-	}
-
-	/**
-	 * @private
-	 * @param $varname String: name of the checkbox.
-	 * @param $checked Boolean: set true to check the box (default False).
-	 */
-	function checkbox( $varname, $checked=false ) {
-		if ( $this->mRequest->wasPosted() && !is_null( $this->mRequest->getVal( $varname ) ) ) {
-			$checked = $this->mRequest->getCheck( $varname );
-		}
-		return "<div><input type='checkbox' value=\"1\" id=\"{$varname}\" name=\"wpOp{$varname}\"" .
-			( $checked ? ' checked="checked"' : '' ) .
-			" /><label for=\"{$varname}\">". wfMsg( $this->mName.'-'.$varname ) .
-			"</label></div>\n";
-	}
-
-	/**
-	 * @private
-	 * @param $varname String: name of the textbox.
-	 * @param $value String: optional value (default empty)
-	 * @param $size Integer: optional size of the textbox (default 20)
-	 */
-	function textbox( $varname, $value='', $size=20 ) {
-		if ( $this->mRequest->wasPosted() ) {
-			$value = $this->mRequest->getText( $varname, $value );
-		}
-		$value = htmlspecialchars( $value );
-		return "<div><label>". wfMsg( $this->mName.'-'.$varname ) .
-			"<input type='text' name=\"{$varname}\" value=\"{$value}\" size=\"{$size}\" /></label></div>\n";
-	}
-
-	/**
-	 * @private
-	 * @param $varname String: name of the radiobox.
-	 * @param $fields Array: Various fields.
-	 */
-	function radiobox( $varname, $fields ) {
-		foreach ( $fields as $value => $checked ) {
-			$s .= "<div><label><input type='radio' name=\"{$varname}\" value=\"{$value}\"" .
-				( $checked ? ' checked="checked"' : '' ) . " />" . wfMsg( $this->mName.'-'.$varname.'-'.$value ) .
-				"</label></div>\n";
-		}
-		return $this->fieldset( $varname, $s );
-	}
-
-	/**
-	 * @private
-	 * @param $varname String: name of the textareabox.
-	 * @param $value String: optional value (default empty)
-	 * @param $size Integer: optional size of the textarea (default 20)
-	 */
-	function textareabox ( $varname, $value='', $size=20 ) {
-		if ( $this->mRequest->wasPosted() ) {
-			$value = $this->mRequest->getText( $varname, $value );
-		}
-		$value = htmlspecialchars( $value );
-		return '<div><label>'.wfMsg( $this->mName.'-'.$varname ).
-		       "<textarea name=\"{$varname}\" rows=\"5\" cols=\"{$size}\">$value</textarea></label></div>\n";
-	}
-
-	/**
-	 * @private
-	 * @param $varname String: name of the arraybox.
-	 * @param $size Integer: Optional size of the textarea (default 20)
-	 */
-	function arraybox( $varname , $size=20 ) {
-		$s = '';
-		if ( $this->mRequest->wasPosted() ) {
-			$arr = $this->mRequest->getArray( $varname );
-			if ( is_array( $arr ) ) {
-				foreach ( $_POST[$varname] as $element ) {
-					$s .= htmlspecialchars( $element )."\n";
-				}
-			}
-		}
-		return "<div><label>".wfMsg( $this->mName.'-'.$varname ).
-			"<textarea name=\"{$varname}\" rows=\"5\" cols=\"{$size}\">{$s}</textarea>\n";
-	}
-} // end class
diff --git a/includes/SpecialPage.php b/includes/SpecialPage.php
index 9193e1ce58..cbd4400036 100644
--- a/includes/SpecialPage.php
+++ b/includes/SpecialPage.php
@@ -112,7 +112,7 @@ class SpecialPage
 		'Ancientpages'              => array( 'SpecialPage', 'Ancientpages' ),
 		'Deadendpages'              => array( 'SpecialPage', 'Deadendpages' ),
 		'Protectedpages'            => array( 'SpecialPage', 'Protectedpages' ),
-		'Protectedtitles'	    => array( 'SpecialPage', 'Protectedtitles' ),
+		'Protectedtitles'           => array( 'SpecialPage', 'Protectedtitles' ),
 		'Allpages'                  => array( 'IncludableSpecialPage', 'Allpages' ),
 		'Prefixindex'               => array( 'IncludableSpecialPage', 'Prefixindex' ) ,
 		'Ipblocklist'               => array( 'SpecialPage', 'Ipblocklist' ),
@@ -135,7 +135,7 @@ class SpecialPage
 		'Import'                    => array( 'SpecialPage', 'Import', 'import' ),
 		'Lockdb'                    => array( 'SpecialPage', 'Lockdb', 'siteadmin' ),
 		'Unlockdb'                  => array( 'SpecialPage', 'Unlockdb', 'siteadmin' ),
-		'Userrights'                => array( 'SpecialPage', 'Userrights' ),
+		'Userrights'                => 'UserrightsPage',
 		'MIMEsearch'                => array( 'SpecialPage', 'MIMEsearch' ),
 		'Unwatchedpages'            => array( 'SpecialPage', 'Unwatchedpages', 'unwatchedpages' ),
 		'Listredirects'             => array( 'SpecialPage', 'Listredirects' ),
@@ -352,7 +352,7 @@ class SpecialPage
 
 		foreach ( self::$mList as $name => $rec ) {
 			$page = self::getPage( $name );
-			if ( $page->isListed() && $page->getRestriction() == '' ) {
+			if ( $page->isListed() && !$page->isRestricted() ) {
 				$pages[$name] = $page;
 			}
 		}
@@ -373,11 +373,12 @@ class SpecialPage
 
 		foreach ( self::$mList as $name => $rec ) {
 			$page = self::getPage( $name );
-			if ( $page->isListed() ) {
-				$restriction = $page->getRestriction();
-				if ( $restriction != '' && $wgUser->isAllowed( $restriction ) ) {
-					$pages[$name] = $page;
-				}
+			if (
+				$page->isListed()
+				and $page->isRestricted()
+				and $page->userCanExecute( $wgUser )
+			) {
+				$pages[$name] = $page;
 			}
 		}
 		return $pages;
@@ -610,11 +611,26 @@ class SpecialPage
 		return $this->mLocalName;
 	}
 
+	/**
+	 * Can be overridden by subclasses with more complicated permissions
+	 * schemes.
+	 *
+	 * @return bool Should the page be displayed with the restricted-access
+	 *   pages?
+	 */
+	public function isRestricted() {
+		return $this->mRestriction != '';
+	}
+
 	/**
 	 * Checks if the given user (identified by an object) can execute this
-	 * special page (as defined by $mRestriction)
+	 * special page (as defined by $mRestriction).  Can be overridden by sub-
+	 * classes with more complicated permissions schemes.
+	 *
+	 * @param User $user The user to check
+	 * @return bool Does the user have permission to view the page?
 	 */
-	function userCanExecute( &$user ) {
+	public function userCanExecute( &$user ) {
 		return $user->isAllowed( $this->mRestriction );
 	}
 
diff --git a/includes/SpecialUserrights.php b/includes/SpecialUserrights.php
index 2dccb40a02..1b1367d40e 100644
--- a/includes/SpecialUserrights.php
+++ b/includes/SpecialUserrights.php
@@ -4,37 +4,26 @@
  * Special page to allow managing user group membership
  *
  * @addtogroup SpecialPage
- * @todo This code is disgusting and needs a total rewrite
+ * @todo Use checkboxes or something, this list thing is incomprehensible to
+ *   normal human beings.
  */
 
-/** */
-require_once( dirname(__FILE__) . '/HTMLForm.php');
-
-/** Entry point */
-function wfSpecialUserrights() {
-	global $wgRequest;
-	$form = new UserrightsForm($wgRequest);
-	$form->execute();
-}
-
 /**
  * A class to manage user levels rights.
  * @addtogroup SpecialPage
  */
-class UserrightsForm extends HTMLForm {
-	var $mPosted, $mRequest, $mSaveprefs;
-	/** Escaped local url name*/
-	var $action;
-
-	/** Constructor*/
-	public function __construct( &$request ) {
-		$this->mPosted = $request->wasPosted();
-		$this->mRequest =& $request;
-		$this->mName = 'userrights';
-		$this->mReason = $request->getText( 'user-reason' );
-
-		$titleObj = SpecialPage::getTitleFor( 'Userrights' );
-		$this->action = $titleObj->escapeLocalURL();
+class UserrightsPage extends SpecialPage {
+	public function __construct() {
+		parent::__construct( 'Userrights' );
+	}
+
+	public function isRestricted() {
+		return true;
+	}
+
+	public function userCanExecute( $user ) {
+		$available = $this->changeableGroups();
+		return !empty( $available['add'] ) or !empty( $available['remove'] );
 	}
 
 	/**
@@ -44,41 +33,43 @@ class UserrightsForm extends HTMLForm {
 	function execute() {
 		// If the visitor doesn't have permissions to assign or remove
 		// any groups, it's a bit silly to give them the user search prompt.
-		$available = $this->changeableGroups();
-		if( empty( $available['add'] ) && empty( $available['remove'] ) ) {
+		global $wgUser;
+		if( !$this->userCanExecute( $wgUser ) ) {
 			// fixme... there may be intermediate groups we can mention.
-			global $wgOut, $wgUser;
+			global $wgOut;
 			$wgOut->showPermissionsErrorPage( array(
 				$wgUser->isAnon()
 					? 'userrights-nologin'
 					: 'userrights-notallowed' ) );
 			return;
 		}
-		
+
+		$this->setHeaders();
+
 		// show the general form
 		$this->switchForm();
-		if( $this->mPosted ) {
+
+		global $wgRequest;
+		if( $wgRequest->wasPosted() ) {
 			// show some more forms
-			if( $this->mRequest->getCheck( 'ssearchuser' ) ) {
-				$this->editUserGroupsForm( $this->mRequest->getVal( 'user-editname' ) );
+			if( $wgRequest->getCheck( 'ssearchuser' ) ) {
+				$this->editUserGroupsForm( $wgRequest->getVal( 'user-editname' ) );
 			}
 
 			// save settings
-			if( $this->mRequest->getCheck( 'saveusergroups' ) ) {
-				global $wgUser;
-				$username = $this->mRequest->getVal( 'user-editname' );
-				$reason = $this->mRequest->getVal( 'user-reason' );
-				if( $wgUser->matchEditToken( $this->mRequest->getVal( 'wpEditToken' ), $username ) ) {
+			if( $wgRequest->getCheck( 'saveusergroups' ) ) {
+				$username = $wgRequest->getVal( 'user-editname' );
+				$reason = $wgRequest->getVal( 'user-reason' );
+				if( $wgUser->matchEditToken( $wgRequest->getVal( 'wpEditToken' ), $username ) ) {
 					$this->saveUserGroups( $username,
-						$this->mRequest->getArray( 'removable' ),
-						$this->mRequest->getArray( 'available' ),
+						$wgRequest->getArray( 'removable' ),
+						$wgRequest->getArray( 'available' ),
 						$reason );
 				}
 			}
 		}
 	}
 
-
 	/**
 	 * Save user groups changes in the database.
 	 * Data comes from the editUserGroupsForm() form function
@@ -87,7 +78,7 @@ class UserrightsForm extends HTMLForm {
 	 * @param array $removegroup id of groups to be removed.
 	 * @param array $addgroup id of groups to be added.
 	 * @param string $reason Reason for group change
-	 *
+	 * @return null
 	 */
 	function saveUserGroups( $username, $removegroup, $addgroup, $reason = '') {
 		$user = $this->fetchUser( $username );
@@ -130,12 +121,16 @@ class UserrightsForm extends HTMLForm {
 		}
 
 		$log = new LogPage( 'rights' );
+
+		global $wgRequest;
 		$log->addEntry( 'rights',
 			$user->getUserPage(),
-			$this->mReason,
+			$wgRequest->getText( 'user-reason' ),
 			array(
 				$this->makeGroupNameList( $oldGroups ),
-				$this->makeGroupNameList( $newGroups ) ) );
+				$this->makeGroupNameList( $newGroups )
+			)
+		);
 	}
 
 	/**
@@ -154,9 +149,8 @@ class UserrightsForm extends HTMLForm {
 		
 		$this->showEditUserGroupsForm( $user, $groups );
 
-		// This isn't really ideal logging behavior,
-		// but let's not hide the interwiki logs if
-		// we're using them as is.
+		// This isn't really ideal logging behavior, but let's not hide the
+		// interwiki logs if we're using them as is.
 		$this->showLogFragment( $user, $wgOut );
 	}
 
@@ -233,7 +227,7 @@ class UserrightsForm extends HTMLForm {
 	function switchForm() {
 		global $wgOut, $wgRequest;
 		$username = $wgRequest->getText( 'user-editname' );
-		$form  = Xml::openElement( 'form', array( 'method' => 'post', 'action' => $this->action, 'name' => 'uluser' ) );
+		$form  = Xml::openElement( 'form', array( 'method' => 'post', 'action' => $this->getTitle()->escapeLocalURL(), 'name' => 'uluser' ) );
 		$form .= '<fieldset><legend>' . wfMsgHtml( 'userrights-lookup-user' ) . '</legend>';
 		$form .= '<p>' . Xml::inputLabel( wfMsg( 'userrights-user-editname' ), 'user-editname', 'username', 30, $username ) . '</p>';
 		$form .= '<p>' . Xml::submitButton( wfMsg( 'editusergroup' ), array( 'name' => 'ssearchuser' ) ) . '</p>';
@@ -279,7 +273,7 @@ class UserrightsForm extends HTMLForm {
 			$grouplist = '<p>' . wfMsgHtml( 'userrights-groupsmember' ) . ' ' . implode( ', ', $list ) . '</p>';
 		}
 		$wgOut->addHTML(
-			Xml::openElement( 'form', array( 'method' => 'post', 'action' => $this->action, 'name' => 'editGroup' ) ) .
+			Xml::openElement( 'form', array( 'method' => 'post', 'action' => $this->getTitle()->escapeLocalURL(), 'name' => 'editGroup' ) ) .
 			Xml::hidden( 'user-editname', $user->getName() ) .
 			Xml::hidden( 'wpEditToken', $wgUser->editToken( $user->getName() ) ) .
 			Xml::openElement( 'fieldset' ) .
diff --git a/includes/SpecialVersion.php b/includes/SpecialVersion.php
index de0a25002d..f63f995f6b 100644
--- a/includes/SpecialVersion.php
+++ b/includes/SpecialVersion.php
@@ -52,7 +52,7 @@ class SpecialVersion {
 		$ret =
 		"__NOTOC__
 		This wiki is powered by '''[http://www.mediawiki.org/ MediaWiki]''',
-		copyright (C) 2001-2007 Magnus Manske, Brion Vibber, Lee Daniel Crocker,
+		copyright (C) 2001-2008 Magnus Manske, Brion Vibber, Lee Daniel Crocker,
 		Tim Starling, Erik Möller, Gabriel Wicke, Ævar Arnfjörð Bjarmason,
 		Niklas Laxström, Domas Mituzas, Rob Church and others.