Merge #130547

130547:  sql: allow DB owner to set session var defaults in database r=rafiss a=kaustubhbabar5

Fixes: #123287

Release note (sql change): The owner of a database can now set default
session variables per database using the `ALTER ROLE ALL IN DATABASE
... SET` or `ALTER DATABASE ... SET` commands.

Co-authored-by: Kaustubh Babar <[email protected]>

Author: craig[bot]

pkg/sql/alter_role.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/settings"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/decodeusername"
 	"github.com/cockroachdb/cockroach/pkg/sql/paramparse"
@@ -288,8 +289,23 @@ func (*alterRoleNode) Values() tree.Datums          { return tree.Datums{} }
 func (*alterRoleNode) Close(context.Context)        {}
 
 // AlterRoleSet represents a `ALTER ROLE ... SET` statement.
-// Privileges: CREATEROLE, MODIFYCLUSTERSETTING, MODIFYSQLCLUSTERSETTING privilege; or admin-only if `ALTER ROLE ALL`.
+//
+// Privileges:
+//
+// - CREATEROLE, MODIFYCLUSTERSETTING, MODIFYSQLCLUSTERSETTING privilege;
+// - admin-only if `ALTER ROLE ALL`;
+// - database owner or admin if `ALTER ROLE ALL IN DATABASE SET ...`
 func (p *planner) AlterRoleSet(ctx context.Context, n *tree.AlterRoleSet) (planNode, error) {
+	dbDescID := descpb.ID(0)
+	var dbDesc catalog.DatabaseDescriptor
+	if n.DatabaseName != "" {
+		var err error
+		dbDesc, err = p.Descriptors().ByNameWithLeased(p.txn).Get().Database(ctx, string(n.DatabaseName))
+		if err != nil {
+			return nil, err
+		}
+		dbDescID = dbDesc.GetID()
+	}
 	// Note that for Postgres, only superuser can ALTER another superuser.
 	// CockroachDB does not support the superuser role option right now.
 	// However we make it so members of the ADMIN role can only be edited
@@ -298,11 +314,25 @@ func (p *planner) AlterRoleSet(ctx context.Context, n *tree.AlterRoleSet) (planN
 	// modifying their own defaults unless they have CREATEROLE. This is analogous
 	// to our restriction that prevents a user from modifying their own password.
 	if n.AllRoles {
-		if hasAdmin, err := p.HasAdminRole(ctx); err != nil {
+		hasAdmin, err := p.HasAdminRole(ctx)
+		if err != nil {
 			return nil, err
-		} else if !hasAdmin {
-			return nil, pgerror.Newf(pgcode.InsufficientPrivilege,
-				"only users with the admin role are allowed to ALTER ROLE ALL ... SET")
+		}
+		if !hasAdmin {
+			if dbDesc == nil {
+				return nil, pgerror.Newf(pgcode.InsufficientPrivilege,
+					"only users with the admin role are allowed to ALTER ROLE ALL ... SET")
+			}
+			// If the user is not an admin, they must be the owner of the database if
+			// ALL IN DATABASE ... is used
+			isDBOwner, err := p.HasOwnership(ctx, dbDesc)
+			if err != nil {
+				return nil, err
+			}
+			if !isDBOwner {
+				return nil, pgerror.Newf(pgcode.InsufficientPrivilege,
+					"only users with the admin role or the owner of the database is allowed to ALTER ROLE ALL IN DATABASE SET ...")
+			}
 		}
 	} else {
 		canAlterRoleSet, err := p.HasGlobalPrivilegeOrRoleOption(ctx, privilege.CREATEROLE)
@@ -337,15 +367,6 @@ func (p *planner) AlterRoleSet(ctx context.Context, n *tree.AlterRoleSet) (planN
 		}
 	}
 
-	dbDescID := descpb.ID(0)
-	if n.DatabaseName != "" {
-		dbDesc, err := p.Descriptors().ByNameWithLeased(p.txn).Get().Database(ctx, string(n.DatabaseName))
-		if err != nil {
-			return nil, err
-		}
-		dbDescID = dbDesc.GetID()
-	}
-
 	setVarKind, varName, sVar, typedValues, err := p.processSetOrResetClause(ctx, n.SetOrReset)
 	if err != nil {
 		return nil, err

pkg/sql/logictest/testdata/logic_test/alter_role_set

@@ -214,7 +214,7 @@ statement ok
 ALTER ROLE test_set_role RESET application_name
 
 # However, even with CREATEROLE, testuser cannot do RESET ALL.
-statement error only users with the admin role are allowed to ALTER ROLE ALL
+statement error only users with the admin role or the owner of the database is allowed to ALTER ROLE ALL IN DATABASE SET ..
 ALTER ROLE ALL IN DATABASE test_set_db RESET application_name
 
 # Verify that testuser can't edit an ADMIN, even after getting CREATEROLE.
@@ -374,3 +374,60 @@ use_declarative_schema_changer  off                NULL     true
 # We shouldn't be able to leave the role unspecified
 statement error pq: at or near "]": syntax error
 SELECT * FROM [SHOW DEFAULT SESSION VARIABLES FOR ROLE]
+
+statement ok
+CREATE USER testuser2
+
+# Set owner to a user that is not admin.
+statement ok
+ALTER DATABASE test_db OWNER TO testuser2
+
+user testuser2
+
+# Verify that owner can set default session variables for a database.
+statement ok
+ALTER ROLE ALL IN DATABASE test_db SET application_name = 'abc'
+
+user root
+
+query TTT
+SELECT * FROM [SHOW DEFAULT SESSION VARIABLES FOR ROLE ALL] WHERE database='test_db' ORDER BY 1, 2
+----
+application_name    abc     test_db
+
+user testuser2
+
+# Verify that DB owner can reset default session variables for a database.
+statement ok
+ALTER ROLE ALL IN DATABASE test_db RESET application_name
+
+user root
+
+query TTT colnames
+SELECT * FROM [SHOW DEFAULT SESSION VARIABLES FOR ROLE ALL] WHERE database='test_db' ORDER BY 1, 2
+----
+session_variables  default_values  database
+
+statement ok
+CREATE DATABASE test_db2
+
+user testuser2
+
+# Verify that DB owner cannot set default session variables for a database that they do not own.
+statement error only users with the admin role or the owner of the database is allowed to ALTER ROLE ALL IN DATABASE SET
+ALTER ROLE ALL IN DATABASE test_db2 SET application_name = 'abc'
+
+user root
+
+statement ok
+DROP DATABASE test_db2
+
+user testuser2
+
+# Verify that DB owner cannot alter a specific role.
+statement error pq: ALTER ROLE ... SET requires MODIFYCLUSTERSETTING, MODIFYSQLCLUSTERSETTING or CREATEROLE
+ALTER ROLE roach IN DATABASE test_db SET application_name = 'abc'
+
+# Verify that DB owner cannot ALTER ROLE ALL.
+statement error pq: only users with the admin role are allowed to ALTER ROLE ALL ... SET
+ALTER ROLE ALL SET application_name = 'abc'