Only create a password for the service role if one is provided (#283)

* Only create a password for the service role if one is provided * Handle switching between LOGIN and NOLOGIN service role
lunarway · Mar 14, 2024 · aa60fd1 · aa60fd1
1 parent 531a401
commit aa60fd1
Show file tree

Hide file tree

Showing 2 changed files with 177 additions and 7 deletions.
diff --git a/pkg/postgres/database.go b/pkg/postgres/database.go
@@ -27,9 +27,6 @@ func (c Credentials) Validate() error {
 	if c.User == "" {
 		return fmt.Errorf("user is empty")
 	}
-	if c.Password == "" {
-		return fmt.Errorf("password is empty")
-	}
 	return nil
 }
 
@@ -93,7 +90,7 @@ func Database(log logr.Logger, host string, adminCredentials, serviceCredentials
 	}()
 
 	// Create the service user
-	err = createUser(log, serviceConnection, serviceCredentials.User, serviceCredentials.Password)
+	err = createServiceRole(log, serviceConnection, serviceCredentials.User, serviceCredentials.Password)
 	if err != nil {
 		return fmt.Errorf("create service user: %w", err)
 	}
@@ -186,13 +183,23 @@ func Database(log logr.Logger, host string, adminCredentials, serviceCredentials
 	return nil
 }
 
-func createUser(log logr.Logger, db *sql.DB, user, password string) error {
+func createServiceRole(log logr.Logger, db *sql.DB, user, password string) error {
 	log = log.WithValues("user", user)
-	return tryExec(log, db, tryExecReq{
+	err := tryExec(log, db, tryExecReq{
 		objectType: "service user",
 		errorCode:  "duplicate_object",
-		query:      fmt.Sprintf("CREATE ROLE %s LOGIN PASSWORD '%s' NOCREATEROLE VALID UNTIL 'infinity'", user, password),
+		query:      fmt.Sprintf("CREATE ROLE %s NOCREATEROLE", user),
 	})
+	if err != nil {
+		return err
+	}
+
+	if password != "" {
+		err = execf(db, "ALTER ROLE %s LOGIN PASSWORD '%s' NOCREATEROLE VALID UNTIL 'infinity'", user, password)
+	} else {
+		err = execf(db, "ALTER ROLE %s NOLOGIN NOCREATEROLE", user)
+	}
+	return err
 }
 
 func createDatabase(log logr.Logger, host string, adminCredentials Credentials, name string) error {

diff --git a/pkg/postgres/database_test.go b/pkg/postgres/database_test.go
@@ -111,6 +111,8 @@ func TestDatabase_sunshine(t *testing.T) {
 		t.Fatalf("EnsurePostgreSQLDatabase failed: %v", err)
 	}
 
+	assert.True(t, roleCanLogin(t, db, name))
+
 	newDB, err := postgres.Connect(log, postgres.ConnectionString{
 		Host:     postgresqlHost,
 		Database: name,
@@ -135,6 +137,155 @@ func TestDatabase_sunshine(t *testing.T) {
 	assert.Equal(t, []string{name}, owners, "owner not as expected")
 }
 
+func TestDatabase_noPassword(t *testing.T) {
+	postgresqlHost := test.Integration(t)
+	log := test.SetLogger(t)
+	managerRole := "postgres_role_name"
+	db, err := postgres.Connect(log, postgres.ConnectionString{
+		Host:     postgresqlHost,
+		Database: "postgres",
+		User:     "iam_creator",
+		Password: "iam_creator",
+	})
+	if err != nil {
+		t.Fatalf("connect to database failed: %v", err)
+	}
+	err = createManagerRole(log, db, managerRole)
+	if err != nil {
+		t.Fatalf("create manager role failed: %v", err)
+	}
+	defer db.Close()
+
+	name := fmt.Sprintf("test_%d", time.Now().UnixNano())
+
+	err = postgres.Database(logf.Log, postgresqlHost,
+		postgres.Credentials{
+			User:     "iam_creator",
+			Password: "iam_creator",
+		}, postgres.Credentials{
+			Name: name,
+			User: name,
+		}, managerRole)
+	if err != nil {
+		t.Fatalf("EnsurePostgreSQLDatabase failed: %v", err)
+	}
+
+	assert.False(t, roleCanLogin(t, db, name))
+
+	newDB, err := postgres.Connect(log, postgres.ConnectionString{
+		Host:     postgresqlHost,
+		Database: name,
+		User:     "iam_creator",
+		Password: "iam_creator",
+	})
+	if err != nil {
+		t.Fatalf("connect to database failed: %v", err)
+	}
+
+	// Validate Schema
+	schemas := storedSchema(t, newDB, name)
+	assert.Equal(t, []string{name}, schemas, "schema not as expected")
+
+	// Validate iam_creator not able to see schema
+	schemas = storedSchema(t, db, name)
+	assert.Equal(t, []string(nil), schemas, "schema not as expected")
+
+	// Validate owner of database
+	owners := validateOwner(t, db, name)
+	t.Logf("Owners of database: %v", owners)
+	assert.Equal(t, []string{name}, owners, "owner not as expected")
+}
+
+func TestDatabase_switchFromLoginToNoLoginAndBack(t *testing.T) {
+	postgresqlHost := test.Integration(t)
+	log := test.SetLogger(t)
+	managerRole := "postgres_role_name"
+	db, err := postgres.Connect(log, postgres.ConnectionString{
+		Host:     postgresqlHost,
+		Database: "postgres",
+		User:     "iam_creator",
+		Password: "iam_creator",
+	})
+	if err != nil {
+		t.Fatalf("connect to database failed: %v", err)
+	}
+
+	err = createManagerRole(log, db, managerRole)
+	if err != nil {
+		t.Fatalf("create managerRole: %v", err)
+	}
+	defer db.Close()
+
+	name := fmt.Sprintf("test_%d", time.Now().UnixNano())
+	password := "test"
+
+	err = postgres.Database(log, postgresqlHost, postgres.Credentials{
+		User:     "iam_creator",
+		Password: "iam_creator",
+	}, postgres.Credentials{
+		Name:     name,
+		User:     name,
+		Password: password,
+	}, managerRole)
+	if err != nil {
+		t.Fatalf("Database failed: %v", err)
+	}
+
+	assert.True(t, roleCanLogin(t, db, name))
+
+	// Invoke again with same name but no password
+	err = postgres.Database(log, postgresqlHost, postgres.Credentials{
+		User:     "iam_creator",
+		Password: "iam_creator",
+	}, postgres.Credentials{
+		Name: name,
+		User: name,
+	}, managerRole)
+	if err != nil {
+		t.Logf("The error: %#v", err)
+		t.Fatalf("Second Database failed: %v", err)
+	}
+	assert.False(t, roleCanLogin(t, db, name))
+
+	// Invoke again with same name with password
+	err = postgres.Database(log, postgresqlHost, postgres.Credentials{
+		User:     "iam_creator",
+		Password: "iam_creator",
+	}, postgres.Credentials{
+		Name:     name,
+		User:     name,
+		Password: password,
+	}, managerRole)
+	if err != nil {
+		t.Logf("The error: %#v", err)
+		t.Fatalf("Second Database failed: %v", err)
+	}
+	assert.True(t, roleCanLogin(t, db, name))
+
+	newDB, err := postgres.Connect(log, postgres.ConnectionString{
+		Host:     postgresqlHost,
+		Database: name,
+		User:     "iam_creator",
+		Password: "iam_creator",
+	})
+	if err != nil {
+		t.Fatalf("connect to database failed: %v", err)
+	}
+
+	// Validate Schema
+	schemas := storedSchema(t, newDB, name)
+	assert.Equal(t, []string{name}, schemas, "schema not as expected")
+
+	// Validate iam_creator not able to see schema
+	schemas = storedSchema(t, db, name)
+	assert.Equal(t, []string(nil), schemas, "schema not as expected")
+
+	// Validate owner of database
+	owners := validateOwner(t, db, name)
+	t.Logf("Owners of database: %v", owners)
+	assert.Equal(t, []string{name}, owners, "owner not as expected")
+}
+
 // TestDatabase_existingResourcePrivilegesForReadWriteRoles tests that we can
 // gain access to resources created prior to the read and readwrite roles by a
 // service role.
@@ -483,6 +634,18 @@ func TestDatabase_idempotency(t *testing.T) {
 	}
 }
 
+func roleCanLogin(t *testing.T, db *sql.DB, role string) bool {
+	t.Helper()
+	row := db.QueryRow("SELECT rolcanlogin FROM pg_roles WHERE rolname = $1", role)
+
+	var rolcanlogin bool
+	err := row.Scan(&rolcanlogin)
+	if err != nil {
+		t.Fatalf("get rolcanlogin failed: %v", err)
+	}
+	return rolcanlogin
+}
+
 func validateOwner(t *testing.T, db *sql.DB, owner string) []string {
 	t.Helper()
 	rows, err := db.Query("SELECT pg_catalog.pg_get_userbyid(d.datdba) FROM pg_catalog.pg_database d WHERE d.datname = $1", owner)