BYOD permissions [5/N]: Service-layer AccessRights resolver#46129
Open
georgekarrv wants to merge 14 commits into
Open
BYOD permissions [5/N]: Service-layer AccessRights resolver#46129georgekarrv wants to merge 14 commits into
georgekarrv wants to merge 14 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).
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.
![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.
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
georgekarrv
force-pushed
the
gkarr-23242-byod-permissions-dynamic-5
branch
from
d324281 to
098a122
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## gkarr-23242-byod-permissions-dynamic #46129 +/- ##
======================================================================
Coverage 66.82% 66.83%
======================================================================
Files 2754 2755 +1
Lines 220172 220299 +127
Branches 11042 11042
======================================================================
+ Hits 147136 147235 +99
- Misses 59742 59762 +20
- Partials 13294 13302 +8
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
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 5 of N for #23242. Targets feature branch `gkarr-23242-byod-permissions-dynamic`. Stacks on 1/N, 2/N, 3/N, 4/N.
Summary
Service-layer helper that combines PR-4's pure narrowing function with the DB lookups it needs. Given `teamID` and optional `hostID` it returns the AccessRights to bake into the next Apple enrollment profile delivered for that host.
```go
func (svc *Service) computeAppleEnrollmentAccessRights(ctx context.Context, teamID *uint, hostID uint) (int, error) {
// 1) Load (allow_byod_wipe, allow_byod_lock) from TeamMDMConfig (or AppConfig when teamID == nil).
// 2) For hostID == 0, return the fleet ceiling — no stored row to AND with.
// 3) Otherwise load host_mdm_apple_enrollment_permissions; if absent, return ceiling.
// 4) Otherwise return apple_mdm.ComputeAppleEnrollmentAccessRights(allow_wipe, allow_lock, &stored).
}
```
Also adds `fleetBYODPermissions` as a small private helper for the team-vs-global lookup.
Still no behavior change. Nothing calls these yet. PR-6 wires them into the SCEP/ACME profile-serving endpoints and persists the chosen value via `SetHostMDMAppleEnrollmentPermissions` after delivery.
Caller contract
Caller short-circuits non-BYOD cases:
Those just pass `apple_mdm.MDMAccessRightAll` directly to the profile generator. The helper here is for BYOD/manual Apple enrollments only.
Test plan