Revert "Log multiple IPs using the same session or the same user account"

This reverts commit f22549a60539c9aa5c5390c8417c984ba8eef5b2.
Per T125455#2054194.

Bug: T125455
Change-Id: Ic2049381e98586e91974fc5b47d9e857a73414a4

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index da8eed5..9337673 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -2310,28 +2310,6 @@ $wgSessionHandler = null;
  */
 $wgPHPSessionHandling = 'enable';
 
-/**
- * The number of different IPs in the same session within a period of $wgSuspiciousIpExpiry
- * that should cause warnings to be logged. This is meant more for debugging errors in the
- * authentication system than for detecting abuse.
- * @since 1.27
- */
-$wgSuspiciousIpPerSessionLimit = 2;
-
-/**
- * Like $wgSuspiciousIpPerSessionLimit but over all requests from the same user within
- * $wgSuspiciousIpExpiry, whether they are in the same session or not.
- * @since 1.27
- */
-$wgSuspiciousIpPerUserLimit = 5;
-
-/**
- * Time in seconds to remember IPs for, for the purposes of $wgSuspiciousIpPerSessionLimit and
- * $wgSuspiciousIpPerUserLimit.
- * @since 1.27
- */
-$wgSuspiciousIpExpiry = 600;
-
 /**
  * If enabled, will send MemCached debugging information to $wgDebugLogFile
  */

diff --git a/includes/Setup.php b/includes/Setup.php
index 47fb73e..189855e 100644 (file)
--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -827,10 +827,5 @@ if ( !defined( 'MW_NO_SESSION' ) && !$wgCommandLineMode ) {
 wfDebug( "Fully initialised\n" );
 $wgFullyInitialised = true;
 
-// T125455
-if ( !defined( 'MW_NO_SESSION' ) && !$wgCommandLineMode ) {
-       MediaWiki\Session\SessionManager::singleton()->checkIpLimits();
-}
-
 Profiler::instance()->scopedProfileOut( $ps_extensions );
 Profiler::instance()->scopedProfileOut( $ps_setup );

diff --git a/includes/session/SessionManager.php b/includes/session/SessionManager.php
index 9ca3bbc..1aab12a 100644 (file)
--- a/includes/session/SessionManager.php
+++ b/includes/session/SessionManager.php
@@ -24,7 +24,6 @@
 namespace MediaWiki\Session;
 
 use Psr\Log\LoggerInterface;
-use Psr\Log\LogLevel;
 use BagOStuff;
 use CachedBagOStuff;
 use Config;
@@ -1074,96 +1073,6 @@ final class SessionManager implements SessionManagerInterface {
                self::$globalSessionRequest = null;
        }
 
-       /**
-        * Do a sanity check to make sure the session is not used from many different IP addresses
-        * and store some data for later sanity checks.
-        * FIXME remove this once SessionManager is considered stable
-        * @private For use in Setup.php only
-        * @param Session $session Defaults to the global session.
-        */
-       public function checkIpLimits( Session $session = null ) {
-               $session = $session ?: self::getGlobalSession();
-
-               try {
-                       $ip = $session->getRequest()->getIP();
-               } catch ( \MWException $e ) {
-                       return;
-               }
-               if ( $ip === '127.0.0.1' || \IP::isConfiguredProxy( $ip ) ) {
-                       return;
-               }
-               $now = time();
-
-               // Record (and possibly log) that the IP is using the current session.
-               // Don't touch the stored data unless we are adding a new IP or re-adding an expired one.
-               // This is slightly inaccurate (when an existing IP is seen again, the expiry is not
-               // extended) but that shouldn't make much difference and limits the session write frequency
-               // to # of IPs / $wgSuspiciousIpExpiry.
-               $data = $session->get( 'SessionManager-ip', [] );
-               if (
-                       !isset( $data[$ip] )
-                       || $data[$ip] < $now
-               ) {
-                       $data[$ip] = time() + $this->config->get( 'SuspiciousIpExpiry' );
-                       foreach ( $data as $key => $expires ) {
-                               if ( $expires < $now ) {
-                                       unset( $data[$key] );
-                               }
-                       }
-                       $session->set( 'SessionManager-ip', $data );
-
-                       $logger = \MediaWiki\Logger\LoggerFactory::getInstance( 'session-ip' );
-                       $logLevel = count( $data ) >= $this->config->get( 'SuspiciousIpPerSessionLimit' )
-                               ? LogLevel::WARNING : ( count( $data ) === 1 ? LogLevel::DEBUG : LogLevel::INFO );
-                       $logger->log(
-                               $logLevel,
-                               'Same session used from {count} IPs',
-                               [
-                                       'count' => count( $data ),
-                                       'ips' => $data,
-                                       'session' => $session->getId(),
-                                       'user' => $session->getUser()->getName(),
-                                       'persistent' => $session->isPersistent(),
-                               ]
-                       );
-               }
-
-               // Now do the same thing globally for the current user.
-               // We are using the object cache and assume it is shared between all wikis of a farm,
-               // and further assume that the same name belongs to the same user on all wikis. (It's either
-               // that or a central ID lookup which would mean an extra SQL query on every request.)
-               if ( $session->getUser()->isLoggedIn() ) {
-                       $userKey = 'SessionManager-ip:' . md5( $session->getUser()->getName() );
-                       $data = $this->store->get( $userKey ) ?: [];
-                       if (
-                               !isset( $data[$ip] )
-                               || $data[$ip] < $now
-                       ) {
-                               $data[$ip] = time() + $this->config->get( 'SuspiciousIpExpiry' );
-                               foreach ( $data as $key => $expires ) {
-                                       if ( $expires < $now ) {
-                                               unset( $data[$key] );
-                                       }
-                               }
-                               $this->store->set( $userKey, $data, $this->config->get( 'SuspiciousIpExpiry' ) );
-                               $logger = \MediaWiki\Logger\LoggerFactory::getInstance( 'session-ip' );
-                               $logLevel = count( $data ) >= $this->config->get( 'SuspiciousIpPerUserLimit' )
-                                       ? LogLevel::WARNING : ( count( $data ) === 1 ? LogLevel::DEBUG : LogLevel::INFO );
-                               $logger->log(
-                                       $logLevel,
-                                       'Same user had sessions from {count} IPs',
-                                       [
-                                               'count' => count( $data ),
-                                               'ips' => $data,
-                                               'session' => $session->getId(),
-                                               'user' => $session->getUser()->getName(),
-                                               'persistent' => $session->isPersistent(),
-                                       ]
-                               );
-                       }
-               }
-       }
-
        /**@}*/
 
 }

