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.

Sign up for GitHub

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

feat[mysql_user]: add support for mysql user attributes #604

Merged
merged 6 commits into from
Jan 19, 2024
Merged

feat[mysql_user]: add support for mysql user attributes #604

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5273712
add support for mysql user attributes
Jan 9, 2024
7012a27
fix CI
Jan 10, 2024
9a8b53d
write integration tests
Jan 11, 2024
c35bfb9
requested changes pt. 1
Jan 12, 2024
55e896e
requested changes pt. 2
Jan 17, 2024
208719f
fix changelog fragment
Jan 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
177 changes: 126 additions & 51 deletions plugins/module_utils/user.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
# Simplified BSD License (see simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause)

import string
import json
import re

from ansible.module_utils.six import iteritems
Expand Down Expand Up @@ -151,14 +152,20 @@ def get_existing_authentication(cursor, user, host):

def user_add(cursor, user, host, host_all, password, encrypted,
plugin, plugin_hash_string, plugin_auth_string, new_priv,
tls_requires, check_mode, reuse_existing_password):
attributes, tls_requires, reuse_existing_password, module):
# we cannot create users without a proper hostname
if host_all:
return {'changed': False, 'password_changed': False}

if check_mode:
if module.check_mode:
return {'changed': True, 'password_changed': None}

# If attributes are set, perform a sanity check to ensure server supports user attributes before creating user
if attributes and not get_attribute_support(cursor):
module.fail_json(msg="user attributes were specified but the mysql server does not support user attributes")

final_attributes = {}

# Determine what user management method server uses
old_user_mgmt = impl.use_old_user_mgmt(cursor)

