From d433b283df7c9fe8140221f4321b307adb8727e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Can=C3=A9vet?= Date: Tue, 12 Nov 2013 14:23:05 +0100 Subject: [PATCH 1/3] Fix table_grant_spec to show a bug --- spec/system/server/table_grant_spec.rb | 62 ++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/spec/system/server/table_grant_spec.rb b/spec/system/server/table_grant_spec.rb index 0c52c7772d..325541ac16 100644 --- a/spec/system/server/table_grant_spec.rb +++ b/spec/system/server/table_grant_spec.rb @@ -8,6 +8,68 @@ end end + it 'should grant all accesses to a user' do + begin + pp = <<-EOS.unindent + $db = 'table_grant' + $user = 'psql_grant_tester' + $password = 'psql_table_pw' + + class { 'postgresql::server': } + + # Since we are not testing pg_hba or any of that, make a local user for ident auth + user { $user: + ensure => present, + } + + postgresql::server::role { $user: + password_hash => postgresql_password($user, $password), + } + + postgresql::server::database { $db: } + + # Create a rule for the user + postgresql::server::pg_hba_rule { "allow ${user}": + type => 'local', + database => $db, + user => $user, + auth_method => 'ident', + order => 1, + } + + postgresql_psql { 'Create testing table': + command => 'CREATE TABLE "test_table" (field integer NOT NULL)', + db => $db, + unless => "SELECT * FROM pg_tables WHERE tablename = 'test_table'", + require => Postgresql::Server::Database[$db], + } + + postgresql::server::table_grant { 'grant insert test': + privilege => 'ALL', + table => 'test_table', + db => $db, + role => $user, + require => Postgresql_psql['Create testing table'], + } + EOS + + puppet_apply(pp) do |r| + r.exit_code.should_not == 1 + r.refresh + r.exit_code.should == 0 + end + + ## Check that the user can create a table in the database + psql('--command="create table foo (foo int)" postgres', 'psql_grant_tester') do |r| + r.stdout.should =~ /CREATE TABLE/ + r.stderr.should be_empty + r.exit_code.should == 0 + end + ensure + psql('--command="drop table foo" postgres', 'psql_grant_tester') + end + end + it 'should grant access so a user can insert in a table' do begin pp = <<-EOS.unindent From 1ce75d5bc8c4dcd4d5500dfbde2ffa739749ead0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Can=C3=A9vet?= Date: Tue, 12 Nov 2013 16:20:52 +0100 Subject: [PATCH 2/3] Fix granting all privileges on a table --- manifests/server/grant.pp | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/manifests/server/grant.pp b/manifests/server/grant.pp index de424d5fbe..22b8952a69 100644 --- a/manifests/server/grant.pp +++ b/manifests/server/grant.pp @@ -31,15 +31,31 @@ ) ## Validate that the object type's privilege is acceptable + # TODO: this is a terrible hack; if they pass "ALL" as the desired privilege, + # we need a way to test for it--and has_database_privilege does not + # recognize 'ALL' as a valid privilege name. So we probably need to + # hard-code a mapping between 'ALL' and the list of actual privileges that + # it entails, and loop over them to check them. That sort of thing will + # probably need to wait until we port this over to ruby, so, for now, we're + # just going to assume that if they have "CREATE" privileges on a database, + # then they have "ALL". (I told you that it was terrible!) case $_object_type { 'DATABASE': { - validate_string($_privilege,'CREATE','CONNECT','TEMPORARY','TEMP','ALL', - 'ALL PRIVILEGES') + $unless_privilege = $_privilege ? { + 'ALL' => 'CREATE', + default => $_privilege, + } + validate_string($unless_privilege,'CREATE','CONNECT','TEMPORARY','TEMP', + 'ALL','ALL PRIVILEGES') $unless_function = 'has_database_privilege' $on_db = $psql_db } 'TABLE': { - validate_string($_privilege,'SELECT','INSERT','UPDATE','REFERENCES', + $unless_privilege = $_privilege ? { + 'ALL' => 'INSERT', + default => $_privilege, + } + validate_string($unless_privilege,'SELECT','INSERT','UPDATE','REFERENCES', 'ALL','ALL PRIVILEGES') $unless_function = 'has_table_privilege' $on_db = $db @@ -49,18 +65,6 @@ } } - # TODO: this is a terrible hack; if they pass "ALL" as the desired privilege, - # we need a way to test for it--and has_database_privilege does not - # recognize 'ALL' as a valid privilege name. So we probably need to - # hard-code a mapping between 'ALL' and the list of actual privileges that - # it entails, and loop over them to check them. That sort of thing will - # probably need to wait until we port this over to ruby, so, for now, we're - # just going to assume that if they have "CREATE" privileges on a database, - # then they have "ALL". (I told you that it was terrible!) - $unless_privilege = $_privilege ? { - 'ALL' => 'CREATE', - default => $_privilege, - } $grant_cmd = "GRANT ${_privilege} ON ${_object_type} \"${object_name}\" TO \"${role}\"" postgresql_psql { $grant_cmd: db => $on_db, From 4cd64e80ac7473ea1fb47cdfc68ef93a79666084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Can=C3=A9vet?= Date: Fri, 15 Nov 2013 15:20:15 +0100 Subject: [PATCH 3/3] Add missing privileges --- 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 22b8952a69..d24130ca28 100644 --- a/manifests/server/grant.pp +++ b/manifests/server/grant.pp @@ -55,8 +55,8 @@ 'ALL' => 'INSERT', default => $_privilege, } - validate_string($unless_privilege,'SELECT','INSERT','UPDATE','REFERENCES', - 'ALL','ALL PRIVILEGES') + validate_string($unless_privilege,'SELECT','INSERT','UPDATE','DELETE', + 'TRUNCATE','REFERENCES','TRIGGER','ALL','ALL PRIVILEGES') $unless_function = 'has_table_privilege' $on_db = $db }