From f31a0463aaee6f8cc18d05c3af4804f67f36a2c1 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 14 Mar 2013 15:43:42 -0700 Subject: [PATCH] Fixed importScopedSession() and moved exportUserSession() to RequestContext. * Renamed WebRequest::exportUserSession -> RequestContext::exportSession. Updated the only callers of this new function. * Init the user with User::newFromId() instead of relying on the session (which breaks when things like CentralAuth are enabled). * Made RequestContext::exportSession() include the user ID. * Removed now-redundant user ID checks in upload jobs. * Added unit tests for the session import function. Change-Id: I543e6766f7a8a828ea5d270328c3bc7738c6fe94 --- includes/WebRequest.php | 15 ---- includes/api/ApiUpload.php | 6 +- includes/context/ContextSource.php | 11 +++ includes/context/IContextSource.php | 9 +++ includes/context/RequestContext.php | 69 ++++++++++++++----- includes/job/jobs/AssembleUploadChunksJob.php | 2 +- includes/job/jobs/PublishStashedFileJob.php | 2 +- tests/phpunit/includes/RequestContextTest.php | 41 +++++++++++ 8 files changed, 116 insertions(+), 39 deletions(-) diff --git a/includes/WebRequest.php b/includes/WebRequest.php index 9b6d9ce52f..98007ef8e1 100644 --- a/includes/WebRequest.php +++ b/includes/WebRequest.php @@ -1133,21 +1133,6 @@ HTML; public function setIP( $ip ) { $this->ip = $ip; } - - /** - * Export the resolved user IP, HTTP headers, and session ID. - * The result will be reasonably sized to allow for serialization. - * - * @return Array - * @since 1.21 - */ - public function exportUserSession() { - return array( - 'ip' => $this->getIP(), - 'headers' => $this->getAllHeaders(), - 'sessionId' => session_id() - ); - } } /** diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php index f2733bdeef..deeb1c1849 100644 --- a/includes/api/ApiUpload.php +++ b/includes/api/ApiUpload.php @@ -224,8 +224,7 @@ class ApiUpload extends ApiBase { array( 'filename' => $this->mParams['filename'], 'filekey' => $this->mParams['filekey'], - 'session' => $this->getRequest()->exportUserSession(), - 'userid' => $this->getUser()->getId() + 'session' => $this->getContext()->exportSession() ) ) ); if ( $ok ) { @@ -594,8 +593,7 @@ class ApiUpload extends ApiBase { 'comment' => $this->mParams['comment'], 'text' => $this->mParams['text'], 'watch' => $watch, - 'session' => $this->getRequest()->exportUserSession(), - 'userid' => $this->getUser()->getId() + 'session' => $this->getContext()->exportSession() ) ) ); if ( $ok ) { diff --git a/includes/context/ContextSource.php b/includes/context/ContextSource.php index 4a02f0b0cc..33f51cb972 100644 --- a/includes/context/ContextSource.php +++ b/includes/context/ContextSource.php @@ -164,4 +164,15 @@ abstract class ContextSource implements IContextSource { $args = func_get_args(); return call_user_func_array( array( $this->getContext(), 'msg' ), $args ); } + + /** + * Export the resolved user IP, HTTP headers, user ID, and session ID. + * The result will be reasonably sized to allow for serialization. + * + * @return Array + * @since 1.21 + */ + public function exportSession() { + return $this->getContext()->exportSession(); + } } diff --git a/includes/context/IContextSource.php b/includes/context/IContextSource.php index 0399081cfa..c7b221b9b0 100644 --- a/includes/context/IContextSource.php +++ b/includes/context/IContextSource.php @@ -105,4 +105,13 @@ interface IContextSource { * @return Message */ public function msg(); + + /** + * Export the resolved user IP, HTTP headers, user ID, and session ID. + * The result will be reasonably sized to allow for serialization. + * + * @return Array + * @since 1.21 + */ + public function exportSession(); } diff --git a/includes/context/RequestContext.php b/includes/context/RequestContext.php index 60c8cd324c..6aefc98edd 100644 --- a/includes/context/RequestContext.php +++ b/includes/context/RequestContext.php @@ -393,59 +393,92 @@ class RequestContext implements IContextSource { } /** - * Import the resolved user IP, HTTP headers, and session ID. + * Export the resolved user IP, HTTP headers, user ID, and session ID. + * The result will be reasonably sized to allow for serialization. + * + * @return Array + * @since 1.21 + */ + public function exportSession() { + return array( + 'ip' => $this->getRequest()->getIP(), + 'headers' => $this->getRequest()->getAllHeaders(), + 'sessionId' => session_id(), + 'userId' => $this->getUser()->getId() + ); + } + + /** + * Import the resolved user IP, HTTP headers, user ID, and session ID. * This sets the current session and sets $wgUser and $wgRequest. * Once the return value falls out of scope, the old context is restored. * This function can only be called within CLI mode scripts. * * This will setup the session from the given ID. This is useful when - * background scripts inherit some context when acting on behalf of a user. + * background scripts inherit context when acting on behalf of a user. * - * $param array $params Result of WebRequest::exportUserSession() + * $param array $params Result of RequestContext::exportSession() * @return ScopedCallback * @throws MWException * @since 1.21 */ public static function importScopedSession( array $params ) { if ( PHP_SAPI !== 'cli' ) { - // Don't send random private cookie headers to other random users + // Don't send random private cookies or turn $wgRequest into FauxRequest throw new MWException( "Sessions can only be imported in cli mode." ); + } elseif ( !strlen( $params['sessionId'] ) ) { + throw new MWException( "No session ID was specified." ); } - $importSessionFunction = function( array $params ) { + if ( $params['userId'] ) { // logged-in user + $user = User::newFromId( $params['userId'] ); + if ( !$user ) { + throw new MWException( "No user with ID '{$params['userId']}'." ); + } + } elseif ( !IP::isValid( $params['ip'] ) ) { + throw new MWException( "Could not load user '{$params['ip']}'." ); + } else { // anon user + $user = User::newFromName( $params['ip'], false ); + } + + $importSessionFunction = function( User $user, array $params ) { global $wgRequest, $wgUser; - // Write and close any current session + $context = RequestContext::getMain(); + // Commit and close any current session session_write_close(); // persist session_id( '' ); // detach $_SESSION = array(); // clear in-memory array - // Load the new session from the session ID - if ( strlen( $params['sessionId'] ) ) { + // Remove any user IP or agent information + $context->setRequest( new FauxRequest() ); + $wgRequest = $context->getRequest(); // b/c + // Now that all private information is detached from the user, it should + // be safe to load the new user. If errors occur or an exception is thrown + // and caught (leaving the main context in a mixed state), there is no risk + // of the User object being attached to the wrong IP, headers, or session. + $context->setUser( $user ); + $wgUser = $context->getUser(); // b/c + if ( strlen( $params['sessionId'] ) ) { // don't make a new random ID wfSetupSession( $params['sessionId'] ); // sets $_SESSION } - // Build the new WebRequest object $request = new FauxRequest( array(), false, $_SESSION ); $request->setIP( $params['ip'] ); foreach ( $params['headers'] as $name => $value ) { $request->setHeader( $name, $value ); } - - $context = RequestContext::getMain(); // Set the current context to use the new WebRequest $context->setRequest( $request ); $wgRequest = $context->getRequest(); // b/c - // Set the current user based on the new session and WebRequest - $context->setUser( User::newFromSession( $request ) ); // uses $_SESSION - $wgUser = $context->getUser(); // b/c }; // Stash the old session and load in the new one - $oldParams = self::getMain()->getRequest()->exportUserSession(); - $importSessionFunction( $params ); + $oUser = self::getMain()->getUser(); + $oParams = self::getMain()->exportSession(); + $importSessionFunction( $user, $params ); // Set callback to save and close the new session and reload the old one - return new ScopedCallback( function() use ( $importSessionFunction, $oldParams ) { - $importSessionFunction( $oldParams ); + return new ScopedCallback( function() use ( $importSessionFunction, $oUser, $oParams ) { + $importSessionFunction( $oUser, $oParams ); } ); } diff --git a/includes/job/jobs/AssembleUploadChunksJob.php b/includes/job/jobs/AssembleUploadChunksJob.php index f243b0c388..c5dd9eaa4d 100644 --- a/includes/job/jobs/AssembleUploadChunksJob.php +++ b/includes/job/jobs/AssembleUploadChunksJob.php @@ -37,7 +37,7 @@ class AssembleUploadChunksJob extends Job { $context = RequestContext::getMain(); try { $user = $context->getUser(); - if ( !$user->isLoggedIn() || $user->getId() != $this->params['userid'] ) { + if ( !$user->isLoggedIn() ) { $this->setLastError( "Could not load the author user from session." ); return false; } diff --git a/includes/job/jobs/PublishStashedFileJob.php b/includes/job/jobs/PublishStashedFileJob.php index 87dffc99ae..d3feda2834 100644 --- a/includes/job/jobs/PublishStashedFileJob.php +++ b/includes/job/jobs/PublishStashedFileJob.php @@ -37,7 +37,7 @@ class PublishStashedFileJob extends Job { $context = RequestContext::getMain(); try { $user = $context->getUser(); - if ( !$user->isLoggedIn() || $user->getId() != $this->params['userid'] ) { + if ( !$user->isLoggedIn() ) { $this->setLastError( "Could not load the author user from session." ); return false; } diff --git a/tests/phpunit/includes/RequestContextTest.php b/tests/phpunit/includes/RequestContextTest.php index 48cf6dc650..f5871716ae 100644 --- a/tests/phpunit/includes/RequestContextTest.php +++ b/tests/phpunit/includes/RequestContextTest.php @@ -1,5 +1,8 @@ exportSession(); + $this->assertEquals( '127.0.0.1', $oInfo['ip'], "Correct initial IP address." ); + $this->assertEquals( 0, $oInfo['userId'], "Correct initial user ID." ); + + $user = User::newFromName( 'UnitTestContextUser' ); + $user->addToDatabase(); + + $sinfo = array( + 'sessionId' => 'd612ee607c87e749ef14da4983a702cd', + 'userId' => $user->getId(), + 'ip' => '192.0.2.0', + 'headers' => array( 'USER-AGENT' => 'Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:18.0) Gecko/20100101 Firefox/18.0' ) + ); + $sc = RequestContext::importScopedSession( $sinfo ); // load new context + + $info = $context->exportSession(); + $this->assertEquals( $sinfo['ip'], $info['ip'], "Correct IP address." ); + $this->assertEquals( $sinfo['headers'], $info['headers'], "Correct headers." ); + $this->assertEquals( $sinfo['sessionId'], $info['sessionId'], "Correct session ID." ); + $this->assertEquals( $sinfo['userId'], $info['userId'], "Correct user ID." ); + $this->assertEquals( $sinfo['ip'], $context->getRequest()->getIP(), "Correct context IP address." ); + $this->assertEquals( $sinfo['headers'], $context->getRequest()->getAllHeaders(), "Correct context headers." ); + $this->assertEquals( $sinfo['sessionId'], session_id(), "Correct context session ID." ); + $this->assertEquals( true, $context->getUser()->isLoggedIn(), "Correct context user." ); + $this->assertEquals( $sinfo['userId'], $context->getUser()->getId(), "Correct context user ID." ); + $this->assertEquals( 'UnitTestContextUser', $context->getUser()->getName(), "Correct context user name." ); + + unset ( $sc ); // restore previous context + + $info = $context->exportSession(); + $this->assertEquals( $oInfo['ip'], $info['ip'], "Correct initial IP address." ); + $this->assertEquals( $oInfo['headers'], $info['headers'], "Correct initial headers." ); + $this->assertEquals( $oInfo['sessionId'], $info['sessionId'], "Correct initial session ID." ); + $this->assertEquals( $oInfo['userId'], $info['userId'], "Correct initial user ID." ); + } } -- 2.20.1