From 68ddaafac09281a66e23c2369cd15fccb99bb672 Mon Sep 17 00:00:00 2001 From: Martin Angers Date: Wed, 7 Jun 2023 13:29:36 -0400 Subject: [PATCH] Fix bug preventing gitops role from `fleetctl apply`ing macos setup assistant (and bootstrap package) (#12193) --- ...ssue-11449-allow-gitops-apply-mdm-features | 2 + cmd/fleetctl/apply_test.go | 216 ++++++++++++++---- cmd/fleetctl/testing_utils.go | 20 ++ docs/Contributing/API-for-contributors.md | 8 + docs/Using-Fleet/Permissions.md | 12 +- docs/Using-Fleet/REST-API.md | 13 +- ee/server/service/mdm.go | 8 +- ee/server/service/teams.go | 33 +-- server/authz/policy.rego | 16 ++ server/authz/policy_test.go | 90 ++++++++ server/fleet/apple_mdm.go | 2 +- server/fleet/service.go | 5 +- server/service/apple_mdm.go | 10 +- server/service/client.go | 68 +++--- server/service/client_mdm.go | 11 +- server/service/client_teams.go | 8 +- server/service/integration_enterprise_test.go | 20 +- server/service/mdm.go | 1 - server/service/teams.go | 11 +- server/service/teams_test.go | 14 +- 20 files changed, 442 insertions(+), 126 deletions(-) create mode 100644 changes/issue-11449-allow-gitops-apply-mdm-features diff --git a/changes/issue-11449-allow-gitops-apply-mdm-features b/changes/issue-11449-allow-gitops-apply-mdm-features new file mode 100644 index 0000000000..90d7c6d602 --- /dev/null +++ b/changes/issue-11449-allow-gitops-apply-mdm-features @@ -0,0 +1,2 @@ +* Fixed an issue preventing a user with the `gitops` role to apply some MDM settings via `fleetctl apply` (the `macos_setup_assistant` and `bootstrap_package` settings) +* Added a response payload to the `POST /api/latest/fleet/spec/teams` contributor API endpoint, it now returns an object with a `team_ids_by_name` key which maps team names with their corresponding id. diff --git a/cmd/fleetctl/apply_test.go b/cmd/fleetctl/apply_test.go index 83e8db4d6c..2fff5481f2 100644 --- a/cmd/fleetctl/apply_test.go +++ b/cmd/fleetctl/apply_test.go @@ -797,7 +797,6 @@ func TestApplyAsGitOps(t *testing.T) { return nil } - // Apply global config. currentAppConfig := &fleet.AppConfig{ OrgInfo: fleet.OrgInfo{ OrgName: "Fleet", @@ -817,6 +816,52 @@ func TestApplyAsGitOps(t *testing.T) { currentAppConfig = config return nil } + + savedTeam := &fleet.Team{ID: 123} + ds.TeamByNameFunc = func(ctx context.Context, name string) (*fleet.Team, error) { + if name == "Team1" { + return savedTeam, nil + } + return nil, errors.New("unexpected team name!") + } + ds.SaveTeamFunc = func(ctx context.Context, team *fleet.Team) (*fleet.Team, error) { + savedTeam = team + return team, nil + } + ds.TeamFunc = func(ctx context.Context, tid uint) (*fleet.Team, error) { + return savedTeam, nil + } + var teamEnrollSecrets []*fleet.EnrollSecret + ds.ApplyEnrollSecretsFunc = func(ctx context.Context, teamID *uint, secrets []*fleet.EnrollSecret) error { + if teamID == nil || *teamID != 123 { + return fmt.Errorf("unexpected data: %+v", teamID) + } + teamEnrollSecrets = secrets + return nil + } + ds.BatchSetMDMAppleProfilesFunc = func(ctx context.Context, teamID *uint, profiles []*fleet.MDMAppleConfigProfile) error { + return nil + } + ds.BulkSetPendingMDMAppleHostProfilesFunc = func(ctx context.Context, hostIDs, teamIDs, profileIDs []uint, hostUUIDs []string) error { + return nil + } + ds.GetMDMAppleSetupAssistantFunc = func(ctx context.Context, teamID *uint) (*fleet.MDMAppleSetupAssistant, error) { + return nil, ¬FoundError{} + } + ds.SetOrUpdateMDMAppleSetupAssistantFunc = func(ctx context.Context, asst *fleet.MDMAppleSetupAssistant) (*fleet.MDMAppleSetupAssistant, error) { + return asst, nil + } + ds.NewJobFunc = func(ctx context.Context, job *fleet.Job) (*fleet.Job, error) { + return job, nil + } + ds.GetMDMAppleBootstrapPackageMetaFunc = func(ctx context.Context, teamID uint) (*fleet.MDMAppleBootstrapPackage, error) { + return nil, ¬FoundError{} + } + ds.InsertMDMAppleBootstrapPackageFunc = func(ctx context.Context, bp *fleet.MDMAppleBootstrapPackage) error { + return nil + } + + // Apply global config. name := writeTmpYml(t, `--- apiVersion: v1 kind: config @@ -843,38 +888,78 @@ spec: assert.Equal(t, "[+] applied fleet config\n", runAppForTest(t, []string{"apply", "-f", name})) assert.True(t, currentAppConfig.Features.EnableHostUsers) - // Apply team config. - ds.TeamByNameFunc = func(ctx context.Context, name string) (*fleet.Team, error) { - if name == "Team1" { - return &fleet.Team{ID: 123}, nil - } - return nil, errors.New("unexpected team name!") - } - var savedTeam *fleet.Team - ds.SaveTeamFunc = func(ctx context.Context, team *fleet.Team) (*fleet.Team, error) { - savedTeam = team - return team, nil - } - var teamEnrollSecrets []*fleet.EnrollSecret - ds.ApplyEnrollSecretsFunc = func(ctx context.Context, teamID *uint, secrets []*fleet.EnrollSecret) error { - if teamID == nil || *teamID != 123 { - return fmt.Errorf("unexpected data: %+v", teamID) - } - teamEnrollSecrets = secrets - return nil - } - ds.BatchSetMDMAppleProfilesFunc = func(ctx context.Context, teamID *uint, profiles []*fleet.MDMAppleConfigProfile) error { - return nil - } - ds.BulkSetPendingMDMAppleHostProfilesFunc = func(ctx context.Context, hostIDs, teamIDs, profileIDs []uint, hostUUIDs []string) error { - return nil - } - mobileConfig := mobileconfigForTest("foo", "bar") mobileConfigPath := filepath.Join(t.TempDir(), "foo.mobileconfig") err = os.WriteFile(mobileConfigPath, mobileConfig, 0o644) require.NoError(t, err) + emptySetupAsst := writeTmpJSON(t, map[string]any{}) + + // Apply global config with custom setting and macos setup assistant. + name = writeTmpYml(t, fmt.Sprintf(`--- +apiVersion: v1 +kind: config +spec: + mdm: + macos_updates: + minimum_version: 10.10.10 + deadline: 2020-02-02 + macos_settings: + custom_settings: + - %s + macos_setup: + macos_setup_assistant: %s +`, mobileConfigPath, emptySetupAsst)) + assert.Equal(t, "[+] applied fleet config\n", runAppForTest(t, []string{"apply", "-f", name})) + // features left untouched, not provided + assert.True(t, currentAppConfig.Features.EnableHostUsers) + assert.Equal(t, fleet.MDM{ + EnabledAndConfigured: true, + MacOSSetup: fleet.MacOSSetup{ + MacOSSetupAssistant: optjson.SetString(emptySetupAsst), + }, + MacOSUpdates: fleet.MacOSUpdates{ + MinimumVersion: optjson.SetString("10.10.10"), + Deadline: optjson.SetString("2020-02-02"), + }, + MacOSSettings: fleet.MacOSSettings{ + CustomSettings: []string{mobileConfigPath}, + }, + }, currentAppConfig.MDM) + + // start a server to return the bootstrap package + srv, _ := serveMDMBootstrapPackage(t, "../../server/service/testdata/bootstrap-packages/signed.pkg", "signed.pkg") + + // Apply global config with bootstrap package + bootstrapURL := srv.URL + "/signed.pkg" + name = writeTmpYml(t, fmt.Sprintf(`--- +apiVersion: v1 +kind: config +spec: + mdm: + macos_setup: + bootstrap_package: %s +`, bootstrapURL)) + assert.Equal(t, "[+] applied fleet config\n", runAppForTest(t, []string{"apply", "-f", name})) + // features left untouched, not provided + assert.True(t, currentAppConfig.Features.EnableHostUsers) + // MDM settings left untouched except for the bootstrap package + assert.Equal(t, fleet.MDM{ + EnabledAndConfigured: true, + MacOSSetup: fleet.MacOSSetup{ + MacOSSetupAssistant: optjson.SetString(emptySetupAsst), + BootstrapPackage: optjson.SetString(bootstrapURL), + }, + MacOSUpdates: fleet.MacOSUpdates{ + MinimumVersion: optjson.SetString("10.10.10"), + Deadline: optjson.SetString("2020-02-02"), + }, + MacOSSettings: fleet.MacOSSettings{ + CustomSettings: []string{mobileConfigPath}, + }, + }, currentAppConfig.MDM) + + // Apply team config. name = writeTmpYml(t, fmt.Sprintf(` apiVersion: v1 kind: team @@ -913,6 +998,64 @@ spec: assert.True(t, ds.ApplyEnrollSecretsFuncInvoked) assert.True(t, ds.BatchSetMDMAppleProfilesFuncInvoked) + // add macos setup assistant to team + name = writeTmpYml(t, fmt.Sprintf(` +apiVersion: v1 +kind: team +spec: + team: + name: Team1 + mdm: + macos_setup: + macos_setup_assistant: %s +`, emptySetupAsst)) + require.Equal(t, "[+] applied 1 teams\n", runAppForTest(t, []string{"apply", "-f", name})) + require.True(t, ds.GetMDMAppleSetupAssistantFuncInvoked) + require.True(t, ds.SetOrUpdateMDMAppleSetupAssistantFuncInvoked) + require.True(t, ds.NewJobFuncInvoked) + // all left untouched, only setup assistant added + assert.Equal(t, fleet.TeamMDM{ + MacOSSettings: fleet.MacOSSettings{ + CustomSettings: []string{mobileConfigPath}, + EnableDiskEncryption: false, + }, + MacOSUpdates: fleet.MacOSUpdates{ + MinimumVersion: optjson.SetString("10.10.10"), + Deadline: optjson.SetString("1992-03-01"), + }, + MacOSSetup: fleet.MacOSSetup{ + MacOSSetupAssistant: optjson.SetString(emptySetupAsst), + }, + }, savedTeam.Config.MDM) + + // add bootstrap package to team + name = writeTmpYml(t, fmt.Sprintf(` +apiVersion: v1 +kind: team +spec: + team: + name: Team1 + mdm: + macos_setup: + bootstrap_package: %s +`, bootstrapURL)) + require.Equal(t, "[+] applied 1 teams\n", runAppForTest(t, []string{"apply", "-f", name})) + // all left untouched, only bootstrap package added + assert.Equal(t, fleet.TeamMDM{ + MacOSSettings: fleet.MacOSSettings{ + CustomSettings: []string{mobileConfigPath}, + EnableDiskEncryption: false, + }, + MacOSUpdates: fleet.MacOSUpdates{ + MinimumVersion: optjson.SetString("10.10.10"), + Deadline: optjson.SetString("1992-03-01"), + }, + MacOSSetup: fleet.MacOSSetup{ + MacOSSetupAssistant: optjson.SetString(emptySetupAsst), + BootstrapPackage: optjson.SetString(bootstrapURL), + }, + }, savedTeam.Config.MDM) + // Apply policies. var appliedPolicySpecs []*fleet.PolicySpec ds.ApplyPolicySpecsFunc = func(ctx context.Context, authorID uint, specs []*fleet.PolicySpec) error { @@ -1666,23 +1809,10 @@ spec: for _, c := range cases { t.Run(c.pkgName, func(t *testing.T) { - pkgBytes, err := os.ReadFile(filepath.Join("../../server/service/testdata/bootstrap-packages", c.pkgName)) - require.NoError(t, err) - - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Length", strconv.Itoa(len(pkgBytes))) - w.Header().Set("Content-Type", "application/octet-stream") - w.Header().Set("Content-Disposition", fmt.Sprintf(`attachment;filename="%s"`, c.pkgName)) - if n, err := w.Write(pkgBytes); err != nil { - require.NoError(t, err) - require.Equal(t, len(pkgBytes), n) - } - })) - defer srv.Close() - + srv, pkgLen := serveMDMBootstrapPackage(t, filepath.Join("../../server/service/testdata/bootstrap-packages", c.pkgName), c.pkgName) ds := setupServer(t, true) ds.InsertMDMAppleBootstrapPackageFunc = func(ctx context.Context, bp *fleet.MDMAppleBootstrapPackage) error { - require.Equal(t, len(bp.Bytes), len(pkgBytes)) + require.Equal(t, len(bp.Bytes), pkgLen) return nil } ds.GetMDMAppleBootstrapPackageMetaFunc = func(ctx context.Context, teamID uint) (*fleet.MDMAppleBootstrapPackage, error) { diff --git a/cmd/fleetctl/testing_utils.go b/cmd/fleetctl/testing_utils.go index 5ede46966b..1692a2e412 100644 --- a/cmd/fleetctl/testing_utils.go +++ b/cmd/fleetctl/testing_utils.go @@ -3,8 +3,11 @@ package main import ( "bytes" "context" + "fmt" + "net/http" "net/http/httptest" "os" + "strconv" "testing" "time" @@ -83,3 +86,20 @@ func runAppNoChecks(args []string) (*bytes.Buffer, error) { } func noopExitErrHandler(c *cli.Context, err error) {} + +func serveMDMBootstrapPackage(t *testing.T, pkgPath, pkgName string) (*httptest.Server, int) { + pkgBytes, err := os.ReadFile(pkgPath) + require.NoError(t, err) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Length", strconv.Itoa(len(pkgBytes))) + w.Header().Set("Content-Type", "application/octet-stream") + w.Header().Set("Content-Disposition", fmt.Sprintf(`attachment;filename="%s"`, pkgName)) + if n, err := w.Write(pkgBytes); err != nil { + require.NoError(t, err) + require.Equal(t, len(pkgBytes), n) + } + })) + t.Cleanup(srv.Close) + return srv, len(pkgBytes) +} diff --git a/docs/Contributing/API-for-contributors.md b/docs/Contributing/API-for-contributors.md index 3508d56049..a4512f1d60 100644 --- a/docs/Contributing/API-for-contributors.md +++ b/docs/Contributing/API-for-contributors.md @@ -1304,6 +1304,14 @@ If the `name` is not already associated with an existing team, this API route cr `Status: 200` +```json +{ + "team_ids_by_name": { + "Client Platform Engineering": 123 + } +} +``` + ### Apply labels Adds the supplied labels to Fleet. Each label requires the `name`, and `label_membership_type` properties. diff --git a/docs/Using-Fleet/Permissions.md b/docs/Using-Fleet/Permissions.md index a40462a062..4494d9811f 100644 --- a/docs/Using-Fleet/Permissions.md +++ b/docs/Using-Fleet/Permissions.md @@ -80,8 +80,10 @@ GitOps is an API-only and write-only role that can be used on CI/CD pipelines. | Edit [MDM settings for teams](https://fleetdm.com/docs/using-fleet/mdm-macos-settings) | | | | ✅ | ✅ | | Upload an EULA file for MDM automatic enrollment\* | | | | ✅ | | | View/download MDM macOS setup assistant\* | | | ✅ | ✅ | | -| Edit/upload MDM macOS setup assistant\* | | | ✅ | ✅ | | -| Enable/disable MDM macOS setup end user authentication\* | | | ✅ | ✅ | | +| Edit/upload MDM macOS setup assistant\* | | | ✅ | ✅ | ✅ | +| View metadata of MDM macOS bootstrap packages\* | | | ✅ | ✅ | | +| Edit/upload MDM macOS bootstrap packages\* | | | ✅ | ✅ | ✅ | +| Enable/disable MDM macOS setup end user authentication\* | | | ✅ | ✅ | ✅ | \* Applies only to Fleet Premium @@ -139,8 +141,10 @@ Users that are members of multiple teams can be assigned different roles for eac | View results of MDM commands executed on macOS hosts* | ✅ | ✅ | ✅ | ✅ | | | Edit [team MDM settings](https://fleetdm.com/docs/using-fleet/mdm-macos-settings) | | | | ✅ | ✅ | | View/download MDM macOS setup assistant | | | ✅ | ✅ | | -| Edit/upload MDM macOS setup assistant | | | ✅ | ✅ | | -| Enable/disable MDM macOS setup end user authentication | | | ✅ | ✅ | | +| Edit/upload MDM macOS setup assistant | | | ✅ | ✅ | ✅ | +| View metadata of MDM macOS bootstrap packages | | | ✅ | ✅ | | +| Edit/upload MDM macOS bootstrap packages | | | ✅ | ✅ | ✅ | +| Enable/disable MDM macOS setup end user authentication | | | ✅ | ✅ | ✅ | \* Applies only to [Fleet REST API](https://fleetdm.com/docs/using-fleet/rest-api) diff --git a/docs/Using-Fleet/REST-API.md b/docs/Using-Fleet/REST-API.md index c5b0b9f93c..035bfaa943 100644 --- a/docs/Using-Fleet/REST-API.md +++ b/docs/Using-Fleet/REST-API.md @@ -2085,7 +2085,7 @@ Returns the count of all hosts organized by status. `online_count` includes all "platform": "windows", "hosts_count": 12044 } - + ] } ``` @@ -4191,9 +4191,10 @@ Get information about a bootstrap package that was uploaded to Fleet. #### Parameters -| Name | Type | In | Description | -| ------- | ------ | --- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | -| team_id | string | url | **Required** The team id for the package. Zero (0) can be specified to get information about the bootstrap package for hosts that don't belong to a team. | +| Name | Type | In | Description | +| ------- | ------ | --- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | +| team_id | string | url | **Required** The team id for the package. Zero (0) can be specified to get information about the bootstrap package for hosts that don't belong to a team. | +| for_update | boolean | query | If set to `true`, the authorization will be for a `write` action instead of a `read`. Useful for the write-only `gitops` role when requesting the bootstrap metadata to check if the package needs to be replaced. | #### Example @@ -4335,7 +4336,7 @@ _Available in Fleet Premium_ -### Upload an EULA file +### Upload an EULA file _Available in Fleet Premium_ @@ -4374,7 +4375,7 @@ Content-Type: application/octet-stream `Status: 200` -### Get metadata about an EULA file +### Get metadata about an EULA file _Available in Fleet Premium_ diff --git a/ee/server/service/mdm.go b/ee/server/service/mdm.go index 91e671338b..15d87122f2 100644 --- a/ee/server/service/mdm.go +++ b/ee/server/service/mdm.go @@ -320,8 +320,12 @@ func (svc *Service) GetMDMAppleBootstrapPackageBytes(ctx context.Context, token return pkg, nil } -func (svc *Service) GetMDMAppleBootstrapPackageMetadata(ctx context.Context, teamID uint) (*fleet.MDMAppleBootstrapPackage, error) { - if err := svc.authz.Authorize(ctx, &fleet.MDMAppleBootstrapPackage{TeamID: teamID}, fleet.ActionRead); err != nil { +func (svc *Service) GetMDMAppleBootstrapPackageMetadata(ctx context.Context, teamID uint, forUpdate bool) (*fleet.MDMAppleBootstrapPackage, error) { + act := fleet.ActionRead + if forUpdate { + act = fleet.ActionWrite + } + if err := svc.authz.Authorize(ctx, &fleet.MDMAppleBootstrapPackage{TeamID: teamID}, act); err != nil { return nil, err } diff --git a/ee/server/service/teams.go b/ee/server/service/teams.go index 0b8c3f9f13..058eb10cf2 100644 --- a/ee/server/service/teams.go +++ b/ee/server/service/teams.go @@ -578,20 +578,20 @@ func (svc *Service) checkAuthorizationForTeams(ctx context.Context, specs []*fle return nil } -func (svc *Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, applyOpts fleet.ApplySpecOptions) error { +func (svc *Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, applyOpts fleet.ApplySpecOptions) (map[string]uint, error) { if len(specs) == 0 { setAuthCheckedOnPreAuthErr(ctx) // Nothing to do. - return nil + return map[string]uint{}, nil } if err := svc.checkAuthorizationForTeams(ctx, specs); err != nil { - return err + return nil, err } appConfig, err := svc.ds.AppConfig(ctx) if err != nil { - return err + return nil, err } appConfig.Obfuscate() @@ -612,11 +612,11 @@ func (svc *Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, // OK case ctxerr.Cause(err) == sql.ErrNoRows: if spec.Name == "" { - return fleet.NewInvalidArgumentError("name", "name may not be empty") + return nil, fleet.NewInvalidArgumentError("name", "name may not be empty") } create = true default: - return err + return nil, err } if len(spec.AgentOptions) > 0 && !bytes.Equal(spec.AgentOptions, jsonNull) { @@ -626,21 +626,21 @@ func (svc *Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, level.Info(svc.logger).Log("err", err, "msg", "force-apply team agent options with validation errors") } if !applyOpts.Force { - return ctxerr.Wrap(ctx, err, "validate agent options") + return nil, ctxerr.Wrap(ctx, err, "validate agent options") } } } if len(spec.Secrets) > fleet.MaxEnrollSecretsCount { - return ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("secrets", "too many secrets"), "validate secrets") + return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("secrets", "too many secrets"), "validate secrets") } if err := spec.MDM.MacOSUpdates.Validate(); err != nil { - return ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("macos_updates", err.Error())) + return nil, ctxerr.Wrap(ctx, fleet.NewInvalidArgumentError("macos_updates", err.Error())) } if create { team, err := svc.createTeamFromSpec(ctx, spec, appConfig, secrets, applyOpts.DryRun) if err != nil { - return ctxerr.Wrap(ctx, err, "creating team from spec") + return nil, ctxerr.Wrap(ctx, err, "creating team from spec") } details = append(details, fleet.TeamActivityDetail{ ID: team.ID, @@ -650,7 +650,7 @@ func (svc *Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, } if err := svc.editTeamFromSpec(ctx, team, spec, appConfig, secrets, applyOpts.DryRun); err != nil { - return ctxerr.Wrap(ctx, err, "editing team from spec") + return nil, ctxerr.Wrap(ctx, err, "editing team from spec") } details = append(details, fleet.TeamActivityDetail{ @@ -660,10 +660,15 @@ func (svc *Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, } if applyOpts.DryRun { - return nil + return nil, nil } + idsByName := make(map[string]uint, len(details)) if len(details) > 0 { + for _, tm := range details { + idsByName[tm.Name] = tm.ID + } + if err := svc.ds.NewActivity( ctx, authz.UserFromContext(ctx), @@ -671,10 +676,10 @@ func (svc *Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, Teams: details, }, ); err != nil { - return ctxerr.Wrap(ctx, err, "create activity for team spec") + return nil, ctxerr.Wrap(ctx, err, "create activity for team spec") } } - return nil + return idsByName, nil } func (svc *Service) createTeamFromSpec( diff --git a/server/authz/policy.rego b/server/authz/policy.rego index f052439b6b..a604fce2b3 100644 --- a/server/authz/policy.rego +++ b/server/authz/policy.rego @@ -705,6 +705,13 @@ allow { action == [read, write][_] } +# Global gitops can write bootstrap packages. +allow { + object.type == "mdm_apple_bootstrap_package" + subject.global_role == gitops + action == write +} + # Team admins and maintainers can read and write bootstrap packages on their teams. allow { not is_null(object.team_id) @@ -714,6 +721,15 @@ allow { action == [read, write][_] } +# Team gitops can write bootstrap packages on their teams. +allow { + not is_null(object.team_id) + object.team_id != 0 + object.type == "mdm_apple_bootstrap_package" + team_role(subject, object.team_id) == gitops + action == write +} + ## # MDM Apple Setup Assistant ## diff --git a/server/authz/policy_test.go b/server/authz/policy_test.go index 4fa2960ebe..b24f6d448c 100644 --- a/server/authz/policy_test.go +++ b/server/authz/policy_test.go @@ -1480,6 +1480,96 @@ func TestAuthorizeMDMAppleSetupAssistant(t *testing.T) { }) } +func TestAuthorizeMDMAppleBootstrapPackage(t *testing.T) { + t.Parallel() + + globalSettings := &fleet.MDMAppleBootstrapPackage{} + team1Settings := &fleet.MDMAppleBootstrapPackage{ + TeamID: 1, + } + runTestCases(t, []authTestCase{ + {user: test.UserNoRoles, object: globalSettings, action: write, allow: false}, + {user: test.UserNoRoles, object: globalSettings, action: read, allow: false}, + {user: test.UserNoRoles, object: team1Settings, action: write, allow: false}, + {user: test.UserNoRoles, object: team1Settings, action: read, allow: false}, + + {user: test.UserAdmin, object: globalSettings, action: write, allow: true}, + {user: test.UserAdmin, object: globalSettings, action: read, allow: true}, + {user: test.UserAdmin, object: team1Settings, action: write, allow: true}, + {user: test.UserAdmin, object: team1Settings, action: read, allow: true}, + + {user: test.UserMaintainer, object: globalSettings, action: write, allow: true}, + {user: test.UserMaintainer, object: globalSettings, action: read, allow: true}, + {user: test.UserMaintainer, object: team1Settings, action: write, allow: true}, + {user: test.UserMaintainer, object: team1Settings, action: read, allow: true}, + + {user: test.UserObserver, object: globalSettings, action: write, allow: false}, + {user: test.UserObserver, object: globalSettings, action: read, allow: false}, + {user: test.UserObserver, object: team1Settings, action: write, allow: false}, + {user: test.UserObserver, object: team1Settings, action: read, allow: false}, + + {user: test.UserObserverPlus, object: globalSettings, action: write, allow: false}, + {user: test.UserObserverPlus, object: globalSettings, action: read, allow: false}, + {user: test.UserObserverPlus, object: team1Settings, action: write, allow: false}, + {user: test.UserObserverPlus, object: team1Settings, action: read, allow: false}, + + {user: test.UserGitOps, object: globalSettings, action: write, allow: true}, + {user: test.UserGitOps, object: globalSettings, action: read, allow: false}, + {user: test.UserGitOps, object: team1Settings, action: write, allow: true}, + {user: test.UserGitOps, object: team1Settings, action: read, allow: false}, + + {user: test.UserTeamAdminTeam1, object: globalSettings, action: write, allow: false}, + {user: test.UserTeamAdminTeam1, object: globalSettings, action: read, allow: false}, + {user: test.UserTeamAdminTeam1, object: team1Settings, action: write, allow: true}, + {user: test.UserTeamAdminTeam1, object: team1Settings, action: read, allow: true}, + + {user: test.UserTeamAdminTeam2, object: globalSettings, action: write, allow: false}, + {user: test.UserTeamAdminTeam2, object: globalSettings, action: read, allow: false}, + {user: test.UserTeamAdminTeam2, object: team1Settings, action: write, allow: false}, + {user: test.UserTeamAdminTeam2, object: team1Settings, action: read, allow: false}, + + {user: test.UserTeamMaintainerTeam1, object: globalSettings, action: write, allow: false}, + {user: test.UserTeamMaintainerTeam1, object: globalSettings, action: read, allow: false}, + {user: test.UserTeamMaintainerTeam1, object: team1Settings, action: write, allow: true}, + {user: test.UserTeamMaintainerTeam1, object: team1Settings, action: read, allow: true}, + + {user: test.UserTeamMaintainerTeam2, object: globalSettings, action: write, allow: false}, + {user: test.UserTeamMaintainerTeam2, object: globalSettings, action: read, allow: false}, + {user: test.UserTeamMaintainerTeam2, object: team1Settings, action: write, allow: false}, + {user: test.UserTeamMaintainerTeam2, object: team1Settings, action: read, allow: false}, + + {user: test.UserTeamObserverTeam1, object: globalSettings, action: write, allow: false}, + {user: test.UserTeamObserverTeam1, object: globalSettings, action: read, allow: false}, + {user: test.UserTeamObserverTeam1, object: team1Settings, action: write, allow: false}, + {user: test.UserTeamObserverTeam1, object: team1Settings, action: read, allow: false}, + + {user: test.UserTeamObserverTeam2, object: globalSettings, action: write, allow: false}, + {user: test.UserTeamObserverTeam2, object: globalSettings, action: read, allow: false}, + {user: test.UserTeamObserverTeam2, object: team1Settings, action: write, allow: false}, + {user: test.UserTeamObserverTeam2, object: team1Settings, action: read, allow: false}, + + {user: test.UserTeamObserverPlusTeam1, object: globalSettings, action: write, allow: false}, + {user: test.UserTeamObserverPlusTeam1, object: globalSettings, action: read, allow: false}, + {user: test.UserTeamObserverPlusTeam1, object: team1Settings, action: write, allow: false}, + {user: test.UserTeamObserverPlusTeam1, object: team1Settings, action: read, allow: false}, + + {user: test.UserTeamObserverPlusTeam2, object: globalSettings, action: write, allow: false}, + {user: test.UserTeamObserverPlusTeam2, object: globalSettings, action: read, allow: false}, + {user: test.UserTeamObserverPlusTeam2, object: team1Settings, action: write, allow: false}, + {user: test.UserTeamObserverPlusTeam2, object: team1Settings, action: read, allow: false}, + + {user: test.UserTeamGitOpsTeam1, object: globalSettings, action: write, allow: false}, + {user: test.UserTeamGitOpsTeam1, object: globalSettings, action: read, allow: false}, + {user: test.UserTeamGitOpsTeam1, object: team1Settings, action: write, allow: true}, + {user: test.UserTeamGitOpsTeam1, object: team1Settings, action: read, allow: false}, + + {user: test.UserTeamGitOpsTeam2, object: globalSettings, action: write, allow: false}, + {user: test.UserTeamGitOpsTeam2, object: globalSettings, action: read, allow: false}, + {user: test.UserTeamGitOpsTeam2, object: team1Settings, action: write, allow: false}, + {user: test.UserTeamGitOpsTeam2, object: team1Settings, action: read, allow: false}, + }) +} + func assertAuthorized(t *testing.T, user *fleet.User, object, action interface{}) { t.Helper() diff --git a/server/fleet/apple_mdm.go b/server/fleet/apple_mdm.go index ca4ac6362d..7a9ff12953 100644 --- a/server/fleet/apple_mdm.go +++ b/server/fleet/apple_mdm.go @@ -475,7 +475,7 @@ type MDMAppleSetupPayload struct { // AuthzType implements authz.AuthzTyper. func (p MDMAppleSetupPayload) AuthzType() string { - return "mdm_apple_settings" // TODO: add mdm_apple_setup to rego? + return "mdm_apple_settings" } // HostDEPAssignment represents a row in the host_dep_assignments table diff --git a/server/fleet/service.go b/server/fleet/service.go index 35edfc4098..0b1e6c57d1 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -491,7 +491,8 @@ type Service interface { // ModifyTeamEnrollSecrets modifies enroll secrets for a team. ModifyTeamEnrollSecrets(ctx context.Context, teamID uint, secrets []EnrollSecret) ([]*EnrollSecret, error) // ApplyTeamSpecs applies the changes for each team as defined in the specs. - ApplyTeamSpecs(ctx context.Context, specs []*TeamSpec, applyOpts ApplySpecOptions) error + // On success, it returns the mapping of team names to team ids. + ApplyTeamSpecs(ctx context.Context, specs []*TeamSpec, applyOpts ApplySpecOptions) (map[string]uint, error) // ///////////////////////////////////////////////////////////////////////////// // ActivitiesService @@ -699,7 +700,7 @@ type Service interface { GetMDMAppleBootstrapPackageBytes(ctx context.Context, token string) (*MDMAppleBootstrapPackage, error) - GetMDMAppleBootstrapPackageMetadata(ctx context.Context, teamID uint) (*MDMAppleBootstrapPackage, error) + GetMDMAppleBootstrapPackageMetadata(ctx context.Context, teamID uint, forUpdate bool) (*MDMAppleBootstrapPackage, error) DeleteMDMAppleBootstrapPackage(ctx context.Context, teamID *uint) error diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index acc00d2b53..d3d599837d 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -1840,6 +1840,12 @@ func (svc *Service) GetMDMAppleBootstrapPackageBytes(ctx context.Context, token type bootstrapPackageMetadataRequest struct { TeamID uint `url:"team_id"` + + // ForUpdate is used to indicate that the authorization should be for a + // "write" instead of a "read", this is needed specifically for the gitops + // user which is a write-only user, but needs to call this endpoint to check + // if it needs to upload the bootstrap package (if the hashes are different). + ForUpdate bool `query:"for_update,optional"` } type bootstrapPackageMetadataResponse struct { @@ -1851,14 +1857,14 @@ func (r bootstrapPackageMetadataResponse) error() error { return r.Err } func bootstrapPackageMetadataEndpoint(ctx context.Context, request interface{}, svc fleet.Service) (errorer, error) { req := request.(*bootstrapPackageMetadataRequest) - meta, err := svc.GetMDMAppleBootstrapPackageMetadata(ctx, req.TeamID) + meta, err := svc.GetMDMAppleBootstrapPackageMetadata(ctx, req.TeamID, req.ForUpdate) if err != nil { return bootstrapPackageMetadataResponse{Err: err}, nil } return bootstrapPackageMetadataResponse{MDMAppleBootstrapPackage: meta}, nil } -func (svc *Service) GetMDMAppleBootstrapPackageMetadata(ctx context.Context, teamID uint) (*fleet.MDMAppleBootstrapPackage, error) { +func (svc *Service) GetMDMAppleBootstrapPackageMetadata(ctx context.Context, teamID uint, forUpdate bool) (*fleet.MDMAppleBootstrapPackage, error) { // skipauth: No authorization check needed due to implementation returning // only license error. svc.authz.SkipAuthorization(ctx) diff --git a/server/service/client.go b/server/service/client.go index c77d72b897..8ecabb0fea 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -16,6 +16,7 @@ import ( "github.com/fleetdm/fleet/v4/pkg/spec" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/fleet" + kithttp "github.com/go-kit/kit/transport/http" ) // Client is used to consume Fleet APIs from Go code @@ -223,28 +224,40 @@ func (c *Client) authenticatedRequest(params interface{}, verb string, path stri } func (c *Client) CheckMDMEnabled() error { - appCfg, err := c.GetAppConfig() - if err != nil { - return err - } - if !appCfg.MDM.EnabledAndConfigured { - return errors.New("MDM features aren't turned on. Use `fleetctl generate mdm-apple` and then `fleet serve` with `mdm` configuration to turn on MDM features.") - } - return nil + return c.runAppConfigChecks(func(ac *fleet.EnrichedAppConfig) error { + if !ac.MDM.EnabledAndConfigured { + return errors.New("MDM features aren't turned on. Use `fleetctl generate mdm-apple` and then `fleet serve` with `mdm` configuration to turn on MDM features.") + } + return nil + }) } func (c *Client) CheckPremiumMDMEnabled() error { + return c.runAppConfigChecks(func(ac *fleet.EnrichedAppConfig) error { + if ac.License == nil || !ac.License.IsPremium() { + return errors.New("missing or invalid license") + } + if !ac.MDM.EnabledAndConfigured { + return errors.New("MDM features aren't turned on. Use `fleetctl generate mdm-apple` and then `fleet serve` with `mdm` configuration to turn on MDM features.") + } + return nil + }) +} + +func (c *Client) runAppConfigChecks(fn func(ac *fleet.EnrichedAppConfig) error) error { appCfg, err := c.GetAppConfig() if err != nil { + var sce kithttp.StatusCoder + if errors.As(err, &sce) && sce.StatusCode() == http.StatusForbidden { + // do not return an error, user may not have permission to read app + // config (e.g. gitops) and those appconfig checks are just convenience + // to avoid the round-trip with potentially large payload to the server. + // Those will still be validated with the actual API call. + return nil + } return err } - if appCfg.License == nil || !appCfg.License.IsPremium() { - return errors.New("missing or invalid license") - } - if !appCfg.MDM.EnabledAndConfigured { - return errors.New("MDM features aren't turned on. Use `fleetctl generate mdm-apple` and then `fleet serve` with `mdm` configuration to turn on MDM features.") - } - return nil + return fn(appCfg) } // ApplyGroup applies the given spec group to Fleet. @@ -413,7 +426,8 @@ func (c *Client) ApplyGroup( // Next, apply the teams specs before saving the profiles, so that any // non-existing team gets created. - if err := c.ApplyTeams(specs.Teams, opts); err != nil { + teamIDsByName, err := c.ApplyTeams(specs.Teams, opts) + if err != nil { return fmt.Errorf("applying teams: %w", err) } @@ -425,23 +439,15 @@ func (c *Client) ApplyGroup( } } if len(tmBootstrapPackages)+len(tmMacSetupAssistants) > 0 && !opts.DryRun { - // TODO: we need to chat an define on a better way to do this, maybe make - // the endpoints support both id/name? have separate endpoints? Or make - // the apply team spec endpoint return team ids? - tms, err := c.ListTeams("") - if err != nil { - return err - } - - for _, tm := range tms { - if bp, ok := tmBootstrapPackages[tm.Name]; ok { - if err := c.EnsureBootstrapPackage(bp, tm.ID); err != nil { - return fmt.Errorf("uploading bootstrap package for team %q: %w", tm.Name, err) + for tmName, tmID := range teamIDsByName { + if bp, ok := tmBootstrapPackages[tmName]; ok { + if err := c.EnsureBootstrapPackage(bp, tmID); err != nil { + return fmt.Errorf("uploading bootstrap package for team %q: %w", tmName, err) } } - if b, ok := tmMacSetupAssistants[tm.Name]; ok { - if err := c.uploadMacOSSetupAssistant(b, &tm.ID, tmMacSetup[tm.Name].MacOSSetupAssistant.Value); err != nil { - return fmt.Errorf("uploading macOS setup assistant for team %q: %w", tm.Name, err) + if b, ok := tmMacSetupAssistants[tmName]; ok { + if err := c.uploadMacOSSetupAssistant(b, &tmID, tmMacSetup[tmName].MacOSSetupAssistant.Value); err != nil { + return fmt.Errorf("uploading macOS setup assistant for team %q: %w", tmName, err) } } } diff --git a/server/service/client_mdm.go b/server/service/client_mdm.go index 7a69c8939d..e627eb53df 100644 --- a/server/service/client_mdm.go +++ b/server/service/client_mdm.go @@ -50,11 +50,16 @@ func (c *Client) RequestAppleCSR(email, org string) (*fleet.AppleCSR, error) { return responseBody.AppleCSR, err } -func (c *Client) GetBootstrapPackageMetadata(teamID uint) (*fleet.MDMAppleBootstrapPackage, error) { +func (c *Client) GetBootstrapPackageMetadata(teamID uint, forUpdate bool) (*fleet.MDMAppleBootstrapPackage, error) { verb, path := "GET", fmt.Sprintf("/api/latest/fleet/mdm/apple/bootstrap/%d/metadata", teamID) request := bootstrapPackageMetadataRequest{} var responseBody bootstrapPackageMetadataResponse - err := c.authenticatedRequest(request, verb, path, &responseBody) + var err error + if forUpdate { + err = c.authenticatedRequestWithQuery(request, verb, path, &responseBody, "for_update=true") + } else { + err = c.authenticatedRequest(request, verb, path, &responseBody) + } return responseBody.MDMAppleBootstrapPackage, err } @@ -110,7 +115,7 @@ func (c *Client) UploadBootstrapPackage(pkg *fleet.MDMAppleBootstrapPackage) err func (c *Client) EnsureBootstrapPackage(bp *fleet.MDMAppleBootstrapPackage, teamID uint) error { isFirstTime := false - oldMeta, err := c.GetBootstrapPackageMetadata(teamID) + oldMeta, err := c.GetBootstrapPackageMetadata(teamID, true) if err != nil { // not found is OK, it means this is our first time uploading a package if !errors.Is(err, notFoundErr{}) { diff --git a/server/service/client_teams.go b/server/service/client_teams.go index 34efe2b816..959de7c5b8 100644 --- a/server/service/client_teams.go +++ b/server/service/client_teams.go @@ -42,10 +42,14 @@ func (c *Client) DeleteTeam(teamID uint) error { // ApplyTeams sends the list of Teams to be applied to the // Fleet instance. -func (c *Client) ApplyTeams(specs []json.RawMessage, opts fleet.ApplySpecOptions) error { +func (c *Client) ApplyTeams(specs []json.RawMessage, opts fleet.ApplySpecOptions) (map[string]uint, error) { verb, path := "POST", "/api/latest/fleet/spec/teams" var responseBody applyTeamSpecsResponse - return c.authenticatedRequestWithQuery(map[string]interface{}{"specs": specs}, verb, path, &responseBody, opts.RawQuery()) + err := c.authenticatedRequestWithQuery(map[string]interface{}{"specs": specs}, verb, path, &responseBody, opts.RawQuery()) + if err != nil { + return nil, err + } + return responseBody.TeamIDsByName, nil } // ApplyTeamProfiles sends the list of profiles to be applied for the specified diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index b7c9478ef5..1e95016eab 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -108,10 +108,13 @@ func (s *integrationEnterpriseTestSuite) TestTeamSpecs() { }, }, } - s.Do("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK) + var applyResp applyTeamSpecsResponse + s.DoJSON("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK, &applyResp) + require.Len(t, applyResp.TeamIDsByName, 1) team, err := s.ds.TeamByName(context.Background(), teamName) require.NoError(t, err) + require.Equal(t, applyResp.TeamIDsByName[teamName], team.ID) assert.Len(t, team.Secrets, 1) require.JSONEq(t, string(agentOpts), string(*team.Config.AgentOptions)) require.Equal(t, fleet.Features{ @@ -197,7 +200,10 @@ func (s *integrationEnterpriseTestSuite) TestTeamSpecs() { }, }, } - s.Do("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK, "dry_run", "true") + applyResp = applyTeamSpecsResponse{} + s.DoJSON("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK, &applyResp, "dry_run", "true") + // dry-run never returns id to name mappings as it may not have them + require.Empty(t, applyResp.TeamIDsByName) // dry-run with macos disk encryption set to true teamSpecs = map[string]any{ @@ -334,7 +340,9 @@ func (s *integrationEnterpriseTestSuite) TestTeamSpecs() { }, }, } - s.Do("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK) + applyResp = applyTeamSpecsResponse{} + s.DoJSON("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK, &applyResp) + require.Len(t, applyResp.TeamIDsByName, 1) teams, err = s.ds.ListTeams(context.Background(), fleet.TeamFilter{User: user}, fleet.ListOptions{}) require.NoError(t, err) @@ -342,6 +350,7 @@ func (s *integrationEnterpriseTestSuite) TestTeamSpecs() { team, err = s.ds.TeamByName(context.Background(), "team2") require.NoError(t, err) + require.Equal(t, applyResp.TeamIDsByName["team2"], team.ID) appConfig, err := s.ds.AppConfig(context.Background()) require.NoError(t, err) @@ -364,10 +373,13 @@ func (s *integrationEnterpriseTestSuite) TestTeamSpecs() { }, }, } - s.Do("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK) + applyResp = applyTeamSpecsResponse{} + s.DoJSON("POST", "/api/latest/fleet/spec/teams", teamSpecs, http.StatusOK, &applyResp) + require.Len(t, applyResp.TeamIDsByName, 1) team, err = s.ds.TeamByName(context.Background(), "team2") require.NoError(t, err) + require.Equal(t, applyResp.TeamIDsByName["team2"], team.ID) require.Len(t, team.Secrets, 1) assert.Equal(t, "ABC", team.Secrets[0].Secret) diff --git a/server/service/mdm.go b/server/service/mdm.go index caba0d9194..a720cba2bd 100644 --- a/server/service/mdm.go +++ b/server/service/mdm.go @@ -208,7 +208,6 @@ func (svc *Service) VerifyMDMAppleConfigured(ctx context.Context) error { // skipauth: Authorization is currently for user endpoints only. svc.authz.SkipAuthorization(ctx) return fleet.ErrMDMNotConfigured - } return nil diff --git a/server/service/teams.go b/server/service/teams.go index 05df1abbec..ee9dd0da04 100644 --- a/server/service/teams.go +++ b/server/service/teams.go @@ -213,7 +213,8 @@ func (req *applyTeamSpecsRequest) DecodeBody(ctx context.Context, r io.Reader) e } type applyTeamSpecsResponse struct { - Err error `json:"error,omitempty"` + Err error `json:"error,omitempty"` + TeamIDsByName map[string]uint `json:"team_ids_by_name,omitempty"` } func (r applyTeamSpecsResponse) error() error { return r.Err } @@ -230,22 +231,22 @@ func applyTeamSpecsEndpoint(ctx context.Context, request interface{}, svc fleet. } } - err := svc.ApplyTeamSpecs(ctx, actualSpecs, fleet.ApplySpecOptions{ + idsByName, err := svc.ApplyTeamSpecs(ctx, actualSpecs, fleet.ApplySpecOptions{ Force: req.Force, DryRun: req.DryRun, }) if err != nil { return applyTeamSpecsResponse{Err: err}, nil } - return applyTeamSpecsResponse{}, nil + return applyTeamSpecsResponse{TeamIDsByName: idsByName}, nil } -func (svc Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, applyOpts fleet.ApplySpecOptions) error { +func (svc Service) ApplyTeamSpecs(ctx context.Context, specs []*fleet.TeamSpec, applyOpts fleet.ApplySpecOptions) (map[string]uint, error) { // skipauth: No authorization check needed due to implementation returning // only license error. svc.authz.SkipAuthorization(ctx) - return fleet.ErrMissingLicense + return nil, fleet.ErrMissingLicense } //////////////////////////////////////////////////////////////////////////////// diff --git a/server/service/teams_test.go b/server/service/teams_test.go index 205e10079b..73864f912b 100644 --- a/server/service/teams_test.go +++ b/server/service/teams_test.go @@ -188,7 +188,7 @@ func TestTeamAuth(t *testing.T) { _, err = svc.ModifyTeamEnrollSecrets(ctx, 1, []fleet.EnrollSecret{{Secret: "newteamsecret", CreatedAt: time.Now()}}) checkAuthErr(t, tt.shouldFailTeamSecretsWrite, err) - err = svc.ApplyTeamSpecs(ctx, []*fleet.TeamSpec{{Name: "team1"}}, fleet.ApplySpecOptions{}) + _, err = svc.ApplyTeamSpecs(ctx, []*fleet.TeamSpec{{Name: "team1"}}, fleet.ApplySpecOptions{}) checkAuthErr(t, tt.shouldFailTeamWrite, err) }) } @@ -282,7 +282,7 @@ func TestApplyTeamSpecs(t *testing.T) { return nil } - err := svc.ApplyTeamSpecs(ctx, []*fleet.TeamSpec{{Name: "team1", Features: tt.spec}}, fleet.ApplySpecOptions{}) + _, err := svc.ApplyTeamSpecs(ctx, []*fleet.TeamSpec{{Name: "team1", Features: tt.spec}}, fleet.ApplySpecOptions{}) require.NoError(t, err) }) } @@ -350,11 +350,11 @@ func TestApplyTeamSpecs(t *testing.T) { for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { ds.TeamByNameFunc = func(ctx context.Context, name string) (*fleet.Team, error) { - return &fleet.Team{Config: fleet.TeamConfig{Features: tt.old}}, nil + return &fleet.Team{ID: 123, Config: fleet.TeamConfig{Features: tt.old}}, nil } ds.SaveTeamFunc = func(ctx context.Context, team *fleet.Team) (*fleet.Team, error) { - return &fleet.Team{}, nil + return &fleet.Team{ID: 123}, nil } ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails) error { @@ -363,8 +363,10 @@ func TestApplyTeamSpecs(t *testing.T) { return nil } - err := svc.ApplyTeamSpecs(ctx, []*fleet.TeamSpec{{Name: "team1", Features: tt.spec}}, fleet.ApplySpecOptions{}) + idsByTeam, err := svc.ApplyTeamSpecs(ctx, []*fleet.TeamSpec{{Name: "team1", Features: tt.spec}}, fleet.ApplySpecOptions{}) require.NoError(t, err) + require.Len(t, idsByTeam, 1) + require.Equal(t, uint(123), idsByTeam["team1"]) }) } }) @@ -383,7 +385,7 @@ func TestApplyTeamSpecsErrorInTeamByName(t *testing.T) { } authzctx := &authz_ctx.AuthorizationContext{} ctx = authz_ctx.NewContext(ctx, authzctx) - err := svc.ApplyTeamSpecs(ctx, []*fleet.TeamSpec{{Name: "Foo"}}, fleet.ApplySpecOptions{}) + _, err := svc.ApplyTeamSpecs(ctx, []*fleet.TeamSpec{{Name: "Foo"}}, fleet.ApplySpecOptions{}) require.Error(t, err) az, ok := authz_ctx.FromContext(ctx) require.True(t, ok)