Skip to content

(GH-216) Alter role call not idempotent with cleartext passwords #225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 1, 2013
Merged

(GH-216) Alter role call not idempotent with cleartext passwords #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ source 'https://rubygems.org'
group :development, :test do
gem 'rake'
gem 'puppetlabs_spec_helper', :require => false
gem 'rspec-system-puppet', '~>1.0'
gem 'rspec-system', '>=1.2.1'
gem 'rspec-system-puppet', '~>2.0'
gem 'puppet-lint', '~> 0.3.2'
end

Expand Down
26 changes: 16 additions & 10 deletions manifests/role.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@
# limitations under the License.

define postgresql::role(
$password_hash = false,
$createdb = false,
$createrole = false,
$db = 'postgres',
$login = false,
$superuser = false,
$replication = false,
$connection_limit = '-1',
$username = $title
$password_hash = false,
$createdb = false,
$createrole = false,
$db = 'postgres',
$login = false,
$superuser = false,
$replication = false,
$connection_limit = '-1',
$username = $title
) {
include postgresql::params

Expand Down Expand Up @@ -80,8 +80,14 @@
}

if $password_hash {
if($password_hash =~ /^md5.+/) {
$pwd_hash_sql = $password_hash
} else {
$pwd_md5 = md5("${password_hash}${username}")
$pwd_hash_sql = "md5${pwd_md5}"
}
postgresql_psql {"ALTER ROLE \"${username}\" ${password_sql}":
unless => "SELECT usename FROM pg_shadow WHERE usename='${username}' and passwd='${password_hash}'",
unless => "SELECT usename FROM pg_shadow WHERE usename='${username}' and passwd='${pwd_hash_sql}'",
}
}
}
115 changes: 60 additions & 55 deletions spec/system/install_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ class { 'postgresql::server': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
end

puppet_apply(pp) do |r|
r.refresh
r.exit_code.should == 0
end

Expand Down Expand Up @@ -78,9 +76,7 @@ class { 'postgresql::server': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
end

puppet_apply(pp) do |r|
r.refresh
r.exit_code.should == 0
end

Expand Down Expand Up @@ -111,9 +107,7 @@ class { 'postgresql::server': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
end

puppet_apply(pp) do |r|
r.refresh
r.exit_code.should == 0
end

Expand Down Expand Up @@ -146,9 +140,7 @@ class { 'postgresql::server': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
end

puppet_apply(pp) do |r|
r.refresh
r.exit_code.should == 0
end

Expand Down Expand Up @@ -182,9 +174,7 @@ class { 'postgresql::server': }
puppet_apply(pp) do |r|
r.exit_code.should_not == 1
r.stdout.should =~ /postgresql::psql is deprecated/
end

puppet_apply(pp) do |r|
r.refresh
r.exit_code.should == 2
r.stdout.should =~ /postgresql::psql is deprecated/
end
Expand All @@ -207,9 +197,7 @@ class { 'postgresql::server': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
end

puppet_apply(pp) do |r|
r.refresh
r.exit_code.should == 2
end
end
Expand All @@ -229,15 +217,13 @@ class { 'postgresql::server': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
end

puppet_apply(pp) do |r|
r.exit_code.should be_zero
r.refresh
r.exit_code.should == 0
end
end
end

describe 'postgresql::user' do
describe 'postgresql::database_user' do
it 'should idempotently create a user who can log in' do
pp = <<-EOS
$user = "postgresql_test_user"
Expand All @@ -259,16 +245,14 @@ class { 'postgresql::server': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
end

puppet_apply(pp) do |r|
r.exit_code.should be_zero
r.refresh
r.exit_code.should == 0
end

# Check that the user can log in
psql('--command="select datname from pg_database" postgres', 'postgresql_test_user') do |r|
r.stdout.should =~ /template1/
r.stderr.should be_empty
r.stderr.should == ''
r.exit_code.should == 0
end
end
Expand All @@ -294,16 +278,47 @@ class { 'postgresql::server': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
r.refresh
r.exit_code.should == 0
end

# Check that the user can log in
psql('--command="select datname from pg_database" postgres', 'postgresql_test_user') do |r|
r.stdout.should =~ /template1/
r.stderr.should == ''
r.exit_code.should == 0
end
end

it 'should idempotently create a user with a cleartext password' do
pp = <<-EOS
$user = "postgresql_test_user2"
$password = "postgresql_test_password2"

include 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::database_user { $user:
password_hash => $password,
require => [ Class['postgresql::server'],
User[$user] ],
}
EOS

puppet_apply(pp) do |r|
r.exit_code.should be_zero
r.exit_code.should_not == 1
r.refresh
r.exit_code.should == 0
end

# Check that the user can log in
psql('--command="select datname from pg_database" postgres', 'postgresql_test_user') do |r|
psql('--command="select datname from pg_database" postgres', 'postgresql_test_user2') do |r|
r.stdout.should =~ /template1/
r.stderr.should be_empty
r.stderr.should == ''
r.exit_code.should == 0
end
end
Expand Down Expand Up @@ -349,16 +364,14 @@ class { 'postgresql::server': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
end

puppet_apply(pp) do |r|
r.exit_code.should be_zero
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.stderr.should == ''
r.exit_code.should == 0
end
ensure
Expand Down Expand Up @@ -416,10 +429,8 @@ class { 'postgresql::server': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
end

puppet_apply(pp) do |r|
r.exit_code.should be_zero
r.refresh
r.exit_code.should == 0
end

## Check that the user can create a table in the database
Expand Down Expand Up @@ -448,10 +459,8 @@ class { 'postgresql::server': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
end

puppet_apply(pp) do |r|
r.exit_code.should be_zero
r.refresh
r.exit_code.should == 0
end

pp = <<-EOS
Expand All @@ -464,7 +473,7 @@ class { 'postgresql::server': }
EOS

puppet_apply(pp) do |r|
r.exit_code.should be_zero
r.exit_code.should == 0
end
end

Expand Down Expand Up @@ -537,22 +546,20 @@ class { 'postgresql::server': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
end

puppet_apply(pp) do |r|
r.refresh
r.exit_code.should == 0
end

# Check that databases use correct tablespaces
psql('--command="select ts.spcname from pg_database db, pg_tablespace ts where db.dattablespace = ts.oid and db.datname = \'"\'tablespacedb1\'"\'"') do |r|
r.stdout.should =~ /tablespace1/
r.stderr.should be_empty
r.stderr.should == ''
r.exit_code.should == 0
end

psql('--command="select ts.spcname from pg_database db, pg_tablespace ts where db.dattablespace = ts.oid and db.datname = \'"\'tablespacedb3\'"\'"') do |r|
r.stdout.should =~ /tablespace2/
r.stderr.should be_empty
r.stderr.should == ''
r.exit_code.should == 0
end
end
Expand Down Expand Up @@ -633,16 +640,14 @@ class { 'pg_test': }

puppet_apply(pp) do |r|
r.exit_code.should_not == 1
end

puppet_apply(pp) do |r|
r.refresh
r.exit_code.should be_zero
end

psql('--command="show max_connections" -t') do |r|
r.stdout.should =~ /123/
r.stderr.should be_empty
r.exit_code.should be_zero
r.stderr.should == ''
r.exit_code.should == 0
end

pp = <<-EOS
Expand Down
8 changes: 4 additions & 4 deletions spec/system/non_defaults_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ class { "postgresql::plperl": }
# Currently puppetlabs/apt shows deprecated messages
#r.stderr.should be_empty
[2,6].should include(r.exit_code)
end

puppet_apply(pp) do |r|
r.refresh

# Currently puppetlabs/apt shows deprecated messages
#r.stderr.should be_empty
# It also returns a 4
Expand Down Expand Up @@ -90,9 +90,9 @@ class { 'postgresql::server': }
#r.stderr.should be_empty
# It also returns a 6
[2,6].should include(r.exit_code)
end

puppet_apply(pp) do |r|
r.refresh

# Currently puppetlabs/apt shows deprecated messages
#r.stderr.should be_empty
# It also returns a 2
Expand Down