Expand Down Expand Up @@ -203,9 +210,13 @@ def user_add(cursor, user, host, host_all, password, encrypted,
if new_priv is not None:
for db_table, priv in iteritems(new_priv):
privileges_grant(cursor, user, host, db_table, priv, tls_requires)
if attributes is not None:
cursor.execute("ALTER USER %s@%s ATTRIBUTE %s", (user, host, json.dumps(attributes)))
final_attributes = attributes_get(cursor, user, host)
if tls_requires is not None:
privileges_grant(cursor, user, host, "*.*", get_grants(cursor, user, host), tls_requires)
return {'changed': True, 'password_changed': not used_existing_password}

return {'changed': True, 'password_changed': not used_existing_password, 'attributes': final_attributes}


def is_hash(password):
Expand All @@ -218,7 +229,7 @@ def is_hash(password):

def user_mod(cursor, user, host, host_all, password, encrypted,
plugin, plugin_hash_string, plugin_auth_string, new_priv,
append_privs, subtract_privs, tls_requires, module, role=False, maria_role=False):
append_privs, subtract_privs, attributes, tls_requires, module, role=False, maria_role=False):
changed = False
msg = "User unchanged"
grant_option = False
Expand Down Expand Up @@ -278,27 +289,26 @@ def user_mod(cursor, user, host, host_all, password, encrypted,
if current_pass_hash != encrypted_password:
password_changed = True
msg = "Password updated"
if module.check_mode:
return {'changed': True, 'msg': msg, 'password_changed': password_changed}
if old_user_mgmt:
cursor.execute("SET PASSWORD FOR %s@%s = %s", (user, host, encrypted_password))
msg = "Password updated (old style)"
else:
try:
cursor.execute("ALTER USER %s@%s IDENTIFIED WITH mysql_native_password AS %s", (user, host, encrypted_password))
msg = "Password updated (new style)"
except (mysql_driver.Error) as e:
# https://stackoverflow.com/questions/51600000/authentication-string-of-root-user-on-mysql
# Replacing empty root password with new authentication mechanisms fails with error 1396
if e.args[0] == 1396:
cursor.execute(
"UPDATE mysql.user SET plugin = %s, authentication_string = %s, Password = '' WHERE User = %s AND Host = %s",
('mysql_native_password', encrypted_password, user, host)
)
cursor.execute("FLUSH PRIVILEGES")
msg = "Password forced update"
else:
raise e
if not module.check_mode:
if old_user_mgmt:
cursor.execute("SET PASSWORD FOR %s@%s = %s", (user, host, encrypted_password))
msg = "Password updated (old style)"
else:
try:
cursor.execute("ALTER USER %s@%s IDENTIFIED WITH mysql_native_password AS %s", (user, host, encrypted_password))
msg = "Password updated (new style)"
except (mysql_driver.Error) as e:
# https://stackoverflow.com/questions/51600000/authentication-string-of-root-user-on-mysql
# Replacing empty root password with new authentication mechanisms fails with error 1396
if e.args[0] == 1396:
cursor.execute(
"UPDATE mysql.user SET plugin = %s, authentication_string = %s, Password = '' WHERE User = %s AND Host = %s",
('mysql_native_password', encrypted_password, user, host)
)
cursor.execute("FLUSH PRIVILEGES")
msg = "Password forced update"
else:
raise e
changed = True

# Handle plugin authentication
Expand Down Expand Up @@ -352,9 +362,8 @@ def user_mod(cursor, user, host, host_all, password, encrypted,
if db_table not in new_priv:
if user != "root" and "PROXY" not in priv:
msg = "Privileges updated"
if module.check_mode:
return {'changed': True, 'msg': msg, 'password_changed': password_changed}
privileges_revoke(cursor, user, host, db_table, priv, grant_option, maria_role)
if not module.check_mode:
privileges_revoke(cursor, user, host, db_table, priv, grant_option, maria_role)
changed = True

# If the user doesn't currently have any privileges on a db.table, then
Expand All @@ -363,9 +372,8 @@ def user_mod(cursor, user, host, host_all, password, encrypted,
for db_table, priv in iteritems(new_priv):
if db_table not in curr_priv:
msg = "New privileges granted"
if module.check_mode:
return {'changed': True, 'msg': msg, 'password_changed': password_changed}
privileges_grant(cursor, user, host, db_table, priv, tls_requires, maria_role)
if not module.check_mode:
privileges_grant(cursor, user, host, db_table, priv, tls_requires, maria_role)
changed = True

# If the db.table specification exists in both the user's current privileges
Expand Down Expand Up @@ -404,42 +412,66 @@ def user_mod(cursor, user, host, host_all, password, encrypted,

if len(grant_privs) + len(revoke_privs) > 0:
msg = "Privileges updated: granted %s, revoked %s" % (grant_privs, revoke_privs)
if module.check_mode:
return {'changed': True, 'msg': msg, 'password_changed': password_changed}
if len(revoke_privs) > 0:
privileges_revoke(cursor, user, host, db_table, revoke_privs, grant_option, maria_role)
if len(grant_privs) > 0:
privileges_grant(cursor, user, host, db_table, grant_privs, tls_requires, maria_role)
if not module.check_mode:
if len(revoke_privs) > 0:
privileges_revoke(cursor, user, host, db_table, revoke_privs, grant_option, maria_role)
if len(grant_privs) > 0:
privileges_grant(cursor, user, host, db_table, grant_privs, tls_requires, maria_role)

# after privilege manipulation, compare privileges from before and now
after_priv = privileges_get(cursor, user, host, maria_role)
changed = changed or (curr_priv != after_priv)

# Handle attributes
attribute_support = get_attribute_support(cursor)

if attributes:
if not attribute_support:
module.fail_json(msg="user attributes were specified but the mysql server does not support user attributes")
n-cc marked this conversation as resolved.
Show resolved Hide resolved
else:
current_attributes = attributes_get(cursor, user, host)
attributes_to_change = {}

for key, value in attributes.items():
if key not in current_attributes or current_attributes[key] != value:
# The mysql null value (None in python) is used to delete an attribute; we use False to represent None in the attributes parameter
attributes_to_change[key] = None if not value else value

if attributes_to_change:
msg = "Attributes updated: %s" % (", ".join(["%s: %s" % (key, value) for key, value in attributes_to_change.items()]))
if not module.check_mode:
cursor.execute("ALTER USER %s@%s ATTRIBUTE %s", (user, host, json.dumps(attributes_to_change)))
changed = True

if attribute_support:
final_attributes = attributes_get(cursor, user, host)
else:
final_attributes = {}

if role:
continue

# Handle TLS requirements
current_requires = get_tls_requires(cursor, user, host)
if current_requires != tls_requires:
msg = "TLS requires updated"
if module.check_mode:
return {'changed': True, 'msg': msg, 'password_changed': password_changed}
if not old_user_mgmt:
pre_query = "ALTER USER"
else:
pre_query = "GRANT %s ON *.* TO" % ",".join(get_grants(cursor, user, host))
if not module.check_mode:
if not old_user_mgmt:
pre_query = "ALTER USER"
else:
pre_query = "GRANT %s ON *.* TO" % ",".join(get_grants(cursor, user, host))

if tls_requires is not None:
query = " ".join((pre_query, "%s@%s"))
query_with_args = mogrify_requires(query, (user, host), tls_requires)
else:
query = " ".join((pre_query, "%s@%s REQUIRE NONE"))
query_with_args = query, (user, host)
if tls_requires is not None:
query = " ".join((pre_query, "%s@%s"))
query_with_args = mogrify_requires(query, (user, host), tls_requires)
else:
query = " ".join((pre_query, "%s@%s REQUIRE NONE"))
query_with_args = query, (user, host)

cursor.execute(*query_with_args)
cursor.execute(*query_with_args)
changed = True

return {'changed': changed, 'msg': msg, 'password_changed': password_changed}
return {'changed': changed, 'msg': msg, 'password_changed': password_changed, 'attributes': final_attributes}


def user_delete(cursor, user, host, host_all, check_mode):
Expand Down Expand Up @@ -924,6 +956,49 @@ def limit_resources(module, cursor, user, host, resource_limits, check_mode):
return True


def get_attribute_support(cursor):
"""Checks if the MySQL server supports user attributes.

Args:
cursor (cursor): DB driver cursor object.
Returns:
True if attributes are supported, False if they are not.
"""

try:
# information_schema.tables does not hold the tables within information_schema itself
cursor.execute("SELECT attribute FROM INFORMATION_SCHEMA.USER_ATTRIBUTES LIMIT 0")
cursor.fetchone()
except mysql_driver.OperationalError:
return False

return True

def attributes_get(cursor, user, host):
"""Get attributes for a given user.

Args:
cursor (cursor): DB driver cursor object.
user (str): User name.
host (str): User host name.

Returns:
None if the user does not exist, otherwise a dict of attributes set on the user
"""
cursor.execute("SELECT attribute FROM INFORMATION_SCHEMA.USER_ATTRIBUTES WHERE user = %s AND host = %s", (user, host))

r = cursor.fetchone()

if r:
attributes = r[0]
# convert JSON string stored in row into a dict - mysql enforces that user_attributes entires are in JSON format
if attributes:
return json.loads(attributes)
else:
return {}

return None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if r:
attributes = r[0]
# convert JSON string stored in row into a dict - mysql enforces that user_attributes entires are in JSON format
if attributes:
return json.loads(attributes)
else:
return {}
return None
# convert JSON string stored in row into a dict - mysql enforces that user_attributes entires are in JSON format
return json.loads(r[0]) if r and r[0] else None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this will return None rather than an empty dict if the user exists but does not have any attributes set - are you recommending returning None over {} in that scenario?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yep

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That should be fine, but there are a few things to keep in mind about None vs {} when it comes to attributes... if a CREATE USER is ran without the ATTRIBUTES '{}' option, the user's attributes are stored as NULL until attributes are set, and if all attributes are deleted, attributes will then be stored as {} and cannot be changed back to NULL (as far as I can tell):

mysql> CREATE USER 'testuser'@'localhost';
Query OK, 0 rows affected (0.00 sec)

mysql> SELECT ATTRIBUTE from INFORMATION_SCHEMA.USER_ATTRIBUTES WHERE user = 'testuser' and host = 'localhost';
+-----------+
| ATTRIBUTE |
+-----------+
| NULL      |
+-----------+
1 row in set (0.00 sec)

mysql> ALTER USER 'testuser'@'localhost' ATTRIBUTE '{ "foo": "bar" }';
Query OK, 0 rows affected (0.00 sec)

mysql> SELECT ATTRIBUTE from INFORMATION_SCHEMA.USER_ATTRIBUTES WHERE user = 'testuser' and host = 'localhost';
+----------------+
| ATTRIBUTE      |
+----------------+
| {"foo": "bar"} |
+----------------+
1 row in set (0.00 sec)

mysql> ALTER USER 'testuser'@'localhost' ATTRIBUTE '{ "foo": null }';
Query OK, 0 rows affected (0.00 sec)

mysql> SELECT ATTRIBUTE from INFORMATION_SCHEMA.USER_ATTRIBUTES WHERE user = 'testuser' and host = 'localhost';
+-----------+
| ATTRIBUTE |
+-----------+
| {}        |
+-----------+
1 row in set (0.00 sec)

mysql>

I think there are a couple different options:

  1. Represent no attributes as {} even if the ATTRIBUTE database value is NULL
    1. This is what I was doing originally; I chose this method so that someone using register: foo with mysql_user could still treat foo.attributes as a dictionary even if the ATTRIBUTE value is NULL. It also makes the code a bit cleaner if attributes_get always returns a dict for users that exist.
    2. With this method, we could also explicitly run the CREATE USER query in user_add with ATTRIBUTES {} even if no attributes are specified, so that the ATTRIBUTE database value will always be {} for ansible-created users on servers with attribute support.
  2. Represent no attributes as None even if the ATTRIBUTE database value is {} (I think this is what you were proposing)
  3. Represent no attributes as either None or {} depending on the database value

Option 2 is how this feature behaves as of my latest commit (I also wrote tests to assert the attributes return value for users without attributes). I think any of these options are fine depending on ansible best practices. I can stick with option 2 if you think it's preferred, I just wanted to make sure this caveat was documented in this PR.


def get_impl(cursor):
global impl
cursor.execute("SELECT VERSION()")
Expand Down
28 changes: 23 additions & 5 deletions plugins/modules/mysql_user.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@
- Cannot be used to set global variables, use the M(community.mysql.mysql_variables) module instead.
type: dict
version_added: '3.6.0'

column_case_sensitive:
description:
- The default is C(false).
Expand All @@ -165,6 +164,13 @@
fields names in privileges.
type: bool
version_added: '3.8.0'
attributes:
description:
- Create, update, or delete user attributes (arbitrary "key: value" comments) for the user.
- MySQL server must support the INFORMATION_SCHEMA.USER_ATTRIBUTES table. Provided since MySQL 8.0.
- To delete an existing attribute, set its value to False.
n-cc marked this conversation as resolved.
Show resolved Hide resolved
type: dict
version_added: '3.9.0'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not bump the version in galaxy.yml yet; unsure what the process is for shipping new versions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

3.9.0 is OK here


notes:
- "MySQL server installs with default I(login_user) of C(root) and no password.
Expand Down Expand Up @@ -257,6 +263,13 @@
FUNCTION my_db.my_function: EXECUTE
state: present

- name: Modify user attributes, creating the attribute 'foo' and removing the attribute 'bar'
community.mysql.mysql_user:
name: bob
attributes:
foo: "foo"
bar: False
n-cc marked this conversation as resolved.
Show resolved Hide resolved

- name: Modify user to require TLS connection with a valid client certificate
community.mysql.mysql_user:
name: bob
Expand Down Expand Up @@ -405,6 +418,7 @@ def main():
tls_requires=dict(type='dict'),
append_privs=dict(type='bool', default=False),
subtract_privs=dict(type='bool', default=False),
attributes=dict(type='dict'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can change the parameter ordering here if desired.

Copy link
Collaborator

Choose a reason for hiding this comment

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

that's fine now

check_implicit_admin=dict(type='bool', default=False),
update_password=dict(type='str', default='always', choices=['always', 'on_create', 'on_new_username'], no_log=False),
sql_log_bin=dict(type='bool', default=True),
Expand Down Expand Up @@ -437,6 +451,7 @@ def main():
append_privs = module.boolean(module.params["append_privs"])
subtract_privs = module.boolean(module.params['subtract_privs'])
update_password = module.params['update_password']
attributes = module.params['attributes']
ssl_cert = module.params["client_cert"]
ssl_key = module.params["client_key"]
ssl_ca = module.params["ca_cert"]
Expand Down Expand Up @@ -500,21 +515,23 @@ def main():

priv = privileges_unpack(priv, mode, column_case_sensitive, ensure_usage=not subtract_privs)
password_changed = False
final_attributes = {}
if state == "present":
if user_exists(cursor, user, host, host_all):
try:
if update_password == "always":
result = user_mod(cursor, user, host, host_all, password, encrypted,
plugin, plugin_hash_string, plugin_auth_string,
priv, append_privs, subtract_privs, tls_requires, module)
priv, append_privs, subtract_privs, attributes, tls_requires, module)

else:
result = user_mod(cursor, user, host, host_all, None, encrypted,
None, None, None,
priv, append_privs, subtract_privs, tls_requires, module)
priv, append_privs, subtract_privs, attributes, tls_requires, module)
changed = result['changed']
msg = result['msg']
password_changed = result['password_changed']
final_attributes = result['attributes']

except (SQLParseError, InvalidPrivsError, mysql_driver.Error) as e:
module.fail_json(msg=to_native(e))
Expand All @@ -527,9 +544,10 @@ def main():
reuse_existing_password = update_password == 'on_new_username'
result = user_add(cursor, user, host, host_all, password, encrypted,
plugin, plugin_hash_string, plugin_auth_string,
priv, tls_requires, module.check_mode, reuse_existing_password)
priv, attributes, tls_requires, reuse_existing_password, module)
changed = result['changed']
password_changed = result['password_changed']
final_attributes = result['attributes']
if changed:
msg = "User added"

Expand All @@ -546,7 +564,7 @@ def main():
else:
changed = False
msg = "User doesn't exist"
module.exit_json(changed=changed, user=user, msg=msg, password_changed=password_changed)
module.exit_json(changed=changed, user=user, msg=msg, password_changed=password_changed, attributes=final_attributes)


if __name__ == '__main__':
Expand Down
Loading