From 86405b8fe4f2deee3abc03a432aa4ca4908ca060 Mon Sep 17 00:00:00 2001
From: Matteo Ferrando <matteo.ferrando2@gmail.com>
Date: Thu, 14 Mar 2019 14:51:05 +0000
Subject: [PATCH] (postgresql_privs) accept 'ALL_IN_SCHEMA' objs for 'function'
 type (#35331)

* avoid using Postgres formatting function
* add tests for ALL FUNCTIONS IN SCHEMA
* documentation and changelog
* requested changes in tests
* fixed changelog
---
 ...privs-add-all_in_schema-for-functions.yaml |   2 +
 .../database/postgresql/postgresql_privs.py   |  34 ++-
 .../postgresql/tasks/postgresql_privs.yml     | 244 ++++++++++++++----
 3 files changed, 231 insertions(+), 49 deletions(-)
 create mode 100644 changelogs/fragments/35331-postgres_privs-add-all_in_schema-for-functions.yaml

diff --git a/changelogs/fragments/35331-postgres_privs-add-all_in_schema-for-functions.yaml b/changelogs/fragments/35331-postgres_privs-add-all_in_schema-for-functions.yaml
new file mode 100644
index 0000000000..9263721538
--- /dev/null
+++ b/changelogs/fragments/35331-postgres_privs-add-all_in_schema-for-functions.yaml
@@ -0,0 +1,2 @@
+minor_changes:
+  - postgres_privs now accepts 'ALL_IN_SCHEMA' objs for 'function' type (https://github.com/ansible/ansible/pull/35331).
diff --git a/lib/ansible/modules/database/postgresql/postgresql_privs.py b/lib/ansible/modules/database/postgresql/postgresql_privs.py
index 201694ed60..b77727ad79 100644
--- a/lib/ansible/modules/database/postgresql/postgresql_privs.py
+++ b/lib/ansible/modules/database/postgresql/postgresql_privs.py
@@ -49,10 +49,11 @@ options:
   objs:
     description:
       - Comma separated list of database objects to set privileges on.
-      - If I(type) is C(table) or C(sequence), the special value
+      - If I(type) is C(table), C(sequence) or C(function), the special value
         C(ALL_IN_SCHEMA) can be provided instead to specify all database
         objects of type I(type) in the schema specified via I(schema). (This
-        also works with PostgreSQL < 9.0.)
+        also works with PostgreSQL < 9.0.) (C(ALL_IN_SCHEMA) is available for
+        C(function) from version 2.8)
       - If I(type) is C(database), this parameter can be omitted, in which case
         privileges are set for the database specified via I(database).
       - 'If I(type) is I(function), colons (":") in object names will be
@@ -282,6 +283,7 @@ EXAMPLES = """
     type: foreign_data_wrapper
     role: reader
 
+# Available since version 2.8
 # GRANT ALL PRIVILEGES ON FOREIGN SERVER fdw_server TO reader
 - postgresql_privs:
     db: test
@@ -290,6 +292,16 @@ EXAMPLES = """
     type: foreign_server
     role: reader
 
+# Available since version 2.8
+# GRANT EXECUTE ON ALL FUNCTIONS IN SCHEMA common TO caller
+# Grant 'execute' permissions on all functions in schema 'common' to role 'caller'
+- postgresql_privs:
+    type: function
+    state: present
+    privs: EXECUTE
+    roles: caller
+    objs: ALL_IN_SCHEMA
+    schema: common
 """
 
 import traceback
@@ -425,6 +437,16 @@ class Connection(object):
         self.cursor.execute(query, (schema,))
         return [t[0] for t in self.cursor.fetchall()]
 
+    def get_all_functions_in_schema(self, schema):
+        if not self.schema_exists(schema):
+            raise Error('Schema "%s" does not exist.' % schema)
+        query = """SELECT p.proname, oidvectortypes(p.proargtypes)
+                    FROM pg_catalog.pg_proc p
+                    JOIN pg_namespace n ON n.oid = p.pronamespace
+                    WHERE nspname = %s"""
+        self.cursor.execute(query, (schema,))
+        return ["%s(%s)" % (t[0], t[1]) for t in self.cursor.fetchall()]
+
     # Methods for getting access control lists and group membership info
 
     # To determine whether anything has changed after granting/revoking
@@ -824,6 +846,8 @@ def main():
             objs = conn.get_all_tables_in_schema(p.schema)
         elif p.type == 'sequence' and p.objs == 'ALL_IN_SCHEMA':
             objs = conn.get_all_sequences_in_schema(p.schema)
+        elif p.type == 'function' and p.objs == 'ALL_IN_SCHEMA':
+            objs = conn.get_all_functions_in_schema(p.schema)
         elif p.type == 'default_privs':
             if p.objs == 'ALL_DEFAULT':
                 objs = frozenset(VALID_DEFAULT_OBJS.keys())
@@ -841,9 +865,9 @@ def main():
         else:
             objs = p.objs.split(',')
 
-        # function signatures are encoded using ':' to separate args
-        if p.type == 'function':
-            objs = [obj.replace(':', ',') for obj in objs]
+            # function signatures are encoded using ':' to separate args
+            if p.type == 'function':
+                objs = [obj.replace(':', ',') for obj in objs]
 
         # roles
         if p.roles == 'PUBLIC':
diff --git a/test/integration/targets/postgresql/tasks/postgresql_privs.yml b/test/integration/targets/postgresql/tasks/postgresql_privs.yml
index f77cd0ea6f..8de1b28018 100644
--- a/test/integration/targets/postgresql/tasks/postgresql_privs.yml
+++ b/test/integration/targets/postgresql/tasks/postgresql_privs.yml
@@ -1,43 +1,58 @@
----
+# Setup
+- name: Create DB
+  become_user: "{{ pg_user }}"
+  become: yes
+  postgresql_db:
+    state: present
+    name: "{{ db_name }}"
+    login_user: "{{ pg_user }}"
+
+- name: Create a user to be owner of objects
+  postgresql_user:
+    name: "{{ db_user3 }}"
+    state: present
+    encrypted: yes
+    password: password
+    role_attr_flags: CREATEDB,LOGIN
+    db: "{{ db_name }}"
+    login_user: "{{ pg_user }}"
+
+- name: Create a user to be given permissions and other tests
+  postgresql_user:
+    name: "{{ db_user2 }}"
+    state: present
+    encrypted: yes
+    password: password
+    role_attr_flags: LOGIN
+    db: "{{ db_name }}"
+    login_user: "{{ pg_user }}"
 
 ######################################################
 # Test foreign data wrapper and foreign server privs #
 ######################################################
 
-- name: Create DB
-  become_user: "{{ pg_user }}"
-  become: True
-  postgresql_db:
-    state: present
-    name: "{{ db_name }}"
-    login_user: "{{ pg_user }}"
-  register: result
-
-- name: Create test role
-  become: True
-  become_user: "{{ pg_user }}"
-  shell: echo "CREATE ROLE fdw_test" | psql -d "{{ db_name }}"
-
-- name: Create fdw extension
-  become: True
+# Foreign data wrapper setup
+- name: Create foreign data wrapper extension
+  become: yes
   become_user: "{{ pg_user }}"
   shell: echo "CREATE EXTENSION postgres_fdw" | psql -d "{{ db_name }}"
 
-- name: Create foreign data wrapper
-  become: True
+- name: Create dummy foreign data wrapper
+  become: yes
   become_user: "{{ pg_user }}"
   shell: echo "CREATE FOREIGN DATA WRAPPER dummy" | psql -d "{{ db_name }}"
 
 - name: Create foreign server
-  become: True
+  become: yes
   become_user: "{{ pg_user }}"
   shell: echo "CREATE SERVER dummy_server FOREIGN DATA WRAPPER dummy" | psql -d "{{ db_name }}"
 
+# Test
 - name: Grant foreign data wrapper privileges
   postgresql_privs:
     state: present
     type: foreign_data_wrapper
-    roles: fdw_test
+    roles: "{{ db_user2 }}"
     privs: ALL
     objs: dummy
     db: "{{ db_name }}"
@@ -45,12 +60,13 @@
   register: result
   ignore_errors: yes
 
+# Checks
 - assert:
     that:
       - "result.changed == true"
 
 - name: Get foreign data wrapper privileges
-  become: True
+  become: yes
   become_user: "{{ pg_user }}"
   shell: echo "{{ fdw_query }}" | psql -d "{{ db_name }}"
   vars:
@@ -62,13 +78,14 @@
 - assert:
     that:
       - "fdw_result.stdout_lines[-1] == '(1 row)'"
-      - "'fdw_test' in fdw_result.stdout_lines[-2]"
+      - "'{{ db_user2 }}' in fdw_result.stdout_lines[-2]"
 
+# Test
 - name: Grant foreign data wrapper privileges second time
   postgresql_privs:
     state: present
     type: foreign_data_wrapper
-    roles: fdw_test
+    roles: "{{ db_user2 }}"
     privs: ALL
     objs: dummy
     db: "{{ db_name }}"
@@ -76,15 +93,17 @@
   register: result
   ignore_errors: yes
 
+# Checks
 - assert:
     that:
       - "result.changed == false"
 
+# Test
 - name: Revoke foreign data wrapper privileges
   postgresql_privs:
     state: absent
     type: foreign_data_wrapper
-    roles: fdw_test
+    roles: "{{ db_user2 }}"
     privs: ALL
     objs: dummy
     db: "{{ db_name }}"
@@ -92,12 +111,13 @@
   register: result
   ignore_errors: yes
 
+# Checks
 - assert:
     that:
       - "result.changed == true"
 
 - name: Get foreign data wrapper privileges
-  become: True
+  become: yes
   become_user: "{{ pg_user }}"
   shell: echo "{{ fdw_query }}" | psql -d "{{ db_name }}"
   vars:
@@ -109,13 +129,14 @@
 - assert:
     that:
       - "fdw_result.stdout_lines[-1] == '(1 row)'"
-      - "'fdw_test' not in fdw_result.stdout_lines[-2]"
+      - "'{{ db_user2 }}' not in fdw_result.stdout_lines[-2]"
 
+# Test
 - name: Revoke foreign data wrapper privileges for second time
   postgresql_privs:
     state: absent
     type: foreign_data_wrapper
-    roles: fdw_test
+    roles: "{{ db_user2 }}"
     privs: ALL
     objs: dummy
     db: "{{ db_name }}"
@@ -123,15 +144,17 @@
   register: result
   ignore_errors: yes
 
+# Checks
 - assert:
     that:
       - "result.changed == false"
 
+# Test
 - name: Grant foreign server privileges
   postgresql_privs:
     state: present
     type: foreign_server
-    roles: fdw_test
+    roles: "{{ db_user2 }}"
     privs: ALL
     objs: dummy_server
     db: "{{ db_name }}"
@@ -139,12 +162,13 @@
   register: result
   ignore_errors: yes
 
+# Checks
 - assert:
     that:
       - "result.changed == true"
 
 - name: Get foreign server privileges
-  become: True
+  become: yes
   become_user: "{{ pg_user }}"
   shell: echo "{{ fdw_query }}" | psql -d "{{ db_name }}"
   vars:
@@ -156,13 +180,14 @@
 - assert:
     that:
       - "fs_result.stdout_lines[-1] == '(1 row)'"
-      - "'fdw_test' in fs_result.stdout_lines[-2]"
+      - "'{{ db_user2 }}' in fs_result.stdout_lines[-2]"
 
+# Test
 - name: Grant foreign server privileges for second time
   postgresql_privs:
     state: present
     type: foreign_server
-    roles: fdw_test
+    roles: "{{ db_user2 }}"
     privs: ALL
     objs: dummy_server
     db: "{{ db_name }}"
@@ -170,15 +195,17 @@
   register: result
   ignore_errors: yes
 
+# Checks
 - assert:
     that:
       - "result.changed == false"
 
+# Test
 - name: Revoke foreign server privileges
   postgresql_privs:
     state: absent
     type: foreign_server
-    roles: fdw_test
+    roles: "{{ db_user2 }}"
     privs: ALL
     objs: dummy_server
     db: "{{ db_name }}"
@@ -186,12 +213,13 @@
   register: result
   ignore_errors: yes
 
+# Checks
 - assert:
     that:
       - "result.changed == true"
 
 - name: Get foreign server privileges
-  become: True
+  become: yes
   become_user: "{{ pg_user }}"
   shell: echo "{{ fdw_query }}" | psql -d "{{ db_name }}"
   vars:
@@ -203,13 +231,14 @@
 - assert:
     that:
       - "fs_result.stdout_lines[-1] == '(1 row)'"
-      - "'fdw_test' not in fs_result.stdout_lines[-2]"
+      - "'{{ db_user2 }}' not in fs_result.stdout_lines[-2]"
 
+# Test
 - name: Revoke foreign server privileges for second time
   postgresql_privs:
     state: absent
     type: foreign_server
-    roles: fdw_test
+    roles: "{{ db_user2 }}"
     privs: ALL
     objs: dummy_server
     db: "{{ db_name }}"
@@ -217,22 +246,149 @@
   register: result
   ignore_errors: yes
 
+# Checks
 - assert:
     that:
       - "result.changed == false"
 
-- name: Cleanup
-  become: True
+# Foreign data wrapper cleanup
+- name: Drop foreign server
+  become: yes
   become_user: "{{ pg_user }}"
-  shell: echo "{{ item }}" | psql -d "{{ db_name }}"
-  with_items:
-    - DROP ROLE fdw_test
-    - DROP FOREIGN DATA WRAPPER dummy
-    - DROP SERVER dummy_server
+  shell: echo "DROP SERVER dummy_server" | psql -d "{{ db_name }}"
+
+- name: Drop dummy foreign data wrapper
+  become: yes
+  become_user: "{{ pg_user }}"
+  shell: echo "DROP FOREIGN DATA WRAPPER dummy" | psql -d "{{ db_name }}"
+
+- name: Drop foreign data wrapper extension
+  become: yes
+  become_user: "{{ pg_user }}"
+  shell: echo "DROP EXTENSION postgres_fdw" | psql -d "{{ db_name }}"
+
+##########################################
+# Test ALL_IN_SCHEMA for 'function' type #
+##########################################
+
+# Function ALL_IN_SCHEMA Setup
+- name: Create function for test
+  postgresql_query:
+    query: CREATE FUNCTION public.a() RETURNS integer LANGUAGE SQL AS 'SELECT 2';
+    db: "{{ db_name }}"
+    login_user: "{{ db_user3 }}"
+    login_password: password
+
+# Test
+- name: Grant execute to all functions
+  postgresql_privs:
+    type: function
+    state: present
+    privs: EXECUTE
+    roles: "{{ db_user2 }}"
+    objs: ALL_IN_SCHEMA
+    schema: public
+    db: "{{ db_name }}"
+    login_user: "{{ db_user3 }}"
+    login_password: password
+  register: result
+  ignore_errors: yes
+
+# Checks
+- assert:
+    that: result.changed == true
+
+- name: Check that all functions have execute privileges
+  become: yes
+  become_user: "{{ pg_user }}"
+  shell: psql {{ db_name }} -c "SELECT proacl FROM pg_proc WHERE proname = 'a'" -t
+  register: result
+
+- assert:
+    that: "'{{ db_user2 }}=X/{{ db_user3 }}' in '{{ result.stdout_lines[0] }}'"
+
+# Test
+- name: Grant execute to all functions again
+  postgresql_privs:
+    type: function
+    state: present
+    privs: EXECUTE
+    roles: "{{ db_user2 }}"
+    objs: ALL_IN_SCHEMA
+    schema: public
+    db: "{{ db_name }}"
+    login_user: "{{ db_user3 }}"
+    login_password: password
+  register: result
+  ignore_errors: yes
+
+# Checks
+- assert:
+    that: result.changed == false
+
+# Test
+- name: Revoke execute to all functions
+  postgresql_privs:
+    type: function
+    state: absent
+    privs: EXECUTE
+    roles: "{{ db_user2 }}"
+    objs: ALL_IN_SCHEMA
+    schema: public
+    db: "{{ db_name }}"
+    login_user: "{{ db_user3 }}"
+    login_password: password
+  register: result
+  ignore_errors: yes
+
+# Checks
+- assert:
+    that: result.changed == true
+
+# Test
+- name: Revoke execute to all functions again
+  postgresql_privs:
+    type: function
+    state: absent
+    privs: EXECUTE
+    roles: "{{ db_user2 }}"
+    objs: ALL_IN_SCHEMA
+    schema: public
+    db: "{{ db_name }}"
+    login_user: "{{ db_user3 }}"
+    login_password: password
+  register: result
+  ignore_errors: yes
+
+- assert:
+    that: result.changed == false
+
+# Function ALL_IN_SCHEMA cleanup
+- name: Remove function for test
+  postgresql_query:
+    query: DROP FUNCTION public.a();
+    db: "{{ db_name }}"
+    login_user: "{{ db_user3 }}"
+    login_password: password
+
+# Cleanup
+- name: Remove user given permissions
+  postgresql_user:
+    name: "{{ db_user2 }}"
+    state: absent
+    db: "{{ db_name }}"
+    login_user: "{{ pg_user }}"
+
+- name: Remove user owner of objects
+  postgresql_user:
+    name: "{{ db_user3 }}"
+    state: absent
+    db: "{{ db_name }}"
+    login_user: "{{ pg_user }}"
 
 - name: Destroy DB
   become_user: "{{ pg_user }}"
-  become: True
+  become: yes
   postgresql_db:
     state: absent
     name: "{{ db_name }}"