diff --git a/changes/36621-harmonize-installer-conflict-error-messages b/changes/36621-harmonize-installer-conflict-error-messages new file mode 100644 index 0000000000..c9f35c0a50 --- /dev/null +++ b/changes/36621-harmonize-installer-conflict-error-messages @@ -0,0 +1 @@ +- Added validation and harmonized the error message displayed when an installer (FMA, custom package, VPP app, in-house app) conflicts with another one on the same team targeting the same platform. diff --git a/ee/server/service/vpp.go b/ee/server/service/vpp.go index 98efd7a872..49bdc2886c 100644 --- a/ee/server/service/vpp.go +++ b/ee/server/service/vpp.go @@ -543,7 +543,7 @@ func (svc *Service) AddAppStoreApp(ctx context.Context, teamID *uint, appID flee return 0, ctxerr.Wrap(ctx, err, "validating software labels for adding vpp app") } - var teamName string + teamName := fleet.TeamNameNoTeam if teamID != nil && *teamID != 0 { tm, err := svc.ds.TeamLite(ctx, *teamID) if fleet.IsNotFound(err) { @@ -645,10 +645,9 @@ func (svc *Service) AddAppStoreApp(ctx context.Context, teamID *uint, appID flee } if appID.Platform == fleet.MacOSPlatform { - // Check if we've already added an installer for this app - exists, err := svc.ds.UploadedSoftwareExists(ctx, appFromApple.BundleIdentifier, teamID) + exists, err := svc.ds.CheckConflictingInstallerExists(ctx, teamID, appFromApple.BundleIdentifier, string(appID.Platform)) if err != nil { - return 0, ctxerr.Wrap(ctx, err, "checking existence of VPP app installer") + return 0, ctxerr.Wrap(ctx, err, "checking existence of conflicting installer") } if exists { @@ -657,6 +656,19 @@ func (svc *Service) AddAppStoreApp(ctx context.Context, teamID *uint, appID flee assetMD.Attributes.Name, teamName), }, "vpp app conflicts with existing software installer") } + } else if appID.Platform == fleet.IOSPlatform || appID.Platform == fleet.IPadOSPlatform { + // Check if an in-house app (IPA) with the same bundle identifier already exists + exists, err := svc.ds.CheckConflictingInHouseAppExists(ctx, teamID, appFromApple.BundleIdentifier, string(appID.Platform)) + if err != nil { + return 0, ctxerr.Wrap(ctx, err, "checking existence of conflicting installer") + } + + if exists { + return 0, ctxerr.Wrap(ctx, fleet.ConflictError{ + Message: fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, + assetMD.Attributes.Name, teamName), + }, "vpp app conflicts with existing in-house app") + } } appID.ValidatedLabels = validatedLabels diff --git a/frontend/pages/SoftwarePage/SoftwareAddPage/helpers.tests.tsx b/frontend/pages/SoftwarePage/SoftwareAddPage/helpers.tests.tsx index 66b3fb6c33..d58d487aa3 100644 --- a/frontend/pages/SoftwarePage/SoftwareAddPage/helpers.tests.tsx +++ b/frontend/pages/SoftwarePage/SoftwareAddPage/helpers.tests.tsx @@ -31,8 +31,8 @@ describe("ensurePeriod", () => { describe("formatAlreadyAvailableInstallMessage", () => { it("returns a React fragment with the correct text and team when the string matches the regex", () => { - // Example input: "Couldn't add. MyApp already has a package or app available for install on the Marketing team." - const msg = `${ADD_SOFTWARE_ERROR_PREFIX} MyApp already has a package or app available for install on the Marketing team.`; + // Example input: "Couldn't add. MyApp already has an installer available for the Marketing team." + const msg = `${ADD_SOFTWARE_ERROR_PREFIX} MyApp already has an installer available for the Marketing team.`; const result = formatAlreadyAvailableInstallMessage(msg); // Render for querying text @@ -63,7 +63,7 @@ describe("formatAlreadyAvailableInstallMessage", () => { }); it("works for different app names and team names", () => { - const msg = `${ADD_SOFTWARE_ERROR_PREFIX} Zoom already has a package or app available for install on the Engineering team.`; + const msg = `${ADD_SOFTWARE_ERROR_PREFIX} Zoom already has an installer available for the Engineering team.`; const result = formatAlreadyAvailableInstallMessage(msg); const { container } = render(<>{result}); diff --git a/frontend/pages/SoftwarePage/SoftwareAddPage/helpers.tsx b/frontend/pages/SoftwarePage/SoftwareAddPage/helpers.tsx index 32ac97e9dd..5a5b903206 100644 --- a/frontend/pages/SoftwarePage/SoftwareAddPage/helpers.tsx +++ b/frontend/pages/SoftwarePage/SoftwareAddPage/helpers.tsx @@ -23,20 +23,21 @@ export const formatAlreadyAvailableInstallMessage = (msg: string) => { // Remove prefix (with or without trailing space) const cleaned = msg.replace(/^Couldn't add software\.?\s*/, ""); - // New regex for " already has a package or app available for install on the team." - const installerExistsRegex = /^(.+?) already.+on the (.+?) team\./; + // New regex for " already has an installer available for the team." + const installerExistsRegex = /^(.+?) already.+the (.+?) team\./; let match = cleaned.match(installerExistsRegex); if (match) { return ( <> - {ADD_SOFTWARE_ERROR_PREFIX} {match[1]} already has a package or - app available for install on the {match[2]} team.{" "} + {ADD_SOFTWARE_ERROR_PREFIX} {match[1]} already has an installer + available for the {match[2]} team.{" "} ); } // New regex for "SoftwareInstaller already exists with team ." - const packageExistsRegex = /^SoftwareInstaller "(.+?)" already.+ team "(.+?)"\./; + // or "In-house app already exists with team ." + const packageExistsRegex = /^(?:SoftwareInstaller|In-house app) "(.+?)" already.+ team "(.+?)"\./; match = cleaned.match(packageExistsRegex); if (match) { return ( diff --git a/server/datastore/mysql/in_house_apps.go b/server/datastore/mysql/in_house_apps.go index d068aa05a6..b14504dddf 100644 --- a/server/datastore/mysql/in_house_apps.go +++ b/server/datastore/mysql/in_house_apps.go @@ -47,7 +47,11 @@ func (ds *Datastore) insertInHouseApp(ctx context.Context, payload *fleet.InHous } if count > 0 { // ios or ipados version of this installer exists - return alreadyExists("In-house app", payload.Filename) + teamName, err := ds.getTeamName(ctx, payload.TeamID) + if err != nil { + return ctxerr.Wrap(ctx, err) + } + return alreadyExists("In-house app", payload.Filename).WithTeamName(teamName) } argsIos := []any{tid, globalOrTeamID, payload.Filename, payload.StorageID, payload.Version, payload.BundleID, titleIDios, "ios", payload.SelfService} @@ -109,7 +113,12 @@ func (ds *Datastore) insertInHouseAppDB(ctx context.Context, tx sqlx.ExtContext, res, err := tx.ExecContext(ctx, stmt, args...) if err != nil { if IsDuplicate(err) { - err = alreadyExists("In-house app", payload.Filename) + teamName, err := ds.getTeamName(ctx, payload.TeamID) + if err != nil { + return 0, ctxerr.Wrap(ctx, err) + } + err = alreadyExists("In-house app", payload.Filename).WithTeamName(teamName) + return 0, ctxerr.Wrap(ctx, err, "insertInHouseAppDB") } return 0, ctxerr.Wrap(ctx, err, "insertInHouseAppDB") } @@ -267,7 +276,11 @@ func (ds *Datastore) SaveInHouseAppUpdates(ctx context.Context, payload *fleet.U if _, err := tx.ExecContext(ctx, stmt, args...); err != nil { if IsDuplicate(err) { - return alreadyExists("In-house app", payload.Filename) + teamName, err := ds.getTeamName(ctx, payload.TeamID) + if err != nil { + return ctxerr.Wrap(ctx, err) + } + return alreadyExists("In-house app", payload.Filename).WithTeamName(teamName) } return ctxerr.Wrap(ctx, err, "update in house app") } @@ -726,11 +739,11 @@ WHERE (unique_identifier, source, extension_for) IN (%s) ` const getSoftwareTitle = ` -SELECT - id -FROM - software_titles -WHERE +SELECT + id +FROM + software_titles +WHERE unique_identifier = ? AND source = ? AND extension_for = '' ` @@ -1060,8 +1073,39 @@ WHERE return nil } + // Get team name for error messages + teamName, err := ds.getTeamName(ctx, tmID) + if err != nil { + return ctxerr.Wrap(ctx, err, "get team name for conflict check") + } + var args []any for _, installer := range installers { + // Check for installers that target iOS/iPadOS if they conflict with an existing VPP app + if installer.BundleIdentifier != "" { + // Check for iOS VPP app conflict + exists, err := ds.checkVPPAppExistsForTitleIdentifier(ctx, tx, tmID, string(fleet.IOSPlatform), installer.BundleIdentifier, "ios_apps", "") + if err != nil { + return ctxerr.Wrap(ctx, err, "check if VPP app (ios) exists for in-house app") + } + if exists { + return ctxerr.Wrap(ctx, fleet.ConflictError{ + Message: fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, installer.Title, teamName), + }, "in-house app conflicts with existing VPP app (ios)") + } + + // Check for iPadOS VPP app conflict + exists, err = ds.checkVPPAppExistsForTitleIdentifier(ctx, tx, tmID, string(fleet.IPadOSPlatform), installer.BundleIdentifier, "ipados_apps", "") + if err != nil { + return ctxerr.Wrap(ctx, err, "check if VPP app (ipados) exists for in-house app") + } + if exists { + return ctxerr.Wrap(ctx, fleet.ConflictError{ + Message: fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, installer.Title, teamName), + }, "in-house app conflicts with existing VPP app (ipados)") + } + } + var providedTitle *string if installer.Title != "" { providedTitle = &installer.Title // for IPAs downloaded via URL; IPAs referenced by hash won't have this @@ -1451,3 +1495,65 @@ WHERE in_house_app_id = ? return affectedHostIDs, nil } + +func (ds *Datastore) CheckConflictingInstallerExists(ctx context.Context, teamID *uint, bundleIdentifier, platform string) (bool, error) { + return ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), teamID, bundleIdentifier, platform, softwareTypeInstaller) +} + +func (ds *Datastore) CheckConflictingInHouseAppExists(ctx context.Context, teamID *uint, bundleIdentifier, platform string) (bool, error) { + return ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), teamID, bundleIdentifier, platform, softwareTypeInHouseApp) +} + +func (ds *Datastore) checkInstallerOrInHouseAppExists(ctx context.Context, q sqlx.QueryerContext, teamID *uint, bundleIdentifier, platform string, swType softwareType) (bool, error) { + stmt := fmt.Sprintf(` +SELECT 1 +FROM + software_titles st + INNER JOIN %[1]ss ON st.id = %[1]ss.title_id AND %[1]ss.global_or_team_id = ? +WHERE + st.unique_identifier = ? + AND %[1]ss.platform = ? +`, swType) + + var globalOrTeamID uint + if teamID != nil { + globalOrTeamID = *teamID + } + var exists int + err := sqlx.GetContext(ctx, q, &exists, stmt, globalOrTeamID, bundleIdentifier, platform) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return false, ctxerr.Wrap(ctx, err, fmt.Sprintf("check %s exists", swType)) + } + return exists == 1, nil +} + +func (ds *Datastore) checkInHouseAppExistsForAdamID(ctx context.Context, q sqlx.QueryerContext, teamID *uint, appID fleet.VPPAppID) (exists bool, title string, err error) { + const stmt = ` +SELECT st.name +FROM software_titles st +INNER JOIN in_house_apps iha ON iha.title_id = st.id AND + iha.global_or_team_id = ? +INNER JOIN vpp_apps va ON va.bundle_identifier = st.bundle_identifier +INNER JOIN vpp_apps_teams vat ON vat.adam_id = va.adam_id AND vat.platform = va.platform AND + vat.global_or_team_id = ? +WHERE + va.adam_id = ? + AND va.platform = ? + AND iha.platform = va.platform +LIMIT 1 +` + + var globalOrTeamID uint + if teamID != nil { + globalOrTeamID = *teamID + } + + err = sqlx.GetContext(ctx, q, &title, stmt, globalOrTeamID, globalOrTeamID, appID.AdamID, appID.Platform) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return false, "", nil + } + return false, "", err + } + return true, title, nil +} diff --git a/server/datastore/mysql/software_installers.go b/server/datastore/mysql/software_installers.go index 69fa272ded..22e36b8072 100644 --- a/server/datastore/mysql/software_installers.go +++ b/server/datastore/mysql/software_installers.go @@ -163,7 +163,7 @@ func (ds *Datastore) GetSoftwareInstallDetails(ctx context.Context, executionId return result, nil } -func (ds *Datastore) checkVPPAppExistsForTitleIdentifier(ctx context.Context, q sqlx.QueryerContext, teamID *uint, bundleIdentifier, source, browser string) (bool, error) { +func (ds *Datastore) checkVPPAppExistsForTitleIdentifier(ctx context.Context, q sqlx.QueryerContext, teamID *uint, platform, bundleIdentifier, source, browser string) (bool, error) { const stmt = ` SELECT 1 @@ -183,7 +183,7 @@ WHERE globalOrTeamID = *teamID } var exists int - err := sqlx.GetContext(ctx, q, &exists, stmt, fleet.MacOSPlatform, globalOrTeamID, bundleIdentifier, source, browser) + err := sqlx.GetContext(ctx, q, &exists, stmt, platform, globalOrTeamID, bundleIdentifier, source, browser) if err != nil && !errors.Is(err, sql.ErrNoRows) { return false, ctxerr.Wrap(ctx, err, "check VPP app exists for title identifier") } @@ -197,6 +197,18 @@ func (ds *Datastore) MatchOrCreateSoftwareInstaller(ctx context.Context, payload return 0, 0, errors.New("validated labels must not be nil") } + err = ds.checkSoftwareConflictsByIdentifier(ctx, payload) + if err != nil { + teamName, err := ds.getTeamName(ctx, payload.TeamID) + if err != nil { + return 0, 0, ctxerr.Wrap(ctx, err, "get team for installer conflict error") + } + + return 0, 0, ctxerr.Wrap(ctx, fleet.ConflictError{ + Message: fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, payload.Title, teamName), + }, "vpp app conflicts with existing software installer") + } + // Insert in house app instead of software installer // And add both iOS and ipadOS titles per https://github.com/fleetdm/fleet/issues/34283 if payload.Extension == "ipa" { @@ -223,27 +235,6 @@ func (ds *Datastore) MatchOrCreateSoftwareInstaller(ctx context.Context, payload return 0, 0, ctxerr.Wrap(ctx, err, "get or generate software installer title ID") } - // check if a VPP app already exists for that software title in the same - // platform (macOS) and team. - if payload.Platform == string(fleet.MacOSPlatform) { - exists, err := ds.checkVPPAppExistsForTitleIdentifier(ctx, ds.reader(ctx), - payload.TeamID, payload.BundleIdentifier, payload.Source, "") - if err != nil { - return 0, 0, ctxerr.Wrap(ctx, err, "check VPP app exists for title identifier") - } - if exists { - teamName, err := ds.getTeamName(ctx, payload.TeamID) - if err != nil { - return 0, 0, ctxerr.Wrap(ctx, err, "get team for VPP app conflict error") - } - - return 0, 0, ctxerr.Wrap(ctx, fleet.ConflictError{ - Message: fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, - payload.Title, teamName), - }, "vpp app conflicts with existing software installer") - } - } - // Enforce team-scoped uniqueness by storage hash, aligning upload behavior with GitOps. // However, if the duplicate-by-hash is for the same title/source on the same team, // let the DB unique (team,title) constraint surface the conflict (so tests expecting @@ -2368,7 +2359,7 @@ WHERE // platform (if that platform is macOS), then this is a conflict. // See https://github.com/fleetdm/fleet/issues/32082 if installer.Platform == string(fleet.MacOSPlatform) { - exists, err := ds.checkVPPAppExistsForTitleIdentifier(ctx, tx, tmID, installer.BundleIdentifier, installer.Source, "") + exists, err := ds.checkVPPAppExistsForTitleIdentifier(ctx, tx, tmID, installer.Platform, installer.BundleIdentifier, installer.Source, "") if err != nil { return ctxerr.Wrap(ctx, err, "check existing VPP app for installer title identifier") } @@ -3233,3 +3224,56 @@ WHERE return byTeam, nil } + +func (ds *Datastore) checkSoftwareConflictsByIdentifier(ctx context.Context, payload *fleet.UploadSoftwareInstallerPayload) error { + // if this is an in-house app, check if an installer exists + if payload.Extension == "ipa" { + // at the point where this method is called, we attempt to create both iOS and iPadOS entries + // for ipa apps, so check for conflicts on either platform. + for platform, source := range map[string]string{ + string(fleet.IOSPlatform): "ios_apps", + string(fleet.IPadOSPlatform): "ipados_apps", + } { + exists, err := ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.BundleIdentifier, platform, softwareTypeInstaller) + if err != nil { + return ctxerr.Wrap(ctx, err, "check if software installer exists for title identifier") + } + if exists { + return alreadyExists("software installer", payload.Title) + } + + exists, err = ds.checkVPPAppExistsForTitleIdentifier(ctx, ds.reader(ctx), payload.TeamID, platform, payload.BundleIdentifier, source, "") + if err != nil { + return ctxerr.Wrap(ctx, err, "check if VPP app exists for title identifier") + } + if exists { + return alreadyExists("VPP app", payload.Title) + } + } + } else { + // check if a VPP app already exists for that software title in the same + // platform and team. + if payload.Platform == string(fleet.MacOSPlatform) || payload.Platform == string(fleet.IOSPlatform) || payload.Platform == string(fleet.IPadOSPlatform) { + exists, err := ds.checkVPPAppExistsForTitleIdentifier(ctx, ds.reader(ctx), payload.TeamID, payload.Platform, payload.BundleIdentifier, payload.Source, "") + if err != nil { + return ctxerr.Wrap(ctx, err, "check if VPP app exists for title identifier") + } + if exists { + return alreadyExists("VPP app", payload.Title) + } + } + + // check if an in-house app with the same bundle id already exists. + if payload.BundleIdentifier != "" { + exists, err := ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.BundleIdentifier, payload.Platform, softwareTypeInHouseApp) + if err != nil { + return ctxerr.Wrap(ctx, err, "check if in-house app exists for title identifier") + } + if exists { + return alreadyExists("in-house app", payload.Title) + } + } + } + + return nil +} diff --git a/server/datastore/mysql/software_titles.go b/server/datastore/mysql/software_titles.go index 0534170ac0..8c956a8f57 100644 --- a/server/datastore/mysql/software_titles.go +++ b/server/datastore/mysql/software_titles.go @@ -805,32 +805,6 @@ func (ds *Datastore) SyncHostsSoftwareTitles(ctx context.Context, updatedAt time return nil } -func (ds *Datastore) UploadedSoftwareExists(ctx context.Context, bundleIdentifier string, teamID *uint) (bool, error) { - stmt := ` -SELECT - 1 -FROM - software_titles st JOIN software_installers si ON si.title_id = st.id -WHERE - st.bundle_identifier = ? AND si.global_or_team_id = ? - ` - var tmID uint - if teamID != nil { - tmID = *teamID - } - - var titleExists bool - if err := sqlx.GetContext(ctx, ds.reader(ctx), &titleExists, stmt, bundleIdentifier, tmID); err != nil { - if err == sql.ErrNoRows { - return false, nil - } - - return false, ctxerr.Wrap(ctx, err, "checking if software installer exists") - } - - return titleExists, nil -} - func (ds *Datastore) UpdateSoftwareTitleAutoUpdateConfig(ctx context.Context, titleID uint, teamID uint, config fleet.SoftwareAutoUpdateConfig) error { // Validate schedule if enabled. if config.AutoUpdateEnabled != nil && *config.AutoUpdateEnabled { diff --git a/server/datastore/mysql/software_titles_test.go b/server/datastore/mysql/software_titles_test.go index 38a11c91d4..53a46af902 100644 --- a/server/datastore/mysql/software_titles_test.go +++ b/server/datastore/mysql/software_titles_test.go @@ -37,7 +37,6 @@ func TestSoftwareTitles(t *testing.T) { {"ListSoftwareTitlesAvailableForInstallFilter", testListSoftwareTitlesAvailableForInstallFilter}, {"ListSoftwareTitlesOverflow", testListSoftwareTitlesOverflow}, {"ListSoftwareTitlesAllTeams", testListSoftwareTitlesAllTeams}, - {"UploadedSoftwareExists", testUploadedSoftwareExists}, {"ListSoftwareTitlesVulnerabilityFilters", testListSoftwareTitlesVulnerabilityFilters}, {"UpdateSoftwareTitleName", testUpdateSoftwareTitleName}, {"ListSoftwareTitlesDoesnotIncludeDuplicates", testListSoftwareTitlesDoesnotIncludeDuplicates}, @@ -1433,50 +1432,6 @@ func testListSoftwareTitlesAllTeams(t *testing.T, ds *Datastore) { }, names) } -func testUploadedSoftwareExists(t *testing.T, ds *Datastore) { - ctx := context.Background() - - tm, err := ds.NewTeam(ctx, &fleet.Team{Name: "Team Foo"}) - require.NoError(t, err) - user1 := test.NewUser(t, ds, "Alice", "alice@example.com", true) - - installer1, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ - Title: "installer1", - Source: "apps", - InstallScript: "echo", - Filename: "installer1.pkg", - BundleIdentifier: "com.foo.installer1", - UserID: user1.ID, - ValidatedLabels: &fleet.LabelIdentsWithScope{}, - }) - require.NoError(t, err) - require.NotZero(t, installer1) - installer2, _, err := ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ - Title: "installer2", - Source: "apps", - InstallScript: "echo", - Filename: "installer2.pkg", - TeamID: &tm.ID, - BundleIdentifier: "com.foo.installer2", - UserID: user1.ID, - ValidatedLabels: &fleet.LabelIdentsWithScope{}, - }) - require.NoError(t, err) - require.NotZero(t, installer2) - - exists, err := ds.UploadedSoftwareExists(ctx, "com.foo.installer1", nil) - require.NoError(t, err) - require.True(t, exists) - - exists, err = ds.UploadedSoftwareExists(ctx, "com.foo.installer2", nil) - require.NoError(t, err) - require.False(t, exists) - - exists, err = ds.UploadedSoftwareExists(ctx, "com.foo.installer2", &tm.ID) - require.NoError(t, err) - require.True(t, exists) -} - func testListSoftwareTitlesVulnerabilityFilters(t *testing.T, ds *Datastore) { ctx := context.Background() host := test.NewHost(t, ds, "host", "", "hostkey", "hostuuid", time.Now()) diff --git a/server/datastore/mysql/vpp.go b/server/datastore/mysql/vpp.go index ba6f2c79b7..7e465b53ee 100644 --- a/server/datastore/mysql/vpp.go +++ b/server/datastore/mysql/vpp.go @@ -458,16 +458,9 @@ func (ds *Datastore) SetTeamVPPApps(ctx context.Context, teamID *uint, incomingA } } - var teamName string - if len(toAddApps) > 0 { - teamName = fleet.TeamNameNoTeam - if teamID != nil && *teamID > 0 { - tm, err := ds.TeamLite(ctx, *teamID) - if err != nil { - return false, ctxerr.Wrap(ctx, err, "get team name for VPP app conflict error") - } - teamName = tm.Name - } + teamName, err := ds.getTeamName(ctx, teamID) + if err != nil { + return false, ctxerr.Wrap(ctx, err, "get team name for VPP app conflict error") } var vppToken *fleet.VPPTokenDB @@ -491,18 +484,10 @@ func (ds *Datastore) SetTeamVPPApps(ctx context.Context, teamID *uint, incomingA // check if the vpp app conflicts with an existing software installer // already associated with the software title for the same platform - // (macos). - if toAdd.Platform == fleet.MacOSPlatform { - exists, conflictingTitle, err := ds.checkConflictingSoftwareInstallerForVPPApp(ctx, tx, teamID, toAdd.VPPAppID) - if err != nil { - return ctxerr.Wrap(ctx, err, "checking for conflicting software installer") - } - if exists { - return ctxerr.Wrap(ctx, fleet.ConflictError{ - Message: fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, - conflictingTitle, teamName), - }, "vpp app conflicts with existing software installer") - } + // (macos) or any in-house app. + err = ds.checkSoftwareConflictsForVPPApp(ctx, tx, teamID, teamName, toAdd.VPPAppID) + if err != nil { + return ctxerr.Wrap(ctx, err, "check for software conflicts") } if toAdd.ValidatedLabels != nil { @@ -610,6 +595,11 @@ func (ds *Datastore) InsertVPPAppWithTeam(ctx context.Context, app *fleet.VPPApp vppTokenID = &vppToken.ID } + teamName, err := ds.getTeamName(ctx, teamID) + if err != nil { + return nil, ctxerr.Wrap(ctx, err, "get team for VPP app conflict error") + } + err = ds.withRetryTxx(ctx, func(tx sqlx.ExtContext) error { titleID, err := ds.getOrInsertSoftwareTitleForVPPApp(ctx, tx, app) if err != nil { @@ -627,6 +617,11 @@ func (ds *Datastore) InsertVPPAppWithTeam(ctx context.Context, app *fleet.VPPApp return ctxerr.Wrap(ctx, err, "InsertVPPAppWithTeam insertVPPAppTeams transaction") } + err = ds.checkSoftwareConflictsForVPPApp(ctx, tx, teamID, teamName, app.VPPAppID) + if err != nil { + return ctxerr.Wrap(ctx, err, "check for software conflicts") + } + app.VPPAppTeam.AppTeamID = vppAppTeamID if app.ValidatedLabels != nil { @@ -2469,9 +2464,37 @@ WHERE execution_id = ? return isAutoUpdate, nil } +func (ds *Datastore) checkSoftwareConflictsForVPPApp(ctx context.Context, tx sqlx.QueryerContext, teamID *uint, teamName string, appID fleet.VPPAppID) error { + if appID.Platform == fleet.MacOSPlatform { + exists, conflictingTitle, err := ds.checkConflictingSoftwareInstallerForVPPApp(ctx, tx, teamID, appID) + if err != nil { + return ctxerr.Wrap(ctx, err, "checking if software installer exists") + } + if exists { + return ctxerr.Wrap(ctx, fleet.ConflictError{ + Message: fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, + conflictingTitle, teamName)}, "vpp app conflicts with existing software installer") + } + } + + // check if the vpp app conflicts with an existing in-house app + if appID.Platform == fleet.IOSPlatform || appID.Platform == fleet.IPadOSPlatform { + exists, conflictingTitle, err := ds.checkInHouseAppExistsForAdamID(ctx, tx, teamID, appID) + if err != nil { + return ctxerr.Wrap(ctx, err, "check if in-house app exists") + } + if exists { + return ctxerr.Wrap(ctx, fleet.ConflictError{ + Message: fmt.Sprintf(fleet.CantAddSoftwareConflictMessage, conflictingTitle, teamName), + }, "vpp app conflicts with existing in-house app") + } + } + return nil +} + func (ds *Datastore) GetHostVPPInstallByCommandUUID(ctx context.Context, commandUUID string) (*fleet.HostVPPSoftwareInstallLite, error) { const stmt = ` - SELECT + SELECT command_uuid, host_id, retry_count diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index a044aaea3f..f9a64426fd 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -693,10 +693,6 @@ type Datastore interface { // persistence/bookkeeping only and must not be used to trigger user-visible side effects. CreateIntermediateInstallFailureRecord(ctx context.Context, result *HostSoftwareInstallResultPayload) (string, error) - // UploadedSoftwareExists checks if a software title with the given bundle identifier exists in - // the given team. - UploadedSoftwareExists(ctx context.Context, bundleIdentifier string, teamID *uint) (bool, error) - // NewSoftwareCategory creates a new category for software. NewSoftwareCategory(ctx context.Context, name string) (*SoftwareCategory, error) // GetSoftwareCategoryIDs the list of IDs that correspond to the given list of software category names. @@ -724,6 +720,9 @@ type Datastore interface { SetVPPInstallAsFailed(ctx context.Context, hostID uint, installUUID, verificationUUID string) error MarkAllPendingAppleVPPAndInHouseInstallsAsFailed(ctx context.Context, jobName string) error + CheckConflictingInstallerExists(ctx context.Context, teamID *uint, bundleIdentifier, platform string) (bool, error) + CheckConflictingInHouseAppExists(ctx context.Context, teamID *uint, bundleIdentifier, platform string) (bool, error) + /////////////////////////////////////////////////////////////////////////////// // OperatingSystemsStore diff --git a/server/fleet/errors.go b/server/fleet/errors.go index 31b5d67117..3660786ad9 100644 --- a/server/fleet/errors.go +++ b/server/fleet/errors.go @@ -32,7 +32,7 @@ var ( CantDisableDiskEncryptionIfPINRequiredErrMsg = "Couldn't disable disk encryption, you need to disable the BitLocker PIN requirement first." CantEnablePINRequiredIfDiskEncryptionEnabled = "Couldn't enable BitLocker PIN requirement, you must enable disk encryption first." CantResendAppleDeclarationProfilesMessage = "Can't resend declaration (DDM) profiles. Unlike configuration profiles (.mobileconfig), the host automatically checks in to get the latest DDM profiles." - CantAddSoftwareConflictMessage = "Couldn't add software. %s already has a package or app available for install on the %s team." + CantAddSoftwareConflictMessage = "Couldn't add software. %s already has an installer available for the %s team." ) // ErrWithStatusCode is an interface for errors that should set a specific HTTP diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 950ff482d6..4bda5658f0 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -537,8 +537,6 @@ type SetHostSoftwareInstallResultFunc func(ctx context.Context, result *fleet.Ho type CreateIntermediateInstallFailureRecordFunc func(ctx context.Context, result *fleet.HostSoftwareInstallResultPayload) (string, error) -type UploadedSoftwareExistsFunc func(ctx context.Context, bundleIdentifier string, teamID *uint) (bool, error) - type NewSoftwareCategoryFunc func(ctx context.Context, name string) (*fleet.SoftwareCategory, error) type GetSoftwareCategoryIDsFunc func(ctx context.Context, names []string) ([]uint, error) @@ -559,6 +557,10 @@ type SetVPPInstallAsFailedFunc func(ctx context.Context, hostID uint, installUUI type MarkAllPendingAppleVPPAndInHouseInstallsAsFailedFunc func(ctx context.Context, jobName string) error +type CheckConflictingInstallerExistsFunc func(ctx context.Context, teamID *uint, bundleIdentifier string, platform string) (bool, error) + +type CheckConflictingInHouseAppExistsFunc func(ctx context.Context, teamID *uint, bundleIdentifier string, platform string) (bool, error) + type GetHostOperatingSystemFunc func(ctx context.Context, hostID uint) (*fleet.OperatingSystem, error) type ListOperatingSystemsFunc func(ctx context.Context) ([]fleet.OperatingSystem, error) @@ -2513,9 +2515,6 @@ type DataStore struct { CreateIntermediateInstallFailureRecordFunc CreateIntermediateInstallFailureRecordFunc CreateIntermediateInstallFailureRecordFuncInvoked bool - UploadedSoftwareExistsFunc UploadedSoftwareExistsFunc - UploadedSoftwareExistsFuncInvoked bool - NewSoftwareCategoryFunc NewSoftwareCategoryFunc NewSoftwareCategoryFuncInvoked bool @@ -2546,6 +2545,12 @@ type DataStore struct { MarkAllPendingAppleVPPAndInHouseInstallsAsFailedFunc MarkAllPendingAppleVPPAndInHouseInstallsAsFailedFunc MarkAllPendingAppleVPPAndInHouseInstallsAsFailedFuncInvoked bool + CheckConflictingInstallerExistsFunc CheckConflictingInstallerExistsFunc + CheckConflictingInstallerExistsFuncInvoked bool + + CheckConflictingInHouseAppExistsFunc CheckConflictingInHouseAppExistsFunc + CheckConflictingInHouseAppExistsFuncInvoked bool + GetHostOperatingSystemFunc GetHostOperatingSystemFunc GetHostOperatingSystemFuncInvoked bool @@ -6121,13 +6126,6 @@ func (s *DataStore) CreateIntermediateInstallFailureRecord(ctx context.Context, return s.CreateIntermediateInstallFailureRecordFunc(ctx, result) } -func (s *DataStore) UploadedSoftwareExists(ctx context.Context, bundleIdentifier string, teamID *uint) (bool, error) { - s.mu.Lock() - s.UploadedSoftwareExistsFuncInvoked = true - s.mu.Unlock() - return s.UploadedSoftwareExistsFunc(ctx, bundleIdentifier, teamID) -} - func (s *DataStore) NewSoftwareCategory(ctx context.Context, name string) (*fleet.SoftwareCategory, error) { s.mu.Lock() s.NewSoftwareCategoryFuncInvoked = true @@ -6198,6 +6196,20 @@ func (s *DataStore) MarkAllPendingAppleVPPAndInHouseInstallsAsFailed(ctx context return s.MarkAllPendingAppleVPPAndInHouseInstallsAsFailedFunc(ctx, jobName) } +func (s *DataStore) CheckConflictingInstallerExists(ctx context.Context, teamID *uint, bundleIdentifier string, platform string) (bool, error) { + s.mu.Lock() + s.CheckConflictingInstallerExistsFuncInvoked = true + s.mu.Unlock() + return s.CheckConflictingInstallerExistsFunc(ctx, teamID, bundleIdentifier, platform) +} + +func (s *DataStore) CheckConflictingInHouseAppExists(ctx context.Context, teamID *uint, bundleIdentifier string, platform string) (bool, error) { + s.mu.Lock() + s.CheckConflictingInHouseAppExistsFuncInvoked = true + s.mu.Unlock() + return s.CheckConflictingInHouseAppExistsFunc(ctx, teamID, bundleIdentifier, platform) +} + func (s *DataStore) GetHostOperatingSystem(ctx context.Context, hostID uint) (*fleet.OperatingSystem, error) { s.mu.Lock() s.GetHostOperatingSystemFuncInvoked = true diff --git a/server/service/integration_android_software_test.go b/server/service/integration_android_software_test.go index 7a69e9c0c4..b58561a424 100644 --- a/server/service/integration_android_software_test.go +++ b/server/service/integration_android_software_test.go @@ -391,7 +391,7 @@ func (s *integrationMDMTestSuite) TestAndroidAppsSelfService() { // Verify that activity includes configuration s.lastActivityMatches(fleet.ActivityAddedAppStoreApp{}.ActivityName(), fmt.Sprintf(`{"team_name": "%s", "software_title": "%s", "software_title_id": %d, "app_store_id": "%s", "team_id": %s, "platform": "%s", "self_service": true,"configuration": %s}`, - "", androidAppWithConfig.Name, appWithConfigResp.TitleID, androidAppWithConfig.AdamID, "null", androidAppWithConfig.Platform, androidAppWithConfig.Configuration), 0) + fleet.TeamNameNoTeam, androidAppWithConfig.Name, appWithConfigResp.TitleID, androidAppWithConfig.AdamID, "null", androidAppWithConfig.Platform, androidAppWithConfig.Configuration), 0) // Should see it in host software library s.DoJSON("GET", fmt.Sprintf("/api/latest/fleet/hosts/%d/software", host1.ID), nil, http.StatusOK, &getHostSw, "available_for_install", "true") diff --git a/server/service/integration_vpp_install_test.go b/server/service/integration_vpp_install_test.go index 108fbab030..13275409f2 100644 --- a/server/service/integration_vpp_install_test.go +++ b/server/service/integration_vpp_install_test.go @@ -14,6 +14,7 @@ import ( "github.com/fleetdm/fleet/v4/pkg/mdm/mdmtest" "github.com/fleetdm/fleet/v4/server/datastore/mysql" "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/fleetdm/fleet/v4/server/mdm/apple/vpp" "github.com/fleetdm/fleet/v4/server/mdm/nanomdm/mdm" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/jmoiron/sqlx" @@ -1283,7 +1284,7 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleVPPAppSoftwarePackageConflict Title: "DummyApp", TeamID: &team.ID, } - s.uploadSoftwareInstaller(t, pkgDummy, http.StatusConflict, "DummyApp already has a package or app available for install on the Team 1 team.") + s.uploadSoftwareInstaller(t, pkgDummy, http.StatusConflict, "DummyApp already has an installer available for the Team 1 team.") // Add VPP app 2 with bundle ID com.example.noversion (conflicts with NoVersion) vppApp2 := &fleet.VPPApp{ @@ -1297,7 +1298,7 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleVPPAppSoftwarePackageConflict res := s.Do("POST", "/api/latest/fleet/software/app_store_apps", &addAppStoreAppRequest{TeamID: &team.ID, AppStoreID: vppApp2.AdamID, SelfService: true}, http.StatusConflict) txt := extractServerErrorText(res.Body) - require.Contains(t, txt, "NoVersion already has a package or app available for install on the Team 1 team.") + require.Contains(t, txt, "NoVersion already has an installer available for the Team 1 team.") // --- test with batch-set (gitops) --- @@ -1325,7 +1326,7 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleVPPAppSoftwarePackageConflict }, http.StatusAccepted, &batchResponse, "team_name", team.Name) batchResp := waitBatchSetSoftwareInstallers(t, &s.withServer, team.Name, batchResponse.RequestUUID) require.Equal(t, fleet.BatchSetSoftwareInstallersStatusFailed, batchResp.Status) - require.Contains(t, batchResp.Message, "DummyApp already has a package or app available for install on the Team 1 team.") + require.Contains(t, batchResp.Message, "DummyApp already has an installer available for the Team 1 team.") // batch-set the VPP apps, including one in conflict res = s.Do("POST", "/api/latest/fleet/software/app_store_apps/batch", batchAssociateAppStoreAppsRequest{Apps: []fleet.VPPBatchPayload{ @@ -1333,7 +1334,7 @@ func (s *integrationMDMTestSuite) TestSoftwareTitleVPPAppSoftwarePackageConflict {AppStoreID: "2"}, }}, http.StatusConflict, "team_name", team.Name) txt = extractServerErrorText(res.Body) - require.Contains(t, txt, "NoVersion already has a package or app available for install on the Team 1 team.") + require.Contains(t, txt, "NoVersion already has an installer available for the Team 1 team.") // listing software available to install only lists the dummy app and noversion installer var listSw listSoftwareTitlesResponse @@ -2720,3 +2721,112 @@ CMD_LOOP: } */ } + +// TestInHouseAppVPPConflict tests that IPA (in-house apps) and VPP iOS/iPadOS apps +// with the same bundle identifier cannot coexist on the same team. +func (s *integrationMDMTestSuite) TestInHouseAppVPPConflict() { + t := s.T() + s.setSkipWorkerJobs(t) + + s.registerResetVPPProxyData(t) + + s.appleVPPProxySrvData = map[string]string{ + "100": `{"id": "100", "attributes": {"name": "IPA Test App", "platformAttributes": {"ios": {"bundleId": "com.ipa-test.ipa-test", "artwork": {"url": "https://example.com/images/100/{w}x{h}.{f}"}, "latestVersionInfo": {"versionDisplay": "1.0.0"}}}, "deviceFamilies": ["iphone"]}}`, + "101": `{"id": "101", "attributes": {"name": "IPA Test App iPad", "platformAttributes": {"ios": {"bundleId": "com.ipa-test.ipa-test", "artwork": {"url": "https://example.com/images/101/{w}x{h}.{f}"}, "latestVersionInfo": {"versionDisplay": "1.0.0"}}}, "deviceFamilies": ["ipad"]}}`, + "102": `{"id": "102", "attributes": {"name": "Different App", "platformAttributes": {"ios": {"bundleId": "com.example.different", "artwork": {"url": "https://example.com/images/102/{w}x{h}.{f}"}, "latestVersionInfo": {"versionDisplay": "1.0.0"}}}, "deviceFamilies": ["iphone"]}}`, + } + + originalAssets := s.appleVPPConfigSrvConfig.Assets + t.Cleanup(func() { s.appleVPPConfigSrvConfig.Assets = originalAssets }) + + s.appleVPPConfigSrvConfig.Assets = append(s.appleVPPConfigSrvConfig.Assets, vpp.Asset{ + AdamID: "100", + PricingParam: "STDQ", + AvailableCount: 10, + }, vpp.Asset{ + AdamID: "101", + PricingParam: "STDQ", + AvailableCount: 10, + }, vpp.Asset{ + AdamID: "102", + PricingParam: "STDQ", + AvailableCount: 10, + }) + + var newTeamResp teamResponse + s.DoJSON("POST", "/api/latest/fleet/teams", &createTeamRequest{TeamPayload: fleet.TeamPayload{Name: ptr.String("IPA Conflict Team")}}, http.StatusOK, &newTeamResp) + team := newTeamResp.Team + + s.setVPPTokenForTeam(team.ID) + + // Test Case 1: Upload IPA first, then try to add VPP iOS app with same bundle ID + s.uploadSoftwareInstaller(t, &fleet.UploadSoftwareInstallerPayload{ + Filename: "ipa_test.ipa", + TeamID: &team.ID, + }, http.StatusOK, "") + + res := s.Do("POST", "/api/latest/fleet/software/app_store_apps", &addAppStoreAppRequest{ + TeamID: &team.ID, + AppStoreID: "100", + Platform: "ios", + }, http.StatusConflict) + txt := extractServerErrorText(res.Body) + require.Contains(t, txt, "already has an installer available for the IPA Conflict Team team.") + + res = s.Do("POST", "/api/latest/fleet/software/app_store_apps", &addAppStoreAppRequest{ + TeamID: &team.ID, + AppStoreID: "101", + Platform: "ipados", + }, http.StatusConflict) + txt = extractServerErrorText(res.Body) + require.Contains(t, txt, "already has an installer available for the IPA Conflict Team team.") + + var addAppResp addAppStoreAppResponse + s.DoJSON("POST", "/api/latest/fleet/software/app_store_apps", &addAppStoreAppRequest{ + TeamID: &team.ID, + AppStoreID: "102", + Platform: "ios", + }, http.StatusOK, &addAppResp) + + // Test Case 2: Add VPP iOS app first, then try to upload IPA with same bundle ID + var newTeamResp2 teamResponse + s.DoJSON("POST", "/api/latest/fleet/teams", &createTeamRequest{TeamPayload: fleet.TeamPayload{Name: ptr.String("IPA Conflict Team 2")}}, http.StatusOK, &newTeamResp2) + team2 := newTeamResp2.Team + + var tokenResp getVPPTokensResponse + s.DoJSON("GET", "/api/latest/fleet/vpp_tokens", &getVPPTokensRequest{}, http.StatusOK, &tokenResp) + var resPatchVPP patchVPPTokensTeamsResponse + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/vpp_tokens/%d/teams", tokenResp.Tokens[0].ID), patchVPPTokensTeamsRequest{TeamIDs: []uint{team.ID, team2.ID}}, http.StatusOK, &resPatchVPP) + + s.DoJSON("POST", "/api/latest/fleet/software/app_store_apps", &addAppStoreAppRequest{ + TeamID: &team2.ID, + AppStoreID: "100", + Platform: "ios", + }, http.StatusOK, &addAppResp) + + s.uploadSoftwareInstaller(t, &fleet.UploadSoftwareInstallerPayload{ + Filename: "ipa_test.ipa", + TeamID: &team2.ID, + }, http.StatusConflict, "already has an installer available for the IPA Conflict Team 2 team.") + + // Test Case 3: Verify "No team" works correctly + s.uploadSoftwareInstaller(t, &fleet.UploadSoftwareInstallerPayload{ + Filename: "ipa_test.ipa", + TeamID: nil, + }, http.StatusOK, "") + + var newTeamResp3 teamResponse + s.DoJSON("POST", "/api/latest/fleet/teams", &createTeamRequest{TeamPayload: fleet.TeamPayload{Name: ptr.String("IPA Conflict Team 3")}}, http.StatusOK, &newTeamResp3) + team3 := newTeamResp3.Team + + s.DoJSON("GET", "/api/latest/fleet/vpp_tokens", &getVPPTokensRequest{}, http.StatusOK, &tokenResp) + s.DoJSON("PATCH", fmt.Sprintf("/api/latest/fleet/vpp_tokens/%d/teams", tokenResp.Tokens[0].ID), patchVPPTokensTeamsRequest{TeamIDs: []uint{team.ID, team2.ID, team3.ID, 0}}, http.StatusOK, &resPatchVPP) + + res = s.Do("POST", "/api/latest/fleet/software/app_store_apps", &addAppStoreAppRequest{ + TeamID: nil, + AppStoreID: "100", + Platform: "ios", + }, http.StatusConflict) + txt = extractServerErrorText(res.Body) + require.Contains(t, txt, "already has an installer available for the No team team.") +}