Mattflaschen has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/334744 )

Change subject: SECURITY: Attribute Special:EnableFlow to initiating user
......................................................................

SECURITY: Attribute Special:EnableFlow to initiating user

It already does when the page doesn't exist.  When it does,
it uses the Converter Flow, which didn't fully support this.

So refactor the Converter to allow this, and pass in the right user.

Bug: T146425
Change-Id: I3636e637fe725f96fd80e57c55f6b0cbcea90744
---
M includes/Import/Converter.php
M includes/Import/Importer.php
M includes/Import/Wikitext/ConversionStrategy.php
M includes/Import/Wikitext/ImportSource.php
M includes/Specials/SpecialEnableFlow.php
M maintenance/convertLqtPageFromRemoteApiForTesting.php
M maintenance/convertNamespaceFromWikitext.php
M tests/phpunit/Import/TalkpageImportOperationTest.php
M tests/phpunit/Import/Wikitext/ConversionStrategyTest.php
M tests/phpunit/Import/Wikitext/ImportSourceTest.php
10 files changed, 70 insertions(+), 19 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow 
refs/changes/44/334744/1

diff --git a/includes/Import/Converter.php b/includes/Import/Converter.php
index 44c8e84..e2d7710 100644
--- a/includes/Import/Converter.php
+++ b/includes/Import/Converter.php
@@ -65,8 +65,7 @@
         * @param DatabaseBase $dbw Master wiki database to read from
         * @param Importer $importer
         * @param LoggerInterface $logger
-        * @param User $user Administrative user for moves and edits related
-        *  to the conversion process.
+        * @param User $user User for moves and edits related to the conversion 
process
         * @param IConversionStrategy $strategy
         * @throws ImportException When $user does not have an Id
         */
@@ -185,7 +184,7 @@
                }
 
                $source = $this->strategy->createImportSource( $archiveTitle );
