PostgreSQL: leaked pg privs (#14817)

* Fix doc bug. Spell `collation` like `lc_collate`.

* Whitespace nit in error message

* Use %q as the format verb for error messages in postgresql_database resource messages.

* REVOKE the `GRANT` given to the connection user when creating a database.

For `ROLE`s who have been delegated `CREATEDB` privileges and are not a
superuser, in order for them to `CREATE DATABASE` they need to be a member
of the `ROLE` who will be `OWNER` for the new database.  Once the
`CREATE DATABASE` is complete, `REVOKE` the `GRANT` that was given to role
so that the user who ran the `CREATE DATABASE` looses all privileges to the
target database (unless of course they're a superuser).

Fixes a regression introduced in #11452

* Delegated DBA ROLEs can now fix OWNER drift for PostgreSQL databases.

Uses the helper functions introduced in #11452

builtin/providers/postgresql/resource_postgresql_database.go

@@ -125,8 +125,15 @@ func resourcePostgreSQLDatabaseCreate(d *schema.ResourceData, meta interface{})
 	//needed in order to set the owner of the db if the connection user is not a superuser
 	err = grantRoleMembership(conn, d.Get(dbOwnerAttr).(string), c.username)
 	if err != nil {
-		return errwrap.Wrapf(fmt.Sprintf("Error granting role membership on  database %s: {{err}}", dbName), err)
+		return errwrap.Wrapf(fmt.Sprintf("Error adding connection user (%q) to ROLE %q: {{err}}", c.username, d.Get(dbOwnerAttr).(string)), err)
 	}
+	defer func() {
+		//undo the grant if the connection user is not a superuser
+		err = revokeRoleMembership(conn, d.Get(dbOwnerAttr).(string), c.username)
+		if err != nil {
+			err = errwrap.Wrapf(fmt.Sprintf("Error removing connection user (%q) from ROLE %q: {{err}}", c.username, d.Get(dbOwnerAttr).(string)), err)
+		}
+	}()
 
 	// Handle each option individually and stream results into the query
 	// buffer.
@@ -190,12 +197,15 @@ func resourcePostgreSQLDatabaseCreate(d *schema.ResourceData, meta interface{})
 	query := b.String()
 	_, err = conn.Query(query)
 	if err != nil {
-		return errwrap.Wrapf(fmt.Sprintf("Error creating database %s: {{err}}", dbName), err)
+		return errwrap.Wrapf(fmt.Sprintf("Error creating database %q: {{err}}", dbName), err)
 	}
 
 	d.SetId(dbName)
 
-	return resourcePostgreSQLDatabaseReadImpl(d, meta)
+	// Set err outside of the return so that the deferred revoke can override err
+	// if necessary.
+	err = resourcePostgreSQLDatabaseReadImpl(d, meta)
+	return err
 }
 
 func resourcePostgreSQLDatabaseDelete(d *schema.ResourceData, meta interface{}) error {
@@ -278,7 +288,7 @@ func resourcePostgreSQLDatabaseReadImpl(d *schema.ResourceData, meta interface{}
 	err = conn.QueryRow("SELECT d.datname, pg_catalog.pg_get_userbyid(d.datdba) from pg_database d WHERE datname=$1", dbId).Scan(&dbName, &ownerName)
 	switch {
 	case err == sql.ErrNoRows:
-		log.Printf("[WARN] PostgreSQL database (%s) not found", dbId)
+		log.Printf("[WARN] PostgreSQL database (%q) not found", dbId)
 		d.SetId("")
 		return nil
 	case err != nil:
@@ -295,7 +305,7 @@ func resourcePostgreSQLDatabaseReadImpl(d *schema.ResourceData, meta interface{}
 		)
 	switch {
 	case err == sql.ErrNoRows:
-		log.Printf("[WARN] PostgreSQL database (%s) not found", dbId)
+		log.Printf("[WARN] PostgreSQL database (%q) not found", dbId)
 		d.SetId("")
 		return nil
 	case err != nil:
@@ -334,7 +344,7 @@ func resourcePostgreSQLDatabaseUpdate(d *schema.ResourceData, meta interface{})
 		return err
 	}
 
-	if err := setDBOwner(conn, d); err != nil {
+	if err := setDBOwner(c, conn, d); err != nil {
 		return err
 	}
 
@@ -380,7 +390,7 @@ func setDBName(conn *sql.DB, d *schema.ResourceData) error {
 	return nil
 }
 
-func setDBOwner(conn *sql.DB, d *schema.ResourceData) error {
+func setDBOwner(c *Client, conn *sql.DB, d *schema.ResourceData) error {
 	if !d.HasChange(dbOwnerAttr) {
 		return nil
 	}
@@ -390,13 +400,26 @@ func setDBOwner(conn *sql.DB, d *schema.ResourceData) error {
 		return nil
 	}
 
+	//needed in order to set the owner of the db if the connection user is not a superuser
+	err := grantRoleMembership(conn, d.Get(dbOwnerAttr).(string), c.username)
+	if err != nil {
+		return errwrap.Wrapf(fmt.Sprintf("Error adding connection user (%q) to ROLE %q: {{err}}", c.username, d.Get(dbOwnerAttr).(string)), err)
+	}
+	defer func() {
+		// undo the grant if the connection user is not a superuser
+		err = revokeRoleMembership(conn, d.Get(dbOwnerAttr).(string), c.username)
+		if err != nil {
+			err = errwrap.Wrapf(fmt.Sprintf("Error removing connection user (%q) from ROLE %q: {{err}}", c.username, d.Get(dbOwnerAttr).(string)), err)
+		}
+	}()
+
 	dbName := d.Get(dbNameAttr).(string)
 	query := fmt.Sprintf("ALTER DATABASE %s OWNER TO %s", pq.QuoteIdentifier(dbName), pq.QuoteIdentifier(owner))
 	if _, err := conn.Query(query); err != nil {
 		return errwrap.Wrapf("Error updating database OWNER: {{err}}", err)
 	}
 
-	return nil
+	return err
 }
 
 func setDBTablespace(conn *sql.DB, d *schema.ResourceData) error {
@@ -485,3 +508,14 @@ func grantRoleMembership(conn *sql.DB, dbOwner string, connUsername string) erro
 	}
 	return nil
 }
+
+func revokeRoleMembership(conn *sql.DB, dbOwner string, connUsername string) error {
+	if dbOwner != "" && dbOwner != connUsername {
+		query := fmt.Sprintf("REVOKE %s FROM %s", pq.QuoteIdentifier(dbOwner), pq.QuoteIdentifier(connUsername))
+		_, err := conn.Query(query)
+		if err != nil {
+			return errwrap.Wrapf("Error revoking membership: {{err}}", err)
+		}
+	}
+	return nil
+}

website/source/docs/providers/postgresql/r/postgresql_database.html.markdown

@@ -20,7 +20,7 @@ resource "postgresql_database" "my_db" {
   name              = "my_db"
   owner             = "my_role"
   template          = "template0"
-  collation         = "C"
+  lc_collate        = "C"
   connection_limit  = -1
   allow_connections = true
 }