postgresql_user: allow to pass user name with dots (#63565)

lib/ansible/module_utils/postgres.py

@@ -35,7 +35,6 @@ except ImportError:
     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
@@ -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:
@@ -223,8 +223,7 @@ class PgMembership(object):
                 if self.__check_membership(group, role):
                     continue
 
-                query = "GRANT %s TO %s" % ((pg_quote_identifier(group, 'role'),
+                query = 'GRANT "%s" TO "%s"' % (group, role)
-                                            (pg_quote_identifier(role, 'role'))))
                 self.changed = exec_sql(self, query, ddl=True)
 
                 if self.changed:
@@ -241,8 +240,7 @@ class PgMembership(object):
                 if not self.__check_membership(group, role):
                     continue
 
-                query = "REVOKE %s FROM %s" % ((pg_quote_identifier(group, 'role'),
+                query = 'REVOKE "%s" FROM "%s"' % (group, role)
-                                               (pg_quote_identifier(role, 'role'))))
                 self.changed = exec_sql(self, query, ddl=True)
 
                 if self.changed:

lib/ansible/modules/database/postgresql/postgresql_user.py

@@ -302,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' %
+    query = ['CREATE USER "%(user)s"' %
-             {"user": pg_quote_identifier(user, 'role')}]
+             {"user": user}]
     if password is not None and password != '':
         query.append("WITH %(crypt)s" % {"crypt": encrypted})
         query.append("PASSWORD %(password)s")
@@ -420,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})
@@ -475,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' %
+        alter = ['ALTER USER "%(user)s"' %
-                 {"user": pg_quote_identifier(user, 'role')}]
+                 {"user": user}]
         if role_attr_flags:
             alter.append('WITH %s' % role_attr_flags)
 
@@ -506,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:
@@ -549,8 +549,8 @@ 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' % (
+    query = 'GRANT %s ON TABLE %s TO "%s"' % (
-        privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role'))
+        privs, pg_quote_identifier(table, 'table'), user)
     executed_queries.append(query)
     cursor.execute(query)
 
@@ -558,8 +558,8 @@ def grant_table_privileges(cursor, user, table, privs):
 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' % (
+    query = 'REVOKE %s ON TABLE %s FROM "%s"' % (
-        privs, pg_quote_identifier(table, 'table'), pg_quote_identifier(user, 'role'))
+        privs, pg_quote_identifier(table, 'table'), user)
     executed_queries.append(query)
     cursor.execute(query)
 
@@ -608,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' % (
+        query = 'GRANT %s ON DATABASE %s TO "%s"' % (
-                privs, pg_quote_identifier(db, 'database'),
+                privs, pg_quote_identifier(db, 'database'), user)
-                pg_quote_identifier(user, 'role'))
 
     executed_queries.append(query)
     cursor.execute(query)
@@ -623,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' % (
+        query = 'REVOKE %s ON DATABASE %s FROM "%s"' % (
-                privs, pg_quote_identifier(db, 'database'),
+                privs, pg_quote_identifier(db, 'database'), user)
-                pg_quote_identifier(user, 'role'))
 
     executed_queries.append(query)
     cursor.execute(query)

test/integration/targets/postgresql_user/tasks/postgresql_user_general.yml

@@ -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
@@ -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
 
@@ -666,5 +673,6 @@
       state: absent
     loop:
     - '{{ test_user }}'
+    - '{{ test_user2 }}'
     - '{{ test_group1 }}'
     - '{{ test_group2 }}'