BYOD permissions [10/N]: Controls > OS Settings UI card#46136
Open
georgekarrv wants to merge 28 commits into
Open
BYOD permissions [10/N]: Controls > OS Settings UI card#46136georgekarrv wants to merge 28 commits into
georgekarrv wants to merge 28 commits into
Conversation
Backend types and defaults for the BYOD permissions feature. No behavior change yet — wipe/lock are not gated by these flags in this PR. New fields default to true everywhere they could otherwise be read as the zero value, preserving today's behavior. - AppConfig.MDM: new optjson.Bool fields, default true via MarshalJSON - TeamMDM (bool) / TeamPayloadMDM / TeamSpecMDM (optjson.Bool): symmetric fields with the same default-true semantics - ee Service.NewTeam: initialize new teams with both flags true - ModifyTeam (payload) + team-spec apply (full and incremental): honor incoming values when set - generate-gitops: include both keys in TeamMDM seeded from AppConfig and in the result map emitted per team - GitOpsControls: new keys for gitops yaml round-trip; client.go mapping defaults to true if the gitops file omits them
- AppConfig YAML/JSON fixtures: add allow_byod_wipe/allow_byod_lock = true (AppConfig.MarshalJSON defaults absent → true on API output). - TeamMDM (per-fleet) JSON fixtures: keep new fields out unless the test data explicitly creates a team with them. team1/team2 test fixtures in get_test.go and generate_gitops_test.go are updated to set true at the source so expected output matches. - macosSetupExpectedTeam*.yml: new fields set to false (savedTeam mock starts as zero-value and the spec doesn't override). - generateGitops fixtures: new keys appear alphabetically at top of controls block; teamConfig.json / appConfig.json seeded with true. - apply_test.go: assert.Equal for team2 (newly created via spec) expects true (createTeamFromSpec defaults true); assertions for team1/savedTeam (existing teams updated via spec) keep false.
Tracks the AccessRights integer last delivered to each BYOD Apple host enrollment profile. Apple MDM profile-replacements cannot widen access rights, only narrow them; SCEP/ACME renewal will use this table as the floor when computing new rights. - Migration 20260523111410: creates the table (host_id PK, access_rights, delivered_at) and backfills existing non-DEP-enrolled hosts at 8191 (the value every enrollment has carried since forever). - HostMDMApplePermissions type added to server/fleet/hosts.go. - Datastore interface: GetHostMDMAppleEnrollmentPermissions + SetHostMDMAppleEnrollmentPermissions. - MySQL implementations + regenerated mock.
- Drop FOREIGN KEY hosts(id) from the new table per handbook/engineering/scaling-fleet.md (avoid InnoDB locking on the hosts table). Test CI explicitly fails on host_id FKs. - Add host_mdm_apple_enrollment_permissions to hostRefs in server/datastore/mysql/hosts.go so rows are cleaned up when a host is deleted (the manual cleanup that replaces FK cascade). - Update migration test: drop the FK-cascade assertion, add an upsert assertion covering the ON DUPLICATE KEY UPDATE path used by the datastore method. - Regenerated schema.sql via make test-schema (includes both the new table and the allow_byod_wipe/lock keys seeded in app_config_json from PR #1; PR #1 will land its app_config_json portion separately).
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## gkarr-23242-byod-permissions-dynamic #46136 +/- ##
========================================================================
+ Coverage 66.82% 66.84% +0.01%
========================================================================
Files 2754 2756 +2
Lines 220172 220477 +305
Branches 11042 10914 -128
========================================================================
+ Hits 147136 147379 +243
- Misses 59742 59777 +35
- Partials 13294 13321 +27
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:
|
6 tasks
georgekarrv
force-pushed
the
gkarr-23242-byod-permissions-dynamic-10
branch
from
a96ff82 to
84ea89d
Compare
georgekarrv
force-pushed
the
gkarr-23242-byod-permissions-dynamic-10
branch
from
84ea89d to
988da69
Compare
georgekarrv
force-pushed
the
gkarr-23242-byod-permissions-dynamic-10
branch
from
988da69 to
da33a5d
Compare
georgekarrv
force-pushed
the
gkarr-23242-byod-permissions-dynamic-10
branch
from
da33a5d to
aaf5727
Compare
Two review-driven fixes to PR-2:
- Existing migration (20260523111410): the backfill SELECT had no
platform predicate, so any Windows host with host_mdm.enrolled=1
AND installed_from_dep=0 (the common case — only AAD/Autopilot
sets installed_from_dep=1 on Windows) would land in this
Apple-specific table. Add JOIN hosts ... WHERE platform IN
('darwin','ios','ipados'). Added a Windows-host case to the
existing test to assert the exclusion.
- New migration (20260524120459_BackfillTeamBYODPermissions):
TeamMDM.AllowBYODWipe/Lock are plain bool, and TeamMDMConfig
decodes the stored JSON without defaults. Pre-PR teams' configs
lack both keys and would silently decode to false the moment
PR-7's BYOD gate goes live, blocking every Wipe/Lock on every
manually-enrolled Apple host on every pre-existing team. Backfill
both keys to true (matching the new-team initializer in
ee/server/service/teams.go and the AppConfig MarshalJSON default)
via JSON_SET ... WHERE JSON_EXTRACT IS NULL so any explicit values
(e.g. from a gitops apply between PR-1's land and this migration's
run) are preserved. Test covers absent-keys backfill, explicit
false preserved, and partial set (one key present, other missing).
Regenerated server/datastore/mysql/schema.sql via make test-schema.
Templates the AccessRights integer in the SCEP-backed manual enrollment
profile and the ACME enrollment profile, instead of hard-coding 8191.
Callers pass the value explicitly; for now they all pass
apple_mdm.MDMAccessRightAll so behavior is unchanged. PR-4 will compute
the value from the host's stored permissions intersected with the
owning fleet's BYOD config.
- New constants in server/mdm/apple/apple_mdm.go:
MDMAccessRightAll = 8191
MDMAccessRightDeviceLock = 512 (bit 9)
MDMAccessRightDeviceErase = 1024 (bit 10)
- New helper AppleEnrollmentAccessRights(allowWipe, allowLock bool) int
starts from MDMAccessRightAll and strips bits per the flags.
- enrollmentProfileMobileconfigTemplate and
acmeEnrollmentProfileMobileconfigTemplate now take {{ .AccessRights }}.
The account-driven user-enrollment template is unchanged — it does not
carry AccessRights (Apple's User Enrollment imposes its own limits).
- GenerateEnrollmentProfileMobileconfig and
GenerateACMEEnrollmentProfileMobileconfig grow an accessRights int
parameter; all 8 production call sites in server/service and
ee/server/service now pass apple_mdm.MDMAccessRightAll.
- Tests: TestAppleEnrollmentAccessRights covers all four
(wipe, lock) combinations. TestGenerateEnrollmentProfileMobileconfig_AccessRights
parses the generated mobileconfig and asserts the templated bitmask
matches what was passed in.
Pure function that computes the AccessRights to bake into a host's
next-delivered Apple enrollment profile. Implements the monotonic
narrowing rule Apple enforces on profile-replacement:
new_rights = stored_rights AND fleet_ceiling
For initial enrollment (storedRights == nil) the result is the fleet
ceiling. For renewals the bitwise AND ensures Fleet can never request
more rights than were previously installed — which Apple would reject.
No call sites are wired yet. PR-5 will wire it into the SCEP/ACME
profile-serving code paths and persist the chosen value via the
host_mdm_apple_enrollment_permissions table introduced in PR-2.
Tests cover both worked examples from gk/23242-plan.md:
- Example 1: stored=8191, fleet narrows to lock-only → 7679
- Example 2: stored=6655 (no wipe/lock at enrollment), fleet now
allows both → still 6655 (AND with 8191 is no-op; Apple would
reject widening)
Service-layer helper that combines PR-4's pure narrowing function with DB lookups. Given a teamID and optional hostID it: 1. Loads the team's BYOD permissions (or global AppConfig when teamID is nil) to derive the fleet ceiling. 2. For hostID != 0, loads the host's previously-delivered AccessRights from host_mdm_apple_enrollment_permissions (PR-2's table). 3. Returns the bitwise AND of the two (or just the ceiling for initial enrollments where no row exists yet). Callers are responsible for short-circuiting non-BYOD cases (ADE-enrolled hosts, non-Apple platforms) before calling this — those pass apple_mdm.MDMAccessRightAll directly to the profile generator. Also adds fleetBYODPermissions(teamID) helper used internally. Still no behavior change: nothing calls these yet. PR-6 will wire them into the SCEP/ACME profile-serving endpoints + persist via SetHostMDMAppleEnrollmentPermissions after the profile is delivered. Tests cover: - global config path (with and without flags explicitly set) - team config path (and that it bypasses AppConfig) - hostID=0 skipping the stored-rights lookup - NotFound stored-rights handled as "no floor" - monotonic-narrowing (example 2 from the plan: re-enabling can't widen) - error propagation from both AppConfig and the permissions table
Threads machineInfo through generateMDMAppleSCEPEnrollProfile and
generateMDMAppleACMEEnrollProfile so they can resolve the host context
(by UDID) at profile-serve time and compute the right AccessRights to
bake into the .mobileconfig.
Adds two small glue methods:
- resolveEnrollmentAccessRightsForServe(ctx, machineInfo)
Looks up the host by UDID via HostLiteByIdentifier. If found,
uses its TeamID + ID to drive computeAppleEnrollmentAccessRights
(renewal path, monotonic narrowing). If not found (initial
enrollment with no host record yet), falls back to global config.
- persistEnrollmentAccessRights(ctx, hostID, accessRights)
Records what we delivered via SetHostMDMAppleEnrollmentPermissions
so the next renewal can AND with it. Skips for hostID == 0
(initial enrollment; persistence happens at TokenUpdate later).
Note on initial enrollment: at serve time the device hasn't yet created
its host record, so we don't know the team. We use global AppConfig.
Per-enrollment-token team resolution would be a follow-up if product
wants different defaults per team enrollment URL.
For non-BYOD cases (ADE hosts), this endpoint is not the relevant flow;
ADE enrollments go through a separate DEP-served profile path that still
passes MDMAccessRightAll.
Tests cover:
- nil machineInfo → global config + hostID=0 (no DB lookup, no persist)
- unknown UDID → global config + hostID=0
- known host with team → team config + stored rights AND
- HostLiteByIdentifier non-NotFound error propagates
- persistEnrollmentAccessRights writes only when hostID != 0
After this PR, resolveEnrollmentAccessRightsForServe looks up the host by UDID to decide what AccessRights to bake into the served profile. TestGetMDMAppleEnrollmentProfileByToken exercises initial-enrollment fixtures where the host does not yet exist, so the lookup must return NotFound (not nil) — otherwise it panics on a nil mock func.
Two enrollment-profile generation paths were still passing apple_mdm.MDMAccessRightAll instead of threading the new BYOD permission ceiling from PRs 4-5: - ee/server/service/mdm.go GetMDMManualEnrollmentProfile (admin-UI download). The handler already loads AppConfig - use its BYOD flags. No host/team context at this point (the team is only known after TokenUpdate), so the global AppConfig ceiling applies. - server/service/apple_mdm.go MDMAppleProcessOTAEnrollment (the canonical BYOD enrollment flow). enrollSecretInfo.TeamID is in scope and used elsewhere in the same function, so use team-level narrowing via the existing svc.fleetBYODPermissions helper. Without these fixes, when an admin set allow_byod_wipe=false (or allow_byod_lock=false), a device enrolling through either path would install a profile with full rights and not narrow until the first SCEP renewal (up to scepCertRenewalThresholdDays=180 days later). Fleet's service-layer wipe/lock gate still blocked the Fleet-issued command, but the device-level defense-in-depth the design relies on was missing.
Adds the service-layer enforcement that makes the BYOD permissions
feature actually functional. For manually-enrolled (On (manual)) Apple
hosts, LockHost and WipeHost in the EE service now refuse the command
when the host's effective AccessRights lack the corresponding bit.
The effective rights are the bitwise AND of:
- the host's stored AccessRights from
host_mdm_apple_enrollment_permissions (or MDMAccessRightAll if no
row exists yet), and
- the owning fleet's BYOD ceiling derived from allow_byod_wipe /
allow_byod_lock (with global AppConfig as the fallback when the
host has no team).
ADE-enrolled hosts and non-Apple platforms are not affected — the gate
is bypassed for any enrollment status other than "On (manual)".
New helper: (svc *Service).effectiveAppleAccessRights(ctx, host).
New error messages (server/fleet/errors.go):
- WipeNotAllowedForBYODMessage
- LockNotAllowedForBYODMessage
Both copy is intentionally generic so it covers both possible reasons
(fleet config disallows OR host enrolled before the right was granted),
matching the UX guidance in the plan.
Tests cover:
- ceiling with/without explicit AppConfig flags
- team-level config used and AppConfig bypassed when team is set
- stored-AND-ceiling intersection
- monotonic narrowing example (stored=no-wipe, fleet=allow-all -> stays no-wipe)
- error propagation from AppConfig, TeamMDMConfig, and the permissions table
When MDM isn't configured for a host, or the host isn't connected to Fleet MDM, the existing checks already short-circuit with appropriate errors. Running the BYOD permission gate before those checks broke an existing fleetctl integration test (TestMDMLockCommand: "valid macos but only windows mdm") because the gate's enrollment-permissions lookup ran for a host that was never going to be wipeable/lockable in the first place — and the test's mock store doesn't wire GetHostMDMAppleEnrollmentPermissions, so it nil-panicked. Reorder: BYOD gate runs only after VerifyMDMAppleConfigured and the Fleet MDM connectivity check have succeeded. Same change applied to both LockHost and WipeHost. Also apply gofmt to hosts_test.go.
TestMDMLockCommand / TestMDMWipeCommand exercise the lock/wipe code paths after PR-7 added the BYOD permission gate to LockHost/WipeHost. For a manually-enrolled Apple host that passes the MDM-configured + connected checks, the gate calls ds.GetHostMDMAppleEnrollmentPermissions to resolve the host's currently-effective rights. Without a mock the call nil-panics. Return NotFound so the gate falls back to the fleet ceiling alone (default-true allow_byod_*) — i.e. no restriction, matching the test's prior expectation that lock/wipe succeed on a happy-path "valid macos" / "valid windows" fixture.
Adds two booleans to the host details MDMHostData so the frontend can
disable the wipe/lock action buttons without re-deriving the policy.
GET /api/v1/fleet/hosts/:id → body.host.mdm.wipe_allowed: boolean
body.host.mdm.lock_allowed: boolean
Fields are *bool with omitempty so they only render on endpoints that
populate them (host details). The list-hosts response is unchanged.
Rules (resolveHostWipeLockAllowed):
- non-Apple host → both true
- "On (automatic)" (ADE) → both true
- "On (personal)" (account-driven user enrollment) → both false
- "On (manual)" BYOD → derived from effective AccessRights
(host's stored AccessRights AND fleet ceiling); mirrors the gate
enforced server-side by LockHost/WipeHost in the EE service. iOS
and iPadOS manual hosts additionally get lock_allowed=false
because Apple MDM has no manual-iOS lock.
- Off / not enrolled → both false
Populated in svc.getHostDetails next to DeviceStatus / PendingAction.
Tests cover all branches including the monotonic-narrowing case from
the plan (host enrolled before wipe was enabled → wipe_allowed stays
false even when fleet config now allows it).
…lowed getHostDetails now populates host.mdm.wipe_allowed and host.mdm.lock_allowed (true for the non-MDM test host fixture). Two fleetctl changes follow: - testdata/expectedHostDetailResponseJson.json testdata/expectedHostDetailResponseYaml.yml Add the new keys (both true) to the expected host detail payload. Keys are placed alphabetically to match the actual JSON / YAML output ordering. - mdm_test.go TestMDMRunCommand's `HostByIdentifier` calls reach getHostDetails → resolveHostWipeLockAllowed → computeAppleEnrollmentAccessRights → ds.GetHostMDMAppleEnrollmentPermissions. The test sets up its own mocks (separate from setupDSMocks). Wire the new mock with NotFound so the gate falls back to the fleet ceiling alone (true by default).
Updates the RenewSCEPCertificates cron so each host's renewal profile is generated with the AccessRights it should actually have right now, rather than a hard-coded 8191. For each host going through SCEP/ACME renewal: 1. Look up host by UUID (HostLiteByIdentifier) -> host_id, team_id. 2. Load fleet ceiling from team config (or global AppConfig). 3. Load stored AccessRights from host_mdm_apple_enrollment_permissions (treat NotFound as "no prior delivery"). 4. Compute new_rights = ComputeAppleEnrollmentAccessRights(...) = stored AND ceiling. 5. Generate per-host enrollment profile with new_rights. 6. Send via renewMDMAppleEnrollmentProfile (single assoc). 7. Persist new_rights via SetHostMDMAppleEnrollmentPermissions. Code paths updated: - assocsWithoutRefs (was batched, now per-host) - assocsWithRefs (was per-host, now also computes rights + persists) - acmeAssocsByHostUUID (was per-host, now also computes rights + persists) Skipped (correctly): - userDeviceAssocs (Account Driven User Enrollment — its mobileconfig template carries no AccessRights key; Apple's User Enrollment imposes its own permission ceiling). - assocsFromMigration (uses external FLEET_SILENT_MIGRATION_ENROLLMENT_ PROFILE which we don't control). The previous batch optimization for assocsWithoutRefs is gone. Renewal runs hourly with maxCertsRenewalPerRun=100 so per-host work is bounded to ~100 profile generations per run — well within budget for SCEP cert renewal which is rare per host (every ~6 months). New free function: resolveRenewalAccessRights(ctx, ds, hostUUID). New free function: loadFleetBYODPermissions(ctx, ds, teamID).
- server/service/apple_mdm.go: gofmt fixed an indented-block formula comment in the resolveRenewalAccessRights doc. - server/service/apple_mdm_test.go: TestRenewSCEPCertificatesBranches and TestRenewACMECertificatesBranches now exercise the per-host AccessRights resolution added in this PR. The shared test setup doesn't seed host records, so wire HostLiteByIdentifierFunc to return NotFound. The helper then falls back to MDMAccessRightAll with hostID=0, which skips persistence — matching the existing test expectations.
The SCEP / ACME renewal cron called resolveRenewalAccessRights on every renewing host, including ADE (DEP / Automated Device Enrollment) hosts. When an admin set allow_byod_wipe=false or allow_byod_lock=false in the fleet config, those settings would silently leak into the renewal profile of supervised, corporate- owned devices and strip Wipe / Lock from IT's toolbox. The BYOD settings target personal enrollments by design; ADE hosts are out of scope. Look up host_mdm at the start of the helper and return MDMAccessRightAll when installed_from_dep is set, before any fleet config or stored rights are consulted. If host_mdm has no row for the host (extremely unusual at renewal time — the cron pulls candidates from the SCEP table, not host_mdm) fall through to the BYOD path. host_mdm DB errors propagate. Test coverage: ADE short-circuit (verifies AppConfig / TeamMDMConfig / GetHostMDMAppleEnrollmentPermissions are NOT invoked), GetHostMDM not-found fall-through, GetHostMDM error propagation. Existing known-host tests gained GetHostMDMFunc mocks returning a non-ADE row so they reach the BYOD path.
New "BYOD permissions" section under Controls > OS Settings with two
checkboxes — "Allow wipe" and "Allow lock" — for limiting what Fleet
can do to manually enrolled (BYOD) Apple hosts that belong to a fleet.
- Component: frontend/pages/ManageControlsPage/OSSettings/cards/
BYODPermissions/{BYODPermissions.tsx, _styles.scss, index.ts}
Mirrors the Passwords card structure: section header, page
description, two checkboxes, save button, GitOps lock support, MDM-
enabled empty state, premium feature gate.
- Route: PATHS.CONTROLS_BYOD_PERMISSIONS = /controls/os-settings/byod-permissions
- OSSettingsNavItems: appended to the sidebar after "Passwords"
- IMdmConfig + ITeam.mdm: new allow_byod_wipe / allow_byod_lock booleans
(typed; both default to true on read)
Apply path:
- Global config (no fleet selected) -> configAPI.update({ mdm: { ... } })
- Per-fleet -> teamsAPI.updateConfig({ mdm: { ... } }, currentTeamId)
Both endpoints accept arbitrary JSON keys via mdm, so no service-layer
changes are needed for the round-trip.
The description copy in the card warns users about the asymmetric
nature of the setting (re-enabling does not restore on already-
enrolled hosts — they would need to re-enroll), matching the design
guidance in gk/23242-plan.md.
IMdmConfig grew two required fields in this PR; the shared test config mock has to provide them or TypeScript compilation fails in any test that imports the mock. Both default to true to match the production default.
georgekarrv
force-pushed
the
gkarr-23242-byod-permissions-dynamic-10
branch
from
a8ea071 to
a3e82b2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked PR 10 of N for #23242. First frontend PR in the stack. Targets feature branch `gkarr-23242-byod-permissions-dynamic`. Stacks on PRs 1/N–9/N.
Summary
Adds a new "BYOD permissions" section under Controls > OS Settings with two checkboxes — "Allow wipe" and "Allow lock" — for limiting what Fleet can do to manually enrolled (BYOD) Apple hosts in a fleet.
Files
Component behavior
Mirrors the Passwords card pattern:
Copy
The page description warns users the setting is asymmetric:
This matches the Apple monotonic-narrowing rule encoded in PRs 4–9 of this stack.
Test plan
What's next