Remove stale users fix and associated tests (#46382)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves # Unreleased bugfix in https://github.com/fleetdm/fleet/issues/31138 We are setting the email on users Fleet creates via the API. We decided to remove the existing logic we were using to try and link VPP Users back to Fleet users if they get removed from the DB but by setting the email we can follow up(later) with a tool that can query the Apple APIs and list all users by their emails and we can insert them into the VPP users table # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops - [x] If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * VPP app installation failures now report immediately without automatic retry or recovery attempts * Improved error transparency for Apple app provisioning failures * **Refactor** * Simplified VPP user management and error handling logic * Removed redundant user lookup and retry mechanisms from app distribution workflows <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/fleetdm/fleet/pull/46382?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
+4
-111
@@ -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.
|
||||
|
||||
@@ -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())
|
||||
|
||||
Reference in New Issue
Block a user