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

(#3243) Pass authstring (password_hash) for plugins in mysql_user #932

Conversation

jhriggs
Copy link
Contributor

@jhriggs jhriggs commented Mar 14, 2017

Various authentication plugins use the authentication string to pass additional settings (e.g. CREATE USER <user>@<host> IDENTIFIED WITH <plugin> AS <plugin_settings>). The mysql_user provider currently only passes this data for sha256_password. This change passes the authentication string for all plugins when specified.

This should allow, for example:

# Percona's auth_pam or auth_pam_compat:
mysql_user { 'foo@localhost':
  ensure        => present,
  password_hash => 'mysql, root=developer, users=data_entry',
  plugin        => 'auth_pam_compat',
  tls_options   => 'SSL',
}

# MySQL Enterprise's authentication_pam:
mysql_user { 'bar@localhost':
  ensure        => present,
  password_hash => 'mysql, root=developer, users=data_entry',
  plugin        => 'authentication_pam',
  tls_options   => 'SSL',
}

Various authentication plugins use the authentication string to pass additional settings (e.g. `CREATE USER <user>@<host> IDENTIFIED WITH <plugin> AS <plugin_settings>`). The mysql_user provider currently only passes this data for sha256_password. This change passes the authentication string for all plugins when specified.
…contain spaces, so base64 encode it in the query and decode the results
@jhriggs
Copy link
Contributor Author

jhriggs commented May 8, 2017

Any takers on this PR? It is a pretty innocuous change to the code for a much-needed feature.

Copy link
Contributor

@hunner hunner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be great! Would you be able to supply some examples for the README.md as well? And if possible an acceptance test would be great though if you have real-world examples of plugins being used that we could turn into acceptance tests ourselves that would be helpful as well.

if string.match(/^\*/)
mysql([defaults_file, system_database, '-e', "ALTER USER #{merged_name} IDENTIFIED WITH mysql_native_password AS '#{string}'"].compact)
else
raise ArgumentError, "Only mysql_native_password (*ABCD...XXX) hashes are supported"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just eyeballing it, but it looks like this removes validation for accidentally passing in non-hash passwords, which could cause confusion later down the road. Perhaps the validation should still be performed unless @resource[:plugin] == 'sha256_password'?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the docs https://dev.mysql.com/doc/refman/5.7/en/create-user.html the forms are IDENTIFIED WITH auth_plugin BY 'auth_string' and IDENTIFIED WITH auth_plugin AS 'hash_string'

Your pull request only uses the AS keyword so I'm guessing is still hashed, but the example you gave in the description is not hashed. Should they be?

Would it make more sense to introduce an auth_string parameter for strings other than password hashes?

@david22swan
Copy link
Member

@jhriggs Apologies but due to the amount of time that has gone by without you responding to the above requested changes we feel that we must close this PR. If in the future you wish to continue on this work we will of course be happy to review it once again.
Best Wishes

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

Successfully merging this pull request may close these issues.

None yet

3 participants