[SECURITY] [API BREAKING CHANGE] Require logout token.

Special:Userlogout now requires a token

Api action=logout requires a csrf token and the request to be POSTed

Patch author: bawolff

Bug: T25227
Change-Id: Icb674095956bb3f6c847c9553c53e404402ea774

RELEASE-NOTES-1.33

@@ -171,6 +171,7 @@ For notes on 1.32.x and older releases, see HISTORY.
 * (T216245) Autoblocks will now be spread by action=edit and action=move.
 * action=query&meta=userinfo has a new uiprop, 'latestcontrib', that returns
   the date of user's latest contribution.
+* (T25227) action=logout now requires to be posted and have a csrf token.
 
 === Action API internal changes in 1.33 ===
 * A number of deprecated methods for API documentation, intended for overriding

includes/api/ApiLogout.php

@@ -59,13 +59,21 @@ class ApiLogout extends ApiBase {
 		Hooks::run( 'UserLogoutComplete', [ &$user, &$injected_html, $oldName ] );
 	}
 
+	public function mustBePosted() {
+		return true;
+	}
+
+	public function needsToken() {
+		return 'csrf';
+	}
+
 	public function isReadMode() {
 		return false;
 	}
 
 	protected function getExamplesMessages() {
 		return [
-			'action=logout'
+			'action=logout&token=123ABC'
 				=> 'apihelp-logout-example-logout',
 		];
 	}

includes/skins/SkinTemplate.php

@@ -617,16 +617,18 @@ class SkinTemplate extends Skin {
 			$page = Title::newFromText( $request->getVal( 'title', '' ) );
 		}
 		$page = $request->getVal( 'returnto', $page );
-		$a = [];
+		$returnto = [];
 		if ( strval( $page ) !== '' ) {
-			$a['returnto'] = $page;
+			$returnto['returnto'] = $page;
 			$query = $request->getVal( 'returntoquery', $this->thisquery );
+			$paramsArray = wfCgiToArray( $query );
+			unset( $paramsArray['logoutToken'] );
+			$query = wfArrayToCgi( $paramsArray );
 			if ( $query != '' ) {
-				$a['returntoquery'] = $query;
+				$returnto['returntoquery'] = $query;
 			}
 		}
 
