diff --git a/pkg/ccl/serverccl/statusccl/tenant_status_test.go b/pkg/ccl/serverccl/statusccl/tenant_status_test.go index e716103b1c11..645bfd42e4ca 100644 --- a/pkg/ccl/serverccl/statusccl/tenant_status_test.go +++ b/pkg/ccl/serverccl/statusccl/tenant_status_test.go @@ -161,6 +161,17 @@ func testTenantSpanStats(ctx context.Context, t *testing.T, helper serverccl.Ten require.Error(t, err) require.Contains(t, err.Error(), "Forbidden") + // VIEWCLUSTERMETADATA should allow the user to see the span stats. + grantStmt := `GRANT SYSTEM VIEWCLUSTERMETADATA TO authentic_user_noadmin;` + helper.TestCluster().TenantConn(0).Exec(t, grantStmt) + + err = client.PostJSONChecked("/_status/span", &req, &resp) + require.NoError(t, err) + require.NotEmpty(t, resp.SpanToStats) + + revokeStmt := `REVOKE SYSTEM VIEWCLUSTERMETADATA FROM authentic_user_noadmin;` + helper.TestCluster().TenantConn(0).Exec(t, revokeStmt) + adminClient := helper.TestCluster().TenantHTTPClient(t, 1, true) adminClient.PostJSON("/_status/span", &req, &resp) require.Greaterf(t, resp.SpanToStats[aSpan.String()].RangeCount, int32(0), "positive range count") @@ -1511,6 +1522,9 @@ func testTenantHotRanges(_ context.Context, t *testing.T, helper serverccl.Tenan client.PostJSON("/_status/v2/hotranges", &req, &resp) require.NotEmpty(t, resp.Ranges) + + revokeStmt := `REVOKE SYSTEM VIEWCLUSTERMETADATA FROM authentic_user_noadmin;` + helper.TestCluster().TenantConn(0).Exec(t, revokeStmt) }) t.Run("test tenant hot ranges respects tenant isolation", func(t *testing.T) { diff --git a/pkg/server/admin.go b/pkg/server/admin.go index 78c8a0702c7b..e36fa47acb81 100644 --- a/pkg/server/admin.go +++ b/pkg/server/admin.go @@ -1582,7 +1582,7 @@ func (s *adminServer) Events( ) (_ *serverpb.EventsResponse, retErr error) { ctx = s.AnnotateCtx(ctx) - userName, err := s.requireAdminUser(ctx) + err := s.requireViewClusterMetadataPermission(ctx) if err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. @@ -1595,6 +1595,10 @@ func (s *adminServer) Events( limit = defaultAPIEventLimit } + userName, err := userFromIncomingRPCContext(ctx) + if err != nil { + return nil, serverError(ctx, err) + } r, err := s.eventsHelper(ctx, req, userName, int(limit), 0, redactEvents) if err != nil { return nil, serverError(ctx, err) @@ -3241,7 +3245,7 @@ func (s *systemAdminServer) EnqueueRange( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.requireAdminUser(ctx); err != nil { + if err := s.requireRepairClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -3389,7 +3393,7 @@ func (s *systemAdminServer) SendKVBatch( ctx = s.AnnotateCtx(ctx) // Note: the root user will bypass SQL auth checks, which is useful in case of // a cluster outage. - user, err := s.requireAdminUser(ctx) + err := s.requireRepairClusterMetadataPermission(ctx) if err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. @@ -3399,6 +3403,11 @@ func (s *systemAdminServer) SendKVBatch( return nil, grpcstatus.Errorf(codes.InvalidArgument, "BatchRequest cannot be nil") } + user, err := userFromIncomingRPCContext(ctx) + if err != nil { + return nil, serverError(ctx, err) + } + // Emit a structured log event for the call. jsonpb := protoutil.JSONPb{} baJSON, err := jsonpb.Marshal(ba) @@ -3447,7 +3456,7 @@ func (s *systemAdminServer) RecoveryCollectReplicaInfo( ) error { ctx := stream.Context() ctx = s.server.AnnotateCtx(ctx) - _, err := s.requireAdminUser(ctx) + err := s.requireViewClusterMetadataPermission(ctx) if err != nil { return err } @@ -3462,7 +3471,7 @@ func (s *systemAdminServer) RecoveryCollectLocalReplicaInfo( ) error { ctx := stream.Context() ctx = s.server.AnnotateCtx(ctx) - _, err := s.requireAdminUser(ctx) + err := s.requireViewClusterMetadataPermission(ctx) if err != nil { return err } @@ -3475,7 +3484,7 @@ func (s *systemAdminServer) RecoveryStagePlan( ctx context.Context, request *serverpb.RecoveryStagePlanRequest, ) (*serverpb.RecoveryStagePlanResponse, error) { ctx = s.server.AnnotateCtx(ctx) - _, err := s.requireAdminUser(ctx) + err := s.requireRepairClusterMetadataPermission(ctx) if err != nil { return nil, err } @@ -3488,7 +3497,7 @@ func (s *systemAdminServer) RecoveryNodeStatus( ctx context.Context, request *serverpb.RecoveryNodeStatusRequest, ) (*serverpb.RecoveryNodeStatusResponse, error) { ctx = s.server.AnnotateCtx(ctx) - _, err := s.requireAdminUser(ctx) + err := s.requireViewClusterMetadataPermission(ctx) if err != nil { return nil, err } @@ -3500,7 +3509,7 @@ func (s *systemAdminServer) RecoveryVerify( ctx context.Context, request *serverpb.RecoveryVerifyRequest, ) (*serverpb.RecoveryVerifyResponse, error) { ctx = s.server.AnnotateCtx(ctx) - _, err := s.requireAdminUser(ctx) + err := s.requireViewClusterMetadataPermission(ctx) if err != nil { return nil, err } @@ -3918,20 +3927,6 @@ type adminPrivilegeChecker struct { makePlanner func(opName string) (interface{}, func()) } -// requireAdminUser's error return is a gRPC error. -func (c *adminPrivilegeChecker) requireAdminUser( - ctx context.Context, -) (userName username.SQLUsername, err error) { - userName, isAdmin, err := c.getUserAndRole(ctx) - if err != nil { - return userName, serverError(ctx, err) - } - if !isAdmin { - return userName, errRequiresAdmin - } - return userName, nil -} - // requireViewActivityPermission's error return is a gRPC error. func (c *adminPrivilegeChecker) requireViewActivityPermission(ctx context.Context) (err error) { userName, isAdmin, err := c.getUserAndRole(ctx) @@ -4071,6 +4066,29 @@ func (c *adminPrivilegeChecker) requireViewClusterMetadataPermission( privilege.VIEWCLUSTERMETADATA) } +// requireRepairClusterMetadataPermission requires the user have admin +// or the REPAIRCLUSTERMETADATA system privilege and returns an error if +// the user does not have it. +func (c *adminPrivilegeChecker) requireRepairClusterMetadataPermission( + ctx context.Context, +) (err error) { + userName, isAdmin, err := c.getUserAndRole(ctx) + if err != nil { + return serverError(ctx, err) + } + if isAdmin { + return nil + } + if hasRepairClusterMetadata, err := c.hasGlobalPrivilege(ctx, userName, privilege.REPAIRCLUSTERMETADATA); err != nil { + return serverError(ctx, err) + } else if hasRepairClusterMetadata { + return nil + } + return grpcstatus.Errorf( + codes.PermissionDenied, "this operation requires the %s system privilege", + privilege.REPAIRCLUSTERMETADATA) +} + // requireViewDebugPermission requires the user have admin or the VIEWDEBUG system privilege // and returns an error if the user does not have it. func (c *adminPrivilegeChecker) requireViewDebugPermission(ctx context.Context) (err error) { diff --git a/pkg/server/index_usage_stats.go b/pkg/server/index_usage_stats.go index d342f6f78a77..addf77d6eef1 100644 --- a/pkg/server/index_usage_stats.go +++ b/pkg/server/index_usage_stats.go @@ -134,7 +134,7 @@ func (s *statusServer) ResetIndexUsageStats( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireRepairClusterMetadataPermission(ctx); err != nil { return nil, err } diff --git a/pkg/server/sql_stats.go b/pkg/server/sql_stats.go index 0fd95d1a820c..ecb7fe59a509 100644 --- a/pkg/server/sql_stats.go +++ b/pkg/server/sql_stats.go @@ -26,7 +26,7 @@ func (s *statusServer) ResetSQLStats( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireRepairClusterMetadataPermission(ctx); err != nil { return nil, err } diff --git a/pkg/server/status.go b/pkg/server/status.go index 88c1070cb534..24f7ed374db5 100644 --- a/pkg/server/status.go +++ b/pkg/server/status.go @@ -708,7 +708,7 @@ func (s *systemStatusServer) Gossip( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -736,7 +736,7 @@ func (s *systemStatusServer) EngineStats( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -875,7 +875,7 @@ func (s *systemStatusServer) CriticalNodes( ctx context.Context, req *serverpb.CriticalNodesRequest, ) (*serverpb.CriticalNodesResponse, error) { ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { return nil, err } conformance, err := s.node.SpanConfigConformance( @@ -1008,7 +1008,7 @@ func (s *statusServer) Certificates( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -1129,7 +1129,7 @@ func (s *statusServer) Details( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -1170,7 +1170,7 @@ func (s *statusServer) GetFiles( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -1226,7 +1226,7 @@ func (s *statusServer) LogFilesList( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -1261,7 +1261,7 @@ func (s *statusServer) LogFile( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -1359,7 +1359,7 @@ func (s *statusServer) Logs( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -1447,7 +1447,7 @@ func (s *statusServer) Stacks( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -1480,7 +1480,7 @@ func (s *statusServer) Profile( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -1560,8 +1560,8 @@ func (s *statusServer) NodesList( ctx = s.AnnotateCtx(ctx) // The node status contains details about the command line, network - // addresses, env vars etc which are admin-only. - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + // addresses, env vars etc which are privileged information. + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -1835,8 +1835,8 @@ func (s *statusServer) Node( ctx = s.AnnotateCtx(ctx) // The node status contains details about the command line, network - // addresses, env vars etc which are admin-only. - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + // addresses, env vars etc which are privileged information.. + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -2255,8 +2255,8 @@ func (t *statusServer) TenantRanges( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = t.AnnotateCtx(ctx) - // The tenant range report contains replica metadata which is admin-only. - if _, err := t.privilegeChecker.requireAdminUser(ctx); err != nil { + // The tenant range report contains replica metadata which is privileged. + if err := t.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { return nil, err } @@ -2268,7 +2268,7 @@ func (s *systemStatusServer) TenantRanges( ) (*serverpb.TenantRangesResponse, error) { forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { return nil, err } @@ -2727,7 +2727,7 @@ func (s *statusServer) KeyVisSamples( ctx context.Context, req *serverpb.KeyVisSamplesRequest, ) (*serverpb.KeyVisSamplesResponse, error) { - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { return nil, err } @@ -3471,7 +3471,7 @@ func (s *statusServer) SpanStats( ctx context.Context, req *roachpb.SpanStatsRequest, ) (*roachpb.SpanStatsResponse, error) { ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -3490,7 +3490,7 @@ func (s *systemStatusServer) SpanStats( ) (*roachpb.SpanStatsResponse, error) { ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -3661,7 +3661,7 @@ func (s *statusServer) JobRegistryStatus( ctx = forwardSQLIdentityThroughRPCCalls(ctx) ctx = s.AnnotateCtx(ctx) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -3699,7 +3699,7 @@ func (s *statusServer) JobStatus( ) (*serverpb.JobStatusResponse, error) { ctx = s.AnnotateCtx(forwardSQLIdentityThroughRPCCalls(ctx)) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { // NB: not using serverError() here since the priv checker // already returns a proper gRPC error status. return nil, err @@ -3731,7 +3731,7 @@ func (s *statusServer) TxnIDResolution( ctx context.Context, req *serverpb.TxnIDResolutionRequest, ) (*serverpb.TxnIDResolutionResponse, error) { ctx = s.AnnotateCtx(forwardSQLIdentityThroughRPCCalls(ctx)) - if _, err := s.privilegeChecker.requireAdminUser(ctx); err != nil { + if err := s.privilegeChecker.requireViewClusterMetadataPermission(ctx); err != nil { return nil, err }