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

server: allow VIEWACTIVITY users access to /_admin/v1/stmtbundle #128938

Merged
merged 1 commit into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions pkg/server/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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)
})
Expand Down Expand Up @@ -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,
)
Expand All @@ -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,
)
Expand Down
58 changes: 39 additions & 19 deletions pkg/server/application_api/stmtdiag_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package application_api_test
import (
"context"
"fmt"
"net/http"
"testing"
"time"

Expand All @@ -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) {
Expand Down
Loading