Team Guardian: Refactor it to remove the bus dependency (#43058)

* Refactor team guardian to avoid to use bus

* Fix lint

* Fix lint

pkg/api/http_server.go

@@ -54,6 +54,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/secrets"
 	"github.com/grafana/grafana/pkg/services/shorturls"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
+	"github.com/grafana/grafana/pkg/services/teamguardian"
 	"github.com/grafana/grafana/pkg/services/updatechecker"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util/errutil"
@@ -113,6 +114,7 @@ type HTTPServer struct {
 	tracingService            tracing.Tracer
 	updateChecker             *updatechecker.Service
 	searchUsersService        searchusers.Service
+	teamGuardian              teamguardian.TeamGuardian
 	queryDataService          *query.Service
 	serviceAccountsService    serviceaccounts.Service
 }
@@ -138,8 +140,8 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 	libraryPanelService librarypanels.Service, libraryElementService libraryelements.Service,
 	quotaService *quota.QuotaService, socialService social.Service, tracingService tracing.Tracer,
 	encryptionService encryption.Internal, updateChecker *updatechecker.Service, searchUsersService searchusers.Service,
-	dataSourcesService *datasources.Service, secretsService secrets.Service,
-	queryDataService *query.Service, serviceaccountsService serviceaccounts.Service) (*HTTPServer, error) {
+	dataSourcesService *datasources.Service, secretsService secrets.Service, queryDataService *query.Service,
+	teamGuardian teamguardian.TeamGuardian, serviceaccountsService serviceaccounts.Service) (*HTTPServer, error) {
 	web.Env = cfg.Env
 	m := web.New()
 
@@ -190,6 +192,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 		SecretsService:            secretsService,
 		DataSourcesService:        dataSourcesService,
 		searchUsersService:        searchUsersService,
+		teamGuardian:              teamGuardian,
 		queryDataService:          queryDataService,
 		serviceAccountsService:    serviceaccountsService,
 	}

pkg/api/team.go

@@ -9,7 +9,6 @@ import (
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
-	"github.com/grafana/grafana/pkg/services/teamguardian"
 	"github.com/grafana/grafana/pkg/util"
 	"github.com/grafana/grafana/pkg/web"
 )
