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

mysql_user: fix column specific grants #58928

Open
wants to merge 5 commits into
base: devel
from

Conversation

Projects
None yet
2 participants
@twouters
Copy link

commented Jul 10, 2019

SUMMARY

mysql_user plugin has some issues with handling column specific grants:

  • MySQL doesn't keep track of the order of specified columns which results in ansible reporting the task as changed, even though the required grants are set. This change splits the columns and sorts them to work around this.

  • If a column name matches a reserved keyword, ansible fails because of the lack of quotes. Add quotes where needed, depending on the mode (ANSI quoting or regular).

Fixes #25158
Fixes #40879

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

mysql_user

ADDITIONAL INFORMATION

The code changes are obtained from a patch attached to #25158

A backport to 2.8 would be appreciated.

Example of how MariaDB/MySQL handle grants:

MariaDB [(none)]> create database a;
Query OK, 1 row affected (0.001 sec)

MariaDB [(none)]> use a;
Database changed
MariaDB [a]> create table t1 (c1 int(1), c2 int(1), c3 int(1));
Query OK, 0 rows affected (0.035 sec)

MariaDB [a]> GRANT USAGE ON *.* TO 'user'@'localhost' IDENTIFIED BY 'pwd';
Query OK, 0 rows affected (0.010 sec)

MariaDB [a]> GRANT SELECT (c2, c3) ON `a`.`t1` TO 'user'@'localhost';
Query OK, 0 rows affected (0.010 sec)

MariaDB [a]> SHOW GRANTS FOR 'user'@'localhost';
+-------------------------------------------------------------------------------------------------------------+
| Grants for user@localhost                                                                                   |
+-------------------------------------------------------------------------------------------------------------+
| GRANT USAGE ON *.* TO 'user'@'localhost' IDENTIFIED BY PASSWORD '*975B2CD4FF9AE554FE8AD33168FBFC326D2021DD' |
| GRANT SELECT (c3, c2) ON `a`.`t1` TO 'user'@'localhost'                                                     |
+-------------------------------------------------------------------------------------------------------------+
2 rows in set (0.000 sec)

MariaDB [a]> REVOKE SELECT (c3, c2) ON `a`.`t1` FROM 'user'@'localhost';
Query OK, 0 rows affected (0.010 sec)

MariaDB [a]> SHOW GRANTS FOR 'user'@'localhost';
+-------------------------------------------------------------------------------------------------------------+
| Grants for user@localhost                                                                                   |
+-------------------------------------------------------------------------------------------------------------+
| GRANT USAGE ON *.* TO 'user'@'localhost' IDENTIFIED BY PASSWORD '*975B2CD4FF9AE554FE8AD33168FBFC326D2021DD' |
+-------------------------------------------------------------------------------------------------------------+
1 row in set (0.000 sec)

MariaDB [a]> GRANT SELECT (c3, c1) ON `a`.`t1` TO 'user'@'localhost';
Query OK, 0 rows affected (0.005 sec)

MariaDB [a]> SHOW GRANTS FOR 'user'@'localhost';
+-------------------------------------------------------------------------------------------------------------+
| Grants for user@localhost                                                                                   |
+-------------------------------------------------------------------------------------------------------------+
| GRANT USAGE ON *.* TO 'user'@'localhost' IDENTIFIED BY PASSWORD '*975B2CD4FF9AE554FE8AD33168FBFC326D2021DD' |
| GRANT SELECT (c1, c3) ON `a`.`t1` TO 'user'@'localhost'                                                     |
+-------------------------------------------------------------------------------------------------------------+
2 rows in set (0.000 sec)

MariaDB [a]> GRANT SELECT (c1, c3) ON `a`.`t1` TO 'user'@'localhost';
Query OK, 0 rows affected (0.013 sec)

MariaDB [a]> SHOW GRANTS FOR 'user'@'localhost';
+-------------------------------------------------------------------------------------------------------------+
| Grants for user@localhost                                                                                   |
+-------------------------------------------------------------------------------------------------------------+
| GRANT USAGE ON *.* TO 'user'@'localhost' IDENTIFIED BY PASSWORD '*975B2CD4FF9AE554FE8AD33168FBFC326D2021DD' |
| GRANT SELECT (c1, c3) ON `a`.`t1` TO 'user'@'localhost'                                                     |
+-------------------------------------------------------------------------------------------------------------+
2 rows in set (0.000 sec)

MariaDB [a]> REVOKE SELECT (c3, c1) ON `a`.`t1` FROM 'user'@'localhost';
Query OK, 0 rows affected (0.009 sec)

MariaDB [a]> SHOW GRANTS FOR 'user'@'localhost';
+-------------------------------------------------------------------------------------------------------------+
| Grants for user@localhost                                                                                   |
+-------------------------------------------------------------------------------------------------------------+
| GRANT USAGE ON *.* TO 'user'@'localhost' IDENTIFIED BY PASSWORD '*975B2CD4FF9AE554FE8AD33168FBFC326D2021DD' |
+-------------------------------------------------------------------------------------------------------------+
1 row in set (0.000 sec)

MariaDB [a]> GRANT SELECT (c1, c3) ON `a`.`t1` TO 'user'@'localhost';
Query OK, 0 rows affected (0.010 sec)

MariaDB [a]> SHOW GRANTS FOR 'user'@'localhost';
+-------------------------------------------------------------------------------------------------------------+
| Grants for user@localhost                                                                                   |
+-------------------------------------------------------------------------------------------------------------+
| GRANT USAGE ON *.* TO 'user'@'localhost' IDENTIFIED BY PASSWORD '*975B2CD4FF9AE554FE8AD33168FBFC326D2021DD' |
| GRANT SELECT (c3, c1) ON `a`.`t1` TO 'user'@'localhost'                                                     |
+-------------------------------------------------------------------------------------------------------------+
2 rows in set (0.000 sec)
@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2019

