diff --git a/cmd/fleetctl/gitops_test.go b/cmd/fleetctl/gitops_test.go index 241a6752f7..46a172bb0b 100644 --- a/cmd/fleetctl/gitops_test.go +++ b/cmd/fleetctl/gitops_test.go @@ -198,8 +198,9 @@ func TestGitOpsBasicGlobalPremium(t *testing.T) { license := &fleet.LicenseInfo{Tier: fleet.TierPremium, Expiration: time.Now().Add(24 * time.Hour)} _, ds := runServerWithMockedDS( t, &service.TestServerOpts{ - License: license, - KeyValueStore: newMemKeyValueStore(), + License: license, + KeyValueStore: newMemKeyValueStore(), + EnableSCEPProxy: true, }, ) @@ -289,6 +290,12 @@ queries: policies: agent_options: org_settings: + integrations: + ndes_scep_proxy: + url: https://ndes.example.com/scep + admin_url: https://ndes.example.com/admin + username: ndes_user + password: ndes_password server_settings: server_url: $FLEET_SERVER_URL org_info: @@ -312,6 +319,8 @@ software: assert.Equal(t, orgName, savedAppConfig.OrgInfo.OrgName) assert.Equal(t, fleetServerURL, savedAppConfig.ServerSettings.ServerURL) assert.Empty(t, enrolledSecrets) + assert.True(t, savedAppConfig.Integrations.NDESSCEPProxy.Valid) + assert.Equal(t, "https://ndes.example.com/scep", savedAppConfig.Integrations.NDESSCEPProxy.Value.URL) } func TestGitOpsBasicTeam(t *testing.T) { diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index 10bafd76da..f054e38008 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -167,6 +167,7 @@ func ExpandEnv(s string) (string, error) { } s = escapeString(s, preventEscapingPrefix) + s = escapeFleetVar(s, preventEscapingPrefix) var err *multierror.Error s = os.Expand(s, func(env string) string { if strings.HasPrefix(env, preventEscapingPrefix) { @@ -203,3 +204,11 @@ func escapeString(s string, preventEscapingPrefix string) string { return strings.Repeat("\\", (len(match)/2)-1) + "$" + preventEscapingPrefix }) } + +var escapeFleetVarPattern = regexp.MustCompile(`(\$FLEET_VAR_\w+)|(\${FLEET_VAR_\w+})`) + +func escapeFleetVar(s string, preventEscapingPrefix string) string { + return escapeFleetVarPattern.ReplaceAllStringFunc(s, func(match string) string { + return strings.ReplaceAll(match, "$", "$"+preventEscapingPrefix) + }) +} diff --git a/pkg/spec/spec_test.go b/pkg/spec/spec_test.go index e3036baa16..ac9f92671c 100644 --- a/pkg/spec/spec_test.go +++ b/pkg/spec/spec_test.go @@ -156,6 +156,7 @@ func TestExpandEnv(t *testing.T) { checkErr func(error) }{ {map[string]string{"foo": "1"}, `$foo`, `1`, nil}, + {map[string]string{"foo": "1"}, `$foo $FLEET_VAR_BAR ${FLEET_VAR_BAR}x ${foo}`, `1 $FLEET_VAR_BAR ${FLEET_VAR_BAR}x 1`, nil}, {map[string]string{"foo": ""}, `$foo`, ``, nil}, {map[string]string{"foo": "", "bar": "", "zoo": ""}, `$foo${bar}$zoo`, ``, nil}, {map[string]string{}, `$foo`, ``, checkMultiErrors("environment variable \"foo\" not set")}, @@ -177,11 +178,13 @@ func TestExpandEnv(t *testing.T) { {map[string]string{"foo": ""}, `${foo}var`, `var`, nil}, {map[string]string{"foo": "", "$": "2"}, `${$}${foo}var`, `2var`, nil}, {map[string]string{}, `${foo}var`, ``, checkMultiErrors("environment variable \"foo\" not set")}, - {map[string]string{}, `foo PREVENT_ESCAPING_bar`, `foo PREVENT_ESCAPING_bar`, nil}, // nothing to replace + {map[string]string{}, `foo PREVENT_ESCAPING_bar $ FLEET_VAR_`, `foo PREVENT_ESCAPING_bar $ FLEET_VAR_`, nil}, // nothing to replace + {map[string]string{"foo": "BAR"}, `\$FLEET_VAR_$foo \${FLEET_VAR_$foo} \${FLEET_VAR_${foo}2}`, + `$FLEET_VAR_BAR ${FLEET_VAR_BAR} ${FLEET_VAR_BAR2}`, nil}, // nested variables } { os.Clearenv() for k, v := range tc.environment { - os.Setenv(k, v) + _ = os.Setenv(k, v) } result, err := ExpandEnv(tc.s) if tc.checkErr == nil { diff --git a/server/service/appconfig.go b/server/service/appconfig.go index c97fbaa7a7..ba93c48f0d 100644 --- a/server/service/appconfig.go +++ b/server/service/appconfig.go @@ -355,7 +355,9 @@ func (svc *Service) ModifyAppConfig(ctx context.Context, p []byte, applyOpts fle case !newAppConfig.Integrations.NDESSCEPProxy.Valid: // User is explicitly clearing this setting appConfig.Integrations.NDESSCEPProxy.Valid = false - ndesStatus = ndesStatusDeleted + if oldAppConfig.Integrations.NDESSCEPProxy.Valid { + ndesStatus = ndesStatusDeleted + } default: // User is updating the setting appConfig.Integrations.NDESSCEPProxy.Value.URL = fleet.Preprocess(newAppConfig.Integrations.NDESSCEPProxy.Value.URL) diff --git a/server/service/appconfig_test.go b/server/service/appconfig_test.go index f50db4cfcf..44d81c7566 100644 --- a/server/service/appconfig_test.go +++ b/server/service/appconfig_test.go @@ -1593,6 +1593,7 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { } ` // First, dry run. + appConfig.Integrations.NDESSCEPProxy.Valid = true ac, err = svc.ModifyAppConfig(ctx, []byte(payload), fleet.ApplySpecOptions{DryRun: true}) require.NoError(t, err) assert.False(t, ac.Integrations.NDESSCEPProxy.Valid) @@ -1605,6 +1606,7 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { ds.NewActivityFuncInvoked = false // Second, real run. + appConfig.Integrations.NDESSCEPProxy.Valid = true ds.NewActivityFunc = func(ctx context.Context, user *fleet.User, activity fleet.ActivityDetails, details []byte, createdAt time.Time) error { assert.IsType(t, fleet.ActivityDeletedNDESSCEPProxy{}, activity) @@ -1621,9 +1623,23 @@ func TestModifyAppConfigForNDESSCEPProxy(t *testing.T) { assert.False(t, validateNDESSCEPURLCalled) assert.False(t, validateNDESSCEPAdminURLCalled) assert.True(t, ds.HardDeleteMDMConfigAssetFuncInvoked) + ds.HardDeleteMDMConfigAssetFuncInvoked = false assert.True(t, ds.NewActivityFuncInvoked) ds.NewActivityFuncInvoked = false + // Deleting again should be a no-op + appConfig.Integrations.NDESSCEPProxy.Valid = false + ac, err = svc.ModifyAppConfig(ctx, []byte(payload), fleet.ApplySpecOptions{}) + require.NoError(t, err) + assert.False(t, ac.Integrations.NDESSCEPProxy.Valid) + assert.False(t, appConfig.Integrations.NDESSCEPProxy.Valid) + assert.False(t, validateNDESSCEPURLCalled) + assert.False(t, validateNDESSCEPAdminURLCalled) + assert.False(t, ds.HardDeleteMDMConfigAssetFuncInvoked) + ds.HardDeleteMDMConfigAssetFuncInvoked = false + assert.False(t, ds.NewActivityFuncInvoked) + ds.NewActivityFuncInvoked = false + // Cannot configure NDES without private key fleetConfig.Server.PrivateKey = "" svc, ctx = newTestServiceWithConfig(t, ds, fleetConfig, nil, nil, &TestServerOpts{License: &fleet.LicenseInfo{Tier: fleet.TierPremium}}) diff --git a/server/service/client.go b/server/service/client.go index 4eb260818d..247ba1b935 100644 --- a/server/service/client.go +++ b/server/service/client.go @@ -1256,6 +1256,10 @@ func (c *Client) DoGitOps( integrations = map[string]interface{}{} group.AppConfig.(map[string]interface{})["integrations"] = integrations } + integrations, ok = integrations.(map[string]interface{}) + if !ok { + return nil, errors.New("org_settings.integrations config is not a map") + } if jira, ok := integrations.(map[string]interface{})["jira"]; !ok || jira == nil { integrations.(map[string]interface{})["jira"] = []interface{}{} } @@ -1265,6 +1269,14 @@ func (c *Client) DoGitOps( if googleCal, ok := integrations.(map[string]interface{})["google_calendar"]; !ok || googleCal == nil { integrations.(map[string]interface{})["google_calendar"] = []interface{}{} } + if ndesSCEPProxy, ok := integrations.(map[string]interface{})["ndes_scep_proxy"]; !ok || ndesSCEPProxy == nil { + // Per backend patterns.md, best practice is to clear a JSON config field with `null` + integrations.(map[string]interface{})["ndes_scep_proxy"] = nil + } else { + if _, ok = ndesSCEPProxy.(map[string]interface{}); !ok { + return nil, errors.New("org_settings.integrations.ndes_scep_proxy config is not a map") + } + } // Ensure mdm config exists mdmConfig, ok := group.AppConfig.(map[string]interface{})["mdm"] diff --git a/server/service/client_test.go b/server/service/client_test.go index 7eb7314da1..aeea339561 100644 --- a/server/service/client_test.go +++ b/server/service/client_test.go @@ -1,12 +1,15 @@ package service import ( + "context" + "encoding/json" "os" "path/filepath" "testing" "github.com/fleetdm/fleet/v4/pkg/spec" "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -744,3 +747,40 @@ func TestGetProfilesContents(t *testing.T) { }) } } + +func TestGitOpsErrors(t *testing.T) { + t.Parallel() + ctx := context.Background() + client, err := NewClient("https://foo.bar", true, "", "") + require.NoError(t, err) + + tests := []struct { + name string + rawJSON string + wantErr string + }{ + { + name: "invalid integrations value", + rawJSON: `{ "integrations": false }`, + wantErr: "org_settings.integrations", + }, + { + name: "invalid ndes_scep_proxy value", + rawJSON: `{ "integrations": { "ndes_scep_proxy": [] } }`, + wantErr: "org_settings.integrations.ndes_scep_proxy", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := &spec.GitOps{} + config.OrgSettings = make(map[string]interface{}) + err = json.Unmarshal([]byte(tt.rawJSON), &config.OrgSettings) + require.NoError(t, err) + config.OrgSettings["secrets"] = []*fleet.EnrollSecret{} + _, err = client.DoGitOps(ctx, config, "/filename", nil, false, nil, nil, nil, nil) + assert.ErrorContains(t, err, tt.wantErr) + }) + } + +} diff --git a/server/service/testing_utils.go b/server/service/testing_utils.go index 68650f39a2..0df3089b43 100644 --- a/server/service/testing_utils.go +++ b/server/service/testing_utils.go @@ -38,6 +38,7 @@ import ( "github.com/fleetdm/fleet/v4/server/service/redis_lock" "github.com/fleetdm/fleet/v4/server/sso" "github.com/fleetdm/fleet/v4/server/test" + "github.com/go-kit/kit/log" kitlog "github.com/go-kit/log" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -387,6 +388,18 @@ func RunServerForTestsWithDS(t *testing.T, ds fleet.Datastore, opts ...*TestServ logger, ) require.NoError(t, err) + origValidateNDESSCEPURL := validateNDESSCEPURL + origValidateNDESSCEPAdminURL := validateNDESSCEPAdminURL + t.Cleanup(func() { + validateNDESSCEPURL = origValidateNDESSCEPURL + validateNDESSCEPAdminURL = origValidateNDESSCEPAdminURL + }) + validateNDESSCEPURL = func(_ context.Context, _ fleet.NDESSCEPProxyIntegration, _ log.Logger) error { + return nil + } + validateNDESSCEPAdminURL = func(_ context.Context, _ fleet.NDESSCEPProxyIntegration) error { + return nil + } } }