Enforce MW_NO_SESSION, add MW_NO_SESSION_HANDLER
authorBrad Jorsch <bjorsch@wikimedia.org>
Thu, 18 Feb 2016 20:56:40 +0000 (15:56 -0500)
committerBrad Jorsch <bjorsch@wikimedia.org>
Mon, 22 Feb 2016 17:17:31 +0000 (12:17 -0500)
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
includes/GlobalFunctions.php
includes/Setup.php
includes/installer/Installer.php
includes/session/PHPSessionHandler.php
includes/session/SessionManager.php
includes/user/User.php

index 08538ee..da8eed5 100644 (file)
@@ -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.
index ac1dd6d..7a41f11 100644 (file)
@@ -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 );
        }
index 4854727..47fb73e 100644 (file)
@@ -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 );
 
index 7ebab67..70fa857 100644 (file)
@@ -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;
        }
 
        /**
index 93b0b36..8630809 100644 (file)
@@ -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
index 0abec1b..14bc752 100644 (file)
@@ -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] ) ) {
index 31f6807..eb3853a 100644 (file)
@@ -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