Android commands (frontend + more backend) - #46174
Merged
Merged
Conversation
Tracks AMAPI commands issued by Fleet via EnterprisesDevicesService.IssueCommand
(Lock, Wipe, Clear passcode for Android hosts). host_mdm_actions.{lock_ref, wipe_ref}
points into this table via command_uuid for Android hosts, mirroring how those
columns point into nano_commands for Apple and mdm_windows_commands for Windows.
MDMAndroidCommand maps to the mdm_android_commands table. Status and type values mirror the strings AMAPI uses on the wire so the same enum doubles as the value we send in IssueCommand and the value we read back from Pub/Sub COMMAND notifications.
Adds the four CRUD methods Fleet needs at AMAPI command-issue and
Pub/Sub COMMAND-notification time:
- NewMDMAndroidCommand: insert at issue time (auto-generates command_uuid
if caller leaves it empty)
- GetMDMAndroidCommandByUUID: lookup by Fleet command_uuid (the value
host_mdm_actions.{lock_ref, wipe_ref} points to)
- GetMDMAndroidCommandByOperationName: lookup by AMAPI operation name,
used by the Pub/Sub handler to correlate a notification back to the
originating Fleet command
- UpdateMDMAndroidCommandStatus: transition status (and optional error
code/message) when the device acks or AMAPI rejects
Interface declared on fleet.Datastore (mirrors NewAndroidPolicyRequest /
GetAndroidPolicyRequestByUUID), with matching mock entries and an
integration test exercising all four methods plus NotFound paths.
Teaches HostLockWipeStatus to interpret AMAPI lock/wipe state stored in mdm_android_commands, so the existing PendingAction / DeviceStatus / IsLocked / IsWiped helpers work for android hosts the same way they do for darwin, ios, ipados, windows, and linux. - GetHostLockWipeStatus: new android case that maps lock_ref/wipe_ref through mdm_android_commands and populates Lock/WipeMDMCommand + Lock/WipeMDMCommandResult. Pending status leaves the result nil so IsPendingLock/IsPendingWipe report "pending". Orphan refs log and return zero-state (mirrors apple/windows). - GetHostsLockWipeStatusBatch: matching android collection + batch query against mdm_android_commands. - HostLockWipeStatus methods (server/fleet/scripts.go): android arms added to IsPendingLock, IsPendingWipe (covered by the existing fall through), IsLocked, and IsWiped. They compare against the literal "acknowledged" string (the value of android.MDMAndroidCommandStatusAcknowledged) to avoid pulling the android package into server/fleet.
Wires the Android Management API IssueCommand endpoint through Fleet's three Client implementations so the service layer can dispatch LOCK, RESET_PASSWORD, and WIPE commands to enrolled Android devices: - Client interface (androidmgmt/client.go): add the method declaration with a docstring pointing at the AMAPI reference. - GoogleClient: direct AMAPI passthrough with the standard error wrapping pattern used by neighbouring methods. - ProxyClient: same passthrough, plus the Authorization: Bearer header the proxy needs for every call. - mock/Client: hand-maintained mock additions (Func type, struct field + Invoked flag, method wrapper) so service-layer tests can swap in fakes without contacting real AMAPI. Validated by porting the spike code that exercised this against real BYO and COBO devices for all three command types.
Replaces the spike's log-only implementations with the real
issue-and-persist flow:
1. Service layer (server/mdm/android/service.go + service/service.go):
- LockAndroidHost: AMAPI LOCK -> NewMDMAndroidCommand +
host_mdm_actions.lock_ref (via LockHostViaAndroidMDM).
- ClearAndroidPasscode: AMAPI RESET_PASSWORD with newPassword=""
(clears, does not regenerate, per product); persists the row but
does NOT touch host_mdm_actions (one-shot, no UI state).
- WipeAndroidHost: AMAPI WIPE -> NewMDMAndroidCommand +
host_mdm_actions.wipe_ref (via WipeHostViaAndroidMDM). WipeParams
intentionally set to an empty struct -- the spike confirmed AMAPI
rejects WIPE without it, despite the SDK marking it Optional.
- resolveAndroidCommandTarget helper centralises the host/enterprise
lookup + secret refresh shared by all three methods.
- longCommandDuration = "315360000s" (10 years) on every command, so
undelivered commands stay queued forever -- matches Apple/Windows
MDM semantics. AMAPI default of 600s would silently expire.
2. Datastore (server/fleet/datastore.go + datastore/mysql/android.go):
- LockHostViaAndroidMDM / WipeHostViaAndroidMDM: transactional
two-write helpers that insert the mdm_android_commands row and
upsert host_mdm_actions in a single retried tx. Mirrors
WipeHostViaWindowsMDM. Generated mocks updated.
- Integration tests cover the lock-pending path and the wipe-requeue
path (later wipe_ref overwrites earlier, both command rows preserved
for audit).
Pub/Sub COMMAND ack handling (Phase 9) and EE dispatch (Phase 10) are
follow-ups; this commit gets the command-issue side end-to-end.
Per product (2026-05-20): unenrolling a BYO Android host should run an AMAPI WIPE command, which on a personal device only wipes the work profile and leaves the personal side intact. The mdm_unenrolled activity is still emitted on the subsequent Pub/Sub COMMAND path (unchanged from the previous behavior). COBO unenroll keeps the existing EnterprisesDevicesDelete call -- it terminates management without factory-resetting the device. UnenrollAndroidHost branches on host_mdm.is_personal_enrollment. The BYO branch persists an mdm_android_commands row (audit trail) but does NOT write host_mdm_actions.wipe_ref, because BYO unenroll surfaces in the UI as "unenroll", not as a wipe.
AMAPI delivers a COMMAND notification when a device acks (or rejects) an issued command. The notification is an Operation envelope whose Name matches the operation_name we recorded at IssueCommand time. This commit turns the spike's log-only handler into a real state machine: - Decodes the payload as androidmanagement.Operation. - Looks up the Fleet row by operation_name. NotFound (e.g. a command issued from a previous deployment) is acked without retry to keep Pub/Sub from looping. - Empty op.Name is logged and acked (malformed/foreign payload). - If op.Error is set, transitions the row to "error" with error_code (the int google.rpc.Code rendered as string) and error_message. - Otherwise transitions to "acknowledged". - Already-terminal rows are ignored -- AMAPI delivers at-least-once, so we must be idempotent. host_mdm_actions does not need a separate update because the android branch of GetHostLockWipeStatus reads the row status string directly, and HostLockWipeStatus.IsLocked/IsWiped compare against "acknowledged". Unit tests cover all five paths (ack, error, idempotent re-delivery, unknown op, empty op.Name).
Threads the android.Service Lock/Wipe/ClearPasscode helpers (added in the previous commit) into the EE Service layer so they're reachable from the existing /hosts/:id/lock, /hosts/:id/wipe, /hosts/:id/mdm/passcode API endpoints. - LockHost case "android": validates AndroidEnabledAndConfigured + IsHostConnectedToFleetMDM. Lock is supported on both BYO and COBO. - enqueueLockHostRequest case "android": dispatches to LockAndroidHost and explicitly resets activity.ViewPIN -- Android has no unlock PIN. - WipeHost case "android": rejects BYO with a descriptive error (BYO unenroll already runs an AMAPI WIPE under the hood) and validates AndroidEnabledAndConfigured. requireMDM is set so the existing MDM-connection precondition runs. - enqueueWipeHostRequest case "android": dispatches to WipeAndroidHost. - ClearPasscode: new clearPasscodeAndroid branch mirroring clearPasscodeApple's shape. Validates AndroidEnabledAndConfigured, dispatches to ClearAndroidPasscode, emits the existing cleared_passcode activity (shared with Apple), returns a CommandEnqueueResult shaped for API compatibility. Error message updated to mention Android.
Mirrors the existing lock/unlock/wipe subcommands and exposes clear- passcode at the CLI for iOS/iPadOS (existing) and Android (new in this PR). The server endpoint at /hosts/:id/clear_passcode already dispatches by platform, so the CLI doesn't need android-specific branching. - Promotes the previously-private clearPasscodeResponse to fleet.ClearPasscodeResponse, matching LockHostResponse / WipeHostResponse and letting client + server share the type. - Adds Client.MDMClearPasscodeHost in server/service/client_mdm.go. - Adds mdmClearPasscodeCommand in cmd/fleetctl/fleetctl/mdm.go and registers it under `fleetctl mdm clear-passcode` (alias clear_passcode). - Unit test TestMDMClearPasscodeCommand exercises the failure paths (missing flag, unknown host, MDM-off). Happy paths against real iOS/Android hosts are covered by integration tests.
End-to-end coverage against the real Fleet HTTP handler stack with the
mock AMAPI client. Verifies that for each command the API endpoint
issues the right AMAPI call, persists the mdm_android_commands row,
updates host_mdm_actions where applicable, and surfaces the right
device/pending status on the host detail endpoint.
- TestAndroidHostUnenrollMDM: updated to match the new BYO behavior
-- BYO unenroll now issues an AMAPI WIPE (work-profile only), not
EnterprisesDevicesDelete. Added a COBO sub-case to assert the
delete path is still used for company-owned hosts.
- TestAndroidLockWipeClearPasscode (new):
- Lock COBO: AMAPI LOCK + Duration=315360000s; GET host returns
PendingAction=lock + DeviceStatus=unlocked; Pub/Sub COMMAND ack
transitions the row to acknowledged and the host page to locked.
- Wipe BYO rejected with the "personally-owned" error message.
- Wipe COBO: AMAPI WIPE with non-nil empty WipeParams + long
duration; PendingAction=wipe surfaces.
- Clear passcode: AMAPI RESET_PASSWORD with newPassword=""; no
host_mdm_actions write.
- Pub/Sub COMMAND with op.Error transitions the row to "error" and
populates error_code (the int rpc code as string) and
error_message.
To support both BYO and COBO test fixtures, factored
createAndroidHostWithStorage into createAndroidHostForTest taking an
explicit companyOwned bool; the existing helper now just wraps it for
BYO callers.
Also replaced the private clearPasscodeResponse reference in the
existing iOS clear-passcode integration test with the public
fleet.ClearPasscodeResponse promoted earlier in this branch.
Addresses the two failing CI checks on PR #46031: test-mock-changes: server/mock/datastore_mock.go had manual edits from the earlier commits adding android Lock/Wipe/Clear-passcode datastore methods. CI runs `make mock` and fails on any diff. Regenerated via `make mock`; the only change is field/type ordering. lint-incremental: - modernize: `ptr.Bool(true)` -> `new(true)` in the new clear-passcode CLI test fixture. - modernize: `map[string]interface{}` -> `map[string]any` in the BYO- unenroll and Wipe payload-marshal helpers (4 sites). - testifylint: `require.Equal(t, "", ...)` -> `require.Empty(t, ...)` in three places and `require.Equal(t, n, len(s))` -> `require.Len(t, s, n)` in one place within TestAndroidLockWipeClearPasscode.
…commands-backend # Conflicts: # server/datastore/mysql/schema.sql
After merging main (#46079 renumbered 11 migrations to 20260522195224-..235), our 20260521205417_AddMDMAndroidCommands ended up before them in the migration timeline. Bump our timestamp to 20260522195236 so the migration runs last, and regenerate schema.sql.
![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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/mdm/android/service/pubsub.go (1)
548-583:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSuppress duplicate
mdm_unenrolledactivity in the ENROLLMENTDELETEDpath.This branch still ignores the
didUnenrollresult fromSetAndroidHostUnenrolled, so a redelivered ENROLLMENT notification—or one that arrives after the COMMAND/STATUS_REPORT path already flipped the host—will emit a secondmdm_unenrolledactivity for the same event. The STATUS_REPORT branch already has the right guard.🔧 Minimal fix
- if _, err := svc.ds.SetAndroidHostUnenrolled(ctx, host.Host.ID); err != nil { + didUnenroll, err := svc.ds.SetAndroidHostUnenrolled(ctx, host.Host.ID) + if err != nil { return ctxerr.Wrap(ctx, err, "set android host unenrolled on DELETED state (ENROLLMENT)") } // cancel any apps pending install for this host users, acts, err := svc.ds.MarkAllPendingVPPInstallsAsFailedForAndroidHost(ctx, host.Host.ID) @@ for i, act := range acts { user := users[i] if err := svc.newActivity(ctx, user, act); err != nil { return ctxerr.Wrap(ctx, err, "create failed app install activity") } } + + if !didUnenroll { + return nil + } var displayName, serial string if hosts, herr := svc.fleetDS.ListHostsLiteByIDs(ctx, []uint{host.Host.ID}); herr == nil && len(hosts) == 1 && hosts[0] != nil { displayName = hosts[0].DisplayName() serial = hosts[0].HardwareSerial🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/mdm/android/service/pubsub.go` around lines 548 - 583, Capture the boolean returned by svc.ds.SetAndroidHostUnenrolled (e.g., didUnenroll, err := svc.ds.SetAndroidHostUnenrolled(...)) and if didUnenroll is false return nil (or otherwise skip the unenroll-specific work) so you do not emit a duplicate mdm_unenrolled activity; only run clearAndroidBYOWipeRef, svc.ds.MarkAllPendingVPPInstallsAsFailedForAndroidHost, and the svc.newActivity(...) that emits fleet.ActivityTypeMDMUnenrolled when didUnenroll is true (keep existing error handling for the DB calls).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@server/mdm/android/service/pubsub.go`:
- Around line 548-583: Capture the boolean returned by
svc.ds.SetAndroidHostUnenrolled (e.g., didUnenroll, err :=
svc.ds.SetAndroidHostUnenrolled(...)) and if didUnenroll is false return nil (or
otherwise skip the unenroll-specific work) so you do not emit a duplicate
mdm_unenrolled activity; only run clearAndroidBYOWipeRef,
svc.ds.MarkAllPendingVPPInstallsAsFailedForAndroidHost, and the
svc.newActivity(...) that emits fleet.ActivityTypeMDMUnenrolled when didUnenroll
is true (keep existing error handling for the DB calls).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d1f66774-9288-41a8-803e-1e467974fbae
📒 Files selected for processing (23)
frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tests.tsxfrontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsxfrontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsxfrontend/pages/hosts/details/HostDetailsPage/modals/ClearPasscodeModal/ClearPasscodeModal.tsxfrontend/pages/hosts/details/HostDetailsPage/modals/LockModal/LockModal.tsxfrontend/pages/hosts/details/HostDetailsPage/modals/UnenrollMdmModal/UnenrollMdmModal.tsxfrontend/pages/hosts/details/HostDetailsPage/modals/WipeModal/WipeModal.tsxfrontend/pages/hosts/details/cards/HostHeader/HostHeader.tests.tsxfrontend/pages/hosts/details/cards/HostHeader/HostHeader.tsxserver/datastore/mysql/android.goserver/datastore/mysql/android_test.goserver/datastore/mysql/hosts_test.goserver/datastore/mysql/migrations/tables/20260526120000_AddClearPasscodeRefToHostMDMActions.goserver/datastore/mysql/schema.sqlserver/datastore/mysql/scripts.goserver/fleet/datastore.goserver/fleet/scripts.goserver/mdm/android/service/pubsub.goserver/mdm/android/service/pubsub_test.goserver/mdm/android/service/reconcile_devices.goserver/mdm/android/service/service.goserver/mock/datastore_mock.goserver/service/integration_mdm_test.go
💤 Files with no reviewable changes (2)
- frontend/pages/hosts/details/HostDetailsPage/modals/LockModal/LockModal.tsx
- frontend/pages/hosts/details/HostDetailsPage/modals/WipeModal/WipeModal.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- server/fleet/datastore.go
- server/datastore/mysql/hosts_test.go
- server/datastore/mysql/migrations/tables/20260526120000_AddClearPasscodeRefToHostMDMActions.go
- server/fleet/scripts.go
- server/datastore/mysql/scripts.go
- frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx
- frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tests.tsx
- server/mock/datastore_mock.go
ksykulev
reviewed
May 28, 2026
| const isAndroidBYOWipe = | ||
| isAndroid(platform) && | ||
| hostMdmDeviceStatus === "wiping" && | ||
| hostMdmEnrollmentStatus === "On (personal)"; |
Contributor
There was a problem hiding this comment.
Can we use pending_action here like we do for location tracking?
https://github.com/fleetdm/fleet/blob/main/frontend/pages/hosts/details/helpers.ts#L114-L117
We could store something like pending_action="unenroll" in host_mdm_actions?
Contributor
There was a problem hiding this comment.
ok so it's not as easy because there is no host_last_known_locations table to look at. I think it might not be worth changing this from what it is right now.
| const isLockableAndroidDevice = | ||
| isAndroid(hostPlatform) && | ||
| isAndroidMdmEnabledAndConfigured && | ||
| isConnectedToFleetMdm && |
Contributor
There was a problem hiding this comment.
isLockableMacOSDevice and isLockableIosOrIpadDevice don't check isConnectedToFleetMdm? I'm not sure which one is correct. My assumption is we might be missing isConnectedToFleetMdm for MacOS and iOS?
Member
Author
There was a problem hiding this comment.
They do check isConnectedToFleetMdm, like here: https://github.com/fleetdm/fleet/blob/victor/41683-android-commands-frontend/frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx#L196-L200
…commands-frontend # Conflicts: # server/datastore/mysql/schema.sql
Contributor
|
Actionable comments posted: 0 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/interfaces/mdm.ts`:
- Around line 330-332: The isAndroidCOBO function currently only checks for
enrollmentStatus === "On (automatic)" and will miss the alternate company-owned
string; update isAndroidCOBO (in frontend/interfaces/mdm.ts) to return true for
both "On (automatic)" and "On (company-owned)" (or otherwise normalize/enumerate
MdmEnrollmentStatus values) so COBO-specific actions are shown for either
string; ensure the comparison handles null safely (enrollmentStatus:
MdmEnrollmentStatus | null).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d93aa9ba-36e8-44d4-9563-f2d5a5bab657
📒 Files selected for processing (6)
frontend/interfaces/mdm.tsfrontend/pages/hosts/details/DeviceUserPage/DeviceUserPage.tsxfrontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsxfrontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsxfrontend/pages/hosts/details/cards/HostHeader/HostHeader.tests.tsxfrontend/pages/hosts/details/cards/HostHeader/HostHeader.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/pages/hosts/details/cards/HostHeader/HostHeader.tests.tsx
- frontend/pages/hosts/details/HostDetailsPage/HostDetailsPage.tsx
Member
Author
|
@ksykulev Ready for re-review |
ksykulev
previously approved these changes
May 28, 2026
…commands-frontend # Conflicts: # server/datastore/mysql/schema.sql
ksykulev
approved these changes
May 28, 2026
3 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
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.
Related issue: Resolves #41683
Updated frontend for Android commands along with additional changes in the backend. Did full QA testing with test plan.
Checklist for submitter
Testing
Database migrations
COLLATE utf8mb4_unicode_ci).Summary by CodeRabbit
New Features
Bug Fixes
Tests