Various Userrights-related fixes:
authorAryeh Gregor <simetrical@users.mediawiki.org>
Thu, 3 Jan 2008 23:43:24 +0000 (23:43 +0000)
committerAryeh Gregor <simetrical@users.mediawiki.org>
Thu, 3 Jan 2008 23:43:24 +0000 (23:43 +0000)
* 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.

includes/AutoLoader.php
includes/HTMLForm.php [deleted file]
includes/SpecialPage.php
includes/SpecialUserrights.php
includes/SpecialVersion.php

index 312ea4f..0d1c751 100644 (file)
@@ -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 (file)
index 69ec100..0000000
+++ /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
index 9193e1c..cbd4400 100644 (file)
@@ -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 );
        }
 
index 2dccb40..1b1367d 100644 (file)
@@ -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' ) .
index de0a250..f63f995 100644 (file)
@@ -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.