From e75f2ab0f2d430015775e8c7fecc4f58aabeb974 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Fri, 26 Feb 2016 15:02:56 -0500 Subject: [PATCH] Improve SessionManager unit test coverage, and fix two namespacing bugs Change-Id: Ie0bcba77625e04ca3e89eb400626f63024c6e1a1 --- .../session/BotPasswordSessionProvider.php | 7 ++- includes/session/MetadataMergeException.php | 1 + includes/session/PHPSessionHandler.php | 2 + includes/session/SessionManager.php | 8 +++ .../BotPasswordSessionProviderTest.php | 54 +++++++++++++++++++ .../session/CookieSessionProviderTest.php | 21 ++++++++ .../session/MetadataMergeExceptionTest.php | 30 +++++++++++ .../includes/session/SessionBackendTest.php | 36 +++++++++++++ .../includes/session/SessionProviderTest.php | 5 ++ 9 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 tests/phpunit/includes/session/MetadataMergeExceptionTest.php diff --git a/includes/session/BotPasswordSessionProvider.php b/includes/session/BotPasswordSessionProvider.php index 44199bdec4..70c771dc31 100644 --- a/includes/session/BotPasswordSessionProvider.php +++ b/includes/session/BotPasswordSessionProvider.php @@ -165,16 +165,19 @@ class BotPasswordSessionProvider extends ImmutableSessionProviderWithCookie { return true; } + /** + * @codeCoverageIgnore + */ public function preventSessionsForUser( $username ) { BotPassword::removeAllPasswordsForUser( $username ); } public function getAllowedUserRights( SessionBackend $backend ) { if ( $backend->getProvider() !== $this ) { - throw new InvalidArgumentException( 'Backend\'s provider isn\'t $this' ); + throw new \InvalidArgumentException( 'Backend\'s provider isn\'t $this' ); } $data = $backend->getProviderMetadata(); - if ( $data ) { + if ( $data && isset( $data['rights'] ) && is_array( $data['rights'] ) ) { return $data['rights']; } diff --git a/includes/session/MetadataMergeException.php b/includes/session/MetadataMergeException.php index 9f42c278bc..882084d04e 100644 --- a/includes/session/MetadataMergeException.php +++ b/includes/session/MetadataMergeException.php @@ -22,6 +22,7 @@ namespace MediaWiki\Session; +use Exception; use UnexpectedValueException; /** diff --git a/includes/session/PHPSessionHandler.php b/includes/session/PHPSessionHandler.php index 8630809456..695ce5a92a 100644 --- a/includes/session/PHPSessionHandler.php +++ b/includes/session/PHPSessionHandler.php @@ -111,9 +111,11 @@ class PHPSessionHandler implements \SessionHandlerInterface { return; } + // @codeCoverageIgnoreStart if ( defined( 'MW_NO_SESSION_HANDLER' ) ) { throw new \BadMethodCallException( 'MW_NO_SESSION_HANDLER is defined' ); } + // @codeCoverageIgnoreEnd self::$instance = new self( $manager ); diff --git a/includes/session/SessionManager.php b/includes/session/SessionManager.php index b1d5d77e38..6a8b8a32ec 100644 --- a/includes/session/SessionManager.php +++ b/includes/session/SessionManager.php @@ -296,9 +296,11 @@ final class SessionManager implements SessionManagerInterface { } public function getVaryHeaders() { + // @codeCoverageIgnoreStart if ( defined( 'MW_NO_SESSION' ) && MW_NO_SESSION !== 'warn' ) { return []; } + // @codeCoverageIgnoreEnd if ( $this->varyHeaders === null ) { $headers = []; foreach ( $this->getProviders() as $provider ) { @@ -317,9 +319,11 @@ final class SessionManager implements SessionManagerInterface { } public function getVaryCookies() { + // @codeCoverageIgnoreStart if ( defined( 'MW_NO_SESSION' ) && MW_NO_SESSION !== 'warn' ) { return []; } + // @codeCoverageIgnoreEnd if ( $this->varyCookies === null ) { $cookies = []; foreach ( $this->getProviders() as $provider ) { @@ -513,12 +517,14 @@ final class SessionManager implements SessionManagerInterface { } # Notify AuthPlugin + // @codeCoverageIgnoreStart $tmpUser = $user; $wgAuth->initUser( $tmpUser, true ); if ( $tmpUser !== $user ) { $logger->warning( __METHOD__ . ': ' . get_class( $wgAuth ) . '::initUser() replaced the user object' ); } + // @codeCoverageIgnoreEnd # Notify hooks (e.g. Newuserlog) \Hooks::run( 'AuthPluginAutoCreate', [ $user ] ); @@ -957,6 +963,7 @@ final class SessionManager implements SessionManagerInterface { * @return Session */ public function getSessionFromInfo( SessionInfo $info, WebRequest $request ) { + // @codeCoverageIgnoreStart if ( defined( 'MW_NO_SESSION' ) ) { if ( MW_NO_SESSION === 'warn' ) { // Undocumented safety case for converting existing entry points @@ -965,6 +972,7 @@ final class SessionManager implements SessionManagerInterface { throw new \BadMethodCallException( 'Sessions are disabled for this entry point' ); } } + // @codeCoverageIgnoreEnd $id = $info->getId(); diff --git a/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php b/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php index b40a05cc97..590f2878ba 100644 --- a/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php +++ b/tests/phpunit/includes/session/BotPasswordSessionProviderTest.php @@ -281,4 +281,58 @@ class BotPasswordSessionProviderTest extends MediaWikiTestCase { $this->assertSame( [], $logger->getBuffer() ); $this->assertEquals( $dataMD + [ 'rights' => [ 'read' ] ], $metadata ); } + + public function testGetAllowedUserRights() { + $logger = new \TestLogger( true ); + $provider = $this->getProvider(); + $provider->setLogger( $logger ); + + $backend = TestUtils::getDummySessionBackend(); + $backendPriv = \TestingAccessWrapper::newFromObject( $backend ); + + try { + $provider->getAllowedUserRights( $backend ); + $this->fail( 'Expected exception not thrown' ); + } catch ( \InvalidArgumentException $ex ) { + $this->assertSame( 'Backend\'s provider isn\'t $this', $ex->getMessage() ); + } + + $backendPriv->provider = $provider; + $backendPriv->providerMetadata = [ 'rights' => [ 'foo', 'bar', 'baz' ] ]; + $this->assertSame( [ 'foo', 'bar', 'baz' ], $provider->getAllowedUserRights( $backend ) ); + $this->assertSame( [], $logger->getBuffer() ); + + $backendPriv->providerMetadata = [ 'foo' => 'bar' ]; + $this->assertSame( [], $provider->getAllowedUserRights( $backend ) ); + $this->assertSame( [ + [ + LogLevel::DEBUG, + 'MediaWiki\\Session\\BotPasswordSessionProvider::getAllowedUserRights: ' . + 'No provider metadata, returning no rights allowed' + ] + ], $logger->getBuffer() ); + $logger->clearBuffer(); + + $backendPriv->providerMetadata = [ 'rights' => 'bar' ]; + $this->assertSame( [], $provider->getAllowedUserRights( $backend ) ); + $this->assertSame( [ + [ + LogLevel::DEBUG, + 'MediaWiki\\Session\\BotPasswordSessionProvider::getAllowedUserRights: ' . + 'No provider metadata, returning no rights allowed' + ] + ], $logger->getBuffer() ); + $logger->clearBuffer(); + + $backendPriv->providerMetadata = null; + $this->assertSame( [], $provider->getAllowedUserRights( $backend ) ); + $this->assertSame( [ + [ + LogLevel::DEBUG, + 'MediaWiki\\Session\\BotPasswordSessionProvider::getAllowedUserRights: ' . + 'No provider metadata, returning no rights allowed' + ] + ], $logger->getBuffer() ); + $logger->clearBuffer(); + } } diff --git a/tests/phpunit/includes/session/CookieSessionProviderTest.php b/tests/phpunit/includes/session/CookieSessionProviderTest.php index d376159dce..a52aa4bbbe 100644 --- a/tests/phpunit/includes/session/CookieSessionProviderTest.php +++ b/tests/phpunit/includes/session/CookieSessionProviderTest.php @@ -762,4 +762,25 @@ class CookieSessionProviderTest extends MediaWikiTestCase { public function onUserSetCookies( $user, &$sessionData, &$cookies ) { } + public function testGetCookie() { + $provider = new CookieSessionProvider( [ + 'priority' => 1, + 'sessionName' => 'MySessionName', + 'cookieOptions' => [ 'prefix' => 'x' ], + ] ); + $provider->setLogger( new \Psr\Log\NullLogger() ); + $provider->setConfig( $this->getConfig() ); + $provider->setManager( SessionManager::singleton() ); + $provider = \TestingAccessWrapper::newFromObject( $provider ); + + $request = new \FauxRequest(); + $request->setCookies( [ + 'xFoo' => 'foo!', + 'xBar' => 'deleted', + ], '' ); + $this->assertSame( 'foo!', $provider->getCookie( $request, 'Foo', 'x' ) ); + $this->assertNull( $provider->getCookie( $request, 'Bar', 'x' ) ); + $this->assertNull( $provider->getCookie( $request, 'Baz', 'x' ) ); + } + } diff --git a/tests/phpunit/includes/session/MetadataMergeExceptionTest.php b/tests/phpunit/includes/session/MetadataMergeExceptionTest.php new file mode 100644 index 0000000000..0981f026a1 --- /dev/null +++ b/tests/phpunit/includes/session/MetadataMergeExceptionTest.php @@ -0,0 +1,30 @@ + 'bar' ]; + + $ex = new MetadataMergeException(); + $this->assertInstanceOf( 'UnexpectedValueException', $ex ); + $this->assertSame( [], $ex->getContext() ); + + $ex2 = new MetadataMergeException( 'Message', 42, $ex, $data ); + $this->assertSame( 'Message', $ex2->getMessage() ); + $this->assertSame( 42, $ex2->getCode() ); + $this->assertSame( $ex, $ex2->getPrevious() ); + $this->assertSame( $data, $ex2->getContext() ); + + $ex->setContext( $data ); + $this->assertSame( $data, $ex->getContext() ); + } + +} diff --git a/tests/phpunit/includes/session/SessionBackendTest.php b/tests/phpunit/includes/session/SessionBackendTest.php index 5824ce10bb..54ad0f4f8b 100644 --- a/tests/phpunit/includes/session/SessionBackendTest.php +++ b/tests/phpunit/includes/session/SessionBackendTest.php @@ -216,6 +216,42 @@ class SessionBackendTest extends MediaWikiTestCase { $this->assertArrayHasKey( $backend->getId(), $manager->allSessionIds ); } + public function testSetProviderMetadata() { + $backend = $this->getBackend(); + $priv = \TestingAccessWrapper::newFromObject( $backend ); + $priv->providerMetadata = [ 'dummy' ]; + + try { + $backend->setProviderMetadata( 'foo' ); + $this->fail( 'Expected exception not thrown' ); + } catch ( \InvalidArgumentException $ex ) { + $this->assertSame( '$metadata must be an array or null', $ex->getMessage() ); + } + + try { + $backend->setProviderMetadata( (object)[] ); + $this->fail( 'Expected exception not thrown' ); + } catch ( \InvalidArgumentException $ex ) { + $this->assertSame( '$metadata must be an array or null', $ex->getMessage() ); + } + + $this->assertFalse( $this->store->getSession( self::SESSIONID ), 'sanity check' ); + $backend->setProviderMetadata( [ 'dummy' ] ); + $this->assertFalse( $this->store->getSession( self::SESSIONID ) ); + + $this->assertFalse( $this->store->getSession( self::SESSIONID ), 'sanity check' ); + $backend->setProviderMetadata( [ 'test' ] ); + $this->assertNotFalse( $this->store->getSession( self::SESSIONID ) ); + $this->assertSame( [ 'test' ], $backend->getProviderMetadata() ); + $this->store->deleteSession( self::SESSIONID ); + + $this->assertFalse( $this->store->getSession( self::SESSIONID ), 'sanity check' ); + $backend->setProviderMetadata( null ); + $this->assertNotFalse( $this->store->getSession( self::SESSIONID ) ); + $this->assertSame( null, $backend->getProviderMetadata() ); + $this->store->deleteSession( self::SESSIONID ); + } + public function testResetId() { $id = session_id(); diff --git a/tests/phpunit/includes/session/SessionProviderTest.php b/tests/phpunit/includes/session/SessionProviderTest.php index 24b97165bd..e92eb09d1f 100644 --- a/tests/phpunit/includes/session/SessionProviderTest.php +++ b/tests/phpunit/includes/session/SessionProviderTest.php @@ -128,6 +128,11 @@ class SessionProviderTest extends MediaWikiTestCase { $provider->preventSessionsForUser( 'Foo' ); $this->fail( 'Expected exception not thrown' ); } catch ( \BadMethodCallException $ex ) { + $this->assertSame( + 'MediaWiki\\Session\\SessionProvider::preventSessionsForUser must be implmented ' . + 'when canChangeUser() is false', + $ex->getMessage() + ); } } -- 2.20.1