Aaron Schulz has submitted this change and it was merged.
Change subject: Fixed importScopedSession() and moved exportUserSession() to
RequestContext.
......................................................................
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
(cherry picked from commit f31a0463aaee6f8cc18d05c3af4804f67f36a2c1)
---
M includes/WebRequest.php
M includes/api/ApiUpload.php
M includes/context/ContextSource.php
M includes/context/IContextSource.php
M includes/context/RequestContext.php
M includes/job/jobs/AssembleUploadChunksJob.php
M includes/job/jobs/PublishStashedFileJob.php
M tests/phpunit/includes/RequestContextTest.php
8 files changed, 116 insertions(+), 39 deletions(-)
Approvals:
Aaron Schulz: Verified; Looks good to me, approved
diff --git a/includes/WebRequest.php b/includes/WebRequest.php
index 2195fbb..8f68572 100644
--- a/includes/WebRequest.php
+++ b/includes/WebRequest.php
@@ -1133,21 +1133,6 @@
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 05d3b5a..31e76e2 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -222,8 +222,7 @@
array(
'filename' =>
$this->mParams['filename'],
'filekey' =>
$this->mParams['filekey'],
- 'session' =>
$this->getRequest()->exportUserSession(),
- 'userid' =>
$this->getUser()->getId()
+ 'session' =>
$this->getContext()->exportSession()
)
) );
if ( $ok ) {
@@ -593,8 +592,7 @@
'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 4a02f0b..33f51cb 100644
--- a/includes/context/ContextSource.php
+++ b/includes/context/ContextSource.php
@@ -164,4 +164,15 @@
$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 0399081..c7b221b 100644
--- a/includes/context/IContextSource.php
+++ b/includes/context/IContextSource.php
@@ -105,4 +105,13 @@
* @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 60c8cd3..6aefc98 100644
--- a/includes/context/RequestContext.php
+++ b/includes/context/RequestContext.php
@@ -393,59 +393,92 @@
}
/**
- * 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 f243b0c..c5dd9ea 100644
--- a/includes/job/jobs/AssembleUploadChunksJob.php
+++ b/includes/job/jobs/AssembleUploadChunksJob.php
@@ -37,7 +37,7 @@
$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 87dffc9..d3feda2 100644
--- a/includes/job/jobs/PublishStashedFileJob.php
+++ b/includes/job/jobs/PublishStashedFileJob.php
@@ -37,7 +37,7 @@
$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 48cf6dc..f587171 100644
--- a/tests/phpunit/includes/RequestContextTest.php
+++ b/tests/phpunit/includes/RequestContextTest.php
@@ -1,5 +1,8 @@
<?php
+/**
+ * @group Database
+ */
class RequestContextTest extends MediaWikiTestCase {
/**
@@ -25,4 +28,42 @@
}
+ public function testImportScopedSession() {
+ $context = RequestContext::getMain();
+
+ $oInfo = $context->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." );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/54084
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I543e6766f7a8a828ea5d270328c3bc7738c6fe94
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: wmf/1.21wmf11
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
