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: some role options [e.g. CREATEROLE] don't get passed down to the members of roles #103237

Closed
Arivijay opened this issue May 12, 2023 · 2 comments · Fixed by #110220
Closed
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

@Arivijay
Copy link

Arivijay commented May 12, 2023

Describe the problem

Role options given to roles don't get passed down to the members of those roles.
Even though system privileges are inherited by role members, there isn't an equivalent system privilege for role options such as CREATEROLE.

To Reproduce

CREATE ROLE can_create_role WITH CREATEROLE;
CREATE USER can_create_user WITH PASSWORD 'xxx';
GRANT can_create_role TO can_create_user;
Login as USER and create a role

[email protected]:26257/defaultdb> CREATE ROLE test;
ERROR: user can_create_user1 does not have CREATEROLE privilege

Environment:

  • CockroachDB version v22.2.x

Additional context
Since customer has more than 50 AppDBAs, assigning permissions at the user level would be quite tedious and could lead to errors.

Jira issue: CRDB-27893

Epic CRDB-27601

@Arivijay Arivijay added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label May 12, 2023
@rafiss rafiss changed the title Role options given to roles don't get passed down to the members of those roles SQL: some ole options [e.g. CREATEROLE] don't get passed down to the members of roles May 12, 2023
@rafiss rafiss changed the title SQL: some ole options [e.g. CREATEROLE] don't get passed down to the members of roles SQL: some role options [e.g. CREATEROLE] don't get passed down to the members of roles May 12, 2023
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 12, 2023
@rafiss
Copy link
Collaborator

rafiss commented May 16, 2023

@andyyang890 could you take a look at this request? we will want to backport it to 23.1. i think this should be a a few days of work, and it will be a good issue to learn about how new system privileges are added.

@andyyang890
Copy link
Collaborator

andyyang890 commented May 16, 2023

To clarify, do we want to add new system-level privileges for all remaining role options or are we making role options inheritable through role membership?

Edit: Confirmed offline that we want to add new system-level privileges since we want role options to remain compatible with Postgres.

craig bot pushed a commit that referenced this issue Aug 31, 2023
109245: cluster-ui: handle partial response errors on the databases page r=THardy98 a=THardy98

Part of: #102386

**Demos** (Note: these demos show this same logic applied to both the database details and database table pages as well):
DB-Console
- https://www.loom.com/share/5108dd655ad342f28323e72eaf68219c
- https://www.loom.com/share/1973383dacd7494a84e10bf39e5b85a3


Prior to this change, any query error in the reponse payload for the
databases page would cause the page to render an error. This was
problematic as some queries used to power the databases page directly
query `system` tables, meaning only `ADMIN` users could access the page.
This change allows the databases page to handle network responses with
query errors, consequently allowing non-admin users to view the data
they are privy to.

On the databases page, two types of requests are made:
- a single request to fetch all database names
- a request to fetch the database details for each database name


The error handling for these requests has changed as such:
- if we encounter a network or a non size-related query error when
  requesting database names, render a page error
- if we encounter a 'max size' query error when requesting database
  names, render an alert that we're showing partial results
- if we encounter any error requesting a database's details, render a
  `Caution` icon next to the database's name to indicate there was an
issue getting results, the `Caution` icon has a tooltip providing a
general explanation as to what the issue is
- network errors when fetching database details provide no data for the
  database's table row, consequently the row of statistics for that
database is `unavailable`, the network error message is provided in the
`Caution` icon tooltip
- query errors when fetching database details are scoped to the row
  cells for that query, which are `unavailable`
- `unavailable` cells have a tooltip that highlight the error for that
  cell as well

**Note**: the change in `planner.go` allows users to see span statistics for any table, not just tables a user has `SELECT` privileges for. This is particularly useful for the databases page where users need to see the span statistics for a database, but only have `SELECT` privileges on a subset of tables. Regardless, span statistics are gated by `VIEWACTIVITY`, not `SELECT` privileges.

Release note (ui change): Allow non-admin users to view the databases
page.

109438: roachprod: azure gc cleans up empty clusters r=renatolabs a=DarrylWong

Azure allows for clusters to exist with no attached VM, which roachprod assumes is not possible. This would occur if azure.create failed after creating a resource group but before creating a VM. Roachprod GC only searches for VMs when searching for clusters, so these VM-less clusters would never be found and cleaned up.

This change adds the ability to list empty clusters by querying resource groups instead of VMs. This is used by GC and destroy jobs to correctly identify and cleanup these dangling resources.

Epic: https://cockroachlabs.atlassian.net/browse/CRDB-3373
Release note: None

109652: sql: refactor global privilege and legacy role option checking r=rafiss a=andyyang890

This patch abstracts the logic for checking whether a user has a
global (system) privilege or the corresponding legacy role option
into a new function (`HasGlobalPrivilegeOrRoleOption`).

It also adds a wrapper function that returns an insufficient
privileges error when the user has neither the privilege nor
the legacy role option (`CheckGlobalPrivilegeOrRoleOption`).

Informs #103237

Release note: None

Co-authored-by: Thomas Hardy <[email protected]>
Co-authored-by: DarrylWong <[email protected]>
Co-authored-by: Andy Yang <[email protected]>
craig bot pushed a commit that referenced this issue Sep 6, 2023
109258: sql: add new CREATEROLE system privilege r=rafiss a=andyyang890

This patch adds a new `CREATEROLE` system privilege, which is analogous
to the existing `CREATEROLE` role option, but can also be inherited
by role membership.

Informs #103237

Release note (sql change): There is now a `CREATEROLE` system privilege,
which is analogous to the existing `CREATEROLE` role option, but can
also be inherited by role membership.

Co-authored-by: Andy Yang <[email protected]>
@craig craig bot closed this as completed in 4922a93 Sep 8, 2023
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
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants