Fix MDM SSO callback 'missing profile' error for Android enrollment - #45046
Merged
Conversation
…45024) When an Android device enrolls via SSO (OTA enrollment), the callback handler tried to fetch an Apple DEP automatic enrollment profile that doesn't exist when only Android MDM is configured. Add an early return for OTA enrollments that skips the DEP profile fetch, matching the existing guard pattern for account-driven enrollments.
Adds TestOTAEnrollSSOWithoutAppleDEPProfile which verifies the full OTA enrollment SSO flow succeeds even when no Apple DEP automatic enrollment profile exists (simulating an Android-only instance). Also adds LoginOTAEnrollSSOUser test helper that drives the complete OTA SSO flow starting from GET /enroll through SAML IdP login to the callback.
![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.
Contributor
WalkthroughThis PR introduces SSO initiator constants and updates MDM SSO initiation and callback logic to dispatch by initiator. The callback now omits profile_token when empty and skips automatic DEP profile lookup for OTA enrollments. Frontend and Orbit code use the new constants. A test helper (LoginOTAEnrollSSOUser) and an integration test (TestOTAEnrollSSOWithoutAppleDEPProfile) validate that OTA SSO redirects back to /enroll without an error when Apple DEP profiles are absent. A changelog entry and a test stub were added. 🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/service/testing_client.go (1)
489-513: 💤 Low valueOptional: extract the shared SAML POST-to-callback flow to reduce duplication.
Steps 3-5 (lines 489–513) are nearly identical to
loginSSOUserWithBodylines 592–618 — the same IdP form-submit, SAMLResponse regex extraction, and callback POST. The only structural difference between the two helpers is how the IdP URL is obtained (via/enrollredirect here vs. Fleet/api/v1/fleet/mdm/ssoinitiation inloginSSOUserWithBody).Extracting a private helper that accepts an already-known IdP URL and drives the SAML credential submission + callback POST would eliminate the duplication:
♻️ Suggested refactor sketch
+// doSAMLFlow drives the IdP credential submission and Fleet SSO callback given +// an already-resolved IdP URL. The cookie jar on client must carry any session +// cookie set before the call. Returns the callback HTTP response. +func (ts *withServer) doSAMLFlow(client *http.Client, idpURL, callbackPath, username, password string) *http.Response { + t := ts.s.T() + resp, err := client.Get(idpURL) + require.NoError(t, err) + parsed, err := url.Parse(resp.Header.Get("Location")) + require.NoError(t, err) + data := url.Values{ + "username": {username}, + "password": {password}, + "AuthState": {parsed.Query().Get("AuthState")}, + } + resp, err = client.PostForm(parsed.Scheme+"://"+parsed.Host+parsed.Path, data) + require.NoError(t, err) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + re := regexp.MustCompile(`name="SAMLResponse" value="([^\s]*)" />`) + matches := re.FindSubmatch(body) + require.NotEmptyf(t, matches, "callback HTML doesn't contain a SAMLResponse value, got body: %s", body) + q := url.QueryEscape(string(matches[1])) + cbResp, err := client.Post(ts.server.URL+callbackPath+"?SAMLResponse="+q, "application/x-www-form-urlencoded", nil) + require.NoError(t, err) + return cbResp +}Then
LoginOTAEnrollSSOUserbecomes:- // Step 2: Follow IdP redirect to get the login page - resp, err = client.Get(idpURL) - require.NoError(t, err) - // Step 3-5: ...identical block... + return ts.doSAMLFlow(client, idpURL, "/api/v1/fleet/mdm/sso/callback", username, password)🤖 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/service/testing_client.go` around lines 489 - 513, Steps 3–5 duplicate logic across LoginOTAEnrollSSOUser and loginSSOUserWithBody; extract that flow into a private helper (e.g., submitSAMLFromIdP) that takes the IdP URL (string), an http.Client instance, credentials (username, password), and the callback base/URL, then performs the form POST to the IdP (reusing client.PostForm), reads the response body, runs the existing regexp.MustCompile(`name="SAMLResponse" value="([^\s]*)" />`) to extract the SAMLResponse, and finally posts the SAMLResponse to the Fleet callback endpoint (reusing client.Post). Replace the duplicated blocks in LoginOTAEnrollSSOUser and loginSSOUserWithBody with calls to this helper, preserving error handling/assertions (require.NoError/require.NotEmptyf) semantics.
🤖 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.
Nitpick comments:
In `@server/service/testing_client.go`:
- Around line 489-513: Steps 3–5 duplicate logic across LoginOTAEnrollSSOUser
and loginSSOUserWithBody; extract that flow into a private helper (e.g.,
submitSAMLFromIdP) that takes the IdP URL (string), an http.Client instance,
credentials (username, password), and the callback base/URL, then performs the
form POST to the IdP (reusing client.PostForm), reads the response body, runs
the existing regexp.MustCompile(`name="SAMLResponse" value="([^\s]*)" />`) to
extract the SAMLResponse, and finally posts the SAMLResponse to the Fleet
callback endpoint (reusing client.Post). Replace the duplicated blocks in
LoginOTAEnrollSSOUser and loginSSOUserWithBody with calls to this helper,
preserving error handling/assertions (require.NoError/require.NotEmptyf)
semantics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 267bf5b9-2884-4e40-972e-0d14887c9efc
📒 Files selected for processing (4)
changes/45024-android-sso-missing-profileee/server/service/mdm.goserver/service/integration_mdm_test.goserver/service/testing_client.go
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes an SSO callback failure during OTA enrollment by skipping Apple DEP profile lookup when enrolling via /enroll?, allowing Android/BYOD SSO enrollment to succeed on instances without Apple MDM configured.
Changes:
- Add an early return in
mdmSSOHandleCallbackAuthfor OTA enrollment callbacks (/enroll?) to avoid Apple DEP profile requirements. - Add an integration regression test covering OTA enrollment SSO without any Apple DEP profile present.
- Add a testing client helper to execute the full OTA enrollment SSO flow via
/enroll?enroll_secret=....
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| server/service/testing_client.go | Adds a helper to drive the OTA enrollment SSO flow end-to-end in integration tests. |
| server/service/integration_mdm_test.go | Adds regression test ensuring OTA enrollment SSO succeeds without Apple DEP profile. |
| ee/server/service/mdm.go | Skips Apple DEP profile access for /enroll? OTA SSO callbacks. |
| changes/45024-android-sso-missing-profile | Adds changelog entry for the fixed Android SSO “missing profile” error. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace raw string literals "ota_enroll", "setup_experience", and "account_driven_enroll" with named constants in fleet.SSO* to prevent typos and missed cases. Constants are defined in server/fleet/app.go so they can be imported by all packages including orbit.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #45046 +/- ##
==========================================
+ Coverage 66.77% 66.81% +0.04%
==========================================
Files 2718 2722 +4
Lines 218795 219073 +278
Branches 10625 10625
==========================================
+ Hits 146100 146381 +281
+ Misses 59531 59526 -5
- Partials 13164 13166 +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:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@server/service/scripts_test.go`:
- Around line 1045-1046: The TestBatchScriptCancel test is an empty stub that
provides no coverage; either implement the test logic for batch script
cancellation by exercising the relevant cancellation flow (create a batch script
via the same helpers used elsewhere in tests, trigger cancellation via the
cancel function or API, assert the script's state changes and any errors) in
TestBatchScriptCancel, or if the functionality is out of scope for this PR,
replace the empty body with a deliberate placeholder by calling t.Skip("TODO:
implement batch script cancellation tests") so the intent is explicit, or simply
remove the TestBatchScriptCancel function if no test is required; locate the
TestBatchScriptCancel function in the file and apply one of these three changes.
In `@server/service/testing_client.go`:
- Around line 521-526: The test helper currently closes the returned HTTP
response (the require.NoError(t, resp.Body.Close()) call) before returning resp,
making the caller unable to read the body; remove the line that closes resp.Body
so the function returns resp intact (keep callbackURL, samlResponse, client.Post
and resp as-is) and ensure callers are responsible for closing resp.Body to
avoid leaks and to match the behavior of the neighboring SSO helpers.
🪄 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: 3fb98dd8-4185-44ff-b3e2-7eb2b095f978
📒 Files selected for processing (3)
server/service/integration_mdm_test.goserver/service/scripts_test.goserver/service/testing_client.go
🚧 Files skipped from review as they are similar to previous changes (1)
- server/service/integration_mdm_test.go
JordanMontgomery
approved these changes
May 12, 2026
5 tasks
JordanMontgomery
added a commit
that referenced
this pull request
May 12, 2026
… enrollment (#45252) Cherry pick of #45046 into v4.85 Closes #45024 ## Summary - Fixed the MDM SSO callback handler returning a `"missing profile: missing profile"` error when an Android device enrolls via SSO (OTA enrollment) on a Fleet instance that does **not** have Apple MDM configured. - Refactored all MDM SSO initiator magic strings (`"ota_enroll"`, `"setup_experience"`, `"account_driven_enroll"`) into named constants (`fleet.SSOInitiatorOTAEnroll`, etc.) to prevent typos and missed cases — which is the class of bug that caused this issue. ## Code walkthrough ### The bug The bug is in `ee/server/service/mdm.go` in `mdmSSOHandleCallbackAuth()`. **The flow:** 1. Android enrollment hits `/enroll?enroll_secret=xxx` → frontend calls `InitiateMDMSSO` with initiator `"ota_enroll"` (`server/service/frontend.go:248`) 2. User authenticates at the SAML IdP 3. The SSO callback arrives at `MDMSSOCallback` → calls `mdmSSOHandleCallbackAuth` 4. After successful SAML auth, the function checks early-exit conditions: - Line 1133: account-driven enrollment (`originalURL == appleMDMAccountDrivenEnrollmentUrl`) → **no match** for OTA - Line 1139: `Initiator != "setup_experience"` → **true** for `"ota_enroll"` → enters the block 5. Line 1140: calls `getAutomaticEnrollmentProfile()` → returns `nil` because **no Apple MDM is configured** 6. Line 1144–1146: `depProf == nil` → **returns `"missing profile"` error** Note that `MDMSSOCallback` (the caller) already has a guard at line 931 that correctly skips the Apple MDM verification for `/enroll?` paths: ```go if !strings.HasPrefix(originalURL, "/enroll?") && ssoRequestData.Initiator != "setup_experience" { if err := svc.VerifyMDMAppleConfigured(ctx); err != nil { ... } } ``` But `mdmSSOHandleCallbackAuth` was missing the equivalent guard — it unconditionally tried to fetch the Apple DEP profile for any non-`setup_experience` initiator. ### The fix Adds an early return for OTA enrollments (where `originalURL` starts with `/enroll?`), matching the existing pattern for account-driven enrollments right above it. OTA enrollments don't use the Apple DEP profile token. ### The refactor Replaced all raw initiator string literals across the backend with named constants defined in `server/fleet/app.go`: | Constant | Value | Used by | |---|---|---| | `fleet.SSOInitiatorOTAEnroll` | `"ota_enroll"` | `/enroll` page (Android, BYOD iPhone/iPad) | | `fleet.SSOInitiatorSetupExperience` | `"setup_experience"` | Orbit agent (macOS Setup Assistant) | | `fleet.SSOInitiatorAccountDrivenEnroll` | `"account_driven_enroll"` | Apple account-driven MDM enrollment | Constants are in `server/fleet/` (not `server/sso/`) so orbit can import them without pulling in Redis dependencies. **Files changed:** - `ee/server/service/mdm.go` — 6 string replacements (switch cases + comparisons) - `server/service/frontend.go` — 1 replacement - `orbit/cmd/orbit/orbit.go` — 1 replacement - `server/service/testing_client.go` — 1 replacement - `server/service/integration_mdm_test.go` — 1 replacement ## Local reproduction ### Setup 1. Started dev server: `build/fleet serve --dev --dev_license` 2. Infrastructure: MySQL, Redis, SimpleSAML IdP via `docker compose up` 3. Created admin user and enroll secret 4. Configured MDM SSO (`entity_id: mdm.test.com`, SimpleSAML IdP at `localhost:9080`) 5. Set `enable_end_user_authentication: true` directly in DB (API blocks this without Apple MDM — matches customer state) 6. **Did NOT configure Apple MDM** — only SSO + EUA, simulating Android-only instance ### Steps 1. `GET https://localhost:8080/enroll?enroll_secret=test_enroll_secret` → 303 redirect to SimpleSAML IdP 2. Completed SAML login programmatically (user: `sso_user`, pass: `user123#`) 3. `POST https://localhost:8080/api/v1/fleet/mdm/sso/callback` with the SAMLResponse ### Before fix ``` === CALLBACK RESULT === Status: HTTP/2 303 Location: /mdm/sso/callback?error=true === SERVER LOGS === ts=2026-05-08T16:53:49Z level=error component=http method=POST uri=/api/v1/fleet/mdm/sso/callback took=12.148708ms err="missing profile: missing profile" ``` ### After fix ``` === CALLBACK RESULT === Status: HTTP/2 303 Location: /enroll?enroll_secret=test_enroll_secret&enrollment_reference=7c67326c-...&initiator=ota_enroll&profile_token= === SERVER LOGS === ts=2026-05-08T17:27:54Z level=info component=http method=POST uri=/api/v1/fleet/mdm/sso/callback took=15.973ms ``` No errors. Successful redirect back to the enrollment page with the enrollment reference. ## Integration test Added `TestOTAEnrollSSOWithoutAppleDEPProfile` which: 1. Configures SSO and creates a team with IdP enabled 2. **Deletes all Apple DEP enrollment profiles** to simulate an Android-only instance 3. Runs the full OTA enrollment SSO flow (GET `/enroll` → SAML IdP login → callback) 4. Verifies the callback redirects to `/enroll?...` with `enrollment_reference` and `initiator=ota_enroll` (not `?error=true`) Confirmed the test **fails without the fix** (`err="missing profile: missing profile"`) and **passes with the fix**. Also added a `LoginOTAEnrollSSOUser` test helper that drives the complete OTA SSO flow starting from `GET /enroll` through SAML IdP login to the callback, using a single cookie jar. ## Test plan - [ ] Verify Android SSO enrollment works on an instance with **only** Android MDM configured (no Apple MDM) - [ ] Verify Apple DEP enrollment with SSO still works (the DEP profile path is unchanged) - [ ] Verify Apple OTA enrollment with SSO still works (also uses `/enroll?` path) - [ ] Verify account-driven enrollment with SSO still works (has its own early return) - [ ] Verify setup experience SSO still works (uses `Initiator == "setup_experience"`) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Resolved a regression where OTA enrollment via SSO could return a "missing profile" error on Android when Apple MDM is not configured; OTA SSO now redirects correctly to the enrollment flow. [](https://app.coderabbit.ai/change-stack/fleetdm/fleet/pull/45046) <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Sharon Katz <121527325+sharon-fdm@users.noreply.github.com> Co-authored-by: Magnus Jensen <magnus@fleetdm.com>
3 tasks
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
](https://app.coderabbit.ai/change-stack/fleetdm/fleet/pull/45046){kind=link}
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.
Closes #45024
Summary
"missing profile: missing profile"error when an Android device enrolls via SSO (OTA enrollment) on a Fleet instance that does not have Apple MDM configured."ota_enroll","setup_experience","account_driven_enroll") into named constants (fleet.SSOInitiatorOTAEnroll, etc.) to prevent typos and missed cases — which is the class of bug that caused this issue.Code walkthrough
The bug
The bug is in
ee/server/service/mdm.goinmdmSSOHandleCallbackAuth().The flow:
/enroll?enroll_secret=xxx→ frontend callsInitiateMDMSSOwith initiator"ota_enroll"(server/service/frontend.go:248)MDMSSOCallback→ callsmdmSSOHandleCallbackAuthoriginalURL == appleMDMAccountDrivenEnrollmentUrl) → no match for OTAInitiator != "setup_experience"→ true for"ota_enroll"→ enters the blockgetAutomaticEnrollmentProfile()→ returnsnilbecause no Apple MDM is configureddepProf == nil→ returns"missing profile"errorNote that
MDMSSOCallback(the caller) already has a guard at line 931 that correctly skips the Apple MDM verification for/enroll?paths:But
mdmSSOHandleCallbackAuthwas missing the equivalent guard — it unconditionally tried to fetch the Apple DEP profile for any non-setup_experienceinitiator.The fix
Adds an early return for OTA enrollments (where
originalURLstarts with/enroll?), matching the existing pattern for account-driven enrollments right above it. OTA enrollments don't use the Apple DEP profile token.The refactor
Replaced all raw initiator string literals across the backend with named constants defined in
server/fleet/app.go:fleet.SSOInitiatorOTAEnroll"ota_enroll"/enrollpage (Android, BYOD iPhone/iPad)fleet.SSOInitiatorSetupExperience"setup_experience"fleet.SSOInitiatorAccountDrivenEnroll"account_driven_enroll"Constants are in
server/fleet/(notserver/sso/) so orbit can import them without pulling in Redis dependencies.Files changed:
ee/server/service/mdm.go— 6 string replacements (switch cases + comparisons)server/service/frontend.go— 1 replacementorbit/cmd/orbit/orbit.go— 1 replacementserver/service/testing_client.go— 1 replacementserver/service/integration_mdm_test.go— 1 replacementLocal reproduction
Setup
build/fleet serve --dev --dev_licensedocker compose upentity_id: mdm.test.com, SimpleSAML IdP atlocalhost:9080)enable_end_user_authentication: truedirectly in DB (API blocks this without Apple MDM — matches customer state)Steps
GET https://localhost:8080/enroll?enroll_secret=test_enroll_secret→ 303 redirect to SimpleSAML IdPsso_user, pass:user123#)POST https://localhost:8080/api/v1/fleet/mdm/sso/callbackwith the SAMLResponseBefore fix
After fix
No errors. Successful redirect back to the enrollment page with the enrollment reference.
Integration test
Added
TestOTAEnrollSSOWithoutAppleDEPProfilewhich:/enroll→ SAML IdP login → callback)/enroll?...withenrollment_referenceandinitiator=ota_enroll(not?error=true)Confirmed the test fails without the fix (
err="missing profile: missing profile") and passes with the fix.Also added a
LoginOTAEnrollSSOUsertest helper that drives the complete OTA SSO flow starting fromGET /enrollthrough SAML IdP login to the callback, using a single cookie jar.Test plan
/enroll?path)Initiator == "setup_experience")Summary by CodeRabbit