From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Thu, 25 Apr 2019 13:49:01 +0000 (-0400)
Subject: ApiLogout: Follow up Icb674095
X-Git-Tag: 1.31.2~17
X-Git-Url: http://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=f1e85dde176d152498c6c677cdb08a26e3492497

ApiLogout: Follow up Icb674095

This implements getWebUITokenSalt(), as mentioned in T25227#2008199 and
implemented in F3328897. Somehow it didn't make it into Icb674095.

This also fixes some issues in the unit test:
* Properly link the user to the request's Session so User::doLogout()
  won't log a warning. This also gives use to the otherwise-unneeded
  implementation of setUp(), and lets us get rid of the broken call to
  User::newFromId() that was passing an IP address rather than a user ID.
* Privatize some internal methods.
* Use setExpectedApiException() instead of manually catching and
  hard-coding the English exception message.
* Also assert that the bad token error didn't result in a logout.

Bug: T25227
Change-Id: I2aecfba821cca3c367c5e7e8d188a88197fb82d2
---

diff --git a/includes/api/ApiLogout.php b/includes/api/ApiLogout.php
index 39a96ac563..d549e75fe4 100644
--- a/includes/api/ApiLogout.php
+++ b/includes/api/ApiLogout.php
@@ -67,6 +67,10 @@ class ApiLogout extends ApiBase {
 		return 'csrf';
 	}
 
+	protected function getWebUITokenSalt( array $params ) {
+		return 'logoutToken';
+	}
+
 	public function isReadMode() {
 		return false;
 	}
diff --git a/tests/phpunit/includes/api/ApiLogoutTest.php b/tests/phpunit/includes/api/ApiLogoutTest.php
index fcdb745aaa..8254fdba7d 100644
--- a/tests/phpunit/includes/api/ApiLogoutTest.php
+++ b/tests/phpunit/includes/api/ApiLogoutTest.php
@@ -8,33 +8,53 @@
  * @covers ApiLogout
  */
 class ApiLogoutTest extends ApiTestCase {
-	public function setUp() {
+
+	protected function setUp() {
+		global $wgRequest, $wgUser;
+
 		parent::setUp();
+
+		// Link the user to the Session properly so User::doLogout() doesn't complain.
+		$wgRequest->getSession()->setUser( $wgUser );
+		$wgUser = User::newFromSession( $wgRequest );
+		$this->apiContext->setUser( $wgUser );
 	}
 
 	public function testUserLogoutBadToken() {
+		global $wgUser;
+
+		$this->setExpectedApiException( 'apierror-badtoken' );
+
 		try {
 			$token = 'invalid token';
-			$retLogout = $this->doUserLogout( $token );
-		}
-		catch ( ApiUsageException $e ) {
-			$exceptionMsg = $e->getMessage();
+			$this->doUserLogout( $token );
+		} finally {
+			$this->assertTrue( $wgUser->isLoggedIn(), 'not logged out' );
 		}
-
-		$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' );
 
+		$this->assertTrue( $wgUser->isLoggedIn(), 'sanity check' );
 		$token = $this->getUserCsrfTokenFromApi();
-		$retLogout = $this->doUserLogout( $token );
+		$this->doUserLogout( $token );
+		$this->assertFalse( $wgUser->isLoggedIn() );
+	}
+
+	public function testUserLogoutWithWebToken() {
+		global $wgUser, $wgRequest;
+
+		$this->assertTrue( $wgUser->isLoggedIn(), 'sanity check' );
+
+		// Logic copied from SkinTemplate.
+		$token = $wgUser->getEditToken( 'logoutToken', $wgRequest );
+
+		$this->doUserLogout( $token );
 		$this->assertFalse( $wgUser->isLoggedIn() );
 	}
 
-	public function getUserCsrfTokenFromApi() {
+	private function getUserCsrfTokenFromApi() {
 		$retToken = $this->doApiRequest( [
 			'action' => 'query',
 			'meta' => 'tokens',
@@ -46,7 +66,7 @@ class ApiLogoutTest extends ApiTestCase {
 		return $retToken[0]['query']['tokens']['csrftoken'];
 	}
 
-	public function doUserLogout( $logoutToken ) {
+	private function doUserLogout( $logoutToken ) {
 		return $this->doApiRequest( [
 			'action' => 'logout',
 			'token' => $logoutToken