postgresql_privs: fix schema quoting (#382)

* postgresql_privs: fix schema quoting * add changelog fragment * postgresql_privs: add integration test for schema names with '.' or '"' * postgresql_privs: extend integration test to check if permissions on tables in schemas with names that need to be quoted are set correctly * postgresql_privs: fix integration test syntax * postgresql_privs: fix integration test syntax * postgresql_privs: fix integration test syntax Co-authored-by: Felix Hamme <felix.hamme@ionos.com>
ansible-collections · Dec 8, 2022 · 3ddff9c · 3ddff9c
1 parent 70b915a
commit 3ddff9c
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 7 deletions.
diff --git a/changelogs/fragments/382-postgresql_privs_fix_schemas_with_special_names.yml b/changelogs/fragments/382-postgresql_privs_fix_schemas_with_special_names.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - postgresql_privs - fix quoting of the ``schema`` parameter in SQL statements (https://github.com/ansible-collections/community.postgresql/pull/382).
diff --git a/plugins/modules/postgresql_privs.py b/plugins/modules/postgresql_privs.py
@@ -817,6 +817,7 @@ def manipulate_privs(self, obj_type, privs, objs, orig_objs, roles, target_roles
         if not objs:
             return False
 
+        quoted_schema_qualifier = '"%s"' % schema_qualifier.replace('"', '""') if schema_qualifier else None
         # obj_ids: quoted db object identifiers (sometimes schema-qualified)
         if obj_type in ('function', 'procedure'):
             obj_ids = []
@@ -825,9 +826,9 @@ def manipulate_privs(self, obj_type, privs, objs, orig_objs, roles, target_roles
                     f, args = obj.split('(', 1)
                 except Exception:
                     raise Error('Illegal function / procedure signature: "%s".' % obj)
-                obj_ids.append('"%s"."%s"(%s' % (schema_qualifier, f, args))
+                obj_ids.append('%s."%s"(%s' % (quoted_schema_qualifier, f, args))
         elif obj_type in ['table', 'sequence', 'type']:
-            obj_ids = ['"%s"."%s"' % (schema_qualifier, o) for o in objs]
+            obj_ids = ['%s."%s"' % (quoted_schema_qualifier, o) for o in objs]
         else:
             obj_ids = ['"%s"' % o for o in objs]
 
@@ -846,7 +847,7 @@ def manipulate_privs(self, obj_type, privs, objs, orig_objs, roles, target_roles
             # and privs was escaped when it was parsed
             # Note: Underscores are replaced with spaces to support multi-word obj_type
             if orig_objs is not None:
-                set_what = '%s ON %s %s' % (','.join(privs), orig_objs, schema_qualifier)
+                set_what = '%s ON %s %s' % (','.join(privs), orig_objs, quoted_schema_qualifier)
             else:
                 set_what = '%s ON %s %s' % (','.join(privs), obj_type.replace('_', ' '), ','.join(obj_ids))
 
@@ -875,17 +876,14 @@ def manipulate_privs(self, obj_type, privs, objs, orig_objs, roles, target_roles
         if target_roles:
             as_who = ','.join('"%s"' % r for r in target_roles)
 
-        if schema_qualifier:
-            schema_qualifier = '"%s"' % schema_qualifier
-
         status_before = get_status(objs)
 
         query = QueryBuilder(state) \
             .for_objtype(obj_type) \
             .with_grant_option(grant_option) \
             .for_whom(for_whom) \
             .as_who(as_who) \
-            .for_schema(schema_qualifier) \
+            .for_schema(quoted_schema_qualifier) \
             .set_what(set_what) \
             .for_objs(objs) \
             .usage_on_types(usage_on_types) \

diff --git a/tests/integration/targets/postgresql_privs/defaults/main.yml b/tests/integration/targets/postgresql_privs/defaults/main.yml
@@ -7,6 +7,8 @@ db_user_with_dots2: role.with.dots2
 db_name_with_hyphens: ansible-db
 db_user_with_hyphens: ansible-db-user
 db_schema_with_hyphens: ansible-db-schema
+db_schema_with_dot: test.schema
+db_schema_with_quote: 'TEST_schema"'
 db_session_role1: session_role1
 db_session_role2: session_role2
 dangerous_name: 'curious.anonymous"; SELECT * FROM information_schema.tables; --'
diff --git a/tests/integration/targets/postgresql_privs/tasks/postgresql_privs_general.yml b/tests/integration/targets/postgresql_privs/tasks/postgresql_privs_general.yml
@@ -1476,6 +1476,114 @@
     that:
     - result is not changed
 
+##############
+# Issue https://github.com/ansible-collections/community.postgresql/issues/381
+- name: create schemas with special names
+  become: true
+  become_user: "{{ pg_user }}"
+  postgresql_schema:
+    login_user: "{{ pg_user }}"
+    login_password: password
+    db: "{{ db_name }}"
+    name: '"{{ item }}"'
+    state: present
+  loop:
+    - "{{ db_schema_with_dot|replace('\"', '\"\"') }}"
+    - "{{ db_schema_with_quote|replace('\"', '\"\"') }}"
+  register: result
+- assert:
+    that:
+      - result is changed
+- name: create tables in schemas with special names
+  become: true
+  become_user: "{{ pg_user }}"
+  postgresql_table:
+    login_user: "{{ pg_user }}"
+    login_password: password
+    db: "{{ db_name }}"
+    name: '"{{ item }}"."test.table.name"'
+    columns: []
+  loop:
+    - "{{ db_schema_with_dot|replace('\"', '\"\"') }}"
+    - "{{ db_schema_with_quote|replace('\"', '\"\"') }}"
+  register: result
+- assert:
+    that:
+      - result is changed
+- name: grant privileges on all tables in schemas with special names
+  become: yes
+  become_user: "{{ pg_user }}"
+  postgresql_privs:
+    login_user: "{{ pg_user }}"
+    login_db: "{{ db_name }}"
+    roles: PUBLIC
+    objs: ALL_IN_SCHEMA
+    type: table
+    privs: SELECT
+    schema: "{{ item }}"
+  loop:
+    - "{{ db_schema_with_dot }}"
+    - "{{ db_schema_with_quote }}"
+  register: result
+- assert:
+    that:
+      - result is changed
+- name: grant privileges on some table in schemas with special names
+  become: yes
+  become_user: "{{ pg_user }}"
+  postgresql_privs:
+    login_user: "{{ pg_user }}"
+    login_db: "{{ db_name }}"
+    roles: PUBLIC
+    objs: 'test.table.name'
+    type: table
+    privs: SELECT
+    schema: "{{ item }}"
+  loop:
+    - "{{ db_schema_with_dot }}"
+    - "{{ db_schema_with_quote }}"
+  register: result
+- assert:
+    that:
+      - result is changed
+- name: check permissions on tables in schemas with special names
+  become: true
+  become_user: "{{ pg_user }}"
+  postgresql_query:
+    login_user: "{{ pg_user }}"
+    db: "{{ db_name }}"
+    query: |
+      select true as granted from information_schema.role_table_grants
+      where table_schema=%s and table_name='test.table.name' and privilege_type='SELECT' and grantee='PUBLIC'
+    positional_args:
+      - "{{ item }}"
+  loop:
+    - "{{ db_schema_with_dot }}"
+    - "{{ db_schema_with_quote }}"
+  register: result
+- assert:
+    that:
+      - 'result.results|length == 2'
+      - 'result.results[0].rowcount == 1'
+      - 'not result.results[0].failed'
+      - 'result.results[1].rowcount == 1'
+      - 'not result.results[1].failed'
+- name: cleanup test schemas with special names
+  become: true
+  become_user: "{{ pg_user }}"
+  postgresql_schema:
+    login_user: "{{ pg_user }}"
+    login_password: password
+    db: "{{ db_name }}"
+    name: '"{{ item }}"'
+    state: absent
+    cascade_drop: true
+  loop:
+    - "{{ db_schema_with_dot|replace('\"', '\"\"') }}"
+    - "{{ db_schema_with_quote|replace('\"', '\"\"') }}"
+  register: result
+
+
 ##############
 # Issue https://github.com/ansible-collections/community.postgresql/issues/332
 - name: Test community.postgresql issue 332 grant usage
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		bugfixes:
		- postgresql_privs - fix quoting of the ``schema`` parameter in SQL statements (https://github.com/ansible-collections/community.postgresql/pull/382).