sqlstore split: dashboard permissions (#49962)

* backend/sqlstore split: remove unused GetDashboardPermissionsForUser from sqlstore * remove debugging line * backend/sqlstore: move dashboard permission related functions to dashboard service
2025-02-25 18:55:37 -06:00 · 2022-06-01 14:16:26 -04:00
parent bb94681d5a
commit 2edfbb7767
30 changed files with 557 additions and 481 deletions
--- a/pkg/services/guardian/guardian.go
+++ b/pkg/services/guardian/guardian.go
@@ -6,6 +6,7 @@ import (

 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
 )
@@ -36,14 +37,15 @@ type DashboardGuardian interface {
 }

 type dashboardGuardianImpl struct {
-	user   *models.SignedInUser
-	dashId int64
-	orgId  int64
-	acl    []*models.DashboardAclInfoDTO
-	teams  []*models.TeamDTO
-	log    log.Logger
-	ctx    context.Context
-	store  sqlstore.Store
+	user             *models.SignedInUser
+	dashId           int64
+	orgId            int64
+	acl              []*models.DashboardAclInfoDTO
+	teams            []*models.TeamDTO
+	log              log.Logger
+	ctx              context.Context
+	store            sqlstore.Store
+	dashboardService dashboards.DashboardService
 }

 // New factory for creating a new dashboard guardian instance
@@ -52,14 +54,15 @@ var New = func(ctx context.Context, dashId int64, orgId int64, user *models.Sign
 	panic("no guardian factory implementation provided")
 }

-func newDashboardGuardian(ctx context.Context, dashId int64, orgId int64, user *models.SignedInUser, store sqlstore.Store) *dashboardGuardianImpl {
+func newDashboardGuardian(ctx context.Context, dashId int64, orgId int64, user *models.SignedInUser, store sqlstore.Store, dashSvc dashboards.DashboardService) *dashboardGuardianImpl {
 	return &dashboardGuardianImpl{
-		user:   user,
-		dashId: dashId,
-		orgId:  orgId,
-		log:    log.New("dashboard.permissions"),
-		ctx:    ctx,
-		store:  store,
+		user:             user,
+		dashId:           dashId,
+		orgId:            orgId,
+		log:              log.New("dashboard.permissions"),
+		ctx:              ctx,
+		store:            store,
+		dashboardService: dashSvc,
 	}
 }

@@ -222,7 +225,7 @@ func (g *dashboardGuardianImpl) GetAcl() ([]*models.DashboardAclInfoDTO, error)
 	}

 	query := models.GetDashboardAclInfoListQuery{DashboardID: g.dashId, OrgID: g.orgId}
-	if err := g.store.GetDashboardAclInfoList(g.ctx, &query); err != nil {
+	if err := g.dashboardService.GetDashboardAclInfoList(g.ctx, &query); err != nil {
 		return nil, err
 	}
 	g.acl = query.Result
--- a/pkg/services/guardian/guardian_test.go
+++ b/pkg/services/guardian/guardian_test.go
@@ -7,11 +7,13 @@ import (
 	"runtime"
 	"testing"

+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
 	"github.com/grafana/grafana/pkg/setting"
-
-	"github.com/stretchr/testify/require"
 )

 const (
@@ -683,11 +685,15 @@ func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsWithOverrideShou
 func TestGuardianGetHiddenACL(t *testing.T) {
 	t.Run("Get hidden ACL tests", func(t *testing.T) {
 		store := mockstore.NewSQLStoreMock()
-		store.ExpectedDashboardAclInfoList = []*models.DashboardAclInfoDTO{
-			{Inherited: false, UserId: 1, UserLogin: "user1", Permission: models.PERMISSION_EDIT},
-			{Inherited: false, UserId: 2, UserLogin: "user2", Permission: models.PERMISSION_ADMIN},
-			{Inherited: true, UserId: 3, UserLogin: "user3", Permission: models.PERMISSION_VIEW},
-		}
+		dashSvc := dashboards.NewFakeDashboardService(t)
+		dashSvc.On("GetDashboardAclInfoList", mock.Anything, mock.AnythingOfType("*models.GetDashboardAclInfoListQuery")).Run(func(args mock.Arguments) {
+			q := args.Get(1).(*models.GetDashboardAclInfoListQuery)
+			q.Result = []*models.DashboardAclInfoDTO{
+				{Inherited: false, UserId: 1, UserLogin: "user1", Permission: models.PERMISSION_EDIT},
+				{Inherited: false, UserId: 2, UserLogin: "user2", Permission: models.PERMISSION_ADMIN},
+				{Inherited: true, UserId: 3, UserLogin: "user3", Permission: models.PERMISSION_VIEW},
+			}
+		}).Return(nil)

 		cfg := setting.NewCfg()
 		cfg.HiddenUsers = map[string]struct{}{"user2": {}}
@@ -698,7 +704,7 @@ func TestGuardianGetHiddenACL(t *testing.T) {
 				UserId: 1,
 				Login:  "user1",
 			}
-			g := newDashboardGuardian(context.Background(), dashboardID, orgID, user, store)
+			g := newDashboardGuardian(context.Background(), dashboardID, orgID, user, store, dashSvc)

 			hiddenACL, err := g.GetHiddenACL(cfg)
 			require.NoError(t, err)
@@ -714,7 +720,7 @@ func TestGuardianGetHiddenACL(t *testing.T) {
 				Login:          "user1",
 				IsGrafanaAdmin: true,
 			}
-			g := newDashboardGuardian(context.Background(), dashboardID, orgID, user, store)
+			g := newDashboardGuardian(context.Background(), dashboardID, orgID, user, store, &dashboards.FakeDashboardService{})

 			hiddenACL, err := g.GetHiddenACL(cfg)
 			require.NoError(t, err)
@@ -727,17 +733,20 @@ func TestGuardianGetHiddenACL(t *testing.T) {
 func TestGuardianGetAclWithoutDuplicates(t *testing.T) {
 	t.Run("Get hidden ACL tests", func(t *testing.T) {
 		store := mockstore.NewSQLStoreMock()
-
-		store.ExpectedDashboardAclInfoList = []*models.DashboardAclInfoDTO{
-			{Inherited: true, UserId: 3, UserLogin: "user3", Permission: models.PERMISSION_EDIT},
-			{Inherited: false, UserId: 3, UserLogin: "user3", Permission: models.PERMISSION_VIEW},
-			{Inherited: false, UserId: 2, UserLogin: "user2", Permission: models.PERMISSION_ADMIN},
-			{Inherited: true, UserId: 4, UserLogin: "user4", Permission: models.PERMISSION_ADMIN},
-			{Inherited: false, UserId: 4, UserLogin: "user4", Permission: models.PERMISSION_ADMIN},
-			{Inherited: false, UserId: 5, UserLogin: "user5", Permission: models.PERMISSION_EDIT},
-			{Inherited: true, UserId: 6, UserLogin: "user6", Permission: models.PERMISSION_VIEW},
-			{Inherited: false, UserId: 6, UserLogin: "user6", Permission: models.PERMISSION_EDIT},
-		}
+		dashSvc := dashboards.NewFakeDashboardService(t)
+		dashSvc.On("GetDashboardAclInfoList", mock.Anything, mock.AnythingOfType("*models.GetDashboardAclInfoListQuery")).Run(func(args mock.Arguments) {
+			q := args.Get(1).(*models.GetDashboardAclInfoListQuery)
+			q.Result = []*models.DashboardAclInfoDTO{
+				{Inherited: true, UserId: 3, UserLogin: "user3", Permission: models.PERMISSION_EDIT},
+				{Inherited: false, UserId: 3, UserLogin: "user3", Permission: models.PERMISSION_VIEW},
+				{Inherited: false, UserId: 2, UserLogin: "user2", Permission: models.PERMISSION_ADMIN},
+				{Inherited: true, UserId: 4, UserLogin: "user4", Permission: models.PERMISSION_ADMIN},
+				{Inherited: false, UserId: 4, UserLogin: "user4", Permission: models.PERMISSION_ADMIN},
+				{Inherited: false, UserId: 5, UserLogin: "user5", Permission: models.PERMISSION_EDIT},
+				{Inherited: true, UserId: 6, UserLogin: "user6", Permission: models.PERMISSION_VIEW},
+				{Inherited: false, UserId: 6, UserLogin: "user6", Permission: models.PERMISSION_EDIT},
+			}
+		}).Return(nil)

 		t.Run("Should get acl without duplicates", func(t *testing.T) {
 			user := &models.SignedInUser{
@@ -745,7 +754,7 @@ func TestGuardianGetAclWithoutDuplicates(t *testing.T) {
 				UserId: 1,
 				Login:  "user1",
 			}
-			g := newDashboardGuardian(context.Background(), dashboardID, orgID, user, store)
+			g := newDashboardGuardian(context.Background(), dashboardID, orgID, user, store, dashSvc)

 			acl, err := g.GetACLWithoutDuplicates()
 			require.NoError(t, err)
--- a/pkg/services/guardian/guardian_util_test.go
+++ b/pkg/services/guardian/guardian_util_test.go
@@ -8,8 +8,10 @@ import (
 	"testing"

 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"

 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/dashboards"
 	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
 )

@@ -38,7 +40,7 @@ func orgRoleScenario(desc string, t *testing.T, role models.RoleType, fn scenari
 			OrgRole: role,
 		}
 		store := mockstore.NewSQLStoreMock()
-		guard := newDashboardGuardian(context.Background(), dashboardID, orgID, user, store)
+		guard := newDashboardGuardian(context.Background(), dashboardID, orgID, user, store, &dashboards.FakeDashboardService{})

 		sc := &scenarioContext{
 			t:                t,
@@ -60,7 +62,7 @@ func apiKeyScenario(desc string, t *testing.T, role models.RoleType, fn scenario
 			ApiKeyId: 10,
 		}
 		store := mockstore.NewSQLStoreMock()
-		guard := newDashboardGuardian(context.Background(), dashboardID, orgID, user, store)
+		guard := newDashboardGuardian(context.Background(), dashboardID, orgID, user, store, &dashboards.FakeDashboardService{})
 		sc := &scenarioContext{
 			t:                t,
 			orgRoleScenario:  desc,
@@ -77,7 +79,6 @@ func permissionScenario(desc string, dashboardID int64, sc *scenarioContext,
 	permissions []*models.DashboardAclInfoDTO, fn scenarioFunc) {
 	sc.t.Run(desc, func(t *testing.T) {
 		store := mockstore.NewSQLStoreMock()
-		store.ExpectedDashboardAclInfoList = permissions
 		teams := []*models.TeamDTO{}

 		for _, p := range permissions {
@@ -87,8 +88,14 @@ func permissionScenario(desc string, dashboardID int64, sc *scenarioContext,
 		}
 		store.ExpectedTeamsByUser = teams

+		dashSvc := dashboards.NewFakeDashboardService(t)
+		dashSvc.On("GetDashboardAclInfoList", mock.Anything, mock.AnythingOfType("*models.GetDashboardAclInfoListQuery")).Run(func(args mock.Arguments) {
+			q := args.Get(1).(*models.GetDashboardAclInfoListQuery)
+			q.Result = permissions
+		}).Return(nil)
+
 		sc.permissionScenario = desc
-		sc.g = newDashboardGuardian(context.Background(), dashboardID, sc.givenUser.OrgId, sc.givenUser, store)
+		sc.g = newDashboardGuardian(context.Background(), dashboardID, sc.givenUser.OrgId, sc.givenUser, store, dashSvc)
 		sc.givenDashboardID = dashboardID
 		sc.givenPermissions = permissions
 		sc.givenTeams = teams
--- a/pkg/services/guardian/provider.go
+++ b/pkg/services/guardian/provider.go
@@ -20,14 +20,14 @@ func ProvideService(
 		// TODO: Fix this hack, see https://github.com/grafana/grafana-enterprise/issues/2935
 		InitAccessControlGuardian(store, ac, folderPermissionsService, dashboardPermissionsService, dashboardService)
 	} else {
-		InitLegacyGuardian(store)
+		InitLegacyGuardian(store, dashboardService)
 	}
 	return &Provider{}
 }

-func InitLegacyGuardian(store sqlstore.Store) {
+func InitLegacyGuardian(store sqlstore.Store, dashSvc dashboards.DashboardService) {
 	New = func(ctx context.Context, dashId int64, orgId int64, user *models.SignedInUser) DashboardGuardian {
-		return newDashboardGuardian(ctx, dashId, orgId, user, store)
+		return newDashboardGuardian(ctx, dashId, orgId, user, store, dashSvc)
 	}
 }