From f61cb18b71dac4b8117c36c4b54653742f6e118c Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Thu, 18 Feb 2016 15:56:40 -0500 Subject: [PATCH] Enforce MW_NO_SESSION, add MW_NO_SESSION_HANDLER When an entry point specifies MW_NO_SESSION, actually enforce that by having both SessionManager and PHP's session handling (session_start() and friends) throw exceptions. If an entry point needs the old behavior of using PHP's default session handling (as defined in php.ini), it should define MW_NO_SESSION_HANDLER instead of or in addition to MW_NO_SESSION. This also makes PHPSessionHandler be installed in CLI mode, where it wasn't installed before. Bug: T127233 Change-Id: I2a3db06ee8e44a044096c57a819b5fd5e51c5c5c --- includes/DefaultSettings.php | 8 ++++++++ includes/GlobalFunctions.php | 6 ------ includes/Setup.php | 26 ++++++++++++++++++++++---- includes/installer/Installer.php | 9 ++++++++- includes/session/PHPSessionHandler.php | 4 ++++ includes/session/SessionManager.php | 9 +++++++++ includes/user/User.php | 26 +++++++++++++++++--------- 7 files changed, 68 insertions(+), 20 deletions(-) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 08538eebf2..da8eed5652 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -2294,6 +2294,14 @@ $wgSessionHandler = null; /** * Whether to use PHP session handling ($_SESSION and session_*() functions) + * + * If the constant MW_NO_SESSION is defined, this is forced to 'disable'. + * + * If the constant MW_NO_SESSION_HANDLER is defined, this is ignored and PHP + * session handling will function independently of SessionHandler. + * SessionHandler and PHP's session handling may attempt to override each + * others' cookies. + * * @since 1.27 * @var string * - 'enable': Integrate with PHP's session handling as much as possible. diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index ac1dd6d049..7a41f1115e 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -3046,12 +3046,6 @@ function wfResetSessionID() { function wfSetupSession( $sessionId = false ) { wfDeprecated( __FUNCTION__, '1.27' ); - // If they're calling this, they probably want our session management even - // if NO_SESSION was set for Setup.php. - if ( !MediaWiki\Session\PHPSessionHandler::isInstalled() ) { - MediaWiki\Session\PHPSessionHandler::install( SessionManager::singleton() ); - } - if ( $sessionId ) { session_id( $sessionId ); } diff --git a/includes/Setup.php b/includes/Setup.php index 4854727d68..47fb73e143 100644 --- a/includes/Setup.php +++ b/includes/Setup.php @@ -517,6 +517,11 @@ if ( $wgPHPSessionHandling !== 'enable' && ) { $wgPHPSessionHandling = 'warn'; } +if ( defined( 'MW_NO_SESSION' ) ) { + // If the entry point wants no session, force 'disable' here unless they + // specifically set it to the (undocumented) 'warn'. + $wgPHPSessionHandling = MW_NO_SESSION === 'warn' ? 'warn' : 'disable'; +} Profiler::instance()->scopedProfileOut( $ps_default ); @@ -702,10 +707,13 @@ if ( !defined( 'MW_NO_SESSION' ) && !$wgCommandLineMode ) { session_name( $wgSessionName ? $wgSessionName : $wgCookiePrefix . '_session' ); } - // Create the SessionManager singleton and set up our session handler - MediaWiki\Session\PHPSessionHandler::install( - MediaWiki\Session\SessionManager::singleton() - ); + // Create the SessionManager singleton and set up our session handler, + // unless we're specifically asked not to. + if ( !defined( 'MW_NO_SESSION_HANDLER' ) ) { + MediaWiki\Session\PHPSessionHandler::install( + MediaWiki\Session\SessionManager::singleton() + ); + } // Initialize the session try { @@ -740,6 +748,16 @@ if ( !defined( 'MW_NO_SESSION' ) && !$wgCommandLineMode ) { session_id( $session->getId() ); MediaWiki\quietCall( 'session_start' ); } + + unset( $session ); +} else { + // Even if we didn't set up a global Session, still install our session + // handler unless specifically requested not to. + if ( !defined( 'MW_NO_SESSION_HANDLER' ) ) { + MediaWiki\Session\PHPSessionHandler::install( + MediaWiki\Session\SessionManager::singleton() + ); + } } Profiler::instance()->scopedProfileOut( $ps_session ); diff --git a/includes/installer/Installer.php b/includes/installer/Installer.php index 7ebab67eb9..70fa857691 100644 --- a/includes/installer/Installer.php +++ b/includes/installer/Installer.php @@ -1715,7 +1715,9 @@ abstract class Installer { * Override the necessary bits of the config to run an installation. */ public static function overrideConfig() { - define( 'MW_NO_SESSION', 1 ); + // Use PHP's built-in session handling, since MediaWiki's + // SessionHandler can't work before we have an object cache set up. + define( 'MW_NO_SESSION_HANDLER', 1 ); // Don't access the database $GLOBALS['wgUseDatabaseMessages'] = false; @@ -1739,6 +1741,8 @@ abstract class Installer { // Some of the environment checks make shell requests, remove limits $GLOBALS['wgMaxShellMemory'] = 0; + // Override the default CookieSessionProvider with a dummy + // implementation that won't stomp on PHP's cookies. $GLOBALS['wgSessionProviders'] = [ [ 'class' => 'InstallerSessionProvider', @@ -1747,6 +1751,9 @@ abstract class Installer { ] ] ] ]; + + // Don't try to use any object cache for SessionManager either. + $GLOBALS['wgSessionCacheType'] = CACHE_NONE; } /** diff --git a/includes/session/PHPSessionHandler.php b/includes/session/PHPSessionHandler.php index 93b0b36c07..8630809456 100644 --- a/includes/session/PHPSessionHandler.php +++ b/includes/session/PHPSessionHandler.php @@ -111,6 +111,10 @@ class PHPSessionHandler implements \SessionHandlerInterface { return; } + if ( defined( 'MW_NO_SESSION_HANDLER' ) ) { + throw new \BadMethodCallException( 'MW_NO_SESSION_HANDLER is defined' ); + } + self::$instance = new self( $manager ); // Close any auto-started session, before we replace it diff --git a/includes/session/SessionManager.php b/includes/session/SessionManager.php index 0abec1be4e..14bc75204c 100644 --- a/includes/session/SessionManager.php +++ b/includes/session/SessionManager.php @@ -953,6 +953,15 @@ final class SessionManager implements SessionManagerInterface { * @return Session */ public function getSessionFromInfo( SessionInfo $info, WebRequest $request ) { + if ( defined( 'MW_NO_SESSION' ) ) { + if ( MW_NO_SESSION === 'warn' ) { + // Undocumented safety case for converting existing entry points + $this->logger->error( 'Sessions are supposed to be disabled for this entry point' ); + } else { + throw new \BadMethodCallException( 'Sessions are disabled for this entry point' ); + } + } + $id = $info->getId(); if ( !isset( $this->allSessionBackends[$id] ) ) { diff --git a/includes/user/User.php b/includes/user/User.php index 31f68079d4..eb3853a45d 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -1111,7 +1111,8 @@ class User implements IDBAccessObject { $this->mOptionOverrides = null; $this->mOptionsLoaded = false; - $loggedOut = $this->mRequest ? $this->mRequest->getSession()->getLoggedOutTimestamp() : 0; + $loggedOut = $this->mRequest && !defined( 'MW_NO_SESSION' ) + ? $this->mRequest->getSession()->getLoggedOutTimestamp() : 0; if ( $loggedOut !== 0 ) { $this->mTouched = wfTimestamp( TS_MW, $loggedOut ); } else { @@ -3080,9 +3081,13 @@ class User implements IDBAccessObject { if ( is_null( $this->mRights ) ) { $this->mRights = self::getGroupPermissions( $this->getEffectiveGroups() ); - $allowedRights = $this->getRequest()->getSession()->getAllowedUserRights(); - if ( $allowedRights !== null ) { - $this->mRights = array_intersect( $this->mRights, $allowedRights ); + // Deny any rights denied by the user's session, unless this + // endpoint has no sessions. + if ( !defined( 'MW_NO_SESSION' ) ) { + $allowedRights = $this->getRequest()->getSession()->getAllowedUserRights(); + if ( $allowedRights !== null ) { + $this->mRights = array_intersect( $this->mRights, $allowedRights ); + } } Hooks::run( 'UserGetRights', [ $this, &$this->mRights ] ); @@ -4605,11 +4610,14 @@ class User implements IDBAccessObject { } } - // Remove any rights that aren't allowed to the global-session user - $allowedRights = SessionManager::getGlobalSession()->getAllowedUserRights(); - if ( $allowedRights !== null && !in_array( $right, $allowedRights, true ) ) { - $cache[$right] = false; - return false; + // Remove any rights that aren't allowed to the global-session user, + // unless there are no sessions for this endpoint. + if ( !defined( 'MW_NO_SESSION' ) ) { + $allowedRights = SessionManager::getGlobalSession()->getAllowedUserRights(); + if ( $allowedRights !== null && !in_array( $right, $allowedRights, true ) ) { + $cache[$right] = false; + return false; + } } // Allow extensions to say false -- 2.20.1