public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Chore: Move team store implementation to a separate package (#55514)

Browse Source 
* Chore: move team store implementation to a separate package

* trying to fix more tests

* fix tests in service accounts and access control

* fix common tests

* restore commented out test

* add todos
This commit is contained in:
Serge Zaitsev
2022-09-22 19:16:21 +02:00
committed by GitHub
parent 591df92265
commit 4c19e83ff0
19 changed files with 444 additions and 236 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/accesscontrol/database/database_test.go
+14 -11
View File
@@ -12,6 +12,8 @@ import (
rs "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/services/team/teamimpl"
"github.com/grafana/grafana/pkg/services/user"
)
@@ -79,9 +81,9 @@ func TestAccessControlStore_GetUserPermissions(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
store, permissionStore, sql := setupTestEnv(t)
store, permissionStore, sql, teamSvc := setupTestEnv(t)
user, team := createUserAndTeam(t, sql, tt.orgID)
user, team := createUserAndTeam(t, sql, teamSvc, tt.orgID)
for _, id := range tt.userPermissions {
_, err := permissionStore.SetUserResourcePermission(context.Background(), tt.orgID, accesscontrol.User{ID: user.ID}, rs.SetResourcePermissionCommand{
@@ -142,8 +144,8 @@ func TestAccessControlStore_GetUserPermissions(t *testing.T) {
func TestAccessControlStore_DeleteUserPermissions(t *testing.T) {
t.Run("expect permissions in all orgs to be deleted", func(t *testing.T) {
store, permissionsStore, sql := setupTestEnv(t)
user, _ := createUserAndTeam(t, sql, 1)
store, permissionsStore, sql, teamSvc := setupTestEnv(t)
user, _ := createUserAndTeam(t, sql, teamSvc, 1)
// generate permissions in org 1
_, err := permissionsStore.SetUserResourcePermission(context.Background(), 1, accesscontrol.User{ID: user.ID}, rs.SetResourcePermissionCommand{
@@ -182,8 +184,8 @@ func TestAccessControlStore_DeleteUserPermissions(t *testing.T) {
})
t.Run("expect permissions in org 1 to be deleted", func(t *testing.T) {
store, permissionsStore, sql := setupTestEnv(t)
user, _ := createUserAndTeam(t, sql, 1)
store, permissionsStore, sql, teamSvc := setupTestEnv(t)
user, _ := createUserAndTeam(t, sql, teamSvc, 1)
// generate permissions in org 1
_, err := permissionsStore.SetUserResourcePermission(context.Background(), 1, accesscontrol.User{ID: user.ID}, rs.SetResourcePermissionCommand{
@@ -222,7 +224,7 @@ func TestAccessControlStore_DeleteUserPermissions(t *testing.T) {
})
}
func createUserAndTeam(t *testing.T, sql *sqlstore.SQLStore, orgID int64) (*user.User, models.Team) {
func createUserAndTeam(t *testing.T, sql *sqlstore.SQLStore, teamSvc team.Service, orgID int64) (*user.User, models.Team) {
t.Helper()
user, err := sql.CreateUser(context.Background(), user.CreateUserCommand{
@@ -231,18 +233,19 @@ func createUserAndTeam(t *testing.T, sql *sqlstore.SQLStore, orgID int64) (*user
})
require.NoError(t, err)
team, err := sql.CreateTeam("team", "", orgID)
team, err := teamSvc.CreateTeam("team", "", orgID)
require.NoError(t, err)
err = sql.AddTeamMember(user.ID, orgID, team.Id, false, models.PERMISSION_VIEW)
err = teamSvc.AddTeamMember(user.ID, orgID, team.Id, false, models.PERMISSION_VIEW)
require.NoError(t, err)
return user, team
}
func setupTestEnv(t testing.TB) (*AccessControlStore, rs.Store, *sqlstore.SQLStore) {
func setupTestEnv(t testing.TB) (*AccessControlStore, rs.Store, *sqlstore.SQLStore, team.Service) {
sql := sqlstore.InitTestDB(t)
acstore := ProvideService(sql)
permissionStore := rs.NewStore(sql)
return acstore, permissionStore, sql
teamService := teamimpl.ProvideService(sql, sql.Cfg)
return acstore, permissionStore, sql, teamService
}
pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go
+14 -8
View File
@@ -13,6 +13,8 @@ import (
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/serviceaccounts"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/services/team/teamimpl"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
)
@@ -38,6 +40,7 @@ var (
func ProvideTeamPermissions(
cfg *setting.Cfg, router routing.RouteRegister, sql *sqlstore.SQLStore,
ac accesscontrol.AccessControl, license models.Licensing, service accesscontrol.Service,
teamService team.Service,
) (*TeamPermissionsService, error) {
options := resourcepermissions.Options{
Resource: "teams",
@@ -49,7 +52,7 @@ func ProvideTeamPermissions(
return err
}
err = sql.GetTeamById(context.Background(), &models.GetTeamByIdQuery{
err = teamService.GetTeamById(context.Background(), &models.GetTeamByIdQuery{
OrgId: orgID,
Id: id,
})
@@ -78,11 +81,11 @@ func ProvideTeamPermissions(
}
switch permission {
case "Member":
return sqlstore.AddOrUpdateTeamMemberHook(session, user.ID, orgID, teamId, user.IsExternal, 0)
return teamimpl.AddOrUpdateTeamMemberHook(session, user.ID, orgID, teamId, user.IsExternal, 0)
case "Admin":
return sqlstore.AddOrUpdateTeamMemberHook(session, user.ID, orgID, teamId, user.IsExternal, models.PERMISSION_ADMIN)
return teamimpl.AddOrUpdateTeamMemberHook(session, user.ID, orgID, teamId, user.IsExternal, models.PERMISSION_ADMIN)
case "":
return sqlstore.RemoveTeamMemberHook(session, &models.RemoveTeamMemberCommand{
return teamimpl.RemoveTeamMemberHook(session, &models.RemoveTeamMemberCommand{
OrgId: orgID,
UserId: user.ID,
TeamId: teamId,
@@ -93,7 +96,7 @@ func ProvideTeamPermissions(
},
}
srv, err := resourcepermissions.New(options, cfg, router, license, ac, service, sql)
srv, err := resourcepermissions.New(options, cfg, router, license, ac, service, sql, teamService)
if err != nil {
return nil, err
}
@@ -111,6 +114,7 @@ var DashboardAdminActions = append(DashboardEditActions, []string{dashboards.Act
func ProvideDashboardPermissions(
cfg *setting.Cfg, router routing.RouteRegister, sql *sqlstore.SQLStore, ac accesscontrol.AccessControl,
license models.Licensing, dashboardStore dashboards.Store, service accesscontrol.Service,
teamService team.Service,
) (*DashboardPermissionsService, error) {
getDashboard := func(ctx context.Context, orgID int64, resourceID string) (*models.Dashboard, error) {
query := &models.GetDashboardQuery{Uid: resourceID, OrgId: orgID}
@@ -164,7 +168,7 @@ func ProvideDashboardPermissions(
RoleGroup: "Dashboards",
}
srv, err := resourcepermissions.New(options, cfg, router, license, ac, service, sql)
srv, err := resourcepermissions.New(options, cfg, router, license, ac, service, sql, teamService)
if err != nil {
return nil, err
}
@@ -189,6 +193,7 @@ var FolderAdminActions = append(FolderEditActions, []string{dashboards.ActionFol
func ProvideFolderPermissions(
cfg *setting.Cfg, router routing.RouteRegister, sql *sqlstore.SQLStore, accesscontrol accesscontrol.AccessControl,
license models.Licensing, dashboardStore dashboards.Store, service accesscontrol.Service,
teamService team.Service,
) (*FolderPermissionsService, error) {
options := resourcepermissions.Options{
Resource: "folders",
@@ -219,7 +224,7 @@ func ProvideFolderPermissions(
WriterRoleName: "Folder permission writer",
RoleGroup: "Folders",
}
srv, err := resourcepermissions.New(options, cfg, router, license, accesscontrol, service, sql)
srv, err := resourcepermissions.New(options, cfg, router, license, accesscontrol, service, sql, teamService)
if err != nil {
return nil, err
}
@@ -279,6 +284,7 @@ type ServiceAccountPermissionsService struct {
func ProvideServiceAccountPermissions(
cfg *setting.Cfg, router routing.RouteRegister, sql *sqlstore.SQLStore, ac accesscontrol.AccessControl,
license models.Licensing, serviceAccountStore serviceaccounts.Store, service accesscontrol.Service,
teamService team.Service,
) (*ServiceAccountPermissionsService, error) {
options := resourcepermissions.Options{
Resource: "serviceaccounts",
@@ -305,7 +311,7 @@ func ProvideServiceAccountPermissions(
RoleGroup: "Service accounts",
}
srv, err := resourcepermissions.New(options, cfg, router, license, ac, service, sql)
srv, err := resourcepermissions.New(options, cfg, router, license, ac, service, sql, teamService)
if err != nil {
return nil, err
}
pkg/services/accesscontrol/resourcepermissions/api_test.go
+9 -7
View File
@@ -19,6 +19,7 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/team/teamimpl"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/web"
@@ -112,7 +113,7 @@ func TestApi_getDescription(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
service, _ := setupTestEnvironment(t, tt.permissions, tt.options)
service, _, _ := setupTestEnvironment(t, tt.permissions, tt.options)
server := setupTestServer(t, &user.SignedInUser{OrgID: 1}, service)
req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("/api/access-control/%s/description", tt.options.Resource), nil)
@@ -159,7 +160,7 @@ func TestApi_getPermissions(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
service, sql := setupTestEnvironment(t, tt.permissions, testOptions)
service, sql, _ := setupTestEnvironment(t, tt.permissions, testOptions)
server := setupTestServer(t, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service)
seedPermissions(t, tt.resourceID, sql, service)
@@ -236,7 +237,7 @@ func TestApi_setBuiltinRolePermission(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
service, _ := setupTestEnvironment(t, tt.permissions, testOptions)
service, _, _ := setupTestEnvironment(t, tt.permissions, testOptions)
server := setupTestServer(t, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service)
recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "builtInRoles", tt.builtInRole)
@@ -314,11 +315,11 @@ func TestApi_setTeamPermission(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
service, sql := setupTestEnvironment(t, tt.permissions, testOptions)
service, _, teamSvc := setupTestEnvironment(t, tt.permissions, testOptions)
server := setupTestServer(t, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service)
// seed team
_, err := sql.CreateTeam("test", "test@test.com", 1)
_, err := teamSvc.CreateTeam("test", "test@test.com", 1)
require.NoError(t, err)
recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "teams", strconv.Itoa(int(tt.teamID)))
@@ -397,7 +398,7 @@ func TestApi_setUserPermission(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
service, sql := setupTestEnvironment(t, tt.permissions, testOptions)
service, sql, _ := setupTestEnvironment(t, tt.permissions, testOptions)
server := setupTestServer(t, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service)
// seed user
@@ -498,7 +499,8 @@ func checkSeededPermissions(t *testing.T, permissions []resourcePermissionDTO) {
func seedPermissions(t *testing.T, resourceID string, sql *sqlstore.SQLStore, service *Service) {
t.Helper()
// seed team 1 with "Edit" permission on dashboard 1
team, err := sql.CreateTeam("test", "test@test.com", 1)
teamSvc := teamimpl.ProvideService(sql, sql.Cfg)
team, err := teamSvc.CreateTeam("test", "test@test.com", 1)
require.NoError(t, err)
_, err = service.SetTeamPermission(context.Background(), team.OrgId, team.Id, resourceID, "Edit")
require.NoError(t, err)
pkg/services/accesscontrol/resourcepermissions/service.go
+5 -1
View File
@@ -10,6 +10,7 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
)
@@ -50,6 +51,7 @@ type Store interface {
func New(
options Options, cfg *setting.Cfg, router routing.RouteRegister, license models.Licensing,
ac accesscontrol.AccessControl, service accesscontrol.Service, sqlStore *sqlstore.SQLStore,
teamService team.Service,
) (*Service, error) {
var permissions []string
actionSet := make(map[string]struct{})
@@ -80,6 +82,7 @@ func New(
actions: actions,
sqlStore: sqlStore,
service: service,
teamService: teamService,
}
s.api = newApi(ac, router, s)
@@ -106,6 +109,7 @@ type Service struct {
permissions []string
actions []string
sqlStore *sqlstore.SQLStore
teamService team.Service
}
func (s *Service) GetPermissions(ctx context.Context, user *user.SignedInUser, resourceID string) ([]accesscontrol.ResourcePermission, error) {
@@ -293,7 +297,7 @@ func (s *Service) validateTeam(ctx context.Context, orgID, teamID int64) error {
return ErrInvalidAssignment
}
if err := s.sqlStore.GetTeamById(ctx, &models.GetTeamByIdQuery{OrgId: orgID, Id: teamID}); err != nil {
if err := s.teamService.GetTeamById(ctx, &models.GetTeamByIdQuery{OrgId: orgID, Id: teamID}); err != nil {
return err
}
return nil
pkg/services/accesscontrol/resourcepermissions/service_test.go
+12 -9
View File
@@ -12,6 +12,8 @@ import (
accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
"github.com/grafana/grafana/pkg/services/licensing/licensingtest"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/services/team/teamimpl"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/setting"
)
@@ -35,7 +37,7 @@ func TestService_SetUserPermission(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
service, sql := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{
service, sql, _ := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{
Resource: "dashboards",
Assignments: Assignments{Users: true},
PermissionsToActions: nil,
@@ -79,14 +81,14 @@ func TestService_SetTeamPermission(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
service, sql := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{
service, _, teamSvc := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{
Resource: "dashboards",
Assignments: Assignments{Teams: true},
PermissionsToActions: nil,
})
// seed team
team, err := sql.CreateTeam("test", "test@test.com", 1)
team, err := teamSvc.CreateTeam("test", "test@test.com", 1)
require.NoError(t, err)
var hookCalled bool
@@ -123,7 +125,7 @@ func TestService_SetBuiltInRolePermission(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
service, _ := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{
service, _, _ := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{
Resource: "dashboards",
Assignments: Assignments{BuiltInRoles: true},
PermissionsToActions: nil,
@@ -196,12 +198,12 @@ func TestService_SetPermissions(t *testing.T) {
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
service, sql := setupTestEnvironment(t, []accesscontrol.Permission{}, tt.options)
service, sql, teamSvc := setupTestEnvironment(t, []accesscontrol.Permission{}, tt.options)
// seed user
_, err := sql.CreateUser(context.Background(), user.CreateUserCommand{Login: "user", OrgID: 1})
require.NoError(t, err)
_, err = sql.CreateTeam("team", "", 1)
_, err = teamSvc.CreateTeam("team", "", 1)
require.NoError(t, err)
permissions, err := service.SetPermissions(context.Background(), 1, "1", tt.commands...)
@@ -215,19 +217,20 @@ func TestService_SetPermissions(t *testing.T) {
}
}
func setupTestEnvironment(t *testing.T, permissions []accesscontrol.Permission, ops Options) (*Service, *sqlstore.SQLStore) {
func setupTestEnvironment(t *testing.T, permissions []accesscontrol.Permission, ops Options) (*Service, *sqlstore.SQLStore, team.Service) {
t.Helper()
sql := sqlstore.InitTestDB(t)
cfg := setting.NewCfg()
teamSvc := teamimpl.ProvideService(sql, cfg)
license := licensingtest.NewFakeLicensing()
license.On("FeatureEnabled", "accesscontrol.enforcement").Return(true).Maybe()
mock := accesscontrolmock.New().WithPermissions(permissions)
service, err := New(
ops, cfg, routing.NewRouteRegister(), license,
accesscontrolmock.New().WithPermissions(permissions), mock, sql,
accesscontrolmock.New().WithPermissions(permissions), mock, sql, teamSvc,
)
require.NoError(t, err)
return service, sql
return service, sql, teamSvc
}
pkg/services/accesscontrol/resourcepermissions/store_bench_test.go
+4 -2
View File
@@ -15,6 +15,7 @@ import (
"github.com/grafana/grafana/pkg/services/datasources"
datasourcesService "github.com/grafana/grafana/pkg/services/datasources/service"
"github.com/grafana/grafana/pkg/services/sqlstore"
"github.com/grafana/grafana/pkg/services/team/teamimpl"
"github.com/grafana/grafana/pkg/services/user"
)
@@ -133,6 +134,7 @@ func GenerateDatasourcePermissions(b *testing.B, db *sqlstore.SQLStore, ac *stor
}
func generateTeamsAndUsers(b *testing.B, db *sqlstore.SQLStore, users int) ([]int64, []int64) {
teamSvc := teamimpl.ProvideService(db, db.Cfg)
numberOfTeams := int(math.Ceil(float64(users) / UsersPerTeam))
globalUserId := 0
@@ -142,7 +144,7 @@ func generateTeamsAndUsers(b *testing.B, db *sqlstore.SQLStore, users int) ([]in
// Create team
teamName := fmt.Sprintf("%s%v", "team", i)
teamEmail := fmt.Sprintf("%s@example.org", teamName)
team, err := db.CreateTeam(teamName, teamEmail, 1)
team, err := teamSvc.CreateTeam(teamName, teamEmail, 1)
require.NoError(b, err)
teamId := team.Id
teamIds = append(teamIds, teamId)
@@ -159,7 +161,7 @@ func generateTeamsAndUsers(b *testing.B, db *sqlstore.SQLStore, users int) ([]in
globalUserId++
userIds = append(userIds, userId)
err = db.AddTeamMember(userId, 1, teamId, false, 1)
err = teamSvc.AddTeamMember(userId, 1, teamId, false, 1)
require.NoError(b, err)
}
}