Skip to content

Commit 53de337

Browse files
committed
sql: avoid dangling role reference in default privileges
Release note (bug fix): Fixed a bug that could leave behind a dangling reference to a dropped role if that role had default privileges granted to itself. The bug was caused by defining privileges such as: `ALTER DEFAULT PRIVILEGES FOR ROLE self_referencing_role GRANT INSERT ON TABLES TO self_referencing_role`
1 parent 1c8ef74 commit 53de337

File tree

2 files changed

+32
-6
lines changed

2 files changed

+32
-6
lines changed

pkg/sql/catalog/catprivilege/default_privilege.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ func (d *Mutable) GrantDefaultPrivileges(
118118
return err
119119
}
120120
}
121+
d.removeRoleIfEmptyPrivileges(role, defaultPrivilegesForRole)
121122
return nil
122123
}
123124

@@ -135,14 +136,20 @@ func (d *Mutable) RevokeDefaultPrivileges(
135136
return err
136137
}
137138
}
139+
d.removeRoleIfEmptyPrivileges(role, defaultPrivilegesForRole)
140+
return nil
141+
}
138142

143+
// removeRoleIfEmptyPrivilegesc checks if there are any default privileges for
144+
// the given role remaining on the descriptor. If there are no privileges left
145+
// remaining and the descriptor is in the default state, the role is removed.
146+
func (d *Mutable) removeRoleIfEmptyPrivileges(
147+
role catpb.DefaultPrivilegesRole, defaultPrivilegesForRole *catpb.DefaultPrivilegesForRole,
148+
) {
139149
defaultPrivilegesPerObject := defaultPrivilegesForRole.DefaultPrivilegesPerObject
140-
// Check if there are any default privileges remaining on the descriptor.
141-
// If there are no privileges left remaining and the descriptor is in the
142-
// default state, we can remove it.
143150
for _, defaultPrivs := range defaultPrivilegesPerObject {
144151
if len(defaultPrivs.Users) != 0 {
145-
return nil
152+
return
146153
}
147154
}
148155

@@ -155,12 +162,11 @@ func (d *Mutable) RevokeDefaultPrivileges(
155162
!GetRoleHasAllPrivilegesOnTargetObject(defaultPrivilegesForRole, privilege.Schemas) ||
156163
!GetPublicHasUsageOnTypes(defaultPrivilegesForRole) ||
157164
!GetPublicHasExecuteOnFunctions(defaultPrivilegesForRole)) {
158-
return nil
165+
return
159166
}
160167

161168
// There no entries remaining, remove the entry for the role.
162169
d.defaultPrivilegeDescriptor.RemoveUser(role)
163-
return nil
164170
}
165171

166172
// CreatePrivilegesFromDefaultPrivileges creates privileges for a

pkg/sql/logictest/testdata/logic_test/drop_role_with_default_privileges

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,3 +183,23 @@ statement ok
183183
DROP USER IF EXISTS a_user_that_does_not_exist;
184184

185185
subtest end
186+
187+
# Verify that a granting default privileges to the same role the defaults are
188+
# being defined for has no undesirable side effect.
189+
subtest default_priv_granted_to_self
190+
191+
statement ok
192+
CREATE ROLE self_referencing_role
193+
194+
statement ok
195+
ALTER DEFAULT PRIVILEGES FOR ROLE self_referencing_role GRANT INSERT ON TABLES TO self_referencing_role
196+
197+
statement ok
198+
DROP ROLE self_referencing_role
199+
200+
query I
201+
SELECT count(*) FROM crdb_internal.invalid_objects
202+
----
203+
0
204+
205+
subtest end

0 commit comments

Comments
 (0)