-
Notifications
You must be signed in to change notification settings - Fork 728
Install setup-experience VPP apps on manually-enrolled iOS/iPadOS devices #35906
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| if m.AwaitingConfiguration { | ||
| // Always run setup experience on non-macOS hosts(i.e. iOS/iPadOS), only run it on macOS if | ||
| // this is not an ABM MDM migration | ||
| if info.Platform != "darwin" || !info.MigrationInProgress { | ||
| // Enqueue setup experience items and mark the host as being in setup experience | ||
| hasSetupExpItems, err = svc.ds.EnqueueSetupExperienceItems(r.Context, info.Platform, r.ID, info.TeamID) | ||
| if err != nil { | ||
| return ctxerr.Wrap(r.Context, err, "queueing setup experience tasks") | ||
| } | ||
| } else { | ||
| if info.MigrationInProgress { | ||
| svc.logger.Log("info", "skipping setup experience enqueueing because DEP migration is in progress", "host_uuid", r.ID) | ||
| } else { | ||
| enqueueSetupExperienceItems = true | ||
| } | ||
| } else if info.Platform != "darwin" && r.Type == mdm.Device && !info.InstalledFromDEP { | ||
| // For manual iOS/iPadOS device enrollments, check the `TokenUpdateTally` so that | ||
| // we only run the setup experience enqueueing once per device. | ||
| nanoEnroll, err := svc.ds.GetNanoMDMEnrollment(r.Context, r.ID) | ||
| if err != nil { | ||
| return ctxerr.Wrap(r.Context, err, "getting nanomdm enrollment") | ||
| } | ||
| if nanoEnroll != nil && nanoEnroll.TokenUpdateTally == 1 { | ||
| enqueueSetupExperienceItems = true | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, the logic here was "only enqueue setup experience items if the device reports the AwaitingConfiguration flag, AND it is an ios/ipad device OR it is any device that's not in the middle of a migration. After confirming that only macOS devices can be in the MigrationInProgress state, I expanded and simplified this logic to:
- If a device reports
AwaitingConfigurationand notMigrationInProgress, enqueue setup experience items. - Otherwise if it's a non-DEP-enrolled iOS/iPad device AND this is the first time it's checked in (
TokenUpdateTally == 1), enqueue setup experience items
This ensures that enqueuing the items (which involves wiping out any existing items in the queue for the device) only happens once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably also want to check the enrollment type here. I think we only want to do this if it is a Device enrollment - a User enrollment(Device)(it's a thing in nano) aka a personal or byod device both doesn't currently support VPP when enrolled in fleet and is probably not part of the spec for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also you can check the type by checking r.Type before you fetch the enrollment if that makes things cleaner. It should be set for all requests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably also want to check the enrollment type here.
This is a separate check from the r.Type == mdm.Device that I'm doing? What other property should I look at?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I somehow totally missed that. My bad - your check is totally fine
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #35906 +/- ##
==========================================
- Coverage 66.25% 66.24% -0.02%
==========================================
Files 2112 2108 -4
Lines 179510 179260 -250
Branches 7546 7420 -126
==========================================
- Hits 118937 118748 -189
+ Misses 49675 49612 -63
- Partials 10898 10900 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Related issue: Resolves #34042
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Tested on iPad and iOS.
Full disclosure, VPP installs on my devices seemed to sometimes (not not always) fail silently the first time I tried them, with no
errorshowing in the setup experience results. This could be due to the vagaries of user-based vpp licensing vs. device-based, which is perhaps not a real-world situation, or something else I'm not following with how VPP license assignments work. I'll continue trying to reproduce it but it's difficult since it only seems to happen once per app at most, and I can't remove the user licenses from a device without wiping it (I don't have any physical devices I can do this on).