@@ -61,7 +60,7 @@ func (hs *HTTPServer) UpdateTeam(c *models.ReqContext) response.Response {
 	cmd.OrgId = c.OrgId
 	cmd.Id = c.ParamsInt64(":teamId")
 
-	if err := teamguardian.CanAdmin(c.Req.Context(), hs.Bus, cmd.OrgId, cmd.Id, c.SignedInUser); err != nil {
+	if err := hs.teamGuardian.CanAdmin(c.Req.Context(), cmd.OrgId, cmd.Id, c.SignedInUser); err != nil {
 		return response.Error(403, "Not allowed to update team", err)
 	}
 
@@ -81,7 +80,7 @@ func (hs *HTTPServer) DeleteTeamByID(c *models.ReqContext) response.Response {
 	teamId := c.ParamsInt64(":teamId")
 	user := c.SignedInUser
 
-	if err := teamguardian.CanAdmin(c.Req.Context(), hs.Bus, orgId, teamId, user); err != nil {
+	if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, user); err != nil {
 		return response.Error(403, "Not allowed to delete team", err)
 	}
 
@@ -161,7 +160,7 @@ func (hs *HTTPServer) GetTeamPreferences(c *models.ReqContext) response.Response
 	teamId := c.ParamsInt64(":teamId")
 	orgId := c.OrgId
 
-	if err := teamguardian.CanAdmin(c.Req.Context(), hs.Bus, orgId, teamId, c.SignedInUser); err != nil {
+	if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, c.SignedInUser); err != nil {
 		return response.Error(403, "Not allowed to view team preferences.", err)
 	}
 
@@ -177,7 +176,7 @@ func (hs *HTTPServer) UpdateTeamPreferences(c *models.ReqContext) response.Respo
 	teamId := c.ParamsInt64(":teamId")
 	orgId := c.OrgId
 
-	if err := teamguardian.CanAdmin(c.Req.Context(), hs.Bus, orgId, teamId, c.SignedInUser); err != nil {
+	if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, c.SignedInUser); err != nil {
 		return response.Error(403, "Not allowed to update team preferences.", err)
 	}
 

pkg/api/team_members.go

@@ -9,7 +9,6 @@ import (
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
-	"github.com/grafana/grafana/pkg/services/teamguardian"
 	"github.com/grafana/grafana/pkg/util"
 	"github.com/grafana/grafana/pkg/web"
 )
@@ -51,7 +50,7 @@ func (hs *HTTPServer) AddTeamMember(c *models.ReqContext) response.Response {
 	cmd.OrgId = c.OrgId
 	cmd.TeamId = c.ParamsInt64(":teamId")
 
-	if err := teamguardian.CanAdmin(c.Req.Context(), hs.Bus, cmd.OrgId, cmd.TeamId, c.SignedInUser); err != nil {
+	if err := hs.teamGuardian.CanAdmin(c.Req.Context(), cmd.OrgId, cmd.TeamId, c.SignedInUser); err != nil {
 		return response.Error(403, "Not allowed to add team member", err)
 	}
 
@@ -82,7 +81,7 @@ func (hs *HTTPServer) UpdateTeamMember(c *models.ReqContext) response.Response {
 	teamId := c.ParamsInt64(":teamId")
 	orgId := c.OrgId
 
-	if err := teamguardian.CanAdmin(c.Req.Context(), hs.Bus, orgId, teamId, c.SignedInUser); err != nil {
+	if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, c.SignedInUser); err != nil {
 		return response.Error(403, "Not allowed to update team member", err)
 	}
 
@@ -109,7 +108,7 @@ func (hs *HTTPServer) RemoveTeamMember(c *models.ReqContext) response.Response {
 	teamId := c.ParamsInt64(":teamId")
 	userId := c.ParamsInt64(":userId")
 
-	if err := teamguardian.CanAdmin(c.Req.Context(), hs.Bus, orgId, teamId, c.SignedInUser); err != nil {
+	if err := hs.teamGuardian.CanAdmin(c.Req.Context(), orgId, teamId, c.SignedInUser); err != nil {
 		return response.Error(403, "Not allowed to remove team member", err)
 	}
 

pkg/server/wire.go

@@ -59,6 +59,9 @@ import (
 	serviceaccountsmanager "github.com/grafana/grafana/pkg/services/serviceaccounts/manager"
 	"github.com/grafana/grafana/pkg/services/shorturls"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
+	"github.com/grafana/grafana/pkg/services/teamguardian"
+	teamguardianDatabase "github.com/grafana/grafana/pkg/services/teamguardian/database"
+	teamguardianManager "github.com/grafana/grafana/pkg/services/teamguardian/manager"
 	"github.com/grafana/grafana/pkg/services/thumbs"
 	"github.com/grafana/grafana/pkg/services/updatechecker"
 	"github.com/grafana/grafana/pkg/setting"
@@ -173,6 +176,10 @@ var wireBasicSet = wire.NewSet(
 	serviceaccountsmanager.ProvideServiceAccountsService,
 	wire.Bind(new(serviceaccounts.Service), new(*serviceaccountsmanager.ServiceAccountsService)),
 	expr.ProvideService,
+	teamguardianDatabase.ProvideTeamGuardianStore,
+	wire.Bind(new(teamguardian.Store), new(*teamguardianDatabase.TeamGuardianStoreImpl)),
+	teamguardianManager.ProvideService,
+	wire.Bind(new(teamguardian.TeamGuardian), new(*teamguardianManager.Service)),
 )
 
 var wireSet = wire.NewSet(

pkg/services/teamguardian/database/database.go

@@ -0,0 +1,22 @@
+package database
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/sqlstore"
+)
+
+type TeamGuardianStoreImpl struct{}
+
+func ProvideTeamGuardianStore() *TeamGuardianStoreImpl {
+	return &TeamGuardianStoreImpl{}
+}
+
+func (t *TeamGuardianStoreImpl) GetTeamMembers(ctx context.Context, query models.GetTeamMembersQuery) ([]*models.TeamMemberDTO, error) {
+	if err := sqlstore.GetTeamMembers(ctx, &query); err != nil {
+		return nil, err
+	}
+
+	return query.Result, nil
+}

pkg/services/teamguardian/database/database_mock.go

@@ -0,0 +1,17 @@
+package database
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/models"
+	"github.com/stretchr/testify/mock"
+)
+
+type TeamGuardianStoreMock struct {
+	mock.Mock
+}
+
+func (t *TeamGuardianStoreMock) GetTeamMembers(ctx context.Context, query models.GetTeamMembersQuery) ([]*models.TeamMemberDTO, error) {
+	args := t.Called(ctx, query)
+	return args.Get(0).([]*models.TeamMemberDTO), args.Error(1)
+}

pkg/services/teamguardian/manager/service.go

@@ -0,0 +1,45 @@
+package manager
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/teamguardian"
+)
+
+type Service struct {
+	store teamguardian.Store
+}
+
+func ProvideService(store teamguardian.Store) *Service {
+	return &Service{store: store}
+}
+
+func (s *Service) CanAdmin(ctx context.Context, orgId int64, teamId int64, user *models.SignedInUser) error {
+	if user.OrgRole == models.ROLE_ADMIN {
+		return nil
+	}
+
+	if user.OrgId != orgId {
+		return models.ErrNotAllowedToUpdateTeamInDifferentOrg
+	}
+
+	cmd := models.GetTeamMembersQuery{
+		OrgId:  orgId,
+		TeamId: teamId,
+		UserId: user.UserId,
+	}
+
+	results, err := s.store.GetTeamMembers(ctx, cmd)
+	if err != nil {
+		return err
+	}
+
+	for _, member := range results {
+		if member.UserId == user.UserId && member.Permission == models.PERMISSION_ADMIN {
+			return nil
+		}
+	}
+
+	return models.ErrNotAllowedToUpdateTeam
+}

pkg/services/teamguardian/manager/service_mock.go

@@ -0,0 +1,17 @@
+package manager
+
+import (
+	"context"
+
+	"github.com/grafana/grafana/pkg/models"
+	"github.com/stretchr/testify/mock"
+)
+
+type TeamGuardianMock struct {
+	mock.Mock
+}
+
+func (t *TeamGuardianMock) CanAdmin(ctx context.Context, orgId int64, teamId int64, user *models.SignedInUser) error {
+	args := t.Called(ctx, orgId, teamId, user)
+	return args.Error(0)
+}

pkg/services/teamguardian/manager/service_test.go

@@ -1,19 +1,20 @@
-package teamguardian
+package manager
 
 import (
 	"context"
 	"testing"
 
-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
-
+	"github.com/grafana/grafana/pkg/services/teamguardian/database"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
 func TestUpdateTeam(t *testing.T) {
-	t.Run("Updating a team", func(t *testing.T) {
-		bus.ClearBusHandlers()
+	store := new(database.TeamGuardianStoreMock)
+	teamGuardianService := ProvideService(store)
 
+	t.Run("Updating a team", func(t *testing.T) {
 		admin := models.SignedInUser{
 			UserId:  1,
 			OrgId:   1,
@@ -31,58 +32,55 @@ func TestUpdateTeam(t *testing.T) {
 
 		t.Run("Given an editor and a team he isn't a member of", func(t *testing.T) {
 			t.Run("Should not be able to update the team", func(t *testing.T) {
-				bus.AddHandler("test", func(ctx context.Context, cmd *models.GetTeamMembersQuery) error {
-					cmd.Result = []*models.TeamMemberDTO{}
-					return nil
-				})
-
-				err := CanAdmin(context.Background(), bus.GetBus(), testTeam.OrgId, testTeam.Id, &editor)
+				ctx := context.Background()
+				store.On("GetTeamMembers", ctx, mock.Anything).Return([]*models.TeamMemberDTO{}, nil).Once()
+				err := teamGuardianService.CanAdmin(ctx, testTeam.OrgId, testTeam.Id, &editor)
 				require.Equal(t, models.ErrNotAllowedToUpdateTeam, err)
 			})
 		})
 
 		t.Run("Given an editor and a team he is an admin in", func(t *testing.T) {
 			t.Run("Should be able to update the team", func(t *testing.T) {
-				bus.AddHandler("test", func(ctx context.Context, cmd *models.GetTeamMembersQuery) error {
-					cmd.Result = []*models.TeamMemberDTO{{
-						OrgId:      testTeam.OrgId,
-						TeamId:     testTeam.Id,
-						UserId:     editor.UserId,
-						Permission: models.PERMISSION_ADMIN,
-					}}
-					return nil
-				})
+				ctx := context.Background()
+
+				result := []*models.TeamMemberDTO{{
+					OrgId:      testTeam.OrgId,
+					TeamId:     testTeam.Id,
+					UserId:     editor.UserId,
+					Permission: models.PERMISSION_ADMIN,
+				}}
 
-				err := CanAdmin(context.Background(), bus.GetBus(), testTeam.OrgId, testTeam.Id, &editor)
+				store.On("GetTeamMembers", ctx, mock.Anything).Return(result, nil).Once()
+				err := teamGuardianService.CanAdmin(ctx, testTeam.OrgId, testTeam.Id, &editor)
 				require.NoError(t, err)
 			})
 		})
 
 		t.Run("Given an editor and a team in another org", func(t *testing.T) {
+			ctx := context.Background()
+
 			testTeamOtherOrg := models.Team{
 				Id:    1,
 				OrgId: 2,
 			}
 
 			t.Run("Shouldn't be able to update the team", func(t *testing.T) {
-				bus.AddHandler("test", func(ctx context.Context, cmd *models.GetTeamMembersQuery) error {
-					cmd.Result = []*models.TeamMemberDTO{{
-						OrgId:      testTeamOtherOrg.OrgId,
-						TeamId:     testTeamOtherOrg.Id,
-						UserId:     editor.UserId,
-						Permission: models.PERMISSION_ADMIN,
-					}}
-					return nil
-				})
+				result := []*models.TeamMemberDTO{{
+					OrgId:      testTeamOtherOrg.OrgId,
+					TeamId:     testTeamOtherOrg.Id,
+					UserId:     editor.UserId,
+					Permission: models.PERMISSION_ADMIN,
+				}}
 
-				err := CanAdmin(context.Background(), bus.GetBus(), testTeamOtherOrg.OrgId, testTeamOtherOrg.Id, &editor)
+				store.On("GetTeamMembers", ctx, mock.Anything).Return(result, nil).Once()
+				err := teamGuardianService.CanAdmin(ctx, testTeamOtherOrg.OrgId, testTeamOtherOrg.Id, &editor)
 				require.Equal(t, models.ErrNotAllowedToUpdateTeamInDifferentOrg, err)
 			})
 		})
 
 		t.Run("Given an org admin and a team", func(t *testing.T) {
 			t.Run("Should be able to update the team", func(t *testing.T) {
-				err := CanAdmin(context.Background(), bus.GetBus(), testTeam.OrgId, testTeam.Id, &admin)
+				err := teamGuardianService.CanAdmin(context.Background(), testTeam.OrgId, testTeam.Id, &admin)
 				require.NoError(t, err)
 			})
 		})

pkg/services/teamguardian/team.go

@@ -3,34 +3,13 @@ package teamguardian
 import (
 	"context"
 
-	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
 )
 
-func CanAdmin(ctx context.Context, bus bus.Bus, orgId int64, teamId int64, user *models.SignedInUser) error {
-	if user.OrgRole == models.ROLE_ADMIN {
-		return nil
-	}
-
-	if user.OrgId != orgId {
-		return models.ErrNotAllowedToUpdateTeamInDifferentOrg
-	}
-
-	cmd := models.GetTeamMembersQuery{
-		OrgId:  orgId,
-		TeamId: teamId,
-		UserId: user.UserId,
-	}
-
-	if err := bus.Dispatch(ctx, &cmd); err != nil {
-		return err
-	}
-
-	for _, member := range cmd.Result {
-		if member.UserId == user.UserId && member.Permission == models.PERMISSION_ADMIN {
-			return nil
-		}
-	}
+type TeamGuardian interface {
+	CanAdmin(ctx context.Context, orgId int64, teamId int64, user *models.SignedInUser) error
+}
 
-	return models.ErrNotAllowedToUpdateTeam
+type Store interface {
+	GetTeamMembers(ctx context.Context, query models.GetTeamMembersQuery) ([]*models.TeamMemberDTO, error)
 }