Skip to content

Commit

Permalink
changefeedccl: add new fine grained permissions
Browse files Browse the repository at this point in the history 
This change updates permissions semantics related to creating and managing changefeeds.

The `CONTROLCHANGEFEED` role option will be deprecated in the future (see cockroachdb#94757). With this change, usages of `CONTROLCHANGEFEED` will come with a deprecation warning. Its existing behavior (see rules for creating changefeeds below) remains the same.

The `SELECT` and `CHANGEFEED` privileges will be used for changefeeds henceforth:

The `SELECT` privilege on a set of tables allows a user to run core changefeeds against them.

The `CHANGEFEED` privilege on a set of tables allows a user to run enterprise changefeeds on them, and also manage the underlying changefeed job (ie. view, pause, cancel, and resume the job). Notably, a new cluster setting `changefeed.permissions.enforce_external_connections` is added and set to `false` by default. Enabling this setting restricts users with `CHANGEFEED` on a set of tables to create enterprise changefeeds into external connections only. To use a given external connection, a user typically needs the `USAGE` privilege on it.

Note `ALTER DEFAULT PRIVILEGES` can be used with both the `CHANGEFEED` and `SELECT` privileges to assign course-grained permissions (ie. assign permissions to all tables in a schema rather than manually assign them for each table).

Before this change, to create a changefeed, these checks were made in order:
(a) If the user has the `CONTROLCHANGEFEED` role, then they require the `SELECT`privilege
     on all targeted tables
(b) Otherwise, the user requires  the `CHANGEFEED` privilege on all targeted tables
With this change, these checks are updated:
(a) If the user has the `CONTROLCHANGEFEED` role, then they require the `SELECT`
     privilege on all targeted tables. Note: creating a changefeed this way will now produce a
     deprecation notice.
(b) If the changefeed is a core changefeed, they require the `SELECT` privilege on all targeted
     tables
(c) Otherwise, the user requires the `CHANGEFEED` privilege on all targeted tables. Note: If
     `changefeed.permissions.enforce_external_connections` (disabled by default) is set to true,
     then the user will only be able to create a changefeed into an external connection which they
     have the `USAGE` privilege on.

Before this change, to manage a changefeed job `J` (defined a viewing, pausing, resuming, and canceling), a user `U` could do so if they met at least one of the following conditions:
(a) `U` is an admin
(b) `U` is not an admin and `J` is owned by `U`  (only for SHOW JOBS)
(c) `U` is not an admin, `J` is not owned by an admin, and `U` has the `CONTROLJOB` role
With this change, the conditions are updated:
(a) `U` is an admin
(b) `U` is not an admin and `J` is owned by `U`  (only for `SHOW JOBS` or `SHOW
     CHANGEFEED JOBS`)
(c) `U` is not an admin, `J` is not owned by an admin, and `U` has the `CONTROLJOB` role
(d) `U` is not an admin, `J` is not owned by an admin, `J` is a changefeed job, and `U` has
     the `CHANGEFEED` privilege on targeted tables

Before this change, permissions related to altering changefeeds with `ALTER CHANGEFEED` were not explicitly defined (we did not have tests to assert its behavior, but there were some permissions checks regardless). Basically, a user needed access to view a job (ie. look up it’s job ID via `SHOW JOBS`) and they needed to be able to create a new job. After all, `ALTER CHANGEFEED` is essentially the same as creating a new job after stopping the old one.

With this change, the same rules apply: the user needs to be able to access the existing job and to be able to create a new changefeed with the new rules introduced in this change respectively.

Fixes: cockroachdb#94756
Fixes: cockroachdb#92261
Fixes: cockroachdb#87884
Fixes: cockroachdb#85082
Fixes: cockroachdb#94759
Informs: cockroachdb#94757
Epic: CRDB-19709

Release note (enterprise change):

The `CONTROLCHANGEFEED` role option will be deprecated in the future (see cockroachdb#94757). With this change, usages of `CONTROLCHANGEFEED` will come with a deprecation warning. Its existing behavior (see rules for creating changefeeds above) remains the same.

The `SELECT` and `CHANGEFEED` privileges will be used for changefeeds henceforth:

The `SELECT` privilege on a set of tables allows a user to run core changefeeds against them.

The `CHANGEFEED` privilege on a set of tables allows a user to run enterprise changefeeds on them, and also manage the underlying changefeed job (ie. view, pause, cancel, and resume the job). Notably, a new cluster setting `changefeed.permissions.enforce_external_connections` is added and set to `false` by default. Enabling this setting restricts users with `CHANGEFEED` on a set of tables to create enterprise changefeeds into external connections only. To use a given external connection, a user typically needs the `USAGE` privilege on it.

Note `ALTER DEFAULT PRIVILEGES` can be used with both both the `CHANGEFEED` and `SELECT` privileges to assign coarse-grained permissions (ie. assign permissions to all tables in a schema rather than manually assign them for each table).
  • Loading branch information
@jayshrivastava
jayshrivastava committed Jan 11, 2023
1 parent 0f6333c commit 00dac04
Show file tree
Hide file tree
Showing 31 changed files with 1,443 additions and 186 deletions.
10 changes: 10 additions & 0 deletions pkg/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ ALL_TESTS = [
"//pkg/internal/sqlsmith:sqlsmith_test",
"//pkg/internal/team:team_test",
"//pkg/jobs/joberror:joberror_test",
"//pkg/jobs/jobsauth:authorization_test",
"//pkg/jobs/jobsauth:jobsauth_test",
"//pkg/jobs/jobsauth:privilege_test",
"//pkg/jobs/jobsprotectedts:jobsprotectedts_test",
"//pkg/jobs:jobs_test",
"//pkg/keys:keys_test",
Expand Down Expand Up @@ -1110,6 +1113,12 @@ GO_TARGETS = [
"//pkg/internal/team:team_test",
"//pkg/jobs/joberror:joberror",
"//pkg/jobs/joberror:joberror_test",
"//pkg/jobs/jobsauth:authorization",
"//pkg/jobs/jobsauth:authorization_test",
"//pkg/jobs/jobsauth:jobsauth",
"//pkg/jobs/jobsauth:jobsauth_test",
"//pkg/jobs/jobsauth:privilege",
"//pkg/jobs/jobsauth:privilege_test",
"//pkg/jobs/jobspb:jobspb",
"//pkg/jobs/jobsprotectedts:jobsprotectedts",
"//pkg/jobs/jobsprotectedts:jobsprotectedts_test",
Expand Down Expand Up @@ -2510,6 +2519,7 @@ GET_X_DATA_TARGETS = [
"//pkg/internal/team:get_x_data",
"//pkg/jobs:get_x_data",
"//pkg/jobs/joberror:get_x_data",
"//pkg/jobs/jobsauth:get_x_data",
"//pkg/jobs/jobspb:get_x_data",
"//pkg/jobs/jobsprotectedts:get_x_data",
"//pkg/jobs/jobstest:get_x_data",
Expand Down
4 changes: 4 additions & 0 deletions pkg/ccl/changefeedccl/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go_library(
name = "changefeedccl",
srcs = [
"alter_changefeed_stmt.go",
"authorization.go",
"avro.go",
"changefeed.go",
"changefeed_dist.go",
Expand Down Expand Up @@ -60,6 +61,7 @@ go_library(
"//pkg/geo",
"//pkg/geo/geopb",
"//pkg/jobs",
"//pkg/jobs/jobsauth",
"//pkg/jobs/jobspb",
"//pkg/jobs/jobsprotectedts",
"//pkg/keys",
Expand Down Expand Up @@ -106,6 +108,7 @@ go_library(
"//pkg/sql/sessiondata",
"//pkg/sql/sessiondatapb",
"//pkg/sql/sqlutil",
"//pkg/sql/syntheticprivilege",
"//pkg/sql/types",
"//pkg/util",
"//pkg/util/admission",
Expand Down Expand Up @@ -235,6 +238,7 @@ go_test(
"//pkg/sql",
"//pkg/sql/catalog",
"//pkg/sql/catalog/bootstrap",
"//pkg/sql/catalog/catpb",
"//pkg/sql/catalog/colinfo",
"//pkg/sql/catalog/descbuilder",
"//pkg/sql/catalog/descpb",
Expand Down
30 changes: 28 additions & 2 deletions pkg/ccl/changefeedccl/alter_changefeed_stmt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/ccl/changefeedccl/changefeedbase"
"github.com/cockroachdb/cockroach/pkg/ccl/changefeedccl/changefeedvalidators"
"github.com/cockroachdb/cockroach/pkg/jobs"
"github.com/cockroachdb/cockroach/pkg/jobs/jobsauth"
"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/roachpb"
Expand Down Expand Up @@ -100,6 +101,12 @@ func alterChangefeedPlanHook(
return err
}

jobPayload := job.Payload()

if err := jobsauth.Authorize(ctx, p, jobID, &jobPayload, jobsauth.ControlAccess); err != nil {
return err
}

prevDetails, ok := job.Details().(jobspb.ChangefeedDetails)
if !ok {
return errors.Errorf(`job %d is not changefeed job`, jobID)
Expand All @@ -123,11 +130,12 @@ func alterChangefeedPlanHook(
return err
}

newTargets, newProgress, newStatementTime, originalSpecs, err := generateNewTargets(
newTargets, newProgress, newStatementTime, originalSpecs, err := generateAndValidateNewTargets(
ctx, exprEval, p,
alterChangefeedStmt.Cmds,
newOptions.AsMap(), // TODO: Remove .AsMap()
prevDetails, job.Progress(),
newSinkURI,
)
if err != nil {
return err
Expand Down Expand Up @@ -320,14 +328,15 @@ func generateNewOpts(
return changefeedbase.MakeStatementOptions(newOptions), sinkURI, nil
}

func generateNewTargets(
func generateAndValidateNewTargets(
ctx context.Context,
exprEval exprutil.Evaluator,
p sql.PlanHookState,
alterCmds tree.AlterChangefeedCmds,
opts map[string]string,
prevDetails jobspb.ChangefeedDetails,
prevProgress jobspb.Progress,
sinkURI string,
) (
tree.ChangefeedTargets,
*jobspb.Progress,
Expand Down Expand Up @@ -490,6 +499,7 @@ func generateNewTargets(
existingTargetSpans := fetchSpansForDescs(p, existingTargetDescs)
var newTargetDescs []catalog.Descriptor
for _, target := range v.Targets {

desc, found, err := getTargetDesc(ctx, p, descResolver, target.TableName)
if err != nil {
return nil, nil, hlc.Timestamp{}, nil, err
Expand All @@ -501,6 +511,7 @@ func generateNewTargets(
tree.ErrString(&target),
)
}

k := targetKey{TableID: desc.GetID(), FamilyName: target.FamilyName}
newTargets[k] = target
newTableDescs[desc.GetID()] = desc
Expand Down Expand Up @@ -546,6 +557,7 @@ func generateNewTargets(
tree.ErrString(&target),
)
}
newTableDescs[desc.GetID()] = desc
delete(newTargets, k)
}
telemetry.CountBucketed(telemetryPath+`.dropped_targets`, int64(len(v.Targets)))
Expand Down Expand Up @@ -580,6 +592,20 @@ func generateNewTargets(
newTargetList = append(newTargetList, target)
}

hasSelectPrivOnAllTables := true
hasChangefeedPrivOnAllTables := true
for _, desc := range newTableDescs {
hasSelect, hasChangefeed, err := checkPrivilegesForDescriptor(ctx, p, desc)
if err != nil {
return nil, nil, hlc.Timestamp{}, nil, err
}
hasSelectPrivOnAllTables = hasSelectPrivOnAllTables && hasSelect
hasChangefeedPrivOnAllTables = hasChangefeedPrivOnAllTables && hasChangefeed
}
if err := authorizeUserToCreateChangefeed(ctx, p, sinkURI, hasSelectPrivOnAllTables, hasChangefeedPrivOnAllTables); err != nil {
return nil, nil, hlc.Timestamp{}, nil, err
}

if err := validateNewTargets(ctx, p, newTargetList, newJobProgress, newJobStatementTime); err != nil {
return nil, nil, hlc.Timestamp{}, nil, err
}
Expand Down
Loading

0 comments on commit 00dac04

Please sign in to comment.