From b537e44c4ea80c06e969f94a77377ea7e3626084 Mon Sep 17 00:00:00 2001 From: =?utf8?q?M=C3=A1t=C3=A9=20Szab=C3=B3?= Date: Sat, 18 May 2019 18:31:55 +0200 Subject: [PATCH] EditPage: Migrate Title::userCan() calls to PermissionManager T208768 introduced the PermissionManager service that can now be used for page specific permission checks. This change replaces calls to Title::userCan() with the new service in the EditPage class. Bug: T220191 Change-Id: I835d68d6c47785cf35386bca0431907fee87f0c1 --- includes/EditPage.php | 39 ++++++++++-- tests/phpunit/includes/EditPageTest.php | 80 +++++++++++++++++++++---- 2 files changed, 101 insertions(+), 18 deletions(-) diff --git a/includes/EditPage.php b/includes/EditPage.php index 7908fcc585..47a8b5b87f 100644 --- a/includes/EditPage.php +++ b/includes/EditPage.php @@ -1483,8 +1483,9 @@ class EditPage { $user = $this->context->getUser(); $title = Title::newFromText( $preload ); + # Check for existence to avoid getting MediaWiki:Noarticletext - if ( $title === null || !$title->exists() || !$title->userCan( 'read', $user ) ) { + if ( !$this->isPageExistingAndViewable( $title, $user ) ) { // TODO: somehow show a warning to the user! return $handler->makeEmptyContent(); } @@ -1493,7 +1494,7 @@ class EditPage { if ( $page->isRedirect() ) { $title = $page->getRedirectTarget(); # Same as before - if ( $title === null || !$title->exists() || !$title->userCan( 'read', $user ) ) { + if ( !$this->isPageExistingAndViewable( $title, $user ) ) { // TODO: somehow show a warning to the user! return $handler->makeEmptyContent(); } @@ -1526,6 +1527,21 @@ class EditPage { return $content->preloadTransform( $title, $parserOptions, $params ); } + /** + * Verify if a given title exists and the given user is allowed to view it + * + * @see EditPage::getPreloadedContent() + * @param Title|null $title + * @param User $user + * @return bool + * @throws Exception + */ + private function isPageExistingAndViewable( $title, User $user ) { + $permissionManager = MediaWikiServices::getInstance()->getPermissionManager(); + + return $title && $title->exists() && $permissionManager->userCan( 'read', $user, $title ); + } + /** * Make sure the form isn't faking a user's credentials. * @@ -1988,6 +2004,8 @@ ERROR; } } + $permissionManager = MediaWikiServices::getInstance()->getPermissionManager(); + $changingContentModel = false; if ( $this->contentModel !== $this->mTitle->getContentModel() ) { if ( !$config->get( 'ContentHandlerUseDB' ) ) { @@ -2001,10 +2019,19 @@ ERROR; // 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', $user ) - || !$titleWithNewContentModel->userCan( 'edit', $user ) + + $canEditModel = $permissionManager->userCan( + 'editcontentmodel', + $user, + $titleWithNewContentModel + ); + + if ( + !$canEditModel + || !$permissionManager->userCan( 'edit', $user, $titleWithNewContentModel ) ) { $status->setResult( false, self::AS_NO_CHANGE_CONTENT_MODEL ); + return $status; } @@ -2048,7 +2075,7 @@ ERROR; if ( $new ) { // Late check for create permission, just in case *PARANOIA* - if ( !$this->mTitle->userCan( 'create', $user ) ) { + if ( !$permissionManager->userCan( 'create', $user, $this->mTitle ) ) { $status->fatal( 'nocreatetext' ); $status->value = self::AS_NO_CREATE_PERMISSION; wfDebug( __METHOD__ . ": no create permission\n" ); @@ -2672,7 +2699,7 @@ ERROR; protected function showCustomIntro() { if ( $this->editintro ) { $title = Title::newFromText( $this->editintro ); - if ( $title instanceof Title && $title->exists() && $title->userCan( 'read' ) ) { + if ( $this->isPageExistingAndViewable( $title, $this->context->getUser() ) ) { // Added using template syntax, to take 's into account. $this->context->getOutput()->addWikiTextAsContent( '
{{:' . $title->getFullText() . '}}
', diff --git a/tests/phpunit/includes/EditPageTest.php b/tests/phpunit/includes/EditPageTest.php index f5fef61a0c..d2540f6f4d 100644 --- a/tests/phpunit/includes/EditPageTest.php +++ b/tests/phpunit/includes/EditPageTest.php @@ -693,14 +693,6 @@ hello * @covers EditPage */ public function testCheckDirectEditingDisallowed_forNonTextContent() { - $title = Title::newFromText( 'Dummy:NonTextPageForEditPage' ); - $page = WikiPage::factory( $title ); - - $article = new Article( $title ); - $article->getContext()->setTitle( $title ); - $ep = new EditPage( $article ); - $ep->setContextTitle( $title ); - $user = $GLOBALS['wgUser']; $edit = [ @@ -711,15 +703,79 @@ hello 'wpUnicodeCheck' => EditPage::UNICODE_CHECK, ]; - $req = new FauxRequest( $edit, true ); - $ep->importFormData( $req ); - $this->setExpectedException( MWException::class, 'This content model is not supported: testing' ); - $ep->internalAttemptSave( $result, false ); + $this->doEditDummyNonTextPage( $edit ); + } + + /** @covers EditPage */ + public function testShouldPreventChangingContentModelWhenUserCannotChangeModelForTitle() { + $this->setTemporaryHook( 'getUserPermissionsErrors', + function ( Title $page, $user, $action, &$result ) { + if ( $action === 'editcontentmodel' && + $page->getContentModel() === CONTENT_MODEL_WIKITEXT ) { + $result = false; + + return false; + } + } ); + + $user = $GLOBALS['wgUser']; + + $status = $this->doEditDummyNonTextPage( [ + 'wpTextbox1' => 'some text', + 'wpEditToken' => $user->getEditToken(), + 'wpEdittime' => '', + 'wpStarttime' => wfTimestampNow(), + 'wpUnicodeCheck' => EditPage::UNICODE_CHECK, + 'model' => CONTENT_MODEL_WIKITEXT, + 'format' => CONTENT_FORMAT_WIKITEXT, + ] ); + + $this->assertFalse( $status->isOK() ); + $this->assertEquals( EditPage::AS_NO_CHANGE_CONTENT_MODEL, $status->getValue() ); } + /** @covers EditPage */ + public function testShouldPreventChangingContentModelWhenUserCannotEditTargetTitle() { + $this->setTemporaryHook( 'getUserPermissionsErrors', + function ( Title $page, $user, $action, &$result ) { + if ( $action === 'edit' && $page->getContentModel() === CONTENT_MODEL_WIKITEXT ) { + $result = false; + return false; + } + } ); + + $user = $GLOBALS['wgUser']; + + $status = $this->doEditDummyNonTextPage( [ + 'wpTextbox1' => 'some text', + 'wpEditToken' => $user->getEditToken(), + 'wpEdittime' => '', + 'wpStarttime' => wfTimestampNow(), + 'wpUnicodeCheck' => EditPage::UNICODE_CHECK, + 'model' => CONTENT_MODEL_WIKITEXT, + 'format' => CONTENT_FORMAT_WIKITEXT, + ] ); + + $this->assertFalse( $status->isOK() ); + $this->assertEquals( EditPage::AS_NO_CHANGE_CONTENT_MODEL, $status->getValue() ); + } + + private function doEditDummyNonTextPage( array $edit ): Status { + $title = Title::newFromText( 'Dummy:NonTextPageForEditPage' ); + + $article = new Article( $title ); + $article->getContext()->setTitle( $title ); + $ep = new EditPage( $article ); + $ep->setContextTitle( $title ); + + $req = new FauxRequest( $edit, true ); + $ep->importFormData( $req ); + + return $ep->internalAttemptSave( $result, false ); + } } -- 2.20.1