-		$returnto = wfArrayToCgi( $a );
 		if ( $this->loggedin ) {
 			$personal_urls['userpage'] = [
 				'text' => $this->username,
@@ -691,9 +693,10 @@ class SkinTemplate extends Skin {
 				$personal_urls['logout'] = [
 					'text' => $this->msg( 'pt-userlogout' )->text(),
 					'href' => self::makeSpecialUrl( 'Userlogout',
-						// userlogout link must always contain an & character, otherwise we might not be able
+						// Note: userlogout link must always contain an & character, otherwise we might not be able
 						// to detect a buggy precaching proxy (T19790)
-						$title->isSpecial( 'Preferences' ) ? 'noreturnto' : $returnto ),
+						( $title->isSpecial( 'Preferences' ) ? [] : $returnto )
+						+ [ 'logoutToken' => $this->getUser()->getEditToken( 'logoutToken', $this->getRequest() ) ] ),
 					'active' => false
 				];
 			}

includes/specials/SpecialUserLogout.php

@@ -48,6 +48,28 @@ class SpecialUserLogout extends UnlistedSpecialPage {
 		$this->setHeaders();
 		$this->outputHeader();
 
+		$out = $this->getOutput();
+		$user = $this->getUser();
+		$request = $this->getRequest();
+
+		$logoutToken = $request->getVal( 'logoutToken' );
+		$urlParams = [
+			'logoutToken' => $user->getEditToken( 'logoutToken', $request )
+		] + $request->getValues();
+		unset( $urlParams['title'] );
+		$continueLink = $this->getFullTitle()->getFullUrl( $urlParams );
+
+		if ( $logoutToken === null ) {
+			$this->getOutput()->addWikiMsg( 'userlogout-continue', $continueLink );
+			return;
+		}
+		if ( !$this->getUser()->matchEditToken(
+			$logoutToken, 'logoutToken', $this->getRequest(), 24 * 60 * 60
+		) ) {
+			$this->getOutput()->addWikiMsg( 'userlogout-sessionerror', $continueLink );
+			return;
+		}
+
 		// Make sure it's possible to log out
 		$session = MediaWiki\Session\SessionManager::getGlobalSession();
 		if ( !$session->canSetUser() ) {

languages/i18n/en.json

@@ -4220,5 +4220,7 @@
 	"passwordpolicies-policyflag-forcechange": "must change on login",
 	"passwordpolicies-policyflag-suggestchangeonlogin": "suggest change on login",
 	"easydeflate-invaliddeflate": "Content provided is not properly deflated",
-	"unprotected-js": "For security reasons JavaScript cannot be loaded from unprotected pages. Please only create javascript in the MediaWiki: namespace or as a User subpage"
+	"unprotected-js": "For security reasons JavaScript cannot be loaded from unprotected pages. Please only create javascript in the MediaWiki: namespace or as a User subpage",
+	"userlogout-continue": "If you wish to log out please [$1 continue to the log out page].",
+	"userlogout-sessionerror": "Log out failed due to session error. Please [$1 try again]."
 }

languages/i18n/qqq.json

@@ -4427,5 +4427,7 @@
 	"passwordpolicies-policyflag-forcechange": "Password policy flag that enforces changing invalid passwords on login.",
 	"passwordpolicies-policyflag-suggestchangeonlogin": "Password policy flag that suggests changing invalid passwords on login.",
 	"easydeflate-invaliddeflate": "Error message if the content passed to easydeflate was not deflated (compressed) properly",
-	"unprotected-js": "Error message shown when trying to load javascript via action=raw that is not protected"
+	"unprotected-js": "Error message shown when trying to load javascript via action=raw that is not protected",
+	"userlogout-continue": "Shown if user attempted to log out without a token specified. Probably the user clicked on an old link that hasn't been updated to use the new system. $1 - url that user should click on in order to log out.",
+	"userlogout-sessionerror": "Shown when a user tries to log out with an invalid token. $1 is url with correct token that user should click."
 }

tests/phpunit/includes/api/ApiLogoutTest.php

@@ -0,0 +1,55 @@
+<?php
+
+/**
+ * @group API
+ * @group Database
+ * @group medium
+ *
+ * @covers ApiLogout
+ */
+class ApiLogoutTest extends ApiTestCase {
+	public function setUp() {
+		parent::setUp();
+	}
+
+	public function testUserLogoutBadToken() {
+		try {
+			$token = 'invalid token';
+			$retLogout = $this->doUserLogout( $token );
+		}
+		catch ( ApiUsageException $e ) {
+			$exceptionMsg = $e->getMessage();
+		}
+
+		$this->assertSame( "Invalid CSRF token.", $exceptionMsg );
+	}
+
+	public function testUserLogout() {
+		// TODO: there has to be a cleaner way to make User::doLogout happy
+		global $wgUser;
+		$wgUser = User::newFromId( '127.0.0.1' );
+
+		$token = $this->getUserCsrfTokenFromApi();
+		$retLogout = $this->doUserLogout( $token );
+		$this->assertFalse( $wgUser->isLoggedIn() );
+	}
+
+	public function getUserCsrfTokenFromApi() {
+		$retToken = $this->doApiRequest( [
+			'action' => 'query',
+			'meta' => 'tokens',
+			'type' => 'csrf'
+		] );
+
+		$this->assertArrayNotHasKey( 'warnings', $retToken );
+
+		return $retToken[0]['query']['tokens']['csrftoken'];
+	}
+
+	public function doUserLogout( $logoutToken ) {
+		return $this->doApiRequest( [
+			'action' => 'logout',
+			'token' => $logoutToken
+		] );
+	}
+}