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, server: regulate access to remaining observability features #85769

Merged
merged 1 commit into from Aug 11, 2022

Conversation

Santamaura
Copy link
Contributor

@Santamaura Santamaura commented Aug 8, 2022

This change will control access to various observability
features based on system privileges including the following:

  • admin ui databases/tables/schema endpoints requires admin or VIEWACTIVITY
  • EXPERIMENTAL_AUDIT requires admin or MODIFYCLUSTERSETTING
  • sql login requires not having NOSQLLOGIN or the equivalent
    role option

Resolves: #83848, #83863, #83862

Release note (security update): Change requirements to access some
observability features. Databases/tables/schema endpoints for
admin ui require admin or VIEWACTIVITY. EXPERIMENTAL_AUDIT
requires admin or MODIFYCLUSTERSETTING. SQL login requires not
having NOSQLLOGIN or the equivalent role option.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@Santamaura Santamaura changed the title server: regulate access to remaining observability features sql, server: regulate access to remaining observability features Aug 8, 2022
@Santamaura Santamaura force-pushed the sys-priv-gating-obs branch 2 times, most recently from 877f665 to a2bab72 Compare August 9, 2022 16:25
@Santamaura Santamaura requested review from a team, koorosh and zachlite August 9, 2022 19:41
@Santamaura Santamaura marked this pull request as ready for review August 9, 2022 20:41
@Santamaura Santamaura requested review from a team as code owners August 9, 2022 20:41
@Santamaura Santamaura requested a review from a team August 9, 2022 20:41
@RichardJCai RichardJCai self-requested a review August 10, 2022 15:07
Copy link
Contributor

@RichardJCai RichardJCai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! LGTM modulo updating the release note

Reviewed 8 of 9 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @koorosh, @Santamaura, and @zachlite)


-- commits line 4 at r1:
This should be moved into the release note so docs can write about the new requirements


pkg/server/admin_test.go line 393 at r1 (raw file):

	query = fmt.Sprintf(
		"GRANT SYSTEM %s TO %s",
		strings.Join(sysPrivileges, ", "),

Don't need to join here

Copy link
Contributor Author

@Santamaura Santamaura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @koorosh, @RichardJCai, and @zachlite)


-- commits line 4 at r1:

Previously, RichardJCai (Richard Cai) wrote…

This should be moved into the release note so docs can write about the new requirements

Ok, I have added the details to the release note.


pkg/server/admin_test.go line 393 at r1 (raw file):

Previously, RichardJCai (Richard Cai) wrote…

Don't need to join here

Done

This change will control access to various observability
features based on system privileges including the following:
- admin ui databases/tables/schema endpoints requires admin or VIEWACTIVITY
- EXPERIMENTAL_AUDIT requires admin or MODIFYCLUSTERSETTING
- sql login requires not having NOSQLLOGIN or the equivalent
role option

Resolves: cockroachdb#83848, cockroachdb#83863, cockroachdb#83862

Release note (security update): Change requirements to access some
observability features. Databases/tables/schema endpoints for
admin ui require admin or VIEWACTIVITY. EXPERIMENTAL_AUDIT
requires admin or MODIFYCLUSTERSETTING. SQL login requires not
having NOSQLLOGIN or the equivalent role option.
@Santamaura
Copy link
Contributor Author

bors r+

@craig craig bot merged commit 5845f4b into cockroachdb:master Aug 11, 2022
@craig
Copy link
Contributor

craig bot commented Aug 11, 2022

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Control access to view database/table schema/size
3 participants