Skip to content

Commit

Permalink
Sanitize passwords by default (#3344)
Browse files Browse the repository at this point in the history
  • Loading branch information
weitzman authored Feb 3, 2018
1 parent 67c8f9e commit 704fd3f
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 68 deletions.
15 changes: 11 additions & 4 deletions src/Drupal/Commands/sql/SanitizeUserTableCommands.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Drupal\Core\Database\Database;
use Drush\Commands\DrushCommands;
use Drush\Sql\SqlBase;
use Drush\Utils\StringUtils;
use Symfony\Component\Console\Input\InputInterface;

/**
Expand Down Expand Up @@ -39,8 +40,13 @@ public function sanitize($result, CommandData $commandData)

// Sanitize passwords.
if ($this->isEnabled($options['sanitize-password'])) {
$password = $options['sanitize-password'];
if (is_null($password)) {
$password = StringUtils::generatePassword();
}

// Mimic Drupal's /scripts/password-hash.sh
$hash = $this->passwordHasher->hash($options['sanitize-password']);
$hash = $this->passwordHasher->hash($password);
$query->fields(['pass' => $hash]);
$messages[] = dt('User passwords sanitized.');
}
Expand Down Expand Up @@ -83,10 +89,11 @@ public function sanitize($result, CommandData $commandData)
* @option sanitize-email The pattern for test email addresses in the
* sanitization operation, or "no" to keep email addresses unchanged. May
* contain replacement patterns %uid, %mail or %name.
* @option sanitize-password The password to assign to all accounts in the
* sanitization operation, or "no" to keep passwords unchanged.
* @option sanitize-password
* By default, passwords are randomized. Specify 'no' to disable that. Specify any other value to set all passwords
* to that value.
*/
public function options($options = ['sanitize-email' => 'user+%[email protected]', 'sanitize-password' => 'password'])
public function options($options = ['sanitize-email' => 'user+%[email protected]', 'sanitize-password' => null])
{
}

Expand Down
124 changes: 61 additions & 63 deletions tests/SqlSyncTest.php
Original file line number Diff line number Diff line change
@@ -1,38 +1,38 @@
<?php

/**
* @file
* For now we only test sql-sync in simulated mode.
*
* Future: Using two copies of Drupal, we could test
* overwriting one site with another.
*/
* @file
* For now we only test sql-sync in simulated mode.
*
* Future: Using two copies of Drupal, we could test
* overwriting one site with another.
*/

namespace Unish;

/**
* @group slow
* @group commands
* @group sql
* @group slow
* @group commands
* @group sql
*/
class SqlSyncTest extends CommandUnishTestCase
{

public function testSimulatedSqlSync()
{
$fixtureSites = [
'remote' => [
'host' => 'server.isp.simulated',
'user' => 'www-admin',
'ssh' => [
'options' => '-o PasswordAuthentication=whatever'
],
'paths' => [
'drush-script' => '/path/to/drush',
],
],
'local' => [
],
'remote' => [
'host' => 'server.isp.simulated',
'user' => 'www-admin',
'ssh' => [
'options' => '-o PasswordAuthentication=whatever'
],
'paths' => [
'drush-script' => '/path/to/drush',
],
],
'local' => [
],
];
$this->setUpSettings($fixtureSites, 'synctest');
$options = [
Expand Down Expand Up @@ -64,30 +64,30 @@ public function testSimulatedSqlSync()
$this->assertContains("Simulating backend invoke: ssh -o PasswordAuthentication=no user@server 'drush --alias-path=__DIR__/resources/alias-fixtures:__SANDBOX__/etc/drush/sites --root=/path/to/drupal --uri=sitename --no-interaction sql:sync '\''@synctest.remote'\'' '\''@synctest.local'\''", $output);
}

/**
* Covers the following responsibilities.
* - A user created on the source site is copied to the destination site.
* - The email address of the copied user is sanitized on the destination site.
*
* General handling of site aliases will be in sitealiasTest.php.
*/
/**
* Covers the following responsibilities.
* - A user created on the source site is copied to the destination site.
* - The email address of the copied user is sanitized on the destination site.
*
* General handling of site aliases will be in sitealiasTest.php.
*/
public function testLocalSqlSync()
{
if ($this->dbDriver() == 'sqlite') {
$this->markTestSkipped('SQL Sync does not apply to SQLite.');
return;
}

$sites = $this->setUpDrupal(2, true);
$this->setUpDrupal(2, true);
return $this->localSqlSync();
}

public function localSqlSync()
{

$options = [
'uri' => 'stage',
'yes' => null,
'uri' => 'stage',
'yes' => null,
];

// Create a user in the staging site
Expand All @@ -96,19 +96,13 @@ public function localSqlSync()

// Add user fields and a test User.
$this->drush('pm-enable', ['field,text,telephone,comment'], $options + ['yes' => null]);
$this->drush('php-script', [
'user_fields-D' . UNISH_DRUPAL_MAJOR_VERSION,
$name,
$mail
], $options + [
'script-path' => __DIR__ . '/resources',
]);
$this->drush('php-script', ['user_fields-D' . UNISH_DRUPAL_MAJOR_VERSION, $name, $mail], $options + ['script-path' => __DIR__ . '/resources',]);

// Copy stage to dev, and then sql:sanitize.
$sync_options = [
'yes' => null,
// Test wildcards expansion from within sql-sync. Also avoid D8 persistent entity cache.
'structure-tables-list' => 'cache,cache*',
'yes' => null,
// Test wildcards expansion from within sql-sync. Also avoid D8 persistent entity cache.
'structure-tables-list' => 'cache,cache*',
];
$this->drush('sql-sync', ['@unish.stage', '@unish.dev'], $sync_options);
$this->drush('sql-sanitize', [], ['yes' => null], '@unish.dev');
Expand All @@ -118,18 +112,22 @@ public function localSqlSync()
$info = $this->getOutputFromJSON(2);
$this->assertEquals($mail, $info->mail, 'Email address is unchanged on source site.');
$this->assertEquals($name, $info->name);
// Get the unchanged pass.
$this->drush('user-information', [$name], $options + ['field' => 'pass']);
$original_hashed_pass = $this->getOutput();

// Confirm that the sample user's email address has been sanitized on the dev site
$this->drush('user-information', [$name], $options + ['format' => 'json', 'yes' => null], '@unish.dev');
// Confirm that the sample user's email and password have been sanitized on the dev site
$this->drush('user-information', [$name], $options + ['fields' => 'uid,name,mail,pass', 'format' => 'json', 'yes' => null], '@unish.dev');
$info = $this->getOutputFromJSON(2);
$this->assertEquals("[email protected]", $info->mail, 'Email address was sanitized on destination site.');
$this->assertEquals($name, $info->name);
$this->assertNotEquals($info->pass, $original_hashed_pass);

// Copy stage to dev with --sanitize and a fixed sanitized email
$sync_options = [
'yes' => null,
// Test wildcards expansion from within sql-sync. Also avoid D8 persistent entity cache.
'structure-tables-list' => 'cache,cache*',
'yes' => null,
// Test wildcards expansion from within sql-sync. Also avoid D8 persistent entity cache.
'structure-tables-list' => 'cache,cache*',
];
$this->drush('sql-sync', ['@unish.stage', '@unish.dev'], $sync_options);
$this->drush('sql-sanitize', [], ['yes' => null, 'sanitize-email' => '[email protected]'], '@unish.dev');
Expand All @@ -142,12 +140,12 @@ public function localSqlSync()


$fields = [
'field_user_email' => '[email protected]',
'field_user_string' => 'Private info',
'field_user_string_long' => 'Really private info',
'field_user_text' => 'Super private info',
'field_user_text_long' => 'Super duper private info',
'field_user_text_with_summary' => 'Private',
'field_user_email' => '[email protected]',
'field_user_string' => 'Private info',
'field_user_string_long' => 'Really private info',
'field_user_text' => 'Super private info',
'field_user_text_long' => 'Super duper private info',
'field_user_text_with_summary' => 'Private',
];
// Assert that field DO NOT contain values.
foreach ($fields as $field_name => $value) {
Expand All @@ -158,21 +156,21 @@ public function localSqlSync()
$this->assertUserFieldContents('field_user_telephone', '5555555555', true);
}

/**
* Assert that a field on the user entity does or does not contain a value.
*
* @param string $field_name
* The machine name of the field.
* @param string $value
* The field value.
* @param bool $should_contain
* Whether the field should contain the value. Defaults to false.
*/
/**
* Assert that a field on the user entity does or does not contain a value.
*
* @param string $field_name
* The machine name of the field.
* @param string $value
* The field value.
* @param bool $should_contain
* Whether the field should contain the value. Defaults to false.
*/
public function assertUserFieldContents($field_name, $value, $should_contain = false)
{
$table = 'user__' . $field_name;
$column = $field_name . '_value';
$this->drush('sql-query', [ "SELECT $column FROM $table LIMIT 1" ], [], '@unish.dev');
$this->drush('sql-query', ["SELECT $column FROM $table LIMIT 1"], [], '@unish.dev');
$output = $this->getOutput();
$this->assertNotEmpty($output);

Expand Down
2 changes: 1 addition & 1 deletion tests/resources/user_fields-D8.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
$user = User::create([
'name' => $extra[0],
'mail' => $extra[1],
'pass' => 'password',
'pass' => 'pw',
]);

foreach ($values as $field_name => $value) {
Expand Down

0 comments on commit 704fd3f

Please sign in to comment.