From 545f6a4ec266a2a02c1e21e37b904ae32143aed2 Mon Sep 17 00:00:00 2001 From: Dante Catalfamo <43040593+dantecatalfamo@users.noreply.github.com> Date: Fri, 6 Sep 2024 18:37:40 -0400 Subject: [PATCH] Remove the ability to have fallback 'all teams' token (#21893) --- server/datastore/mysql/vpp.go | 45 +++++--- server/datastore/mysql/vpp_test.go | 143 +++++++++++-------------- server/service/integration_mdm_test.go | 6 ++ 3 files changed, 102 insertions(+), 92 deletions(-) diff --git a/server/datastore/mysql/vpp.go b/server/datastore/mysql/vpp.go index e2a260c91f..a40416697a 100644 --- a/server/datastore/mysql/vpp.go +++ b/server/datastore/mysql/vpp.go @@ -793,20 +793,6 @@ func (ds *Datastore) UpdateVPPTokenTeams(ctx context.Context, id uint, teams []u stmtDeleteApps := `DELETE FROM vpp_apps_teams WHERE vpp_token_id = ?` deleteArgs := []any{id} - if len(teams) > 0 { - // If we're adding a VPP token to one or more teams, delete - // any VPP apps already assigned to those teams (using the All - // teams token) - questions := make([]string, 0, len(teams)) - - for _, team := range teams { - questions = append(questions, "?") - deleteArgs = append(deleteArgs, team) - } - - stmtDeleteApps += fmt.Sprintf(" OR global_or_team_id IN (%s)", strings.Join(questions, ",")) - } - var values string var args []any // No DB constraint for null_team_type, if no team or all teams @@ -1112,6 +1098,37 @@ TEAMLOOP: func checkVPPNullTeam(ctx context.Context, tx sqlx.ExtContext, currentID *uint, nullTeam fleet.NullTeamType) error { nullTeamStmt := `SELECT vpp_token_id FROM vpp_token_teams WHERE null_team_type = ?` + anyTeamStmt := `SELECT vpp_token_id FROM vpp_token_teams WHERE null_team_type = 'allteams' OR null_team_type = 'noteam' OR team_id IS NOT NULL` + + if nullTeam == fleet.NullTeamAllTeams { + var ids []uint + if err := sqlx.SelectContext(ctx, tx, &ids, anyTeamStmt); err != nil { + return ctxerr.Wrap(ctx, err, "scanning row in check vpp token null team") + } + + if len(ids) > 0 { + if len(ids) > 1 { + return ctxerr.Wrap(ctx, errors.New("Cannot assign token to All teams, other teams have tokens")) + } + if currentID == nil || ids[0] != *currentID { + return ctxerr.Wrap(ctx, errors.New("Cannot assign token to All teams, other teams have tokens")) + } + } + } + + var id uint + allTeamsFound := true + if err := sqlx.GetContext(ctx, tx, &id, nullTeamStmt, fleet.NullTeamAllTeams); err != nil { + if errors.Is(err, sql.ErrNoRows) { + allTeamsFound = false + } else { + return ctxerr.Wrap(ctx, err, "scanning row in check vpp token null team") + } + } + + if allTeamsFound && currentID != nil && *currentID != id { + return ctxerr.Wrap(ctx, errors.New("All teams token already exists")) + } if nullTeam != fleet.NullTeamNone { var id uint diff --git a/server/datastore/mysql/vpp_test.go b/server/datastore/mysql/vpp_test.go index dffabad6f0..b9bfb8dfe3 100644 --- a/server/datastore/mysql/vpp_test.go +++ b/server/datastore/mysql/vpp_test.go @@ -740,7 +740,7 @@ func testVPPTokensCRUD(t *testing.T, ds *Datastore) { assert.Equal(t, dataToken.Token, tok.Token) assert.Equal(t, orgName, tok.OrgName) assert.Equal(t, location, tok.Location) - assert.NotNil(t, tok.Teams) // "All Teams" teamm array is non-nil but empty + assert.NotNil(t, tok.Teams) // "All Teams" teams array is non-nil but empty assert.Len(t, tok.Teams, 0) toks, err = ds.ListVPPTokens(ctx) @@ -775,7 +775,7 @@ func testVPPTokensCRUD(t *testing.T, ds *Datastore) { // Assign to team "No Team" upTok, err = ds.UpdateVPPTokenTeams(ctx, tok.ID, []uint{0}) - assert.NoError(t, err) + require.NoError(t, err) assert.Len(t, upTok.Teams, 1) assert.Equal(t, tokID, upTok.ID) assert.Equal(t, uint(0), upTok.Teams[0].ID) @@ -921,13 +921,33 @@ func testVPPTokensCRUD(t *testing.T, ds *Datastore) { assert.NoError(t, err) assert.Len(t, toks, 2) + // Remove tokAll from All teams + tokAll, err = ds.UpdateVPPTokenTeams(ctx, tokAll.ID, nil) + assert.NoError(t, err) + tokTeam, err := ds.InsertVPPToken(ctx, dataToken3) assert.NoError(t, err) + _, err = ds.UpdateVPPTokenTeams(ctx, tokTeam.ID, []uint{team.ID}) assert.NoError(t, err) _, err = ds.UpdateVPPTokenTeams(ctx, tokTeam.ID, []uint{team.ID, team.ID}) assert.Error(t, err) + // Cannot move tokAll to all teams now + _, err = ds.UpdateVPPTokenTeams(ctx, tokAll.ID, []uint{}) + assert.Error(t, err) + + _, err = ds.UpdateVPPTokenTeams(ctx, tokTeam.ID, []uint{0}) + assert.NoError(t, err) + + _, err = ds.UpdateVPPTokenTeams(ctx, tokAll.ID, []uint{}) + assert.Error(t, err) + + _, err = ds.UpdateVPPTokenTeams(ctx, tokTeam.ID, []uint{team.ID}) + assert.NoError(t, err) + + /// + toks, err = ds.ListVPPTokens(ctx) assert.NoError(t, err) assert.Len(t, toks, 3) @@ -968,7 +988,7 @@ func testVPPTokensCRUD(t *testing.T, ds *Datastore) { assert.NoError(t, err) assert.Len(t, toks, 5) - // Test fallback to all teams + /// tokNil, err := ds.GetVPPTokenByTeamID(ctx, nil) assert.NoError(t, err) assert.Equal(t, tokTeams.ID, tokNil.ID) @@ -993,8 +1013,7 @@ func testVPPTokensCRUD(t *testing.T, ds *Datastore) { assert.Equal(t, tokTeams.ID, tokNil.ID) tokTeam1, err = ds.GetVPPTokenByTeamID(ctx, &team.ID) - assert.NoError(t, err) - assert.Equal(t, tokAll.ID, tokTeam1.ID) + assert.Error(t, err) tokTeam2, err = ds.GetVPPTokenByTeamID(ctx, &team2.ID) assert.NoError(t, err) @@ -1004,6 +1023,22 @@ func testVPPTokensCRUD(t *testing.T, ds *Datastore) { err = ds.DeleteVPPToken(ctx, tokTeams.ID) assert.NoError(t, err) + tokNil, err = ds.GetVPPTokenByTeamID(ctx, nil) + assert.Error(t, err) + assert.True(t, fleet.IsNotFound(err)) + + tokTeam1, err = ds.GetVPPTokenByTeamID(ctx, &team.ID) + assert.Error(t, err) + assert.True(t, fleet.IsNotFound(err)) + + tokTeam2, err = ds.GetVPPTokenByTeamID(ctx, &team2.ID) + assert.Error(t, err) + assert.True(t, fleet.IsNotFound(err)) + + //// + tokAll, err = ds.UpdateVPPTokenTeams(ctx, tokAll.ID, []uint{}) + assert.NoError(t, err) + tokNil, err = ds.GetVPPTokenByTeamID(ctx, nil) assert.NoError(t, err) assert.Equal(t, tokAll.ID, tokNil.ID) @@ -1016,22 +1051,9 @@ func testVPPTokensCRUD(t *testing.T, ds *Datastore) { assert.NoError(t, err) assert.Equal(t, tokAll.ID, tokTeam2.ID) - //// err = ds.DeleteVPPToken(ctx, tokAll.ID) assert.NoError(t, err) - tokNil, err = ds.GetVPPTokenByTeamID(ctx, nil) - assert.Error(t, err) - assert.True(t, fleet.IsNotFound(err)) - - tokTeam1, err = ds.GetVPPTokenByTeamID(ctx, &team.ID) - assert.Error(t, err) - assert.True(t, fleet.IsNotFound(err)) - - tokTeam2, err = ds.GetVPPTokenByTeamID(ctx, &team2.ID) - assert.Error(t, err) - assert.True(t, fleet.IsNotFound(err)) - //// _, err = ds.UpdateVPPTokenTeams(ctx, tokNone.ID, []uint{0, team.ID, team2.ID}) assert.NoError(t, err) @@ -1051,6 +1073,7 @@ func testVPPTokensCRUD(t *testing.T, ds *Datastore) { //// err = ds.DeleteVPPToken(ctx, tokNone.ID) assert.NoError(t, err) + } func testVPPTokenAppTeamAssociations(t *testing.T, ds *Datastore) { @@ -1062,10 +1085,6 @@ func testVPPTokenAppTeamAssociations(t *testing.T, ds *Datastore) { team2, err := ds.NewTeam(ctx, &fleet.Team{Name: "Zingers"}) assert.NoError(t, err) - team3, err := ds.NewTeam(ctx, &fleet.Team{Name: "Kremling"}) - _ = team3 - assert.NoError(t, err) - dataToken, err := test.CreateVPPTokenData(time.Now().Add(24*time.Hour), "Donkey Kong", "Jungle") require.NoError(t, err) @@ -1078,10 +1097,10 @@ func testVPPTokenAppTeamAssociations(t *testing.T, ds *Datastore) { tok2, err := ds.InsertVPPToken(ctx, dataToken2) assert.NoError(t, err) - _, err = ds.UpdateVPPTokenTeams(ctx, tok1.ID, []uint{}) + _, err = ds.UpdateVPPTokenTeams(ctx, tok1.ID, []uint{team1.ID}) assert.NoError(t, err) - _, err = ds.UpdateVPPTokenTeams(ctx, tok2.ID, []uint{team1.ID}) + _, err = ds.UpdateVPPTokenTeams(ctx, tok2.ID, []uint{team2.ID}) assert.NoError(t, err) app1 := &fleet.VPPApp{ @@ -1094,13 +1113,10 @@ func testVPPTokenAppTeamAssociations(t *testing.T, ds *Datastore) { }, BundleIdentifier: "app1", } - vppApp1, err := ds.InsertVPPAppWithTeam(ctx, app1, &team1.ID) + _, err = ds.InsertVPPAppWithTeam(ctx, app1, &team1.ID) assert.NoError(t, err) _, err = ds.InsertVPPAppWithTeam(ctx, app1, &team2.ID) assert.NoError(t, err) - _ = vppApp1 - _, err = ds.InsertVPPAppWithTeam(ctx, app1, &team3.ID) - assert.NoError(t, err) app2 := &fleet.VPPApp{ Name: "app2", @@ -1116,9 +1132,8 @@ func testVPPTokenAppTeamAssociations(t *testing.T, ds *Datastore) { _ = vppApp2 assert.NoError(t, err) - // team1: token, app1, app2 - // team2: global token, app 1 - // team3: global token, app 1 + // team1: token 1, app1, app2 + // team2: token 2, app 1 apps, err := ds.GetAssignedVPPApps(ctx, &team1.ID) assert.NoError(t, err) @@ -1131,19 +1146,28 @@ func testVPPTokenAppTeamAssociations(t *testing.T, ds *Datastore) { assert.Len(t, apps, 1) assert.Contains(t, apps, app1.VPPAppID) - apps, err = ds.GetAssignedVPPApps(ctx, &team3.ID) + /// Try to move team 1 token to team 2 + + _, err = ds.UpdateVPPTokenTeams(ctx, tok1.ID, []uint{team2.ID}) + assert.Error(t, err) + + // team1: token 1, app1, app2 + // team2: token 2, app 1 + + apps, err = ds.GetAssignedVPPApps(ctx, &team1.ID) + assert.NoError(t, err) + assert.Len(t, apps, 2) + + apps, err = ds.GetAssignedVPPApps(ctx, &team2.ID) assert.NoError(t, err) assert.Len(t, apps, 1) assert.Contains(t, apps, app1.VPPAppID) - /// Move team 1 token to team 3 - - _, err = ds.UpdateVPPTokenTeams(ctx, tok2.ID, []uint{team3.ID}) + _, err = ds.UpdateVPPTokenTeams(ctx, tok1.ID, nil) assert.NoError(t, err) - // team1: global token, no apps - // team2: global token, app 1 - // team3: token, no apps + // team1: no token, no apps + // team2: token 2, app 1 apps, err = ds.GetAssignedVPPApps(ctx, &team1.ID) assert.NoError(t, err) @@ -1154,45 +1178,13 @@ func testVPPTokenAppTeamAssociations(t *testing.T, ds *Datastore) { assert.Len(t, apps, 1) assert.Contains(t, apps, app1.VPPAppID) - apps, err = ds.GetAssignedVPPApps(ctx, &team3.ID) - assert.NoError(t, err) - assert.Len(t, apps, 0) + // Move team 2 token to team 1 - /// Add apps with new token assignments - - _, err = ds.InsertVPPAppWithTeam(ctx, app1, &team3.ID) + _, err = ds.UpdateVPPTokenTeams(ctx, tok2.ID, []uint{team1.ID}) assert.NoError(t, err) - _, err = ds.InsertVPPAppWithTeam(ctx, app2, &team1.ID) - assert.NoError(t, err) - - // team1: global token, app 2 - // team2: global token, app 1 - // team3: token, app 1 - - apps, err = ds.GetAssignedVPPApps(ctx, &team1.ID) - assert.NoError(t, err) - assert.Len(t, apps, 1) - assert.Contains(t, apps, app2.VPPAppID) - - apps, err = ds.GetAssignedVPPApps(ctx, &team2.ID) - assert.NoError(t, err) - assert.Len(t, apps, 1) - assert.Contains(t, apps, app1.VPPAppID) - - apps, err = ds.GetAssignedVPPApps(ctx, &team3.ID) - assert.NoError(t, err) - assert.Len(t, apps, 1) - assert.Contains(t, apps, app1.VPPAppID) - - /// Move global token to team 1, no global token now - - _, err = ds.UpdateVPPTokenTeams(ctx, tok1.ID, []uint{team1.ID}) - assert.NoError(t, err) - - // team1: token, no apps + // team1: token 2, app 1 // team2: no token, no apps - // team3: token, app 1 apps, err = ds.GetAssignedVPPApps(ctx, &team1.ID) assert.NoError(t, err) @@ -1202,11 +1194,6 @@ func testVPPTokenAppTeamAssociations(t *testing.T, ds *Datastore) { assert.NoError(t, err) assert.Len(t, apps, 0) - apps, err = ds.GetAssignedVPPApps(ctx, &team3.ID) - assert.NoError(t, err) - assert.Len(t, apps, 1) - assert.Contains(t, apps, app1.VPPAppID) - /// Can't assaign apps with no token _, err = ds.InsertVPPAppWithTeam(ctx, app1, &team2.ID) diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index fd9074219a..3519a4c9af 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -10513,6 +10513,9 @@ func (s *integrationMDMTestSuite) TestVPPApps() { s.DoJSON("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/install/%d", orbitHost.ID, macOSTitleID), &installSoftwareRequest{}, http.StatusBadRequest, &installResp) + // Disable all teams token + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/vpp_tokens/%d/teams", validToken.Token.ID), patchVPPTokensTeamsRequest{}, http.StatusOK, &resPatchVPP) + // Spoof an expired VPP token and attempt to install VPP app tokenJSONBad := fmt.Sprintf(`{"expDate":"%s","token":"%s","orgName":"%s"}`, "2099-06-24T15:50:50+0000", "badtoken", "Evil Fleet") s.appleVPPConfigSrvConfig.Location = "Spooky Haunted House" @@ -10534,6 +10537,9 @@ func (s *integrationMDMTestSuite) TestVPPApps() { // Disable the token s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/vpp_tokens/%d/teams", vppRes.Token.ID), patchVPPTokensTeamsRequest{}, http.StatusOK, &resPatchVPP) + // Enable all teams token + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/vpp_tokens/%d/teams", validToken.Token.ID), patchVPPTokensTeamsRequest{TeamIDs: []uint{}}, http.StatusOK, &resPatchVPP) + // Attempt to install non-existent app r := s.Do("POST", fmt.Sprintf("/api/latest/fleet/hosts/%d/software/install/%d", mdmHost.ID, 99999), &installSoftwareRequest{}, http.StatusBadRequest) require.Contains(t, extractServerErrorText(r.Body), "Couldn't install software. Software title is not available for install. Please add software package or App Store app to install.")