From fe689965b2b93c7f96f02cd86025bc72dfec8e10 Mon Sep 17 00:00:00 2001
From: Faizan Qazi <faizan@cockroachlabs.com>
Date: Thu, 23 May 2024 14:34:59 +0000
Subject: [PATCH] sql: drop user/role did not detect usage by types

Previously, it was possible to drop a user / role in use by a type
because the detection logic only checked ownership. To address this,
this patch will check if the user exists within any types before
allowing the drop to go through.

Note: An automated repair during upgrade exists to fix descriptors with
this issue, since they can break SHOW GRANTS. The same repair query can
also be invoked manually.

Fixes: #124441

Release note (bug fix): Drop role/user could leave references behind
inside types, which could prevent SHOW GRANTS from working
---
 pkg/sql/drop_role.go                       | 13 +++++++++++
 pkg/sql/logictest/testdata/logic_test/role | 26 ++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/pkg/sql/drop_role.go b/pkg/sql/drop_role.go
index f1145825fc78..ac7e93eada31 100644
--- a/pkg/sql/drop_role.go
+++ b/pkg/sql/drop_role.go
@@ -249,6 +249,19 @@ func (n *DropRoleNode) startExec(params runParams) error {
 					ObjectName: tn.String(),
 				})
 		}
+		for _, u := range typDesc.GetPrivileges().Users {
+			if _, ok := userNames[u.User()]; ok {
+				tn, err := getTypeNameFromTypeDescriptor(lCtx, typDesc)
+				if err != nil {
+					return err
+				}
+				if privilegeObjectFormatter.Len() > 0 {
+					privilegeObjectFormatter.WriteString(", ")
+				}
+				privilegeObjectFormatter.FormatNode(&tn)
+				break
+			}
+		}
 	}
 	for _, fnDesc := range lCtx.fnDescs {
 		if _, ok := userNames[fnDesc.GetPrivileges().Owner()]; ok {
diff --git a/pkg/sql/logictest/testdata/logic_test/role b/pkg/sql/logictest/testdata/logic_test/role
index c596ccdc83da..58f2b026e375 100644
--- a/pkg/sql/logictest/testdata/logic_test/role
+++ b/pkg/sql/logictest/testdata/logic_test/role
@@ -1881,3 +1881,29 @@ statement error SUBJECT role option is only supported after v24.1 upgrade is fin
 ALTER ROLE testuser SUBJECT 'foo'
 
 subtest end
+
+# Validates that drop role will be prevented if the role is in use by
+# any schema objects. This is validates and prevents the regression found
+# in #124441
+subtest drop_role_block_validation
+
+statement ok
+CREATE DATABASE block_db;
+USE block_db;
+CREATE TABLE t(n int);
+CREATE TYPE  typ AS ENUM ('open', 'closed', 'inactive');
+CREATE FUNCTION f() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$;
+CREATE ROLE block_user;
+GRANT ALL ON DATABASE block_db to block_user;
+GRANT ALL ON SCHEMA public to block_user;
+GRANT ALL ON TABLE t to block_user;
+GRANT ALL ON TYPE typ to block_user;
+GRANT ALL ON FUNCTION f to block_user;
+
+statement error pgcode 2BP01 cannot drop role/user block_user: grants still exist on block_db, block_db.public.t, block_db.public, block_db.public.typ, block_db.public.f
+DROP ROLE block_user
+
+statement ok
+USE defaultdb;
+
+subtest end