From cd71a28623ef38c7e5215791b1272688c895e515 Mon Sep 17 00:00:00 2001 From: Oliver Freyermuth Date: Mon, 2 Mar 2020 13:45:14 +0100 Subject: [PATCH 1/7] grant: Fixup escaping of function arguments in granting. The "arguments" in this case are the actual types, and they must not be quoted. --- manifests/server/grant.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/server/grant.pp b/manifests/server/grant.pp index 80ca540085..e07f624173 100644 --- a/manifests/server/grant.pp +++ b/manifests/server/grant.pp @@ -392,7 +392,7 @@ true => 'function_exists', default => undef, } - $_quoted_args = join($object_arguments.map |$arg| { "\"${arg}\"" }, ',') + $_quoted_args = join($object_arguments, ',') $arguments = "(${_quoted_args})" } From 0cac23f521f73f66375e7fca40744fa2f273273e Mon Sep 17 00:00:00 2001 From: Oliver Freyermuth Date: Mon, 2 Mar 2020 13:55:08 +0100 Subject: [PATCH 2/7] grant: Fixup quotation of function name plus arguments. Function and type arguments must stay together: "pg_catalog.pg_ls_dir"(text,boolean,boolean) => "pg_catalog.pg_ls_dir(text,boolean,boolean)" --- manifests/server/grant.pp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/manifests/server/grant.pp b/manifests/server/grant.pp index e07f624173..677b2b83b2 100644 --- a/manifests/server/grant.pp +++ b/manifests/server/grant.pp @@ -49,11 +49,11 @@ case $ensure { default: { # default is 'present' - $sql_command = 'GRANT %s ON %s "%s"%s TO "%s"' + $sql_command = 'GRANT %s ON %s "%s%s" TO "%s"' $unless_is = true } 'absent': { - $sql_command = 'REVOKE %s ON %s "%s"%s FROM "%s"' + $sql_command = 'REVOKE %s ON %s "%s%s" FROM "%s"' $unless_is = false } } From 4c091d577fcaa3dafe196714abb811582df30276 Mon Sep 17 00:00:00 2001 From: Oliver Freyermuth Date: Mon, 2 Mar 2020 15:14:12 +0100 Subject: [PATCH 3/7] grant: Prevent quotation of the full function in GRANT commant. GRANT EXECUTE ON FUNCTION "func()" fails, we need to unquote in that case. --- manifests/server/grant.pp | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/manifests/server/grant.pp b/manifests/server/grant.pp index 677b2b83b2..52ed1add9e 100644 --- a/manifests/server/grant.pp +++ b/manifests/server/grant.pp @@ -50,10 +50,12 @@ default: { # default is 'present' $sql_command = 'GRANT %s ON %s "%s%s" TO "%s"' + $sql_command_unquoted = 'GRANT %s ON %s %s%s TO "%s"' $unless_is = true } 'absent': { $sql_command = 'REVOKE %s ON %s "%s%s" FROM "%s"' + $sql_command_unquoted = 'REVOKE %s ON %s %s%s FROM "%s"' $unless_is = false } } @@ -115,6 +117,7 @@ 'absent' => 'role_exists', } $arguments = '' + $_enquote_object = true } 'SCHEMA': { $unless_privilege = $_privilege ? { @@ -131,6 +134,7 @@ $on_db = $db $onlyif_function = undef $arguments = '' + $_enquote_object = true } 'SEQUENCE': { $unless_privilege = $_privilege ? { @@ -148,6 +152,7 @@ $on_db = $db $onlyif_function = undef $arguments = '' + $_enquote_object = true } 'ALL SEQUENCES IN SCHEMA': { case $_privilege { @@ -165,6 +170,7 @@ $on_db = $db $onlyif_function = undef $arguments = '' + $_enquote_object = true $schema = $object_name @@ -282,6 +288,7 @@ default => undef, } $arguments = '' + $_enquote_object = true } 'ALL TABLES IN SCHEMA': { case $_privilege { @@ -303,6 +310,7 @@ $on_db = $db $onlyif_function = undef $arguments = '' + $_enquote_object = true $schema = $object_name @@ -375,6 +383,7 @@ default => undef, } $arguments = '' + $_enquote_object = false } 'FUNCTION': { $unless_privilege = $_privilege ? { @@ -392,8 +401,9 @@ true => 'function_exists', default => undef, } - $_quoted_args = join($object_arguments, ',') - $arguments = "(${_quoted_args})" + $_joined_args = join($object_arguments, ',') + $arguments = "(${_joined_args})" + $_enquote_object = false } default: { @@ -441,7 +451,10 @@ default => undef, } - $grant_cmd = sprintf($sql_command, $_privilege, $_object_type, $_togrant_object, $arguments, $role) + $grant_cmd = $_enquote_object ? { + false => sprintf($sql_command_unquoted, $_privilege, $_object_type, $_togrant_object, $arguments, $role), + default => sprintf($sql_command, $_privilege, $_object_type, $_togrant_object, $arguments, $role), + } postgresql_psql { "grant:${name}": command => $grant_cmd, From 48ecb4d519862b142ff9eaad3a3fc85148a2a1ab Mon Sep 17 00:00:00 2001 From: Oliver Freyermuth Date: Fri, 13 Mar 2020 21:07:27 +0100 Subject: [PATCH 4/7] grant: Fixup indentation bug. Co-Authored-By: sanfrancrisko <55992494+sanfrancrisko@users.noreply.github.com> --- manifests/server/grant.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/server/grant.pp b/manifests/server/grant.pp index 52ed1add9e..d02206463a 100644 --- a/manifests/server/grant.pp +++ b/manifests/server/grant.pp @@ -383,7 +383,7 @@ default => undef, } $arguments = '' - $_enquote_object = false + $_enquote_object = false } 'FUNCTION': { $unless_privilege = $_privilege ? { From 699535bf64f5a8ed35b3bad418bff21a01f29b7d Mon Sep 17 00:00:00 2001 From: Oliver Freyermuth Date: Fri, 13 Mar 2020 22:03:25 +0100 Subject: [PATCH 5/7] server::grant_spec: Adapt test to correct function quoting. Types must not be quoted neither for granting not checking, and the overall expression must not be quoted for granting. --- spec/unit/defines/server/grant_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/unit/defines/server/grant_spec.rb b/spec/unit/defines/server/grant_spec.rb index db4214b732..52a57112ca 100644 --- a/spec/unit/defines/server/grant_spec.rb +++ b/spec/unit/defines/server/grant_spec.rb @@ -249,7 +249,7 @@ class {'postgresql::server':} role: 'test', privilege: 'execute', object_name: 'test', - object_arguments: ['text', 'text'], + object_arguments: ['text', 'boolean'], object_type: 'function', } end @@ -262,8 +262,8 @@ class {'postgresql::server':} it { is_expected.to contain_postgresql__server__grant('test') } it do is_expected.to contain_postgresql_psql('grant:test') - .with_command(%r{GRANT EXECUTE ON FUNCTION "test"\("text","text"\) TO\s* "test"}m) - .with_unless(%r{SELECT 1 WHERE has_function_privilege\('test',\s* 'test\("text","text"\)', 'EXECUTE'\)}m) + .with_command(%r{GRANT EXECUTE ON FUNCTION test\(text,boolean\) TO\s* "test"}m) + .with_unless(%r{SELECT 1 WHERE has_function_privilege\('test',\s* 'test\(text,boolean\)', 'EXECUTE'\)}m) end end @@ -274,7 +274,7 @@ class {'postgresql::server':} role: 'test', privilege: 'execute', object_name: ['myschema', 'test'], - object_arguments: ['text', 'text'], + object_arguments: ['text', 'boolean'], object_type: 'function', } end @@ -287,8 +287,8 @@ class {'postgresql::server':} it { is_expected.to contain_postgresql__server__grant('test') } it do is_expected.to contain_postgresql_psql('grant:test') - .with_command(%r{GRANT EXECUTE ON FUNCTION "myschema"."test"\("text","text"\) TO\s* "test"}m) - .with_unless(%r{SELECT 1 WHERE has_function_privilege\('test',\s* 'myschema.test\("text","text"\)', 'EXECUTE'\)}m) + .with_command(%r{GRANT EXECUTE ON FUNCTION myschema.test\(text,boolean\) TO\s* "test"}m) + .with_unless(%r{SELECT 1 WHERE has_function_privilege\('test',\s* 'myschema.test\(text,boolean\)', 'EXECUTE'\)}m) end end From 215b350bc6c7b1aace9778464c3380e40a56c163 Mon Sep 17 00:00:00 2001 From: Oliver Freyermuth Date: Fri, 13 Mar 2020 22:15:47 +0100 Subject: [PATCH 6/7] grant: Fixup quoting for function permission granting if schema is given. This was missed before and found by the test. --- manifests/server/grant.pp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/manifests/server/grant.pp b/manifests/server/grant.pp index d02206463a..56c5c4ca6e 100644 --- a/manifests/server/grant.pp +++ b/manifests/server/grant.pp @@ -421,7 +421,10 @@ # } case $_object_name { Array: { - $_togrant_object = join($_object_name, '"."') + $_togrant_object = $_enquote_object ? { + false => join($_object_name, '.'), + default => join($_object_name, '"."'), + } # Never put double quotes into has_*_privilege function $_granted_object = join($_object_name, '.') } From ec600a7b3de06861b16f9935c33d1b9e0be205be Mon Sep 17 00:00:00 2001 From: Oliver Freyermuth Date: Fri, 13 Mar 2020 23:19:18 +0100 Subject: [PATCH 7/7] acceptance: grant_spec: test grant EXECUTE on function with argument. --- spec/acceptance/server/grant_spec.rb | 32 ++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/spec/acceptance/server/grant_spec.rb b/spec/acceptance/server/grant_spec.rb index 48f35c27a5..05aef5f6bc 100644 --- a/spec/acceptance/server/grant_spec.rb +++ b/spec/acceptance/server/grant_spec.rb @@ -224,6 +224,25 @@ class { 'postgresql::server': } require => [ Postgresql_psql['create test function'], Postgresql::Server::Role[$user], ] } + + postgresql_psql { 'create test function with argument': + command => "CREATE FUNCTION test_func_with_arg(val integer) RETURNS integer AS 'SELECT val + 1' LANGUAGE 'sql'", + db => $db, + psql_user => $owner, + unless => "SELECT 1 FROM (SELECT format('%I.%I(%s)', ns.nspname, p.proname, oidvectortypes(p.proargtypes)) as func_with_args FROM pg_proc p INNER JOIN pg_namespace ns ON (p.pronamespace = ns.oid) WHERE ns.nspname not in ('pg_catalog', 'information_schema')) as funclist WHERE func_with_args='public.test_func_with_arg(integer)'", + require => Postgresql::Server::Database[$db], + } + + postgresql::server::grant { 'grant execute on test_func_with_arg': + privilege => 'EXECUTE', + object_type => 'FUNCTION', + object_name => 'test_func_with_arg', + object_arguments => ['integer'], + db => $db, + role => $user, + require => [ Postgresql_psql['create test function with argument'], + Postgresql::Server::Role[$user], ] + } MANIFEST end @@ -240,6 +259,19 @@ class { 'postgresql::server': } end end end + it 'grants execute on a function with argument to a user' do + begin + if Gem::Version.new(postgresql_version) >= Gem::Version.new('9.0') + idempotent_apply(pp) + + ## Check that the privilege was granted to the user + psql("-d #{db} --command=\"SELECT 1 WHERE has_function_privilege('#{user}', 'test_func_with_arg(integer)', 'EXECUTE')\"", user) do |r| + expect(r.stdout).to match(%r{\(1 row\)}) + expect(r.stderr).to eq('') + end + end + end + end end ### TABLE grants context 'table' do