Skip to content

Commit ddee95c

Browse files
kalleepkalleep
authored
Team: Create permission type for team membership (grafana#92352)
* Create permission type enum for team and remove usage of dashboard permission type
1 parent 927ce6c commit ddee95c

File tree

10 files changed

+93
-99
lines changed

10 files changed

+93
-99
lines changed

‎pkg/api/folder_bench_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import (
3131
"github.com/grafana/grafana/pkg/services/contexthandler/ctxkey"
3232
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
3333
"github.com/grafana/grafana/pkg/services/dashboards"
34-
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
3534
"github.com/grafana/grafana/pkg/services/dashboards/database"
3635
dashboardservice "github.com/grafana/grafana/pkg/services/dashboards/service"
3736
"github.com/grafana/grafana/pkg/services/featuremgmt"
@@ -261,7 +260,7 @@ func setupDB(b testing.TB) benchScenario {
261260
UserID: userID,
262261
TeamID: teamID,
263262
OrgID: orgID,
264-
Permission: dashboardaccess.PERMISSION_VIEW,
263+
Permission: team.PermissionTypeMember,
265264
Created: now,
266265
Updated: now,
267266
})

‎pkg/services/accesscontrol/database/database_test.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"github.com/grafana/grafana/pkg/services/accesscontrol"
1818
"github.com/grafana/grafana/pkg/services/accesscontrol/database"
1919
rs "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions"
20-
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
2120
"github.com/grafana/grafana/pkg/services/featuremgmt"
2221
"github.com/grafana/grafana/pkg/services/org"
2322
"github.com/grafana/grafana/pkg/services/org/orgimpl"
@@ -403,15 +402,15 @@ func createUserAndTeam(t *testing.T, store db.DB, userSrv user.Service, teamSvc
403402
})
404403
require.NoError(t, err)
405404

406-
team, err := teamSvc.CreateTeam(context.Background(), "team", "", orgID)
405+
createdTeam, err := teamSvc.CreateTeam(context.Background(), "team", "", orgID)
407406
require.NoError(t, err)
408407

409408
err = store.WithDbSession(context.Background(), func(sess *db.Session) error {
410-
return teamimpl.AddOrUpdateTeamMemberHook(sess, user.ID, orgID, team.ID, false, dashboardaccess.PERMISSION_VIEW)
409+
return teamimpl.AddOrUpdateTeamMemberHook(sess, user.ID, orgID, createdTeam.ID, false, team.PermissionTypeMember)
411410
})
412411
require.NoError(t, err)
413412

414-
return user, team
413+
return user, createdTeam
415414
}
416415

417416
type helperServices struct {
@@ -453,19 +452,19 @@ func createUsersAndTeams(t *testing.T, store db.DB, svcs helperServices, orgID i
453452
continue
454453
}
455454

456-
team, err := svcs.teamSvc.CreateTeam(context.Background(), fmt.Sprintf("team%v", i+1), "", orgID)
455+
createdTeam, err := svcs.teamSvc.CreateTeam(context.Background(), fmt.Sprintf("team%v", i+1), "", orgID)
457456
require.NoError(t, err)
458457

459458
err = store.WithDbSession(context.Background(), func(sess *db.Session) error {
460-
return teamimpl.AddOrUpdateTeamMemberHook(sess, user.ID, orgID, team.ID, false, dashboardaccess.PERMISSION_VIEW)
459+
return teamimpl.AddOrUpdateTeamMemberHook(sess, user.ID, orgID, createdTeam.ID, false, team.PermissionTypeMember)
461460
})
462461
require.NoError(t, err)
463462

464463
err = svcs.orgSvc.UpdateOrgUser(context.Background(),
465464
&org.UpdateOrgUserCommand{Role: users[i].orgRole, OrgID: orgID, UserID: user.ID})
466465
require.NoError(t, err)
467466

468-
res = append(res, dbUser{userID: user.ID, teamID: team.ID})
467+
res = append(res, dbUser{userID: user.ID, teamID: createdTeam.ID})
469468
}
470469

471470
return res

