From 518a0d0458e48a27e43c519630a41caffcd19e59 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 23 Sep 2021 17:43:32 +0200 Subject: [PATCH] Chore: Propagate context for dashboard guardian (#39201) Require guardian.New to take context.Context as first argument. Migrates the GetDashboardAclInfoListQuery to be dispatched using context. Ref #36734 Co-authored-by: Emil Tullstedt Co-authored-by: sam boyer --- pkg/api/acl.go | 11 ++++-- pkg/api/alerting.go | 2 +- pkg/api/annotations.go | 2 +- pkg/api/dashboard.go | 14 +++---- pkg/api/dashboard_permission.go | 6 +-- pkg/api/dashboard_permission_test.go | 10 +++-- pkg/api/dashboard_snapshot.go | 2 +- pkg/api/folder.go | 8 ++-- pkg/api/folder_permission.go | 9 +++-- pkg/api/folder_permission_test.go | 11 ++++-- pkg/dashboards/ifaces.go | 8 +++- pkg/services/dashboards/acl_service.go | 2 +- pkg/services/dashboards/dashboard_service.go | 4 +- pkg/services/dashboards/folder_service.go | 8 ++-- pkg/services/guardian/guardian.go | 10 +++-- pkg/services/guardian/guardian_test.go | 7 ++-- pkg/services/guardian/guardian_util_test.go | 7 ++-- pkg/services/libraryelements/guard.go | 2 +- pkg/services/live/features/dashboard.go | 4 +- pkg/services/ngalert/store/alert_rule.go | 2 +- pkg/services/sqlstore/dashboard_acl.go | 41 ++++++++++++-------- pkg/services/sqlstore/dashboard_acl_test.go | 23 +++++------ pkg/services/sqlstore/org_test.go | 4 +- pkg/services/sqlstore/sqlstore.go | 1 + pkg/services/sqlstore/team_test.go | 2 +- pkg/services/sqlstore/user_test.go | 4 +- 26 files changed, 117 insertions(+), 87 deletions(-) diff --git a/pkg/api/acl.go b/pkg/api/acl.go index 6c839a2f11a..eae0cf21aeb 100644 --- a/pkg/api/acl.go +++ b/pkg/api/acl.go @@ -1,10 +1,15 @@ package api -import "github.com/grafana/grafana/pkg/models" +import ( + "context" + + "github.com/grafana/grafana/pkg/dashboards" + "github.com/grafana/grafana/pkg/models" +) // updateDashboardACL updates a dashboard's ACL items. // // Stubbable by tests. -var updateDashboardACL = func(hs *HTTPServer, dashID int64, items []*models.DashboardAcl) error { - return hs.SQLStore.UpdateDashboardACL(dashID, items) +var updateDashboardACL = func(ctx context.Context, s dashboards.Store, dashID int64, items []*models.DashboardAcl) error { + return s.UpdateDashboardACLCtx(ctx, dashID, items) } diff --git a/pkg/api/alerting.go b/pkg/api/alerting.go index e4181509df0..f2469eac478 100644 --- a/pkg/api/alerting.go +++ b/pkg/api/alerting.go @@ -462,7 +462,7 @@ func PauseAlert(c *models.ReqContext, dto dtos.PauseAlertCommand) response.Respo return response.Error(500, "Get Alert failed", err) } - guardian := guardian.New(query.Result.DashboardId, c.OrgId, c.SignedInUser) + guardian := guardian.New(c.Req.Context(), query.Result.DashboardId, c.OrgId, c.SignedInUser) if canEdit, err := guardian.CanEdit(); err != nil || !canEdit { if err != nil { return response.Error(500, "Error while checking permissions for Alert", err) diff --git a/pkg/api/annotations.go b/pkg/api/annotations.go index cb3154e19dc..031fd55f2ca 100644 --- a/pkg/api/annotations.go +++ b/pkg/api/annotations.go @@ -265,7 +265,7 @@ func canSaveByDashboardID(c *models.ReqContext, dashboardID int64) (bool, error) } if dashboardID != 0 { - guard := guardian.New(dashboardID, c.OrgId, c.SignedInUser) + guard := guardian.New(c.Req.Context(), dashboardID, c.OrgId, c.SignedInUser) if canEdit, err := guard.CanEdit(); err != nil || !canEdit { return false, err } diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index bd4744aa781..850d1431b8d 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -91,7 +91,7 @@ func (hs *HTTPServer) GetDashboard(c *models.ReqContext) response.Response { } } - guardian := guardian.New(dash.Id, c.OrgId, c.SignedInUser) + guardian := guardian.New(c.Req.Context(), dash.Id, c.OrgId, c.SignedInUser) if canView, err := guardian.CanView(); err != nil || !canView { return dashboardGuardianResponse(err) } @@ -238,7 +238,7 @@ func (hs *HTTPServer) deleteDashboard(c *models.ReqContext) response.Response { return rsp } - guardian := guardian.New(dash.Id, c.OrgId, c.SignedInUser) + guardian := guardian.New(c.Req.Context(), dash.Id, c.OrgId, c.SignedInUser) if canSave, err := guardian.CanSave(); err != nil || !canSave { return dashboardGuardianResponse(err) } @@ -508,7 +508,7 @@ func (hs *HTTPServer) addGettingStartedPanelToHomeDashboard(c *models.ReqContext func GetDashboardVersions(c *models.ReqContext) response.Response { dashID := c.ParamsInt64(":dashboardId") - guardian := guardian.New(dashID, c.OrgId, c.SignedInUser) + guardian := guardian.New(c.Req.Context(), dashID, c.OrgId, c.SignedInUser) if canSave, err := guardian.CanSave(); err != nil || !canSave { return dashboardGuardianResponse(err) } @@ -547,7 +547,7 @@ func GetDashboardVersions(c *models.ReqContext) response.Response { func GetDashboardVersion(c *models.ReqContext) response.Response { dashID := c.ParamsInt64(":dashboardId") - guardian := guardian.New(dashID, c.OrgId, c.SignedInUser) + guardian := guardian.New(c.Req.Context(), dashID, c.OrgId, c.SignedInUser) if canSave, err := guardian.CanSave(); err != nil || !canSave { return dashboardGuardianResponse(err) } @@ -584,13 +584,13 @@ func GetDashboardVersion(c *models.ReqContext) response.Response { // POST /api/dashboards/calculate-diff performs diffs on two dashboards func CalculateDashboardDiff(c *models.ReqContext, apiOptions dtos.CalculateDiffOptions) response.Response { - guardianBase := guardian.New(apiOptions.Base.DashboardId, c.OrgId, c.SignedInUser) + guardianBase := guardian.New(c.Req.Context(), apiOptions.Base.DashboardId, c.OrgId, c.SignedInUser) if canSave, err := guardianBase.CanSave(); err != nil || !canSave { return dashboardGuardianResponse(err) } if apiOptions.Base.DashboardId != apiOptions.New.DashboardId { - guardianNew := guardian.New(apiOptions.New.DashboardId, c.OrgId, c.SignedInUser) + guardianNew := guardian.New(c.Req.Context(), apiOptions.New.DashboardId, c.OrgId, c.SignedInUser) if canSave, err := guardianNew.CanSave(); err != nil || !canSave { return dashboardGuardianResponse(err) } @@ -633,7 +633,7 @@ func (hs *HTTPServer) RestoreDashboardVersion(c *models.ReqContext, apiCmd dtos. return rsp } - guardian := guardian.New(dash.Id, c.OrgId, c.SignedInUser) + guardian := guardian.New(c.Req.Context(), dash.Id, c.OrgId, c.SignedInUser) if canSave, err := guardian.CanSave(); err != nil || !canSave { return dashboardGuardianResponse(err) } diff --git a/pkg/api/dashboard_permission.go b/pkg/api/dashboard_permission.go index 4867d68e41c..c32339cf931 100644 --- a/pkg/api/dashboard_permission.go +++ b/pkg/api/dashboard_permission.go @@ -18,7 +18,7 @@ func (hs *HTTPServer) GetDashboardPermissionList(c *models.ReqContext) response. return rsp } - g := guardian.New(dashID, c.OrgId, c.SignedInUser) + g := guardian.New(c.Req.Context(), dashID, c.OrgId, c.SignedInUser) if canAdmin, err := g.CanAdmin(); err != nil || !canAdmin { return dashboardGuardianResponse(err) @@ -62,7 +62,7 @@ func (hs *HTTPServer) UpdateDashboardPermissions(c *models.ReqContext, apiCmd dt return rsp } - g := guardian.New(dashID, c.OrgId, c.SignedInUser) + g := guardian.New(c.Req.Context(), dashID, c.OrgId, c.SignedInUser) if canAdmin, err := g.CanAdmin(); err != nil || !canAdmin { return dashboardGuardianResponse(err) } @@ -99,7 +99,7 @@ func (hs *HTTPServer) UpdateDashboardPermissions(c *models.ReqContext, apiCmd dt return response.Error(403, "Cannot remove own admin permission for a folder", nil) } - if err := updateDashboardACL(hs, dashID, items); err != nil { + if err := updateDashboardACL(c.Req.Context(), hs.SQLStore, dashID, items); err != nil { if errors.Is(err, models.ErrDashboardAclInfoMissing) || errors.Is(err, models.ErrDashboardPermissionDashboardEmpty) { return response.Error(409, err.Error(), err) diff --git a/pkg/api/dashboard_permission_test.go b/pkg/api/dashboard_permission_test.go index 93e148a6148..9ff803230a9 100644 --- a/pkg/api/dashboard_permission_test.go +++ b/pkg/api/dashboard_permission_test.go @@ -6,15 +6,17 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/dashboards" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/setting" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestDashboardPermissionAPIEndpoint(t *testing.T) { @@ -342,7 +344,7 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { updateDashboardACL = origUpdateDashboardACL }) var gotItems []*models.DashboardAcl - updateDashboardACL = func(hs *HTTPServer, folderID int64, items []*models.DashboardAcl) error { + updateDashboardACL = func(_ context.Context, _ dashboards.Store, folderID int64, items []*models.DashboardAcl) error { gotItems = items return nil } @@ -368,7 +370,7 @@ func callUpdateDashboardPermissions(t *testing.T, sc *scenarioContext) { t.Cleanup(func() { updateDashboardACL = origUpdateDashboardACL }) - updateDashboardACL = func(hs *HTTPServer, dashID int64, items []*models.DashboardAcl) error { + updateDashboardACL = func(_ context.Context, _ dashboards.Store, dashID int64, items []*models.DashboardAcl) error { return nil } diff --git a/pkg/api/dashboard_snapshot.go b/pkg/api/dashboard_snapshot.go index b108a0b1141..434e85a46c4 100644 --- a/pkg/api/dashboard_snapshot.go +++ b/pkg/api/dashboard_snapshot.go @@ -254,7 +254,7 @@ func DeleteDashboardSnapshot(c *models.ReqContext) response.Response { dashboardID := query.Result.Dashboard.Get("id").MustInt64() - guardian := guardian.New(dashboardID, c.OrgId, c.SignedInUser) + guardian := guardian.New(c.Req.Context(), dashboardID, c.OrgId, c.SignedInUser) canEdit, err := guardian.CanEdit() if err != nil { return response.Error(500, "Error while checking permissions for snapshot", err) diff --git a/pkg/api/folder.go b/pkg/api/folder.go index d6e4e60d3c9..b6e25130285 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -44,7 +44,7 @@ func (hs *HTTPServer) GetFolderByUID(c *models.ReqContext) response.Response { return apierrors.ToFolderErrorResponse(err) } - g := guardian.New(folder.Id, c.OrgId, c.SignedInUser) + g := guardian.New(c.Req.Context(), folder.Id, c.OrgId, c.SignedInUser) return response.JSON(200, toFolderDto(c.Req.Context(), g, folder)) } @@ -55,7 +55,7 @@ func (hs *HTTPServer) GetFolderByID(c *models.ReqContext) response.Response { return apierrors.ToFolderErrorResponse(err) } - g := guardian.New(folder.Id, c.OrgId, c.SignedInUser) + g := guardian.New(c.Req.Context(), folder.Id, c.OrgId, c.SignedInUser) return response.JSON(200, toFolderDto(c.Req.Context(), g, folder)) } @@ -73,7 +73,7 @@ func (hs *HTTPServer) CreateFolder(c *models.ReqContext, cmd models.CreateFolder } } - g := guardian.New(folder.Id, c.OrgId, c.SignedInUser) + g := guardian.New(c.Req.Context(), folder.Id, c.OrgId, c.SignedInUser) return response.JSON(200, toFolderDto(c.Req.Context(), g, folder)) } @@ -84,7 +84,7 @@ func (hs *HTTPServer) UpdateFolder(c *models.ReqContext, cmd models.UpdateFolder return apierrors.ToFolderErrorResponse(err) } - g := guardian.New(cmd.Result.Id, c.OrgId, c.SignedInUser) + g := guardian.New(c.Req.Context(), cmd.Result.Id, c.OrgId, c.SignedInUser) return response.JSON(200, toFolderDto(c.Req.Context(), g, cmd.Result)) } diff --git a/pkg/api/folder_permission.go b/pkg/api/folder_permission.go index 2f5e68faef2..aa6fa4736a6 100644 --- a/pkg/api/folder_permission.go +++ b/pkg/api/folder_permission.go @@ -4,6 +4,8 @@ import ( "errors" "time" + macaron "gopkg.in/macaron.v1" + "github.com/grafana/grafana/pkg/api/apierrors" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" @@ -11,7 +13,6 @@ import ( "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/util" - macaron "gopkg.in/macaron.v1" ) func (hs *HTTPServer) GetFolderPermissionList(c *models.ReqContext) response.Response { @@ -22,7 +23,7 @@ func (hs *HTTPServer) GetFolderPermissionList(c *models.ReqContext) response.Res return apierrors.ToFolderErrorResponse(err) } - g := guardian.New(folder.Id, c.OrgId, c.SignedInUser) + g := guardian.New(c.Req.Context(), folder.Id, c.OrgId, c.SignedInUser) if canAdmin, err := g.CanAdmin(); err != nil || !canAdmin { return apierrors.ToFolderErrorResponse(models.ErrFolderAccessDenied) @@ -69,7 +70,7 @@ func (hs *HTTPServer) UpdateFolderPermissions(c *models.ReqContext, apiCmd dtos. return apierrors.ToFolderErrorResponse(err) } - g := guardian.New(folder.Id, c.OrgId, c.SignedInUser) + g := guardian.New(c.Req.Context(), folder.Id, c.OrgId, c.SignedInUser) canAdmin, err := g.CanAdmin() if err != nil { return apierrors.ToFolderErrorResponse(err) @@ -112,7 +113,7 @@ func (hs *HTTPServer) UpdateFolderPermissions(c *models.ReqContext, apiCmd dtos. return response.Error(403, "Cannot remove own admin permission for a folder", nil) } - if err := updateDashboardACL(hs, folder.Id, items); err != nil { + if err := updateDashboardACL(c.Req.Context(), hs.SQLStore, folder.Id, items); err != nil { if errors.Is(err, models.ErrDashboardAclInfoMissing) { err = models.ErrFolderAclInfoMissing } diff --git a/pkg/api/folder_permission_test.go b/pkg/api/folder_permission_test.go index bcbcbeb4b5a..e5eee71ce06 100644 --- a/pkg/api/folder_permission_test.go +++ b/pkg/api/folder_permission_test.go @@ -1,20 +1,23 @@ package api import ( + "context" "encoding/json" "fmt" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/bus" + dashboardifaces "github.com/grafana/grafana/pkg/dashboards" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/guardian" "github.com/grafana/grafana/pkg/setting" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestFolderPermissionAPIEndpoint(t *testing.T) { @@ -360,7 +363,7 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) { updateDashboardACL = origUpdateDashboardACL }) var gotItems []*models.DashboardAcl - updateDashboardACL = func(hs *HTTPServer, dashID int64, items []*models.DashboardAcl) error { + updateDashboardACL = func(_ context.Context, _ dashboardifaces.Store, _ int64, items []*models.DashboardAcl) error { gotItems = items return nil } @@ -385,7 +388,7 @@ func callUpdateFolderPermissions(t *testing.T, sc *scenarioContext) { t.Cleanup(func() { updateDashboardACL = origUpdateDashboardACL }) - updateDashboardACL = func(hs *HTTPServer, dashID int64, items []*models.DashboardAcl) error { + updateDashboardACL = func(_ context.Context, _ dashboardifaces.Store, dashID int64, items []*models.DashboardAcl) error { return nil } diff --git a/pkg/dashboards/ifaces.go b/pkg/dashboards/ifaces.go index 101cd362676..4e524d58d1a 100644 --- a/pkg/dashboards/ifaces.go +++ b/pkg/dashboards/ifaces.go @@ -1,6 +1,10 @@ package dashboards -import "github.com/grafana/grafana/pkg/models" +import ( + "context" + + "github.com/grafana/grafana/pkg/models" +) // Store is a dashboard store. type Store interface { @@ -12,7 +16,7 @@ type Store interface { GetProvisionedDashboardData(name string) ([]*models.DashboardProvisioning, error) SaveProvisionedDashboard(cmd models.SaveDashboardCommand, provisioning *models.DashboardProvisioning) (*models.Dashboard, error) SaveDashboard(cmd models.SaveDashboardCommand) (*models.Dashboard, error) - UpdateDashboardACL(uid int64, items []*models.DashboardAcl) error + UpdateDashboardACLCtx(ctx context.Context, uid int64, items []*models.DashboardAcl) error // SaveAlerts saves dashboard alerts. SaveAlerts(dashID int64, alerts []*models.Alert) error } diff --git a/pkg/services/dashboards/acl_service.go b/pkg/services/dashboards/acl_service.go index f445e515e7d..0a9b7b4f024 100644 --- a/pkg/services/dashboards/acl_service.go +++ b/pkg/services/dashboards/acl_service.go @@ -43,7 +43,7 @@ func (dr *dashboardServiceImpl) MakeUserAdmin(ctx context.Context, orgID int64, ) } - if err := dr.dashboardStore.UpdateDashboardACL(dashboardID, items); err != nil { + if err := dr.dashboardStore.UpdateDashboardACLCtx(ctx, dashboardID, items); err != nil { return err } diff --git a/pkg/services/dashboards/dashboard_service.go b/pkg/services/dashboards/dashboard_service.go index 4470bd08566..e36575b1303 100644 --- a/pkg/services/dashboards/dashboard_service.go +++ b/pkg/services/dashboards/dashboard_service.go @@ -124,7 +124,7 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, } if isParentFolderChanged { - folderGuardian := guardian.New(dash.FolderId, dto.OrgId, dto.User) + folderGuardian := guardian.New(context.TODO(), dash.FolderId, dto.OrgId, dto.User) if canSave, err := folderGuardian.CanSave(); err != nil || !canSave { if err != nil { return nil, err @@ -144,7 +144,7 @@ func (dr *dashboardServiceImpl) buildSaveDashboardCommand(dto *SaveDashboardDTO, } } - guard := guardian.New(dash.GetDashboardIdForSavePermissionCheck(), dto.OrgId, dto.User) + guard := guardian.New(context.TODO(), dash.GetDashboardIdForSavePermissionCheck(), dto.OrgId, dto.User) if canSave, err := guard.CanSave(); err != nil || !canSave { if err != nil { return nil, err diff --git a/pkg/services/dashboards/folder_service.go b/pkg/services/dashboards/folder_service.go index 97cad653bc9..b1dfcfc0ad9 100644 --- a/pkg/services/dashboards/folder_service.go +++ b/pkg/services/dashboards/folder_service.go @@ -72,7 +72,7 @@ func (dr *dashboardServiceImpl) GetFolderByID(ctx context.Context, id int64) (*m return nil, toFolderError(err) } - g := guardian.New(dashFolder.Id, dr.orgId, dr.user) + g := guardian.New(ctx, dashFolder.Id, dr.orgId, dr.user) if canView, err := g.CanView(); err != nil || !canView { if err != nil { return nil, toFolderError(err) @@ -91,7 +91,7 @@ func (dr *dashboardServiceImpl) GetFolderByUID(ctx context.Context, uid string) return nil, toFolderError(err) } - g := guardian.New(dashFolder.Id, dr.orgId, dr.user) + g := guardian.New(ctx, dashFolder.Id, dr.orgId, dr.user) if canView, err := g.CanView(); err != nil || !canView { if err != nil { return nil, toFolderError(err) @@ -108,7 +108,7 @@ func (dr *dashboardServiceImpl) GetFolderByTitle(ctx context.Context, title stri return nil, toFolderError(err) } - g := guardian.New(dashFolder.Id, dr.orgId, dr.user) + g := guardian.New(ctx, dashFolder.Id, dr.orgId, dr.user) if canView, err := g.CanView(); err != nil || !canView { if err != nil { return nil, toFolderError(err) @@ -200,7 +200,7 @@ func (dr *dashboardServiceImpl) DeleteFolder(ctx context.Context, uid string, fo return nil, toFolderError(err) } - guardian := guardian.New(dashFolder.Id, dr.orgId, dr.user) + guardian := guardian.New(ctx, dashFolder.Id, dr.orgId, dr.user) if canSave, err := guardian.CanSave(); err != nil || !canSave { if err != nil { return nil, toFolderError(err) diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index 91f6868bb57..5a561bc4aaa 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -1,6 +1,7 @@ package guardian import ( + "context" "errors" "github.com/grafana/grafana/pkg/bus" @@ -40,15 +41,17 @@ type dashboardGuardianImpl struct { acl []*models.DashboardAclInfoDTO teams []*models.TeamDTO log log.Logger + ctx context.Context } // New factory for creating a new dashboard guardian instance -var New = func(dashId int64, orgId int64, user *models.SignedInUser) DashboardGuardian { +var New = func(ctx context.Context, dashId int64, orgId int64, user *models.SignedInUser) DashboardGuardian { return &dashboardGuardianImpl{ user: user, dashId: dashId, orgId: orgId, log: log.New("dashboard.permissions"), + ctx: ctx, } } @@ -201,7 +204,7 @@ func (g *dashboardGuardianImpl) GetAcl() ([]*models.DashboardAclInfoDTO, error) } query := models.GetDashboardAclInfoListQuery{DashboardID: g.dashId, OrgID: g.orgId} - if err := bus.Dispatch(&query); err != nil { + if err := bus.DispatchCtx(g.ctx, &query); err != nil { return nil, err } @@ -251,6 +254,7 @@ func (g *dashboardGuardianImpl) getTeams() ([]*models.TeamDTO, error) { } query := models.GetTeamsByUserQuery{OrgId: g.orgId, UserId: g.user.UserId} + // TODO: Use bus.DispatchCtx(g.Ctx, &query) when GetTeamsByUserQuery supports context. err := bus.Dispatch(&query) g.teams = query.Result @@ -343,7 +347,7 @@ func (g *FakeDashboardGuardian) GetHiddenACL(cfg *setting.Cfg) ([]*models.Dashbo // nolint:unused func MockDashboardGuardian(mock *FakeDashboardGuardian) { - New = func(dashId int64, orgId int64, user *models.SignedInUser) DashboardGuardian { + New = func(_ context.Context, dashId int64, orgId int64, user *models.SignedInUser) DashboardGuardian { mock.OrgId = orgId mock.DashId = dashId mock.User = user diff --git a/pkg/services/guardian/guardian_test.go b/pkg/services/guardian/guardian_test.go index 3b9fdcb4ed0..a46a198dad3 100644 --- a/pkg/services/guardian/guardian_test.go +++ b/pkg/services/guardian/guardian_test.go @@ -1,6 +1,7 @@ package guardian import ( + "context" "errors" "fmt" "runtime" @@ -701,7 +702,7 @@ func TestGuardianGetHiddenACL(t *testing.T) { UserId: 1, Login: "user1", } - g := New(dashboardID, orgID, user) + g := New(context.Background(), dashboardID, orgID, user) hiddenACL, err := g.GetHiddenACL(cfg) So(err, ShouldBeNil) @@ -717,7 +718,7 @@ func TestGuardianGetHiddenACL(t *testing.T) { Login: "user1", IsGrafanaAdmin: true, } - g := New(dashboardID, orgID, user) + g := New(context.Background(), dashboardID, orgID, user) hiddenACL, err := g.GetHiddenACL(cfg) So(err, ShouldBeNil) @@ -751,7 +752,7 @@ func TestGuardianGetAclWithoutDuplicates(t *testing.T) { UserId: 1, Login: "user1", } - g := New(dashboardID, orgID, user) + g := New(context.Background(), dashboardID, orgID, user) acl, err := g.GetACLWithoutDuplicates() require.NoError(t, err) diff --git a/pkg/services/guardian/guardian_util_test.go b/pkg/services/guardian/guardian_util_test.go index 1ac35ea1d7d..91eed18d775 100644 --- a/pkg/services/guardian/guardian_util_test.go +++ b/pkg/services/guardian/guardian_util_test.go @@ -2,6 +2,7 @@ package guardian import ( "bytes" + "context" "fmt" "strings" "testing" @@ -35,7 +36,7 @@ func orgRoleScenario(desc string, t *testing.T, role models.RoleType, fn scenari OrgId: orgID, OrgRole: role, } - guard := New(dashboardID, orgID, user) + guard := New(context.Background(), dashboardID, orgID, user) sc := &scenarioContext{ t: t, @@ -56,7 +57,7 @@ func apiKeyScenario(desc string, t *testing.T, role models.RoleType, fn scenario OrgRole: role, ApiKeyId: 10, } - guard := New(dashboardID, orgID, user) + guard := New(context.Background(), dashboardID, orgID, user) sc := &scenarioContext{ t: t, orgRoleScenario: desc, @@ -107,7 +108,7 @@ func permissionScenario(desc string, dashboardID int64, sc *scenarioContext, }) sc.permissionScenario = desc - sc.g = New(dashboardID, sc.givenUser.OrgId, sc.givenUser) + sc.g = New(context.Background(), dashboardID, sc.givenUser.OrgId, sc.givenUser) sc.givenDashboardID = dashboardID sc.givenPermissions = permissions sc.givenTeams = teams diff --git a/pkg/services/libraryelements/guard.go b/pkg/services/libraryelements/guard.go index 9745cefd887..8d483488140 100644 --- a/pkg/services/libraryelements/guard.go +++ b/pkg/services/libraryelements/guard.go @@ -39,7 +39,7 @@ func (l *LibraryElementService) requirePermissionsOnFolder(ctx context.Context, return err } - g := guardian.New(folder.Id, user.OrgId, user) + g := guardian.New(ctx, folder.Id, user.OrgId, user) canEdit, err := g.CanEdit() if err != nil { diff --git a/pkg/services/live/features/dashboard.go b/pkg/services/live/features/dashboard.go index 600f3918a67..64f186eb9ad 100644 --- a/pkg/services/live/features/dashboard.go +++ b/pkg/services/live/features/dashboard.go @@ -68,7 +68,7 @@ func (h *DashboardHandler) OnSubscribe(ctx context.Context, user *models.SignedI } dash := query.Result - guard := guardian.New(dash.Id, user.OrgId, user) + guard := guardian.New(ctx, dash.Id, user.OrgId, user) if canView, err := guard.CanView(); err != nil || !canView { return models.SubscribeReply{}, backend.SubscribeStreamStatusPermissionDenied, nil } @@ -114,7 +114,7 @@ func (h *DashboardHandler) OnPublish(ctx context.Context, user *models.SignedInU return models.PublishReply{}, backend.PublishStreamStatusNotFound, nil } - guard := guardian.New(query.Result.Id, user.OrgId, user) + guard := guardian.New(ctx, query.Result.Id, user.OrgId, user) canEdit, err := guard.CanEdit() if err != nil { return models.PublishReply{}, backend.PublishStreamStatusNotFound, fmt.Errorf("internal error") diff --git a/pkg/services/ngalert/store/alert_rule.go b/pkg/services/ngalert/store/alert_rule.go index c1ee47b8071..9804a5978c8 100644 --- a/pkg/services/ngalert/store/alert_rule.go +++ b/pkg/services/ngalert/store/alert_rule.go @@ -404,7 +404,7 @@ func (st DBstore) GetNamespaceByTitle(ctx context.Context, namespace string, org } if withCanSave { - g := guardian.New(folder.Id, orgID, user) + g := guardian.New(ctx, folder.Id, orgID, user) if canSave, err := g.CanSave(); err != nil || !canSave { if err != nil { st.Logger.Error("checking can save permission has failed", "userId", user.UserId, "username", user.Login, "namespace", namespace, "orgId", orgID, "error", err) diff --git a/pkg/services/sqlstore/dashboard_acl.go b/pkg/services/sqlstore/dashboard_acl.go index 2cf15c4fe2a..1327d8cc313 100644 --- a/pkg/services/sqlstore/dashboard_acl.go +++ b/pkg/services/sqlstore/dashboard_acl.go @@ -8,12 +8,16 @@ import ( "github.com/grafana/grafana/pkg/models" ) -func init() { - bus.AddHandler("sql", GetDashboardAclInfoList) +func (ss *SQLStore) addDashboardACLQueryAndCommandHandlers() { + bus.AddHandlerCtx("sql", ss.GetDashboardAclInfoList) } func (ss *SQLStore) UpdateDashboardACL(dashboardID int64, items []*models.DashboardAcl) error { - return ss.WithTransactionalDbSession(context.Background(), func(sess *DBSession) error { + return ss.UpdateDashboardACLCtx(context.TODO(), dashboardID, items) +} + +func (ss *SQLStore) UpdateDashboardACLCtx(ctx context.Context, dashboardID int64, items []*models.DashboardAcl) error { + return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { // delete existing items _, err := sess.Exec("DELETE FROM dashboard_acl WHERE dashboard_id=?", dashboardID) if err != nil { @@ -47,13 +51,13 @@ func (ss *SQLStore) UpdateDashboardACL(dashboardID int64, items []*models.Dashbo // 1) Permissions for the dashboard // 2) permissions for its parent folder // 3) if no specific permissions have been set for the dashboard or its parent folder then get the default permissions -func GetDashboardAclInfoList(query *models.GetDashboardAclInfoListQuery) error { - var err error +func (ss *SQLStore) GetDashboardAclInfoList(ctx context.Context, query *models.GetDashboardAclInfoListQuery) error { + outerErr := ss.WithDbSession(ctx, func(dbSession *DBSession) error { + query.Result = make([]*models.DashboardAclInfoDTO, 0) + falseStr := dialect.BooleanStr(false) - falseStr := dialect.BooleanStr(false) - - if query.DashboardID == 0 { - sql := `SELECT + if query.DashboardID == 0 { + sql := `SELECT da.id, da.org_id, da.dashboard_id, @@ -69,13 +73,13 @@ func GetDashboardAclInfoList(query *models.GetDashboardAclInfoListQuery) error { '' as title, '' as slug, '' as uid,` + - falseStr + ` AS is_folder,` + - falseStr + ` AS inherited + falseStr + ` AS is_folder,` + + falseStr + ` AS inherited FROM dashboard_acl as da WHERE da.dashboard_id = -1` - query.Result = make([]*models.DashboardAclInfoDTO, 0) - err = x.SQL(sql).Find(&query.Result) - } else { + return dbSession.SQL(sql).Find(&query.Result) + } + rawSQL := ` -- get permissions for the dashboard and its parent folder SELECT @@ -115,13 +119,16 @@ func GetDashboardAclInfoList(query *models.GetDashboardAclInfoListQuery) error { ORDER BY da.id ASC ` - query.Result = make([]*models.DashboardAclInfoDTO, 0) - err = x.SQL(rawSQL, query.OrgID, query.DashboardID).Find(&query.Result) + return dbSession.SQL(rawSQL, query.OrgID, query.DashboardID).Find(&query.Result) + }) + + if outerErr != nil { + return outerErr } for _, p := range query.Result { p.PermissionName = p.Permission.String() } - return err + return nil } diff --git a/pkg/services/sqlstore/dashboard_acl_test.go b/pkg/services/sqlstore/dashboard_acl_test.go index 13128fcb5af..24d6aea954d 100644 --- a/pkg/services/sqlstore/dashboard_acl_test.go +++ b/pkg/services/sqlstore/dashboard_acl_test.go @@ -4,6 +4,7 @@ package sqlstore import ( + "context" "testing" "github.com/grafana/grafana/pkg/models" @@ -32,7 +33,7 @@ func TestDashboardAclDataAccess(t *testing.T) { Convey("When reading folder acl should include default acl", func() { query := models.GetDashboardAclInfoListQuery{DashboardID: savedFolder.Id, OrgID: 1} - err := GetDashboardAclInfoList(&query) + err := sqlStore.GetDashboardAclInfoList(context.Background(), &query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 2) @@ -48,7 +49,7 @@ func TestDashboardAclDataAccess(t *testing.T) { Convey("When reading dashboard acl should include acl for parent folder", func() { query := models.GetDashboardAclInfoListQuery{DashboardID: childDash.Id, OrgID: 1} - err := GetDashboardAclInfoList(&query) + err := sqlStore.GetDashboardAclInfoList(context.Background(), &query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 2) @@ -69,7 +70,7 @@ func TestDashboardAclDataAccess(t *testing.T) { Convey("When reading dashboard acl should return no acl items", func() { query := models.GetDashboardAclInfoListQuery{DashboardID: childDash.Id, OrgID: 1} - err := GetDashboardAclInfoList(&query) + err := sqlStore.GetDashboardAclInfoList(context.Background(), &query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 0) @@ -88,7 +89,7 @@ func TestDashboardAclDataAccess(t *testing.T) { Convey("When reading dashboard acl should include acl for parent folder", func() { query := models.GetDashboardAclInfoListQuery{DashboardID: childDash.Id, OrgID: 1} - err := GetDashboardAclInfoList(&query) + err := sqlStore.GetDashboardAclInfoList(context.Background(), &query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 1) @@ -107,7 +108,7 @@ func TestDashboardAclDataAccess(t *testing.T) { Convey("When reading dashboard acl should include acl for parent folder and child", func() { query := models.GetDashboardAclInfoListQuery{OrgID: 1, DashboardID: childDash.Id} - err := GetDashboardAclInfoList(&query) + err := sqlStore.GetDashboardAclInfoList(context.Background(), &query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 2) @@ -131,7 +132,7 @@ func TestDashboardAclDataAccess(t *testing.T) { Convey("When reading dashboard acl should include default acl for parent folder and the child acl", func() { query := models.GetDashboardAclInfoListQuery{OrgID: 1, DashboardID: childDash.Id} - err := GetDashboardAclInfoList(&query) + err := sqlStore.GetDashboardAclInfoList(context.Background(), &query) So(err, ShouldBeNil) defaultPermissionsId := -1 @@ -157,7 +158,7 @@ func TestDashboardAclDataAccess(t *testing.T) { So(err, ShouldBeNil) q1 := &models.GetDashboardAclInfoListQuery{DashboardID: savedFolder.Id, OrgID: 1} - err = GetDashboardAclInfoList(q1) + err = sqlStore.GetDashboardAclInfoList(context.Background(), q1) So(err, ShouldBeNil) So(q1.Result[0].DashboardId, ShouldEqual, savedFolder.Id) @@ -172,7 +173,7 @@ func TestDashboardAclDataAccess(t *testing.T) { So(err, ShouldBeNil) q3 := &models.GetDashboardAclInfoListQuery{DashboardID: savedFolder.Id, OrgID: 1} - err = GetDashboardAclInfoList(q3) + err = sqlStore.GetDashboardAclInfoList(context.Background(), q3) So(err, ShouldBeNil) So(len(q3.Result), ShouldEqual, 0) }) @@ -192,7 +193,7 @@ func TestDashboardAclDataAccess(t *testing.T) { So(err, ShouldBeNil) q1 := &models.GetDashboardAclInfoListQuery{DashboardID: savedFolder.Id, OrgID: 1} - err = GetDashboardAclInfoList(q1) + err = sqlStore.GetDashboardAclInfoList(context.Background(), q1) So(err, ShouldBeNil) So(q1.Result[0].DashboardId, ShouldEqual, savedFolder.Id) So(q1.Result[0].Permission, ShouldEqual, models.PERMISSION_EDIT) @@ -209,7 +210,7 @@ func TestDashboardAclDataAccess(t *testing.T) { So(err, ShouldBeNil) q3 := &models.GetDashboardAclInfoListQuery{DashboardID: savedFolder.Id, OrgID: 1} - err = GetDashboardAclInfoList(q3) + err = sqlStore.GetDashboardAclInfoList(context.Background(), q3) So(err, ShouldBeNil) So(len(q3.Result), ShouldEqual, 1) So(q3.Result[0].DashboardId, ShouldEqual, savedFolder.Id) @@ -225,7 +226,7 @@ func TestDashboardAclDataAccess(t *testing.T) { Convey("When reading dashboard acl should return default permissions", func() { query := models.GetDashboardAclInfoListQuery{DashboardID: rootFolderId, OrgID: 1} - err := GetDashboardAclInfoList(&query) + err := sqlStore.GetDashboardAclInfoList(context.Background(), &query) So(err, ShouldBeNil) So(len(query.Result), ShouldEqual, 2) diff --git a/pkg/services/sqlstore/org_test.go b/pkg/services/sqlstore/org_test.go index b94c3f6e35b..fb027d09a78 100644 --- a/pkg/services/sqlstore/org_test.go +++ b/pkg/services/sqlstore/org_test.go @@ -338,7 +338,7 @@ func TestAccountDataAccess(t *testing.T) { Convey("Should remove dependent permissions for deleted org user", func() { permQuery := &models.GetDashboardAclInfoListQuery{DashboardID: dash1.Id, OrgID: ac1.OrgId} - err = GetDashboardAclInfoList(permQuery) + err = sqlStore.GetDashboardAclInfoList(context.Background(), permQuery) So(err, ShouldBeNil) So(len(permQuery.Result), ShouldEqual, 0) @@ -346,7 +346,7 @@ func TestAccountDataAccess(t *testing.T) { Convey("Should not remove dashboard permissions for same user in another org", func() { permQuery := &models.GetDashboardAclInfoListQuery{DashboardID: dash2.Id, OrgID: ac3.OrgId} - err = GetDashboardAclInfoList(permQuery) + err = sqlStore.GetDashboardAclInfoList(context.Background(), permQuery) So(err, ShouldBeNil) So(len(permQuery.Result), ShouldEqual, 1) diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index 03459a421f2..c37d6370cac 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -113,6 +113,7 @@ func newSQLStore(cfg *setting.Cfg, cacheService *localcache.CacheService, bus bu ss.addAlertNotificationUidByIdHandler() ss.addPreferencesQueryAndCommandHandlers() ss.addDashboardQueryAndCommandHandlers() + ss.addDashboardACLQueryAndCommandHandlers() ss.addQuotaQueryAndCommandHandlers() // if err := ss.Reset(); err != nil { diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index 981285442f7..7ac44d142b6 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/pkg/services/sqlstore/team_test.go @@ -262,7 +262,7 @@ func TestTeamCommandsAndQueries(t *testing.T) { So(err, ShouldEqual, models.ErrTeamNotFound) permQuery := &models.GetDashboardAclInfoListQuery{DashboardID: 1, OrgID: testOrgID} - err = GetDashboardAclInfoList(permQuery) + err = sqlStore.GetDashboardAclInfoList(context.Background(), permQuery) So(err, ShouldBeNil) So(len(permQuery.Result), ShouldEqual, 0) diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index ae0ed8369d8..5306b8d47da 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -261,7 +261,7 @@ func TestUserDataAccess(t *testing.T) { require.Len(t, query1.Result, 1) permQuery := &models.GetDashboardAclInfoListQuery{DashboardID: 1, OrgID: users[0].OrgId} - err = GetDashboardAclInfoList(permQuery) + err = ss.GetDashboardAclInfoList(context.Background(), permQuery) require.Nil(t, err) require.Len(t, permQuery.Result, 0) @@ -347,7 +347,7 @@ func TestUserDataAccess(t *testing.T) { require.Len(t, query2.Result, 1) permQuery = &models.GetDashboardAclInfoListQuery{DashboardID: 1, OrgID: users[0].OrgId} - err = GetDashboardAclInfoList(permQuery) + err = ss.GetDashboardAclInfoList(context.Background(), permQuery) require.Nil(t, err) require.Len(t, permQuery.Result, 0)