Configure logged in session length independently

* Add the $wgExtendedLoginCookies configuration variable, which defines
  the set of login cookies that can have their lifetime configured
  independently
* Add the $wgExtendedLoginCookieExpiration configuration variable, which
  dictates when the extended lifetime login cookies expire
* Default $wgExtendedLoginCookieExpiration to null so that the current
  behaviour is unaffected

Bug: T68699
Change-Id: I0cc24524e4d7d9d1d21c9fa8a28c7c76b677b96c

diff --git a/RELEASE-NOTES-1.26 b/RELEASE-NOTES-1.26
index e23fde2..885448f 100644 (file)
--- a/RELEASE-NOTES-1.26
+++ b/RELEASE-NOTES-1.26
@@ -27,7 +27,9 @@ production.
 * Added a new hook, 'RejectParserCacheValue', which allows extensions to
   reject an otherwise-successful parser cache lookup. The intent is to allow
   extensions to manage the eviction of archaic HTML output from the cache.
-
+* (T68699) The expiration of the UserID and Token login cookies
+  ($wgExtendedLoginCookieExpiration) can be configured independently of the
+  expiration of all other cookies ($wgCookieExpiration).
 
 ==== External libraries ====
 * Update es5-shim from v4.0.0 to v4.1.5.

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index a16bccb..45102ee 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -5273,6 +5273,22 @@ $wgProxyList = array();
  */
 $wgCookieExpiration = 180 * 86400;
 
