diff --git a/core/core.services.yml b/core/core.services.yml index 2b31f93..440b35b 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -455,9 +455,10 @@ services: arguments: ['@state'] csrf_token: class: Drupal\Core\Access\CsrfTokenGenerator - arguments: ['@private_key'] + arguments: ['@private_key', '@settings'] calls: - [setCurrentUser, ['@?current_user']] + - [setRequest, ['@?request']] access_manager: class: Drupal\Core\Access\AccessManager arguments: ['@router.route_provider', '@url_generator', '@paramconverter_manager'] diff --git a/core/lib/Drupal/Core/Access/CsrfTokenGenerator.php b/core/lib/Drupal/Core/Access/CsrfTokenGenerator.php index 527fffd..930c31a 100644 --- a/core/lib/Drupal/Core/Access/CsrfTokenGenerator.php +++ b/core/lib/Drupal/Core/Access/CsrfTokenGenerator.php @@ -8,8 +8,10 @@ namespace Drupal\Core\Access; use Drupal\Component\Utility\Crypt; +use Drupal\Component\Utility\Settings; use Drupal\Core\PrivateKey; use Drupal\Core\Session\AccountInterface; +use Symfony\Component\HttpFoundation\Request; /** * Generates and validates CSRF tokens. @@ -26,6 +28,13 @@ class CsrfTokenGenerator { protected $privateKey; /** + * The settings service. + * + * @var \Drupal\Component\Utility\Settings + */ + protected $settings; + + /** * The current user. * * @var \Drupal\Core\Session\AccountInterface @@ -33,13 +42,23 @@ class CsrfTokenGenerator { protected $currentUser; /** + * The current request. + * + * @var \Symfony\Component\HttpFoundation\Request + */ + protected $request; + + /** * Constructs the token generator. * * @param \Drupal\Core\PrivateKey $private_key * The private key service. + * @param \Drupal\Component\Utility\Settings $settings + * The settings service. */ - public function __construct(PrivateKey $private_key) { + public function __construct(PrivateKey $private_key, Settings $settings) { $this->privateKey = $private_key; + $this->settings = $settings; } /** @@ -53,6 +72,16 @@ public function setCurrentUser(AccountInterface $current_user = NULL) { } /** + * Sets the current request. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The current request. + */ + public function setRequest(Request $request) { + $this->request = $request; + } + + /** * Generates a token based on $value, the user session, and the private key. * * The generated token is based on the session ID of the current user. Normally, @@ -72,7 +101,22 @@ public function setCurrentUser(AccountInterface $current_user = NULL) { * @see drupal_session_start() */ public function get($value = '') { - return Crypt::hmacBase64($value, session_id() . $this->privateKey->get() . drupal_get_hash_salt()); + // For mixed HTTP(S) sessions, use a constant identifier so that tokens can + // be shared between protocols. + $identifier = NULL; + if ($this->request->isSecure() && $this->settings->get('mixed_mode_sessions', FALSE)) { + $insecure_session_name = substr(session_name(), 1); + if ($this->request->cookies->has($insecure_session_name)) { + $identifier = $this->request->cookies->get($insecure_session_name); + } + } + + // Otherwise, use the session ID. + if (empty($identifier)) { + $identifier = session_id(); + } + + return Crypt::hmacBase64($value, $identifier . $this->privateKey->get() . drupal_get_hash_salt()); } /** diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php index 8e8a82a..3fbe146 100644 --- a/core/lib/Drupal/Core/Form/FormBuilder.php +++ b/core/lib/Drupal/Core/Form/FormBuilder.php @@ -814,6 +814,11 @@ public function validateForm($form_id, &$form, &$form_state) { } } + // Ensure the correct protocol when #https is set. + if (!empty($form['#https']) && !\Drupal::request()->isSecure() && settings()->get('mixed_mode_sessions', FALSE)) { + \Drupal::formBuilder()->setErrorByName('', $form_state, t('This form must be submitted over a secure connection.')); + } + // Recursively validate each form element. $this->doValidateForm($form, $form_state, $form_id); // After validation, loop through and assign each element its errors. diff --git a/core/modules/system/lib/Drupal/system/Tests/Session/SessionHttpsTest.php b/core/modules/system/lib/Drupal/system/Tests/Session/SessionHttpsTest.php index 02523d0..6f82cb4 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Session/SessionHttpsTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Session/SessionHttpsTest.php @@ -113,11 +113,6 @@ protected function testHttpsSession() { // Clear browser cookie jar. $this->cookies = array(); - if ($this->request->isSecure()) { - // The functionality does not make sense when running on HTTPS. - return; - } - // Enable secure pages. $this->settingsSet('mixed_mode_sessions', TRUE); // Write that value also into the test settings.php file. @@ -150,13 +145,23 @@ protected function testHttpsSession() { $this->drupalGet('user'); $form = $this->xpath('//form[@id="user-login-form"]'); $this->assertEqual(substr($form[0]['action'], 0, 6), 'https:', 'Login form action is secure'); - $form[0]['action'] = $this->httpsUrl('user'); + // Change the form to submit to an insecure URL and make sure the + // submission fails. + $form[0]['action'] = $this->httpUrl('user'); $edit = array( 'name' => $user->getUsername(), 'pass' => $user->pass_raw, ); $this->drupalPostForm(NULL, $edit, t('Log in')); + $this->assertText(t('This form must be submitted over a secure connection.'), 'Form submission failed over HTTP.'); + + // Now try with a secure URL and make sure the submission succeeds. + $form[0]['action'] = $this->httpsUrl('user'); + $this->drupalPost(NULL, $edit, t('Log in')); + $this->assertNoText(t('This form must be submitted over a secure connection.'), 'Form submission succeeded over HTTPS.'); + $this->assertLink(t('Log out'), 0, t('User %name successfully logged in.', array('%name' => $user->name))); + // Check secure cookie on secure page. $this->assertTrue($this->cookies[$secure_session_name]['secure'], 'The secure cookie has the secure attribute'); // Check insecure cookie on secure page. @@ -212,6 +217,19 @@ protected function testHttpsSession() { // Test that the user is also authenticated on the insecure site. $this->drupalGet("user/" . $user->id() . "/edit"); $this->assertResponse(200); + + // Test that tokens can be shared in mixed mode. + $this->drupalGet($this->httpUrl('session-test/shared-token')); + $matches = array(); + preg_match('/\s*sharedToken:(.*)\n/', $this->drupalGetContent(), $matches); + $this->assertTrue(!empty($matches[1]) , 'Found shared token on the HTTP URL.'); + $token_plain = $matches[1]; + $this->drupalGet($this->httpsUrl('session-test/shared-token')); + $matches = array(); + preg_match('/\s*sharedToken:(.*)\n/', $this->drupalGetContent(), $matches); + $this->assertTrue(!empty($matches[1]) , 'Found shared token on the HTTPS URL.'); + $token_secure = $matches[1]; + $this->assertEqual($token_plain, $token_secure, 'Tokens are shared in mixed HTTPS sessions.'); } /** diff --git a/core/modules/system/tests/modules/session_test/lib/Drupal/session_test/Controller/SessionTestController.php b/core/modules/system/tests/modules/session_test/lib/Drupal/session_test/Controller/SessionTestController.php index 6b00515..b417717 100644 --- a/core/modules/system/tests/modules/session_test/lib/Drupal/session_test/Controller/SessionTestController.php +++ b/core/modules/system/tests/modules/session_test/lib/Drupal/session_test/Controller/SessionTestController.php @@ -135,4 +135,14 @@ public function setNotStarted() { public function isLoggedIn() { return $this->t('User is logged in.'); } + + /** + * Display the result of Drupal::csrfToken()->get(). + * + * @return string + * A notification message with a CSRF token. + */ + public function sharedToken() { + return 'sharedToken:' . \Drupal::csrfToken()->get(); + } } diff --git a/core/modules/system/tests/modules/session_test/session_test.routing.yml b/core/modules/system/tests/modules/session_test/session_test.routing.yml index b1dc489..85ee080 100644 --- a/core/modules/system/tests/modules/session_test/session_test.routing.yml +++ b/core/modules/system/tests/modules/session_test/session_test.routing.yml @@ -75,3 +75,11 @@ session_test.is_logged_in: _content: '\Drupal\session_test\Controller\SessionTestController::isLoggedIn' requirements: _user_is_logged_in: 'TRUE' + +session_test.shared_token: + path: '/session-test/shared-token' + defaults: + _title: 'Test that tokens can be shared in mixed mode' + _controller: '\Drupal\session_test\Controller\SessionTestController::sharedToken' + requirements: + _permission: 'access content' diff --git a/core/tests/Drupal/Tests/Core/Access/CsrfTokenGeneratorTest.php b/core/tests/Drupal/Tests/Core/Access/CsrfTokenGeneratorTest.php index 3a6d75c..f362e5a 100644 --- a/core/tests/Drupal/Tests/Core/Access/CsrfTokenGeneratorTest.php +++ b/core/tests/Drupal/Tests/Core/Access/CsrfTokenGeneratorTest.php @@ -7,6 +7,7 @@ namespace Drupal\Tests\Core\Access { +use Drupal\Component\Utility\Settings; use Drupal\Tests\UnitTestCase; use Drupal\Core\Access\CsrfTokenGenerator; use Drupal\Component\Utility\Crypt; @@ -48,7 +49,12 @@ function setUp() { ->method('get') ->will($this->returnValue($this->key)); - $this->generator = new CsrfTokenGenerator($private_key); + $settings = new Settings(array()); + + $this->generator = new CsrfTokenGenerator($private_key, $settings); + + $request = new Request(); + $this->generator->setRequest($request); } /** @@ -58,6 +64,9 @@ public function testGet() { $this->assertInternalType('string', $this->generator->get()); $this->assertNotSame($this->generator->get(), $this->generator->get($this->randomName())); $this->assertNotSame($this->generator->get($this->randomName()), $this->generator->get($this->randomName())); + + // @todo Test code paths using Request::isSecure, settings, and cookie + // attributes. } /**