‎pkg/services/accesscontrol/ossaccesscontrol/team.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"github.com/grafana/grafana/pkg/infra/db"
1010
"github.com/grafana/grafana/pkg/services/accesscontrol"
1111
"github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions"
12-
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
1312
"github.com/grafana/grafana/pkg/services/featuremgmt"
1413
"github.com/grafana/grafana/pkg/services/licensing"
1514
"github.com/grafana/grafana/pkg/services/team"
@@ -83,9 +82,9 @@ func ProvideTeamPermissions(
8382
}
8483
switch permission {
8584
case "Member":
86-
return teamimpl.AddOrUpdateTeamMemberHook(session, user.ID, orgID, teamId, user.IsExternal, 0)
85+
return teamimpl.AddOrUpdateTeamMemberHook(session, user.ID, orgID, teamId, user.IsExternal, team.PermissionTypeMember)
8786
case "Admin":
88-
return teamimpl.AddOrUpdateTeamMemberHook(session, user.ID, orgID, teamId, user.IsExternal, dashboardaccess.PERMISSION_ADMIN)
87+
return teamimpl.AddOrUpdateTeamMemberHook(session, user.ID, orgID, teamId, user.IsExternal, team.PermissionTypeAdmin)
8988
case "":
9089
return teamimpl.RemoveTeamMemberHook(session, &team.RemoveTeamMemberCommand{
9190
OrgID: orgID,

‎pkg/services/sqlstore/migrations/accesscontrol/team_membership.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"xorm.io/xorm"
99

1010
"github.com/grafana/grafana/pkg/services/accesscontrol"
11-
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
1211
"github.com/grafana/grafana/pkg/services/org"
1312
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
1413
"github.com/grafana/grafana/pkg/services/team"
@@ -64,12 +63,12 @@ func (p *teamPermissionMigrator) setRolePermissions(roleID int64, permissions []
6463
}
6564

6665
// mapPermissionToRBAC translates the legacy membership (Member or Admin) into RBAC permissions
67-
func (p *teamPermissionMigrator) mapPermissionToRBAC(permission dashboardaccess.PermissionType, teamID int64) []accesscontrol.Permission {
66+
func (p *teamPermissionMigrator) mapPermissionToRBAC(permission team.PermissionType, teamID int64) []accesscontrol.Permission {
6867
teamIDScope := accesscontrol.Scope("teams", "id", strconv.FormatInt(teamID, 10))
6968
switch permission {
70-
case 0:
69+
case team.PermissionTypeMember:
7170
return []accesscontrol.Permission{{Action: "teams:read", Scope: teamIDScope}}
72-
case dashboardaccess.PERMISSION_ADMIN:
71+
case team.PermissionTypeAdmin:
7372
return []accesscontrol.Permission{
7473
{Action: "teams:delete", Scope: teamIDScope},
7574
{Action: "teams:read", Scope: teamIDScope},
@@ -210,7 +209,7 @@ func (p *teamPermissionMigrator) generateAssociatedPermissions(teamMemberships [
210209
// Downgrade team permissions if needed:
211210
// only admins or editors (when editorsCanAdmin option is enabled)
212211
// can access team administration endpoints
213-
if m.Permission == dashboardaccess.PERMISSION_ADMIN {
212+
if m.Permission == team.PermissionTypeAdmin {
214213
if userRolesByOrg[m.OrgID][m.UserID] == string(org.RoleViewer) || (userRolesByOrg[m.OrgID][m.UserID] == string(org.RoleEditor) && !p.editorsCanAdmin) {
215214
m.Permission = 0
216215

‎pkg/services/sqlstore/migrations/accesscontrol/test/ac_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212

1313
"github.com/grafana/grafana/pkg/infra/log"
1414
"github.com/grafana/grafana/pkg/services/accesscontrol"
15-
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
1615
"github.com/grafana/grafana/pkg/services/featuremgmt"
1716
"github.com/grafana/grafana/pkg/services/org"
1817
"github.com/grafana/grafana/pkg/services/sqlstore/migrations"
@@ -328,7 +327,7 @@ func setupTeams(t *testing.T, x *xorm.Engine) {
328327
TeamID: 1,
329328
UserID: 1,
330329
External: false,
331-
Permission: 0,
330+
Permission: team.PermissionTypeMember,
332331
Created: now,
333332
Updated: now,
334333
},
@@ -338,7 +337,7 @@ func setupTeams(t *testing.T, x *xorm.Engine) {
338337
TeamID: 1,
339338
UserID: 2,
340339
External: false,
341-
Permission: dashboardaccess.PERMISSION_ADMIN,
340+
Permission: team.PermissionTypeAdmin,
342341
Created: now,
343342
Updated: now,
344343
},
@@ -348,7 +347,7 @@ func setupTeams(t *testing.T, x *xorm.Engine) {
348347
TeamID: 1,
349348
UserID: 3,
350349
External: false,
351-
Permission: dashboardaccess.PERMISSION_ADMIN,
350+
Permission: team.PermissionTypeAdmin,
352351
Created: now,
353352
Updated: now,
354353
},
@@ -358,7 +357,7 @@ func setupTeams(t *testing.T, x *xorm.Engine) {
358357
TeamID: 1,
359358
UserID: 4,
360359
External: false,
361-
Permission: dashboardaccess.PERMISSION_ADMIN,
360+
Permission: team.PermissionTypeAdmin,
362361
Created: now,
363362
Updated: now,
364363
},
@@ -368,7 +367,7 @@ func setupTeams(t *testing.T, x *xorm.Engine) {
368367
TeamID: 2,
369368
UserID: 5,
370369
External: false,
371-
Permission: 0,
370+
Permission: team.PermissionTypeMember,
372371
Created: now,
373372
Updated: now,
374373
},

‎pkg/services/team/model.go

Lines changed: 39 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"time"
66

77
"github.com/grafana/grafana/pkg/apimachinery/identity"
8-
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
98
"github.com/grafana/grafana/pkg/services/search/model"
109
)
1110

@@ -21,9 +20,6 @@ var (
2120
ErrTeamMemberAlreadyAdded = errors.New("user is already added to this team")
2221
)
2322

24-
const MemberPermissionName = "Member"
25-
const AdminPermissionName = "Admin"
26-
2723
// Team model
2824
type Team struct {
2925
ID int64 `json:"id" xorm:"pk autoincr 'id'"`
@@ -91,15 +87,29 @@ type SearchTeamsQuery struct {
9187
}
9288

9389
type TeamDTO struct {
94-
ID int64 `json:"id" xorm:"id"`
95-
UID string `json:"uid" xorm:"uid"`
96-
OrgID int64 `json:"orgId" xorm:"org_id"`
97-
Name string `json:"name"`
98-
Email string `json:"email"`
99-
AvatarURL string `json:"avatarUrl"`
100-
MemberCount int64 `json:"memberCount"`
101-
Permission dashboardaccess.PermissionType `json:"permission"`
102-
AccessControl map[string]bool `json:"accessControl"`
90+
ID int64 `json:"id" xorm:"id"`
91+
UID string `json:"uid" xorm:"uid"`
92+
OrgID int64 `json:"orgId" xorm:"org_id"`
93+
Name string `json:"name"`
94+
Email string `json:"email"`
95+
AvatarURL string `json:"avatarUrl"`
96+
MemberCount int64 `json:"memberCount"`
97+
Permission PermissionType `json:"permission"`
98+
AccessControl map[string]bool `json:"accessControl"`
99+
}
100+
101+
type PermissionType int
102+
103+
const (
104+
PermissionTypeMember PermissionType = 0
105+
PermissionTypeAdmin PermissionType = 4
106+
)
107+
108+
func (p PermissionType) String() string {
109+
if p == PermissionTypeAdmin {
110+
return "Admin"
111+
}
112+
return "Member"
103113
}
104114

105115
type SearchTeamQueryResult struct {
@@ -116,7 +126,7 @@ type TeamMember struct {
116126
TeamID int64 `xorm:"team_id"`
117127
UserID int64 `xorm:"user_id"`
118128
External bool // Signals that the membership has been created by an external systems, such as LDAP
119-
Permission dashboardaccess.PermissionType
129+
Permission PermissionType
120130

121131
Created time.Time
122132
Updated time.Time
@@ -126,12 +136,12 @@ type TeamMember struct {
126136
// COMMANDS
127137

128138
type AddTeamMemberCommand struct {
129-
UserID int64 `json:"userId" binding:"Required"`
130-
Permission dashboardaccess.PermissionType `json:"-"`
139+
UserID int64 `json:"userId" binding:"Required"`
140+
Permission PermissionType `json:"-"`
131141
}
132142

133143
type UpdateTeamMemberCommand struct {
134-
Permission dashboardaccess.PermissionType `json:"permission"`
144+
Permission PermissionType `json:"permission"`
135145
}
136146

137147
type SetTeamMembershipsCommand struct {
@@ -161,16 +171,16 @@ type GetTeamMembersQuery struct {
161171
// Projections and DTOs
162172

163173
type TeamMemberDTO struct {
164-
OrgID int64 `json:"orgId" xorm:"org_id"`
165-
TeamID int64 `json:"teamId" xorm:"team_id"`
166-
TeamUID string `json:"teamUID" xorm:"uid"`
167-
UserID int64 `json:"userId" xorm:"user_id"`
168-
External bool `json:"-"`
169-
AuthModule string `json:"auth_module"`
170-
Email string `json:"email"`
171-
Name string `json:"name"`
172-
Login string `json:"login"`
173-
AvatarURL string `json:"avatarUrl" xorm:"avatar_url"`
174-
Labels []string `json:"labels"`
175-
Permission dashboardaccess.PermissionType `json:"permission"`
174+
OrgID int64 `json:"orgId" xorm:"org_id"`
175+
TeamID int64 `json:"teamId" xorm:"team_id"`
176+
TeamUID string `json:"teamUID" xorm:"uid"`
177+
UserID int64 `json:"userId" xorm:"user_id"`
178+
External bool `json:"-"`
179+
AuthModule string `json:"auth_module"`
180+
Email string `json:"email"`
181+
Name string `json:"name"`
182+
Login string `json:"login"`
183+
AvatarURL string `json:"avatarUrl" xorm:"avatar_url"`
184+
Labels []string `json:"labels"`
185+
Permission PermissionType `json:"permission"`
176186
}

‎pkg/services/team/teamapi/team_members.go

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/grafana/grafana/pkg/apimachinery/identity"
1313
"github.com/grafana/grafana/pkg/services/accesscontrol"
1414
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
15-
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
1615
"github.com/grafana/grafana/pkg/services/login"
1716
"github.com/grafana/grafana/pkg/services/team"
1817
"github.com/grafana/grafana/pkg/services/user"
@@ -92,7 +91,10 @@ func (tapi *TeamAPI) addTeamMember(c *contextmodel.ReqContext) response.Response
9291
return response.Error(http.StatusBadRequest, "User is already added to this team", nil)
9392
}
9493

95-
err = addOrUpdateTeamMember(c.Req.Context(), tapi.teamPermissionsService, cmd.UserID, c.SignedInUser.GetOrgID(), teamID, team.MemberPermissionName)
94+
err = addOrUpdateTeamMember(
95+
c.Req.Context(), tapi.teamPermissionsService,
96+
cmd.UserID, c.SignedInUser.GetOrgID(), teamID, team.PermissionTypeMember.String(),
97+
)
9698
if err != nil {
9799
return response.Error(http.StatusInternalServerError, "Failed to add Member to Team", err)
98100
}
@@ -135,7 +137,7 @@ func (tapi *TeamAPI) updateTeamMember(c *contextmodel.ReqContext) response.Respo
135137
return response.Error(http.StatusNotFound, "Team member not found.", nil)
136138
}
137139

138-
err = addOrUpdateTeamMember(c.Req.Context(), tapi.teamPermissionsService, userId, orgId, teamId, getPermissionName(cmd.Permission))
140+
err = addOrUpdateTeamMember(c.Req.Context(), tapi.teamPermissionsService, userId, orgId, teamId, cmd.Permission.String())
139141
if err != nil {
140142
return response.Error(http.StatusInternalServerError, "Failed to update team member.", err)
141143
}
@@ -202,13 +204,13 @@ func (tapi *TeamAPI) getTeamMembershipUpdates(ctx context.Context, orgID, teamID
202204
membersToRemove := make([]int64, 0)
203205
for _, member := range currentMemberships {
204206
if _, ok := adminEmails[member.Email]; ok {
205-
if getPermissionName(member.Permission) == team.AdminPermissionName {
207+
if member.Permission == team.PermissionTypeAdmin {
206208
delete(adminEmails, member.Email)
207209
}
208210
continue
209211
}
210212
if _, ok := memberEmails[member.Email]; ok {
211-
if getPermissionName(member.Permission) == team.MemberPermissionName {
213+
if member.Permission == team.PermissionTypeMember {
212214
delete(memberEmails, member.Email)
213215
}
214216
continue
@@ -227,10 +229,10 @@ func (tapi *TeamAPI) getTeamMembershipUpdates(ctx context.Context, orgID, teamID
227229

228230
teamMemberships := make([]accesscontrol.SetResourcePermissionCommand, 0, len(adminIDs)+len(memberIDs)+len(membersToRemove))
229231
for _, admin := range adminIDs {
230-
teamMemberships = append(teamMemberships, accesscontrol.SetResourcePermissionCommand{Permission: team.AdminPermissionName, UserID: admin})
232+
teamMemberships = append(teamMemberships, accesscontrol.SetResourcePermissionCommand{Permission: team.PermissionTypeAdmin.String(), UserID: admin})
231233
}
232234
for _, member := range memberIDs {
233-
teamMemberships = append(teamMemberships, accesscontrol.SetResourcePermissionCommand{Permission: team.MemberPermissionName, UserID: member})
235+
teamMemberships = append(teamMemberships, accesscontrol.SetResourcePermissionCommand{Permission: team.PermissionTypeMember.String(), UserID: member})
234236
}
235237
for _, member := range membersToRemove {
236238
teamMemberships = append(teamMemberships, accesscontrol.SetResourcePermissionCommand{Permission: "", UserID: member})
@@ -252,16 +254,6 @@ func (tapi *TeamAPI) getUserIDs(ctx context.Context, emails map[string]struct{})
252254
return userIDs, nil
253255
}
254256

255-
func getPermissionName(permission dashboardaccess.PermissionType) string {
256-
permissionName := permission.String()
257-
// Team member permission is 0, which maps to an empty string.
258-
// However, we want the team permission service to display "Member" for team members. This is a hack to make it work.
259-
if permissionName == "" {
260-
permissionName = team.MemberPermissionName
261-
}
262-
return permissionName
263-
}
264-
265257
// swagger:route DELETE /teams/{team_id}/members/{user_id} teams removeTeamMember
266258
//
267259
// Remove Member From Team.

0 commit comments

Comments
 (0)