Skip to content

Commit a1b9728

Browse files
author
Ashley Penney
committed
Merge pull request puppetlabs#225 from kbarber/ticket/master/puppetlabsGH-216-alter-role-not-idempotent
(puppetlabsGH-216) Alter role call not idempotent with cleartext passwords
2 parents 721d5e9 + 9c2dab9 commit a1b9728

File tree

4 files changed

+81
-71
lines changed

4 files changed

+81
-71
lines changed

‎Gemfile‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ source 'https://rubygems.org'
33
group :development, :test do
44
gem 'rake'
55
gem 'puppetlabs_spec_helper', :require => false
6-
gem 'rspec-system-puppet', '~>1.0'
7-
gem 'rspec-system', '>=1.2.1'
6+
gem 'rspec-system-puppet', '~>2.0'
87
gem 'puppet-lint', '~> 0.3.2'
98
end
109

‎manifests/role.pp‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@
1717
# limitations under the License.
1818

1919
define postgresql::role(
20-
$password_hash = false,
21-
$createdb = false,
22-
$createrole = false,
23-
$db = 'postgres',
24-
$login = false,
25-
$superuser = false,
26-
$replication = false,
27-
$connection_limit = '-1',
28-
$username = $title
20+
$password_hash = false,
21+
$createdb = false,
22+
$createrole = false,
23+
$db = 'postgres',
24+
$login = false,
25+
$superuser = false,
26+
$replication = false,
27+
$connection_limit = '-1',
28+
$username = $title
2929
) {
3030
include postgresql::params
3131

@@ -80,8 +80,14 @@
8080
}
8181

8282
if $password_hash {
83+
if($password_hash =~ /^md5.+/) {
84+
$pwd_hash_sql = $password_hash
85+
} else {
86+
$pwd_md5 = md5("${password_hash}${username}")
87+
$pwd_hash_sql = "md5${pwd_md5}"
88+
}
8389
postgresql_psql {"ALTER ROLE \"${username}\" ${password_sql}":
84-
unless => "SELECT usename FROM pg_shadow WHERE usename='${username}' and passwd='${password_hash}'",
90+
unless => "SELECT usename FROM pg_shadow WHERE usename='${username}' and passwd='${pwd_hash_sql}'",
8591
}
8692
}
8793
}

‎spec/system/install_spec.rb‎

Lines changed: 60 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,7 @@ class { 'postgresql::server': }
3737

3838
puppet_apply(pp) do |r|
3939
r.exit_code.should_not == 1
40-
end
41-
42-
puppet_apply(pp) do |r|
40+
r.refresh
4341
r.exit_code.should == 0
4442
end
4543

@@ -78,9 +76,7 @@ class { 'postgresql::server': }
7876

7977
puppet_apply(pp) do |r|
8078
r.exit_code.should_not == 1
81-
end
82-
83-
puppet_apply(pp) do |r|
79+
r.refresh
8480
r.exit_code.should == 0
8581
end
8682

@@ -111,9 +107,7 @@ class { 'postgresql::server': }
111107

112108
puppet_apply(pp) do |r|
113109
r.exit_code.should_not == 1
114-
end
115-
116-
puppet_apply(pp) do |r|
110+
r.refresh
117111
r.exit_code.should == 0
118112
end
119113

@@ -146,9 +140,7 @@ class { 'postgresql::server': }
146140

147141
puppet_apply(pp) do |r|
148142
r.exit_code.should_not == 1
149-
end
150-
151-
puppet_apply(pp) do |r|
143+
r.refresh
152144
r.exit_code.should == 0
153145
end
154146

@@ -182,9 +174,7 @@ class { 'postgresql::server': }
182174
puppet_apply(pp) do |r|
183175
r.exit_code.should_not == 1
184176
r.stdout.should =~ /postgresql::psql is deprecated/
185-
end
186-
187-
puppet_apply(pp) do |r|
177+
r.refresh
188178
r.exit_code.should == 2
189179
r.stdout.should =~ /postgresql::psql is deprecated/
190180
end
@@ -207,9 +197,7 @@ class { 'postgresql::server': }
207197

208198
puppet_apply(pp) do |r|
209199
r.exit_code.should_not == 1
210-
end
211-
212-
puppet_apply(pp) do |r|
200+
r.refresh
213201
r.exit_code.should == 2
214202
end
215203
end
@@ -229,15 +217,13 @@ class { 'postgresql::server': }
229217

230218
puppet_apply(pp) do |r|
231219
r.exit_code.should_not == 1
232-
end
233-
234-
puppet_apply(pp) do |r|
235-
r.exit_code.should be_zero
220+
r.refresh
221+
r.exit_code.should == 0
236222
end
237223
end
238224
end
239225

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

260246
puppet_apply(pp) do |r|
261247
r.exit_code.should_not == 1
262-
end
263-
264-
puppet_apply(pp) do |r|
265-
r.exit_code.should be_zero
248+
r.refresh
249+
r.exit_code.should == 0
266250
end
267251

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

295279
puppet_apply(pp) do |r|
296280
r.exit_code.should_not == 1
281+
r.refresh
282+
r.exit_code.should == 0
283+
end
284+
285+
# Check that the user can log in
286+
psql('--command="select datname from pg_database" postgres', 'postgresql_test_user') do |r|
287+
r.stdout.should =~ /template1/
288+
r.stderr.should == ''
289+
r.exit_code.should == 0
297290
end
291+
end
292+
293+
it 'should idempotently create a user with a cleartext password' do
294+
pp = <<-EOS
295+
$user = "postgresql_test_user2"
296+
$password = "postgresql_test_password2"
297+
298+
include postgresql::server
299+
300+
# Since we are not testing pg_hba or any of that, make a local user for ident auth
301+
user { $user:
302+
ensure => present,
303+
}
304+
305+
postgresql::database_user { $user:
306+
password_hash => $password,
307+
require => [ Class['postgresql::server'],
308+
User[$user] ],
309+
}
310+
EOS
298311

299312
puppet_apply(pp) do |r|
300-
r.exit_code.should be_zero
313+
r.exit_code.should_not == 1
314+
r.refresh
315+
r.exit_code.should == 0
301316
end
302317

303318
# Check that the user can log in
304-
psql('--command="select datname from pg_database" postgres', 'postgresql_test_user') do |r|
319+
psql('--command="select datname from pg_database" postgres', 'postgresql_test_user2') do |r|
305320
r.stdout.should =~ /template1/
306-
r.stderr.should be_empty
321+
r.stderr.should == ''
307322
r.exit_code.should == 0
308323
end
309324
end
@@ -349,16 +364,14 @@ class { 'postgresql::server': }
349364

350365
puppet_apply(pp) do |r|
351366
r.exit_code.should_not == 1
352-
end
353-
354-
puppet_apply(pp) do |r|
355-
r.exit_code.should be_zero
367+
r.refresh
368+
r.exit_code.should == 0
356369
end
357370

358371
# Check that the user can create a table in the database
359372
psql('--command="create table foo (foo int)" postgres', 'psql_grant_tester') do |r|
360373
r.stdout.should =~ /CREATE TABLE/
361-
r.stderr.should be_empty
374+
r.stderr.should == ''
362375
r.exit_code.should == 0
363376
end
364377
ensure
@@ -416,10 +429,8 @@ class { 'postgresql::server': }
416429

417430
puppet_apply(pp) do |r|
418431
r.exit_code.should_not == 1
419-
end
420-
421-
puppet_apply(pp) do |r|
422-
r.exit_code.should be_zero
432+
r.refresh
433+
r.exit_code.should == 0
423434
end
424435

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

449460
puppet_apply(pp) do |r|
450461
r.exit_code.should_not == 1
451-
end
452-
453-
puppet_apply(pp) do |r|
454-
r.exit_code.should be_zero
462+
r.refresh
463+
r.exit_code.should == 0
455464
end
456465

457466
pp = <<-EOS
@@ -464,7 +473,7 @@ class { 'postgresql::server': }
464473
EOS
465474

466475
puppet_apply(pp) do |r|
467-
r.exit_code.should be_zero
476+
r.exit_code.should == 0
468477
end
469478
end
470479

@@ -537,22 +546,20 @@ class { 'postgresql::server': }
537546

538547
puppet_apply(pp) do |r|
539548
r.exit_code.should_not == 1
540-
end
541-
542-
puppet_apply(pp) do |r|
549+
r.refresh
543550
r.exit_code.should == 0
544551
end
545552

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

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

634641
puppet_apply(pp) do |r|
635642
r.exit_code.should_not == 1
636-
end
637-
638-
puppet_apply(pp) do |r|
643+
r.refresh
639644
r.exit_code.should be_zero
640645
end
641646

642647
psql('--command="show max_connections" -t') do |r|
643648
r.stdout.should =~ /123/
644-
r.stderr.should be_empty
645-
r.exit_code.should be_zero
649+
r.stderr.should == ''
650+
r.exit_code.should == 0
646651
end
647652

648653
pp = <<-EOS

‎spec/system/non_defaults_spec.rb‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ class { "postgresql::plperl": }
5656
# Currently puppetlabs/apt shows deprecated messages
5757
#r.stderr.should be_empty
5858
[2,6].should include(r.exit_code)
59-
end
6059

61-
puppet_apply(pp) do |r|
60+
r.refresh
61+
6262
# Currently puppetlabs/apt shows deprecated messages
6363
#r.stderr.should be_empty
6464
# It also returns a 4
@@ -90,9 +90,9 @@ class { 'postgresql::server': }
9090
#r.stderr.should be_empty
9191
# It also returns a 6
9292
[2,6].should include(r.exit_code)
93-
end
9493

95-
puppet_apply(pp) do |r|
94+
r.refresh
95+
9696
# Currently puppetlabs/apt shows deprecated messages
9797
#r.stderr.should be_empty
9898
# It also returns a 2

0 commit comments

Comments
 (0)