r78165 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r78164‎ | r78165 | r78166 >
Date:03:02, 10 December 2010
Author:tstarling
Status:ok
Tags:
Comment:
* Hide the "back" buttons on the completion pages, they are potentially confusing and almost useless.
* Made the links to the wiki on the completion pages open in the same window, not a popup.
* Do not allow the user to regenerate LocalSettings.php when $wgUpgradeKey was given and the DB settings were prefilled, since this allows a leak of $wgUpgradeKey to escalate to a leak of $wgDBpassword. It's not unreasonable to require that the user removes their old LocalSettings.php when they wish to generate a new one.
* Rewrote the doc comment on $wgUpgradeKey, to discourage users from setting it to an easily guessable string, per concerns on CR r78118.
Modified paths:
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/installer/Installer.i18n.php (modified) (history)
  • /trunk/phase3/includes/installer/Installer.php (modified) (history)
  • /trunk/phase3/includes/installer/WebInstallerPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/installer/Installer.php
@@ -324,6 +324,19 @@
325325 return $html;
326326 }
327327
 328+ public function getParserOptions() {
 329+ return $this->parserOptions;
 330+ }
 331+
 332+ public function disableLinkPopups() {
 333+ $this->parserOptions->setExternalLinkTarget( false );
 334+ }
 335+
 336+ public function restoreLinkPopups() {
 337+ global $wgExternalLinkTarget;
 338+ $this->parserOptions->setExternalLinkTarget( $wgExternalLinkTarget );
 339+ }
 340+
328341 /**
329342 * TODO: document
330343 *
Index: trunk/phase3/includes/installer/Installer.i18n.php
@@ -284,6 +284,9 @@
285285
286286 If you want to regenerate your <code>LocalSettings.php</code> file, click the button below.
287287 This is '''not recommended''' unless you are having problems with your wiki.",
 288+ 'config-upgrade-done-no-regenerate' => "Upgrade complete.
 289+
 290+You can now [$1 start using your wiki].",
288291 'config-regenerate' => 'Regenerate LocalSettings.php →',
289292 'config-show-table-status' => 'SHOW TABLE STATUS query failed!',
290293 'config-unknown-collation' => "'''Warning:''' Database is using unrecognised collation.",
Index: trunk/phase3/includes/installer/WebInstallerPage.php
@@ -49,7 +49,7 @@
5050 );
5151 }
5252
53 - public function endForm( $continue = 'continue' ) {
 53+ public function endForm( $continue = 'continue', $back = 'back' ) {
5454 $s = "<div class=\"config-submit\">\n";
5555 $id = $this->getId();
5656
@@ -63,10 +63,10 @@
6464 array( 'name' => "enter-$continue", 'style' => 'visibility:hidden;overflow:hidden;width:1px;margin:0' ) ) . "\n";
6565 }
6666
67 - if ( $id !== 0 ) {
68 - $s .= Xml::submitButton( wfMsg( 'config-back' ),
 67+ if ( $back ) {
 68+ $s .= Xml::submitButton( wfMsg( "config-$back" ),
6969 array(
70 - 'name' => 'submit-back',
 70+ 'name' => "submit-$back",
7171 'tabindex' => $this->parent->nextTabIndex()
7272 ) ) . "\n";
7373 }
@@ -172,7 +172,7 @@
173173 $this->getLanguageSelector( 'UserLang', 'config-your-language', $userLang, $this->parent->getHelpBox( 'config-your-language-help' ) ) .
174174 $this->getLanguageSelector( 'ContLang', 'config-wiki-language', $contLang, $this->parent->getHelpBox( 'config-wiki-language-help' ) );
175175 $this->addHTML( $s );
176 - $this->endForm();
 176+ $this->endForm( 'continue', false );
177177 }
178178
179179 /**
@@ -435,7 +435,10 @@
436436
437437 public function execute() {
438438 if ( $this->getVar( '_UpgradeDone' ) ) {
439 - if ( $this->parent->request->wasPosted() ) {
 439+ // Allow regeneration of LocalSettings.php, unless we are working
 440+ // from a pre-existing LocalSettings.php file and we want to avoid
 441+ // leaking its contents
 442+ if ( $this->parent->request->wasPosted() && !$this->getVar( '_ExistingDBSettings' ) ) {
440443 // Done message acknowledged
441444 return 'continue';
442445 } else {
@@ -483,16 +486,24 @@
484487
485488 public function showDoneMessage() {
486489 $this->startForm();
 490+ $regenerate = !$this->getVar( '_ExistingDBSettings' );
 491+ if ( $regenerate ) {
 492+ $msg = 'config-upgrade-done';
 493+ } else {
 494+ $msg = 'config-upgrade-done-no-regenerate';
 495+ }
 496+ $this->parent->disableLinkPopups();
487497 $this->addHTML(
488498 $this->parent->getInfoBox(
489 - wfMsgNoTrans( 'config-upgrade-done',
 499+ wfMsgNoTrans( $msg,
490500 $GLOBALS['wgServer'] .
491501 $this->getVar( 'wgScriptPath' ) . '/index' .
492502 $this->getVar( 'wgScriptExtension' )
493503 ), 'tick-32.png'
494504 )
495505 );
496 - $this->endForm( 'regenerate' );
 506+ $this->parent->restoreLinkPopups();
 507+ $this->endForm( $regenerate ? 'regenerate' : false, false );
497508 }
498509
499510 }
@@ -1029,6 +1040,7 @@
10301041 $this->parent->request->response()->header( "Refresh: 0;$lsUrl" );
10311042
10321043 $this->startForm();
 1044+ $this->parent->disableLinkPopups();
10331045 $this->addHTML(
10341046 $this->parent->getInfoBox(
10351047 wfMsgNoTrans( 'config-install-done',
@@ -1040,7 +1052,8 @@
10411053 ), 'tick-32.png'
10421054 )
10431055 );
1044 - $this->endForm( false );
 1056+ $this->parent->restoreLinkPopups();
 1057+ $this->endForm( false, false );
10451058 }
10461059 }
10471060
Index: trunk/phase3/includes/DefaultSettings.php
@@ -4132,7 +4132,13 @@
41334133 $wgReadOnlyFile = false;
41344134
41354135 /**
4136 - * Set this to a random string to allow web-based upgrades
 4136+ * When you run the web-based upgrade utility, it will tell you what to set
 4137+ * this to in order to authorize the upgrade process. It will subsequently be
 4138+ * used as a password, to authorize further upgrades.
 4139+ *
 4140+ * For security, do not set this to a guessable string. Use the value supplied
 4141+ * by the install/upgrade process. To cause the upgrader to generate a new key,
 4142+ * delete the old key from LocalSettings.php.
41374143 */
41384144 $wgUpgradeKey = false;
41394145

Follow-up revisions

RevisionCommit summaryAuthorDate
r784371.17: Merge tagged revisions from trunk: r77878, r77981, r77982, r77994, r780...catrope14:14, 15 December 2010
r79508Remove testOnlyBackButtonAvailability() function as per the code review comme...nadeesha09:05, 3 January 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r78118* Made the web upgrade process more friendly. Instead of saying "access denie...tstarling08:24, 9 December 2010

Status & tagging log