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

has_sequence_privilege(..., 'USAGE') incorrectly using SELECT #82465

Closed
ecwall opened this issue Jun 6, 2022 · 2 comments · Fixed by #82458
Closed

has_sequence_privilege(..., 'USAGE') incorrectly using SELECT #82465

ecwall opened this issue Jun 6, 2022 · 2 comments · Fixed by #82458
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@ecwall
Copy link
Contributor

ecwall commented Jun 6, 2022

It looks like USAGE can be granted to sequences so this should be updated to reflect that.

	"has_sequence_privilege": makePGPrivilegeInquiryDef(
		"sequence",
		argTypeOpts{{"sequence", strOrOidTypes}},
		func(ctx *eval.Context, args tree.Datums, user username.SQLUsername) (eval.HasAnyPrivilegeResult, error) {
			seqArg := eval.UnwrapDatum(ctx, args[0])
			specifier, err := tableHasPrivilegeSpecifier(seqArg, true /* isSequence */)
			if err != nil {
				return eval.HasNoPrivilege, err
			}
			privs, err := parsePrivilegeStr(args[1], privMap{
				// Sequences and other table objects cannot be given a USAGE privilege,
				// so we check for SELECT here instead. See privilege.TablePrivileges.
				"USAGE":                    {Kind: privilege.SELECT},
				"USAGE WITH GRANT OPTION":  {Kind: privilege.SELECT, GrantOption: true},
				"SELECT":                   {Kind: privilege.SELECT},
				"SELECT WITH GRANT OPTION": {Kind: privilege.SELECT, GrantOption: true},
				"UPDATE":                   {Kind: privilege.UPDATE},
				"UPDATE WITH GRANT OPTION": {Kind: privilege.UPDATE, GrantOption: true},
			})
			if err != nil {
				return eval.HasPrivilege, err
			}
			return ctx.Planner.HasAnyPrivilege(ctx.Context, specifier, user, privs)
		},
	),

Jira issue: CRDB-16409
Epic CRDB-14491

@ecwall ecwall added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Jun 6, 2022
@blathers-crl blathers-crl bot added this to Triage in SQL Sessions - Deprecated Jun 6, 2022
@rafiss
Copy link
Collaborator

rafiss commented Jun 6, 2022

I believe this only affects v22.2, since sequence privileges are new (#79862), but we should confirm

ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Jun 6, 2022
From cockroachdb#79862 on we support the `USAGE` privilege on sequence. This commit
is to fix the `has_sequence_privilege()` builtin function on checking the
`USAGE` privilege on sequences.

fixes cockroachdb#82465

Release note (sql change): fix `has_sequence_privilege()` on `USAGE` privilege
@ZhouXing19
Copy link
Collaborator

ZhouXing19 commented Jun 6, 2022

Will be fixed by #82458 as well

@rafiss rafiss moved this from Triage to 22.2 Now in SQL Sessions - Deprecated Jun 9, 2022
ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Jun 14, 2022
From cockroachdb#79862 on we support the `USAGE` privilege on sequence. This commit
is to fix the `has_sequence_privilege()` builtin function on checking the
`USAGE` privilege on sequences.

fixes cockroachdb#82465

Release note (sql change): fix `has_sequence_privilege()` on `USAGE` privilege
ZhouXing19 added a commit to ZhouXing19/cockroach that referenced this issue Jun 14, 2022
From cockroachdb#79862 on we support the `USAGE` privilege on sequence. This commit
is to fix the `has_sequence_privilege()` builtin function on checking the
`USAGE` privilege on sequences.

fixes cockroachdb#82465

Release note (sql change): fix `has_sequence_privilege()` on `USAGE` privilege
craig bot pushed a commit that referenced this issue Jun 22, 2022
82458: sql: add logic for `GRANT ... ON seq_names` r=rafiss a=ZhouXing19

Currently, the `GRANT ... ON names` syntax by default take all the target
as table, while in Postgres 14, the `names` field accepts names for both
table and sequence. This commit is to add similar sementics.

Given that a table's privilege list is the subset of a sequence's, if the
targets list contains any table, the allowed privilege should be the table
privilege. Only when all targets are of the type sequence, the allowed list
is the sequence privilege.

Since the target list may contain both tables and sequences, when validating,
we need to sepcify the privilege type for each target descriptor.

fixes #82414
fixes #82465

Release note (sql): add logic for GRANT ... ON seq_names

82694: lint: minor cleanup r=andreimatei a=andreimatei

I had screwed up in a previous patch and left the logic for checking the
license headers in a duplicate state. Deleting some lines redundant with
what's right above them.

Release note: None

Co-authored-by: Jane Xing <zhouxing@uchicago.edu>
Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>
@craig craig bot closed this as completed in c1afadf Jun 22, 2022
SQL Sessions - Deprecated automation moved this from 22.2 Now to Done Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants