diff --git a/pkg/server/admin.go b/pkg/server/admin.go index 9e44240abcdc..e4ca6f7fdf2a 100644 --- a/pkg/server/admin.go +++ b/pkg/server/admin.go @@ -81,6 +81,7 @@ import ( gwutil "github.com/grpc-ecosystem/grpc-gateway/utilities" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" grpcstatus "google.golang.org/grpc/status" ) @@ -275,13 +276,15 @@ func (s *adminServer) RegisterGateway( http.Error(w, "invalid id", http.StatusBadRequest) return } - // Add default user when running in Insecure mode because we don't - // retrieve the user from gRPC metadata (which falls back to `root`) - // but from HTTP metadata (which does not). - if s.sqlServer.cfg.Insecure { - ctx := req.Context() - ctx = authserver.ContextWithHTTPAuthInfo(ctx, username.RootUser, 0) - req = req.WithContext(ctx) + + // The privilege checks in the privilege checker below checks the user in the incoming + // gRPC metadata. + md := authserver.TranslateHTTPAuthInfoToGRPCMetadata(req.Context(), req) + authCtx := metadata.NewIncomingContext(req.Context(), md) + authCtx = s.AnnotateCtx(authCtx) + if err := s.privilegeChecker.RequireViewActivityAndNoViewActivityRedactedPermission(authCtx); err != nil { + http.Error(w, err.Error(), http.StatusForbidden) + return } s.getStatementBundle(req.Context(), id, w) }) @@ -2601,12 +2604,12 @@ func (s *adminServer) QueryPlan( } // getStatementBundle retrieves the statement bundle with the given id and -// writes it out as an attachment. +// writes it out as an attachment. Note this function assumes the user has +// permission to access the statement bundle. func (s *adminServer) getStatementBundle(ctx context.Context, id int64, w http.ResponseWriter) { - sqlUsername := authserver.UserFromHTTPAuthInfoContext(ctx) row, err := s.internalExecutor.QueryRowEx( ctx, "admin-stmt-bundle", nil, /* txn */ - sessiondata.InternalExecutorOverride{User: sqlUsername}, + sessiondata.NodeUserSessionDataOverride, "SELECT bundle_chunks FROM system.statement_diagnostics WHERE id=$1 AND bundle_chunks IS NOT NULL", id, ) @@ -2625,7 +2628,7 @@ func (s *adminServer) getStatementBundle(ctx context.Context, id int64, w http.R for _, chunkID := range chunkIDs { chunkRow, err := s.internalExecutor.QueryRowEx( ctx, "admin-stmt-bundle", nil, /* txn */ - sessiondata.InternalExecutorOverride{User: sqlUsername}, + sessiondata.NodeUserSessionDataOverride, "SELECT data FROM system.statement_bundle_chunks WHERE id=$1", chunkID, ) diff --git a/pkg/server/application_api/stmtdiag_test.go b/pkg/server/application_api/stmtdiag_test.go index 637ab1ef00b4..0ff3e3b3ca91 100644 --- a/pkg/server/application_api/stmtdiag_test.go +++ b/pkg/server/application_api/stmtdiag_test.go @@ -13,6 +13,7 @@ package application_api_test import ( "context" "fmt" + "net/http" "testing" "time" @@ -34,39 +35,58 @@ func TestAdminAPIStatementDiagnosticsBundle(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) - s, db, _ := serverutils.StartServer(t, base.TestServerArgs{}) + s := serverutils.StartServerOnly(t, base.TestServerArgs{}) defer s.Stopper().Stop(context.Background()) - ts := s.ApplicationLayer() + conn := sqlutils.MakeSQLRunner(ts.SQLConn(t)) query := "EXPLAIN ANALYZE (DEBUG) SELECT 'secret'" - _, err := db.Exec(query) - require.NoError(t, err) + conn.Exec(t, query) query = "SELECT id FROM system.statement_diagnostics LIMIT 1" - idRow, err := db.Query(query) - require.NoError(t, err) + idRow := conn.Query(t, query) var diagnosticRow string if idRow.Next() { - err = idRow.Scan(&diagnosticRow) + err := idRow.Scan(&diagnosticRow) require.NoError(t, err) } else { t.Fatal("no results") } - client, err := ts.GetAuthenticatedHTTPClient(false, serverutils.SingleTenantSession) - require.NoError(t, err) - resp, err := client.Get(ts.AdminURL().WithPath("/_admin/v1/stmtbundle/" + diagnosticRow).String()) - require.NoError(t, err) - defer resp.Body.Close() - require.Equal(t, 500, resp.StatusCode) + reqBundle := func(isAdmin bool, expectedStatusCode int) { + client, err := ts.GetAuthenticatedHTTPClient(isAdmin, serverutils.SingleTenantSession) + require.NoError(t, err) + resp, err := client.Get(ts.AdminURL().WithPath("/_admin/v1/stmtbundle/" + diagnosticRow).String()) + require.NoError(t, err) + defer func() { + err := resp.Body.Close() + require.NoError(t, err) + }() + require.Equal(t, expectedStatusCode, resp.StatusCode) + } - adminClient, err := ts.GetAuthenticatedHTTPClient(true, serverutils.SingleTenantSession) - require.NoError(t, err) - adminResp, err := adminClient.Get(ts.AdminURL().WithPath("/_admin/v1/stmtbundle/" + diagnosticRow).String()) - require.NoError(t, err) - defer adminResp.Body.Close() - require.Equal(t, 200, adminResp.StatusCode) + t.Run("with admin role", func(t *testing.T) { + reqBundle(true, http.StatusOK) + }) + + t.Run("no permissions", func(t *testing.T) { + reqBundle(false, http.StatusForbidden) + }) + + t.Run("with VIEWACTIVITYREDACTED role", func(t *testing.T) { + // VIEWACTIVITYREDACTED cannot download bundles due to PII. + // This will be revisited once we allow requesting and downloading redacted bundles. + conn.Exec(t, fmt.Sprintf("GRANT SYSTEM VIEWACTIVITYREDACTED TO %s", apiconstants.TestingUserNameNoAdmin().Normalized())) + reqBundle(false, http.StatusForbidden) + conn.Exec(t, fmt.Sprintf("REVOKE SYSTEM VIEWACTIVITYREDACTED FROM %s", apiconstants.TestingUserNameNoAdmin().Normalized())) + }) + + t.Run("with VIEWACTIVITY role", func(t *testing.T) { + // VIEWACTIVITY users can download bundles. + conn.Exec(t, fmt.Sprintf("GRANT SYSTEM VIEWACTIVITY TO %s", apiconstants.TestingUserNameNoAdmin().Normalized())) + reqBundle(false, http.StatusOK) + conn.Exec(t, fmt.Sprintf("REVOKE SYSTEM VIEWACTIVITY FROM %s", apiconstants.TestingUserNameNoAdmin().Normalized())) + }) } func TestCreateStatementDiagnosticsReport(t *testing.T) {