diff --git a/postgresql/helpers.go b/postgresql/helpers.go index 3c6a6f51..f922a5a0 100644 --- a/postgresql/helpers.go +++ b/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 } diff --git a/postgresql/resource_postgresql_default_privileges.go b/postgresql/resource_postgresql_default_privileges.go index 9f40e23c..4d2f8c50 100644 --- a/postgresql/resource_postgresql_default_privileges.go +++ b/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 { diff --git a/postgresql/resource_postgresql_default_privileges_test.go b/postgresql/resource_postgresql_default_privileges_test.go index 58b6b109..5b7bf063 100644 --- a/postgresql/resource_postgresql_default_privileges_test.go +++ b/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), + ), + }, + }, + }) +} diff --git a/postgresql/resource_postgresql_grant_test.go b/postgresql/resource_postgresql_grant_test.go index b8132683..6d7a1561 100644 --- a/postgresql/resource_postgresql_grant_test.go +++ b/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(` diff --git a/postgresql/utils_test.go b/postgresql/utils_test.go index f175a36a..4e5a066e 100644 --- a/postgresql/utils_test.go +++ b/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) + } } } }