Issue #2207585 by damiankloip, Xano, dawehner: Find a new OO home for drupal_get_hash_salt().

core/includes/bootstrap.inc

@@ -1499,15 +1499,12 @@ function drupal_get_user_timezone() {
  *
  * @return
  *   A salt based on information in settings.php, not in the database.
+ *
+ * @deprecated in Drupal 8.x-dev, will be removed before Drupal 8.0. Use
+ *   \Drupal\Core\Site\Settings::getHashSalt() instead.
  */
 function drupal_get_hash_salt() {
-  $hash_salt = Settings::get('hash_salt');
-  // This should never happen, as it breaks user logins and many other services.
-  // Therefore, explicitly notify the user (developer) by throwing an exception.
-  if (empty($hash_salt)) {
-    throw new \RuntimeException('Missing $settings[\'hash_salt\'] in settings.php.');
-  }
-  return $hash_salt;
+  return Settings::getHashSalt();
 }
 
 /**

core/lib/Drupal/Core/Access/CsrfTokenGenerator.php

@@ -10,6 +10,7 @@
 use Drupal\Component\Utility\Crypt;
 use Drupal\Core\PrivateKey;
 use Drupal\Core\Session\AccountInterface;
+use Drupal\Core\Site\Settings;
 
 /**
  * Generates and validates CSRF tokens.
@@ -95,7 +96,7 @@ public function validate($token, $value = '') {
    *   'drupal_private_key' configuration variable.
    */
   protected function computeToken($seed, $value = '') {
-    return Crypt::hmacBase64($value, $seed . $this->privateKey->get() . drupal_get_hash_salt());
+    return Crypt::hmacBase64($value, $seed . $this->privateKey->get() . Settings::getHashSalt());
   }
 
 }

core/lib/Drupal/Core/Site/Settings.php

@@ -80,4 +80,24 @@ public static function getAll() {
     return self::$instance->storage;
   }
 
+  /**
+   * Gets a salt useful for hardening against SQL injection.
+   *
+   * @return string
+   *   A salt based on information in settings.php, not in the database.
+   *
+   * @throws \RuntimeException
+   */
+  public static function getHashSalt() {
+    $hash_salt = self::$instance->get('hash_salt');
+    // This should never happen, as it breaks user logins and many other
+    // services. Therefore, explicitly notify the user (developer) by throwing
+    // an exception.
+    if (empty($hash_salt)) {
+      throw new \RuntimeException('Missing $settings[\'hash_salt\'] in settings.php.');
+    }
+
+    return $hash_salt;
+  }
+
 }

core/tests/Drupal/Tests/Core/Access/CsrfTokenGeneratorTest.php

@@ -5,8 +5,9 @@
  * Contains \Drupal\Tests\Core\Access\CsrfTokenGeneratorTest.
  */
 
-namespace Drupal\Tests\Core\Access {
+namespace Drupal\Tests\Core\Access;
 
+use Drupal\Core\Site\Settings;
 use Drupal\Tests\UnitTestCase;
 use Drupal\Core\Access\CsrfTokenGenerator;
 use Drupal\Component\Utility\Crypt;
@@ -24,6 +25,13 @@ class CsrfTokenGeneratorTest extends UnitTestCase {
    */
   protected $generator;
 
+  /**
+   * The mock private key instance.
+   *
+   * @var \Drupal\Core\PrivateKey|\PHPUnit_Framework_MockObject_MockObject
+   */
+  protected $privateKey;
+
   public static function getInfo() {
     return array(
       'name' => 'CsrfTokenGenerator test',
@@ -39,16 +47,22 @@ function setUp() {
     parent::setUp();
     $this->key = Crypt::randomBytesBase64(55);
 
-    $private_key = $this->getMockBuilder('Drupal\Core\PrivateKey')
+    $this->privateKey = $this->getMockBuilder('Drupal\Core\PrivateKey')
       ->disableOriginalConstructor()
       ->setMethods(array('get'))
       ->getMock();
 
-    $private_key->expects($this->any())
+    $this->privateKey->expects($this->any())
       ->method('get')
       ->will($this->returnValue($this->key));
 
-    $this->generator = new CsrfTokenGenerator($private_key);
+    $settings = array(
+      'hash_salt' => $this->randomName(),
+    );
+
+    new Settings($settings);
+
+    $this->generator = new CsrfTokenGenerator($this->privateKey);
   }
 
   /**
@@ -141,17 +155,16 @@ public function providerTestInvalidParameterTypes() {
     );
   }
 
-}
-
-}
-
-/**
- * @todo Remove this when https://drupal.org/node/2036259 is resolved.
- */
-namespace {
-  if (!function_exists('drupal_get_hash_salt')) {
-    function drupal_get_hash_salt() {
-      return hash('sha256', 'test_hash_salt');
-    }
+  /**
+   * Tests the exception thrown when no 'hash_salt' is provided in settings.
+   *
+   * @expectedException \RuntimeException
+   */
+  public function testGetWithNoHashSalt() {
+    // Update settings with no hash salt.
+    new Settings(array());
+    $generator = new CsrfTokenGenerator($this->privateKey);
+    $generator->get();
   }
+
 }

core/tests/Drupal/Tests/Core/Site/SettingsTest.php

@@ -13,6 +13,8 @@
 /**
  * Tests read-only settings.
  *
+ * @group Drupal
+ *
  * @coversDefaultClass \Drupal\Core\Site\Settings
  */
 class SettingsTest extends UnitTestCase {
@@ -49,6 +51,7 @@ public function setUp(){
     $this->config = array(
       'one' => '1',
       'two' => '2',
+      'hash_salt' => $this->randomName(),
     );
     $this->settings = new Settings($this->config);
   }
@@ -58,7 +61,7 @@ public function setUp(){
    */
   public function testGet() {
     // Test stored settings.
-    $this->assertEquals($this->config['one'], Settings::get('one'), 'The correect setting was not returned.');
+    $this->assertEquals($this->config['one'], Settings::get('one'), 'The correct setting was not returned.');
     $this->assertEquals($this->config['two'], Settings::get('two'), 'The correct setting was not returned.');
 
     // Test setting that isn't stored with default.
@@ -81,4 +84,41 @@ public function testGetInstance() {
     $this->assertEquals($singleton, $this->settings);
   }
 
+  /**
+   * Tests Settings::getHashSalt();
+   *
+   * @covers ::getHashSalt
+   */
+  public function testGetHashSalt() {
+    $this->assertSame($this->config['hash_salt'], $this->settings->getHashSalt());
+  }
+
+  /**
+   * Tests Settings::getHashSalt() with no hash salt value.
+   *
+   * @covers ::getHashSalt
+   *
+   * @dataProvider providerTestGetHashSaltEmpty
+   *
+   * @expectedException \RuntimeException
+   */
+  public function testGetHashSaltEmpty(array $config) {
+    // Re-create settings with no 'hash_salt' key.
+    $settings = new Settings($config);
+    $settings->getHashSalt();
+  }
+
+  /**
+   * Data provider for testGetHashSaltEmpty.
+   *
+   * @return array
+   */
+  public function providerTestGetHashSaltEmpty() {
+   return array(
+     array(array()),
+     array(array('hash_salt' => '')),
+     array(array('hash_salt' => NULL)),
+   );
+  }
+
 }