Skip to content

Commit 10c9407

Browse files
KeithChKeithCh
committed
changefeedccl: support ALTER DEFAULT PRIVILEGES for databases
This will allow users to be granted privileges for all databases. The intent of this commit is to allow grants on changefeeds specifically i.e. ALTER DEFAULT PRIVILEGES GRANT CHANGEFEED ON DATABASES to foo; but I don't know how to do that without allowing other database privileges to be granted on all databases. Resolves: #152196 Release note: Users can now perform ALTER DEFAULT PRIVILEGES on DATABASES
1 parent 0163245 commit 10c9407

File tree

11 files changed

+456
-4
lines changed

11 files changed

+456
-4
lines changed

‎docs/generated/sql/bnf/alter_default_privileges_stmt.bnf

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
alter_default_privileges_stmt ::=
2-
'ALTER' 'DEFAULT' 'PRIVILEGES' ( 'FOR' ( 'ROLE' | 'USER' ) role_spec_list | ) ( 'IN' 'SCHEMA' ( ( qualifiable_schema_name ) ( ( ',' qualifiable_schema_name ) )* ) | ) ( 'GRANT' privileges 'ON' ( 'TABLES' | 'SEQUENCES' | 'TYPES' | 'SCHEMAS' | 'FUNCTIONS' | 'ROUTINES' ) 'TO' role_spec_list ( 'WITH' 'GRANT' 'OPTION' | ) )
3-
| 'ALTER' 'DEFAULT' 'PRIVILEGES' ( 'FOR' ( 'ROLE' | 'USER' ) role_spec_list | ) ( 'IN' 'SCHEMA' ( ( qualifiable_schema_name ) ( ( ',' qualifiable_schema_name ) )* ) | ) ( 'REVOKE' privileges 'ON' ( 'TABLES' | 'SEQUENCES' | 'TYPES' | 'SCHEMAS' | 'FUNCTIONS' | 'ROUTINES' ) 'FROM' role_spec_list ( 'CASCADE' | 'RESTRICT' | ) | 'REVOKE' 'GRANT' 'OPTION' 'FOR' privileges 'ON' ( 'TABLES' | 'SEQUENCES' | 'TYPES' | 'SCHEMAS' | 'FUNCTIONS' | 'ROUTINES' ) 'FROM' role_spec_list ( 'CASCADE' | 'RESTRICT' | ) )
4-
| 'ALTER' 'DEFAULT' 'PRIVILEGES' 'FOR' 'ALL' 'ROLES' ( 'IN' 'SCHEMA' ( ( qualifiable_schema_name ) ( ( ',' qualifiable_schema_name ) )* ) | ) ( 'GRANT' privileges 'ON' ( 'TABLES' | 'SEQUENCES' | 'TYPES' | 'SCHEMAS' | 'FUNCTIONS' | 'ROUTINES' ) 'TO' role_spec_list ( 'WITH' 'GRANT' 'OPTION' | ) )
5-
| 'ALTER' 'DEFAULT' 'PRIVILEGES' 'FOR' 'ALL' 'ROLES' ( 'IN' 'SCHEMA' ( ( qualifiable_schema_name ) ( ( ',' qualifiable_schema_name ) )* ) | ) ( 'REVOKE' privileges 'ON' ( 'TABLES' | 'SEQUENCES' | 'TYPES' | 'SCHEMAS' | 'FUNCTIONS' | 'ROUTINES' ) 'FROM' role_spec_list ( 'CASCADE' | 'RESTRICT' | ) | 'REVOKE' 'GRANT' 'OPTION' 'FOR' privileges 'ON' ( 'TABLES' | 'SEQUENCES' | 'TYPES' | 'SCHEMAS' | 'FUNCTIONS' | 'ROUTINES' ) 'FROM' role_spec_list ( 'CASCADE' | 'RESTRICT' | ) )
2+
'ALTER' 'DEFAULT' 'PRIVILEGES' ( 'FOR' ( 'ROLE' | 'USER' ) role_spec_list | ) ( 'IN' 'SCHEMA' ( ( qualifiable_schema_name ) ( ( ',' qualifiable_schema_name ) )* ) | ) ( 'GRANT' privileges 'ON' ( 'TABLES' | 'SEQUENCES' | 'TYPES' | 'SCHEMAS' | 'FUNCTIONS' | 'ROUTINES' | 'DATABASES' ) 'TO' role_spec_list ( 'WITH' 'GRANT' 'OPTION' | ) )
3+
| 'ALTER' 'DEFAULT' 'PRIVILEGES' ( 'FOR' ( 'ROLE' | 'USER' ) role_spec_list | ) ( 'IN' 'SCHEMA' ( ( qualifiable_schema_name ) ( ( ',' qualifiable_schema_name ) )* ) | ) ( 'REVOKE' privileges 'ON' ( 'TABLES' | 'SEQUENCES' | 'TYPES' | 'SCHEMAS' | 'FUNCTIONS' | 'ROUTINES' | 'DATABASES' ) 'FROM' role_spec_list ( 'CASCADE' | 'RESTRICT' | ) | 'REVOKE' 'GRANT' 'OPTION' 'FOR' privileges 'ON' ( 'TABLES' | 'SEQUENCES' | 'TYPES' | 'SCHEMAS' | 'FUNCTIONS' | 'ROUTINES' | 'DATABASES' ) 'FROM' role_spec_list ( 'CASCADE' | 'RESTRICT' | ) )
4+
| 'ALTER' 'DEFAULT' 'PRIVILEGES' 'FOR' 'ALL' 'ROLES' ( 'IN' 'SCHEMA' ( ( qualifiable_schema_name ) ( ( ',' qualifiable_schema_name ) )* ) | ) ( 'GRANT' privileges 'ON' ( 'TABLES' | 'SEQUENCES' | 'TYPES' | 'SCHEMAS' | 'FUNCTIONS' | 'ROUTINES' | 'DATABASES' ) 'TO' role_spec_list ( 'WITH' 'GRANT' 'OPTION' | ) )
5+
| 'ALTER' 'DEFAULT' 'PRIVILEGES' 'FOR' 'ALL' 'ROLES' ( 'IN' 'SCHEMA' ( ( qualifiable_schema_name ) ( ( ',' qualifiable_schema_name ) )* ) | ) ( 'REVOKE' privileges 'ON' ( 'TABLES' | 'SEQUENCES' | 'TYPES' | 'SCHEMAS' | 'FUNCTIONS' | 'ROUTINES' | 'DATABASES' ) 'FROM' role_spec_list ( 'CASCADE' | 'RESTRICT' | ) | 'REVOKE' 'GRANT' 'OPTION' 'FOR' privileges 'ON' ( 'TABLES' | 'SEQUENCES' | 'TYPES' | 'SCHEMAS' | 'FUNCTIONS' | 'ROUTINES' | 'DATABASES' ) 'FROM' role_spec_list ( 'CASCADE' | 'RESTRICT' | ) )

