Skip to content

Commit

Permalink
Self service vpp fixes (#21624)
Browse files Browse the repository at this point in the history
for

- #21497
- #21498

# 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] Input data is properly validated, `SELECT *` is avoided, SQL
injection is prevented (using placeholders for values in statements)
- [x] Added/updated tests
- [x] Manual QA for all new/changed functionality
  • Loading branch information
roperzh authored Aug 28, 2024
1 parent d2ed768 commit 685bf58
Show file tree
Hide file tree
Showing 9 changed files with 150 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1203,4 +1203,34 @@ describe("Activity Feed", () => {
render(<ActivityItem activity={activity} isPremiumTier />);
expect(screen.getByText("An end user")).toBeInTheDocument();
});

it("renders the correct actor for a installed_app_store_app activity without self_service", () => {
const activity = createMockActivity({
type: ActivityType.InstalledAppStoreApp,
actor_id: 1,
actor_full_name: "Test Admin",
details: {
software_title: "Foo Software",
host_display_name: "Foo Host",
},
});

render(<ActivityItem activity={activity} isPremiumTier />);
expect(screen.getByText("Test Admin")).toBeInTheDocument();
});

it("renders the correct actor for a installed_app_store_app activity that was self_service", () => {
const activity = createMockActivity({
type: ActivityType.InstalledAppStoreApp,
actor_id: 1,
details: {
software_title: "Foo Software",
self_service: true,
host_display_name: "Foo Host",
},
});

render(<ActivityItem activity={activity} isPremiumTier />);
expect(screen.getByText("An end user")).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,7 @@ const ActivityItem = ({
DEFAULT_ACTOR_DISPLAY
);
case ActivityType.InstalledSoftware:
case ActivityType.InstalledAppStoreApp:
return activity.details?.self_service ? (
<span>An end user</span>
) : (
Expand Down
1 change: 1 addition & 0 deletions server/datastore/mysql/activities.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ SELECT
'software_title', st.name,
'app_store_id', hvsi.adam_id,
'command_uuid', hvsi.command_uuid,
'self_service', hvsi.self_service IS TRUE,
-- status is always pending because only pending MDM commands are upcoming.
'status', :software_status_pending
) AS details
Expand Down
85 changes: 63 additions & 22 deletions server/datastore/mysql/activities_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,11 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {

// create three hosts
h1 := test.NewHost(t, ds, "h1.local", "10.10.10.1", "1", "1", time.Now())
nanoEnrollAndSetHostMDMData(t, ds, h1, false)
h2 := test.NewHost(t, ds, "h2.local", "10.10.10.2", "2", "2", time.Now())
nanoEnrollAndSetHostMDMData(t, ds, h2, false)
h3 := test.NewHost(t, ds, "h3.local", "10.10.10.3", "3", "3", time.Now())
nanoEnrollAndSetHostMDMData(t, ds, h3, false)

// create a couple of named scripts
scr1, err := ds.NewScript(ctx, &fleet.Script{
Expand Down Expand Up @@ -415,6 +418,35 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {
sw2Meta, err := ds.GetSoftwareInstallerMetadataByID(ctx, sw2)
require.NoError(t, err)

// insert a VPP app
vppCommand1, vppCommand2 := "vpp-command-1", "vpp-command-2"
vppApp := &fleet.VPPApp{
Name: "vpp_no_team_app_1", VPPAppTeam: fleet.VPPAppTeam{VPPAppID: fleet.VPPAppID{AdamID: "3", Platform: fleet.MacOSPlatform}},
BundleIdentifier: "b3",
}
_, err = ds.InsertVPPAppWithTeam(ctx, vppApp, nil)
require.NoError(t, err)

// install the VPP app on h1
commander, _ := createMDMAppleCommanderAndStorage(t, ds)
err = ds.InsertHostVPPSoftwareInstall(ctx, h1.ID, vppApp.VPPAppID, vppCommand1, "event-id-1", false)
require.NoError(t, err)
err = commander.EnqueueCommand(
ctx,
[]string{h1.UUID},
createRawAppleCmd("InstallApplication", vppCommand1),
)
require.NoError(t, err)
// install the VPP app on h2, self-service
err = ds.InsertHostVPPSoftwareInstall(noUserCtx, h2.ID, vppApp.VPPAppID, vppCommand2, "event-id-2", true)
require.NoError(t, err)
err = commander.EnqueueCommand(
ctx,
[]string{h1.UUID},
createRawAppleCmd("InstallApplication", vppCommand2),
)
require.NoError(t, err)

// create a sync script request for h1 that has been pending for > MaxWaitTime, will not show up
hsr, err := ds.NewHostScriptExecutionRequest(ctx, &fleet.HostScriptRequestPayload{HostID: h1.ID, ScriptContents: "sync", UserID: &u.ID, SyncRequest: true})
require.NoError(t, err)
Expand Down Expand Up @@ -484,19 +516,22 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {
endTime = SetOrderedCreatedAtTimestamps(t, ds, endTime, "host_script_results", "execution_id", h1C, h1D, h1E)
endTime = SetOrderedCreatedAtTimestamps(t, ds, endTime, "host_software_installs", "execution_id", h1FooInstalled, h1Foo)
endTime = SetOrderedCreatedAtTimestamps(t, ds, endTime, "host_software_installs", "execution_id", h2Bar)
SetOrderedCreatedAtTimestamps(t, ds, endTime, "host_script_results", "execution_id", h2A, h2F)
endTime = SetOrderedCreatedAtTimestamps(t, ds, endTime, "host_script_results", "execution_id", h2A, h2F)
SetOrderedCreatedAtTimestamps(t, ds, endTime, "host_vpp_software_installs", "command_uuid", vppCommand1, vppCommand2)

execIDsWithUser := map[string]bool{
h1A: true,
h1B: true,
h1C: true,
h1D: false,
h1E: false,
h2A: true,
h2F: true,
h1Foo: false,
h1Bar: true,
h2Bar: true,
h1A: true,
h1B: true,
h1C: true,
h1D: false,
h1E: false,
h2A: true,
h2F: true,
h1Foo: false,
h1Bar: true,
h2Bar: true,
vppCommand1: true,
vppCommand2: false,
}
execIDsScriptName := map[string]string{
h1A: scr1.Name,
Expand All @@ -519,49 +554,49 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {
opts: fleet.ListOptions{PerPage: 2},
hostID: h1.ID,
wantExecs: []string{h1A, h1B},
wantMeta: &fleet.PaginationMetadata{HasNextResults: true, HasPreviousResults: false, TotalResults: 7},
wantMeta: &fleet.PaginationMetadata{HasNextResults: true, HasPreviousResults: false, TotalResults: 8},
},
{
opts: fleet.ListOptions{Page: 1, PerPage: 2},
hostID: h1.ID,
wantExecs: []string{h1Bar, h1C},
wantMeta: &fleet.PaginationMetadata{HasNextResults: true, HasPreviousResults: true, TotalResults: 7},
wantMeta: &fleet.PaginationMetadata{HasNextResults: true, HasPreviousResults: true, TotalResults: 8},
},
{
opts: fleet.ListOptions{Page: 2, PerPage: 2},
hostID: h1.ID,
wantExecs: []string{h1D, h1E},
wantMeta: &fleet.PaginationMetadata{HasNextResults: true, HasPreviousResults: true, TotalResults: 7},
wantMeta: &fleet.PaginationMetadata{HasNextResults: true, HasPreviousResults: true, TotalResults: 8},
},
{
opts: fleet.ListOptions{Page: 3, PerPage: 2},
hostID: h1.ID,
wantExecs: []string{h1Foo},
wantMeta: &fleet.PaginationMetadata{HasNextResults: false, HasPreviousResults: true, TotalResults: 7},
wantExecs: []string{h1Foo, vppCommand1},
wantMeta: &fleet.PaginationMetadata{HasNextResults: false, HasPreviousResults: true, TotalResults: 8},
},
{
opts: fleet.ListOptions{PerPage: 4},
hostID: h1.ID,
wantExecs: []string{h1A, h1B, h1Bar, h1C},
wantMeta: &fleet.PaginationMetadata{HasNextResults: true, HasPreviousResults: false, TotalResults: 7},
wantMeta: &fleet.PaginationMetadata{HasNextResults: true, HasPreviousResults: false, TotalResults: 8},
},
{
opts: fleet.ListOptions{Page: 1, PerPage: 4},
hostID: h1.ID,
wantExecs: []string{h1D, h1E, h1Foo},
wantMeta: &fleet.PaginationMetadata{HasNextResults: false, HasPreviousResults: true, TotalResults: 7},
wantExecs: []string{h1D, h1E, h1Foo, vppCommand1},
wantMeta: &fleet.PaginationMetadata{HasNextResults: false, HasPreviousResults: true, TotalResults: 8},
},
{
opts: fleet.ListOptions{Page: 2, PerPage: 4},
hostID: h1.ID,
wantExecs: []string{},
wantMeta: &fleet.PaginationMetadata{HasNextResults: false, HasPreviousResults: true, TotalResults: 7},
wantMeta: &fleet.PaginationMetadata{HasNextResults: false, HasPreviousResults: true, TotalResults: 8},
},
{
opts: fleet.ListOptions{PerPage: 3},
hostID: h2.ID,
wantExecs: []string{h2Bar, h2A},
wantMeta: &fleet.PaginationMetadata{HasNextResults: false, HasPreviousResults: false, TotalResults: 2},
wantExecs: []string{h2Bar, h2A, vppCommand2},
wantMeta: &fleet.PaginationMetadata{HasNextResults: false, HasPreviousResults: false, TotalResults: 3},
},
{
opts: fleet.ListOptions{},
Expand Down Expand Up @@ -604,6 +639,12 @@ func testListHostUpcomingActivities(t *testing.T, ds *Datastore) {
require.Equal(t, execIDsSoftwareTitle[wantExec], details["software_title"], "result %d", i)
wantUser = u2

case fleet.ActivityInstalledAppStoreApp{}.ActivityName():
require.Equal(t, wantExec, details["command_uuid"], "result %d", i)
require.Equal(t, "vpp_no_team_app_1", details["software_title"], "result %d", i)
require.Equal(t, !execIDsWithUser[wantExec], details["self_service"], "result %d", i)
wantUser = u2

default:
t.Fatalf("unknown activity type %s", a.Type)
}
Expand Down
2 changes: 1 addition & 1 deletion server/datastore/mysql/testing_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ func GetAggregatedStats(ctx context.Context, ds *Datastore, aggregate fleet.Aggr
func SetOrderedCreatedAtTimestamps(t testing.TB, ds *Datastore, afterTime time.Time, table, keyCol string, keys ...any) time.Time {
now := afterTime
for i := 0; i < len(keys); i++ {
now = afterTime.Add(time.Second)
now = now.Add(time.Second)
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
_, err := q.ExecContext(context.Background(),
fmt.Sprintf(`UPDATE %s SET created_at=? WHERE %s=?`, table, keyCol), now, keys[i])
Expand Down
31 changes: 17 additions & 14 deletions server/datastore/mysql/vpp.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ SELECT
st.name AS software_title,
hvsi.adam_id AS app_store_id,
hvsi.command_uuid AS command_uuid,
hvsi.self_service AS self_service
hvsi.self_service AS self_service
FROM
host_vpp_software_installs hvsi
LEFT OUTER JOIN users u ON hvsi.user_id = u.id
Expand All @@ -490,15 +490,15 @@ WHERE
`

type result struct {
HostID uint `db:"host_id"`
HostDisplayName string `db:"host_display_name"`
SoftwareTitle string `db:"software_title"`
AppStoreID string `db:"app_store_id"`
CommandUUID string `db:"command_uuid"`
UserName string `db:"user_name"`
UserID uint `db:"user_id"`
UserEmail string `db:"user_email"`
SelfService bool `db:"self_service"`
HostID uint `db:"host_id"`
HostDisplayName string `db:"host_display_name"`
SoftwareTitle string `db:"software_title"`
AppStoreID string `db:"app_store_id"`
CommandUUID string `db:"command_uuid"`
UserName *string `db:"user_name"`
UserID *uint `db:"user_id"`
UserEmail *string `db:"user_email"`
SelfService bool `db:"self_service"`
}

listStmt, args, err := sqlx.Named(stmt, map[string]any{
Expand All @@ -519,10 +519,13 @@ WHERE
return nil, nil, ctxerr.Wrap(ctx, err, "select past activity data for VPP app install")
}

user := &fleet.User{
ID: res.UserID,
Name: res.UserName,
Email: res.UserEmail,
var user *fleet.User
if res.UserID != nil {
user = &fleet.User{
ID: *res.UserID,
Name: *res.UserName,
Email: *res.UserEmail,
}
}

var status string
Expand Down
14 changes: 14 additions & 0 deletions server/datastore/mysql/vpp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ func testVPPAppStatus(t *testing.T, ds *Datastore) {
require.Equal(t, user.ID, actUser.ID)
require.Equal(t, user.Name, actUser.Name)
require.Equal(t, cmd3, act.CommandUUID)
require.False(t, act.SelfService)

summary, err = ds.GetSummaryHostVPPAppInstalls(ctx, nil, vpp1)
require.NoError(t, err)
Expand Down Expand Up @@ -350,6 +351,19 @@ func testVPPAppStatus(t *testing.T, ds *Datastore) {
summary, err = ds.GetSummaryHostVPPAppInstalls(ctx, &team1.ID, vpp3)
require.NoError(t, err)
require.Equal(t, &fleet.VPPAppStatusSummary{Pending: 0, Failed: 0, Installed: 1}, summary)

// simulate a self-service request
ExecAdhocSQL(t, ds, func(q sqlx.ExtContext) error {
_, err := q.ExecContext(ctx,
`UPDATE host_vpp_software_installs SET self_service = true, user_id = NULL WHERE command_uuid = ?`,
cmd3)
return err
})
actUser, act, err = ds.GetPastActivityDataForVPPAppInstall(ctx, &mdm.CommandResults{CommandUUID: cmd3})
require.NoError(t, err)
require.Nil(t, actUser)
require.Equal(t, cmd3, act.CommandUUID)
require.True(t, act.SelfService)
}

// simulates creating the VPP app install request on the host, returns the command UUID.
Expand Down
2 changes: 1 addition & 1 deletion server/service/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ func (c *Client) ApplyGroup(
for tmName, apps := range tmSoftwareApps {
appPayloads := make([]fleet.VPPBatchPayload, 0, len(apps))
for _, app := range apps {
appPayloads = append(appPayloads, fleet.VPPBatchPayload{AppStoreID: app.AppStoreID})
appPayloads = append(appPayloads, fleet.VPPBatchPayload{AppStoreID: app.AppStoreID, SelfService: app.SelfService})
}
tmSoftwareAppsPayloads[tmName] = appPayloads
}
Expand Down
33 changes: 22 additions & 11 deletions server/service/integration_mdm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10628,6 +10628,13 @@ func (s *integrationMDMTestSuite) TestVPPApps() {
strconv.Itoa(int(team.ID)), "software_title_id", strconv.Itoa(int(titleID)))
require.Equal(t, 1, countResp.Count)

// send an idle request to grab the command uuid
cmd, err = mdmClient.Idle()
require.NoError(t, err)
var fullCmd micromdm.CommandPayload
require.NoError(t, plist.Unmarshal(cmd.Raw, &fullCmd))
cmdUUID = cmd.CommandUUID

// Get pending activity
var hostActivitiesResp listHostUpcomingActivitiesResponse
s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/activities/upcoming", installHost.ID),
Expand All @@ -10642,20 +10649,24 @@ func (s *integrationMDMTestSuite) TestVPPApps() {
require.Len(t, hostActivitiesResp.Activities, 1, "got activities: %v", activitiesToString(hostActivitiesResp.Activities))
assert.Equal(t, hostActivitiesResp.Activities[0].Type, fleet.ActivityInstalledAppStoreApp{}.ActivityName())
assert.EqualValues(t, 1, hostActivitiesResp.Count)
assert.JSONEq(
t,
fmt.Sprintf(
`{"host_id": %d, "host_display_name": "%s", "software_title": "%s", "app_store_id": "%s", "command_uuid": "%s", "status": "%s", "self_service": %v}`,
installHost.ID,
installHost.DisplayName(),
app.Name,
app.AdamID,
cmdUUID,
fleet.SoftwareInstallerPending,
install.deviceToken != "",
),
string(*hostActivitiesResp.Activities[0].Details),
)

// Simulate successful installation on the host
cmd, err = mdmClient.Idle()
cmd, err = mdmClient.Acknowledge(cmd.CommandUUID)
require.NoError(t, err)
for cmd != nil {
var fullCmd micromdm.CommandPayload
switch cmd.Command.RequestType {
case "InstallApplication":
require.NoError(t, plist.Unmarshal(cmd.Raw, &fullCmd))
cmdUUID = cmd.CommandUUID
cmd, err = mdmClient.Acknowledge(cmd.CommandUUID)
require.NoError(t, err)
}
}

listResp = listHostsResponse{}
s.DoJSON("GET", "/api/latest/fleet/hosts", nil, http.StatusOK, &listResp, "software_status", "installed", "team_id",
Expand Down

0 comments on commit 685bf58

Please sign in to comment.