Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: ensure revalidate_unique_constraint* builtins respect privileges #83959

Merged
merged 1 commit into from
Jul 9, 2022
Merged

sql: ensure revalidate_unique_constraint* builtins respect privileges #83959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/multi_region_privileges
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,3 +268,50 @@ user testuser
statement ok
ALTER DATABASE repartition_privs ADD REGION "ap-southeast-2";
ALTER DATABASE repartition_privs DROP REGION "us-east-1"

subtest revalidate_privs

user root

statement ok
CREATE DATABASE revalidate_privs PRIMARY REGION "ca-central-1" REGIONS "us-east-1"

statement ok
CREATE TABLE revalidate_privs.rbr () LOCALITY REGIONAL BY ROW

user testuser

statement ok
USE revalidate_privs

# Check that revalidate_unique_constraint* builtins respect privileges.
query error user testuser does not have SELECT privilege on relation rbr
SELECT crdb_internal.revalidate_unique_constraints_in_all_tables()

query error user testuser does not have SELECT privilege on relation rbr
SELECT crdb_internal.revalidate_unique_constraints_in_table('revalidate_privs.rbr')

query error user testuser does not have SELECT privilege on relation rbr
SELECT crdb_internal.revalidate_unique_constraint('revalidate_privs.rbr', 'rbr_pkey')

user root

statement ok
GRANT SELECT ON revalidate_privs.rbr TO testuser

user testuser

query T
SELECT crdb_internal.revalidate_unique_constraints_in_all_tables()
----
·

query T
SELECT crdb_internal.revalidate_unique_constraints_in_table('repartition_privs.rbr')
----
·

