Explore: Check for RBAC permissions when hitting query history endpoints (#91156)

* Check for RBAC permissions when hitting query history endpoints; extract checking logic into a middleware

* Fix lint errors

* Fix test

* Use permissions for patch path; rename callback handler

pkg/services/queryhistory/api.go

@@ -7,6 +7,7 @@ import (
 	"github.com/grafana/grafana/pkg/api/response"
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/middleware"
+	ac "github.com/grafana/grafana/pkg/services/accesscontrol"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/util"
@@ -15,15 +16,27 @@ import (
 
 func (s *QueryHistoryService) registerAPIEndpoints() {
 	s.RouteRegister.Group("/api/query-history", func(entities routing.RouteRegister) {
-		entities.Post("/", middleware.ReqSignedIn, routing.Wrap(s.createHandler))
+		entities.Post("/", middleware.ReqSignedIn, routing.Wrap(s.permissionsMiddleware(s.createHandler, "Failed to create query history")))
-		entities.Get("/", middleware.ReqSignedIn, routing.Wrap(s.searchHandler))
+		entities.Get("/", middleware.ReqSignedIn, routing.Wrap(s.permissionsMiddleware(s.searchHandler, "Failed to get query history")))
-		entities.Delete("/:uid", middleware.ReqSignedIn, routing.Wrap(s.deleteHandler))
+		entities.Delete("/:uid", middleware.ReqSignedIn, routing.Wrap(s.permissionsMiddleware(s.deleteHandler, "Failed to delete query history")))
-		entities.Post("/star/:uid", middleware.ReqSignedIn, routing.Wrap(s.starHandler))
+		entities.Post("/star/:uid", middleware.ReqSignedIn, routing.Wrap(s.permissionsMiddleware(s.starHandler, "Failed to star query history")))
-		entities.Delete("/star/:uid", middleware.ReqSignedIn, routing.Wrap(s.unstarHandler))
+		entities.Delete("/star/:uid", middleware.ReqSignedIn, routing.Wrap(s.permissionsMiddleware(s.unstarHandler, "Failed to unstar query history")))
-		entities.Patch("/:uid", middleware.ReqSignedIn, routing.Wrap(s.patchCommentHandler))
+		entities.Patch("/:uid", middleware.ReqSignedIn, routing.Wrap(s.permissionsMiddleware(s.patchCommentHandler, "Failed to update comment of query in query history")))
 	})
 }
 
+type CallbackHandler func(c *contextmodel.ReqContext) response.Response
+
+func (s *QueryHistoryService) permissionsMiddleware(handler CallbackHandler, errorMessage string) CallbackHandler {
+	return func(c *contextmodel.ReqContext) response.Response {
+		hasAccess := ac.HasAccess(s.accessControl, c)
+		if c.GetOrgRole() == org.RoleViewer && !s.Cfg.ViewersCanEdit && !hasAccess(ac.EvalPermission(ac.ActionDatasourcesExplore)) {
+			return response.Error(http.StatusUnauthorized, errorMessage, nil)
+		}
+		return handler(c)
+	}
+}
+
 // swagger:route POST /query-history query_history createQuery
 //
 // Add query to query history.
@@ -36,10 +49,6 @@ func (s *QueryHistoryService) registerAPIEndpoints() {
 // 401: unauthorisedError
 // 500: internalServerError
 func (s *QueryHistoryService) createHandler(c *contextmodel.ReqContext) response.Response {
-	if c.GetOrgRole() == org.RoleViewer && !s.Cfg.ViewersCanEdit {
-		return response.Error(http.StatusUnauthorized, "Failed to create query history", nil)
-	}
-
 	cmd := CreateQueryInQueryHistoryCommand{}
 	if err := web.Bind(c.Req, &cmd); err != nil {
 		return response.Error(http.StatusBadRequest, "bad request data", err)
@@ -66,10 +75,6 @@ func (s *QueryHistoryService) createHandler(c *contextmodel.ReqContext) response
 // 401: unauthorisedError
 // 500: internalServerError
 func (s *QueryHistoryService) searchHandler(c *contextmodel.ReqContext) response.Response {
-	if c.GetOrgRole() == org.RoleViewer && !s.Cfg.ViewersCanEdit {
-		return response.Error(http.StatusUnauthorized, "Failed to get query history", nil)
-	}
-
 	timeRange := gtime.NewTimeRange(c.Query("from"), c.Query("to"))
 
 	query := SearchInQueryHistoryQuery{
@@ -102,10 +107,6 @@ func (s *QueryHistoryService) searchHandler(c *contextmodel.ReqContext) response
 // 401: unauthorisedError
 // 500: internalServerError
 func (s *QueryHistoryService) deleteHandler(c *contextmodel.ReqContext) response.Response {
-	if c.GetOrgRole() == org.RoleViewer && !s.Cfg.ViewersCanEdit {
-		return response.Error(http.StatusUnauthorized, "Failed to delete query history", nil)
-	}
-
 	queryUID := web.Params(c.Req)[":uid"]
 	if len(queryUID) > 0 && !util.IsValidShortUID(queryUID) {
 		return response.Error(http.StatusNotFound, "Query in query history not found", nil)
@@ -163,9 +164,6 @@ func (s *QueryHistoryService) patchCommentHandler(c *contextmodel.ReqContext) re
 // 401: unauthorisedError
 // 500: internalServerError
 func (s *QueryHistoryService) starHandler(c *contextmodel.ReqContext) response.Response {
-	if c.GetOrgRole() == org.RoleViewer && !s.Cfg.ViewersCanEdit {
-		return response.Error(http.StatusUnauthorized, "Failed to star query history", nil)
-	}
 	queryUID := web.Params(c.Req)[":uid"]
 	if len(queryUID) > 0 && !util.IsValidShortUID(queryUID) {
 		return response.Error(http.StatusNotFound, "Query in query history not found", nil)
@@ -190,9 +188,6 @@ func (s *QueryHistoryService) starHandler(c *contextmodel.ReqContext) response.R
 // 401: unauthorisedError
 // 500: internalServerError
 func (s *QueryHistoryService) unstarHandler(c *contextmodel.ReqContext) response.Response {
-	if c.GetOrgRole() == org.RoleViewer && !s.Cfg.ViewersCanEdit {
-		return response.Error(http.StatusUnauthorized, "Failed to unstar query history", nil)
-	}
 	queryUID := web.Params(c.Req)[":uid"]
 	if len(queryUID) > 0 && !util.IsValidShortUID(queryUID) {
 		return response.Error(http.StatusNotFound, "Query in query history not found", nil)

pkg/services/queryhistory/queryhistory.go

@@ -7,17 +7,19 @@ import (
 	"github.com/grafana/grafana/pkg/api/routing"
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/log"
+	ac "github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/setting"
 )
 
-func ProvideService(cfg *setting.Cfg, sqlStore db.DB, routeRegister routing.RouteRegister) *QueryHistoryService {
+func ProvideService(cfg *setting.Cfg, sqlStore db.DB, routeRegister routing.RouteRegister, accessControl ac.AccessControl) *QueryHistoryService {
 	s := &QueryHistoryService{
 		store:         sqlStore,
 		Cfg:           cfg,
 		RouteRegister: routeRegister,
 		log:           log.New("query-history"),
 		now:           time.Now,
+		accessControl: accessControl,
 	}
 
 	// Register routes only when query history is enabled
@@ -45,6 +47,7 @@ type QueryHistoryService struct {
 	RouteRegister routing.RouteRegister
 	log           log.Logger
 	now           func() time.Time
+	accessControl ac.AccessControl
 }
 
 func (s QueryHistoryService) CreateQueryInQueryHistory(ctx context.Context, user *user.SignedInUser, cmd CreateQueryInQueryHistoryCommand) (QueryHistoryDTO, error) {

pkg/services/queryhistory/queryhistory_create_test.go

@@ -12,7 +12,7 @@ func TestIntegrationCreateQueryInQueryHistory(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")
 	}
-	testScenario(t, "When users tries to create query in query history it should succeed", false,
+	testScenario(t, "When users tries to create query in query history it should succeed", true, true,
 		func(t *testing.T, sc scenarioContext) {
 			command := CreateQueryInQueryHistoryCommand{
 				DatasourceUID: "NCzh67i",
@@ -21,7 +21,25 @@ func TestIntegrationCreateQueryInQueryHistory(t *testing.T) {
 				}),
 			}
 			sc.reqContext.Req.Body = mockRequestBody(command)
-			resp := sc.service.createHandler(sc.reqContext)
+			permissionsMiddlewareCallback := sc.service.permissionsMiddleware(sc.service.createHandler, "Failed to create query history")
+
+			resp := permissionsMiddlewareCallback(sc.reqContext)
 			require.Equal(t, 200, resp.Status())
 		})
+
+	testScenario(t, "When users tries to create query in query history without permissions it should fail", true, false,
+		func(t *testing.T, sc scenarioContext) {
+			command := CreateQueryInQueryHistoryCommand{
+				DatasourceUID: "NCzh67i",
+				Queries: simplejson.NewFromAny(map[string]any{
+					"expr": "test",
+				}),
+			}
+
+			sc.reqContext.Req.Body = mockRequestBody(command)
+			permissionsMiddlewareCallback := sc.service.permissionsMiddleware(sc.service.createHandler, "Failed to create query history")
+
+			resp := permissionsMiddlewareCallback(sc.reqContext)
+			require.Equal(t, 401, resp.Status())
+		})
 }

pkg/services/queryhistory/queryhistory_search_test.go

@@ -12,7 +12,7 @@ func TestIntegrationGetQueriesFromQueryHistory(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test")
 	}
-	testScenario(t, "When users tries to get query in empty query history, it should return empty result", false,
+	testScenario(t, "When users tries to get query in empty query history, it should return empty result", false, false,
 		func(t *testing.T, sc scenarioContext) {
 			sc.reqContext.Req.Form.Add("datasourceUid", "test")
 			resp := sc.service.searchHandler(sc.reqContext)
@@ -262,10 +262,11 @@ func TestIntegrationGetQueriesFromQueryHistory(t *testing.T) {
 			require.Equal(t, 0, response.Result.TotalCount)
 		})
 
-	testScenario(t, "When user is viewer, return 401", true,
+	testScenario(t, "When user is viewer and has no RBAC permissions, return 401", true, false,
 		func(t *testing.T, sc scenarioContext) {
 			sc.reqContext.Req.Form.Add("datasourceUid", "test")
-			resp := sc.service.searchHandler(sc.reqContext)
+			permissionsMiddlewareCallback := sc.service.permissionsMiddleware(sc.service.searchHandler, "Failed to get query history")
+			resp := permissionsMiddlewareCallback(sc.reqContext)
 			require.Equal(t, 401, resp.Status())
 		})
 

pkg/services/queryhistory/queryhistory_test.go

@@ -17,6 +17,7 @@ import (
 	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/infra/tracing"
+	accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/org/orgimpl"
@@ -48,7 +49,7 @@ type scenarioContext struct {
 	initialResult QueryHistoryResponse
 }
 
-func testScenario(t *testing.T, desc string, isViewer bool, fn func(t *testing.T, sc scenarioContext)) {
+func testScenario(t *testing.T, desc string, isViewer bool, hasDatasourceExplorePermission bool, fn func(t *testing.T, sc scenarioContext)) {
 	t.Helper()
 
 	t.Run(desc, func(t *testing.T) {
@@ -59,9 +60,10 @@ func testScenario(t *testing.T, desc string, isViewer bool, fn func(t *testing.T
 		ctx.Req.Header.Add("Content-Type", "application/json")
 		sqlStore, cfg := db.InitTestDBWithCfg(t)
 		service := QueryHistoryService{
-			Cfg:   setting.NewCfg(),
+			Cfg:           setting.NewCfg(),
-			store: sqlStore,
+			store:         sqlStore,
-			now:   time.Now,
+			now:           time.Now,
+			accessControl: accesscontrolmock.New(),
 		}
 		service.Cfg.QueryHistoryEnabled = true
 		quotaService := quotatest.New(false, nil)
@@ -80,14 +82,22 @@ func testScenario(t *testing.T, desc string, isViewer bool, fn func(t *testing.T
 			role = org.RoleEditor
 		}
 
+		permissions := make(map[int64]map[string][]string)
+
+		if hasDatasourceExplorePermission {
+			permissions[testUserID] = make(map[string][]string)
+			permissions[testUserID]["datasources:explore"] = []string{}
+		}
+
 		usr := user.SignedInUser{
-			UserID:     testUserID,
+			UserID:      testUserID,
-			Name:       "Signed In User",
+			Name:        "Signed In User",
-			Login:      "signed_in_user",
+			Login:       "signed_in_user",
-			Email:      "signed.in.user@test.com",
+			Email:       "signed.in.user@test.com",
-			OrgID:      testOrgID,
+			OrgID:       testOrgID,
-			OrgRole:    role,
+			OrgRole:     role,
-			LastSeenAt: service.now(),
+			LastSeenAt:  service.now(),
+			Permissions: permissions,
 		}
 
 		_, err = usrSvc.Create(context.Background(), &user.CreateUserCommand{
@@ -113,7 +123,7 @@ func testScenario(t *testing.T, desc string, isViewer bool, fn func(t *testing.T
 func testScenarioWithQueryInQueryHistory(t *testing.T, desc string, fn func(t *testing.T, sc scenarioContext)) {
 	t.Helper()
 
-	testScenario(t, desc, false, func(t *testing.T, sc scenarioContext) {
+	testScenario(t, desc, false, false, func(t *testing.T, sc scenarioContext) {
 		command := CreateQueryInQueryHistoryCommand{
 			DatasourceUID: testDsUID1,
 			Queries: simplejson.NewFromAny([]interface{}{
@@ -132,7 +142,7 @@ func testScenarioWithQueryInQueryHistory(t *testing.T, desc string, fn func(t *t
 func testScenarioWithMultipleQueriesInQueryHistory(t *testing.T, desc string, fn func(t *testing.T, sc scenarioContext)) {
 	t.Helper()
 
-	testScenario(t, desc, false, func(t *testing.T, sc scenarioContext) {
+	testScenario(t, desc, false, false, func(t *testing.T, sc scenarioContext) {
 		start := time.Now().Add(-3 * time.Second)
 		sc.service.now = func() time.Time { return start }
 		command1 := CreateQueryInQueryHistoryCommand{
@@ -193,7 +203,7 @@ func testScenarioWithMultipleQueriesInQueryHistory(t *testing.T, desc string, fn
 func testScenarioWithMixedQueriesInQueryHistory(t *testing.T, desc string, fn func(t *testing.T, sc scenarioContext)) {
 	t.Helper()
 
-	testScenario(t, desc, false, func(t *testing.T, sc scenarioContext) {
+	testScenario(t, desc, false, false, func(t *testing.T, sc scenarioContext) {
 		start := time.Now()
 		sc.service.now = func() time.Time { return start }
 		command1 := CreateQueryInQueryHistoryCommand{