SessionManager: Abstract forceHTTPS cookie setting

This allows CentralAuthSessionProvider to avoid doing craziness like
this all the time:

  Set-Cookie: forceHTTPS=true; path=/; httponly
  Set-Cookie: forceHTTPS=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0; path=/; httponly
  Set-Cookie: forceHTTPS=true; path=/; domain=.wikipedia.org; httponly
  Set-Cookie: forceHTTPS=true; path=/; httponly
  Set-Cookie: forceHTTPS=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0; path=/; httponly
  Set-Cookie: forceHTTPS=true; path=/; domain=.wikipedia.org; httponly

Bug: T124421
Change-Id: I7e02afd032a246df6850208c26d3447798bc0fc2

diff --git a/includes/session/CookieSessionProvider.php b/includes/session/CookieSessionProvider.php
index 7854416..915127f 100644 (file)
--- a/includes/session/CookieSessionProvider.php
+++ b/includes/session/CookieSessionProvider.php
@@ -173,9 +173,9 @@ class CookieSessionProvider extends SessionProvider {
                }
 
                $options = $this->cookieOptions;
-               if ( $session->shouldForceHTTPS() || $user->requiresHTTPS() ) {
-                       $response->setCookie( 'forceHTTPS', 'true', $session->shouldRememberUser() ? 0 : null,
-                               array( 'prefix' => '', 'secure' => false ) + $options );
+
+               $forceHTTPS = $session->shouldForceHTTPS() || $user->requiresHTTPS();
+               if ( $forceHTTPS ) {
                        $options['secure'] = true;
                }
 
@@ -199,6 +199,7 @@ class CookieSessionProvider extends SessionProvider {
                        }
                }
 
+               $this->setForceHTTPSCookie( $forceHTTPS, $session, $request );
                $this->setLoggedOutCookie( $session->getLoggedOutTimestamp(), $request );
 
                if ( $sessionData ) {
@@ -227,8 +228,26 @@ class CookieSessionProvider extends SessionProvider {
                        $response->clearCookie( $key, $this->cookieOptions );
                }
 
-               $response->clearCookie( 'forceHTTPS',
-                       array( 'prefix' => '', 'secure' => false ) + $this->cookieOptions );
+               $this->setForceHTTPSCookie( false, null, $request );
+       }
+
+       /**
+        * Set the "forceHTTPS" cookie
+        * @param bool $set Whether the cookie should be set or not
+        * @param SessionBackend|null $backend
+        * @param WebRequest $request
+        */
+       protected function setForceHTTPSCookie(
+               $set, SessionBackend $backend = null, WebRequest $request
+       ) {
+               $response = $request->response();
+               if ( $set ) {
+                       $response->setCookie( 'forceHTTPS', 'true', $backend->shouldRememberUser() ? 0 : null,
+                               array( 'prefix' => '', 'secure' => false ) + $this->cookieOptions );
+               } else {
+                       $response->clearCookie( 'forceHTTPS',
+                               array( 'prefix' => '', 'secure' => false ) + $this->cookieOptions );
+               }
        }
 
        /**

diff --git a/tests/phpunit/includes/session/CookieSessionProviderTest.php b/tests/phpunit/includes/session/CookieSessionProviderTest.php
index a73bf7c..ccf45f6 100644 (file)
--- a/tests/phpunit/includes/session/CookieSessionProviderTest.php
+++ b/tests/phpunit/includes/session/CookieSessionProviderTest.php
@@ -382,7 +382,7 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
                $this->assertSame( '', $request->response()->getCookie( 'xUserID' ) );
                $this->assertSame( null, $request->response()->getCookie( 'xUserName' ) );
                $this->assertSame( '', $request->response()->getCookie( 'xToken' ) );
-               $this->assertSame( null, $request->response()->getCookie( 'forceHTTPS' ) );
+               $this->assertSame( '', $request->response()->getCookie( 'forceHTTPS' ) );
                $this->assertSame( array(), $backend->getData() );
 
                // Logged-in user, no remember
@@ -395,7 +395,7 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
                $this->assertSame( (string)$user->getId(), $request->response()->getCookie( 'xUserID' ) );
                $this->assertSame( $user->getName(), $request->response()->getCookie( 'xUserName' ) );
                $this->assertSame( '', $request->response()->getCookie( 'xToken' ) );
-               $this->assertSame( null, $request->response()->getCookie( 'forceHTTPS' ) );
+               $this->assertSame( '', $request->response()->getCookie( 'forceHTTPS' ) );
                $this->assertSame( array(), $backend->getData() );
 
                // Logged-in user, remember
@@ -488,10 +488,10 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
                                'value' => $remember ? $user->getToken() : '',
                                'expire' => $remember ? $extendedExpiry : -31536000,
                        ) + $defaults,
-                       'forceHTTPS' => !$secure ? null : array(
-                               'value' => 'true',
+                       'forceHTTPS' => array(
+                               'value' => $secure ? 'true' : '',
                                'secure' => false,
-                               'expire' => $remember ? $defaults['expire'] : null,
+                               'expire' => $secure ? $remember ? $defaults['expire'] : 0 : -31536000,
                        ) + $defaults,
                );
                foreach ( $expect as $key => $value ) {
@@ -571,7 +571,7 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
                $this->assertSame( '', $request->response()->getCookie( 'xUserID' ) );
                $this->assertSame( null, $request->response()->getCookie( 'xUserName' ) );
                $this->assertSame( '', $request->response()->getCookie( 'xToken' ) );
-               $this->assertSame( null, $request->response()->getCookie( 'forceHTTPS' ) );
+               $this->assertSame( '', $request->response()->getCookie( 'forceHTTPS' ) );
                $this->assertSame( array(), $backend->getData() );
 
                $provider->persistSession( $backend, $this->getSentRequest() );
@@ -607,7 +607,7 @@ class CookieSessionProviderTest extends MediaWikiTestCase {
                $this->assertSame( (string)$user->getId(), $request->response()->getCookie( 'xUserID' ) );
                $this->assertSame( $user->getName(), $request->response()->getCookie( 'xUserName' ) );
                $this->assertSame( '', $request->response()->getCookie( 'xToken' ) );
-               $this->assertSame( null, $request->response()->getCookie( 'forceHTTPS' ) );
+               $this->assertSame( '', $request->response()->getCookie( 'forceHTTPS' ) );
                $this->assertSame( 'bar!', $request->response()->getCookie( 'xbar' ) );
                $this->assertSame( (string)$loggedOut, $request->response()->getCookie( 'xLoggedOut' ) );
                $this->assertEquals( array(