Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drush 9: passwords should be scrambled by default when running sql-sanitize #3086

Closed
timcosgrove opened this issue Oct 20, 2017 · 5 comments
Closed

Comments

@timcosgrove
Copy link

timcosgrove commented Oct 20, 2017

(Reposted from Drupal slack #drush:)

Minor drush sql-sanitize suggestion:
https://github.com/drush-ops/drush/blob/master/src/Drupal/Commands/sql/SanitizeUserTableCommands.php#L89

The default for password is the literal string "password". This means that, under default options for sql-sanitize, given a sanitized database, anyone with access can log in as any user. This feels like a security risk, as in theory users with access to lower lifecycle environments could log in as admin and gain knowledge of a production system's settings; or, could log in as other users, and gain knowledge of their correspondance, their own user settings, etc (which should maybe also be sanitized).

Would it make sense to create a randomized password by default? Then not only would the users' passwords be scrubbed, but their accounts would become inaccessible (at least without direct DB access, at which point all bets are off.)

I'll provide a PR. Putting this here for documentation's sake.

@weitzman
Copy link
Member

weitzman commented Feb 1, 2018

I'm happy to change this behavior as suggested. PRs welcome.

@damienmckenna
Copy link

damienmckenna commented Feb 2, 2018

Should this be backported to Drush 8 (or older?) given how few installs of Drush 9 there are and will be?

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Feb 2, 2018

Backports to Drush 8 are still being accepted.

@CashWilliams
Copy link
Contributor

CashWilliams commented Feb 2, 2018

Tried to reference this issue in #3341 but did it wrong I guess

@CashWilliams
Copy link
Contributor

CashWilliams commented Feb 8, 2018

I think this can be closed.

@weitzman weitzman closed this as completed Feb 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants