Fix wrong Sensitive handling for updating role passwords

[cruelsmith]

Before merging a String with a Sensitive the Sensitive need to be unwrapped else the value of the Sensitive will just Sensitive [value redacted].

Since $pwd_hash_sql will be merged with $pw_command and $unless_pw_command that also later will be Sensitive we just now ensure that $pwd_hash_sql will always be not a Sensitive.

Fixes #1402

[puppet-community-rangefinder]

postgresql::server::role is a type

Breaking changes to this file WILL impact these 22 modules (exact match):

• nicopaez-dockerserver
• braiins-zabbix
• jjulien-trac
• landcareresearch-pidservice
• braiins-sentry
• nnutter-testdb
• deric-pgprobackup
• Aethylred-gitlab
• nicopaez-alfred
• nicopaez-triage
• lwo-dataverse
• hetzner-roundcube
• maestrodev-maestro
• it2ndq-barman
• npwalker-pe_external_postgresql
• adullact-demarchessimplifiees
• deric-barman
• puppetfinland-puppetmaster
• landcareresearch-ckan
• groupbuddies-gb
• blackknight36-bacula
• puppetlabs-puppetdb

Breaking changes to this file MAY impact these 6 modules (near match):

• Firebladee-moodle
• tedivm-hieratic
• Aethylred-puppetdashboard
• puppetfinland-patchwork
• echoes-wrappers
• soli-wrappers

This module is declared in 70 of 580 indexed public Puppetfiles.

---

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

[smortex]

Calling postgresql::postgresql_password() with a variable $password_hash as the password parameter look wrong: as far as I can see, postgresql::server::role only has a password_hash parameter, so I would not expect it to hash anything itself (and be passed an already hashed password).

The postgresql::server::db class has a password parameter which is copied to the password_hash parameter of postgresql::server::role verbatim. This is inconsistent, but I think this is for backward compatibility since when I check my (old) code I have things like:

postgresql::server::db { $taiga::back::db_name:
  user     => $taiga::back::db_user,
  password => postgresql_password($taiga::back::db_user, $taiga::back::db_password),
}

The test suite mostly checks that values are sensitive ones, so basically that they match Sensitive [value redacted], but not the actual unwrapped value. I opened a PR to unwrap Sensitive values when running the test suite and as you guessed on Slack, some content seems to be wrapped twice.

[cruelsmith]

Yes, the hashing of a hash was also my first guess but it works since it returns the same value as long as the hashtype and salt does match.
Already have an idea to do a check like that to prevent duplicate hashing:

unless $password_hash_unsensitive =~ /^md5[0-9a-f]{32}$/ and $password_hash_unsensitive =~ /^scram-sha-256:/ { 
  $password_sql = postgresql_password([...])
} else {
  $password_sql = $password_hash_unsensitive
}

But i would say i create an additional PR since this PR is bug fixing and the duplicate hashing is more or less a maintenance change.

---

Edit: The function postgresql_password itself checks for the hash already and then just return the password as it is. But it will handle a Sensitive password hash as a password that needs to be hashed and does not convert between Sensitive and not Sensitive with third parameter expect to do.

puppetlabs-postgresql/lib/puppet/functions/postgresql/postgresql_password.rb

Lines 31 to 33 in 7ee8e5c

	if password.is_a?(String) && password.match?(%r{^(md5|SCRAM-SHA-256).+})
	return password
	end

[alexjfisher]

Or just edit lines 182 and 183??

eg.

$pw_command = "ALTER ROLE \"${username}\" ENCRYPTED PASSWORD '${pwd_hash_sql}'"

becomes

$pw_command = "ALTER ROLE \"${username}\" ENCRYPTED PASSWORD '${pwd_hash_sql.unwrap}'"

[alexjfisher]

From puppet 6.24.0 onwards, using unwrap on non Sensitive strings is fine.

[cruelsmith]

Sure but then we should also update the Deferred way to use Sensitive and rework to remove this, or?

puppetlabs-postgresql/manifests/server/role.pp

Lines 45 to 49 in ad330fc

	$password_hash_unsensitive = if $password_hash =~ Sensitive[String] {
	$password_hash.unwrap
	} else {
	$password_hash
	}

[ekohl]

That part predates the Puppet 6.24 minimum version bound and can indeed be simplified.

[CLAassistant]

All committers have signed the CLA.

[smortex]

Hey! #1405 was merged in the main branch. Rebasing your branch on top of it should fix the conflict.

From your working directory:

git fetch origin       # Download the latest code we have here
git rebase origin/main # Move your commits on top of the main branch
git push -f            # Send the changes (-f is required because we re-wrote history)

[smortex]

LGTM!