Chore: Remove bus.Dispatch from guardian package (#46711)

* replace bus in guardian with sqlstore * fix a couple of tests * replace bus in the rest of the tests * allow init guardian from other packages * make linter happy * init guardian in library elements * fix another test in libraryelements * fix more tests * move guardian mock one level deeper * fix more tests * rename init functions
2025-02-25 18:55:37 -06:00 · 2022-03-21 10:49:49 +01:00
parent 788fde7ead
commit fec634a091
14 changed files with 107 additions and 139 deletions
--- a/pkg/services/guardian/accesscontrol_guardian.go
+++ b/pkg/services/guardian/accesscontrol_guardian.go
@@ -22,7 +22,7 @@ var _ DashboardGuardian = new(AccessControlDashboardGuardian)

 func NewAccessControlDashboardGuardian(
 	ctx context.Context, dashboardId int64, user *models.SignedInUser,
-	store *sqlstore.SQLStore, ac accesscontrol.AccessControl, permissionsServices accesscontrol.PermissionsServices,
+	store sqlstore.Store, ac accesscontrol.AccessControl, permissionsServices accesscontrol.PermissionsServices,
 ) *AccessControlDashboardGuardian {
 	return &AccessControlDashboardGuardian{
 		ctx:                ctx,
@@ -41,7 +41,7 @@ type AccessControlDashboardGuardian struct {
 	dashboardID        int64
 	dashboard          *models.Dashboard
 	user               *models.SignedInUser
-	store              *sqlstore.SQLStore
+	store              sqlstore.Store
 	ac                 accesscontrol.AccessControl
 	permissionServices accesscontrol.PermissionsServices
 }
--- a/pkg/services/guardian/guardian.go
+++ b/pkg/services/guardian/guardian.go
@@ -4,9 +4,9 @@ import (
 	"context"
 	"errors"

-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/setting"
 )

@@ -43,17 +43,23 @@ type dashboardGuardianImpl struct {
 	teams  []*models.TeamDTO
 	log    log.Logger
 	ctx    context.Context
+	store  sqlstore.Store
 }

 // New factory for creating a new dashboard guardian instance
 // When using access control this function is replaced on startup and the AccessControlDashboardGuardian is returned
 var New = func(ctx context.Context, dashId int64, orgId int64, user *models.SignedInUser) DashboardGuardian {
+	panic("no guardian factory implementation provided")
+}
+
+func newDashboardGuardian(ctx context.Context, dashId int64, orgId int64, user *models.SignedInUser, store sqlstore.Store) *dashboardGuardianImpl {
 	return &dashboardGuardianImpl{
 		user:   user,
 		dashId: dashId,
 		orgId:  orgId,
 		log:    log.New("dashboard.permissions"),
 		ctx:    ctx,
+		store:  store,
 	}
 }

@@ -146,7 +152,7 @@ func (g *dashboardGuardianImpl) checkAcl(permission models.PermissionType, acl [
 	}

 	// load teams
-	teams, err := g.getTeams(g.ctx)
+	teams, err := g.getTeams()
 	if err != nil {
 		return false, err
 	}
@@ -216,10 +222,9 @@ func (g *dashboardGuardianImpl) GetAcl() ([]*models.DashboardAclInfoDTO, error)
 	}

 	query := models.GetDashboardAclInfoListQuery{DashboardID: g.dashId, OrgID: g.orgId}
-	if err := bus.Dispatch(g.ctx, &query); err != nil {
+	if err := g.store.GetDashboardAclInfoList(g.ctx, &query); err != nil {
 		return nil, err
 	}
-
 	g.acl = query.Result
 	return g.acl, nil
 }
@@ -260,14 +265,13 @@ func (g *dashboardGuardianImpl) GetACLWithoutDuplicates() ([]*models.DashboardAc
 	return result, nil
 }

-func (g *dashboardGuardianImpl) getTeams(ctx context.Context) ([]*models.TeamDTO, error) {
+func (g *dashboardGuardianImpl) getTeams() ([]*models.TeamDTO, error) {
 	if g.teams != nil {
 		return g.teams, nil
 	}

 	query := models.GetTeamsByUserQuery{OrgId: g.orgId, UserId: g.user.UserId}
-	// TODO: Use bus.Dispatch(g.Ctx, &query) when GetTeamsByUserQuery supports context.
-	err := bus.Dispatch(ctx, &query)
+	err := g.store.GetTeamsByUser(g.ctx, &query)

 	g.teams = query.Result
 	return query.Result, err
--- a/pkg/services/guardian/guardian_test.go
+++ b/pkg/services/guardian/guardian_test.go
@@ -7,8 +7,8 @@ import (
 	"runtime"
 	"testing"

-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
 	"github.com/grafana/grafana/pkg/setting"

 	"github.com/stretchr/testify/require"
@@ -682,16 +682,12 @@ func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsWithOverrideShou

 func TestGuardianGetHiddenACL(t *testing.T) {
 	t.Run("Get hidden ACL tests", func(t *testing.T) {
-		bus.ClearBusHandlers()
-
-		bus.AddHandler("test", func(ctx context.Context, query *models.GetDashboardAclInfoListQuery) error {
-			query.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
-		})
+		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},
+		}

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

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

 			hiddenACL, err := g.GetHiddenACL(cfg)
 			require.NoError(t, err)
@@ -730,21 +726,18 @@ func TestGuardianGetHiddenACL(t *testing.T) {

 func TestGuardianGetAclWithoutDuplicates(t *testing.T) {
 	t.Run("Get hidden ACL tests", func(t *testing.T) {
-		t.Cleanup(bus.ClearBusHandlers)
+		store := mockstore.NewSQLStoreMock()

-		bus.AddHandler("test", func(ctx context.Context, query *models.GetDashboardAclInfoListQuery) error {
-			query.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
-		})
+		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},
+		}

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

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

-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/sqlstore/mockstore"
 	"github.com/stretchr/testify/assert"
 )

@@ -36,7 +36,8 @@ func orgRoleScenario(desc string, t *testing.T, role models.RoleType, fn scenari
 			OrgId:   orgID,
 			OrgRole: role,
 		}
-		guard := New(context.Background(), dashboardID, orgID, user)
+		store := mockstore.NewSQLStoreMock()
+		guard := newDashboardGuardian(context.Background(), dashboardID, orgID, user, store)

 		sc := &scenarioContext{
 			t:                t,
@@ -57,7 +58,8 @@ func apiKeyScenario(desc string, t *testing.T, role models.RoleType, fn scenario
 			OrgRole:  role,
 			ApiKeyId: 10,
 		}
-		guard := New(context.Background(), dashboardID, orgID, user)
+		store := mockstore.NewSQLStoreMock()
+		guard := newDashboardGuardian(context.Background(), dashboardID, orgID, user, store)
 		sc := &scenarioContext{
 			t:                t,
 			orgRoleScenario:  desc,
@@ -73,20 +75,8 @@ func apiKeyScenario(desc string, t *testing.T, role models.RoleType, fn scenario
 func permissionScenario(desc string, dashboardID int64, sc *scenarioContext,
 	permissions []*models.DashboardAclInfoDTO, fn scenarioFunc) {
 	sc.t.Run(desc, func(t *testing.T) {
-		bus.ClearBusHandlers()
-
-		bus.AddHandler("test", func(ctx context.Context, query *models.GetDashboardAclInfoListQuery) error {
-			if query.OrgID != sc.givenUser.OrgId {
-				sc.reportFailure("Invalid organization id for GetDashboardAclInfoListQuery", sc.givenUser.OrgId, query.OrgID)
-			}
-			if query.DashboardID != sc.givenDashboardID {
-				sc.reportFailure("Invalid dashboard id for GetDashboardAclInfoListQuery", sc.givenDashboardID, query.DashboardID)
-			}
-
-			query.Result = permissions
-			return nil
-		})
-
+		store := mockstore.NewSQLStoreMock()
+		store.ExpectedDashboardAclInfoList = permissions
 		teams := []*models.TeamDTO{}

 		for _, p := range permissions {
@@ -94,21 +84,10 @@ func permissionScenario(desc string, dashboardID int64, sc *scenarioContext,
 				teams = append(teams, &models.TeamDTO{Id: p.TeamId})
 			}
 		}
-
-		bus.AddHandler("test", func(ctx context.Context, query *models.GetTeamsByUserQuery) error {
-			if query.OrgId != sc.givenUser.OrgId {
-				sc.reportFailure("Invalid organization id for GetTeamsByUserQuery", sc.givenUser.OrgId, query.OrgId)
-			}
-			if query.UserId != sc.givenUser.UserId {
-				sc.reportFailure("Invalid user id for GetTeamsByUserQuery", sc.givenUser.UserId, query.UserId)
-			}
-
-			query.Result = teams
-			return nil
-		})
+		store.ExpectedTeamsByUser = teams

 		sc.permissionScenario = desc
-		sc.g = New(context.Background(), dashboardID, sc.givenUser.OrgId, sc.givenUser)
+		sc.g = newDashboardGuardian(context.Background(), dashboardID, sc.givenUser.OrgId, sc.givenUser, store)
 		sc.givenDashboardID = dashboardID
 		sc.givenPermissions = permissions
 		sc.givenTeams = teams
--- a/pkg/services/guardian/provider.go
+++ b/pkg/services/guardian/provider.go
@@ -14,9 +14,21 @@ type Provider struct{}
 func ProvideService(store *sqlstore.SQLStore, ac accesscontrol.AccessControl, permissionsServices accesscontrol.PermissionsServices, features featuremgmt.FeatureToggles) *Provider {
 	if features.IsEnabled(featuremgmt.FlagAccesscontrol) {
 		// TODO: Fix this hack, see https://github.com/grafana/grafana-enterprise/issues/2935
-		New = func(ctx context.Context, dashId int64, orgId int64, user *models.SignedInUser) DashboardGuardian {
-			return NewAccessControlDashboardGuardian(ctx, dashId, user, store, ac, permissionsServices)
-		}
+		InitAcessControlGuardian(store, ac, permissionsServices)
+	} else {
+		InitLegacyGuardian(store)
 	}
 	return &Provider{}
 }
+
+func InitLegacyGuardian(store sqlstore.Store) {
+	New = func(ctx context.Context, dashId int64, orgId int64, user *models.SignedInUser) DashboardGuardian {
+		return newDashboardGuardian(ctx, dashId, orgId, user, store)
+	}
+}
+
+func InitAcessControlGuardian(store sqlstore.Store, ac accesscontrol.AccessControl, permissionsServices accesscontrol.PermissionsServices) {
+	New = func(ctx context.Context, dashId int64, orgId int64, user *models.SignedInUser) DashboardGuardian {
+		return NewAccessControlDashboardGuardian(ctx, dashId, user, store, ac, permissionsServices)
+	}
+}