diff --git a/pkg/services/accesscontrol/resourcepermissions/api.go b/pkg/services/accesscontrol/resourcepermissions/api.go index c979b8ade0b..4dc8aafd51f 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api.go +++ b/pkg/services/accesscontrol/resourcepermissions/api.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/services/team" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" "go.opentelemetry.io/otel" @@ -42,21 +43,27 @@ func (a *api) registerEndpoints() { licenseMW = nopMiddleware } + teamUIDResolver := team.MiddlewareTeamUIDResolver(a.service.teamService, ":teamID") + teamUIDResolverResource := func() web.Handler { return func(c *contextmodel.ReqContext) {} }() // no-op + if a.service.options.Resource == "teams" { + teamUIDResolverResource = team.MiddlewareTeamUIDResolver(a.service.teamService, ":resourceID") + } + a.router.Group(fmt.Sprintf("/api/access-control/%s", a.service.options.Resource), func(r routing.RouteRegister) { actionRead := fmt.Sprintf("%s.permissions:read", a.service.options.Resource) actionWrite := fmt.Sprintf("%s.permissions:write", a.service.options.Resource) scope := accesscontrol.Scope(a.service.options.Resource, a.service.options.ResourceAttribute, accesscontrol.Parameter(":resourceID")) r.Get("/description", auth(accesscontrol.EvalPermission(actionRead)), routing.Wrap(a.getDescription)) - r.Get("/:resourceID", auth(accesscontrol.EvalPermission(actionRead, scope)), routing.Wrap(a.getPermissions)) - r.Post("/:resourceID", licenseMW, auth(accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setPermissions)) + r.Get("/:resourceID", teamUIDResolverResource, auth(accesscontrol.EvalPermission(actionRead, scope)), routing.Wrap(a.getPermissions)) + r.Post("/:resourceID", teamUIDResolverResource, licenseMW, auth(accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setPermissions)) if a.service.options.Assignments.Users { - r.Post("/:resourceID/users/:userID", licenseMW, auth(accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setUserPermission)) + r.Post("/:resourceID/users/:userID", licenseMW, teamUIDResolverResource, auth(accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setUserPermission)) } if a.service.options.Assignments.Teams { - r.Post("/:resourceID/teams/:teamID", licenseMW, auth(accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setTeamPermission)) + r.Post("/:resourceID/teams/:teamID", licenseMW, teamUIDResolverResource, teamUIDResolver, auth(accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setTeamPermission)) } if a.service.options.Assignments.BuiltInRoles { - r.Post("/:resourceID/builtInRoles/:builtInRole", licenseMW, auth(accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setBuiltinRolePermission)) + r.Post("/:resourceID/builtInRoles/:builtInRole", teamUIDResolverResource, licenseMW, auth(accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setBuiltinRolePermission)) } }) } diff --git a/pkg/services/accesscontrol/resourcepermissions/api_test.go b/pkg/services/accesscontrol/resourcepermissions/api_test.go index 8719451294b..ea71d5c2b3a 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/api_test.go @@ -257,6 +257,7 @@ type setTeamPermissionTestCase struct { expectedStatus int permission string permissions []accesscontrol.Permission + byUID bool } func TestApi_setTeamPermission(t *testing.T) { @@ -308,6 +309,20 @@ func TestApi_setTeamPermission(t *testing.T) { {Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}, }, }, + { + desc: "should set View permission for team with id 1 but through UID", + teamID: 1, + resourceID: "1", + expectedStatus: 200, + permission: "View", + byUID: true, + permissions: []accesscontrol.Permission{ + {Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}, + {Action: "dashboards.permissions:write", Scope: "dashboards:id:1"}, + {Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll}, + {Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll}, + }, + }, } for _, tt := range tests { @@ -316,10 +331,16 @@ func TestApi_setTeamPermission(t *testing.T) { server := setupTestServer(t, &user.SignedInUser{OrgID: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByActionContext(context.Background(), tt.permissions)}}, service) // seed team - _, err := teamSvc.CreateTeam(context.Background(), "test", "test@test.com", 1) + team, err := teamSvc.CreateTeam(context.Background(), "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))) + assignTo := strconv.Itoa(int(tt.teamID)) + if tt.byUID { + if team.ID == tt.teamID { + assignTo = team.UID + } + } + recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "teams", assignTo) assert.Equal(t, tt.expectedStatus, recorder.Code) assert.Equal(t, tt.expectedStatus, recorder.Code) diff --git a/pkg/services/team/model.go b/pkg/services/team/model.go index debb2cd32d9..1fcc79f941f 100644 --- a/pkg/services/team/model.go +++ b/pkg/services/team/model.go @@ -54,8 +54,10 @@ type DeleteTeamCommand struct { } type GetTeamByIDQuery struct { - OrgID int64 + OrgID int64 + // Get team by ID or UID. If ID is set, UID is ignored. ID int64 + UID string SignedInUser identity.Requester HiddenUsers map[string]struct{} } diff --git a/pkg/services/team/team.go b/pkg/services/team/team.go index 3a44a8883fa..3df15bcddc1 100644 --- a/pkg/services/team/team.go +++ b/pkg/services/team/team.go @@ -2,6 +2,10 @@ package team import ( "context" + "strconv" + + contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" + "github.com/grafana/grafana/pkg/web" ) type Service interface { @@ -18,3 +22,24 @@ type Service interface { GetTeamMembers(ctx context.Context, query *GetTeamMembersQuery) ([]*TeamMemberDTO, error) RegisterDelete(query string) } + +func MiddlewareTeamUIDResolver(teamService Service, paramName string) web.Handler { + return func(c *contextmodel.ReqContext) { + // Get team id from request, fetch team and replace teamId with team id + teamID := web.Params(c.Req)[paramName] + // if teamID is empty or is an integer, we assume it's a team id and we don't need to resolve it + _, err := strconv.ParseInt(teamID, 10, 64) + if teamID == "" || err == nil { + return + } + + team, err := teamService.GetTeamByID(c.Req.Context(), &GetTeamByIDQuery{UID: teamID, OrgID: c.OrgID}) + if err == nil { + gotParams := web.Params(c.Req) + gotParams[paramName] = strconv.FormatInt(team.ID, 10) + web.SetURLParams(c.Req, gotParams) + } else { + c.JsonApiErr(404, "Not found", nil) + } + } +} diff --git a/pkg/services/team/teamapi/api.go b/pkg/services/team/teamapi/api.go index 955c6f119d2..f18072436ed 100644 --- a/pkg/services/team/teamapi/api.go +++ b/pkg/services/team/teamapi/api.go @@ -55,34 +55,35 @@ func ProvideTeamAPI( func (tapi *TeamAPI) registerRoutes(router routing.RouteRegister, ac accesscontrol.AccessControl) { authorize := accesscontrol.Middleware(ac) + teamResolver := team.MiddlewareTeamUIDResolver(tapi.teamService, ":teamId") router.Group("/api", func(apiRoute routing.RouteRegister) { // team (admin permission required) apiRoute.Group("/teams", func(teamsRoute routing.RouteRegister) { teamsRoute.Post("/", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsCreate)), routing.Wrap(tapi.createTeam)) - teamsRoute.Put("/:teamId", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsWrite, + teamsRoute.Put("/:teamId", teamResolver, authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsWrite, accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.updateTeam)) - teamsRoute.Delete("/:teamId", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsDelete, + teamsRoute.Delete("/:teamId", teamResolver, authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsDelete, accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.deleteTeamByID)) - teamsRoute.Get("/:teamId/members", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsRead, + teamsRoute.Get("/:teamId/members", teamResolver, authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsRead, accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.getTeamMembers)) - teamsRoute.Post("/:teamId/members", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite, + teamsRoute.Post("/:teamId/members", teamResolver, authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite, accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.addTeamMember)) - teamsRoute.Put("/:teamId/members/:userId", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite, + teamsRoute.Put("/:teamId/members/:userId", teamResolver, authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite, accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.updateTeamMember)) - teamsRoute.Put("/:teamId/members", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite, + teamsRoute.Put("/:teamId/members", teamResolver, authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite, accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.setTeamMemberships)) - teamsRoute.Delete("/:teamId/members/:userId", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite, + teamsRoute.Delete("/:teamId/members/:userId", teamResolver, authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite, accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.removeTeamMember)) - teamsRoute.Get("/:teamId/preferences", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsRead, + teamsRoute.Get("/:teamId/preferences", teamResolver, authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsRead, accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.getTeamPreferences)) - teamsRoute.Put("/:teamId/preferences", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsWrite, + teamsRoute.Put("/:teamId/preferences", teamResolver, authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsWrite, accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.updateTeamPreferences)) }, requestmeta.SetOwner(requestmeta.TeamAuth)) // team without requirement of user to be org admin apiRoute.Group("/teams", func(teamsRoute routing.RouteRegister) { - teamsRoute.Get("/:teamId", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsRead, + teamsRoute.Get("/:teamId", teamResolver, authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsRead, accesscontrol.ScopeTeamsID)), routing.Wrap(tapi.getTeamByID)) teamsRoute.Get("/search", authorize(accesscontrol.EvalPermission(accesscontrol.ActionTeamsRead)), routing.Wrap(tapi.searchTeams)) diff --git a/pkg/services/team/teamapi/team_members_test.go b/pkg/services/team/teamapi/team_members_test.go index fd616925b31..43727d6e5cf 100644 --- a/pkg/services/team/teamapi/team_members_test.go +++ b/pkg/services/team/teamapi/team_members_test.go @@ -28,14 +28,18 @@ import ( "github.com/grafana/grafana/pkg/web/webtest" ) -func SetupAPITestServer(t *testing.T, opts ...func(a *TeamAPI)) *webtest.Server { +func SetupAPITestServer(t *testing.T, teamService team.Service, opts ...func(a *TeamAPI)) *webtest.Server { t.Helper() router := routing.NewRouteRegister() cfg := setting.NewCfg() cfg.LDAPAuthEnabled = true + if teamService == nil { + teamService = teamtest.NewFakeService() + } + a := ProvideTeamAPI(router, - teamtest.NewFakeService(), + teamService, actest.FakeService{}, acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient()), &actest.FakePermissionsService{}, @@ -55,7 +59,7 @@ func SetupAPITestServer(t *testing.T, opts ...func(a *TeamAPI)) *webtest.Server } func TestAddTeamMembersAPIEndpoint(t *testing.T) { - server := SetupAPITestServer(t) + server := SetupAPITestServer(t, &teamtest.FakeService{ExpectedTeamDTO: &team.TeamDTO{ID: 1, UID: "a00001"}}) t.Run("should be able to add team member with correct permission", func(t *testing.T) { req := webtest.RequestWithSignedInUser( @@ -68,6 +72,17 @@ func TestAddTeamMembersAPIEndpoint(t *testing.T) { require.NoError(t, res.Body.Close()) }) + t.Run("should be able to add team member with correct permission by UID", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPost, "/api/teams/a00001/members", strings.NewReader("{\"userId\": 1}")), + authedUserWithPermissions(1, 1, []accesscontrol.Permission{{Action: accesscontrol.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()) + }) + 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}")), @@ -81,7 +96,7 @@ func TestAddTeamMembersAPIEndpoint(t *testing.T) { } func TestGetTeamMembersAPIEndpoint(t *testing.T) { - server := SetupAPITestServer(t) + server := SetupAPITestServer(t, &teamtest.FakeService{ExpectedIsMember: true, ExpectedTeamDTO: &team.TeamDTO{ID: 1, UID: "a00001"}}) t.Run("should be able to get team members with correct permission", func(t *testing.T) { req := webtest.RequestWithSignedInUser( @@ -93,6 +108,18 @@ func TestGetTeamMembersAPIEndpoint(t *testing.T) { assert.Equal(t, http.StatusOK, res.StatusCode) require.NoError(t, res.Body.Close()) }) + + t.Run("should be able to get team members with correct permission by UID", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewGetRequest("/api/teams/a00001/members"), + authedUserWithPermissions(1, 1, []accesscontrol.Permission{{Action: accesscontrol.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"), @@ -106,9 +133,7 @@ func TestGetTeamMembersAPIEndpoint(t *testing.T) { } func TestUpdateTeamMembersAPIEndpoint(t *testing.T) { - server := SetupAPITestServer(t, func(hs *TeamAPI) { - hs.teamService = &teamtest.FakeService{ExpectedIsMember: true} - }) + server := SetupAPITestServer(t, &teamtest.FakeService{ExpectedIsMember: true, ExpectedTeamDTO: &team.TeamDTO{ID: 1, UID: "a00001"}}) t.Run("should be able to update team member with correct permission", func(t *testing.T) { req := webtest.RequestWithSignedInUser( @@ -120,6 +145,18 @@ func TestUpdateTeamMembersAPIEndpoint(t *testing.T) { assert.Equal(t, http.StatusOK, res.StatusCode) require.NoError(t, res.Body.Close()) }) + + t.Run("should be able to update team member with correct permission by team UID", func(t *testing.T) { + req := webtest.RequestWithSignedInUser( + server.NewRequest(http.MethodPut, "/api/teams/a00001/members/1", strings.NewReader("{\"permission\": 1}")), + authedUserWithPermissions(1, 1, []accesscontrol.Permission{{Action: accesscontrol.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()) + }) + 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}")), @@ -133,7 +170,7 @@ func TestUpdateTeamMembersAPIEndpoint(t *testing.T) { } func TestDeleteTeamMembersAPIEndpoint(t *testing.T) { - server := SetupAPITestServer(t, func(hs *TeamAPI) { + server := SetupAPITestServer(t, nil, func(hs *TeamAPI) { hs.teamService = &teamtest.FakeService{ExpectedIsMember: true} hs.teamPermissionsService = &actest.FakePermissionsService{} }) diff --git a/pkg/services/team/teamapi/team_test.go b/pkg/services/team/teamapi/team_test.go index 4cd3f541046..ba95cf53192 100644 --- a/pkg/services/team/teamapi/team_test.go +++ b/pkg/services/team/teamapi/team_test.go @@ -21,16 +21,14 @@ import ( const ( searchTeamsURL = "/api/teams/search" createTeamURL = "/api/teams/" - detailTeamURL = "/api/teams/%d" - detailTeamPreferenceURL = "/api/teams/%d/preferences" + detailTeamURL = "/api/teams/%v" + detailTeamPreferenceURL = "/api/teams/%v/preferences" teamCmd = `{"name": "MyTestTeam%d"}` teamPreferenceCmd = `{"theme": "dark"}` ) func TestTeamAPIEndpoint_CreateTeam(t *testing.T) { - server := SetupAPITestServer(t, func(hs *TeamAPI) { - hs.teamService = teamtest.NewFakeService() - }) + server := SetupAPITestServer(t, nil) input := strings.NewReader(fmt.Sprintf(teamCmd, 1)) t.Run("Access control allows creating teams with the correct permissions", func(t *testing.T) { @@ -54,9 +52,7 @@ func TestTeamAPIEndpoint_CreateTeam(t *testing.T) { } func TestTeamAPIEndpoint_SearchTeams(t *testing.T) { - server := SetupAPITestServer(t, func(hs *TeamAPI) { - hs.teamService = teamtest.NewFakeService() - }) + server := SetupAPITestServer(t, nil) t.Run("Access control prevents searching for teams with the incorrect permissions", func(t *testing.T) { req := server.NewGetRequest(searchTeamsURL) @@ -80,9 +76,7 @@ func TestTeamAPIEndpoint_SearchTeams(t *testing.T) { } func TestTeamAPIEndpoint_GetTeamByID(t *testing.T) { - server := SetupAPITestServer(t, func(hs *TeamAPI) { - hs.teamService = &teamtest.FakeService{ExpectedTeamDTO: &team.TeamDTO{}} - }) + server := SetupAPITestServer(t, &teamtest.FakeService{ExpectedTeamDTO: &team.TeamDTO{ID: 1, UID: "a00001"}}) url := fmt.Sprintf(detailTeamURL, 1) @@ -106,6 +100,30 @@ func TestTeamAPIEndpoint_GetTeamByID(t *testing.T) { require.NoError(t, res.Body.Close()) }) + t.Run("Access control prevents getting a team when missing permissions by UID", func(t *testing.T) { + url := fmt.Sprintf(detailTeamURL, "a00001") + req := server.NewGetRequest(url) + req = webtest.RequestWithSignedInUser(req, authedUserWithPermissions(1, 1, []accesscontrol.Permission{ + {Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:2"}, + })) + res, err := server.Send(req) + require.NoError(t, err) + assert.Equal(t, http.StatusForbidden, res.StatusCode) + require.NoError(t, res.Body.Close()) + }) + + t.Run("Access control allows getting a team by UID with the correct permissions", func(t *testing.T) { + url := fmt.Sprintf(detailTeamURL, "a00001") + req := server.NewGetRequest(url) + req = webtest.RequestWithSignedInUser(req, authedUserWithPermissions(1, 1, []accesscontrol.Permission{ + {Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}, + })) + res, err := server.Send(req) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) + }) + t.Run("Access control allows getting a team with wildcard scope", func(t *testing.T) { req := server.NewGetRequest(url) req = webtest.RequestWithSignedInUser(req, authedUserWithPermissions(1, 1, []accesscontrol.Permission{ @@ -122,11 +140,9 @@ func TestTeamAPIEndpoint_GetTeamByID(t *testing.T) { // Then the endpoint should return 200 if the user has accesscontrol.ActionTeamsWrite with teams:id:1 scope // else return 403 func TestTeamAPIEndpoint_UpdateTeam(t *testing.T) { - server := SetupAPITestServer(t, func(hs *TeamAPI) { - hs.teamService = &teamtest.FakeService{ExpectedTeamDTO: &team.TeamDTO{}} - }) + server := SetupAPITestServer(t, &teamtest.FakeService{ExpectedTeamDTO: &team.TeamDTO{ID: 1, UID: "a00001"}}) - request := func(teamID int64, user *user.SignedInUser) (*http.Response, error) { + request := func(teamID any, user *user.SignedInUser) (*http.Response, error) { req := server.NewRequest(http.MethodPut, fmt.Sprintf(detailTeamURL, teamID), strings.NewReader(teamCmd)) req = webtest.RequestWithSignedInUser(req, user) return server.SendJSON(req) @@ -141,6 +157,15 @@ func TestTeamAPIEndpoint_UpdateTeam(t *testing.T) { require.NoError(t, res.Body.Close()) }) + t.Run("Access control allows updating team by UID with the correct permissions", func(t *testing.T) { + res, err := request("a00001", authedUserWithPermissions(1, 1, []accesscontrol.Permission{ + {Action: accesscontrol.ActionTeamsWrite, Scope: "teams:id:1"}, + })) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) + }) + t.Run("Access control allows updating teams with the wildcard scope", func(t *testing.T) { res, err := request(1, authedUserWithPermissions(1, 1, []accesscontrol.Permission{ {Action: accesscontrol.ActionTeamsWrite, Scope: "teams:*"}, @@ -164,11 +189,9 @@ func TestTeamAPIEndpoint_UpdateTeam(t *testing.T) { // Then the endpoint should return 200 if the user has accesscontrol.ActionTeamsDelete with teams:id:1 scope // else return 403 func TestTeamAPIEndpoint_DeleteTeam(t *testing.T) { - server := SetupAPITestServer(t, func(hs *TeamAPI) { - hs.teamService = &teamtest.FakeService{ExpectedTeamDTO: &team.TeamDTO{}} - }) + server := SetupAPITestServer(t, &teamtest.FakeService{ExpectedTeamDTO: &team.TeamDTO{ID: 1, UID: "a00001"}}) - request := func(teamID int64, user *user.SignedInUser) (*http.Response, error) { + request := func(teamID any, user *user.SignedInUser) (*http.Response, error) { req := server.NewRequest(http.MethodDelete, fmt.Sprintf(detailTeamURL, teamID), http.NoBody) req = webtest.RequestWithSignedInUser(req, user) return server.Send(req) @@ -191,17 +214,26 @@ func TestTeamAPIEndpoint_DeleteTeam(t *testing.T) { assert.Equal(t, http.StatusOK, res.StatusCode) require.NoError(t, res.Body.Close()) }) + + t.Run("Access control allows deleting teams with the correct permissions by UID", func(t *testing.T) { + res, err := request("a00001", authedUserWithPermissions(1, 1, []accesscontrol.Permission{ + {Action: accesscontrol.ActionTeamsDelete, Scope: "teams:id:1"}, + })) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) + }) } // Given a team with a user, when the user is granted X permission, // Then the endpoint should return 200 if the user has accesscontrol.ActionTeamsRead with teams:id:1 scope // else return 403 func TestTeamAPIEndpoint_GetTeamPreferences(t *testing.T) { - server := SetupAPITestServer(t, func(hs *TeamAPI) { + server := SetupAPITestServer(t, &teamtest.FakeService{ExpectedTeamDTO: &team.TeamDTO{ID: 1, UID: "a00001"}}, func(hs *TeamAPI) { hs.preferenceService = &preftest.FakePreferenceService{ExpectedPreference: &pref.Preference{}} }) - request := func(teamID int64, user *user.SignedInUser) (*http.Response, error) { + request := func(teamID any, user *user.SignedInUser) (*http.Response, error) { req := server.NewGetRequest(fmt.Sprintf(detailTeamPreferenceURL, teamID)) req = webtest.RequestWithSignedInUser(req, user) return server.Send(req) @@ -216,6 +248,15 @@ func TestTeamAPIEndpoint_GetTeamPreferences(t *testing.T) { require.NoError(t, res.Body.Close()) }) + t.Run("Access control allows getting team preferences with the correct permissions by UID", func(t *testing.T) { + res, err := request("a00001", authedUserWithPermissions(1, 1, []accesscontrol.Permission{ + {Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:1"}, + })) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, res.Body.Close()) + }) + t.Run("Access control prevents getting team preferences with the incorrect permissions", func(t *testing.T) { res, err := request(1, authedUserWithPermissions(1, 1, []accesscontrol.Permission{ {Action: accesscontrol.ActionTeamsRead, Scope: "teams:id:2"}, @@ -230,7 +271,7 @@ func TestTeamAPIEndpoint_GetTeamPreferences(t *testing.T) { // Then the endpoint should return 200 if the user has accesscontrol.ActionTeamsWrite with teams:id:1 scope // else return 403 func TestTeamAPIEndpoint_UpdateTeamPreferences(t *testing.T) { - server := SetupAPITestServer(t, func(hs *TeamAPI) { + server := SetupAPITestServer(t, nil, func(hs *TeamAPI) { hs.preferenceService = &preftest.FakePreferenceService{ExpectedPreference: &pref.Preference{}} }) diff --git a/pkg/services/team/teamimpl/store.go b/pkg/services/team/teamimpl/store.go index e1069cd9f4e..23824060677 100644 --- a/pkg/services/team/teamimpl/store.go +++ b/pkg/services/team/teamimpl/store.go @@ -3,6 +3,7 @@ package teamimpl import ( "bytes" "context" + "errors" "fmt" "strings" "time" @@ -268,6 +269,12 @@ func (ss *xormStore) Search(ctx context.Context, query *team.SearchTeamsQuery) ( func (ss *xormStore) GetByID(ctx context.Context, query *team.GetTeamByIDQuery) (*team.TeamDTO, error) { var queryResult *team.TeamDTO + + // Check if both ID and UID are unset + if query.ID == 0 && query.UID == "" { + return nil, errors.New("either ID or UID must be set") + } + err := ss.db.WithDbSession(ctx, func(sess *db.Session) error { var sql bytes.Buffer params := make([]any, 0) @@ -278,8 +285,14 @@ func (ss *xormStore) GetByID(ctx context.Context, query *team.GetTeamByIDQuery) params = append(params, user) } - sql.WriteString(` WHERE team.org_id = ? and team.id = ?`) - params = append(params, query.OrgID, query.ID) + // Prioritize ID over UID + if query.ID != 0 { + sql.WriteString(` WHERE team.org_id = ? and team.id = ?`) + params = append(params, query.OrgID, query.ID) + } else { + sql.WriteString(` WHERE team.org_id = ? and team.uid = ?`) + params = append(params, query.OrgID, query.UID) + } var t team.TeamDTO exists, err := sess.SQL(sql.String(), params...).Get(&t) diff --git a/pkg/services/team/teamimpl/team.go b/pkg/services/team/teamimpl/team.go index e7c9537b751..e31e6c75db6 100644 --- a/pkg/services/team/teamimpl/team.go +++ b/pkg/services/team/teamimpl/team.go @@ -64,6 +64,7 @@ func (s *Service) GetTeamByID(ctx context.Context, query *team.GetTeamByIDQuery) ctx, span := s.tracer.Start(ctx, "team.GetTeamByID", trace.WithAttributes( attribute.Int64("orgID", query.OrgID), attribute.Int64("teamID", query.ID), + attribute.String("teamUID", query.UID), )) defer span.End() return s.store.GetByID(ctx, query) diff --git a/public/app/features/teams/TeamList.tsx b/public/app/features/teams/TeamList.tsx index 7faa4f46f57..a96bd633fd0 100644 --- a/public/app/features/teams/TeamList.tsx +++ b/public/app/features/teams/TeamList.tsx @@ -39,6 +39,7 @@ export interface State { // this is dummy data to pass to the table while the real data is loading const skeletonData: Team[] = new Array(3).fill(null).map((_, index) => ({ id: index, + uid: '', memberCount: 0, name: '', orgId: 0, diff --git a/public/app/features/teams/__mocks__/teamMocks.ts b/public/app/features/teams/__mocks__/teamMocks.ts index e3f21d7c390..0ee357c4daa 100644 --- a/public/app/features/teams/__mocks__/teamMocks.ts +++ b/public/app/features/teams/__mocks__/teamMocks.ts @@ -12,6 +12,7 @@ export const getMultipleMockTeams = (numberOfTeams: number): Team[] => { export const getMockTeam = (i = 1, overrides = {}): Team => { return { id: i, + uid: '', name: `test-${i}`, avatarUrl: 'some/url/', email: `test-${i}@test.com`, diff --git a/public/app/features/teams/state/navModel.ts b/public/app/features/teams/state/navModel.ts index 3a676de1c28..8969845b0e9 100644 --- a/public/app/features/teams/state/navModel.ts +++ b/public/app/features/teams/state/navModel.ts @@ -9,6 +9,7 @@ import { AccessControlAction, Team, TeamPermissionLevel } from 'app/types'; const loadingTeam = { avatarUrl: 'public/img/user_profile.png', id: 1, + uid: '', name: 'Loading', email: 'loading', memberCount: 0, diff --git a/public/app/types/teams.ts b/public/app/types/teams.ts index 325bb908b0d..52407a544f9 100644 --- a/public/app/types/teams.ts +++ b/public/app/types/teams.ts @@ -15,6 +15,7 @@ export interface TeamDTO { // This is the team resource with permissions and metadata expanded export interface Team { id: number; // TODO switch to UUID + uid: string; // Prefer UUID /** * AccessControl metadata associated with a given resource.