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

postgresql_user: allow to pass user name with dots #63565

Merged
merged 5 commits into from
Oct 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 4 additions & 6 deletions lib/ansible/module_utils/postgres.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
HAS_PSYCOPG2 = False

from ansible.module_utils.basic import missing_required_lib
from ansible.module_utils.database import pg_quote_identifier
from ansible.module_utils._text import to_native
from ansible.module_utils.six import iteritems
from distutils.version import LooseVersion
Expand Down Expand Up @@ -94,8 +93,9 @@ def connect_to_db(module, conn_params, autocommit=False, fail_on_conn=True):
# Switch role, if specified:
if module.params.get('session_role'):
cursor = db_connection.cursor(cursor_factory=psycopg2.extras.DictCursor)

try:
cursor.execute('SET ROLE %s' % module.params['session_role'])
cursor.execute('SET ROLE "%s"' % module.params['session_role'])
except Exception as e:
module.fail_json(msg="Could not switch role: %s" % to_native(e))
finally:
Expand Down Expand Up @@ -223,8 +223,7 @@ def grant(self):
if self.__check_membership(group, role):
continue

query = "GRANT %s TO %s" % ((pg_quote_identifier(group, 'role'),
(pg_quote_identifier(role, 'role'))))
query = 'GRANT "%s" TO "%s"' % (group, role)
self.changed = exec_sql(self, query, ddl=True)

if self.changed:
Expand All @@ -241,8 +240,7 @@ def revoke(self):
if not self.__check_membership(group, role):
continue

query = "REVOKE %s FROM %s" % ((pg_quote_identifier(group, 'role'),
(pg_quote_identifier(role, 'role'))))
query = 'REVOKE "%s" FROM "%s"' % (group, role)
self.changed = exec_sql(self, query, ddl=True)

if self.changed:
Expand Down
31 changes: 14 additions & 17 deletions lib/ansible/modules/database/postgresql/postgresql_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,6 @@
from ansible.module_utils.database import pg_quote_identifier, SQLParseError
from ansible.module_utils.postgres import (
connect_to_db,
exec_sql,
get_conn_params,
PgMembership,
postgres_common_argument_spec,
Expand Down Expand Up @@ -303,8 +302,8 @@ def user_add(cursor, user, password, role_attr_flags, encrypted, expires, conn_l
# Note: role_attr_flags escaped by parse_role_attrs and encrypted is a
# literal
query_password_data = dict(password=password, expires=expires)
query = ['CREATE USER %(user)s' %
{"user": pg_quote_identifier(user, 'role')}]
query = ['CREATE USER "%(user)s"' %
{"user": user}]
if password is not None and password != '':
query.append("WITH %(crypt)s" % {"crypt": encrypted})
query.append("PASSWORD %(password)s")
Expand Down Expand Up @@ -421,7 +420,7 @@ def user_alter(db_connection, module, user, password, role_attr_flags, encrypted
if not pwchanging and not role_attr_flags_changing and not expires_changing and not conn_limit_changing:
return False

alter = ['ALTER USER %(user)s' % {"user": pg_quote_identifier(user, 'role')}]
alter = ['ALTER USER "%(user)s"' % {"user": user}]
if pwchanging:
if password != '':
alter.append("WITH %(crypt)s" % {"crypt": encrypted})
Expand Down Expand Up @@ -476,8 +475,8 @@ def user_alter(db_connection, module, user, password, role_attr_flags, encrypted
if not role_attr_flags_changing:
return False

alter = ['ALTER USER %(user)s' %
{"user": pg_quote_identifier(user, 'role')}]
alter = ['ALTER USER "%(user)s"' %
{"user": user}]
if role_attr_flags:
alter.append('WITH %s' % role_attr_flags)

Expand Down Expand Up @@ -507,7 +506,7 @@ def user_delete(cursor, user):
"""Try to remove a user. Returns True if successful otherwise False"""
cursor.execute("SAVEPOINT ansible_pgsql_user_delete")
try:
query = "DROP USER %s" % pg_quote_identifier(user, 'role')
query = 'DROP USER "%s"' % user
executed_queries.append(query)
cursor.execute(query)
except Exception:
Expand Down Expand Up @@ -550,17 +549,17 @@ def get_table_privileges(cursor, user, table):
def grant_table_privileges(cursor, user, table, privs):
# Note: priv escaped by parse_privs
privs = ', '.join(privs)
query = 'GRANT %s ON TABLE %s TO %s' % (
privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role'))
query = 'GRANT %s ON TABLE %s TO "%s"' % (
privs, pg_quote_identifier(table, 'table'), user)
executed_queries.append(query)
cursor.execute(query)


def revoke_table_privileges(cursor, user, table, privs):
# Note: priv escaped by parse_privs
privs = ', '.join(privs)
query = 'REVOKE %s ON TABLE %s FROM %s' % (
privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role'))
query = 'REVOKE %s ON TABLE %s FROM "%s"' % (
privs, pg_quote_identifier(table, 'table'), user)
executed_queries.append(query)
cursor.execute(query)

Expand Down Expand Up @@ -609,9 +608,8 @@ def grant_database_privileges(cursor, user, db, privs):
query = 'GRANT %s ON DATABASE %s TO PUBLIC' % (
privs, pg_quote_identifier(db, 'database'))
else:
query = 'GRANT %s ON DATABASE %s TO %s' % (
privs, pg_quote_identifier(db, 'database'),
pg_quote_identifier(user, 'role'))
query = 'GRANT %s ON DATABASE %s TO "%s"' % (
privs, pg_quote_identifier(db, 'database'), user)

executed_queries.append(query)
cursor.execute(query)
Expand All @@ -624,9 +622,8 @@ def revoke_database_privileges(cursor, user, db, privs):
query = 'REVOKE %s ON DATABASE %s FROM PUBLIC' % (
privs, pg_quote_identifier(db, 'database'))
else:
query = 'REVOKE %s ON DATABASE %s FROM %s' % (
privs, pg_quote_identifier(db, 'database'),
pg_quote_identifier(user, 'role'))
query = 'REVOKE %s ON DATABASE %s FROM "%s"' % (
privs, pg_quote_identifier(db, 'database'), user)

executed_queries.append(query)
cursor.execute(query)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
# Integration tests for postgresql_user module.

- vars:
test_user: hello_user
test_user: hello.user.with.dots
test_user2: hello
test_group1: group1
test_group2: group2
test_table: test
Expand Down Expand Up @@ -490,18 +491,24 @@
#
# fail_on_user
#
- name: Create role for test
<<: *task_parameters
postgresql_user:
<<: *pg_parameters
name: '{{ test_user2 }}'

- name: Create test table, set owner as test_user
<<: *task_parameters
postgresql_table:
<<: *pg_parameters
name: '{{ test_table }}'
owner: '{{ test_user }}'
owner: '{{ test_user2 }}'

- name: Test fail_on_user
<<: *task_parameters
postgresql_user:
<<: *pg_parameters
name: '{{ test_user }}'
name: '{{ test_user2 }}'
state: absent
ignore_errors: yes

Expand Down Expand Up @@ -666,5 +673,6 @@
state: absent
loop:
- '{{ test_user }}'
- '{{ test_user2 }}'
- '{{ test_group1 }}'
- '{{ test_group2 }}'