+/**
+ * The identifiers of the login cookies that can have their lifetimes
+ * extended independently of all other login cookies.
+ *
+ * @var string[]
+ */
+$wgExtendedLoginCookies = array( 'UserID', 'Token' );
+
+/**
+ * Default login cookie lifetime, in seconds. Setting
+ * $wgExtendLoginCookieExpiration to null will use $wgCookieExpiration to
+ * calculate the cookie lifetime. As with $wgCookieExpiration, 0 will make
+ * login cookies session-only.
+ */
+$wgExtendedLoginCookieExpiration = null;
+
 /**
  * Set to set an explicit domain on the login cookies eg, "justthis.domain.org"
  * or ".any.subdomain.net"

diff --git a/includes/User.php b/includes/User.php
index 961520a..1ee8173 100644 (file)
--- a/includes/User.php
+++ b/includes/User.php
@@ -3491,6 +3491,31 @@ class User implements IDBAccessObject {
                $this->setCookie( $name, '', time() - 86400, $secure, $params );
        }
 
+       /**
+        * Set an extended login cookie on the user's client. The expiry of the cookie
+        * is controlled by the $wgExtendedLoginCookieExpiration configuration
+        * variable.
+        *
+        * @see User::setCookie
+        *
+        * @param string $name Name of the cookie to set
+        * @param string $value Value to set
+        * @param bool $secure
+        *  true: Force setting the secure attribute when setting the cookie
+        *  false: Force NOT setting the secure attribute when setting the cookie
+        *  null (default): Use the default ($wgCookieSecure) to set the secure attribute
+        */
+       protected function setExtendedLoginCookie( $name, $value, $secure ) {
+               global $wgExtendedLoginCookieExpiration, $wgCookieExpiration;
+
+               $exp = time();
+               $exp += $wgExtendedLoginCookieExpiration !== null
+                       ? $wgExtendedLoginCookieExpiration
+                       : $wgCookieExpiration;
+
+               $this->setCookie( $name, $value, $exp, $secure );
+       }
+
        /**
         * Set the default cookies for this session on the user's client.
         *
@@ -3500,6 +3525,8 @@ class User implements IDBAccessObject {
         * @param bool $rememberMe Whether to add a Token cookie for elongated sessions
         */
        public function setCookies( $request = null, $secure = null, $rememberMe = false ) {
+               global $wgExtendedLoginCookies;
+
                if ( $request === null ) {
                        $request = $this->getRequest();
                }
@@ -3541,6 +3568,8 @@ class User implements IDBAccessObject {
                foreach ( $cookies as $name => $value ) {
                        if ( $value === false ) {
                                $this->clearCookie( $name );
+                       } elseif ( $rememberMe && in_array( $name, $wgExtendedLoginCookies ) ) {
+                               $this->setExtendedLoginCookie( $name, $value, $secure );
                        } else {
                                $this->setCookie( $name, $value, 0, $secure, array(), $request );
                        }

diff --git a/includes/specials/SpecialUserlogin.php b/includes/specials/SpecialUserlogin.php
index 11744f6..472fdb7 100644 (file)
--- a/includes/specials/SpecialUserlogin.php
+++ b/includes/specials/SpecialUserlogin.php
@@ -1282,8 +1282,9 @@ class LoginForm extends SpecialPage {
        function mainLoginForm( $msg, $msgtype = 'error' ) {
                global $wgEnableEmail, $wgEnableUserEmail;
                global $wgHiddenPrefs, $wgLoginLanguageSelector;
-               global $wgAuth, $wgEmailConfirmToEdit, $wgCookieExpiration;
+               global $wgAuth, $wgEmailConfirmToEdit;
                global $wgSecureLogin, $wgPasswordResetRoutes;
+               global $wgExtendedLoginCookieExpiration, $wgCookieExpiration;
 
                $titleObj = $this->getPageTitle();
                $user = $this->getUser();
@@ -1406,7 +1407,7 @@ class LoginForm extends SpecialPage {
                $template->set( 'emailothers', $wgEnableUserEmail );
                $template->set( 'canreset', $wgAuth->allowPasswordChange() );
                $template->set( 'resetlink', $resetLink );
-               $template->set( 'canremember', ( $wgCookieExpiration > 0 ) );
+               $template->set( 'canremember', $wgExtendedLoginCookieExpiration === null ? ( $wgCookieExpiration > 0 ) : ( $wgExtendedLoginCookieExpiration > 0 ) );
                $template->set( 'usereason', $user->isLoggedIn() );
                $template->set( 'remember', $this->mRemember );
                $template->set( 'cansecurelogin', ( $wgSecureLogin === true ) );

diff --git a/tests/TestsAutoLoader.php b/tests/TestsAutoLoader.php
index 2682ee1..c3019c9 100644 (file)
--- a/tests/TestsAutoLoader.php
+++ b/tests/TestsAutoLoader.php
@@ -114,6 +114,7 @@ $wgAutoloadClasses += array(
        'MockImageHandler' => "$testDir/phpunit/mocks/media/MockImageHandler.php",
        'MockSvgHandler' => "$testDir/phpunit/mocks/media/MockSvgHandler.php",
        'MockDjVuHandler' => "$testDir/phpunit/mocks/media/MockDjVuHandler.php",
+       'MockWebRequest' => "$testDir/phpunit/mocks/MockWebRequest.php",
 
        # tests/parser
        'NewParserTest' => "$testDir/phpunit/includes/parser/NewParserTest.php",

diff --git a/tests/phpunit/includes/UserTest.php b/tests/phpunit/includes/UserTest.php
index a1f8361..77132bb 100644 (file)
--- a/tests/phpunit/includes/UserTest.php
+++ b/tests/phpunit/includes/UserTest.php
@@ -466,4 +466,89 @@ class UserTest extends MediaWikiTestCase {
                $this->assertGreaterThan(
                        $touched, $user->getDBTouched(), "user_touched increased with casOnTouched() #2" );
        }
+
+       public static function setExtendedLoginCookieDataProvider() {
+               $data = array();
+               $now = time();
+
+               $secondsInDay = 86400;
+
+               // Arbitrary durations, in units of days, to ensure it chooses the
+               // right one.  There is a 5-minute grace period (see testSetExtendedLoginCookie)
+               // to work around slow tests, since we're not currently mocking time() for PHP.
+
+               $durationOne = $secondsInDay * 5;
+               $durationTwo = $secondsInDay * 29;
+               $durationThree = $secondsInDay * 17;
+
+               // If $wgExtendedLoginCookieExpiration is null, then the expiry passed to
+               // set cookie is time() + $wgCookieExpiration
+               $data[] = array(
+                       null,
+                       $durationOne,
+                       $now + $durationOne,
+               );
+
+               // If $wgExtendedLoginCookieExpiration isn't null, then the expiry passed to
+               // set cookie is $now + $wgExtendedLoginCookieExpiration
+               $data[] = array(
+                       $durationTwo,
+                       $durationThree,
+                       $now + $durationTwo,
+               );
+
+               return $data;
+       }
+
+       /**
+        * @dataProvider setExtendedLoginCookieDataProvider
+        * @covers User::getRequest
+        * @covers User::setCookie
+        * @backupGlobals enabled
+        */
+       public function testSetExtendedLoginCookie(
+               $extendedLoginCookieExpiration,
+               $cookieExpiration,
+               $expectedExpiry
+       ) {
+               $this->setMwGlobals( array(
+                       'wgExtendedLoginCookieExpiration' => $extendedLoginCookieExpiration,
+                       'wgCookieExpiration' => $cookieExpiration,
+               ) );
+
+               $response = $this->getMock( 'WebResponse' );
+               $setcookieSpy = $this->any();
+               $response->expects( $setcookieSpy )
+                       ->method( 'setcookie' );
+
+               $request = new MockWebRequest( $response );
+               $user = new UserProxy( User::newFromSession( $request ) );
+               $user->setExtendedLoginCookie( 'name', 'value', true );
+
+               $setcookieInvocations = $setcookieSpy->getInvocations();
+               $setcookieInvocation = end( $setcookieInvocations );
+               $actualExpiry = $setcookieInvocation->parameters[ 2 ];
+
+               // TODO: ± 300 seconds compensates for
+               // slow-running tests. However, the dependency on the time
+               // function should be removed.  This requires some way
+               // to mock/isolate User->setExtendedLoginCookie's call to time()
+               $this->assertEquals( $expectedExpiry, $actualExpiry, '', 300 );
+       }
+}
+
+class UserProxy extends User {
+
+       /**
+        * @var User
+        */
+       protected $user;
+
+       public function __construct( User $user ) {
+               $this->user = $user;
+       }
+
+       public function setExtendedLoginCookie( $name, $value, $secure ) {
+               $this->user->setExtendedLoginCookie( $name, $value, $secure );
+       }
 }

diff --git a/tests/phpunit/mocks/MockWebRequest.php b/tests/phpunit/mocks/MockWebRequest.php
new file mode 100644 (file)
index 0000000..3ac5bfb
--- /dev/null
+++ b/tests/phpunit/mocks/MockWebRequest.php
@@ -0,0 +1,26 @@
+<?php
+
+/**
+ * A mock WebRequest.
+ *
+ * If the code under test accesses the response via the request (see
+ * WebRequest#response), then you might be able to use this mock to simplify
+ * your tests.
+ */
+class MockWebRequest extends WebRequest
+{
+       /**
+        * @var WebResponse
+        */
+       protected $response;
+
+       public function __construct( WebResponse $response ) {
+               parent::__construct();
+
+               $this->response = $response;
+       }
+
+       public function response() {
+               return $this->response;
+       }
+}