From 2108c55ec531fbf44d0736b2ce5c532d677669da Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Sat, 10 Sep 2016 22:14:04 -0700 Subject: [PATCH] Ensure users are able to edit the page after changing the content model It is possible for page restrictions to be dependent upon the content model a page. The best example of this is user JavaScript and CSS subpages. This adds a Title::setContentModel() function which allows mocking a Title's content model for the purpose of permission checks. EditPage and Special:ChangeContentModel were updated to ensure the user can edit the page with the newly proposed content model before making the change. Title::$mContentModel was made private to make sure nothing else mucks around with it. There were no uses outside of Title anyways. Bug: T145316 Change-Id: I28c46f408cadf65ed1868401cd00dc15e5acd2fe --- includes/EditPage.php | 11 +++++- includes/Title.php | 37 ++++++++++++++++--- .../specials/SpecialChangeContentModel.php | 16 ++++++-- 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/includes/EditPage.php b/includes/EditPage.php index 7e4e411d5e..41b986553a 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -1838,8 +1838,17 @@ class EditPage { } elseif ( !$wgUser->isAllowed( 'editcontentmodel' ) ) { $status->setResult( false, self::AS_NO_CHANGE_CONTENT_MODEL ); return $status; - } + // Make sure the user can edit the page under the new content model too + $titleWithNewContentModel = clone $this->mTitle; + $titleWithNewContentModel->setContentModel( $this->contentModel ); + if ( !$titleWithNewContentModel->userCan( 'editcontentmodel', $wgUser ) + || !$titleWithNewContentModel->userCan( 'edit', $wgUser ) + ) { + $status->setResult( false, self::AS_NO_CHANGE_CONTENT_MODEL ); + return $status; + } + $changingContentModel = true; $oldContentModel = $this->mTitle->getContentModel(); } diff --git a/includes/Title.php b/includes/Title.php index 3475b26bbb..16c715d7a7 100644 --- a/includes/Title.php +++ b/includes/Title.php @@ -91,7 +91,13 @@ class Title implements LinkTarget { * @var bool|string ID of the page's content model, i.e. one of the * CONTENT_MODEL_XXX constants */ - public $mContentModel = false; + private $mContentModel = false; + + /** + * @var bool If a content model was forced via setContentModel() + * this will be true to avoid having other code paths reset it + */ + private $mForcedContentModel = false; /** @var int Estimated number of revisions; null of not loaded */ private $mEstimateRevisions; @@ -467,9 +473,9 @@ class Title implements LinkTarget { if ( isset( $row->page_latest ) ) { $this->mLatestID = (int)$row->page_latest; } - if ( isset( $row->page_content_model ) ) { + if ( !$this->mForcedContentModel && isset( $row->page_content_model ) ) { $this->mContentModel = strval( $row->page_content_model ); - } else { + } elseif ( !$this->mForcedContentModel ) { $this->mContentModel = false; # initialized lazily in getContentModel() } if ( isset( $row->page_lang ) ) { @@ -483,7 +489,9 @@ class Title implements LinkTarget { $this->mLength = 0; $this->mRedirect = false; $this->mLatestID = 0; - $this->mContentModel = false; # initialized lazily in getContentModel() + if ( !$this->mForcedContentModel ) { + $this->mContentModel = false; # initialized lazily in getContentModel() + } } } @@ -921,8 +929,9 @@ class Title implements LinkTarget { * @return string Content model id */ public function getContentModel( $flags = 0 ) { - if ( ( !$this->mContentModel || $flags === Title::GAID_FOR_UPDATE ) && - $this->getArticleID( $flags ) + if ( !$this->mForcedContentModel + && ( !$this->mContentModel || $flags === Title::GAID_FOR_UPDATE ) + && $this->getArticleID( $flags ) ) { $linkCache = LinkCache::singleton(); $linkCache->addLinkObj( $this ); # in case we already had an article ID @@ -946,6 +955,22 @@ class Title implements LinkTarget { return $this->getContentModel() == $id; } + /** + * Set a proposed content model for the page for permissions + * checking. This does not actually change the content model + * of a title! + * + * Additionally, you should make sure you've checked + * ContentHandler::canBeUsedOn() first. + * + * @since 1.28 + * @param string $model CONTENT_MODEL_XXX constant + */ + public function setContentModel( $model ) { + $this->mContentModel = $model; + $this->mForcedContentModel = true; + } + /** * Get the namespace text * diff --git a/includes/specials/SpecialChangeContentModel.php b/includes/specials/SpecialChangeContentModel.php index b37c47556f..4fb1b539ca 100644 --- a/includes/specials/SpecialChangeContentModel.php +++ b/includes/specials/SpecialChangeContentModel.php @@ -156,10 +156,20 @@ class SpecialChangeContentModel extends FormSpecialPage { } $this->title = Title::newFromText( $data['pagetitle'] ); + $titleWithNewContentModel = clone $this->title; + $titleWithNewContentModel->setContentModel( $data['model'] ); $user = $this->getUser(); - // Check permissions and make sure the user has permission to edit the specific page - $errors = $this->title->getUserPermissionsErrors( 'editcontentmodel', $user ); - $errors = wfMergeErrorArrays( $errors, $this->title->getUserPermissionsErrors( 'edit', $user ) ); + // Check permissions and make sure the user has permission to: + $errors = wfMergeErrorArrays( + // edit the contentmodel of the page + $this->title->getUserPermissionsErrors( 'editcontentmodel', $user ), + // edit the page under the old content model + $this->title->getUserPermissionsErrors( 'edit', $user ), + // edit the contentmodel under the new content model + $titleWithNewContentModel->getUserPermissionsErrors( 'editcontentmodel', $user ), + // edit the page under the new content model + $titleWithNewContentModel->getUserPermissionsErrors( 'edit', $user ) + ); if ( $errors ) { $out = $this->getOutput(); $wikitext = $out->formatPermissionsErrorMessage( $errors ); -- 2.20.1