grant db admin the auto user role with admin option

[GavinFrazar]

Changelog: Fixed a permissions error with Postgres database user auto-provisioning that occurs when the database admin is not a superuser and the database is upgraded to Postgres v16 or higher.

This PR fixes an issue I found where when RDS postgres on v15 or below is configured with auto user provisioning, and "teleport-admin" is not granted rds_superuser (or SUPERUSER attribute in OSS), auto user provisioning breaks after upgrading the instance to v16 or higher:

tsh db connect error:

psql: error: connection to server at "localhost" (::1), port 52441 failed: Connection refused
	Is the server running on that host and accepting TCP/IP connections?
connection to server at "localhost" (127.0.0.1), port 52441 failed: your Teleport role requires automatic database user provisioning but an attempt to activate database user "alice" failed due to the following error: ERROR: permission denied to grant role "teleport-auto-user" (SQLSTATE 42501)

connecting as the admin manually and trying to grant the auto user role to another user:

psql (16.4 (Homebrew), server 16.3)
SSL connection (protocol: TLSv1.3, cipher: TLS_AES_128_GCM_SHA256, compression: off)
Type "help" for help.

postgres=> grant "teleport-auto-user" to foo;
ERROR:  permission denied to grant role "teleport-auto-user"
DETAIL:  Only roles with the ADMIN option on role "teleport-auto-user" may grant this role.

I tested this manually by upgrading my RDS instance v14 -> v16, and observed that it breaks auto user provisioning.

Had we always granted the auto user role to ourselves, then the upgrade to v16 would not break anything - hence this PR

for more details, see my comment here explaining the nuances of v16+ postgres and RDS: #48897 (comment)

[GavinFrazar]

TODO: test this change against OSS postgres. I'm glad to see the e2e tests passed in AWS though.

[greedy52]

nice find! change looks good to me.

Two questions:

1. Is it possible to improve the performance by reducing number of sql calls per user connection? For example, maybe we can check if teleportAutoUserRole is already assigned to admin user with admin option (so 99% of time 1 call, 1% 3 calls). If not easy, we can leave it as it is since we already making too many calls. but i would like to improve when we can.
2. How does this affect Redshift?

[GavinFrazar]

1. Is it possible to improve the performance by reducing number of sql calls per user connection? For example, maybe we can check if teleportAutoUserRole is already assigned to admin user with admin option (so 99% of time 1 call, 1% 3 calls). If not easy, we can leave it as it is since we already making too many calls. but i would like to improve when we can.

I agree this is a good idea, I'll do that

2. How does this affect Redshift?

If by this you mean the issue:

In redshift, they document this about granting roles https://docs.aws.amazon.com/redshift/latest/dg/t_role_assignment.html

Role administrators include role owners and users who have been granted the role with the ADMIN OPTION permission...
Only superusers or role administrators can grant and revoke roles. You can grant or revoke one or more roles to or from one or more roles or users. Use the WITH ADMIN OPTION option in the GRANT ROLE statement to provide the administration options for all the granted roles to all the grantees.

But they also document that superusers are defined as those users with CREATEUSER attribute: https://docs.aws.amazon.com/redshift/latest/dg/r_superusers.html

To create a new database superuser, log on to the database as a superuser and issue a CREATE USER command or an ALTER USER command with the CREATEUSER permission.

Our auto-user provisioning for redshift involves creating the "teleport-auto-user" role and creating users which are granted that role.
Our docs tell people to grant sys:superuser role to the Teleport admin, but based on these docs I think having CREATEUSER does the same thing.
Either way, there won't be any issues with Redshift upgrades for now.

As for how this PR affects redshift: the syntax is supported and does essentially the same thing. I'll test it out to be sure - worst case there's some issue that causes a debug error log but otherwise doesn't break anything.

[public-teleport-github-review-bot]

@GavinFrazar See the table below for backport results.

Branch	Result
branch/v15	Failed
branch/v16	Create PR
branch/v17	Create PR

[albundy83]

@GavinFrazar the line here:

https://github.com/gravitational/teleport/blob/v17.1.3/lib/srv/db/postgres/users.go#L433

stmt := fmt.Sprintf("grant role %q to %q WITH ADMIN OPTION", teleportAutoUserRole, adminUser)

should be:

stmt := fmt.Sprintf("grant %q to %q WITH ADMIN OPTION", teleportAutoUserRole, adminUser)

I mean without the word "role" no ?

[GavinFrazar]

@GavinFrazar the line here:

https://github.com/gravitational/teleport/blob/v17.1.3/lib/srv/db/postgres/users.go#L433

stmt := fmt.Sprintf("grant role %q to %q WITH ADMIN OPTION", teleportAutoUserRole, adminUser)

should be:

stmt := fmt.Sprintf("grant %q to %q WITH ADMIN OPTION", teleportAutoUserRole, adminUser)

I mean without the word "role" no ?

🤦‍♂️ yes it should not have "role" in it, thank you for mentioning it.
Tests did not catch it because the error only causes a DEBUG log line.
Fixing now