* Made PermissionsError exception accept an optional second parameter for the descrip...
authorAlexandre Emsenhuber <ialex@users.mediawiki.org>
Wed, 2 Nov 2011 15:30:55 +0000 (15:30 +0000)
committerAlexandre Emsenhuber <ialex@users.mediawiki.org>
Wed, 2 Nov 2011 15:30:55 +0000 (15:30 +0000)
* PermissionsError now calls OutputPage::showPermissionsErrorPage() to display the error (this is needed to make the item above work correctly)
* Removed the override of the HTML title in OutputPage::showPermissionsErrorPage() so that it shows "Permission errors - Sitename" instead of simply "Permission errors" for consistency with the other things
* Pass the error array returned by Title::getUserPermissionsErrors() to PermissionsError where available
* Converted direct calls to OutputPage::showPermissionsErrorPage() to throw an PermissionsError error instead
* Added 'action-rollback' message that will be displayed when accessing action=rollback without sufficient rights
* Changed getRestriction() in subclasses of Action to return null when they previously returned 'read' so that user rights can be check with Title::getUserPermissionsErrors()
* Reordered checks to do first user rights, then block (if needed) and finally read only (also if needed) so that users don't think the error is temporary when they both don't have right and the database is locked

19 files changed:
includes/Action.php
includes/Article.php
includes/Exception.php
includes/FileDeleteForm.php
includes/OutputPage.php
includes/actions/HistoryAction.php
includes/actions/InfoAction.php
includes/actions/MarkpatrolledAction.php
includes/actions/RawAction.php
includes/actions/RevertAction.php
includes/actions/RollbackAction.php
includes/actions/WatchAction.php
includes/specials/SpecialImport.php
includes/specials/SpecialMovepage.php
includes/specials/SpecialUserlogin.php
includes/specials/SpecialUserrights.php
languages/messages/MessagesEn.php
languages/messages/MessagesQqq.php
maintenance/language/messages.inc

index 9f362d1..2287941 100644 (file)
@@ -200,18 +200,25 @@ abstract class Action {
         * @throws ErrorPageError
         */
        protected function checkCanExecute( User $user ) {
-               if ( $this->requiresWrite() && wfReadOnly() ) {
-                       throw new ReadOnlyError();
-               }
-
-               if ( $this->getRestriction() !== null && !$user->isAllowed( $this->getRestriction() ) ) {
-                       throw new PermissionsError( $this->getRestriction() );
+               $right = $this->getRestriction();
+               if ( $right !== null ) {
+                       $errors = $this->getTitle()->getUserPermissionsErrors( $right, $user );
+                       if ( count( $errors ) ) {
+                               throw new PermissionsError( $right, $errors );
+                       }
                }
 
                if ( $this->requiresUnblock() && $user->isBlocked() ) {
                        $block = $user->mBlock;
                        throw new UserBlockedError( $block );
                }
+
+               // This should be checked at the end so that the user won't think the
+               // error is only temporary when he also don't have the rights to execute
+               // this action
+               if ( $this->requiresWrite() && wfReadOnly() ) {
+                       throw new ReadOnlyError();
+               }
        }
 
        /**
index 19e74a4..d7b0eee 100644 (file)
@@ -1302,6 +1302,19 @@ class Article extends Page {
        public function delete() {
                global $wgOut, $wgRequest;
 
+               # This code desperately needs to be totally rewritten
+
+               # Check permissions
+               $permission_errors = $this->mTitle->getUserPermissionsErrors( 'delete', $this->getContext()->getUser() );
+               if ( count( $permission_errors ) ) {
+                       throw new PermissionsError( 'delete', $permission_errors );
+               }
+
+               # Read-only check...
+               if ( wfReadOnly() ) {
+                       throw new ReadOnlyError;
+               }
+
                $confirm = $wgRequest->wasPosted() &&
                                $this->getContext()->getUser()->matchEditToken( $wgRequest->getVal( 'wpEditToken' ) );
 
@@ -1320,24 +1333,6 @@ class Article extends Page {
                # Flag to hide all contents of the archived revisions
                $suppress = $wgRequest->getVal( 'wpSuppress' ) && $this->getContext()->getUser()->isAllowed( 'suppressrevision' );
 
-               # This code desperately needs to be totally rewritten
-
-               # Read-only check...
-               if ( wfReadOnly() ) {
-                       $wgOut->readOnlyPage();
-
-                       return;
-               }
-
-               # Check permissions
-               $permission_errors = $this->getTitle()->getUserPermissionsErrors( 'delete', $this->getContext()->getUser() );
-
-               if ( count( $permission_errors ) > 0 ) {
-                       $wgOut->showPermissionsErrorPage( $permission_errors );
-
-                       return;
-               }
-
                $wgOut->setPageTitle( wfMessage( 'delete-confirm', $this->getTitle()->getPrefixedText() ) );
 
                # Better double-check that it hasn't been deleted yet!
index c5bb056..8c4e0a7 100644 (file)
@@ -276,33 +276,34 @@ class ErrorPageError extends MWException {
  * @ingroup Exception
  */
 class PermissionsError extends ErrorPageError {
-       public $permission;
+       public $permission, $errors;
 
-       function __construct( $permission ) {
+       function __construct( $permission, $errors = array() ) {
                global $wgLang;
 
                $this->permission = $permission;
 
-               $groups = array_map(
-                       array( 'User', 'makeGroupLinkWiki' ),
-                       User::getGroupsWithPermission( $this->permission )
-               );
-
-               if( $groups ) {
-                       parent::__construct(
-                               'badaccess',
-                               'badaccess-groups',
-                               array(
-                                       $wgLang->commaList( $groups ),
-                                       count( $groups )
-                               )
-                       );
-               } else {
-                       parent::__construct(
-                               'badaccess',
-                               'badaccess-group0'
+               if ( !count( $errors ) ) {
+                       $groups = array_map(
+                               array( 'User', 'makeGroupLinkWiki' ),
+                               User::getGroupsWithPermission( $this->permission )
                        );
+
+                       if ( $groups ) {
+                               $errors[] = array( 'badaccess-groups', $wgLang->commaList( $groups ), count( $groups ) );
+                       } else {
+                               $errors[] = array( 'badaccess-group0' );
+                       }
                }
+
+               $this->errors = $errors;
+       }
+
+       function report() {
+               global $wgOut;
+
+               $wgOut->showPermissionsErrorPage( $this->errors, $this->permission );
+               $wgOut->output();
        }
 }
 
index 0c5e232..2e80663 100644 (file)
@@ -41,18 +41,18 @@ class FileDeleteForm {
         */
        public function execute() {
                global $wgOut, $wgRequest, $wgUser;
-               $this->setHeaders();
 
-               $permission_errors = $this->title->getUserPermissionsErrors('delete', $wgUser);
-               if ( count( $permission_errors ) > 0 ) {
-                       $wgOut->showPermissionsErrorPage( $permission_errors );
-                       return;
+               $permissionErrors = $this->title->getUserPermissionsErrors( 'delete', $wgUser );
+               if ( count( $permissionErrors ) ) {
+                       throw new PermissionsError( 'delete', $permissionErrors );
                }
 
                if ( wfReadOnly() ) {
                        throw new ReadOnlyError;
                }
 
+               $this->setHeaders();
+
                $this->oldimage = $wgRequest->getText( 'oldimage', false );
                $token = $wgRequest->getText( 'wpEditToken' );
                # Flag to hide all contents of the archived revisions
index 83faccc..af2f264 100644 (file)
@@ -1979,7 +1979,7 @@ class OutputPage extends ContextSource {
         * @param $action String: action that was denied or null if unknown
         */
        public function showPermissionsErrorPage( $errors, $action = null ) {
-               $this->prepareErrorPage( $this->msg( 'permissionserrors' ), $this->msg( 'permissionserrors' ) );
+               $this->prepareErrorPage( $this->msg( 'permissionserrors' ) );
 
                $this->addWikiText( $this->formatPermissionsErrorMessage( $errors, $action ) );
        }
index 779b4db..9efe953 100644 (file)
@@ -24,7 +24,7 @@ class HistoryAction extends FormlessAction {
        }
 
        public function getRestriction() {
-               return 'read';
+               return null;
        }
 
        public function requiresWrite() {
index b0b5f25..4854d30 100644 (file)
@@ -30,7 +30,7 @@ class InfoAction extends FormlessAction {
        }
 
        public function getRestriction() {
-               return 'read';
+               return null;
        }
 
        protected function getDescription() {
index f9b760a..e52d0c4 100644 (file)
@@ -29,7 +29,7 @@ class MarkpatrolledAction extends FormlessAction {
        }
 
        public function getRestriction() {
-               return 'read';
+               return null;
        }
 
        protected function getDescription() {
@@ -73,9 +73,8 @@ class MarkpatrolledAction extends FormlessAction {
                        return;
                }
 
-               if ( !empty( $errors ) ) {
-                       $this->getOutput()->showPermissionsErrorPage( $errors );
-                       return;
+               if ( count( $errors ) ) {
+                       throw new PermissionsErrorPage( 'patrol', $errors );
                }
 
                # Inform the user
index 89952f4..67d3117 100644 (file)
@@ -25,7 +25,7 @@ class RawAction extends FormlessAction {
        }
 
        public function getRestriction() {
-               return 'read';
+               return null;
        }
 
        public function requiresWrite() {
index bcb8cd8..61403aa 100644 (file)
@@ -35,7 +35,7 @@ class RevertAction extends Action {
        }
 
        public function getRestriction() {
-               return 'read';
+               return null;
        }
 
        public function show() {
index e7a6521..1cc64b3 100644 (file)
@@ -83,9 +83,7 @@ class RollbackAction extends FormlessAction {
                                        $out [] = $error;
                                }
                        }
-                       $this->getOutput()->showPermissionsErrorPage( $out );
-
-                       return;
+                       throw new PermissionsError( 'rollback', $out );
                }
 
                if ( $result == array( array( 'readonlytext' ) ) ) {
index 52e6675..4d539f4 100644 (file)
@@ -27,7 +27,7 @@ class WatchAction extends FormAction {
        }
 
        public function getRestriction() {
-               return 'read';
+               return null;
        }
 
        public function requiresUnblock() {
index d346c0e..575c94e 100644 (file)
@@ -60,10 +60,6 @@ class SpecialImport extends SpecialPage {
                        throw new PermissionsError( 'import' );
                }
 
-               if ( wfReadOnly() ) {
-                       throw new ReadOnlyError;
-               }
-
                # @todo Allow Title::getUserPermissionsErrors() to take an array
                # @todo FIXME: Title::checkSpecialsAndNSPermissions() has a very wierd expectation of what
                # getUserPermissionsErrors() might actually be used for, hence the 'ns-specialprotected'
@@ -78,9 +74,12 @@ class SpecialImport extends SpecialPage {
                        )
                );
 
-               if( $errors ){
-                       $this->getOutput()->showPermissionsErrorPage( $errors );
-                       return;
+               if ( $errors ) {
+                       throw new PermissionsError( 'import', $errors );
+               }
+
+               if ( wfReadOnly() ) {
+                       throw new ReadOnlyError;
                }
 
                $request = $this->getRequest();
index 4f66414..38b25af 100644 (file)
@@ -74,8 +74,7 @@ class MovePageForm extends UnlistedSpecialPage {
                if( !empty( $permErrors ) ) {
                        // Auto-block user's IP if the account was "hard" blocked
                        $user->spreadAnyEditBlock();
-                       $this->getOutput()->showPermissionsErrorPage( $permErrors );
-                       return;
+                       throw new PermissionsError( 'move', $permErrors );
                }
 
                $def = !$request->wasPosted();
index 8c779fa..8892d97 100644 (file)
@@ -953,14 +953,14 @@ class LoginForm extends SpecialPage {
                        // Block signup here if in readonly. Keeps user from
                        // going through the process (filling out data, etc)
                        // and being informed later.
-                       if ( wfReadOnly() ) {
-                               throw new ReadOnlyError;
+                       $permErrors = $titleObj->getUserPermissionsErrors( 'createaccount', $user, true );
+                       if ( count( $permErrors ) ) {
+                               throw new PermissionsError( 'createaccount', $permErrors );
                        } elseif ( $user->isBlockedFromCreateAccount() ) {
                                $this->userBlockedMessage( $user->isBlockedFromCreateAccount() );
                                return;
-                       } elseif ( count( $permErrors = $titleObj->getUserPermissionsErrors( 'createaccount', $user, true ) )>0 ) {
-                               $this->getOutput()->showPermissionsErrorPage( $permErrors, 'createaccount' );
-                               return;
+                       } elseif ( wfReadOnly() ) {
+                               throw new ReadOnlyError;
                        }
                }
 
index aeee554..048ed6c 100644 (file)
@@ -99,24 +99,21 @@ class UserrightsPage extends SpecialPage {
                        $this->isself = true;
                }
 
-               $out = $this->getOutput();
-
                if( !$this->userCanChangeRights( $user, true ) ) {
                        // @todo FIXME: There may be intermediate groups we can mention.
-                       $out->showPermissionsErrorPage( array( array(
-                               $user->isAnon()
-                                       ? 'userrights-nologin'
-                                       : 'userrights-notallowed' ) ) );
-                       return;
+                       $msg = $user->isAnon() ? 'userrights-nologin' : 'userrights-notallowed';
+                       throw new PermissionsError( null, array( array( $msg ) ) );
                }
 
                if ( wfReadOnly() ) {
                        throw new ReadOnlyError;
                }
 
+               $this->setHeaders();
                $this->outputHeader();
+
+               $out = $this->getOutput();
                $out->addModuleStyles( 'mediawiki.special' );
-               $this->setHeaders();
 
                // show the general form
                if ( count( $available['add'] ) || count( $available['remove'] ) ) {
index a3b47ee..b150420 100644 (file)
@@ -2016,6 +2016,7 @@ Your e-mail address is not revealed when other users contact you.',
 'action-suppressionlog'       => 'view this private log',
 'action-block'                => 'block this user from editing',
 'action-protect'              => 'change protection levels for this page',
+'action-rollback'             => 'quickly rollback the edits of the last user who edited a particular page',
 'action-import'               => 'import this page from another wiki',
 'action-importupload'         => 'import this page from a file upload',
 'action-patrol'               => "mark others' edit as patrolled",
index c5a6ad8..40752be 100644 (file)
@@ -1684,6 +1684,7 @@ API is an abbreviation for [http://en.wikipedia.org/wiki/API application program
 'action-suppressionlog'       => '{{Doc-action|suppressionlog}}',
 'action-block'                => '{{Doc-action|block}}',
 'action-protect'              => '{{Doc-action|protect}}',
+'action-rollback'             => '{{Doc-action|rollback}}',
 'action-import'               => '{{Doc-action|import}}',
 'action-importupload'         => '{{Doc-action|importupload}}',
 'action-patrol'               => '{{Doc-action|patrol}}',
index a478683..a4cb2e6 100644 (file)
@@ -1181,6 +1181,7 @@ $wgMessageStructure = array(
                'action-suppressionlog',
                'action-block',
                'action-protect',
+               'action-rollback',
                'action-import',
                'action-importupload',
                'action-patrol',