Enforce "user must be able to write to the team they're editing" in set-aside data store method (#37844)

## Testing

- [x] Added/updated automated tests
This commit is contained in:
Ian Littman 2026-01-05 10:02:53 -06:00 committed by GitHub
parent 3b92554e56
commit f291ffae11
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 51 additions and 33 deletions

View file

@ -26,6 +26,33 @@ func (ds *Datastore) SetAsideLabels(ctx context.Context, notOnTeamID *uint, name
return nil
}
// Helper function to check if user has a write role on a specific team
hasWriteRoleOnTeam := func(teamID uint) bool {
for _, team := range user.Teams {
if team.ID == teamID &&
(team.Role == fleet.RoleAdmin ||
team.Role == fleet.RoleMaintainer ||
team.Role == fleet.RoleGitOps) {
return true
}
}
return false
}
// Helper function to check if user has a global write role (admin, maintainer, or gitops)
hasGlobalWriteRole := func() bool {
if user.GlobalRole == nil {
return false
}
return *user.GlobalRole == fleet.RoleAdmin ||
*user.GlobalRole == fleet.RoleMaintainer ||
*user.GlobalRole == fleet.RoleGitOps
}
if !hasGlobalWriteRole() && (notOnTeamID == nil || !hasWriteRoleOnTeam(*notOnTeamID)) {
return ctxerr.New(ctx, "you cannot edit labels on the specified team")
}
type existingLabel struct {
ID uint `db:"id"`
AuthorID *uint `db:"author_id"`
@ -50,16 +77,6 @@ func (ds *Datastore) SetAsideLabels(ctx context.Context, notOnTeamID *uint, name
return errCannotSetAside
}
// Helper function to check if user has a global write role (admin, maintainer, or gitops)
hasGlobalWriteRole := func() bool {
if user.GlobalRole == nil {
return false
}
return *user.GlobalRole == fleet.RoleAdmin ||
*user.GlobalRole == fleet.RoleMaintainer ||
*user.GlobalRole == fleet.RoleGitOps
}
// Helper function to check if user has a write role on any team
hasWriteRoleAnywhere := func() bool {
for _, team := range user.Teams {
@ -72,19 +89,6 @@ func (ds *Datastore) SetAsideLabels(ctx context.Context, notOnTeamID *uint, name
return false
}
// Helper function to check if user has a write role on a specific team
hasWriteRoleOnTeam := func(teamID uint) bool {
for _, team := range user.Teams {
if team.ID == teamID &&
(team.Role == fleet.RoleAdmin ||
team.Role == fleet.RoleMaintainer ||
team.Role == fleet.RoleGitOps) {
return true
}
}
return false
}
for _, label := range labels {
if label.TeamID == nil { // Global label
if notOnTeamID == nil { // Disallow moving aside since the label is on the same team

View file

@ -3077,7 +3077,11 @@ func testSetAsideLabels(t *testing.T, ds *Datastore) {
globalAdmin := newUser(fleet.User{GlobalRole: ptr.String(fleet.RoleAdmin)})
team1Admin := newUser(fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: team1.ID}, Role: fleet.RoleAdmin}}})
team1Maintainer := newUser(fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: team1.ID}, Role: fleet.RoleMaintainer}}})
multiTeamUser := newUser(fleet.User{Teams: []fleet.UserTeam{
multiTeamAdmin := newUser(fleet.User{Teams: []fleet.UserTeam{
{Team: fleet.Team{ID: team1.ID}, Role: fleet.RoleAdmin},
{Team: fleet.Team{ID: team2.ID}, Role: fleet.RoleAdmin},
}})
multiTeamMaintainer := newUser(fleet.User{Teams: []fleet.UserTeam{
{Team: fleet.Team{ID: team1.ID}, Role: fleet.RoleMaintainer},
{Team: fleet.Team{ID: team2.ID}, Role: fleet.RoleMaintainer},
}})
@ -3135,24 +3139,34 @@ func testSetAsideLabels(t *testing.T, ds *Datastore) {
expectError: true,
},
{
name: "team admin can set aside their own team's labels",
name: "team admin can't set aside their own team's labels if they can't edit the not-on team",
labels: []labelSpec{{name: "team1-setaside-1", teamID: &team1.ID, authorID: nil}},
notOnTeamID: &team2.ID,
user: team1Admin,
expectError: true,
},
{
name: "team admin can set aside their own team's labels if they can also edit the not-on team",
labels: []labelSpec{{name: "team1-setaside-admin", teamID: &team1.ID, authorID: nil}},
notOnTeamID: &team2.ID,
user: multiTeamAdmin,
expectError: false,
},
{
name: "team maintainer can set aside their own team's labels",
labels: []labelSpec{{name: "team1-setaside-2", teamID: &team1.ID, authorID: nil}},
name: "team maintainer can set aside their own team's labels if they can also edit the not-on team",
labels: []labelSpec{{name: "team1-setaside-maintain", teamID: &team1.ID, authorID: nil}},
notOnTeamID: &team2.ID,
user: team1Maintainer,
user: multiTeamMaintainer,
expectError: false,
},
{
name: "team gitops can set aside their own team's labels",
labels: []labelSpec{{name: "team1-setaside-3", teamID: &team1.ID, authorID: nil}},
name: "team gitops can set aside their own team's labels if they can also edit the not-on team",
labels: []labelSpec{{name: "team1-setaside-gitops", teamID: &team1.ID, authorID: nil}},
notOnTeamID: &team2.ID,
user: newUser(fleet.User{Teams: []fleet.UserTeam{{Team: fleet.Team{ID: team1.ID}, Role: fleet.RoleGitOps}}}),
user: newUser(fleet.User{Teams: []fleet.UserTeam{
{Team: fleet.Team{ID: team1.ID}, Role: fleet.RoleGitOps},
{Team: fleet.Team{ID: team2.ID}, Role: fleet.RoleGitOps},
}}),
expectError: false,
},
{
@ -3228,11 +3242,11 @@ func testSetAsideLabels(t *testing.T, ds *Datastore) {
{
name: "multiple labels can be set aside at once",
labels: []labelSpec{
{name: "multi-setaside-1", teamID: nil, authorID: &multiTeamUser.ID},
{name: "multi-setaside-1", teamID: nil, authorID: &multiTeamMaintainer.ID},
{name: "multi-setaside-2", teamID: &team2.ID, authorID: nil},
},
notOnTeamID: &team1.ID,
user: multiTeamUser,
user: multiTeamMaintainer,
expectError: false,
},
}