Update default Apple automatic enrollment profile - #40832
Conversation
- We should never skip the Accessibility screen on macOS. Some end users cannot use a computer without these features. - Added some keys that were released after this list was last updated.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #40832 +/- ##
==========================================
+ Coverage 66.30% 66.38% +0.07%
==========================================
Files 2470 2473 +3
Lines 197754 198423 +669
Branches 8668 8668
==========================================
+ Hits 131129 131714 +585
- Misses 54763 54789 +26
- Partials 11862 11920 +58
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:
|
|
@spalmesano0 I happened to notice this as I was working on pulling in the automatic enrollment profile we're using in dogfood.fleetdm.com into a template to be used in the @lukeheath @allenhouchins @noahtalerman fyi Note: My comment is not a blocker to this PR, just getting us on the same page for what's coming in a few weeks. |
|
@mikermcneil We should discuss. I didn't realize we were setting defaults for customers in the code today. I am on the side of we should not have a template of this in our generator or be setting defaults for our customers in our product code. This is a more advanced setting that I'm willing to bet not many customers use. There are also keys that we use internally that we should not be setting on behalf of customers — like skipping TOS, Restore, setting the default language, region (this isn't going to fly in EMEIA), etc. By changing the default behavior, we are potentially creating more work for admins to figure out why something is working differently in Fleet than what they are used to. Also, in most environments I need to revisit the one we use internally. It was set before I joined and there are keys like |
@mikermcneil If we're setting a default enrollment profile when none is provided, do we need to generate one with
@allenhouchins Sounds good, except that we should still skip
I think this one should be set to |
|
Thanks @spalmesano0! Up to @melpike to bring a story through drafting as #g-mdm Product Designer: https://fleetdm.com/handbook/engineering#review-a-community-pull-request |
Co-authored-by: Mike McNeil <mikermcneil@users.noreply.github.com>
|
Some general questions about the PR @spalmesano0
|
The initial version of this PR was much smaller, so that wasn't as big of an issue at the start! Thanks for bringing that up at this point.
Maybe, but that would add (in my opinion) unneeded complexity. Personally, I prefer the points below more.
Completely agree here. Weeks before this change is made (if we're still going to include all of the commits), we should communicate with users, and tell them how to create their own enrollment profiles as this only affects users who aren't deploying a custom enrollment profile. @JordanMontgomery Is setting |
Yes I think that's the only real risk and we should change it back. I think the other things at best would be an annoyance to admins that get caught off guard(since the features can be configured later) but changing this will potentially leave them with hosts they have to wipe in certain cases. I think admins who know what they're doing and know the risks probably do usually make the MDM non-removable but since that has always been an explicit opt-in this feels like a breaking change |
This was a design choice by Fleet though (right?), not the default behavior admins expect for devices owned by the company. Admins expect this profile to not be removable. It introduces security and compliance risks. It seems like we've had far more people asking why this profile is removable than we have customers who expect it to be removable. |
This is no longer needed: as of macOS 10.14.4, iOS 13, and iPadOS 13.1, devices are automatically supervised. https://support.apple.com/guide/deployment/about-device-supervision-dep1d89f0bff/web
Changed it back and removed
Since this seems like a breaking change, my vote is to defer changing this default behavior. If admins want to prevent it from being removable, they still can deploy their own enrollment profile. |
|
Just saw this pop up and making sure no one is waiting on me. Up to the DRI |
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
Merging this as is, from the story perspective. I'm unsure if we want our code default and the fleetctl new enrollment profile to match? |
There was a problem hiding this comment.
Pull request overview
Updates Fleet’s default Apple Automated Device Enrollment (DEP) profile to better support global customers and ensure macOS Accessibility setup is not skipped during Setup Assistant.
Changes:
- Stop skipping the macOS Accessibility setup screen in the default DEP profile.
- Remove the hard-coded
Region: "US"from the default DEP profile. - Add newer Setup Assistant skip keys to the default
SkipSetupItemslist.
Comments suppressed due to low confidence (1)
server/mdm/apple/apple_mdm.go:112
- Changing the default DEP profile (SkipSetupItems/Region/etc.) will break existing tests that hardcode the old default JSON/profile, e.g.
TestDEPService/EnsureDefaultSetupAssistant(server/mdm/apple/apple_mdm_test.go) and the integration testTestMacosSetupAssistant(server/service/integration_mdm_test.go) which includesregion: "US"and skips"Accessibility". Please update those expected fixtures/assertions (or relax them to only check the fields under test) so CI remains green.
SkipSetupItems: []string{
"Appearance",
"AppleID",
"AppStore",
"Biometric",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Caution Review failedPull request was closed or merged during review WalkthroughThis change modifies the default Apple Device Enrollment Program (DEP) enrollment profile configuration in the 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@MagnusHJensen good catch! I think we do. I opened a PR to update the |
Region, since we have customers in more than the US now.IsSupervised, as devices are automatically supervised now.Summary by CodeRabbit
Release Notes