diff --git a/ee/server/service/install_vpp_associate_test.go b/ee/server/service/install_vpp_associate_test.go index ecde6e22d2..a9ed976aa6 100644 --- a/ee/server/service/install_vpp_associate_test.go +++ b/ee/server/service/install_vpp_associate_test.go @@ -13,7 +13,6 @@ import ( "github.com/fleetdm/fleet/v4/server/dev_mode" "github.com/fleetdm/fleet/v4/server/fleet" - "github.com/fleetdm/fleet/v4/server/mdm/apple/vpp" "github.com/fleetdm/fleet/v4/server/mock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -250,107 +249,17 @@ func TestInstallVPPAppPostValidation_AssociateAssetsRouting(t *testing.T) { require.Contains(t, bre.InternalErr.Error(), "9622") }) - t.Run("personal enrollment self-heals by recovering Apple-side user", func(t *testing.T) { - // First /assets/associate call returns Apple's 9609 "unable to find - // the registered user" error. Fleet should: - // - call GET /users?managedAppleId=... and find the existing - // (drifted) user, - // - upsert that clientUserId back into vpp_client_users, - // - retry /assets/associate with the recovered UUID and succeed. - // - // Critically, /registerVPPUserSrv should NOT be called a second time - // — Apple already has a user, re-registering would hit 9635. - const recoveredUUID = "recovered-uuid-from-apple" + t.Run("personal enrollment surfaces associate error without retry", func(t *testing.T) { + // An associate error bubbles up directly — Fleet registers the user + // once up front and does not retry or re-register on failure. var ( - associateCalls int - registerCalls int - getUsersCalls int - capturedFirstClientUserID string - capturedSecondClientUser string - ) - setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) { - switch { - case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/assignments"): - _, _ = w.Write([]byte(`{"assignments": []}`)) - case r.Method == http.MethodGet && r.URL.Path == "/users": - getUsersCalls++ - assert.Equal(t, "user@example.com", r.URL.Query().Get("managedAppleId")) - _, _ = fmt.Fprintf(w, `{"users":[{"clientUserId":%q,"idHash":"hash","status":"Associated"}]}`, recoveredUUID) - case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/assets"): - _, _ = fmt.Fprintf(w, `{"assets":[{"adamId":%q,"pricingParam":"STDQ","availableCount":5}]}`, adamID) - case r.Method == http.MethodPost && r.URL.Path == "/registerVPPUserSrv": - registerCalls++ - handleRegisterUserV1(t, w, r) - case r.Method == http.MethodPost && r.URL.Path == "/assets/associate": - associateCalls++ - var got vpp.AssociateAssetsRequest - assert.NoError(t, json.NewDecoder(r.Body).Decode(&got)) - assert.Len(t, got.ClientUserIds, 1) - if len(got.ClientUserIds) == 0 { - t.Errorf("associate request missing ClientUserIds") - return - } - if associateCalls == 1 { - capturedFirstClientUserID = got.ClientUserIds[0] - // Real-world Apple 9609 signature observed in #31138. - w.WriteHeader(http.StatusBadRequest) - _, _ = w.Write([]byte(`{"errorInfo":{},"errorMessage":"Unable to find the registered user.","errorNumber":9609}`)) - return - } - capturedSecondClientUser = got.ClientUserIds[0] - _, _ = w.Write([]byte(`{"eventId":"associate-evt-2"}`)) - default: - t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) - } - }) - - // Override managed-apple-id so the GET /users assertion above can pin it. - ds := setupDS(t, true) - ds.GetHostManagedAppleIDFunc = func(_ context.Context, _ uint) (string, error) { - return "user@example.com", nil - } - var upserts []*fleet.VPPClientUser - ds.InsertVPPClientUserFunc = func(_ context.Context, row *fleet.VPPClientUser) error { - upserts = append(upserts, row) - return nil - } - - svc := &Service{ds: ds, logger: slog.New(slog.DiscardHandler)} - cmdUUID, err := svc.InstallVPPAppPostValidation(context.Background(), host, vppApp, bearerToken, fleet.HostSoftwareInstallOptions{}) - require.NoError(t, err) - require.NotEmpty(t, cmdUUID) - - require.Equal(t, 2, associateCalls, "associate must be retried exactly once") - require.Equal(t, 1, getUsersCalls, "must look up the user by managed apple id before deciding to re-register") - require.Equal(t, 1, registerCalls, "only the initial ensureVPPClientUser register; no second register since Apple still has the user") - - require.NotEqual(t, capturedFirstClientUserID, capturedSecondClientUser, "second associate must use the recovered clientUserId") - require.Equal(t, recoveredUUID, capturedSecondClientUser, "second associate must use the clientUserId returned by Apple's GET /users") - - require.Len(t, upserts, 2, "initial register + cache resync from Apple") - require.Equal(t, recoveredUUID, upserts[1].ClientUserID) - require.Equal(t, fleet.VPPClientUserStatusRegistered, upserts[1].Status) - require.True(t, ds.InsertHostVPPSoftwareInstallFuncInvoked) - }) - - t.Run("personal enrollment falls back to re-register when Apple has no user", func(t *testing.T) { - // Same trigger (9609) but Apple's GET /users returns no active user - // — the prior one was retired or the Apple ID was somehow purged. - // Fleet should fall through to /registerVPPUserSrv, get a fresh UUID, - // upsert it, and retry the associate. - var ( - associateCalls int registerCalls int - getUsersCalls int + associateCalls int ) setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) { switch { case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/assignments"): _, _ = w.Write([]byte(`{"assignments": []}`)) - case r.Method == http.MethodGet && r.URL.Path == "/users": - getUsersCalls++ - // Empty (or Retired-only) response — caller must re-register. - _, _ = w.Write([]byte(`{"users":[{"clientUserId":"retired-ghost","status":"Retired"}]}`)) case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/assets"): _, _ = fmt.Fprintf(w, `{"assets":[{"adamId":%q,"pricingParam":"STDQ","availableCount":5}]}`, adamID) case r.Method == http.MethodPost && r.URL.Path == "/registerVPPUserSrv": @@ -358,44 +267,8 @@ func TestInstallVPPAppPostValidation_AssociateAssetsRouting(t *testing.T) { handleRegisterUserV1(t, w, r) case r.Method == http.MethodPost && r.URL.Path == "/assets/associate": associateCalls++ - if associateCalls == 1 { - w.WriteHeader(http.StatusBadRequest) - _, _ = w.Write([]byte(`{"errorInfo":{},"errorMessage":"Unable to find the registered user.","errorNumber":9609}`)) - return - } - _, _ = w.Write([]byte(`{"eventId":"associate-evt-fallback"}`)) - default: - t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) - } - }) - - ds := setupDS(t, true) - svc := &Service{ds: ds, logger: slog.New(slog.DiscardHandler)} - - _, err := svc.InstallVPPAppPostValidation(context.Background(), host, vppApp, bearerToken, fleet.HostSoftwareInstallOptions{}) - require.NoError(t, err) - - require.Equal(t, 1, getUsersCalls) - require.Equal(t, 2, registerCalls, "initial ensure + fallback re-register") - require.Equal(t, 2, associateCalls, "associate retried after re-register") - }) - - t.Run("personal enrollment does not self-heal on unrelated associate error", func(t *testing.T) { - // Make sure a non-9612 associate error still bubbles up — we don't - // want to mask the real error or churn through pointless re-registers. - var registerCalls int - setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) { - switch { - case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/assignments"): - _, _ = w.Write([]byte(`{"assignments": []}`)) - case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/assets"): - _, _ = fmt.Fprintf(w, `{"assets":[{"adamId":%q,"pricingParam":"STDQ","availableCount":5}]}`, adamID) - case r.Method == http.MethodPost && r.URL.Path == "/registerVPPUserSrv": - registerCalls++ - handleRegisterUserV1(t, w, r) - case r.Method == http.MethodPost && r.URL.Path == "/assets/associate": w.WriteHeader(http.StatusBadRequest) - _, _ = w.Write([]byte(`{"errorInfo":{},"errorMessage":"Cannot establish a connection.","errorNumber":9610}`)) + _, _ = w.Write([]byte(`{"errorInfo":{},"errorMessage":"Unable to find the registered user.","errorNumber":9609}`)) default: t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) } @@ -406,9 +279,9 @@ func TestInstallVPPAppPostValidation_AssociateAssetsRouting(t *testing.T) { _, err := svc.InstallVPPAppPostValidation(context.Background(), host, vppApp, bearerToken, fleet.HostSoftwareInstallOptions{}) require.Error(t, err) - require.Contains(t, err.Error(), "9610") - // Only the initial ensureVPPClientUser register — no self-heal retry. - require.Equal(t, 1, registerCalls) + require.Contains(t, err.Error(), "9609") + require.Equal(t, 1, registerCalls, "only the initial ensureVPPClientUser register; no retry") + require.Equal(t, 1, associateCalls, "associate is attempted exactly once") }) // Pin the dev_mode override in scope until t.Cleanup runs — referenced by the diff --git a/ee/server/service/software_installers.go b/ee/server/service/software_installers.go index 14ce7c89eb..3b405267f2 100644 --- a/ee/server/service/software_installers.go +++ b/ee/server/service/software_installers.go @@ -1593,17 +1593,12 @@ func (svc *Service) InstallVPPAppPostValidation(ctx context.Context, host *fleet isPersonal := hostMDM != nil && hostMDM.IsPersonalEnrollment var clientUserID string - // personalTokenDB stays in scope past the BYOD branch so the self-heal - // path below (on an "unknown clientUserId" associate error) can - // re-register via Apple's v1 endpoint without re-doing the team-token - // lookup. - var personalTokenDB *fleet.VPPTokenDB if isPersonal { // Token-selection policy (per #44009): use the team's default token — // `GetVPPTokenByTeamID` already returns the first token for the team // (existing behavior). Multi-location support is deferred unless a // customer hits the edge case. - personalTokenDB, err = svc.ds.GetVPPTokenByTeamID(ctx, host.TeamID) + personalTokenDB, err := svc.ds.GetVPPTokenByTeamID(ctx, host.TeamID) if err != nil { return "", ctxerr.Wrap(ctx, err, "fetching VPP token DB row for user-enrolled install") } @@ -1692,71 +1687,7 @@ func (svc *Service) InstallVPPAppPostValidation(ctx context.Context, host *fleet } } - // Self-heal a stale local cache: Apple says it doesn't recognize - // the clientUserId we sent. Recovery is two-step because Apple - // enforces one VPP user per (location, managedAppleId): - // - // 1. Ask Apple for its current user record for this Managed - // Apple ID. If one exists, the local cache simply drifted - // (DB restore, manual edit, prior bug); re-sync to Apple's - // clientUserId and retry the associate. - // 2. If Apple has no user (or only Retired entries), - // register a fresh one via the v1 endpoint. - // - // One retry only either way — if the follow-up associate fails, - // surface that error rather than looping. - if isPersonal && personalTokenDB != nil && vpp.IsUnknownClientUserError(err) { - managedAppleID, idErr := svc.ds.GetHostManagedAppleID(ctx, host.ID) - if idErr != nil { - return "", ctxerr.Wrap(ctx, idErr, "looking up managed apple id for vpp recovery") - } - - existing, lookupErr := vpp.GetUserByManagedAppleID(ctx, personalTokenDB.Token, managedAppleID) - if lookupErr != nil { - return "", ctxerr.Wrap(ctx, lookupErr, "looking up vpp user by managed apple id for recovery") - } - - var recoveredClientUserID string - if existing != nil { - // Apple still has the user — local cache drift only. Re-sync. - recoveredClientUserID = existing.ClientUserID - if upsertErr := svc.ds.InsertVPPClientUser(ctx, &fleet.VPPClientUser{ - VPPTokenID: personalTokenDB.ID, - ManagedAppleID: managedAppleID, - ClientUserID: recoveredClientUserID, - Status: fleet.VPPClientUserStatusRegistered, - }); upsertErr != nil { - return "", ctxerr.Wrap(ctx, upsertErr, "resyncing vpp client user cache from Apple") - } - svc.logger.WarnContext(ctx, "recovered vpp client user from Apple; resynced local cache", - "host_id", host.ID, "vpp_token_id", personalTokenDB.ID, "adam_id", vppApp.AdamID, - "recovered_status", string(existing.Status), "original_err", err.Error()) - } else { - // Apple has no user (likely retired) — safe to mint a new one. - newID, regErr := svc.registerVPPClientUser(ctx, personalTokenDB.ID, managedAppleID, personalTokenDB.Token) - if regErr != nil { - return "", ctxerr.Wrap(ctx, regErr, "re-registering vpp client user after lookup returned no active user") - } - recoveredClientUserID = newID - svc.logger.WarnContext(ctx, "no active vpp user at Apple; registered a new one", - "host_id", host.ID, "vpp_token_id", personalTokenDB.ID, "adam_id", vppApp.AdamID, - "original_err", err.Error()) - } - - req.ClientUserIds = []string{recoveredClientUserID} - eventID, err = vpp.AssociateAssets(token, req) - if err != nil { - return "", ctxerr.Wrapf(ctx, err, "associating asset with adamID %s after vpp client user recovery", vppApp.AdamID) - } - // Same as the first-attempt success branch above: remember the - // reservation so the cleanup block below can release the seat - // if InsertHostVPPSoftwareInstall later fails. Without this - // the recovered seat leaks (no host_vpp_software_installs row - // for cancel-path release to find either). - assocReq = req - } else { - return "", ctxerr.Wrapf(ctx, err, "associating asset with adamID %s to host %s", vppApp.AdamID, host.HardwareSerial) - } + return "", ctxerr.Wrapf(ctx, err, "associating asset with adamID %s to host %s", vppApp.AdamID, host.HardwareSerial) } } diff --git a/ee/server/service/vpp_users.go b/ee/server/service/vpp_users.go index 0085db712c..5076b22ec6 100644 --- a/ee/server/service/vpp_users.go +++ b/ee/server/service/vpp_users.go @@ -54,43 +54,13 @@ func (svc *Service) ensureVPPClientUser(ctx context.Context, host *fleet.Host, t return existing.ClientUserID, nil } - // Non-registered row. Apple enforces uniqueness on - // (location, managedAppleId), so blindly calling - // registerVPPClientUser with a fresh UUID will collide with any existing - // Apple-side user. Ask Apple first; if a user already exists, resync the - // local cache to its clientUserId rather than minting a new one. - if existing != nil { - appleUser, lookupErr := vpp.GetUserByManagedAppleID(ctx, token.Token, managedAppleID) - if lookupErr != nil { - return "", ctxerr.Wrapf(ctx, lookupErr, "looking up vpp user by managed apple id for token %d", token.ID) - } - if appleUser != nil { - row := &fleet.VPPClientUser{ - VPPTokenID: token.ID, - ManagedAppleID: managedAppleID, - ClientUserID: appleUser.ClientUserID, - Status: fleet.VPPClientUserStatusRegistered, - } - if err := svc.ds.InsertVPPClientUser(ctx, row); err != nil { - return "", ctxerr.Wrap(ctx, err, "resyncing vpp client user cache from Apple") - } - return appleUser.ClientUserID, nil - } - } - return svc.registerVPPClientUser(ctx, token.ID, managedAppleID, token.Token) } // registerVPPClientUser unconditionally registers a new VPP user via Apple's // synchronous v1 endpoint and upserts the (vpp_token_id, managed_apple_id) // row with the freshly-generated clientUserId, overwriting any prior cache -// entry. Used by: -// -// - ensureVPPClientUser on its first-call / cache-miss branch. -// - The install-flow self-heal path, when Apple rejects the cached -// clientUserId as unknown — bypassing the cache is the whole point of the -// retry, so this entry point exists to avoid a confusing 'force' flag on -// ensureVPPClientUser. +// entry. Called by ensureVPPClientUser on its first-call / cache-miss branch. func (svc *Service) registerVPPClientUser(ctx context.Context, tokenID uint, managedAppleID, token string) (string, error) { clientUserID := uuid.NewString() diff --git a/ee/server/service/vpp_users_test.go b/ee/server/service/vpp_users_test.go index 0b4d5b5019..3e3df7d285 100644 --- a/ee/server/service/vpp_users_test.go +++ b/ee/server/service/vpp_users_test.go @@ -125,56 +125,9 @@ func TestEnsureVPPClientUser_ExistingRegisteredUser(t *testing.T) { } // A non-registered cache row (typically 'pending' from the legacy v2 async -// flow) must NOT be treated as a fresh registration target: Apple enforces -// uniqueness on (location, managedAppleId), so registering with a new UUID -// would collide. Instead, look the user up on Apple's side and resync. -func TestEnsureVPPClientUser_PendingRowAppleHasUser(t *testing.T) { - const ( - managedAppleID = "user@example.com" - appleClientID = "apple-side-uuid" - ) - tokenDB := &fleet.VPPTokenDB{ID: 1, Token: "tok"} - host := &fleet.Host{ID: 1} - - setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodGet && r.URL.Path == "/users" { - assert.Equal(t, managedAppleID, r.URL.Query().Get("managedAppleId")) - _, _ = fmt.Fprintf(w, `{"users":[{"clientUserId":%q,"status":"Registered"}]}`, appleClientID) - return - } - t.Fatalf("unexpected Apple call %s %s — pending row + Apple-side user should resync without re-registering", r.Method, r.URL.Path) - }) - - ds := new(mock.Store) - ds.GetHostManagedAppleIDFunc = func(_ context.Context, _ uint) (string, error) { - return managedAppleID, nil - } - ds.GetVPPClientUserFunc = func(_ context.Context, _ uint, _ string) (*fleet.VPPClientUser, error) { - return &fleet.VPPClientUser{ - VPPTokenID: tokenDB.ID, - ManagedAppleID: managedAppleID, - ClientUserID: "stale-pending-uuid", - Status: fleet.VPPClientUserStatusPending, - }, nil - } - var insertedRow *fleet.VPPClientUser - ds.InsertVPPClientUserFunc = func(_ context.Context, row *fleet.VPPClientUser) error { - insertedRow = row - return nil - } - - svc := newTestServiceWithDS(ds) - clientUserID, err := svc.ensureVPPClientUser(context.Background(), host, tokenDB) - require.NoError(t, err) - require.Equal(t, appleClientID, clientUserID) - require.NotNil(t, insertedRow) - require.Equal(t, appleClientID, insertedRow.ClientUserID) - require.Equal(t, fleet.VPPClientUserStatusRegistered, insertedRow.Status) -} - -// Pending row + Apple has no user (only retired entries, or fully cleared) — -// safe to mint a fresh registration via the v1 endpoint. -func TestEnsureVPPClientUser_PendingRowAppleHasNoUser(t *testing.T) { +// flow) is not a usable clientUserId, so ensureVPPClientUser registers a fresh +// user via the v1 endpoint rather than returning the stale row. +func TestEnsureVPPClientUser_PendingRowReregisters(t *testing.T) { const managedAppleID = "user@example.com" tokenDB := &fleet.VPPTokenDB{ID: 1, Token: "tok"} host := &fleet.Host{ID: 1} @@ -182,8 +135,6 @@ func TestEnsureVPPClientUser_PendingRowAppleHasNoUser(t *testing.T) { var registerCalls int setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) { switch { - case r.Method == http.MethodGet && r.URL.Path == "/users": - _, _ = fmt.Fprint(w, `{"users":[]}`) case r.Method == http.MethodPost && r.URL.Path == "/registerVPPUserSrv": registerCalls++ var got struct { diff --git a/server/mdm/apple/vpp/api.go b/server/mdm/apple/vpp/api.go index 14695cec17..f5a1775b14 100644 --- a/server/mdm/apple/vpp/api.go +++ b/server/mdm/apple/vpp/api.go @@ -75,49 +75,6 @@ func IsMaxDevicesPerUserError(err error) bool { return false } -// IsUnknownClientUserError reports whether err indicates that Apple does not -// recognize the clientUserId(s) Fleet sent on an associate-assets or -// assignment query — typically because the user record was retired or never -// completed registration on Apple's side, while Fleet still has it cached as -// 'registered'. -// -// Used by the install flow to self-heal: on this error the caller should -// re-register the VPP user via the v1 endpoint, replace the stale row, and -// retry the original associate-assets call once. -// -// Confirmed code from production traffic: -// - 9609 / "Unable to find the registered user." -// -// Other codes (9605, 9612, 9627) are listed defensively against Apple's -// docs; same substring backstop as IsMaxDevicesPerUserError catches future -// drift. -func IsUnknownClientUserError(err error) bool { - if err == nil { - return false - } - var resp *ErrorResponse - if !errors.As(err, &resp) || resp == nil { - return false - } - switch resp.ErrorNumber { - case 9605, 9609, 9612, 9627: - return true - } - msg := strings.ToLower(resp.ErrorMessage) - if strings.Contains(msg, "unable to find") && - (strings.Contains(msg, "registered user") || strings.Contains(msg, "client user") || strings.Contains(msg, "user")) { - return true - } - if strings.Contains(msg, "client user") && - (strings.Contains(msg, "not found") || strings.Contains(msg, "unknown")) { - return true - } - if strings.Contains(msg, "user not found") { - return true - } - return false -} - // ResponseErrorInfo represents the request-specific information regarding the // failure. // @@ -344,15 +301,18 @@ func RegisterUser(token, clientUserID, managedAppleID string) (string, error) { } // v1 takes the VPP server token in the body rather than the - // Authorization header. + // Authorization header. Apple keys the user record on email, so we send + // the Managed Apple ID as both managedAppleIDStr and email. reqParams := struct { SToken string `json:"sToken"` ClientUserIDStr string `json:"clientUserIdStr"` ManagedAppleIDStr string `json:"managedAppleIDStr"` + Email string `json:"email"` }{ SToken: token, ClientUserIDStr: clientUserID, ManagedAppleIDStr: managedAppleID, + Email: managedAppleID, } var reqBody bytes.Buffer @@ -387,73 +347,6 @@ func RegisterUser(token, clientUserID, managedAppleID string) (string, error) { return resp.User.UserID.String(), nil } -// VPPUserStatus mirrors the lifecycle states Apple reports in v2 /users -// responses for a VPP user. -type VPPUserStatus string - -const ( - // VPPUserStatusRegistered: Apple has accepted the registration and - // issued an invite, but the end user has not yet linked their Apple - // Account. - VPPUserStatusRegistered VPPUserStatus = "Registered" - // VPPUserStatusAssociated: end user has accepted the invite and the - // Apple Account is bound to the VPP user record. - VPPUserStatusAssociated VPPUserStatus = "Associated" - // VPPUserStatusRetired: the user has been retired; a new registration - // for the same Managed Apple ID is permitted at this location. - VPPUserStatusRetired VPPUserStatus = "Retired" -) - -// User is a single entry from Apple's v2 /users list response. -type User struct { - ClientUserID string `json:"clientUserId"` - IDHash string `json:"idHash"` - Status VPPUserStatus `json:"status"` -} - -// GetUserByManagedAppleID looks up the active VPP user for the given Managed -// Apple ID at the location identified by the bearer token. Apple enforces -// uniqueness on (location, managedAppleId), so a successful response carries -// at most one non-retired user. -// -// Returns (nil, nil) when Apple has no user (or only retired users) for the -// Apple ID — callers should fall through to RegisterUser in that case. -// Returns a non-nil error only for transport / Apple-application errors. -// -// Used by the install self-heal path to recover a stale clientUserId after -// Fleet's local cache drifts from Apple's record (e.g. a stale DB restore). -// -// https://developer.apple.com/documentation/devicemanagement/get-users -func GetUserByManagedAppleID(ctx context.Context, token, managedAppleID string) (*User, error) { - if managedAppleID == "" { - return nil, errors.New("GetUserByManagedAppleID: managedAppleID is required") - } - - q := url.Values{} - q.Set("managedAppleId", managedAppleID) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, getBaseURL()+"/users?"+q.Encode(), nil) - if err != nil { - return nil, fmt.Errorf("creating request to Apple VPP endpoint: %w", err) - } - - var resp struct { - Users []User `json:"users"` - } - if err := do(req, token, &resp); err != nil { - return nil, fmt.Errorf("making request to Apple VPP endpoint: %w", err) - } - - // Apple's contract is at-most-one non-retired user per (location, Apple ID), - // but we scan the full slice defensively in case a Retired ghost is - // returned alongside an active record on some iOS revision. - for i := range resp.Users { - if resp.Users[i].Status != VPPUserStatusRetired { - return &resp.Users[i], nil - } - } - return nil, nil -} - // AssetFilter represents the filters for querying assets. type AssetFilter struct { // PageIndex is the requested page index. diff --git a/server/mdm/apple/vpp/api_test.go b/server/mdm/apple/vpp/api_test.go index d6b69ac47f..d5f17563ec 100644 --- a/server/mdm/apple/vpp/api_test.go +++ b/server/mdm/apple/vpp/api_test.go @@ -618,11 +618,14 @@ func TestRegisterUser(t *testing.T) { SToken string `json:"sToken"` ClientUserIDStr string `json:"clientUserIdStr"` ManagedAppleIDStr string `json:"managedAppleIDStr"` + Email string `json:"email"` } assert.NoError(t, json.Unmarshal(body, &got)) assert.Equal(t, "valid_token", got.SToken) assert.Equal(t, "uuid-1", got.ClientUserIDStr) assert.Equal(t, "user1@example.com", got.ManagedAppleIDStr) + // Apple keys on email — Fleet sends the Managed Apple ID for both. + assert.Equal(t, "user1@example.com", got.Email) _, _ = w.Write([]byte(`{ "status": 0, @@ -681,75 +684,6 @@ func TestRegisterUser(t *testing.T) { }) } -func TestGetUserByManagedAppleID(t *testing.T) { - t.Run("rejects empty managed apple id", func(t *testing.T) { - _, err := GetUserByManagedAppleID(t.Context(), "tok", "") - require.Error(t, err) - }) - - t.Run("returns the single non-retired user", func(t *testing.T) { - setupFakeServer(t, func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, http.MethodGet, r.Method) - assert.Equal(t, "/users", r.URL.Path) - assert.Equal(t, "user@example.com", r.URL.Query().Get("managedAppleId")) - assert.Equal(t, "Bearer tok", r.Header.Get("Authorization")) - _, _ = w.Write([]byte(`{"users":[{"clientUserId":"uuid-1","idHash":"hash-1","status":"Associated"}]}`)) - }) - - got, err := GetUserByManagedAppleID(t.Context(), "tok", "user@example.com") - require.NoError(t, err) - require.NotNil(t, got) - require.Equal(t, "uuid-1", got.ClientUserID) - require.Equal(t, VPPUserStatusAssociated, got.Status) - }) - - t.Run("returns nil when Apple has no users", func(t *testing.T) { - setupFakeServer(t, func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write([]byte(`{"users":[]}`)) - }) - - got, err := GetUserByManagedAppleID(t.Context(), "tok", "missing@example.com") - require.NoError(t, err) - require.Nil(t, got) - }) - - t.Run("skips Retired entries", func(t *testing.T) { - setupFakeServer(t, func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write([]byte(`{"users":[{"clientUserId":"old","status":"Retired"}]}`)) - }) - - got, err := GetUserByManagedAppleID(t.Context(), "tok", "user@example.com") - require.NoError(t, err) - require.Nil(t, got, "Retired-only response must be treated as 'no user' so caller re-registers") - }) - - t.Run("Registered user is recoverable too", func(t *testing.T) { - // A user who's been invited but hasn't accepted yet (status=Registered) - // is still a valid record — Fleet should sync to that clientUserId - // rather than try to mint a duplicate. - setupFakeServer(t, func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write([]byte(`{"users":[{"clientUserId":"pending-uuid","status":"Registered"}]}`)) - }) - - got, err := GetUserByManagedAppleID(t.Context(), "tok", "user@example.com") - require.NoError(t, err) - require.NotNil(t, got) - require.Equal(t, "pending-uuid", got.ClientUserID) - require.Equal(t, VPPUserStatusRegistered, got.Status) - }) - - t.Run("surfaces Apple application error", func(t *testing.T) { - setupFakeServer(t, func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusBadRequest) - _, _ = w.Write([]byte(`{"errorMessage":"Bad Request","errorNumber":400}`)) - }) - - _, err := GetUserByManagedAppleID(t.Context(), "tok", "user@example.com") - require.Error(t, err) - require.Contains(t, err.Error(), "error number: 400") - }) -} - func TestIsMaxDevicesPerUserError(t *testing.T) { cases := []struct { name string @@ -799,88 +733,6 @@ func TestIsMaxDevicesPerUserError(t *testing.T) { } } -func TestIsUnknownClientUserError(t *testing.T) { - cases := []struct { - name string - err error - expected bool - }{ - { - name: "nil error", - err: nil, - expected: false, - }, - { - name: "non-VPP error", - err: errors.New("network down"), - expected: false, - }, - { - name: "candidate code 9605", - err: &ErrorResponse{ErrorMessage: "User not found", ErrorNumber: 9605}, - expected: true, - }, - { - // Production-observed signature from #31138 retry test on - // 2026-05-22 — pinning the exact code/message so a regression - // here flips the self-heal off and we'd notice in CI. - name: "confirmed production signature 9609 / Unable to find the registered user.", - err: &ErrorResponse{ErrorMessage: "Unable to find the registered user.", ErrorNumber: 9609}, - expected: true, - }, - { - name: "9609 substring fallback when code drifts", - err: &ErrorResponse{ErrorMessage: "Unable to find the registered user for this license.", ErrorNumber: 0}, - expected: true, - }, - { - name: "candidate code 9612", - err: &ErrorResponse{ErrorMessage: "Client user not found", ErrorNumber: 9612}, - expected: true, - }, - { - name: "candidate code 9627", - err: &ErrorResponse{ErrorMessage: "Unknown user id", ErrorNumber: 9627}, - expected: true, - }, - { - name: "matched by case-insensitive 'client user not found' message", - err: &ErrorResponse{ErrorMessage: "Client User Not Found in organization.", ErrorNumber: 99999}, - expected: true, - }, - { - name: "matched by 'client user' + 'unknown' phrasing", - err: &ErrorResponse{ErrorMessage: "Unknown client user supplied.", ErrorNumber: 0}, - expected: true, - }, - { - name: "matched by 'user not found' phrasing", - err: &ErrorResponse{ErrorMessage: "User not found.", ErrorNumber: 0}, - expected: true, - }, - { - name: "unrelated VPP error 9610", - err: &ErrorResponse{ErrorMessage: "Cannot establish a connection.", ErrorNumber: 9610}, - expected: false, - }, - { - name: "max-devices error must NOT match unknown-user", - err: &ErrorResponse{ErrorMessage: "User has reached the maximum number of devices.", ErrorNumber: 9622}, - expected: false, - }, - { - name: "wrapped via fmt.Errorf %w still detected", - err: fmt.Errorf("calling vpp: %w", &ErrorResponse{ErrorMessage: "Client user not found", ErrorNumber: 9612}), - expected: true, - }, - } - for _, tt := range cases { - t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.expected, IsUnknownClientUserError(tt.err)) - }) - } -} - func TestGetBaseURL(t *testing.T) { t.Run("Default URL", func(t *testing.T) { require.Equal(t, "https://vpp.itunes.apple.com/mdm/v2", getBaseURL())