<!-- 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 -->
237 lines
7.9 KiB
Go
237 lines
7.9 KiB
Go
package service
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"fmt"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
|
|
"github.com/fleetdm/fleet/v4/server/dev_mode"
|
|
"github.com/fleetdm/fleet/v4/server/fleet"
|
|
"github.com/fleetdm/fleet/v4/server/mock"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
func setupFakeVPPServer(t *testing.T, handler http.HandlerFunc) {
|
|
t.Helper()
|
|
server := httptest.NewServer(handler)
|
|
dev_mode.SetOverride("FLEET_DEV_VPP_URL", server.URL, t)
|
|
t.Cleanup(server.Close)
|
|
}
|
|
|
|
func newTestServiceWithDS(ds fleet.Datastore) *Service {
|
|
return &Service{ds: ds}
|
|
}
|
|
|
|
func TestEnsureVPPClientUser_NewUser(t *testing.T) {
|
|
const (
|
|
hostID = uint(7)
|
|
managedAppleID = "user@example.com"
|
|
)
|
|
tokenDB := &fleet.VPPTokenDB{ID: 42, Token: "valid-token"}
|
|
host := &fleet.Host{ID: hostID}
|
|
|
|
var registerCalls int
|
|
setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) {
|
|
registerCalls++
|
|
assert.Equal(t, http.MethodPost, r.Method)
|
|
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 {
|
|
SToken string `json:"sToken"`
|
|
ClientUserIDStr string `json:"clientUserIdStr"`
|
|
ManagedAppleIDStr string `json:"managedAppleIDStr"`
|
|
}
|
|
assert.NoError(t, json.NewDecoder(r.Body).Decode(&got))
|
|
assert.Equal(t, "valid-token", got.SToken)
|
|
assert.NotEmpty(t, got.ClientUserIDStr)
|
|
assert.Equal(t, managedAppleID, got.ManagedAppleIDStr)
|
|
|
|
_, _ = fmt.Fprintf(w, `{
|
|
"status": 0,
|
|
"user": {
|
|
"userId": 98765,
|
|
"status": "Registered",
|
|
"clientUserIdStr": %q,
|
|
"managedAppleIDStr": %q
|
|
}
|
|
}`, got.ClientUserIDStr, managedAppleID)
|
|
})
|
|
|
|
ds := new(mock.Store)
|
|
ds.GetHostManagedAppleIDFunc = func(_ context.Context, id uint) (string, error) {
|
|
require.Equal(t, hostID, id)
|
|
return managedAppleID, nil
|
|
}
|
|
ds.GetVPPClientUserFunc = func(_ context.Context, tokenID uint, mAppleID string) (*fleet.VPPClientUser, error) {
|
|
require.Equal(t, tokenDB.ID, tokenID)
|
|
require.Equal(t, managedAppleID, mAppleID)
|
|
return nil, ¬FoundError{}
|
|
}
|
|
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.NotEmpty(t, clientUserID)
|
|
require.Equal(t, 1, registerCalls)
|
|
|
|
require.NotNil(t, insertedRow)
|
|
require.Equal(t, tokenDB.ID, insertedRow.VPPTokenID)
|
|
require.Equal(t, managedAppleID, insertedRow.ManagedAppleID)
|
|
require.Equal(t, clientUserID, insertedRow.ClientUserID)
|
|
require.Equal(t, fleet.VPPClientUserStatusRegistered, insertedRow.Status)
|
|
require.NotNil(t, insertedRow.AppleUserID)
|
|
require.Equal(t, "98765", *insertedRow.AppleUserID)
|
|
}
|
|
|
|
func TestEnsureVPPClientUser_ExistingRegisteredUser(t *testing.T) {
|
|
const managedAppleID = "user@example.com"
|
|
tokenDB := &fleet.VPPTokenDB{ID: 1, Token: "tok"}
|
|
host := &fleet.Host{ID: 1}
|
|
|
|
// Apple must NOT be called on cache hit.
|
|
setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) {
|
|
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)
|
|
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: "cached-uuid",
|
|
Status: fleet.VPPClientUserStatusRegistered,
|
|
}, nil
|
|
}
|
|
|
|
svc := newTestServiceWithDS(ds)
|
|
clientUserID, err := svc.ensureVPPClientUser(context.Background(), host, tokenDB)
|
|
require.NoError(t, err)
|
|
require.Equal(t, "cached-uuid", clientUserID)
|
|
require.False(t, ds.InsertVPPClientUserFuncInvoked)
|
|
}
|
|
|
|
// A non-registered cache row (typically 'pending' from the legacy v2 async
|
|
// 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}
|
|
|
|
var registerCalls int
|
|
setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) {
|
|
switch {
|
|
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)
|
|
}
|
|
})
|
|
|
|
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)
|
|
ds.GetHostManagedAppleIDFunc = func(_ context.Context, _ uint) (string, error) {
|
|
return managedAppleID, nil
|
|
}
|
|
ds.GetVPPClientUserFunc = func(_ context.Context, _ uint, _ string) (*fleet.VPPClientUser, error) {
|
|
return nil, ¬FoundError{}
|
|
}
|
|
ds.InsertVPPClientUserFunc = func(_ context.Context, _ *fleet.VPPClientUser) error {
|
|
t.Fatal("InsertVPPClientUser must not be called when v1 register-user returns an error")
|
|
return nil
|
|
}
|
|
|
|
svc := newTestServiceWithDS(ds)
|
|
_, err := svc.ensureVPPClientUser(context.Background(), host, tokenDB)
|
|
require.Error(t, err)
|
|
require.Contains(t, err.Error(), "9637")
|
|
require.False(t, ds.InsertVPPClientUserFuncInvoked)
|
|
}
|
|
|
|
func TestEnsureVPPClientUser_MissingManagedAppleID(t *testing.T) {
|
|
tokenDB := &fleet.VPPTokenDB{ID: 1, Token: "tok"}
|
|
host := &fleet.Host{ID: 1}
|
|
|
|
// Apple must not be called.
|
|
setupFakeVPPServer(t, func(w http.ResponseWriter, r *http.Request) {
|
|
t.Fatalf("Apple VPP create-users must not be called when Managed Apple ID is missing; got %s %s", r.Method, r.URL.Path)
|
|
})
|
|
|
|
ds := new(mock.Store)
|
|
ds.GetHostManagedAppleIDFunc = func(_ context.Context, _ uint) (string, error) {
|
|
return "", nil
|
|
}
|
|
|
|
svc := newTestServiceWithDS(ds)
|
|
_, err := svc.ensureVPPClientUser(context.Background(), host, tokenDB)
|
|
require.Error(t, err)
|
|
require.Contains(t, err.Error(), "Managed Apple ID")
|
|
|
|
// User-facing message surface — important since this is shown to admins.
|
|
var ume *fleet.UserMessageError
|
|
require.ErrorAs(t, err, &ume)
|
|
require.Equal(t, http.StatusUnprocessableEntity, ume.StatusCode())
|
|
|
|
require.False(t, ds.GetVPPClientUserFuncInvoked)
|
|
require.False(t, ds.InsertVPPClientUserFuncInvoked)
|
|
}
|