diff --git a/ee/server/service/vpp.go b/ee/server/service/vpp.go index e25f48c030..41e6255093 100644 --- a/ee/server/service/vpp.go +++ b/ee/server/service/vpp.go @@ -781,6 +781,20 @@ func (svc *Service) UpdateAppStoreApp(ctx context.Context, titleID uint, teamID return nil, nil, err } + // If there's an auto-update config, validate it. + // Note that applying this config is done in a separate service method. + schedule := fleet.SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: fleet.SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: payload.AutoUpdateEnabled, + AutoUpdateStartTime: payload.AutoUpdateStartTime, + AutoUpdateEndTime: payload.AutoUpdateEndTime, + }, + } + + if err := schedule.WindowIsValid(); err != nil { + return nil, nil, ctxerr.Wrap(ctx, err, "UpdateAppStoreApp: validating auto-update schedule") + } + var teamName string if teamID != nil && *teamID != 0 { tm, err := svc.ds.TeamLite(ctx, *teamID) diff --git a/server/datastore/mysql/software_titles.go b/server/datastore/mysql/software_titles.go index 2d2b3e160a..b82c5a19f3 100644 --- a/server/datastore/mysql/software_titles.go +++ b/server/datastore/mysql/software_titles.go @@ -832,29 +832,22 @@ WHERE } func (ds *Datastore) UpdateSoftwareTitleAutoUpdateConfig(ctx context.Context, titleID uint, teamID uint, config fleet.SoftwareAutoUpdateConfig) error { - // Validate start and end time. - invalidTimeErr := "invalid auto-update time format: must be in HH:MM 24-hour format" - for _, t := range []*string{config.AutoUpdateStartTime, config.AutoUpdateEndTime} { - if t == nil { - if config.AutoUpdateEnabled != nil && *config.AutoUpdateEnabled { - return fleet.NewInvalidArgumentError("auto_update_time", invalidTimeErr) - } - continue - } - duration, err := time.Parse("15:04", *t) - if err != nil { - return fleet.NewInvalidArgumentError("auto_update_time", invalidTimeErr) - } - if duration.Hour() < 0 || duration.Hour() > 23 || duration.Minute() < 0 || duration.Minute() > 59 { - return fleet.NewInvalidArgumentError("auto_update_time", invalidTimeErr) - } + // Validate schedule if enabled. + schedule := fleet.SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: fleet.SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: config.AutoUpdateEnabled, + AutoUpdateStartTime: config.AutoUpdateStartTime, + AutoUpdateEndTime: config.AutoUpdateEndTime, + }, + } + if err := schedule.WindowIsValid(); err != nil { + return ctxerr.Wrap(ctx, err, "validating auto-update schedule") } - var startTime, endTime string - if config.AutoUpdateStartTime != nil { + if config.AutoUpdateEnabled != nil && *config.AutoUpdateEnabled && config.AutoUpdateStartTime != nil { startTime = *config.AutoUpdateStartTime } - if config.AutoUpdateEndTime != nil { + if config.AutoUpdateEnabled != nil && *config.AutoUpdateEnabled && config.AutoUpdateEndTime != nil { endTime = *config.AutoUpdateEndTime } diff --git a/server/datastore/mysql/software_titles_test.go b/server/datastore/mysql/software_titles_test.go index cdde9335b8..563c6452c5 100644 --- a/server/datastore/mysql/software_titles_test.go +++ b/server/datastore/mysql/software_titles_test.go @@ -2655,7 +2655,7 @@ func testUpdateAutoUpdateConfig(t *testing.T, ds *Datastore) { AutoUpdateEndTime: ptr.String(endTime), }) require.Error(t, err) - require.Contains(t, err.Error(), "invalid auto-update time format") + require.Contains(t, err.Error(), "Error parsing start time") // Attempt to enable auto-update with invalid end time. startTime = "12:00" @@ -2666,7 +2666,18 @@ func testUpdateAutoUpdateConfig(t *testing.T, ds *Datastore) { AutoUpdateEndTime: ptr.String(endTime), }) require.Error(t, err) - require.Contains(t, err.Error(), "invalid auto-update time format") + require.Contains(t, err.Error(), "Error parsing end time") + + // Attempt to enable auto-update with less than an hour between start and end time. + startTime = "12:00" + endTime = "12:30" + err = ds.UpdateSoftwareTitleAutoUpdateConfig(ctx, titleID, *teamID, fleet.SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: ptr.String(startTime), + AutoUpdateEndTime: ptr.String(endTime), + }) + require.Error(t, err) + require.Contains(t, err.Error(), "The update window must be at least one hour long") // Enable auto-update. startTime = "02:00" @@ -2687,6 +2698,7 @@ func testUpdateAutoUpdateConfig(t *testing.T, ds *Datastore) { require.Equal(t, endTime, *titleResult.AutoUpdateEndTime) // Add valid, disabled auto-update schedule for the other VPP app. + // The schedule should be ignored since it's disabled, but it should still be created. err = ds.UpdateSoftwareTitleAutoUpdateConfig(ctx, title2ID, *teamID, fleet.SoftwareAutoUpdateConfig{ AutoUpdateEnabled: ptr.Bool(false), AutoUpdateStartTime: ptr.String(startTime), @@ -2706,8 +2718,8 @@ func testUpdateAutoUpdateConfig(t *testing.T, ds *Datastore) { require.Equal(t, title2ID, schedules[1].TitleID) require.Equal(t, team1.ID, schedules[1].TeamID) require.False(t, *schedules[1].AutoUpdateEnabled) - require.Equal(t, startTime, *schedules[1].AutoUpdateStartTime) - require.Equal(t, endTime, *schedules[1].AutoUpdateEndTime) + require.Equal(t, "", *schedules[1].AutoUpdateStartTime) + require.Equal(t, "", *schedules[1].AutoUpdateEndTime) // Filter by enabled only. schedules, err = ds.ListSoftwareAutoUpdateSchedules(ctx, *teamID, "ipados_apps", fleet.SoftwareAutoUpdateScheduleFilter{ @@ -2727,9 +2739,7 @@ func testUpdateAutoUpdateConfig(t *testing.T, ds *Datastore) { // Disable auto-update. err = ds.UpdateSoftwareTitleAutoUpdateConfig(ctx, titleID, *teamID, fleet.SoftwareAutoUpdateConfig{ - AutoUpdateEnabled: ptr.Bool(false), - AutoUpdateStartTime: nil, - AutoUpdateEndTime: nil, + AutoUpdateEnabled: ptr.Bool(false), }) require.NoError(t, err) diff --git a/server/fleet/software.go b/server/fleet/software.go index 8654a65fd1..a54f7d71d8 100644 --- a/server/fleet/software.go +++ b/server/fleet/software.go @@ -270,6 +270,36 @@ type SoftwareAutoUpdateSchedule struct { SoftwareAutoUpdateConfig } +func (s SoftwareAutoUpdateSchedule) WindowIsValid() error { + if s.AutoUpdateEnabled == nil || !*s.AutoUpdateEnabled { + return nil + } + if s.AutoUpdateStartTime == nil || s.AutoUpdateEndTime == nil || *s.AutoUpdateStartTime == "" || *s.AutoUpdateEndTime == "" { + return errors.New("Start and end time must both be set") + } + // Validate that the times are in HH:MM format. + // Note that durations can be arbitrarily long, but parsing in this way + // automatically validates that the hours are between 0 and 23 and the minutes are between 0 and 59. + startDuration, err := time.Parse("15:04", *s.AutoUpdateStartTime) + if err != nil { + return fmt.Errorf("Error parsing start time: %w", err) + } + endDuration, err := time.Parse("15:04", *s.AutoUpdateEndTime) + if err != nil { + return fmt.Errorf("Error parsing end time: %w", err) + } + // Validate that the window is at least one hour long. + // If the end time is less than the start time, the window wraps to the next day, so we need to add 24 hours to the end time in that case. + if endDuration.Before(startDuration) { + endDuration = endDuration.Add(24 * time.Hour) + } + if endDuration.Sub(startDuration) < time.Hour { + return errors.New("The update window must be at least one hour long") + } + + return nil +} + type SoftwareAutoUpdateScheduleFilter struct { Enabled *bool } diff --git a/server/fleet/software_test.go b/server/fleet/software_test.go index 8e20a69c1f..d7c46617f9 100644 --- a/server/fleet/software_test.go +++ b/server/fleet/software_test.go @@ -262,3 +262,165 @@ func TestHostSoftwareEntryMarshalJSON(t *testing.T) { assert.JSONEq(t, expectedJSON, string(data)) } + +func TestAutoUpdateScheduleValidation(t *testing.T) { + testCases := []struct { + name string + schedule SoftwareAutoUpdateSchedule + isValid bool + }{ + { + name: "schedule disabled", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(false), + AutoUpdateStartTime: nil, + AutoUpdateEndTime: nil, + }, + }, + isValid: true, + }, + { + name: "missing start time", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: nil, + AutoUpdateEndTime: ptr.String("15:30"), + }, + }, + isValid: false, + }, + { + name: "missing end time", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: ptr.String("14:30"), + AutoUpdateEndTime: nil, + }, + }, + isValid: false, + }, + { + name: "empty start time", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: ptr.String(""), + AutoUpdateEndTime: ptr.String("15:30"), + }, + }, + isValid: false, + }, + { + name: "empty end time", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: ptr.String("14:30"), + AutoUpdateEndTime: ptr.String(""), + }, + }, + isValid: false, + }, + { + name: "valid schedule", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: ptr.String("14:30"), + AutoUpdateEndTime: ptr.String("15:30"), + }, + }, + isValid: true, + }, + { + name: "valid schedule (wrapped around midnight)", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: ptr.String("23:30"), + AutoUpdateEndTime: ptr.String("00:30"), + }, + }, + isValid: true, + }, + { + name: "start time invalid", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: ptr.String("invalid"), + AutoUpdateEndTime: ptr.String("15:30"), + }, + }, + isValid: false, + }, + { + name: "end time invalid", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: ptr.String("14:30"), + AutoUpdateEndTime: ptr.String("invalid"), + }, + }, + isValid: false, + }, + { + name: "start time hour out of range", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: ptr.String("24:00"), + AutoUpdateEndTime: ptr.String("15:30"), + }, + }, + isValid: false, + }, + { + name: "end time hour out of range", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: ptr.String("14:30"), + AutoUpdateEndTime: ptr.String("24:00"), + }, + }, + isValid: false, + }, + { + name: "window is less than one hour", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: ptr.String("14:30"), + AutoUpdateEndTime: ptr.String("15:29"), + }, + }, + isValid: false, + }, + { + name: "window is less than one hour (wrapped around midnight)", + schedule: SoftwareAutoUpdateSchedule{ + SoftwareAutoUpdateConfig: SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: ptr.Bool(true), + AutoUpdateStartTime: ptr.String("23:30"), + AutoUpdateEndTime: ptr.String("00:29"), + }, + }, + isValid: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.schedule.WindowIsValid() + if tc.isValid { + assert.NoError(t, err) + } else { + assert.Error(t, err) + } + }) + } +} diff --git a/server/service/vpp.go b/server/service/vpp.go index 0a5463d1c2..bc38449a16 100644 --- a/server/service/vpp.go +++ b/server/service/vpp.go @@ -136,6 +136,11 @@ func updateAppStoreAppEndpoint(ctx context.Context, request interface{}, svc fle Categories: req.Categories, Configuration: req.Configuration, DisplayName: req.DisplayName, + SoftwareAutoUpdateConfig: fleet.SoftwareAutoUpdateConfig{ + AutoUpdateEnabled: req.AutoUpdateEnabled, + AutoUpdateStartTime: req.AutoUpdateStartTime, + AutoUpdateEndTime: req.AutoUpdateEndTime, + }, }) if err != nil { return updateAppStoreAppResponse{Err: err}, nil