ApiLogout: Follow up Icb674095
authorBrad Jorsch <bjorsch@wikimedia.org>
Thu, 25 Apr 2019 13:49:01 +0000 (09:49 -0400)
committerReedy <reedy@wikimedia.org>
Thu, 25 Apr 2019 14:50:14 +0000 (15:50 +0100)
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

includes/api/ApiLogout.php
tests/phpunit/includes/api/ApiLogoutTest.php

index 39a96ac..d549e75 100644 (file)
@@ -67,6 +67,10 @@ class ApiLogout extends ApiBase {
                return 'csrf';
        }
 
+       protected function getWebUITokenSalt( array $params ) {
+               return 'logoutToken';
+       }
+
        public function isReadMode() {
                return false;
        }
index fcdb745..8254fdb 100644 (file)
@@ -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