From 7859cd14b64cbb93b2721daf35a196a1fafb37f0 Mon Sep 17 00:00:00 2001 From: richardjcai Date: Tue, 2 Nov 2021 16:01:30 +0000 Subject: [PATCH] sql: fix excess privileges being created from default privileges. Release note (bug fix): Previously, when creating an object default privileges from users that were not the user creating the object would be added to the privileges of the object. This fix ensures only the relevant default privileges are applied. --- .../catalog/catprivilege/default_privilege.go | 23 +++++++++++++------ .../alter_default_privileges_for_schema | 17 ++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/pkg/sql/catalog/catprivilege/default_privilege.go b/pkg/sql/catalog/catprivilege/default_privilege.go index a75ce64c7b0f..a2ba632600ab 100644 --- a/pkg/sql/catalog/catprivilege/default_privilege.go +++ b/pkg/sql/catalog/catprivilege/default_privilege.go @@ -145,10 +145,10 @@ func (d *immutable) CreatePrivilegesFromDefaultPrivileges( } newPrivs := descpb.NewDefaultPrivilegeDescriptor(user) - // If default privileges are not defined for the creator role, we handle - // it as the default case where the user has all privileges. role := descpb.DefaultPrivilegesRole{Role: user} - if _, found := d.GetDefaultPrivilegesForRole(role); !found { + if defaultPrivilegesForRole, found := d.GetDefaultPrivilegesForRole(role); !found { + // If default privileges are not defined for the creator role, we handle + // it as the case where the user has all privileges. defaultPrivilegesForCreatorRole := descpb.InitDefaultPrivilegesForRole(role) for _, user := range GetUserPrivilegesForObject(defaultPrivilegesForCreatorRole, targetObject) { newPrivs.Grant( @@ -156,20 +156,29 @@ func (d *immutable) CreatePrivilegesFromDefaultPrivileges( privilege.ListFromBitField(user.Privileges, targetObject.ToPrivilegeObjectType()), ) } + } else { + // If default privileges were defined for the role, we create privileges + // using the default privileges. + for _, user := range GetUserPrivilegesForObject(*defaultPrivilegesForRole, targetObject) { + newPrivs.Grant( + user.UserProto.Decode(), + privilege.ListFromBitField(user.Privileges, targetObject.ToPrivilegeObjectType()), + ) + } } // The privileges for the object are the union of the default privileges // defined for the object for the object creator and the default privileges // defined for all roles. - _ = d.ForEachDefaultPrivilegeForRole(func(defaultPrivilegesForRole descpb.DefaultPrivilegesForRole) error { - for _, user := range GetUserPrivilegesForObject(defaultPrivilegesForRole, targetObject) { + defaultPrivilegesForAllRoles, found := d.GetDefaultPrivilegesForRole(descpb.DefaultPrivilegesRole{ForAllRoles: true}) + if found { + for _, user := range GetUserPrivilegesForObject(*defaultPrivilegesForAllRoles, targetObject) { newPrivs.Grant( user.UserProto.Decode(), privilege.ListFromBitField(user.Privileges, targetObject.ToPrivilegeObjectType()), ) } - return nil - }) + } newPrivs.SetOwner(user) newPrivs.Version = descpb.Version21_2 diff --git a/pkg/sql/logictest/testdata/logic_test/alter_default_privileges_for_schema b/pkg/sql/logictest/testdata/logic_test/alter_default_privileges_for_schema index 79224be816af..c6bd6ad5ac2d 100644 --- a/pkg/sql/logictest/testdata/logic_test/alter_default_privileges_for_schema +++ b/pkg/sql/logictest/testdata/logic_test/alter_default_privileges_for_schema @@ -137,3 +137,20 @@ database_name schema_name grantee privilege_type d s5 admin ALL d s5 root ALL d s5 testuser CREATE + +statement ok +ALTER DEFAULT PRIVILEGES GRANT ALL ON SCHEMAS TO testuser, testuser2 + +user root + +statement ok +CREATE SCHEMA s_72322 + +# When root creates the table, testuser and testuser2 should not get privileges. +query TTTT colnames +SHOW GRANTS ON SCHEMA s_72322 +---- +database_name schema_name grantee privilege_type +d s_72322 admin ALL +d s_72322 root ALL +d s_72322 testuser CREATE