Skip to content

Commit 60a9984

Browse files
craig[bot]KeithCh
andcommitted
Merge #149794
149794: changefeedccl: add privilege check for database-level changefeed r=andyyang890,aerfrei a=KeithCh Only a user with changefeed privileges on a database can create an enterprise changefeed on the database. This also allows them to create an enterprise changefeed on any table in that database. Creation of core changefeeds at the table-level will still require the select privilege on all tables being watched. resolves #149470 Release note: None Co-authored-by: Keith Chow <[email protected]>
2 parents dbdfb73 + feb8aff commit 60a9984

File tree

9 files changed

+219
-52
lines changed

9 files changed

+219
-52
lines changed

pkg/ccl/changefeedccl/alter_changefeed_stmt.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -683,7 +683,8 @@ func generateAndValidateNewTargets(
683683
hasSelectPrivOnAllTables = hasSelectPrivOnAllTables && hasSelect
684684
hasChangefeedPrivOnAllTables = hasChangefeedPrivOnAllTables && hasChangefeed
685685
}
686-
if err := authorizeUserToCreateChangefeed(ctx, p, sinkURI, hasSelectPrivOnAllTables, hasChangefeedPrivOnAllTables); err != nil {
686+
if err := authorizeUserToCreateChangefeed(
687+
ctx, p, sinkURI, hasSelectPrivOnAllTables, hasChangefeedPrivOnAllTables, tree.ChangefeedLevelTable); err != nil {
687688
return nil, nil, hlc.Timestamp{}, nil, err
688689
}
689690

pkg/ccl/changefeedccl/alter_changefeed_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1882,7 +1882,7 @@ func TestAlterChangefeedAccessControl(t *testing.T) {
18821882
})
18831883
// jobController can access the job, but will hit an error re-creating the changefeed.
18841884
asUser(t, f, `jobController`, func(userDB *sqlutils.SQLRunner) {
1885-
userDB.ExpectErr(t, "pq: user jobcontroller requires the CHANGEFEED privilege on all target tables to be able to run an enterprise changefeed", fmt.Sprintf(`ALTER CHANGEFEED %d DROP table_b`, currentFeed.JobID()))
1885+
userDB.ExpectErr(t, `pq: user "jobcontroller" requires the CHANGEFEED privilege on all target tables to be able to run an enterprise changefeed`, fmt.Sprintf(`ALTER CHANGEFEED %d DROP table_b`, currentFeed.JobID()))
18861886
})
18871887
asUser(t, f, `userWithSomeGrants`, func(userDB *sqlutils.SQLRunner) {
18881888
userDB.ExpectErr(t, "does not have privileges for job", fmt.Sprintf(`ALTER CHANGEFEED %d ADD table_b`, currentFeed.JobID()))

pkg/ccl/changefeedccl/authorization.go

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,25 @@ import (
1818
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
1919
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
2020
"github.com/cockroachdb/cockroach/pkg/sql/roleoption"
21+
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
2122
"github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege"
2223
"github.com/cockroachdb/errors"
2324
)
2425

2526
func checkPrivilegesForDescriptor(
2627
ctx context.Context, p sql.PlanHookState, desc catalog.Descriptor,
2728
) (hasSelect bool, hasChangefeed bool, err error) {
28-
if desc.GetObjectType() != privilege.Table {
29-
return false, false, errors.AssertionFailedf("expected descriptor %d to be a table descriptor, found: %s ", desc.GetID(), desc.GetObjectType())
29+
if desc == nil {
30+
return false, false, errors.AssertionFailedf("expected descriptor to be non-nil")
31+
}
32+
switch desc.GetObjectType() {
33+
case privilege.Database, privilege.Table:
34+
default:
35+
return false, false, errors.AssertionFailedf(
36+
"expected descriptor for %q to be a table or database descriptor, found: %s ",
37+
desc.GetName(),
38+
desc.GetObjectType(),
39+
)
3040
}
3141

3242
hasSelect, err = p.HasPrivilege(ctx, desc, privilege.SELECT, p.User())
@@ -60,6 +70,7 @@ func authorizeUserToCreateChangefeed(
6070
sinkURI string,
6171
hasSelectPrivOnAllTables bool,
6272
hasChangefeedPrivOnAllTables bool,
73+
changefeedLevel tree.ChangefeedLevel,
6374
otherExternalURIs ...string,
6475
) error {
6576
isAdmin, err := p.HasAdminRole(ctx)
@@ -94,10 +105,17 @@ func authorizeUserToCreateChangefeed(
94105
return nil
95106
}
96107

108+
requiredPrivilegeTarget := func() string {
109+
if changefeedLevel == tree.ChangefeedLevelDatabase {
110+
return "the target database"
111+
}
112+
return "all target tables"
113+
}()
114+
97115
if !hasChangefeedPrivOnAllTables {
98116
return pgerror.Newf(pgcode.InsufficientPrivilege,
99-
`user %s requires the %s privilege on all target tables to be able to run an enterprise changefeed`,
100-
p.User(), privilege.CHANGEFEED)
117+
`user %q requires the %s privilege on %s to be able to run an enterprise changefeed`,
118+
p.User(), privilege.CHANGEFEED, requiredPrivilegeTarget)
101119
}
102120

103121
enforceExternalConnections := changefeedbase.RequireExternalConnectionSink.Get(&p.ExecCfg().Settings.SV)
@@ -124,8 +142,8 @@ func authorizeUserToCreateChangefeed(
124142
} else {
125143
return pgerror.Newf(
126144
pgcode.InsufficientPrivilege,
127-
`the %s privilege on all tables can only be used with external connection sinks. see cluster setting %s`,
128-
privilege.CHANGEFEED, changefeedbase.RequireExternalConnectionSink.Name(),
145+
`the %s privilege on %s can only be used with external connection sinks. see cluster setting %s`,
146+
privilege.CHANGEFEED, requiredPrivilegeTarget, changefeedbase.RequireExternalConnectionSink.Name(),
129147
)
130148
}
131149
}

pkg/ccl/changefeedccl/changefeed_stmt.go

Lines changed: 117 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -484,13 +484,14 @@ func evalCursor(
484484

485485
func getTargetList(changefeedStmt *annotatedChangefeedStatement) (*tree.BackupTargetList, error) {
486486
targetList := tree.BackupTargetList{}
487-
if changefeedStmt.Level == tree.ChangefeedLevelTable {
487+
switch changefeedStmt.Level {
488+
case tree.ChangefeedLevelTable:
488489
for _, t := range changefeedStmt.TableTargets {
489490
targetList.Tables.TablePatterns = append(targetList.Tables.TablePatterns, t.TableName)
490491
}
491-
} else if changefeedStmt.Level == tree.ChangefeedLevelDatabase {
492+
case tree.ChangefeedLevelDatabase:
492493
targetList.Databases = tree.NameList{tree.Name(changefeedStmt.DatabaseTarget)}
493-
} else {
494+
default:
494495
return nil, errors.AssertionFailedf("unknown changefeed level: %s", changefeedStmt.Level.String())
495496
}
496497
return &targetList, nil
@@ -570,11 +571,23 @@ func createChangefeedJobRecord(
570571

571572
var details jobspb.ChangefeedDetails
572573

573-
tableNameToDescriptor, targetDatabaseDescs, err := getTargetDescriptors(ctx, p, targetList, statementTime, initialHighWater)
574+
tableNameToDescriptor, targetDatabaseDescs, tableAndParentDescs, err := getTargetDescriptors(
575+
ctx,
576+
p,
577+
targetList,
578+
statementTime,
579+
initialHighWater,
580+
)
574581

575582
if err != nil {
576583
return nil, changefeedbase.Targets{}, err
577584
}
585+
if len(tableAndParentDescs) == 0 {
586+
return nil, changefeedbase.Targets{}, errors.AssertionFailedf("expected at least one descriptor")
587+
}
588+
if len(targetDatabaseDescs) > 1 {
589+
return nil, changefeedbase.Targets{}, errors.AssertionFailedf("expected at most one database descriptor, got %d", len(targetDatabaseDescs))
590+
}
578591

579592
for _, t := range tableNameToDescriptor {
580593
if tbl, ok := t.(catalog.TableDescriptor); ok && tbl.ExternalRowData() != nil {
@@ -640,25 +653,65 @@ func createChangefeedJobRecord(
640653
hasSelectPrivOnAllTables := true
641654
hasChangefeedPrivOnAllTables := true
642655
tolerances := opts.GetCanHandle()
643-
for _, desc := range tableNameToDescriptor {
644-
if table, isTable := desc.(catalog.TableDescriptor); isTable {
645-
if err := changefeedvalidators.ValidateTable(targets, table, tolerances); err != nil {
646-
return nil, changefeedbase.Targets{}, err
647-
}
648-
for _, warning := range changefeedvalidators.WarningsForTable(table, tolerances) {
649-
p.BufferClientNotice(ctx, pgnotice.Newf("%s", warning))
650-
}
656+
// Core changefeed:
657+
// - Table-level changefeeds require the user to have SELECT privileges
658+
// on all target tables
659+
// - DB-level feeds are not supported
660+
// Enterprise changefeed:
661+
// - Table-level feeds require the CHANGEFEED privilege on all target tables
662+
// - DB-level feeds require the CHANGEFEED privilege on the target database
663+
switch changefeedStmt.Level {
664+
case tree.ChangefeedLevelTable:
665+
_, tableToDatabaseLookup := buildTableToDatabaseAndSchemaLookup(tableAndParentDescs)
666+
for _, desc := range tableNameToDescriptor {
667+
if table, isTable := desc.(catalog.TableDescriptor); isTable {
668+
if err := changefeedvalidators.ValidateTable(targets, table, tolerances); err != nil {
669+
return nil, changefeedbase.Targets{}, err
670+
}
671+
for _, warning := range changefeedvalidators.WarningsForTable(table, tolerances) {
672+
p.BufferClientNotice(ctx, pgnotice.Newf("%s", warning))
673+
}
651674

652-
hasSelect, hasChangefeed, err := checkPrivilegesForDescriptor(ctx, p, desc)
653-
if err != nil {
654-
return nil, changefeedbase.Targets{}, err
675+
hasSelect, hasChangefeed, err := checkPrivilegesForDescriptor(ctx, p, desc)
676+
if err != nil {
677+
return nil, changefeedbase.Targets{}, err
678+
}
679+
680+
databaseDesc, ok := tableToDatabaseLookup[table.GetID()]
681+
if !ok {
682+
return nil, changefeedbase.Targets{}, errors.AssertionFailedf("expected to find a database descriptor for table %s", table.GetName())
683+
}
684+
if !hasChangefeed {
685+
_, hasChangefeed, err = checkPrivilegesForDescriptor(ctx, p, databaseDesc)
686+
if err != nil {
687+
return nil, changefeedbase.Targets{}, err
688+
}
689+
}
690+
691+
hasSelectPrivOnAllTables = hasSelectPrivOnAllTables && hasSelect
692+
hasChangefeedPrivOnAllTables = hasChangefeedPrivOnAllTables && hasChangefeed
655693
}
656-
hasSelectPrivOnAllTables = hasSelectPrivOnAllTables && hasSelect
657-
hasChangefeedPrivOnAllTables = hasChangefeedPrivOnAllTables && hasChangefeed
658694
}
695+
case tree.ChangefeedLevelDatabase:
696+
_, hasDatabaseChangefeedPriv, err := checkPrivilegesForDescriptor(ctx, p, targetDatabaseDescs[0])
697+
if err != nil {
698+
return nil, changefeedbase.Targets{}, err
699+
}
700+
hasChangefeedPrivOnAllTables = hasDatabaseChangefeedPriv
701+
default:
702+
return nil, changefeedbase.Targets{}, errors.AssertionFailedf("unknown changefeed level: %s", changefeedStmt.Level)
659703
}
704+
660705
if checkPrivs {
661-
if err := authorizeUserToCreateChangefeed(ctx, p, sinkURI, hasSelectPrivOnAllTables, hasChangefeedPrivOnAllTables, opts.GetConfluentSchemaRegistry()); err != nil {
706+
if err := authorizeUserToCreateChangefeed(
707+
ctx,
708+
p,
709+
sinkURI,
710+
hasSelectPrivOnAllTables,
711+
hasChangefeedPrivOnAllTables,
712+
changefeedStmt.Level,
713+
opts.GetConfluentSchemaRegistry(),
714+
); err != nil {
662715
return nil, changefeedbase.Targets{}, err
663716
}
664717
}
@@ -984,22 +1037,24 @@ func getTargetDescriptors(
9841037
) (
9851038
tableNameToDescriptor map[tree.TablePattern]catalog.Descriptor,
9861039
databaseDescs []catalog.DatabaseDescriptor,
1040+
tableAndParentDescs []catalog.Descriptor,
9871041
err error,
9881042
) {
9891043
if len(targets.Databases) > 0 && len(targets.Tables.TablePatterns) > 0 {
990-
return nil, nil, errors.Errorf(`CHANGEFEED cannot target both databases and tables`)
1044+
return nil, nil, nil, errors.Errorf(`CHANGEFEED cannot target both databases and tables`)
9911045
}
1046+
9921047
for _, t := range targets.Tables.TablePatterns {
9931048
p, err := t.NormalizeTablePattern()
9941049
if err != nil {
995-
return nil, nil, err
1050+
return nil, nil, nil, err
9961051
}
9971052
if _, ok := p.(*tree.TableName); !ok {
998-
return nil, nil, errors.Errorf(`CHANGEFEED cannot target %s`, tree.AsString(t))
1053+
return nil, nil, nil, errors.Errorf(`CHANGEFEED cannot target %s`, tree.AsString(t))
9991054
}
10001055
}
1001-
1002-
_, _, targetDatabaseDescs, targetTableDescs, err := backupresolver.ResolveTargetsToDescriptors(ctx, p, statementTime, targets)
1056+
// targetTableDescs is empty if the targets are not tables, targetDatabaseDescs is empty if the targets are not databases
1057+
targetAndParentDescs, _, targetDatabaseDescs, targetTableDescs, err := backupresolver.ResolveTargetsToDescriptors(ctx, p, statementTime, targets)
10031058
if err != nil {
10041059
var m *backupresolver.MissingTableErr
10051060
if errors.As(err, &m) {
@@ -1013,7 +1068,7 @@ func getTargetDescriptors(
10131068
"do the targets exist at the specified cursor time %s?", initialHighWater)
10141069
}
10151070
}
1016-
return targetTableDescs, targetDatabaseDescs, err
1071+
return targetTableDescs, targetDatabaseDescs, targetAndParentDescs, err
10171072
}
10181073

10191074
func getTargetsAndTables(
@@ -1217,8 +1272,13 @@ func makeChangefeedDescription(
12171272
opts changefeedbase.StatementOptions,
12181273
) (string, error) {
12191274
c := &tree.CreateChangefeed{
1220-
TableTargets: changefeed.TableTargets,
1221-
Select: changefeed.Select,
1275+
Select: changefeed.Select,
1276+
Level: changefeed.Level,
1277+
}
1278+
if changefeed.Level == tree.ChangefeedLevelDatabase {
1279+
c.DatabaseTarget = changefeed.DatabaseTarget
1280+
} else {
1281+
c.TableTargets = changefeed.TableTargets
12221282
}
12231283

12241284
if sinkURI != "" {
@@ -2124,3 +2184,34 @@ func getChangefeedEventMigrator(migrateEvent bool) log.StructuredEventMigrator {
21242184
channel.CHANGEFEED,
21252185
)
21262186
}
2187+
2188+
func buildTableToDatabaseAndSchemaLookup(
2189+
targetAndParentDescs []catalog.Descriptor,
2190+
) (
2191+
tableToSchema map[descpb.ID]catalog.SchemaDescriptor,
2192+
tableToDatabase map[descpb.ID]catalog.DatabaseDescriptor,
2193+
) {
2194+
// getSchema maps schema IDs to its schema descriptor.
2195+
getSchema := make(map[descpb.ID]catalog.SchemaDescriptor)
2196+
// getDatabase maps database IDs to its database descriptor.
2197+
getDatabase := make(map[descpb.ID]catalog.DatabaseDescriptor)
2198+
for _, desc := range targetAndParentDescs {
2199+
switch desc.DescriptorType() {
2200+
case catalog.Schema:
2201+
getSchema[desc.GetID()] = desc.(catalog.SchemaDescriptor)
2202+
case catalog.Database:
2203+
getDatabase[desc.GetID()] = desc.(catalog.DatabaseDescriptor)
2204+
}
2205+
}
2206+
// tableToSchema maps table IDs to its parent schema descriptor.
2207+
tableToSchema = make(map[descpb.ID]catalog.SchemaDescriptor)
2208+
// tableToDatabase maps table IDs to its parent database descriptor.
2209+
tableToDatabase = make(map[descpb.ID]catalog.DatabaseDescriptor)
2210+
for _, desc := range targetAndParentDescs {
2211+
if desc.DescriptorType() == catalog.Table {
2212+
tableToDatabase[desc.GetID()] = getDatabase[desc.GetParentID()]
2213+
tableToSchema[desc.GetID()] = getSchema[desc.GetParentSchemaID()]
2214+
}
2215+
}
2216+
return tableToSchema, tableToDatabase
2217+
}

0 commit comments

Comments
 (0)