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.

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: fix SHOW ALL ZONE CONFIGURATION displaying unprivileged entries #42080

Merged
merged 1 commit into from Nov 5, 2019

Conversation

@otan
Copy link
Collaborator

otan commented Oct 31, 2019

Fully resolves #40917.

  • Made crdb_internal.zones not display entries which the executing
    user does not have access to.
  • Add a delegate for ShowZoneConfig, which triggers on SHOW ALL
    ZONE CONFIGURATIONS, to use crdb_internal.zones table and run it as
    the executing user so rows are hidden if the user does not have
    permission.

Release note (bug fix): Previously, SHOW ALL ZONE CONFIGURATION ZONES and crdb_internal.zones shows results for resources the user does not have access to. This will instead filter out those entries from displaying.

@otan otan requested a review from solongordon Oct 31, 2019
@cockroach-teamcity

This comment has been minimized.

Copy link
Member

cockroach-teamcity commented Oct 31, 2019

This change is Reviewable

@otan otan requested a review from cockroachdb/sql-execution Oct 31, 2019
@otan otan force-pushed the otan-cockroach:otan-fix_show_all branch 5 times, most recently from a05f036 to ba5a12d Oct 31, 2019
Copy link
Contributor

solongordon left a comment

Nice, :lgtm: pending a couple comment fixes.

Reviewed 1 of 5 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @otan and @solongordon)


pkg/sql/crdb_internal.go, line 2183 at r1 (raw file):

					continue
				}
			}

Kind of too bad this logic is mostly duplicated with previous PR, but I guess we're looking up descriptors by ID in one case and by name in the other, so it would be awkward to dedup.


pkg/sql/delegate/show_zone_config.go, line 1 at r1 (raw file):

// Copyright 2017 The Cockroach Authors.

I don't think it really matters, but... 2019.


pkg/sql/delegate/show_zone_config.go, line 15 at r1 (raw file):

import "github.com/cockroachdb/cockroach/pkg/sql/sem/tree"

// ShowZoneConfig only delegates if it selecting ALL users.

All configurations, not users.

* Made `crdb_internal.zones` not display entries which the executing
user does not have access to.
* Add a delegate for ShowZoneConfig, which triggers on SHOW ALL
ZONE CONFIGURATIONS, to use crdb_internal.zones table and run it as
the executing user so rows are hidden if the user does not have
permission.

Release note (bug fix): Previously, SHOW ALL ZONE CONFIGURATION ZONES
and crdb_internal.zones shows results for resources the user does not
have access to. This will instead filter out those entries from
displaying.
Copy link
Collaborator Author

otan left a comment

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @solongordon)


pkg/sql/delegate/show_zone_config.go, line 1 at r1 (raw file):

Previously, solongordon (Solon) wrote…

I don't think it really matters, but... 2019.

haha, is there a way of auto-generating these? i've been copy pasting.
Done.


pkg/sql/delegate/show_zone_config.go, line 15 at r1 (raw file):

Previously, solongordon (Solon) wrote…

All configurations, not users.

Done.

@otan otan force-pushed the otan-cockroach:otan-fix_show_all branch from ba5a12d to d07c378 Nov 5, 2019
@otan

This comment has been minimized.

Copy link
Collaborator Author

otan commented Nov 5, 2019

bors r+

craig bot pushed a commit that referenced this pull request Nov 5, 2019
42080: sql: fix SHOW ALL ZONE CONFIGURATION displaying unprivileged entries r=otan a=otan

Fully resolves #40917.

* Made `crdb_internal.zones` not display entries which the executing
    user does not have access to.
* Add a delegate for ShowZoneConfig, which triggers on SHOW ALL
    ZONE CONFIGURATIONS, to use crdb_internal.zones table and run it as
    the executing user so rows are hidden if the user does not have
    permission.

Release note (bug fix): Previously, SHOW ALL ZONE CONFIGURATION ZONES and crdb_internal.zones shows results for resources the user does not have access to. This will instead filter out those entries from displaying.

Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
@craig

This comment has been minimized.

Copy link

craig bot commented Nov 5, 2019

Build succeeded

@craig craig bot merged commit d07c378 into cockroachdb:master Nov 5, 2019
2 of 3 checks passed
2 of 3 checks passed
GitHub CI (Cockroach) TeamCity build failed
Details
bors Build succeeded
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.