-               if ( $this->importer->import( $source, $title, 
$this->strategy->getSourceStore() ) ) {
+               if ( $this->importer->import( $source, $title, $this->user, 
$this->strategy->getSourceStore() ) ) {
                        $this->createArchiveCleanupRevision( $title, 
$archiveTitle );
                        $this->logger->info( "Completed import to $title from 
$archiveTitle" );
                } else {
diff --git a/includes/Import/Importer.php b/includes/Import/Importer.php
index d93a4d8..24834a7 100644
--- a/includes/Import/Importer.php
+++ b/includes/Import/Importer.php
@@ -107,11 +107,14 @@
         *
         * @param IImportSource     $source
         * @param Title             $targetPage
+        * @param User User doing the conversion actions (e.g. initial 
description,
+        *    wikitext archive edit).  However, actions will be attributed to 
the original
+        *    user when possible (e.g. the user who did the original LQT reply)
         * @param ImportSourceStore $sourceStore
         * @return bool True When the import completes with no failures
         */
-       public function import( IImportSource $source, Title $targetPage, 
ImportSourceStore $sourceStore ) {
-               $operation = new TalkpageImportOperation( $source, 
$this->occupationController );
+       public function import( IImportSource $source, Title $targetPage, User 
$user, ImportSourceStore $sourceStore ) {
+               $operation = new TalkpageImportOperation( $source, $user, 
$this->occupationController );
                $pageImportState = new PageImportState(
                        $this->workflowLoaderFactory
                                ->createWorkflowLoader( $targetPage )
@@ -521,14 +524,24 @@
         */
        protected $importSource;
 
+       /** @var User User doing the conversion actions (e.g. initial 
description, wikitext
+        *    archive edit).  However, actions will be attributed to the 
original user when
+        *    possible (e.g. the user who did the original LQT reply)
+        */
+       protected $user;
+
        /** @var OccupationController */
        protected $occupationController;
 
        /**
         * @param IImportSource $source
+        * @param User $user The import user; this will only be used when there 
is no
+        *   'original' user
+        * @param OccupationController $occupationController
         */
-       public function __construct( IImportSource $source, 
OccupationController $occupationController ) {
+       public function __construct( IImportSource $source, User $user, 
OccupationController $occupationController ) {
                $this->importSource = $source;
+               $this->user = $user;
                $this->occupationController = $occupationController;
        }
 
@@ -547,7 +560,7 @@
                        // Explicitly allow creation of board
                        $allowCreationStatus = 
$this->occupationController->allowCreation(
                                $destinationTitle,
-                               
$this->occupationController->getTalkpageManager(),
+                               $this->user,
                                /* $mustNotExist = */ true
                        );
                        if ( !$allowCreationStatus->isGood() ) {
diff --git a/includes/Import/Wikitext/ConversionStrategy.php 
b/includes/Import/Wikitext/ConversionStrategy.php
index 6fd8c30..7037005 100644
--- a/includes/Import/Wikitext/ConversionStrategy.php
+++ b/includes/Import/Wikitext/ConversionStrategy.php
@@ -13,6 +13,7 @@
 use Psr\Log\LoggerInterface;
 use StubObject;
 use Title;
+use User;
 use WikitextContent;
 
 /**
@@ -55,10 +56,19 @@
         */
        protected $headerSuffix;
 
+       /** @var User User doing the conversion actions (e.g. initial 
description, wikitext
+        *    archive edit).  However, actions will be attributed to the 
original user when
+        *    possible (e.g. the user who did the original LQT reply)
+        *
+        */
+        protected $user;
+
        /**
         * @param Parser|StubObject $parser
         * @param ImportSourceStore $sourceStore
         * @param LoggerInterface $logger
+        * @param User $user User to take conversion actions are (applicable 
for actions
+        *   where if there is no 'original' user)
         * @param Title[] $noConvertTemplates List of templates that flag pages 
that
         *  shouldn't be converted (optional)
         * @param string $headerSuffix Wikitext to add to the end of the header 
(optional)
@@ -67,12 +77,14 @@
                $parser,
                ImportSourceStore $sourceStore,
                LoggerInterface $logger,
+               User $user,
                array $noConvertTemplates = array(),
                $headerSuffix = null
        ) {
                $this->parser = $parser;
                $this->sourceStore = $sourceStore;
                $this->logger = $logger;
+               $this->user = $user;
                $this->noConvertTemplates = $noConvertTemplates;
                $this->headerSuffix = $headerSuffix;
 
@@ -124,7 +136,7 @@
         * {@inheritDoc}
         */
        public function createImportSource( Title $title ) {
-               return new ImportSource( $title, $this->parser, 
$this->headerSuffix );
+               return new ImportSource( $title, $this->parser, $this->user, 
$this->headerSuffix );
        }
 
        /**
diff --git a/includes/Import/Wikitext/ImportSource.php 
b/includes/Import/Wikitext/ImportSource.php
index 93f0d41..4257cbe 100644
--- a/includes/Import/Wikitext/ImportSource.php
+++ b/includes/Import/Wikitext/ImportSource.php
@@ -15,6 +15,7 @@
 use Revision;
 use StubObject;
 use Title;
+use User;
 
 /**
  * Imports the header of a wikitext talk page. Does not attempt to
@@ -22,19 +23,26 @@
  * ConversionStrategy for more details.
  */
 class ImportSource implements IImportSource {
+       /** @var User User doing the conversion actions (e.g. initial 
description, wikitext
+        *    archive edit).  However, actions will be attributed to the 
original user if
+        *    applicable.
+        */
+       protected $user;
 
        /**
         * @param Title $title
         * @param Parser|StubObject $parser
         * @param string $headerSuffix
+        * @param User $user User to take actions as
         * @throws ImportException When $title is an external title
         */
-       public function __construct( Title $title, $parser, $headerSuffix = 
null ) {
+       public function __construct( Title $title, $parser, User $user, 
$headerSuffix = null ) {
                if ( $title->isExternal() ) {
                        throw new ImportException( "Invalid non-local title: 
$title" );
                }
                $this->title = $title;
                $this->parser = $parser;
+               $this->user = $user;
                $this->headerSuffix = $headerSuffix;
        }
 
@@ -80,7 +88,7 @@
                        array( new ObjectRevision(
                                $content,
                                wfTimestampNow(),
-                               
FlowHooks::getOccupationController()->getTalkpageManager()->getName(),
+                               $this->user->getName(),
                                
"wikitext-import:header-revision:{$revision->getId()}"
                        ) ),
                        
"wikitext-import:header:{$this->title->getPrefixedText()}"
@@ -115,4 +123,3 @@
                return new ArrayIterator( array() );
        }
 }
-
diff --git a/includes/Specials/SpecialEnableFlow.php 
b/includes/Specials/SpecialEnableFlow.php
index 364d907..f495aac 100644
--- a/includes/Specials/SpecialEnableFlow.php
+++ b/includes/Specials/SpecialEnableFlow.php
@@ -91,11 +91,12 @@
                                wfGetDB( DB_MASTER ),
                                Container::get( 'importer' ),
                                $logger,
-                               
$this->occupationController->getTalkpageManager(),
+                               $this->getUser(),
                                new EnableFlowWikitextConversionStrategy(
                                        Container::get( 'parser' ),
                                        new NullImportSourceStore(),
                                        $logger,
+                                       $this->getUser(),
                                        array(),
                                        $data['header']
                                )
diff --git a/maintenance/convertLqtPageFromRemoteApiForTesting.php 
b/maintenance/convertLqtPageFromRemoteApiForTesting.php
index 25bdee9..5d482a8 100644
--- a/maintenance/convertLqtPageFromRemoteApiForTesting.php
+++ b/maintenance/convertLqtPageFromRemoteApiForTesting.php
@@ -79,7 +79,7 @@
 
                $logger->info( "Starting LQT conversion of page $srcPageName" );
 
-               $importer->import( $source, $dstTitle, $sourceStore );
+               $importer->import( $source, $dstTitle, $talkPageManagerUser, 
$sourceStore );
 
                $logger->info( "Finished LQT conversion of page $srcPageName" );
        }
diff --git a/maintenance/convertNamespaceFromWikitext.php 
b/maintenance/convertNamespaceFromWikitext.php
index a8d6d72..af54d69 100644
--- a/maintenance/convertNamespaceFromWikitext.php
+++ b/maintenance/convertNamespaceFromWikitext.php
@@ -69,15 +69,18 @@
                $logger = new MaintenanceDebugLogger( $this );
 
                $dbw = wfGetDB( DB_MASTER );
+               $talkpageManager = 
FlowHooks::getOccupationController()->getTalkpageManager();
                $converter = new \Flow\Import\Converter(
                        $dbw,
                        Flow\Container::get( 'importer' ),
                        $logger,
-                       
FlowHooks::getOccupationController()->getTalkpageManager(),
+                       $talkpageManager,
+
                        new Flow\Import\Wikitext\ConversionStrategy(
                                $wgParser,
                                new Flow\Import\NullImportSourceStore(),
                                $logger,
+                               $talkpageManager,
                                $noConvertTemplates,
                                $this->getOption( 'archive-pattern', null )
                        )
diff --git a/tests/phpunit/Import/TalkpageImportOperationTest.php 
b/tests/phpunit/Import/TalkpageImportOperationTest.php
index c56e4f1..a4a151c 100644
--- a/tests/phpunit/Import/TalkpageImportOperationTest.php
+++ b/tests/phpunit/Import/TalkpageImportOperationTest.php
@@ -113,7 +113,13 @@
                        )
                );
 
-               $op = new TalkpageImportOperation( $source, Container::get( 
'occupation_controller' ) );
+               $occupationController = Container::get( 'occupation_controller' 
);
+               $op = new TalkpageImportOperation(
+                       $source,
+                       $occupationController->getTalkpageManager(),
+                       $occupationController
+               );
+
                $store = new NullImportSourceStore;
                $op->import( new PageImportState(
                        $workflow,
diff --git a/tests/phpunit/Import/Wikitext/ConversionStrategyTest.php 
b/tests/phpunit/Import/Wikitext/ConversionStrategyTest.php
index 293bc7f..fa5985e 100644
--- a/tests/phpunit/Import/Wikitext/ConversionStrategyTest.php
+++ b/tests/phpunit/Import/Wikitext/ConversionStrategyTest.php
@@ -187,7 +187,8 @@
                return new ConversionStrategy(
                        $parser ?: $wgParser,
                        $sourceStore ?: new NullImportSourceStore,
-                       Container::get( 'default_logger' )
+                       Container::get( 'default_logger' ),
+                       Container::get( 'occupation_controller' 
)->getTalkpageManager()
                );
        }
 }
diff --git a/tests/phpunit/Import/Wikitext/ImportSourceTest.php 
b/tests/phpunit/Import/Wikitext/ImportSourceTest.php
index 8c1db61..6c215ee 100644
--- a/tests/phpunit/Import/Wikitext/ImportSourceTest.php
+++ b/tests/phpunit/Import/Wikitext/ImportSourceTest.php
@@ -4,6 +4,7 @@
 
 use DateTime;
 use DateTimeZone;
+use Flow\Container;
 use Flow\Exception\WikitextException;
 use Flow\Import\Wikitext\ImportSource;
 use Flow\Parsoid\Utils;
@@ -34,7 +35,9 @@
        /**
         * @dataProvider getHeaderProvider
         */
-       public function testGetHeader( $content, $expect ) {
+       public function testGetHeader( $content, $expectText ) {
+               $user = Container::get( 'occupation_controller' 
)->getTalkpageManager();
+
                // create a page with some content
                $status = WikiPage::factory( Title::newMainPage() )
                        ->doEditContent(
@@ -45,7 +48,12 @@
                        $this->fail( $status->getMessage()->plain() );
                }
 
-               $source = new ImportSource( Title::newMainPage(), new Parser );
+               $source = new ImportSource(
+                       Title::newMainPage(),
+                       new Parser,
+                       $user
+               );
+
                $header = $source->getHeader();
                $this->assertNotNull( $header );
                $this->assertGreaterThan( 1, strlen( $header->getObjectKey() ) 
);
@@ -55,7 +63,8 @@
 
                $revision = reset( $revisions );
                $this->assertInstanceOf( 'Flow\Import\IObjectRevision', 
$revision );
-               $this->assertEquals( $expect, $revision->getText() );
+               $this->assertEquals( $expectText, $revision->getText() );
+               $this->assertEquals( $user->getName(), $revision->getAuthor() );
        }
 
        public function getHeaderProvider() {

-- 
To view, visit https://gerrit.wikimedia.org/r/334744
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3636e637fe725f96fd80e57c55f6b0cbcea90744
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: REL1_26
Gerrit-Owner: Mattflaschen <mflasc...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to