Fixing unreleased issue with vpp installs on byod (#46108)

This commit is contained in:
George Karr
2026-05-27 12:41:50 -05:00
committed by GitHub
parent 1cf6cce5b2
commit f592d6a072
13 changed files with 927 additions and 298 deletions
+188 -42
View File
@@ -13,6 +13,7 @@ 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"
@@ -36,29 +37,37 @@ func TestInstallVPPAppPostValidation_AssociateAssetsRouting(t *testing.T) {
body []byte
}
// handleRegisterUserV1 emulates Apple's synchronous v1 registerVPPUserSrv
// endpoint — the token is in the request body, not the Authorization header.
handleRegisterUserV1 := func(t *testing.T, w http.ResponseWriter, r *http.Request) {
t.Helper()
var body struct {
SToken string `json:"sToken"`
ClientUserIDStr string `json:"clientUserIdStr"`
ManagedAppleIDStr string `json:"managedAppleIDStr"`
}
require.NoError(t, json.NewDecoder(r.Body).Decode(&body))
assert.Equal(t, bearerToken, body.SToken)
_, _ = fmt.Fprintf(w, `{"status":0,"user":{"userId":12345,"status":"Registered","clientUserIdStr":%q,"managedAppleIDStr":%q}}`,
body.ClientUserIDStr, body.ManagedAppleIDStr)
}
// Common mock-server setup. Captures the AssociateAssets body so the test
// can assert on the wire payload.
setupServer := func(t *testing.T, capt *captured) {
t.Helper()
setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "Bearer "+bearerToken, r.Header.Get("Authorization"))
switch {
case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/assignments"):
assert.Equal(t, "Bearer "+bearerToken, r.Header.Get("Authorization"))
_, _ = w.Write([]byte(`{"assignments": []}`))
case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/assets"):
assert.Equal(t, "Bearer "+bearerToken, r.Header.Get("Authorization"))
_, _ = fmt.Fprintf(w, `{"assets":[{"adamId":%q,"pricingParam":"STDQ","availableCount":5}]}`, adamID)
case r.Method == http.MethodPost && r.URL.Path == "/users/create":
body := struct {
Users []struct {
ClientUserId string `json:"clientUserId"`
ManagedAppleId string `json:"managedAppleId"`
} `json:"users"`
}{}
assert.NoError(t, json.NewDecoder(r.Body).Decode(&body))
assert.Len(t, body.Users, 1)
_, _ = fmt.Fprintf(w, `{"eventId":"evt","users":[{"userId":"apple-1","clientUserId":%q,"managedAppleId":%q,"status":"Registered"}]}`,
body.Users[0].ClientUserId, body.Users[0].ManagedAppleId)
case r.Method == http.MethodPost && r.URL.Path == "/registerVPPUserSrv":
handleRegisterUserV1(t, w, r)
case r.Method == http.MethodPost && r.URL.Path == "/assets/associate":
assert.Equal(t, "Bearer "+bearerToken, r.Header.Get("Authorization"))
b, err := io.ReadAll(r.Body)
assert.NoError(t, err)
capt.body = b
@@ -157,16 +166,8 @@ func TestInstallVPPAppPostValidation_AssociateAssetsRouting(t *testing.T) {
_, _ = 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 == "/users/create":
body := struct {
Users []struct {
ClientUserId string `json:"clientUserId"`
ManagedAppleId string `json:"managedAppleId"`
} `json:"users"`
}{}
assert.NoError(t, json.NewDecoder(r.Body).Decode(&body))
_, _ = fmt.Fprintf(w, `{"eventId":"evt","users":[{"userId":"apple-1","clientUserId":%q,"managedAppleId":%q,"status":"Registered"}]}`,
body.Users[0].ClientUserId, body.Users[0].ManagedAppleId)
case r.Method == http.MethodPost && r.URL.Path == "/registerVPPUserSrv":
handleRegisterUserV1(t, w, r)
case r.Method == http.MethodPost && r.URL.Path == "/assets/associate":
_, _ = w.Write([]byte(`{"eventId":"associate-evt"}`))
default:
@@ -193,16 +194,8 @@ func TestInstallVPPAppPostValidation_AssociateAssetsRouting(t *testing.T) {
_, _ = fmt.Fprintf(w, `{"assignments":[{"adamId":%q,"pricingParam":"STDQ"}]}`, adamID)
case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/assets"):
t.Errorf("/assets must not be queried when assignments already exist")
case r.Method == http.MethodPost && r.URL.Path == "/users/create":
body := struct {
Users []struct {
ClientUserId string `json:"clientUserId"`
ManagedAppleId string `json:"managedAppleId"`
} `json:"users"`
}{}
assert.NoError(t, json.NewDecoder(r.Body).Decode(&body))
_, _ = fmt.Fprintf(w, `{"eventId":"evt","users":[{"userId":"apple-1","clientUserId":%q,"managedAppleId":%q,"status":"Registered"}]}`,
body.Users[0].ClientUserId, body.Users[0].ManagedAppleId)
case r.Method == http.MethodPost && r.URL.Path == "/registerVPPUserSrv":
handleRegisterUserV1(t, w, r)
case r.Method == http.MethodPost && r.URL.Path == "/assets/associate":
associateCalls++
_, _ = w.Write([]byte(`{"eventId":"associate-evt"}`))
@@ -228,16 +221,8 @@ func TestInstallVPPAppPostValidation_AssociateAssetsRouting(t *testing.T) {
_, _ = 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 == "/users/create":
body := struct {
Users []struct {
ClientUserId string `json:"clientUserId"`
ManagedAppleId string `json:"managedAppleId"`
} `json:"users"`
}{}
assert.NoError(t, json.NewDecoder(r.Body).Decode(&body))
_, _ = fmt.Fprintf(w, `{"eventId":"evt","users":[{"userId":"apple-1","clientUserId":%q,"managedAppleId":%q,"status":"Registered"}]}`,
body.Users[0].ClientUserId, body.Users[0].ManagedAppleId)
case r.Method == http.MethodPost && r.URL.Path == "/registerVPPUserSrv":
handleRegisterUserV1(t, w, r)
case r.Method == http.MethodPost && r.URL.Path == "/assets/associate":
w.WriteHeader(http.StatusBadRequest)
_, _ = w.Write([]byte(`{"errorInfo":{},"errorMessage":"User has reached the maximum number of devices for this license.","errorNumber":9622}`))
@@ -260,6 +245,167 @@ 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"
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
)
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":
registerCalls++
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}`))
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.Error(t, err)
require.Contains(t, err.Error(), "9610")
// Only the initial ensureVPPClientUser register — no self-heal retry.
require.Equal(t, 1, registerCalls)
})
// Pin the dev_mode override in scope until t.Cleanup runs — referenced by the
// helper above, which already registers Cleanup, but make sure the variable is
// not flagged as unused.
+67 -3
View File
@@ -1474,16 +1474,21 @@ 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.
tokenDB, 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")
}
clientUserID, err = svc.ensureVPPClientUser(ctx, host, tokenDB)
clientUserID, err = svc.ensureVPPClientUser(ctx, host, personalTokenDB)
if err != nil {
return "", ctxerr.Wrap(ctx, err, "ensure VPP client user")
}
@@ -1558,7 +1563,66 @@ func (svc *Service) InstallVPPAppPostValidation(ctx context.Context, host *fleet
InternalErr: ctxerr.WrapWithData(ctx, err, "associate asset rejected by Apple per-user device cap", map[string]any{"host_id": host.ID, "team_id": host.TeamID, "adam_id": vppApp.AdamID}),
}
}
return "", ctxerr.Wrapf(ctx, err, "associating asset with adamID %s to host %s", vppApp.AdamID, host.HardwareSerial)
// 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)
}
} else {
return "", ctxerr.Wrapf(ctx, err, "associating asset with adamID %s to host %s", vppApp.AdamID, host.HardwareSerial)
}
}
}
+48 -60
View File
@@ -22,11 +22,12 @@ var errMissingManagedAppleID = fleet.NewUserMessageError(
// ensureVPPClientUser returns the Fleet-generated clientUserId for the host's
// Managed Apple ID at the given VPP token (location), creating the Apple-side
// VPP user via POST /mdm/v2/users/create on first call. Idempotent: subsequent
// calls return the cached clientUserId from vpp_client_users.
// VPP user via Apple's synchronous v1 registerVPPUserSrv endpoint on first
// call. Idempotent: subsequent calls return the cached clientUserId from
// vpp_client_users.
//
// Used by the user-scoped Associate Assets path (subtask 06) for hosts enrolled
// via Account-Driven User Enrollment (BYOD).
// Used by the user-scoped Associate Assets path for hosts enrolled via
// Account-Driven User Enrollment (BYOD).
func (svc *Service) ensureVPPClientUser(ctx context.Context, host *fleet.Host, token *fleet.VPPTokenDB) (string, error) {
if host == nil {
return "", ctxerr.New(ctx, "ensureVPPClientUser: nil host")
@@ -53,74 +54,61 @@ func (svc *Service) ensureVPPClientUser(ctx context.Context, host *fleet.Host, t
return existing.ClientUserID, nil
}
// Either no row, or a prior attempt left it 'pending'. Reuse the
// previously-generated UUID on retry so Apple correlates the request with
// the same user record.
clientUserID := uuid.NewString()
if existing != nil && existing.ClientUserID != "" {
clientUserID = existing.ClientUserID
}
resp, err := vpp.CreateUsers(token.Token, &vpp.CreateUsersRequest{
Users: []vpp.CreateUsersUser{{ClientUserId: clientUserID, ManagedAppleId: managedAppleID}},
})
if err != nil {
// Persist 'pending' so a future retry can reuse the same clientUserID
// rather than minting a fresh one (which would leave us with multiple
// Apple-side users for the same Managed Apple ID). Log if the persist
// itself fails — we still want to surface the original CreateUsers
// error to the caller.
if insertErr := svc.ds.InsertVPPClientUser(ctx, &fleet.VPPClientUser{
VPPTokenID: token.ID,
ManagedAppleID: managedAppleID,
ClientUserID: clientUserID,
Status: fleet.VPPClientUserStatusPending,
}); insertErr != nil {
svc.logger.ErrorContext(ctx, "persisting pending vpp client user after CreateUsers failure",
"host_id", host.ID, "vpp_token_id", token.ID, "err", insertErr)
// 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)
}
return "", ctxerr.Wrap(ctx, err, "calling Apple VPP create-users")
}
// Apple's /users/create is asynchronous in the v2 API: a 200 with eventId
// means user registration has been queued, and the per-user payload is
// returned later (separate /users/get poll, deferred to a follow-up
// subtask). If Apple did echo a per-user entry in the synchronous response,
// surface any per-user error; otherwise treat the eventId as success since
// downstream associate-assets uses our clientUserId, which Apple resolves
// once registration completes.
for i := range resp.Users {
u := &resp.Users[i]
if u.ClientUserId != clientUserID {
continue
}
if u.HasError() {
if insertErr := svc.ds.InsertVPPClientUser(ctx, &fleet.VPPClientUser{
if appleUser != nil {
row := &fleet.VPPClientUser{
VPPTokenID: token.ID,
ManagedAppleID: managedAppleID,
ClientUserID: clientUserID,
Status: fleet.VPPClientUserStatusPending,
}); insertErr != nil {
svc.logger.ErrorContext(ctx, "persisting pending vpp client user after Apple per-user error",
"host_id", host.ID, "vpp_token_id", token.ID, "err", insertErr)
ClientUserID: appleUser.ClientUserID,
Status: fleet.VPPClientUserStatusRegistered,
}
return "", ctxerr.Errorf(ctx, "Apple VPP create-users returned error for managed apple id %q: %s (code %d)",
managedAppleID, u.ErrorMessage, u.ErrorNumber)
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.
func (svc *Service) registerVPPClientUser(ctx context.Context, tokenID uint, managedAppleID, token string) (string, error) {
clientUserID := uuid.NewString()
// v1 registerVPPUserSrv is synchronous — a successful response means the
// user is registered and ready to receive license associations.
appleUserID, err := vpp.RegisterUser(token, clientUserID, managedAppleID)
if err != nil {
return "", ctxerr.Wrapf(ctx, err, "registering vpp user for managed apple id %q", managedAppleID)
}
row := &fleet.VPPClientUser{
VPPTokenID: token.ID,
VPPTokenID: tokenID,
ManagedAppleID: managedAppleID,
ClientUserID: clientUserID,
Status: fleet.VPPClientUserStatusRegistered,
}
for i := range resp.Users {
if resp.Users[i].ClientUserId == clientUserID && resp.Users[i].UserId != "" {
appleUserID := resp.Users[i].UserId
row.AppleUserID = &appleUserID
break
}
if appleUserID != "" {
row.AppleUserID = &appleUserID
}
if err := svc.ds.InsertVPPClientUser(ctx, row); err != nil {
return "", ctxerr.Wrap(ctx, err, "persisting registered vpp client user")
+101 -53
View File
@@ -34,28 +34,33 @@ func TestEnsureVPPClientUser_NewUser(t *testing.T) {
tokenDB := &fleet.VPPTokenDB{ID: 42, Token: "valid-token"}
host := &fleet.Host{ID: hostID}
var createCalls int
var registerCalls int
setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) {
createCalls++
registerCalls++
assert.Equal(t, http.MethodPost, r.Method)
assert.Equal(t, "/users/create", r.URL.Path)
assert.Equal(t, "Bearer valid-token", r.Header.Get("Authorization"))
assert.Equal(t, "/registerVPPUserSrv", r.URL.Path)
// v1 puts the token in the body, not the Authorization header.
assert.Empty(t, r.Header.Get("Authorization"))
var got struct {
Users []struct {
ClientUserId string `json:"clientUserId"`
ManagedAppleId string `json:"managedAppleId"`
} `json:"users"`
SToken string `json:"sToken"`
ClientUserIDStr string `json:"clientUserIdStr"`
ManagedAppleIDStr string `json:"managedAppleIDStr"`
}
assert.NoError(t, json.NewDecoder(r.Body).Decode(&got))
assert.Len(t, got.Users, 1)
assert.NotEmpty(t, got.Users[0].ClientUserId)
assert.Equal(t, managedAppleID, got.Users[0].ManagedAppleId)
assert.Equal(t, "valid-token", got.SToken)
assert.NotEmpty(t, got.ClientUserIDStr)
assert.Equal(t, managedAppleID, got.ManagedAppleIDStr)
_, _ = fmt.Fprintf(w, `{
"eventId": "evt-1",
"users": [{"userId":"apple-user-1","clientUserId":%q,"managedAppleId":%q,"status":"Registered"}]
}`, got.Users[0].ClientUserId, managedAppleID)
"status": 0,
"user": {
"userId": 98765,
"status": "Registered",
"clientUserIdStr": %q,
"managedAppleIDStr": %q
}
}`, got.ClientUserIDStr, managedAppleID)
})
ds := new(mock.Store)
@@ -78,7 +83,7 @@ func TestEnsureVPPClientUser_NewUser(t *testing.T) {
clientUserID, err := svc.ensureVPPClientUser(context.Background(), host, tokenDB)
require.NoError(t, err)
require.NotEmpty(t, clientUserID)
require.Equal(t, 1, createCalls)
require.Equal(t, 1, registerCalls)
require.NotNil(t, insertedRow)
require.Equal(t, tokenDB.ID, insertedRow.VPPTokenID)
@@ -86,7 +91,7 @@ func TestEnsureVPPClientUser_NewUser(t *testing.T) {
require.Equal(t, clientUserID, insertedRow.ClientUserID)
require.Equal(t, fleet.VPPClientUserStatusRegistered, insertedRow.Status)
require.NotNil(t, insertedRow.AppleUserID)
require.Equal(t, "apple-user-1", *insertedRow.AppleUserID)
require.Equal(t, "98765", *insertedRow.AppleUserID)
}
func TestEnsureVPPClientUser_ExistingRegisteredUser(t *testing.T) {
@@ -96,7 +101,7 @@ func TestEnsureVPPClientUser_ExistingRegisteredUser(t *testing.T) {
// Apple must NOT be called on cache hit.
setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) {
t.Fatalf("Apple VPP create-users must not be called when a registered row exists; got %s %s", r.Method, r.URL.Path)
t.Fatalf("Apple VPP register-user must not be called when a registered row exists; got %s %s", r.Method, r.URL.Path)
})
ds := new(mock.Store)
@@ -119,28 +124,25 @@ func TestEnsureVPPClientUser_ExistingRegisteredUser(t *testing.T) {
require.False(t, ds.InsertVPPClientUserFuncInvoked)
}
func TestEnsureVPPClientUser_PendingRetryReusesUUID(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"
priorUUID = "prior-uuid-1234"
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) {
var got struct {
Users []struct {
ClientUserId string `json:"clientUserId"`
ManagedAppleId string `json:"managedAppleId"`
} `json:"users"`
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
}
assert.NoError(t, json.NewDecoder(r.Body).Decode(&got))
assert.Equal(t, priorUUID, got.Users[0].ClientUserId, "retry must reuse the prior clientUserId")
_, _ = fmt.Fprintf(w, `{
"eventId": "evt",
"users": [{"userId":"apple-2","clientUserId":%q,"managedAppleId":%q,"status":"Registered"}]
}`, priorUUID, managedAppleID)
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)
@@ -151,40 +153,88 @@ func TestEnsureVPPClientUser_PendingRetryReusesUUID(t *testing.T) {
return &fleet.VPPClientUser{
VPPTokenID: tokenDB.ID,
ManagedAppleID: managedAppleID,
ClientUserID: priorUUID,
ClientUserID: "stale-pending-uuid",
Status: fleet.VPPClientUserStatusPending,
}, nil
}
var lastInserted *fleet.VPPClientUser
var insertedRow *fleet.VPPClientUser
ds.InsertVPPClientUserFunc = func(_ context.Context, row *fleet.VPPClientUser) error {
lastInserted = row
insertedRow = row
return nil
}
svc := newTestServiceWithDS(ds)
got, err := svc.ensureVPPClientUser(context.Background(), host, tokenDB)
clientUserID, err := svc.ensureVPPClientUser(context.Background(), host, tokenDB)
require.NoError(t, err)
require.Equal(t, priorUUID, got)
require.NotNil(t, lastInserted)
require.Equal(t, fleet.VPPClientUserStatusRegistered, lastInserted.Status)
require.Equal(t, appleClientID, clientUserID)
require.NotNil(t, insertedRow)
require.Equal(t, appleClientID, insertedRow.ClientUserID)
require.Equal(t, fleet.VPPClientUserStatusRegistered, insertedRow.Status)
}
func TestEnsureVPPClientUser_PartialFailureKeepsPending(t *testing.T) {
// 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) {
const managedAppleID = "user@example.com"
tokenDB := &fleet.VPPTokenDB{ID: 1, Token: "tok"}
host := &fleet.Host{ID: 1}
var registerCalls int
setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) {
var got struct {
Users []struct {
ClientUserId string `json:"clientUserId"`
} `json:"users"`
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 {
ClientUserIDStr string `json:"clientUserIdStr"`
ManagedAppleIDStr string `json:"managedAppleIDStr"`
}
assert.NoError(t, json.NewDecoder(r.Body).Decode(&got))
_, _ = fmt.Fprintf(w, `{"status":0,"user":{"userId":1234,"status":"Registered","clientUserIdStr":%q,"managedAppleIDStr":%q}}`,
got.ClientUserIDStr, managedAppleID)
default:
t.Fatalf("unexpected Apple call %s %s", r.Method, r.URL.Path)
}
assert.NoError(t, json.NewDecoder(r.Body).Decode(&got))
_, _ = fmt.Fprintf(w, `{
"eventId": "evt",
"users": [{"clientUserId":%q,"managedAppleId":%q,"errorMessage":"Managed Apple ID not found","errorNumber":9637}]
}`, got.Users[0].ClientUserId, managedAppleID)
})
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
}
ds.InsertVPPClientUserFunc = func(_ context.Context, _ *fleet.VPPClientUser) error {
return nil
}
svc := newTestServiceWithDS(ds)
clientUserID, err := svc.ensureVPPClientUser(context.Background(), host, tokenDB)
require.NoError(t, err)
require.NotEmpty(t, clientUserID)
require.NotEqual(t, "stale-pending-uuid", clientUserID)
require.Equal(t, 1, registerCalls)
}
func TestEnsureVPPClientUser_AppleErrorSurfacesAndSkipsInsert(t *testing.T) {
const managedAppleID = "missing@example.com"
tokenDB := &fleet.VPPTokenDB{ID: 1, Token: "tok"}
host := &fleet.Host{ID: 1}
// v1 reports application-level errors synchronously — no Apple-side user
// exists, so we should surface the error and skip the DB write entirely.
setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) {
_, _ = fmt.Fprint(w, `{
"status": -1,
"errorNumber": 9637,
"errorMessage": "Managed Apple ID not found"
}`)
})
ds := new(mock.Store)
@@ -194,9 +244,8 @@ func TestEnsureVPPClientUser_PartialFailureKeepsPending(t *testing.T) {
ds.GetVPPClientUserFunc = func(_ context.Context, _ uint, _ string) (*fleet.VPPClientUser, error) {
return nil, &notFoundError{}
}
var inserted *fleet.VPPClientUser
ds.InsertVPPClientUserFunc = func(_ context.Context, row *fleet.VPPClientUser) error {
inserted = row
ds.InsertVPPClientUserFunc = func(_ context.Context, _ *fleet.VPPClientUser) error {
t.Fatal("InsertVPPClientUser must not be called when v1 register-user returns an error")
return nil
}
@@ -204,8 +253,7 @@ func TestEnsureVPPClientUser_PartialFailureKeepsPending(t *testing.T) {
_, err := svc.ensureVPPClientUser(context.Background(), host, tokenDB)
require.Error(t, err)
require.Contains(t, err.Error(), "9637")
require.NotNil(t, inserted)
require.Equal(t, fleet.VPPClientUserStatusPending, inserted.Status, "partial failure must persist row as pending so retries can reuse the UUID")
require.False(t, ds.InsertVPPClientUserFuncInvoked)
}
func TestEnsureVPPClientUser_MissingManagedAppleID(t *testing.T) {