fix: do not remove VPP apps from team if not strictly necessary (#25411)

> For #25194

# Checklist for submitter

If some of the following don't apply, delete the relevant line.

<!-- Note that API documentation changes are now addressed by the
product design team. -->

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/Committing-Changes.md#changes-files)
for more information.
- [x] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] Added/updated automated tests
- [x] A detailed QA plan exists on the associated ticket (if it isn't
there, work with the product group's QA engineer to add it)
- [x] Manual QA for all new/changed functionality
This commit is contained in:
Jahziel Villasana-Espinoza 2025-01-14 12:31:04 -05:00 committed by GitHub
parent 3665a7c494
commit e76c3638ff
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 100 additions and 6 deletions

View file

@ -0,0 +1 @@
- Stopped VPP apps from being removed from teams whenever the VPP token team assignment is updated.

View file

@ -910,8 +910,14 @@ func (ds *Datastore) UpdateVPPTokenTeams(ctx context.Context, id uint, teams []u
stmtRemovePolicyAutomations := `UPDATE policies p
JOIN vpp_apps_teams vat ON vat.id = p.vpp_apps_teams_id AND vat.vpp_token_id = ?
SET vpp_apps_teams_id = NULL`
stmtDeleteApps := `DELETE FROM vpp_apps_teams WHERE vpp_token_id = ?`
deleteArgs := []any{id}
stmtDeleteApps := `DELETE FROM vpp_apps_teams WHERE vpp_token_id = ? %s`
var teamsFilter string
if len(teams) > 0 {
teamsFilter = "AND global_or_team_id NOT IN (?)"
}
stmtDeleteApps = fmt.Sprintf(stmtDeleteApps, teamsFilter)
var values string
var args []any
@ -956,11 +962,22 @@ func (ds *Datastore) UpdateVPPTokenTeams(ctx context.Context, id uint, teams []u
return ctxerr.Wrap(ctx, err, "vpp token null team check")
}
if _, err := tx.ExecContext(ctx, stmtRemovePolicyAutomations, deleteArgs...); err != nil {
if _, err := tx.ExecContext(ctx, stmtRemovePolicyAutomations, id); err != nil {
return ctxerr.Wrap(ctx, err, "deleting old vpp team apps policy automations")
}
if _, err := tx.ExecContext(ctx, stmtDeleteApps, deleteArgs...); err != nil {
delArgs := []any{id}
if len(teams) > 0 {
inStmt, inArgs, err := sqlx.In(stmtDeleteApps, id, teams)
if err != nil {
return ctxerr.Wrap(ctx, err, "building IN statement for deleting old vpp apps teams associations")
}
stmtDeleteApps = inStmt
delArgs = inArgs
}
if _, err := tx.ExecContext(ctx, stmtDeleteApps, delArgs...); err != nil {
return ctxerr.Wrap(ctx, err, "deleting old vpp team apps associations")
}

View file

@ -33,6 +33,7 @@ func TestVPP(t *testing.T) {
{"VPPTokenAppTeamAssociations", testVPPTokenAppTeamAssociations},
{"GetOrInsertSoftwareTitleForVPPApp", testGetOrInsertSoftwareTitleForVPPApp},
{"DeleteVPPAssignedToPolicy", testDeleteVPPAssignedToPolicy},
{"TestVPPTokenTeamAssignment", testVPPTokenTeamAssignment},
}
for _, c := range cases {
@ -802,7 +803,7 @@ func testVPPTokensCRUD(t *testing.T, ds *Datastore) {
assert.Equal(t, dataToken.Token, upTok.Token)
assert.Equal(t, orgName, upTok.OrgName)
assert.Equal(t, location, upTok.Location)
assert.NotNil(t, upTok.Teams) // "All Teams" teamm array is non-nil but empty
assert.NotNil(t, upTok.Teams) // "All Teams" team array is non-nil but empty
assert.Len(t, upTok.Teams, 0)
tok, err = ds.GetVPPToken(ctx, tok.ID)
@ -1217,7 +1218,6 @@ func testVPPTokensCRUD(t *testing.T, ds *Datastore) {
////
err = ds.DeleteVPPToken(ctx, tokNone.ID)
assert.NoError(t, err)
}
func testVPPTokenAppTeamAssociations(t *testing.T, ds *Datastore) {
@ -1468,3 +1468,79 @@ func testDeleteVPPAssignedToPolicy(t *testing.T, ds *Datastore) {
err = ds.DeleteVPPAppFromTeam(ctx, ptr.Uint(0), va1.VPPAppID)
require.NoError(t, err)
}
func testVPPTokenTeamAssignment(t *testing.T, ds *Datastore) {
ctx := context.Background()
// Create a couple of teams
team1, err := ds.NewTeam(ctx, &fleet.Team{Name: "team1" + t.Name()})
require.NoError(t, err)
team2, err := ds.NewTeam(ctx, &fleet.Team{Name: "team2" + t.Name()})
require.NoError(t, err)
dataToken, err := test.CreateVPPTokenData(time.Now().Add(24*time.Hour), "vpp_org"+t.Name(), "vpp location"+t.Name())
require.NoError(t, err)
tok1, err := ds.InsertVPPToken(ctx, dataToken)
assert.NoError(t, err)
_, err = ds.UpdateVPPTokenTeams(ctx, tok1.ID, []uint{})
assert.NoError(t, err)
// Insert some VPP apps for no team
app1 := &fleet.VPPApp{Name: "vpp_app_1", VPPAppTeam: fleet.VPPAppTeam{VPPAppID: fleet.VPPAppID{AdamID: "1", Platform: fleet.MacOSPlatform}}, BundleIdentifier: "b1"}
_, err = ds.InsertVPPAppWithTeam(ctx, app1, nil)
require.NoError(t, err)
app2 := &fleet.VPPApp{Name: "vpp_app_2", VPPAppTeam: fleet.VPPAppTeam{VPPAppID: fleet.VPPAppID{AdamID: "2", Platform: fleet.MacOSPlatform}}, BundleIdentifier: "b2"}
_, err = ds.InsertVPPAppWithTeam(ctx, app2, nil)
require.NoError(t, err)
app3 := &fleet.VPPApp{Name: "vpp_app_3", VPPAppTeam: fleet.VPPAppTeam{VPPAppID: fleet.VPPAppID{AdamID: "3", Platform: fleet.MacOSPlatform}}, BundleIdentifier: "b3"}
_, err = ds.InsertVPPAppWithTeam(ctx, app3, nil)
require.NoError(t, err)
app4 := &fleet.VPPApp{Name: "vpp_app_4", VPPAppTeam: fleet.VPPAppTeam{VPPAppID: fleet.VPPAppID{AdamID: "4", Platform: fleet.MacOSPlatform}}, BundleIdentifier: "b4"}
_, err = ds.InsertVPPAppWithTeam(ctx, app4, nil)
require.NoError(t, err)
assigned, err := ds.GetAssignedVPPApps(ctx, &team1.ID)
require.NoError(t, err)
require.Empty(t, assigned)
assigned, err = ds.GetAssignedVPPApps(ctx, &team2.ID)
require.NoError(t, err)
require.Empty(t, assigned)
// assign app1 and app2 to team1
err = ds.SetTeamVPPApps(ctx, &team1.ID, []fleet.VPPAppTeam{
{VPPAppID: app1.VPPAppID, InstallDuringSetup: ptr.Bool(true)},
{VPPAppID: app2.VPPAppID, SelfService: true},
})
require.NoError(t, err)
assigned, err = ds.GetAssignedVPPApps(ctx, &team1.ID)
require.NoError(t, err)
require.Len(t, assigned, 2)
assert.Contains(t, assigned, app1.VPPAppID)
assert.Contains(t, assigned, app2.VPPAppID)
assert.True(t, assigned[app2.VPPAppID].SelfService)
assert.True(t, *assigned[app1.VPPAppID].InstallDuringSetup)
// assign token to team1 and team 2 explicitly
_, err = ds.UpdateVPPTokenTeams(ctx, tok1.ID, []uint{team1.ID, team2.ID})
require.NoError(t, err)
// team1 should still have its apps
assigned, err = ds.GetAssignedVPPApps(ctx, &team1.ID)
require.NoError(t, err)
require.Len(t, assigned, 2)
assert.Contains(t, assigned, app1.VPPAppID)
assert.Contains(t, assigned, app2.VPPAppID)
assert.True(t, assigned[app2.VPPAppID].SelfService)
assert.True(t, *assigned[app1.VPPAppID].InstallDuringSetup)
// assign token to just team2
_, err = ds.UpdateVPPTokenTeams(ctx, tok1.ID, []uint{team2.ID})
require.NoError(t, err)
// team1 should no longer have its apps, since the token was removed
assigned, err = ds.GetAssignedVPPApps(ctx, &team1.ID)
require.NoError(t, err)
require.Empty(t, assigned)
}