Grant owner role to connected user for default privileges management (…

…#71)

* Update resource description in line with PostgreSQL docs

Signed-off-by: Jakub Paweł Głazik <zytek@nuxi.pl>

* Grant owner role to connected user

Make sure connected user has proper permissions to manage default
privileges.

Fixed #70

Signed-off-by: Jakub Paweł Głazik <zytek@nuxi.pl>

* default privileges: grant & revoke the owner in the transaction.

So in this way this temporary grant is not even seen outside the transaction.
This also adds a test to verify that owner is correctly revoked.

postgresql/helpers.go

@@ -37,7 +37,7 @@ func validateConnLimit(v interface{}, key string) (warnings []string, errors []e
 	return
 }
 
-func isRoleMember(db *sql.DB, role, member string) (bool, error) {
+func isRoleMember(db QueryAble, role, member string) (bool, error) {
 	var _rez int
 	err := db.QueryRow(
 		"SELECT 1 FROM pg_auth_members WHERE pg_get_userbyid(roleid) = $1 AND pg_get_userbyid(member) = $2",
@@ -57,7 +57,7 @@ func isRoleMember(db *sql.DB, role, member string) (bool, error) {
 // grantRoleMembership grants the role *role* to the user *member*.
 // It returns false if the grant is not needed because the user is already
 // a member of this role.
-func grantRoleMembership(db *sql.DB, role, member string) (bool, error) {
+func grantRoleMembership(db QueryAble, role, member string) (bool, error) {
 	if member == role {
 		return false, nil
 	}
@@ -80,7 +80,7 @@ func grantRoleMembership(db *sql.DB, role, member string) (bool, error) {
 	return true, nil
 }
 
-func revokeRoleMembership(db *sql.DB, role, member string) error {
+func revokeRoleMembership(db QueryAble, role, member string) error {
 	if member == role {
 		return nil
 	}

postgresql/resource_postgresql_default_privileges.go

@@ -38,7 +38,7 @@ func resourcePostgreSQLDefaultPrivileges() *schema.Resource {
 				Type:        schema.TypeString,
 				Required:    true,
 				ForceNew:    true,
-				Description: "Role for which apply default privileges (You can change default privileges only for objects that will be created by yourself or by roles that you are a member of)",
+				Description: "Target role for which to alter default privileges.",
 			},
 			"schema": {
 				Type:        schema.TypeString,
@@ -100,6 +100,8 @@ func resourcePostgreSQLDefaultPrivilegesCreate(d *schema.ResourceData, meta inte
 	database := d.Get("database").(string)
 
 	client := meta.(*Client)
+	owner := d.Get("owner").(string)
+	currentUser := client.config.getDatabaseUsername()
 
 	client.catalogLock.Lock()
 	defer client.catalogLock.Unlock()
@@ -110,6 +112,13 @@ func resourcePostgreSQLDefaultPrivilegesCreate(d *schema.ResourceData, meta inte
 	}
 	defer deferredRollback(txn)
 
+	// Needed in order to set the owner of the db if the connection user is not a
+	// superuser
+	ownerGranted, err := grantRoleMembership(txn, owner, currentUser)
+	if err != nil {
+		return err
+	}
+
 	// Revoke all privileges before granting otherwise reducing privileges will not work.
 	// We just have to revoke them in the same transaction so role will not lost his privileges between revoke and grant.
 	if err = revokeRoleDefaultPrivileges(txn, d); err != nil {
@@ -120,6 +129,14 @@ func resourcePostgreSQLDefaultPrivilegesCreate(d *schema.ResourceData, meta inte
 		return err
 	}
 
+	// Revoke the owner privileges if we had to grant it.
+	if ownerGranted {
+		err = revokeRoleMembership(txn, owner, currentUser)
+		if err != nil {
+			return err
+		}
+	}
+
 	if err := txn.Commit(); err != nil {
 		return err
 	}
@@ -137,6 +154,8 @@ func resourcePostgreSQLDefaultPrivilegesCreate(d *schema.ResourceData, meta inte
 
 func resourcePostgreSQLDefaultPrivilegesDelete(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*Client)
+	owner := d.Get("owner").(string)
+	currentUser := client.config.getDatabaseUsername()
 
 	client.catalogLock.Lock()
 	defer client.catalogLock.Unlock()
@@ -147,7 +166,25 @@ func resourcePostgreSQLDefaultPrivilegesDelete(d *schema.ResourceData, meta inte
 	}
 	defer deferredRollback(txn)
 
-	revokeRoleDefaultPrivileges(txn, d)
+	// Needed in order to set the owner of the db if the connection user is not a
+	// superuser
+	ownerGranted, err := grantRoleMembership(txn, owner, currentUser)
+	if err != nil {
+		return err
+	}
+
+	if err = revokeRoleDefaultPrivileges(txn, d); err != nil {
+		return err
+	}
+
+	// Revoke the owner privileges if we had to grant it.
+	if ownerGranted {
+		err = revokeRoleMembership(txn, owner, currentUser)
+		if err != nil {
+			return err
+		}
+	}
+
 	if err := txn.Commit(); err != nil {
 		return err
 	}
@@ -203,12 +240,6 @@ func grantRoleDefaultPrivileges(txn *sql.Tx, d *schema.ResourceData) error {
 		privileges = append(privileges, priv.(string))
 	}
 
-	// TODO: We grant default privileges for the DB owner
-	// For that we need to be either superuser or a member of the owner role.
-	// With AWS RDS, It's not possible to create superusers as it is restricted by AWS itself.
-	// In that case, the only solution would be to have the PostgreSQL user used by Terraform
-	// to be also part of the database owner role.
-
 	query := fmt.Sprintf("ALTER DEFAULT PRIVILEGES FOR ROLE %s IN SCHEMA %s GRANT %s ON %sS TO %s",
 		pq.QuoteIdentifier(d.Get("owner").(string)),
 		pq.QuoteIdentifier(pgSchema),
@@ -236,8 +267,11 @@ func revokeRoleDefaultPrivileges(txn *sql.Tx, d *schema.ResourceData) error {
 		pq.QuoteIdentifier(d.Get("role").(string)),
 	)
 
-	_, err := txn.Exec(query)
-	return err
+	if _, err := txn.Exec(query); err != nil {
+		return errwrap.Wrapf("could not revoke default privileges: {{err}}", err)
+	}
+	return nil
+
 }
 
 func generateDefaultPrivilegesID(d *schema.ResourceData) string {

postgresql/resource_postgresql_default_privileges_test.go

@@ -46,7 +46,7 @@ func TestAccPostgresqlDefaultPrivileges(t *testing.T) {
 						tables := []string{"test_schema.test_table"}
 						// To test default privileges, we need to create a table
 						// after having apply the state.
-						dropFunc := createTestTables(t, dbSuffix, tables)
+						dropFunc := createTestTables(t, dbSuffix, tables, "")
 						defer dropFunc()
 
 						return testCheckTablesPrivileges(t, dbSuffix, tables, []string{"SELECT"})
@@ -59,3 +59,66 @@ func TestAccPostgresqlDefaultPrivileges(t *testing.T) {
 		},
 	})
 }
+
+// Test the case where we need to grant the owner to the connected user.
+// The owner should be revoked
+func TestAccPostgresqlDefaultPrivileges_GrantOwner(t *testing.T) {
+	skipIfNotAcc(t)
+
+	// We have to create the database outside of resource.Test
+	// because we need to create a table to assert that grant are correctly applied
+	// and we don't have this resource yet
+	dbSuffix, teardown := setupTestDatabase(t, true, true)
+	defer teardown()
+
+	config := getTestConfig(t)
+	dsn := config.connStr("postgres")
+	dbName, roleName := getTestDBNames(dbSuffix)
+
+	// We set PGUSER as owner as he will create the test table
+	var stateConfig = fmt.Sprintf(`
+
+resource postgresql_role "test_owner" {
+       name = "test_owner"
+}
+
+resource "postgresql_default_privileges" "test_ro" {
+	database    = "%s"
+	owner       = postgresql_role.test_owner.name
+	role        = "%s"
+	schema      = "public"
+	object_type = "table"
+	privileges   = ["SELECT"]
+}
+	`, dbName, roleName)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() {
+			testAccPreCheck(t)
+			testCheckCompatibleVersion(t, featurePrivileges)
+		},
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: stateConfig,
+				Check: resource.ComposeTestCheckFunc(
+					func(*terraform.State) error {
+						tables := []string{"public.test_table"}
+						// To test default privileges, we need to create a table
+						// after having apply the state.
+						dropFunc := createTestTables(t, dbSuffix, tables, "test_owner")
+						defer dropFunc()
+
+						return testCheckTablesPrivileges(t, dbSuffix, tables, []string{"SELECT"})
+					},
+					resource.TestCheckResourceAttr("postgresql_default_privileges.test_ro", "object_type", "table"),
+					resource.TestCheckResourceAttr("postgresql_default_privileges.test_ro", "privileges.#", "1"),
+					resource.TestCheckResourceAttr("postgresql_default_privileges.test_ro", "privileges.3138006342", "SELECT"),
+
+					// check if connected user does not have test_owner granted anymore.
+					checkUserMembership(t, dsn, config.Username, "test_owner", false),
+				),
+			},
+		},
+	})
+}

postgresql/resource_postgresql_grant_test.go

@@ -18,7 +18,7 @@ func TestAccPostgresqlGrant(t *testing.T) {
 	defer teardown()
 
 	testTables := []string{"test_schema.test_table", "test_schema.test_table2"}
-	createTestTables(t, dbSuffix, testTables)
+	createTestTables(t, dbSuffix, testTables, "")
 
 	dbName, roleName := getTestDBNames(dbSuffix)
 	var testGrantSelect = fmt.Sprintf(`

postgresql/utils_test.go

@@ -3,6 +3,7 @@ package postgresql
 import (
 	"database/sql"
 	"fmt"
+	"log"
 	"os"
 	"strconv"
 	"testing"
@@ -107,7 +108,7 @@ func setupTestDatabase(t *testing.T, createDB, createRole bool) (string, func())
 	}
 }
 
-func createTestTables(t *testing.T, dbSuffix string, tables []string) func() {
+func createTestTables(t *testing.T, dbSuffix string, tables []string, owner string) func() {
 	config := getTestConfig(t)
 	dbName, _ := getTestDBNames(dbSuffix)
 
@@ -117,15 +118,35 @@ func createTestTables(t *testing.T, dbSuffix string, tables []string) func() {
 	}
 	defer db.Close()
 
+	if owner != "" {
+		if _, err := db.Exec(fmt.Sprintf("SET ROLE %s", owner)); err != nil {
+			t.Fatalf("could not set role to %s: %v", owner, err)
+		}
+	}
+
 	for _, table := range tables {
 		if _, err := db.Exec(fmt.Sprintf("CREATE TABLE %s (val text)", table)); err != nil {
 			t.Fatalf("could not create test table in db %s: %v", dbName, err)
 		}
+		if owner != "" {
+			if _, err := db.Exec(fmt.Sprintf("ALTER TABLE %s OWNER TO %s", table, owner)); err != nil {
+				t.Fatalf("could not set test_table owner to %s: %v", owner, err)
+			}
+		}
 	}
 	// In this case we need to drop table after each test.
 	return func() {
+		db, err := sql.Open("postgres", config.connStr(dbName))
+		defer db.Close()
+
+		if err != nil {
+			t.Fatalf("could not open connection pool for db %s: %v", dbName, err)
+		}
+
 		for _, table := range tables {
-			db.Exec(fmt.Sprintf("DROP TABLE %s", table))
+			if _, err := db.Exec(fmt.Sprintf("DROP TABLE %s", table)); err != nil {
+				log.Fatalf("could not drop table %s: %v", table, err)
+			}
 		}
 	}
 }