‎docs/generated/sql/bnf/stmt_block.bnf

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3262,6 +3262,7 @@ target_object_type ::=
32623262
| 'SCHEMAS'
32633263
| 'FUNCTIONS'
32643264
| 'ROUTINES'
3265+
| 'DATABASES'
32653266

32663267
alter_changefeed_cmd ::=
32673268
'ADD' changefeed_table_targets opt_with_options

‎pkg/sql/alter_default_privileges.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ var targetObjectToPrivilegeObject = map[privilege.TargetObjectType]privilege.Obj
3131
privilege.Types: privilege.Type,
3232
privilege.Schemas: privilege.Schema,
3333
privilege.Routines: privilege.Routine,
34+
privilege.Databases: privilege.Database,
3435
}
3536

3637
type alterDefaultPrivilegesNode struct {

‎pkg/sql/catalog/catpb/privilege.proto

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ message PrivilegeDescriptor {
4545
// 2: Sequences
4646
// 3: Types
4747
// 4: Schemas
48+
// 5: Databases
4849
// DefaultPrivilegesPerObject are keyed on AlterDefaultPrivilegesTargetObject
4950
// and it's value is a PrivilegeDescriptor that is only used for
5051
// the list of UserPrivileges for that object.
@@ -71,6 +72,7 @@ message DefaultPrivilegesForRole {
7172
optional bool role_has_all_privileges_on_types = 8 [(gogoproto.nullable) = false];
7273
optional bool role_has_all_privileges_on_functions = 9 [(gogoproto.nullable) = false];
7374
optional bool public_has_execute_on_functions = 10 [(gogoproto.nullable) = false];
75+
optional bool role_has_all_privileges_on_databases = 11 [(gogoproto.nullable) = false];
7476
}
7577
// ForAllRoles represents when default privileges are defined
7678
// using FOR ALL ROLES.

‎pkg/sql/catalog/catprivilege/default_privilege.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,8 @@ func GetRoleHasAllPrivilegesOnTargetObject(
460460
return defaultPrivilegesForRole.GetExplicitRole().RoleHasAllPrivilegesOnSchemas
461461
case privilege.Routines:
462462
return defaultPrivilegesForRole.GetExplicitRole().RoleHasAllPrivilegesOnFunctions
463+
case privilege.Databases:
464+
return defaultPrivilegesForRole.GetExplicitRole().RoleHasAllPrivilegesOnDatabases
463465
default:
464466
panic(fmt.Sprintf("unknown target object %s", targetObject))
465467
}
@@ -568,6 +570,8 @@ func setRoleHasAllOnTargetObject(
568570
defaultPrivilegesForRole.GetExplicitRole().RoleHasAllPrivilegesOnSchemas = roleHasAll
569571
case privilege.Routines:
570572
defaultPrivilegesForRole.GetExplicitRole().RoleHasAllPrivilegesOnFunctions = roleHasAll
573+
case privilege.Databases:
574+
defaultPrivilegesForRole.GetExplicitRole().RoleHasAllPrivilegesOnDatabases = roleHasAll
571575
default:
572576
panic(fmt.Sprintf("unknown target object %s", targetObject))
573577
}

‎pkg/sql/catalog/catprivilege/default_privilege_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,13 @@ func TestGrantDefaultPrivileges(t *testing.T) {
154154
targetObject: privilege.Schemas,
155155
objectCreator: creatorUser,
156156
},
157+
{
158+
defaultPrivilegesRole: catpb.DefaultPrivilegesRole{Role: creatorUser},
159+
privileges: privilege.List{privilege.CHANGEFEED},
160+
grantees: []username.SQLUsername{fooUser, barUser, bazUser},
161+
targetObject: privilege.Databases,
162+
objectCreator: creatorUser,
163+
},
157164
}
158165

159166
for _, tc := range testCases {
@@ -270,6 +277,19 @@ func TestRevokeDefaultPrivileges(t *testing.T) {
270277
targetObject: privilege.Schemas,
271278
objectCreator: creatorUser,
272279
},
280+
{
281+
defaultPrivilegesRole: catpb.DefaultPrivilegesRole{ForAllRoles: true},
282+
grantPrivileges: privilege.List{privilege.ALL},
283+
revokePrivileges: privilege.List{privilege.CHANGEFEED},
284+
expectedPrivileges: privilege.List{
285+
privilege.ALL, privilege.BACKUP, privilege.CONNECT,
286+
privilege.CREATE, privilege.DROP, privilege.RESTORE,
287+
privilege.ZONECONFIG, privilege.INSPECT,
288+
},
289+
grantees: []username.SQLUsername{fooUser, barUser, bazUser},
290+
targetObject: privilege.Databases,
291+
objectCreator: creatorUser,
292+
},
273293
}
274294

275295
for _, tc := range testCases {
@@ -677,6 +697,60 @@ func TestDefaultPrivileges(t *testing.T) {
677697
},
678698
},
679699
},
700+
{
701+
objectCreator: username.MakeSQLUsernameFromPreNormalizedString("creator"),
702+
defaultPrivilegesRole: username.MakeSQLUsernameFromPreNormalizedString("creator"),
703+
targetObject: privilege.Databases,
704+
dbID: defaultDatabaseID,
705+
userAndGrants: []userAndGrants{
706+
{
707+
user: username.MakeSQLUsernameFromPreNormalizedString("foo"),
708+
grants: privilege.List{privilege.ALL},
709+
},
710+
},
711+
expectedGrantsOnObject: []userAndGrants{
712+
{
713+
user: username.RootUserName(),
714+
grants: privilege.List{privilege.ALL},
715+
},
716+
{
717+
user: username.AdminRoleName(),
718+
grants: privilege.List{privilege.ALL},
719+
},
720+
{
721+
user: username.MakeSQLUsernameFromPreNormalizedString("foo"),
722+
// Should be the union of the default privileges on the db and schema.
723+
grants: privilege.List{privilege.ALL},
724+
},
725+
},
726+
},
727+
{
728+
objectCreator: username.MakeSQLUsernameFromPreNormalizedString("creator"),
729+
defaultPrivilegesRole: username.MakeSQLUsernameFromPreNormalizedString("creator"),
730+
targetObject: privilege.Databases,
731+
dbID: defaultDatabaseID,
732+
userAndGrants: []userAndGrants{
733+
{
734+
user: username.MakeSQLUsernameFromPreNormalizedString("foo"),
735+
grants: privilege.List{privilege.CHANGEFEED},
736+
},
737+
},
738+
expectedGrantsOnObject: []userAndGrants{
739+
{
740+
user: username.RootUserName(),
741+
grants: privilege.List{privilege.ALL},
742+
},
743+
{
744+
user: username.AdminRoleName(),
745+
grants: privilege.List{privilege.ALL},
746+
},
747+
{
748+
user: username.MakeSQLUsernameFromPreNormalizedString("foo"),
749+
// Should be the union of the default privileges on the db and schema.
750+
grants: privilege.List{privilege.CHANGEFEED},
751+
},
752+
},
753+
},
680754
}
681755
for _, tc := range testCases {
682756
defaultPrivilegeDescriptor := MakeDefaultPrivilegeDescriptor(catpb.DefaultPrivilegeDescriptor_DATABASE)
@@ -752,6 +826,10 @@ func TestModifyDefaultDefaultPrivileges(t *testing.T) {
752826
targetObject: privilege.Schemas,
753827
revokeAndGrantPrivileges: privilege.List{privilege.USAGE},
754828
},
829+
{
830+
targetObject: privilege.Schemas,
831+
revokeAndGrantPrivileges: privilege.List{privilege.CHANGEFEED},
832+
},
755833
}
756834

757835
for _, tc := range testCases {
@@ -932,6 +1010,12 @@ func TestApplyDefaultPrivileges(t *testing.T) {
9321010
privilege.List{privilege.SELECT, privilege.INSERT},
9331011
privilege.List{privilege.CREATE},
9341012
privilege.List{privilege.CREATE}},
1013+
{catpb.NewPrivilegeDescriptor(testUser, privilege.List{privilege.CREATE}, privilege.List{privilege.CREATE}, username.AdminRoleName()),
1014+
testUser, privilege.Database,
1015+
privilege.List{privilege.CHANGEFEED},
1016+
privilege.List{privilege.CHANGEFEED},
1017+
privilege.List{privilege.CREATE, privilege.CHANGEFEED},
1018+
privilege.List{privilege.CREATE, privilege.CHANGEFEED}},
9351019
}
9361020

9371021
for tcNum, tc := range testCases {

0 commit comments

Comments
 (0)