The test ansible-test sanity --test pylint [explain] failed with 8 errors:

lib/ansible/modules/database/mysql/mysql_user.py:483:0: anomalous-backslash-in-string Anomalous backslash in string: '\('. String constant might be missing an r prefix.
lib/ansible/modules/database/mysql/mysql_user.py:483:0: anomalous-backslash-in-string Anomalous backslash in string: '\)'. String constant might be missing an r prefix.
lib/ansible/modules/database/mysql/mysql_user.py:488:0: unnecessary-semicolon Unnecessary semicolon
lib/ansible/modules/database/mysql/mysql_user.py:498:0: anomalous-backslash-in-string Anomalous backslash in string: '\('. String constant might be missing an r prefix.
lib/ansible/modules/database/mysql/mysql_user.py:498:0: anomalous-backslash-in-string Anomalous backslash in string: '\)'. String constant might be missing an r prefix.
lib/ansible/modules/database/mysql/mysql_user.py:549:0: anomalous-backslash-in-string Anomalous backslash in string: '\('. String constant might be missing an r prefix.
lib/ansible/modules/database/mysql/mysql_user.py:549:0: anomalous-backslash-in-string Anomalous backslash in string: '\)'. String constant might be missing an r prefix.
lib/ansible/modules/database/mysql/mysql_user.py:555:0: unnecessary-semicolon Unnecessary semicolon

The test ansible-test sanity --test ansible-doc --python 3.8 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module mysql_user" returned exit status 0.
>>> Standard Error
<unknown>:483: SyntaxWarning: invalid escape sequence \(
<unknown>:498: SyntaxWarning: invalid escape sequence \(
<unknown>:549: SyntaxWarning: invalid escape sequence \(

The test ansible-test sanity --test import --python 3.8 [explain] failed with 3 errors:

lib/ansible/modules/database/mysql/mysql_user.py:483:0: SyntaxWarning: invalid escape sequence \(
lib/ansible/modules/database/mysql/mysql_user.py:498:0: SyntaxWarning: invalid escape sequence \(
lib/ansible/modules/database/mysql/mysql_user.py:549:0: SyntaxWarning: invalid escape sequence \(

The test ansible-test sanity --test pep8 [explain] failed with 18 errors:

lib/ansible/modules/database/mysql/mysql_user.py:483:35: W605 invalid escape sequence '\('
lib/ansible/modules/database/mysql/mysql_user.py:483:41: W605 invalid escape sequence '\)'
lib/ansible/modules/database/mysql/mysql_user.py:488:26: E703 statement ends with a semicolon
lib/ansible/modules/database/mysql/mysql_user.py:490:28: E226 missing whitespace around arithmetic operator
lib/ansible/modules/database/mysql/mysql_user.py:490:30: E226 missing whitespace around arithmetic operator
lib/ansible/modules/database/mysql/mysql_user.py:491:35: E226 missing whitespace around arithmetic operator
lib/ansible/modules/database/mysql/mysql_user.py:491:40: E226 missing whitespace around arithmetic operator
lib/ansible/modules/database/mysql/mysql_user.py:491:58: E226 missing whitespace around arithmetic operator
lib/ansible/modules/database/mysql/mysql_user.py:498:39: W605 invalid escape sequence '\('
lib/ansible/modules/database/mysql/mysql_user.py:498:43: W605 invalid escape sequence '\)'
lib/ansible/modules/database/mysql/mysql_user.py:549:50: W605 invalid escape sequence '\('
lib/ansible/modules/database/mysql/mysql_user.py:549:56: W605 invalid escape sequence '\)'
lib/ansible/modules/database/mysql/mysql_user.py:555:34: E703 statement ends with a semicolon
lib/ansible/modules/database/mysql/mysql_user.py:556:36: E226 missing whitespace around arithmetic operator
lib/ansible/modules/database/mysql/mysql_user.py:556:38: E226 missing whitespace around arithmetic operator
lib/ansible/modules/database/mysql/mysql_user.py:557:79: E226 missing whitespace around arithmetic operator
lib/ansible/modules/database/mysql/mysql_user.py:557:84: E226 missing whitespace around arithmetic operator
lib/ansible/modules/database/mysql/mysql_user.py:557:102: E226 missing whitespace around arithmetic operator

click here for bot help

twouters added some commits Jul 10, 2019

Fix #25158 - split column grants and apply sorting
MySQL doesn't honor the order of column grants while executing GRANT.
Split granted columns and sort the list before comparing changes to work
around this.

@twouters twouters force-pushed the twouters:bugfix/mysql_user_fix_column_privs branch from cc9b9ae to b341297 Jul 10, 2019

@twouters twouters force-pushed the twouters:bugfix/mysql_user_fix_column_privs branch from 0c18d96 to cbeaee0 Jul 11, 2019

@twouters

This comment has been minimized.

Copy link
Author

commented Jul 11, 2019

Added tests to verify column specific privileges.

Test fails when commits are reverted:

TASK [mysql_user : Add privs to specific table columns (expect ok)] ************
changed: [testhost] => {"changed": true, "msg": "Privileges updated", "user": "colprivuser1"}

TASK [mysql_user : Assert that priv did not change] ****************************
fatal: [testhost]: FAILED! => {
    "assertion": "not result.changed",
    "changed": false,
    "evaluated_to": false,
    "msg": "Assertion failed"
}
@twouters

This comment has been minimized.

Copy link
Author

commented Jul 11, 2019

ready_for_review

@twouters twouters force-pushed the twouters:bugfix/mysql_user_fix_column_privs branch from 1c8f46c to df729a1 Jul 11, 2019

@ansibot ansibot added the stale_ci label Jul 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.