From ef663e19efa75fd9bfd7d506872a264270406f0b Mon Sep 17 00:00:00 2001 From: =?utf8?q?Gerg=C5=91=20Tisza?= Date: Mon, 27 Aug 2018 00:51:55 +0200 Subject: [PATCH] Allow admins to delete user JS/CSS pages 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 | 47 ++++++++++++++++++++------------- includes/skins/SkinTemplate.php | 4 +-- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/includes/Title.php b/includes/Title.php index c919b18856..1bff30f310 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -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' ) diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php index b44d4096d5..564220c52c 100644 --- a/includes/skins/SkinTemplate.php +++ b/includes/skins/SkinTemplate.php @@ -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" ) -- 2.20.1