Skip to content

Commit

Permalink
builtins: allow VIEWACTIVITY priv to use crdb_internal.request_statem…
Browse files Browse the repository at this point in the history
…ent_bundle

Previously only those with the VIEWACTIVITY role could use the
crdb_internal.request_statement_bundle builtin. We should allow
the VIEWACTIVITY privilege as well since role options are now
deprecated. This allow also allow stmt bundle requests to be made
from db-console for users with this granted privilege.

Epic: none
Fixes: #118759

Release note (bug fix): Those with VIEWACTIVITY privilege can now
request statement bundles using crdb_internal.requets_statement_bundle
or via db-console's sql activity page.
  • Loading branch information
xinhaoz committed Feb 5, 2024
1 parent 63dc83b commit ce81ca1
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 8 deletions.
4 changes: 2 additions & 2 deletions pkg/server/application_api/stmtdiag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func TestCreateStatementDiagnosticsReportWithViewActivityOptions(t *testing.T) {
require.Contains(t, err.Error(), "requesting statement bundle requires VIEWACTIVITY or ADMIN role option")

// Grant VIEWACTIVITY and all test should work.
db.Exec(t, fmt.Sprintf("ALTER USER %s VIEWACTIVITY", apiconstants.TestingUserNameNoAdmin().Normalized()))
db.Exec(t, fmt.Sprintf("GRANT SYSTEM VIEWACTIVITY TO %s", apiconstants.TestingUserNameNoAdmin().Normalized()))
req := &serverpb.CreateStatementDiagnosticsReportRequest{
StatementFingerprint: "INSERT INTO test VALUES (_)",
}
Expand Down Expand Up @@ -159,7 +159,7 @@ func TestCreateStatementDiagnosticsReportWithViewActivityOptions(t *testing.T) {
`, [][]string{{"1"}})

// Grant VIEWACTIVITYREDACTED and all test should get permission errors.
db.Exec(t, fmt.Sprintf("ALTER USER %s VIEWACTIVITYREDACTED", apiconstants.TestingUserNameNoAdmin().Normalized()))
db.Exec(t, fmt.Sprintf("GRANT SYSTEM VIEWACTIVITYREDACTED TO %s", apiconstants.TestingUserNameNoAdmin().Normalized()))

if err := srvtestutils.PostStatusJSONProtoWithAdminOption(ts, "stmtdiagreports", req, &resp, false); err != nil {
if !testutils.IsError(err, "status: 403") {
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/sem/builtins/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ go_library(
"//pkg/sql/pgwire/pgnotice",
"//pkg/sql/privilege",
"//pkg/sql/protoreflect",
"//pkg/sql/roleoption",
"//pkg/sql/rowenc",
"//pkg/sql/rowenc/keyside",
"//pkg/sql/rowenc/valueside",
Expand Down
9 changes: 4 additions & 5 deletions pkg/sql/sem/builtins/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/protoreflect"
"github.com/cockroachdb/cockroach/pkg/sql/roleoption"
"github.com/cockroachdb/cockroach/pkg/sql/rowenc"
"github.com/cockroachdb/cockroach/pkg/sql/rowenc/keyside"
"github.com/cockroachdb/cockroach/pkg/sql/sem/asof"
Expand Down Expand Up @@ -11521,8 +11520,8 @@ true, then any plan other then the specified gist will be used`
Types: typs,
ReturnType: tree.FixedReturnType(types.Bool),
Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) {
hasViewActivity, err := evalCtx.SessionAccessor.HasRoleOption(
ctx, roleoption.VIEWACTIVITY)
hasViewActivity, err := evalCtx.SessionAccessor.HasGlobalPrivilegeOrRoleOption(
ctx, privilege.VIEWACTIVITY)
if err != nil {
return nil, err
}
Expand All @@ -11537,8 +11536,8 @@ true, then any plan other then the specified gist will be used`
return nil, err
}

hasViewActivityRedacted, err := evalCtx.SessionAccessor.HasRoleOption(
ctx, roleoption.VIEWACTIVITYREDACTED)
hasViewActivityRedacted, err := evalCtx.SessionAccessor.HasGlobalPrivilegeOrRoleOption(
ctx, privilege.VIEWACTIVITYREDACTED)
if err != nil {
return nil, err
}
Expand Down

0 comments on commit ce81ca1

Please sign in to comment.