Allow admins to delete user JS/CSS pages
authorGergő Tisza <tgr.huwiki@gmail.com>
Sun, 26 Aug 2018 22:51:55 +0000 (00:51 +0200)
committerGergő Tisza <gtisza@wikimedia.org>
Sun, 26 Aug 2018 22:57:58 +0000 (22:57 +0000)
As it turns out this is super easy: we do require edit permissions
for some actions but not for delete, so it is enough to just
whitelist it.

Also fix SkinTemplate to not show the undelete action when the
user can undelete some pages but not the current one.

Bug: T200176
Change-Id: I0d326e6afde7ad2c9f7cb7f19ecc6c275c1ef65c

includes/Title.php
includes/skins/SkinTemplate.php

index c919b18..1bff30f 100644 (file)
@@ -2423,25 +2423,34 @@ class Title implements LinkTarget {
                # Protect css/json/js subpages of user pages
                # XXX: this might be better using restrictions
 
-               if ( $action != 'patrol' ) {
-                       if ( preg_match( '/^' . preg_quote( $user->getName(), '/' ) . '\//', $this->mTextform ) ) {
-                               if (
-                                       $this->isUserCssConfigPage()
-                                       && !$user->isAllowedAny( 'editmyusercss', 'editusercss' )
-                               ) {
-                                       $errors[] = [ 'mycustomcssprotected', $action ];
-                               } elseif (
-                                       $this->isUserJsonConfigPage()
-                                       && !$user->isAllowedAny( 'editmyuserjson', 'edituserjson' )
-                               ) {
-                                       $errors[] = [ 'mycustomjsonprotected', $action ];
-                               } elseif (
-                                       $this->isUserJsConfigPage()
-                                       && !$user->isAllowedAny( 'editmyuserjs', 'edituserjs' )
-                               ) {
-                                       $errors[] = [ 'mycustomjsprotected', $action ];
-                               }
-                       } else {
+               if ( $action === 'patrol' ) {
+                       return [];
+               }
+
+               if ( preg_match( '/^' . preg_quote( $user->getName(), '/' ) . '\//', $this->mTextform ) ) {
+                       // Users need editmyuser* to edit their own CSS/JSON/JS subpages.
+                       if (
+                               $this->isUserCssConfigPage()
+                               && !$user->isAllowedAny( 'editmyusercss', 'editusercss' )
+                       ) {
+                               $errors[] = [ 'mycustomcssprotected', $action ];
+                       } elseif (
+                               $this->isUserJsonConfigPage()
+                               && !$user->isAllowedAny( 'editmyuserjson', 'edituserjson' )
+                       ) {
+                               $errors[] = [ 'mycustomjsonprotected', $action ];
+                       } elseif (
+                               $this->isUserJsConfigPage()
+                               && !$user->isAllowedAny( 'editmyuserjs', 'edituserjs' )
+                       ) {
+                               $errors[] = [ 'mycustomjsprotected', $action ];
+                       }
+               } else {
+                       // Users need editmyuser* to edit their own CSS/JSON/JS subpages, except for
+                       // deletion/suppression which cannot be used for attacks and we want to avoid the
+                       // situation where an unprivileged user can post abusive content on their subpages
+                       // and only very highly privileged users could remove it.
+                       if ( !in_array( $action, [ 'delete', 'deleterevision', 'suppressrevision' ], true ) ) {
                                if (
                                        $this->isUserCssConfigPage()
                                        && !$user->isAllowed( 'editusercss' )
index b44d409..564220c 100644 (file)
@@ -1055,13 +1055,13 @@ class SkinTemplate extends Skin {
                                        }
                                } else {
                                        // article doesn't exist or is deleted
-                                       if ( $user->isAllowed( 'deletedhistory' ) ) {
+                                       if ( $title->quickUserCan( 'deletedhistory', $user ) ) {
                                                $n = $title->isDeleted();
                                                if ( $n ) {
                                                        $undelTitle = SpecialPage::getTitleFor( 'Undelete', $title->getPrefixedDBkey() );
                                                        // If the user can't undelete but can view deleted
                                                        // history show them a "View .. deleted" tab instead.
-                                                       $msgKey = $user->isAllowed( 'undelete' ) ? 'undelete' : 'viewdeleted';
+                                                       $msgKey = $title->quickUserCan( 'undelete', $user ) ? 'undelete' : 'viewdeleted';
                                                        $content_navigation['actions']['undelete'] = [
                                                                'class' => $this->getTitle()->isSpecial( 'Undelete' ) ? 'selected' : false,
                                                                'text' => wfMessageFallback( "$skname-action-$msgKey", "{$msgKey}_short" )