From 4eeff5b559e2ae7b8fa1f45572968ba28573a421 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Mon, 25 Jan 2016 14:15:40 -0500 Subject: [PATCH] Use $wgSecureCookie to decide whether to actually mark secure cookies as 'secure' The pre-SessionManager code did this, and the change in combination with the API not honoring forceHTTPS led to T124252. Bug: T124252 Change-Id: Ic6a79fbb30491040facd7c200b1f47d6b99ce637 --- includes/session/CookieSessionProvider.php | 5 ++++- tests/phpunit/includes/session/CookieSessionProviderTest.php | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/includes/session/CookieSessionProvider.php b/includes/session/CookieSessionProvider.php index 915127ff3c..2d01d1d0f4 100644 --- a/includes/session/CookieSessionProvider.php +++ b/includes/session/CookieSessionProvider.php @@ -176,7 +176,10 @@ class CookieSessionProvider extends SessionProvider { $forceHTTPS = $session->shouldForceHTTPS() || $user->requiresHTTPS(); if ( $forceHTTPS ) { - $options['secure'] = true; + // Don't set the secure flag if the request came in + // over "http", for backwards compat. + // @todo Break that backwards compat properly. + $options['secure'] = $this->config->get( 'CookieSecure' ); } $response->setCookie( $this->params['sessionName'], $session->getId(), null, diff --git a/tests/phpunit/includes/session/CookieSessionProviderTest.php b/tests/phpunit/includes/session/CookieSessionProviderTest.php index ccf45f68fd..702f556311 100644 --- a/tests/phpunit/includes/session/CookieSessionProviderTest.php +++ b/tests/phpunit/includes/session/CookieSessionProviderTest.php @@ -431,7 +431,7 @@ class CookieSessionProviderTest extends MediaWikiTestCase { 'cookieOptions' => array( 'prefix' => 'x' ), ) ); $config = $this->getConfig(); - $config->set( 'CookieSecure', false ); + $config->set( 'CookieSecure', $secure ); $provider->setLogger( new \TestLogger() ); $provider->setConfig( $config ); $provider->setManager( SessionManager::singleton() ); -- 2.20.1