query T
SELECT crdb_internal.revalidate_unique_constraint('repartition_privs.rbr', 'rbr_pkey')
----
·
2 changes: 1 addition & 1 deletion pkg/sql/alter_table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ func (n *alterTableNode) startExec(params runParams) error {
if err := validateUniqueWithoutIndexConstraintInTxn(
params.ctx, params.ExecCfg().InternalExecutorFactory(
params.ctx, params.SessionData(),
), n.tableDesc, params.p.Txn(), name,
), n.tableDesc, params.p.Txn(), params.p.User(), name,
); err != nil {
return err
}
Expand Down
19 changes: 16 additions & 3 deletions pkg/sql/backfill.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/security/username"
"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/sql/backfill"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
Expand Down Expand Up @@ -763,7 +764,9 @@ func (sc *SchemaChanger) validateConstraints(
return err
}
} else if c.IsUniqueWithoutIndex() {
if err := validateUniqueWithoutIndexConstraintInTxn(ctx, sc.ieFactory(ctx, evalCtx.SessionData()), desc, txn, c.GetName()); err != nil {
if err := validateUniqueWithoutIndexConstraintInTxn(
ctx, sc.ieFactory(ctx, evalCtx.SessionData()), desc, txn, evalCtx.SessionData().User(), c.GetName(),
); err != nil {
return err
}
} else if c.IsNotNull() {
Expand Down Expand Up @@ -1941,6 +1944,7 @@ func countIndexRowsAndMaybeCheckUniqueness(
idx.GetPredicate(),
ie,
txn,
username.NodeUserName(),
false, /* preExisting */
); err != nil {
return err
Expand Down Expand Up @@ -2416,7 +2420,7 @@ func runSchemaChangesInTxn(
uwi := &c.ConstraintToUpdateDesc().UniqueWithoutIndexConstraint
if uwi.Validity == descpb.ConstraintValidity_Validating {
if err := validateUniqueWithoutIndexConstraintInTxn(
ctx, planner.ExecCfg().InternalExecutor, tableDesc, planner.txn, c.GetName(),
ctx, planner.ExecCfg().InternalExecutor, tableDesc, planner.txn, planner.User(), c.GetName(),
); err != nil {
return err
}
Expand Down Expand Up @@ -2575,6 +2579,7 @@ func validateUniqueWithoutIndexConstraintInTxn(
ie sqlutil.InternalExecutor,
tableDesc *tabledesc.Mutable,
txn *kv.Txn,
user username.SQLUsername,
constraintName string,
) error {
var syntheticDescs []catalog.Descriptor
Expand All @@ -2596,7 +2601,15 @@ func validateUniqueWithoutIndexConstraintInTxn(

return ie.WithSyntheticDescriptors(syntheticDescs, func() error {
return validateUniqueConstraint(
ctx, tableDesc, uc.Name, uc.ColumnIDs, uc.Predicate, ie, txn, false, /* preExisting */
ctx,
tableDesc,
uc.Name,
uc.ColumnIDs,
uc.Predicate,
ie,
txn,
user,
false, /* preExisting */
)
})
}
Expand Down
25 changes: 20 additions & 5 deletions pkg/sql/check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (

"github.com/cockroachdb/cockroach/pkg/jobs"
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/security/username"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
Expand Down Expand Up @@ -381,7 +382,9 @@ func (p *planner) RevalidateUniqueConstraintsInCurrentDB(ctx context.Context) er
}

for _, tableDesc := range tableDescs {
if err = RevalidateUniqueConstraintsInTable(ctx, p.Txn(), p.ExecCfg().InternalExecutor, tableDesc); err != nil {
if err = RevalidateUniqueConstraintsInTable(
ctx, p.Txn(), p.User(), p.ExecCfg().InternalExecutor, tableDesc,
); err != nil {
return err
}
}
Expand All @@ -402,7 +405,9 @@ func (p *planner) RevalidateUniqueConstraintsInTable(ctx context.Context, tableI
if err != nil {
return err
}
return RevalidateUniqueConstraintsInTable(ctx, p.Txn(), p.ExecCfg().InternalExecutor, tableDesc)
return RevalidateUniqueConstraintsInTable(
ctx, p.Txn(), p.User(), p.ExecCfg().InternalExecutor, tableDesc,
)
}

// RevalidateUniqueConstraint verifies that the given unique constraint on the
Expand Down Expand Up @@ -438,6 +443,7 @@ func (p *planner) RevalidateUniqueConstraint(
index.GetPredicate(),
p.ExecCfg().InternalExecutor,
p.Txn(),
p.User(),
true, /* preExisting */
)
}
Expand All @@ -457,6 +463,7 @@ func (p *planner) RevalidateUniqueConstraint(
uc.Predicate,
p.ExecCfg().InternalExecutor,
p.Txn(),
p.User(),
true, /* preExisting */
)
}
Expand Down Expand Up @@ -531,7 +538,11 @@ func HasVirtualUniqueConstraints(tableDesc catalog.TableDescriptor) bool {
// enforced by an index. This includes implicitly partitioned UNIQUE indexes
// and UNIQUE WITHOUT INDEX constraints.
func RevalidateUniqueConstraintsInTable(
ctx context.Context, txn *kv.Txn, ie sqlutil.InternalExecutor, tableDesc catalog.TableDescriptor,
ctx context.Context,
txn *kv.Txn,
user username.SQLUsername,
ie sqlutil.InternalExecutor,
tableDesc catalog.TableDescriptor,
) error {
// Check implicitly partitioned UNIQUE indexes.
for _, index := range tableDesc.ActiveIndexes() {
Expand All @@ -544,6 +555,7 @@ func RevalidateUniqueConstraintsInTable(
index.GetPredicate(),
ie,
txn,
user,
true, /* preExisting */
); err != nil {
log.Errorf(ctx, "validation of unique constraints failed for table %s: %s", tableDesc.GetName(), err)
Expand All @@ -563,6 +575,7 @@ func RevalidateUniqueConstraintsInTable(
uc.Predicate,
ie,
txn,
user,
true, /* preExisting */
); err != nil {
log.Errorf(ctx, "validation of unique constraints failed for table %s: %s", tableDesc.GetName(), err)
Expand Down Expand Up @@ -591,6 +604,7 @@ func validateUniqueConstraint(
pred string,
ie sqlutil.InternalExecutor,
txn *kv.Txn,
user username.SQLUsername,
preExisting bool,
) error {
query, colNames, err := duplicateRowQuery(
Expand All @@ -607,8 +621,9 @@ func validateUniqueConstraint(
query,
)

values, err := ie.QueryRowEx(ctx, "validate unique constraint", txn,
sessiondata.NodeUserSessionDataOverride, query)
sessionDataOverride := sessiondata.NoSessionDataOverride
sessionDataOverride.User = user
values, err := ie.QueryRowEx(ctx, "validate unique constraint", txn, sessionDataOverride, query)
if err != nil {
return err
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/importer/import_job.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ func (r *importResumer) Resume(ctx context.Context, execCtx interface{}) error {
return err
}

if err := r.checkVirtualConstraints(ctx, p.ExecCfg(), r.job); err != nil {
if err := r.checkVirtualConstraints(ctx, p.ExecCfg(), r.job, p.User()); err != nil {
return err
}

Expand Down Expand Up @@ -1103,8 +1103,8 @@ func (r *importResumer) publishSchemas(ctx context.Context, execCfg *sql.Executo

// checkVirtualConstraints checks constraints that are enforced via runtime
// checks, such as uniqueness checks that are not directly backed by an index.
func (*importResumer) checkVirtualConstraints(
ctx context.Context, execCfg *sql.ExecutorConfig, job *jobs.Job,
func (r *importResumer) checkVirtualConstraints(
ctx context.Context, execCfg *sql.ExecutorConfig, job *jobs.Job, user username.SQLUsername,
) error {
for _, tbl := range job.Details().(jobspb.ImportDetails).Tables {
desc := tabledesc.NewBuilder(tbl.Desc).BuildExistingMutableTable()
Expand All @@ -1121,7 +1121,7 @@ func (*importResumer) checkVirtualConstraints(
if err := execCfg.DB.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error {
ie := execCfg.InternalExecutorFactory(ctx, sql.NewFakeSessionData(execCfg.SV()))
return ie.WithSyntheticDescriptors([]catalog.Descriptor{desc}, func() error {
return sql.RevalidateUniqueConstraintsInTable(ctx, txn, ie, desc)
return sql.RevalidateUniqueConstraintsInTable(ctx, txn, user, ie, desc)
})
}); err != nil {
return err
Expand Down