From 183397194a322608c8d64d747edeb313426e7537 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Thu, 5 Jan 2023 20:08:07 +0100 Subject: [PATCH] RBAC: rewrite team member api test to not use mock (#61040) * RBAC: rewrite team member api test to not use mock --- pkg/api/team_members_test.go | 540 +++++++++++++---------------- pkg/services/team/teamtest/team.go | 3 +- 2 files changed, 249 insertions(+), 294 deletions(-) diff --git a/pkg/api/team_members_test.go b/pkg/api/team_members_test.go index 894161fcee6..98aec511cc0 100644 --- a/pkg/api/team_members_test.go +++ b/pkg/api/team_members_test.go @@ -4,12 +4,13 @@ import ( "context" "encoding/json" "fmt" - "math/rand" "net/http" "strings" "testing" + "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/db" @@ -19,15 +20,16 @@ import ( "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org/orgimpl" "github.com/grafana/grafana/pkg/services/quota/quotaimpl" - "github.com/grafana/grafana/pkg/services/quota/quotatest" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/services/team/teamimpl" + "github.com/grafana/grafana/pkg/services/team/teamtest" "github.com/grafana/grafana/pkg/services/teamguardian/database" "github.com/grafana/grafana/pkg/services/teamguardian/manager" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/userimpl" "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/grafana/pkg/web/webtest" ) type TeamGuardianMock struct { @@ -122,350 +124,302 @@ func TestTeamMembersAPIEndpoint_userLoggedIn(t *testing.T) { }) } -func createUser(db db.DB, orgId int64, t *testing.T) int64 { - quotaService := quotaimpl.ProvideService(db, setting.NewCfg()) - orgService, err := orgimpl.ProvideService(db, setting.NewCfg(), quotaService) - require.NoError(t, err) - usrSvc, err := userimpl.ProvideService(db, orgService, setting.NewCfg(), nil, nil, quotaService) - require.NoError(t, err) - - user, err := usrSvc.CreateUserForTests(context.Background(), &user.CreateUserCommand{ - Login: fmt.Sprintf("TestUser%d", rand.Int()), - OrgID: orgId, - Password: "password", - }) - require.NoError(t, err) - - return user.ID -} - -func setupTeamTestScenario(userCount int, db *sqlstore.SQLStore, orgService org.Service, t *testing.T) int64 { - teamService := teamimpl.ProvideService(db, setting.NewCfg()) // FIXME - quotaService := quotaimpl.ProvideService(db, db.Cfg) - usrSvc, err := userimpl.ProvideService(db, orgService, db.Cfg, teamService, nil, quotaService) - require.NoError(t, err) - - user, err := usrSvc.CreateUserForTests(context.Background(), &user.CreateUserCommand{SkipOrgSetup: true, Login: testUserLogin}) - require.NoError(t, err) - cmd := &org.CreateOrgCommand{Name: "TestOrg", UserID: user.ID} - testOrg, err := orgService.CreateWithMember(context.Background(), cmd) - require.NoError(t, err) - - team, err := teamService.CreateTeam("test", "test@test.com", testOrg.ID) - require.NoError(t, err) - - for i := 0; i < userCount; i++ { - userId := createUser(db, testOrg.ID, t) - require.NoError(t, err) - - err = teamService.AddTeamMember(userId, testOrg.ID, team.Id, false, 0) - require.NoError(t, err) - } - - return testOrg.ID -} - -var ( - teamMemberGetRoute = "/api/teams/%s/members" - teamMemberAddRoute = "/api/teams/%s/members" - createTeamMemberCmd = `{"userId": %d}` - teamMemberUpdateRoute = "/api/teams/%s/members/%s" - updateTeamMemberCmd = `{"permission": %d}` - teamMemberDeleteRoute = "/api/teams/%s/members/%s" -) - func TestAddTeamMembersAPIEndpoint_LegacyAccessControl(t *testing.T) { - cfg := setting.NewCfg() - cfg.RBACEnabled = false - cfg.EditorsCanAdmin = true - sc := setupHTTPServerWithCfg(t, true, cfg) - sc.hs.orgService, _ = orgimpl.ProvideService(sc.db, cfg, quotatest.New(false, nil)) - guardian := manager.ProvideService(database.ProvideTeamGuardianStore(sc.db, sc.teamService)) - sc.hs.teamGuardian = guardian - - teamMemberCount := 3 - testOrgId := setupTeamTestScenario(teamMemberCount, sc.db, sc.hs.orgService, t) - - setInitCtxSignedInOrgAdmin(sc.initCtx) - newUserId := createUser(sc.db, testOrgId, t) - input := strings.NewReader(fmt.Sprintf(createTeamMemberCmd, newUserId)) - t.Run("Organisation admins can add a team member", func(t *testing.T) { - response := callAPI(sc.server, http.MethodPost, fmt.Sprintf(teamMemberAddRoute, "1"), input, t) - assert.Equal(t, http.StatusOK, response.Code) + server := SetupAPITestServer(t, func(hs *HTTPServer) { + cfg := setting.NewCfg() + cfg.RBACEnabled = false + cfg.EditorsCanAdmin = true + hs.Cfg = cfg + hs.teamService = teamtest.NewFakeService() + store := &database.TeamGuardianStoreMock{} + store.On("GetTeamMembers", mock.Anything, mock.Anything).Return([]*models.TeamMemberDTO{ + {UserId: 2, Permission: models.PERMISSION_ADMIN}, + {UserId: 3, Permission: models.PERMISSION_VIEW}, + }, nil).Maybe() + hs.teamGuardian = manager.ProvideService(store) + hs.teamPermissionsService = &actest.FakePermissionsService{} }) - setInitCtxSignedInEditor(sc.initCtx) - sc.initCtx.IsGrafanaAdmin = true - newUserId = createUser(sc.db, testOrgId, t) - input = strings.NewReader(fmt.Sprintf(createTeamMemberCmd, newUserId)) - t.Run("Editors cannot add team members", func(t *testing.T) { - response := callAPI(sc.server, http.MethodPost, fmt.Sprintf(teamMemberAddRoute, "1"), input, t) - assert.Equal(t, http.StatusForbidden, response.Code) - }) - - err := sc.teamService.AddTeamMember(sc.initCtx.UserID, 1, 1, false, 0) - require.NoError(t, err) - input = strings.NewReader(fmt.Sprintf(createTeamMemberCmd, newUserId)) - t.Run("Team members cannot add team members", func(t *testing.T) { - response := callAPI(sc.server, http.MethodPost, fmt.Sprintf(teamMemberAddRoute, "1"), input, t) - assert.Equal(t, http.StatusForbidden, response.Code) - }) - - err = sc.teamService.UpdateTeamMember(context.Background(), &models.UpdateTeamMemberCommand{ - UserId: sc.initCtx.UserID, - OrgId: 1, - TeamId: 1, - Permission: models.PERMISSION_ADMIN, - }) - require.NoError(t, err) - input = strings.NewReader(fmt.Sprintf(createTeamMemberCmd, newUserId)) - t.Run("Team admins can add a team member", func(t *testing.T) { - response := callAPI(sc.server, http.MethodPost, fmt.Sprintf(teamMemberAddRoute, "1"), input, t) - assert.Equal(t, http.StatusOK, response.Code) - }) -} - -func TestGetTeamMembersAPIEndpoint_RBAC(t *testing.T) { - sc := setupHTTPServer(t, true) - sc.hs.orgService, _ = orgimpl.ProvideService(sc.db, sc.cfg, quotatest.New(false, nil)) - sc.hs.License = &licensing.OSSLicensingService{} - - teamMemberCount := 3 - // setupTeamTestScenario sets up 3 user (id: 2,3,4) in the team (id: 1) - testOrgId := setupTeamTestScenario(teamMemberCount, sc.db, sc.hs.orgService, t) - - setInitCtxSignedInViewer(sc.initCtx) - t.Run("Access control allows getting a team members with the right permissions", func(t *testing.T) { - setAccessControlPermissions(sc.acmock, - []ac.Permission{ - {Action: ac.ActionTeamsPermissionsRead, Scope: ac.Scope("teams", "id", "1")}, - {Action: ac.ActionOrgUsersRead, Scope: ac.ScopeUsersAll}, - }, - testOrgId, + t.Run("Admin can add team member", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPost, "/api/teams/1/members", strings.NewReader("{\"userId\": 1}")), + &user.SignedInUser{OrgID: 1, OrgRole: org.RoleAdmin}, ) - response := callAPI(sc.server, http.MethodGet, fmt.Sprintf(teamMemberGetRoute, "1"), nil, t) - require.Equal(t, http.StatusOK, response.Code) - - res := []*models.TeamMemberDTO{} - err := json.Unmarshal(response.Body.Bytes(), &res) + res, err := server.SendJSON(req) require.NoError(t, err) - require.Len(t, res, 3, "expected all team members to have been returned") + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) }) - setInitCtxSignedInOrgAdmin(sc.initCtx) - t.Run("Access control filters team members based on user permissions", func(t *testing.T) { - setAccessControlPermissions(sc.acmock, - []ac.Permission{ - {Action: ac.ActionTeamsPermissionsRead, Scope: ac.Scope("teams", "id", "1")}, - {Action: ac.ActionOrgUsersRead, Scope: ac.Scope("users", "id", "2")}, - {Action: ac.ActionOrgUsersRead, Scope: ac.Scope("users", "id", "3")}, - }, - testOrgId) - response := callAPI(sc.server, http.MethodGet, fmt.Sprintf(teamMemberGetRoute, "1"), nil, t) - require.Equal(t, http.StatusOK, response.Code) - - res := []*models.TeamMemberDTO{} - err := json.Unmarshal(response.Body.Bytes(), &res) + t.Run("Editor cannot add team member", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPost, "/api/teams/1/members", strings.NewReader("{\"userId\": 1}")), + &user.SignedInUser{OrgID: 1, OrgRole: org.RoleEditor}, + ) + res, err := server.SendJSON(req) require.NoError(t, err) - require.Len(t, res, 2, "expected a subset team members to have been returned") + assert.Equal(t, http.StatusForbidden, res.StatusCode) + require.NoError(t, res.Body.Close()) }) - setInitCtxSignedInViewer(sc.initCtx) - t.Run("Access control prevents getting a team member with incorrect scope", func(t *testing.T) { - setAccessControlPermissions(sc.acmock, - []ac.Permission{{Action: ac.ActionTeamsPermissionsRead, Scope: ac.Scope("teams", "id", "2")}}, - testOrgId) - response := callAPI(sc.server, http.MethodGet, fmt.Sprintf(teamMemberGetRoute, "1"), nil, t) - require.Equal(t, http.StatusForbidden, response.Code) + t.Run("team member cannot add members", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPost, "/api/teams/1/members", strings.NewReader("{\"userId\": 1}")), + &user.SignedInUser{UserID: 3, OrgID: 1, OrgRole: org.RoleViewer}, + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, res.StatusCode) + require.NoError(t, res.Body.Close()) + }) + + t.Run("team admin can add members", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPost, "/api/teams/1/members", strings.NewReader("{\"userId\": 1}")), + &user.SignedInUser{UserID: 2, OrgID: 1, OrgRole: org.RoleEditor}, + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) }) } func TestAddTeamMembersAPIEndpoint_RBAC(t *testing.T) { - sc := setupHTTPServer(t, true) - sc.hs.orgService, _ = orgimpl.ProvideService(sc.db, sc.cfg, quotatest.New(false, nil)) - sc.hs.License = &licensing.OSSLicensingService{} - - teamMemberCount := 3 - testOrgId := setupTeamTestScenario(teamMemberCount, sc.db, sc.hs.orgService, t) - - setInitCtxSignedInViewer(sc.initCtx) - newUserId := createUser(sc.db, testOrgId, t) - input := strings.NewReader(fmt.Sprintf(createTeamMemberCmd, newUserId)) - t.Run("Access control allows adding a team member with the right permissions", func(t *testing.T) { - setAccessControlPermissions(sc.acmock, []ac.Permission{{Action: ac.ActionTeamsPermissionsWrite, Scope: "teams:id:1"}}, 1) - response := callAPI(sc.server, http.MethodPost, fmt.Sprintf(teamMemberAddRoute, "1"), input, t) - assert.Equal(t, http.StatusOK, response.Code) + server := SetupAPITestServer(t, func(hs *HTTPServer) { + hs.Cfg = setting.NewCfg() + hs.teamService = teamtest.NewFakeService() + hs.teamPermissionsService = &actest.FakePermissionsService{} }) - setInitCtxSignedInOrgAdmin(sc.initCtx) - newUserId = createUser(sc.db, testOrgId, t) - input = strings.NewReader(fmt.Sprintf(teamCmd, newUserId)) - t.Run("Access control prevents from adding a team member with the wrong permissions", func(t *testing.T) { - setAccessControlPermissions(sc.acmock, []ac.Permission{{Action: ac.ActionTeamsPermissionsRead, Scope: "teams:id:1"}}, 1) - response := callAPI(sc.server, http.MethodPost, fmt.Sprintf(teamMemberAddRoute, "1"), input, t) - assert.Equal(t, http.StatusForbidden, response.Code) + t.Run("should be able to add team member with correct permission", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPost, "/api/teams/1/members", strings.NewReader("{\"userId\": 1}")), + userWithPermissions(1, []ac.Permission{{Action: ac.ActionTeamsPermissionsWrite, Scope: "teams:id:1"}}), + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) }) - setInitCtxSignedInViewer(sc.initCtx) - t.Run("Access control prevents adding a team member with incorrect scope", func(t *testing.T) { - setAccessControlPermissions(sc.acmock, []ac.Permission{{Action: ac.ActionTeamsPermissionsWrite, Scope: "teams:id:2"}}, 1) - response := callAPI(sc.server, http.MethodPost, fmt.Sprintf(teamMemberAddRoute, "1"), input, t) - assert.Equal(t, http.StatusForbidden, response.Code) + t.Run("should not be able to add team member without correct permission", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPost, "/api/teams/1/members", strings.NewReader("{\"userId\": 1}")), + userWithPermissions(1, []ac.Permission{{Action: ac.ActionTeamsPermissionsWrite, Scope: "teams:id:2"}}), + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, res.StatusCode) + require.NoError(t, res.Body.Close()) + }) +} + +func TestGetTeamMembersAPIEndpoint_RBAC(t *testing.T) { + server := SetupAPITestServer(t, func(hs *HTTPServer) { + hs.Cfg = setting.NewCfg() + hs.teamService = teamtest.NewFakeService() + hs.teamPermissionsService = &actest.FakePermissionsService{} + }) + + t.Run("should be able to get team members with correct permission", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewGetRequest("/api/teams/1/members"), + userWithPermissions(1, []ac.Permission{{Action: ac.ActionTeamsPermissionsRead, Scope: "teams:id:1"}}), + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) + }) + t.Run("should not be able to get team members without correct permission", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewGetRequest("/api/teams/1/members"), + userWithPermissions(1, []ac.Permission{{Action: ac.ActionTeamsPermissionsRead, Scope: "teams:id:2"}}), + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, res.StatusCode) + require.NoError(t, res.Body.Close()) }) } func TestUpdateTeamMembersAPIEndpoint_LegacyAccessControl(t *testing.T) { - cfg := setting.NewCfg() - cfg.RBACEnabled = false - cfg.EditorsCanAdmin = true - sc := setupHTTPServerWithCfg(t, true, cfg) - sc.hs.orgService, _ = orgimpl.ProvideService(sc.db, cfg, quotatest.New(false, nil)) - guardian := manager.ProvideService(database.ProvideTeamGuardianStore(sc.db, sc.teamService)) - sc.hs.teamGuardian = guardian - - teamMemberCount := 3 - setupTeamTestScenario(teamMemberCount, sc.db, sc.hs.orgService, t) - - setInitCtxSignedInOrgAdmin(sc.initCtx) - input := strings.NewReader(fmt.Sprintf(updateTeamMemberCmd, models.PERMISSION_ADMIN)) - t.Run("Organisation admins can update a team member", func(t *testing.T) { - response := callAPI(sc.server, http.MethodPut, fmt.Sprintf(teamMemberUpdateRoute, "1", "2"), input, t) - assert.Equal(t, http.StatusOK, response.Code) + server := SetupAPITestServer(t, func(hs *HTTPServer) { + cfg := setting.NewCfg() + cfg.RBACEnabled = false + cfg.EditorsCanAdmin = true + hs.Cfg = cfg + hs.teamService = &teamtest.FakeService{ExpectedIsMember: true} + store := &database.TeamGuardianStoreMock{} + store.On("GetTeamMembers", mock.Anything, mock.Anything).Return([]*models.TeamMemberDTO{ + {UserId: 2, Permission: models.PERMISSION_ADMIN}, + {UserId: 3, Permission: models.PERMISSION_VIEW}, + }, nil).Maybe() + hs.teamGuardian = manager.ProvideService(store) + hs.teamPermissionsService = &actest.FakePermissionsService{} }) - setInitCtxSignedInEditor(sc.initCtx) - sc.initCtx.IsGrafanaAdmin = true - input = strings.NewReader(fmt.Sprintf(updateTeamMemberCmd, 0)) - t.Run("Editors cannot update team members", func(t *testing.T) { - response := callAPI(sc.server, http.MethodPut, fmt.Sprintf(teamMemberUpdateRoute, "1", "2"), input, t) - assert.Equal(t, http.StatusForbidden, response.Code) + t.Run("Admin can update team member", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPut, "/api/teams/1/members/1", strings.NewReader("{\"permission\": 4}")), + &user.SignedInUser{OrgID: 1, OrgRole: org.RoleAdmin}, + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) }) - err := sc.teamService.AddTeamMember(sc.initCtx.UserID, 1, 1, false, 0) - require.NoError(t, err) - input = strings.NewReader(fmt.Sprintf(updateTeamMemberCmd, 0)) - t.Run("Team members cannot update team members", func(t *testing.T) { - response := callAPI(sc.server, http.MethodPut, fmt.Sprintf(teamMemberUpdateRoute, "1", "2"), input, t) - assert.Equal(t, http.StatusForbidden, response.Code) + t.Run("Editor cannot update team member", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPut, "/api/teams/1/members/1", strings.NewReader("{\"permission\": 4}")), + &user.SignedInUser{OrgID: 1, OrgRole: org.RoleEditor}, + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, res.StatusCode) + require.NoError(t, res.Body.Close()) }) - err = sc.teamService.UpdateTeamMember(context.Background(), &models.UpdateTeamMemberCommand{ - UserId: sc.initCtx.UserID, - OrgId: 1, - TeamId: 1, - Permission: models.PERMISSION_ADMIN, + t.Run("team member cannot update member", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPut, "/api/teams/1/members/1", strings.NewReader("{\"permission\": 4}")), + &user.SignedInUser{UserID: 3, OrgID: 1, OrgRole: org.RoleViewer}, + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, res.StatusCode) + require.NoError(t, res.Body.Close()) }) - require.NoError(t, err) - input = strings.NewReader(fmt.Sprintf(updateTeamMemberCmd, 0)) - t.Run("Team admins can update a team member", func(t *testing.T) { - response := callAPI(sc.server, http.MethodPut, fmt.Sprintf(teamMemberUpdateRoute, "1", "2"), input, t) - assert.Equal(t, http.StatusOK, response.Code) + + t.Run("team admin can add members", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPut, "/api/teams/1/members/1", strings.NewReader("{\"permission\": 4}")), + &user.SignedInUser{UserID: 2, OrgID: 1, OrgRole: org.RoleEditor}, + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) }) } func TestUpdateTeamMembersAPIEndpoint_RBAC(t *testing.T) { - sc := setupHTTPServer(t, true) - sc.hs.orgService, _ = orgimpl.ProvideService(sc.db, sc.cfg, quotatest.New(false, nil)) - sc.hs.License = &licensing.OSSLicensingService{} - - teamMemberCount := 3 - setupTeamTestScenario(teamMemberCount, sc.db, sc.hs.orgService, t) - - setInitCtxSignedInViewer(sc.initCtx) - input := strings.NewReader(fmt.Sprintf(updateTeamMemberCmd, models.PERMISSION_ADMIN)) - t.Run("Access control allows updating a team member with the right permissions", func(t *testing.T) { - setAccessControlPermissions(sc.acmock, []ac.Permission{{Action: ac.ActionTeamsPermissionsWrite, Scope: "teams:id:1"}}, 1) - response := callAPI(sc.server, http.MethodPut, fmt.Sprintf(teamMemberUpdateRoute, "1", "2"), input, t) - assert.Equal(t, http.StatusOK, response.Code) + server := SetupAPITestServer(t, func(hs *HTTPServer) { + hs.Cfg = setting.NewCfg() + hs.teamService = &teamtest.FakeService{ExpectedIsMember: true} + hs.teamPermissionsService = &actest.FakePermissionsService{} }) - setInitCtxSignedInOrgAdmin(sc.initCtx) - input = strings.NewReader(fmt.Sprintf(updateTeamMemberCmd, models.PERMISSION_ADMIN)) - t.Run("Access control prevents updating a team member with the wrong permissions", func(t *testing.T) { - setAccessControlPermissions(sc.acmock, []ac.Permission{{Action: ac.ActionTeamsPermissionsRead, Scope: "teams:id:1"}}, 1) - response := callAPI(sc.server, http.MethodPut, fmt.Sprintf(teamMemberUpdateRoute, "1", "2"), input, t) - assert.Equal(t, http.StatusForbidden, response.Code) + t.Run("should be able to update team member with correct permission", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPut, "/api/teams/1/members/1", strings.NewReader("{\"permission\": 1}")), + userWithPermissions(1, []ac.Permission{{Action: ac.ActionTeamsPermissionsWrite, Scope: "teams:id:1"}}), + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) }) - - setInitCtxSignedInViewer(sc.initCtx) - t.Run("Access control prevents updating a team member with incorrect scope", func(t *testing.T) { - setAccessControlPermissions(sc.acmock, []ac.Permission{{Action: ac.ActionTeamsPermissionsWrite, Scope: "teams:id:2"}}, 1) - response := callAPI(sc.server, http.MethodPut, fmt.Sprintf(teamMemberUpdateRoute, "1", "2"), input, t) - assert.Equal(t, http.StatusForbidden, response.Code) + t.Run("should not be able to update team member without correct permission", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPut, "/api/teams/1/members/1", strings.NewReader("{\"permission\": 1}")), + userWithPermissions(1, []ac.Permission{{Action: ac.ActionTeamsPermissionsWrite, Scope: "teams:id:2"}}), + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, res.StatusCode) + require.NoError(t, res.Body.Close()) }) } func TestDeleteTeamMembersAPIEndpoint_LegacyAccessControl(t *testing.T) { - cfg := setting.NewCfg() - cfg.RBACEnabled = false - cfg.EditorsCanAdmin = true - sc := setupHTTPServerWithCfg(t, true, cfg) - sc.hs.orgService, _ = orgimpl.ProvideService(sc.db, sc.cfg, quotatest.New(false, nil)) - guardian := manager.ProvideService(database.ProvideTeamGuardianStore(sc.db, sc.teamService)) - sc.hs.teamGuardian = guardian - - teamMemberCount := 3 - setupTeamTestScenario(teamMemberCount, sc.db, sc.hs.orgService, t) - - setInitCtxSignedInOrgAdmin(sc.initCtx) - t.Run("Organisation admins can remove a team member", func(t *testing.T) { - response := callAPI(sc.server, http.MethodDelete, fmt.Sprintf(teamMemberDeleteRoute, "1", "2"), nil, t) - assert.Equal(t, http.StatusOK, response.Code) + server := SetupAPITestServer(t, func(hs *HTTPServer) { + cfg := setting.NewCfg() + cfg.RBACEnabled = false + cfg.EditorsCanAdmin = true + hs.Cfg = cfg + hs.teamService = &teamtest.FakeService{ExpectedIsMember: true} + store := &database.TeamGuardianStoreMock{} + store.On("GetTeamMembers", mock.Anything, mock.Anything).Return([]*models.TeamMemberDTO{ + {UserId: 2, Permission: models.PERMISSION_ADMIN}, + {UserId: 3, Permission: models.PERMISSION_VIEW}, + }, nil).Maybe() + hs.teamGuardian = manager.ProvideService(store) + hs.teamPermissionsService = &actest.FakePermissionsService{} }) - setInitCtxSignedInEditor(sc.initCtx) - sc.initCtx.IsGrafanaAdmin = true - t.Run("Editors cannot remove team members", func(t *testing.T) { - response := callAPI(sc.server, http.MethodDelete, fmt.Sprintf(teamMemberDeleteRoute, "1", "3"), nil, t) - assert.Equal(t, http.StatusForbidden, response.Code) + t.Run("Admin can delete team member", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodDelete, "/api/teams/1/members/1", nil), + &user.SignedInUser{OrgID: 1, OrgRole: org.RoleAdmin}, + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) }) - err := sc.teamService.AddTeamMember(sc.initCtx.UserID, 1, 1, false, 0) - require.NoError(t, err) - t.Run("Team members cannot remove team members", func(t *testing.T) { - response := callAPI(sc.server, http.MethodDelete, fmt.Sprintf(teamMemberDeleteRoute, "1", "3"), nil, t) - assert.Equal(t, http.StatusForbidden, response.Code) + t.Run("Editor cannot delete team member", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodDelete, "/api/teams/1/members/1", nil), + &user.SignedInUser{OrgID: 1, OrgRole: org.RoleEditor}, + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, res.StatusCode) + require.NoError(t, res.Body.Close()) }) - err = sc.teamService.UpdateTeamMember(context.Background(), &models.UpdateTeamMemberCommand{ - UserId: sc.initCtx.UserID, - OrgId: 1, - TeamId: 1, - Permission: models.PERMISSION_ADMIN, + t.Run("team member cannot delete member", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodDelete, "/api/teams/1/members/1", nil), + &user.SignedInUser{UserID: 3, OrgID: 1, OrgRole: org.RoleViewer}, + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, res.StatusCode) + require.NoError(t, res.Body.Close()) }) - require.NoError(t, err) - t.Run("Team admins can remove a team member", func(t *testing.T) { - response := callAPI(sc.server, http.MethodDelete, fmt.Sprintf(teamMemberDeleteRoute, "1", "3"), nil, t) - assert.Equal(t, http.StatusOK, response.Code) + + t.Run("team admin can delete members", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodDelete, "/api/teams/1/members/1", nil), + &user.SignedInUser{UserID: 2, OrgID: 1, OrgRole: org.RoleEditor}, + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) }) } func TestDeleteTeamMembersAPIEndpoint_RBAC(t *testing.T) { - sc := setupHTTPServer(t, true) - sc.hs.orgService, _ = orgimpl.ProvideService(sc.db, sc.cfg, quotatest.New(false, nil)) - sc.hs.License = &licensing.OSSLicensingService{} - - teamMemberCount := 3 - setupTeamTestScenario(teamMemberCount, sc.db, sc.hs.orgService, t) - - setInitCtxSignedInViewer(sc.initCtx) - t.Run("Access control allows removing a team member with the right permissions", func(t *testing.T) { - setAccessControlPermissions(sc.acmock, []ac.Permission{{Action: ac.ActionTeamsPermissionsWrite, Scope: "teams:id:1"}}, 1) - response := callAPI(sc.server, http.MethodDelete, fmt.Sprintf(teamMemberDeleteRoute, "1", "2"), nil, t) - assert.Equal(t, http.StatusOK, response.Code) + server := SetupAPITestServer(t, func(hs *HTTPServer) { + hs.Cfg = setting.NewCfg() + hs.teamService = &teamtest.FakeService{ExpectedIsMember: true} + hs.teamPermissionsService = &actest.FakePermissionsService{} }) - setInitCtxSignedInOrgAdmin(sc.initCtx) - t.Run("Access control prevents removing a team member with the wrong permissions", func(t *testing.T) { - setAccessControlPermissions(sc.acmock, []ac.Permission{{Action: ac.ActionTeamsPermissionsRead, Scope: "teams:id:1"}}, 1) - response := callAPI(sc.server, http.MethodDelete, fmt.Sprintf(teamMemberDeleteRoute, "1", "3"), nil, t) - assert.Equal(t, http.StatusForbidden, response.Code) + t.Run("should be able to delete team member with correct permission", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodDelete, "/api/teams/1/members/1", nil), + userWithPermissions(1, []ac.Permission{{Action: ac.ActionTeamsPermissionsWrite, Scope: "teams:id:1"}}), + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) }) - - setInitCtxSignedInViewer(sc.initCtx) - t.Run("Access control prevents removing a team member with incorrect scope", func(t *testing.T) { - setAccessControlPermissions(sc.acmock, []ac.Permission{{Action: ac.ActionTeamsPermissionsWrite, Scope: "teams:id:2"}}, 1) - response := callAPI(sc.server, http.MethodDelete, fmt.Sprintf(teamMemberDeleteRoute, "1", "3"), nil, t) - assert.Equal(t, http.StatusForbidden, response.Code) + t.Run("should not be able to delete member without correct permission", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodDelete, "/api/teams/1/members/1", nil), + userWithPermissions(1, []ac.Permission{{Action: ac.ActionTeamsPermissionsWrite, Scope: "teams:id:2"}}), + ) + res, err := server.SendJSON(req) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, res.StatusCode) + require.NoError(t, res.Body.Close()) }) } diff --git a/pkg/services/team/teamtest/team.go b/pkg/services/team/teamtest/team.go index 035a00ab8ac..f9fe0f02b3f 100644 --- a/pkg/services/team/teamtest/team.go +++ b/pkg/services/team/teamtest/team.go @@ -8,6 +8,7 @@ import ( type FakeService struct { ExpectedTeam models.Team + ExpectedIsMember bool ExpectedTeamDTO *models.TeamDTO ExpectedTeamsByUser []*models.TeamDTO ExpectedMembers []*models.TeamMemberDTO @@ -53,7 +54,7 @@ func (s *FakeService) UpdateTeamMember(ctx context.Context, cmd *models.UpdateTe } func (s *FakeService) IsTeamMember(orgId int64, teamId int64, userId int64) (bool, error) { - return false, s.ExpectedError + return s.ExpectedIsMember, s.ExpectedError } func (s *FakeService) RemoveTeamMember(ctx context.Context, cmd *models.RemoveTeamMemberCommand) error {