DrupalWebTestCase::drupalGetToken() does not add hash salt

Why do we have this method in the first place?

+++ b/core/modules/simpletest/drupal_web_test_case.php
@@ -1256,7 +1256,7 @@ class DrupalWebTestCase extends DrupalTestCase {
   protected function drupalGetToken($value = '') {
     $private_key = drupal_get_private_key();
-    return drupal_hmac_base64($value, $this->session_id . $private_key);
+    return drupal_hmac_base64($value, $this->session_id . $private_key . drupal_get_hash_salt());
   }

+++ b/core/modules/simpletest/simpletest.test
@@ -318,6 +318,17 @@ class SimpleTestFunctionalTest extends DrupalWebTestCase {
+  function testAliasCode() {
+    // Test security token generation.
+    $session_id = session_id();
+    session_id($this->session_id);
+    $this->assertEqual(drupal_get_token('foo'), $this->drupalGetToken('foo'), 'DrupalWebTestCase::drupalGetToken() and drupal_get_token() generate identical tokens.');
+    session_id($session_id);
+  }

The problem is: drupalGetToken() does not do what testAliasCode() is doing.

Thus, it'll never work, except if you replicate all of the testAliasCode() code. But if you do that, then you no longer need drupalGetToken(), because you can directly call drupal_get_token().

Nope, this patch is correct. Can you re-roll (also, without the EOF newline error)?

While this patch fixes drupalGetToken(), there's actually another problem in that $this->session_id gets reset to NULL on subsequently performed requests after drupalLogin().

Curious, what breaks?

And once more without irrelevant changes.

Not sure that $this->session_id is set by simpletest... let's see
PS: masquerade module tests depends on this issue #1835954: Cleanup code and allow pass tests

$this->session_id is definitely set by WebTestBase's curl methods. That's also why this patch removes an unset() there. ;)

@sun Not sure about 8.x but in 7.x $this->session_id is empty - try debug($this->session_id)

EDIT: Probably $this->session_name is wrong too

+++ b/core/modules/simpletest/lib/Drupal/simpletest/Tests/SimpleTestTest.phpundefined
@@ -334,4 +334,30 @@ function asText(SimpleXMLElement $element) {
+    // Verify that session ID is reset upon logout.
+    $this->drupalLogout();
+    $this->assertFalse($this->session_id);
+  }

Also better to have a check that session_id() !== $this->session_id

+++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.phpundefined
@@ -921,8 +921,7 @@ protected function curlExec($curl_options, $redirect = FALSE) {
-      $this->session_id = NULL;

This really needs more testing? but actually help to pass tests

#11: drupal8.test-drupalgettoken.11.patch queued for re-testing.

#11: drupal8.test-drupalgettoken.11.patch queued for re-testing.

Fixes tests, still can't find a reason why session tests are broken
Related issue commited #1739986: Fix fallback in drupal_get_hash_salt(), move it to bootstrap.inc, use instead of $GLOBALS['drupal_hash_salt']

Patch no longer applies.

Triggering testbot.

27: simpletest-add-hash-salt-1555862-27.patch queued for re-testing.

Re-roll of last patch.

Drupal 7 is also affected by this. Patch contains D7 backport of the fix. I have not backported the test.

This is never going to be fixed in Drupal 8, because in #2328995: Remove unused and defective WebTestBase::drupalGetToken() the whole function drupalGetToken() has been removed.
Re-uploaded the patch from comment #39 and removed the do-not-test naming part.

The function drupal_get_token() builds the token is the same way. So for me it is RTBC.

+1 here, that's used in test for masquerade module

PS: Currently in D8 there's no way to build csrf token properly, within test, so added tag

@Xano: Considering this helper was removed from D8, I would consider them two separate issues at this point. Not having this in D7 core is a major blocker for testing in contrib modules.

We will need a test for D7 now.