diff --git a/tests/phpunit/includes/session/SessionManagerTest.php b/tests/phpunit/includes/session/SessionManagerTest.php
index d3d8509..fe2c3b7 100644 (file)
--- a/tests/phpunit/includes/session/SessionManagerTest.php
+++ b/tests/phpunit/includes/session/SessionManagerTest.php
@@ -3,7 +3,6 @@
 namespace MediaWiki\Session;
 
 use AuthPlugin;
-use MediaWiki\Logger\LoggerFactory;
 use MediaWikiTestCase;
 use Psr\Log\LogLevel;
 use User;
@@ -1677,74 +1676,4 @@ class SessionManagerTest extends MediaWikiTestCase {
                ], $logger->getBuffer() );
                $logger->clearBuffer();
        }
-
-       /**
-        * @dataProvider provideCheckIpLimits
-        */
-       public function testCheckIpLimits( $ip, $sessionData, $userData, $logLevel1, $logLevel2 ) {
-               $this->setMwGlobals( [
-                       'wgSuspiciousIpPerSessionLimit' => 5,
-                       'wgSuspiciousIpPerUserLimit' => 10,
-                       'wgSuspiciousIpExpiry' => 600,
-                       'wgSquidServers' => [ '11.22.33.44' ],
-               ] );
-               $manager = new SessionManager();
-               $logger = $this->getMock( '\Psr\Log\LoggerInterface' );
-               $this->setLogger( 'session-ip', $logger );
-               $request = new \FauxRequest();
-               $request->setIP( $ip );
-
-               $session = $manager->getSessionForRequest( $request );
-               /** @var SessionBackend $backend */
-               $backend = \TestingAccessWrapper::newFromObject( $session )->backend;
-               $data = &$backend->getData();
-               $data = [ 'SessionManager-ip' => $sessionData ];
-               $backend->setUser( User::newFromName( 'UTSysop' ) );
-               $manager = \TestingAccessWrapper::newFromObject( $manager );
-               $manager->store->set( 'SessionManager-ip:' . md5( 'UTSysop' ), $userData );
-
-               $logger->expects( $this->exactly( isset( $logLevel1 ) + isset( $logLevel2 ) ) )->method( 'log' );
-               if ( $logLevel1 ) {
-                       $logger->expects( $this->at( 0 ) )->method( 'log' )->with( $logLevel1,
-                               'Same session used from {count} IPs', $this->isType( 'array' ) );
-               }
-               if ( $logLevel2 ) {
-                       $logger->expects( $this->at( isset( $logLevel1 ) ) )->method( 'log' )->with( $logLevel2,
-                               'Same user had sessions from {count} IPs', $this->isType( 'array' ) );
-               }
-
-               $manager->checkIpLimits( $session );
-       }
-
-       public function provideCheckIpLimits() {
-               $future = time() + 1000;
-               $past = time() - 1000;
-               return [
-                       // DEBUG log for first new IP
-                       [ '1.2.3.4', [], [], LogLevel::DEBUG, LogLevel::DEBUG ],
-                       // no log for same IP
-                       [ '1.2.3.4', [ '1.2.3.4'  => $future ], [ '1.2.3.4' => $future ],
-                                  null, null ],
-                       [ '1.2.3.4', [], [ '1.2.3.4' => $future ],
-                                  LogLevel::DEBUG, null ],
-                       // INFO log for second new IP
-                       [ '1.2.3.4', [ '10.20.30.40'  => $future ], [ '10.20.30.40' => $future ],
-                          LogLevel::INFO, LogLevel::INFO ],
-                       // WARNING above $wgSuspiciousIpPerSessionLimit
-                       [ '1.2.3.4', array_fill_keys( range( 1, 5 ), $future ),
-                          array_fill_keys( range( 1, 5 ), $future ), LogLevel::WARNING, LogLevel::INFO ],
-                       // WARNING above $wgSuspiciousIpPerUserLimit
-
-                       [ '1.2.3.4', array_fill_keys( range( 1, 2 ), $future ),
-                                  array_fill_keys( range( 1, 12 ), $future ), LogLevel::INFO, LogLevel::WARNING ],
-                       // expired keys ignored
-                       [ '1.2.3.4', [ '1.2.3.4'  => $past ], [ '1.2.3.4' => $past ],
-                          LogLevel::DEBUG, LogLevel::DEBUG ],
-                       [ '1.2.3.4', array_fill_keys( range( 1, 5 ), $past ),
-                                  array_fill_keys( range( 1, 5 ), $past ), LogLevel::DEBUG, LogLevel::DEBUG ],
-                       // special IPs are ignored
-                       [ '127.0.0.1', [], [], null, null ],
-                       [ '11.22.33.44', [], [], null, null ],
-               ];
-       }
 }